| Message ID | 20190417204355.469-1-jbrunet@baylibre.com |
|---|---|
| Headers | show |
| Series | mmc: meson-gx: clean up and tuning update | expand |
On Wed, Apr 17, 2019 at 10:44 PM Jerome Brunet <jbrunet@baylibre.com> wrote: > > There is already a function available to poll a register until a > condition is met. Let's use it instead of open coding it. > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> Reviewed-by: Martin Blumenstingl<martin.blumenstingl@googlemail.com> and on my Khadas VIM with patches 1-4 from this series applied: Tested-by: Martin Blumenstingl<martin.blumenstingl@googlemail.com>
On Thu, 2019-04-18 at 22:16 +0200, Martin Blumenstingl wrote: > Hi Jerome, > > On Wed, Apr 17, 2019 at 10:44 PM 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> > it seems that this patch breaks SD card on my Khadas VIM and Khadas VIM2. The error I see in your logs is with eMMC and hs200, not SD card. Either way, There is something I don't really get. eMMC should not go through any DDR mode to reach HS200 (which is an SDR mode), neither should SD to reach HS. All this does is flipping the DDR bit (when necessary) when clock if off for the mmc device, avoiding a glitch on clk line. This patch should not make any difference for SDR only setup, Maybe I missed something, but I don't see how it could make anything different for SDR only. I (repeatedly) tested both vim1 and vim2, without seeing this issue, so I can't debug this. I'll need more detail to progress, something does not make sense here. > I used git bisect within this series to find that issue. > applying your .dts patches on top doesn't fix it > > two boot logs attached: > * kvim-broken.txt has patches 1-5 (= including this patch) applied > * kvim-working.txt has only patches 1-4 (= excluding this patch) applied > > > Regards > Martin
Hi Jerome, On Thu, Apr 18, 2019 at 10:46 PM Jerome Brunet <jbrunet@baylibre.com> wrote: > > On Thu, 2019-04-18 at 22:16 +0200, Martin Blumenstingl wrote: > > Hi Jerome, > > > > On Wed, Apr 17, 2019 at 10:44 PM 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> > > it seems that this patch breaks SD card on my Khadas VIM and Khadas VIM2. > > The error I see in your logs is with eMMC and hs200, not SD card. sorry, I should have been more clear that there are two errors: eMMC, this is what I have been seeing for a while on my Khadas VIM2 (it's probably not related to this patch): mmc1: mmc_select_hs200 failed, error -84 mmc1: error -84 whilst initialising MMC card however, then there's this other error: print_req_error: I/O error, dev mmcblk0, sector 0 flags 0 Buffer I/O error on dev mmcblk0, logical block 0, async page read as result of this the partition table cannot be read and my kernel cannot find the rootfs. > Either way, There is something I don't really get. eMMC should not go through > any DDR mode to reach HS200 (which is an SDR mode), neither should SD to reach > HS. > > All this does is flipping the DDR bit (when necessary) when clock if off for > the mmc device, avoiding a glitch on clk line. > > This patch should not make any difference for SDR only setup, Maybe I missed > something, but I don't see how it could make anything different for SDR only. > > I (repeatedly) tested both vim1 and vim2, without seeing this issue, so I can't > debug this. I'll need more detail to progress, something does not make sense here. please let me know from which part of the driver do you want debug logs Regards Martin
On Thu, 2019-04-18 at 22:53 +0200, Martin Blumenstingl wrote: > Hi Jerome, > > On Thu, Apr 18, 2019 at 10:46 PM Jerome Brunet <jbrunet@baylibre.com> wrote: > > On Thu, 2019-04-18 at 22:16 +0200, Martin Blumenstingl wrote: > > > Hi Jerome, > > > > > > On Wed, Apr 17, 2019 at 10:44 PM 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> > > > it seems that this patch breaks SD card on my Khadas VIM and Khadas VIM2. > > > > The error I see in your logs is with eMMC and hs200, not SD card. > sorry, I should have been more clear that there are two errors: > eMMC, this is what I have been seeing for a while on my Khadas VIM2 > (it's probably not related to this patch): > mmc1: mmc_select_hs200 failed, error -84 > mmc1: error -84 whilst initialising MMC card Following patches were also supposed to help the vim2. ... something I tested numerous time on this particular board. > > however, then there's this other error: > print_req_error: I/O error, dev mmcblk0, sector 0 flags 0 > Buffer I/O error on dev mmcblk0, logical block 0, async page read > as result of this the partition table cannot be read and my kernel > cannot find the rootfs. I don't know what the problem is (probably some CRC error - you can check the log in interrupt for this) but I'm pretty sure it is not related to this patch. I see in the log SD is indeed in HS mode, so not in any DDR mode. I also see that the SDIO fails as well. There something really weird, which can't be explained by this patch alone AFAICT. > > > Either way, There is something I don't really get. eMMC should not go through > > any DDR mode to reach HS200 (which is an SDR mode), neither should SD to reach > > HS. > > > > All this does is flipping the DDR bit (when necessary) when clock if off for > > the mmc device, avoiding a glitch on clk line. > > > > This patch should not make any difference for SDR only setup, Maybe I missed > > something, but I don't see how it could make anything different for SDR only. > > > > I (repeatedly) tested both vim1 and vim2, without seeing this issue, so I can't > > debug this. I'll need more detail to progress, something does not make sense here. > please let me know from which part of the driver do you want debug logs > > > Regards > Martin
On Thu, 2019-04-18 at 23:15 +0200, Jerome Brunet wrote: > On Thu, 2019-04-18 at 22:53 +0200, Martin Blumenstingl wrote: > > Hi Jerome, > > > > On Thu, Apr 18, 2019 at 10:46 PM Jerome Brunet <jbrunet@baylibre.com> wrote: > > > On Thu, 2019-04-18 at 22:16 +0200, Martin Blumenstingl wrote: > > > > Hi Jerome, > > > > > > > > On Wed, Apr 17, 2019 at 10:44 PM 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> > > > > it seems that this patch breaks SD card on my Khadas VIM and Khadas VIM2. > > > > > > The error I see in your logs is with eMMC and hs200, not SD card. > > sorry, I should have been more clear that there are two errors: > > eMMC, this is what I have been seeing for a while on my Khadas VIM2 > > (it's probably not related to this patch): > > mmc1: mmc_select_hs200 failed, error -84 > > mmc1: error -84 whilst initialising MMC card > > Following patches were also supposed to help the vim2. > ... something I tested numerous time on this particular board. > > > however, then there's this other error: > > print_req_error: I/O error, dev mmcblk0, sector 0 flags 0 > > Buffer I/O error on dev mmcblk0, logical block 0, async page read > > as result of this the partition table cannot be read and my kernel > > cannot find the rootfs. > > I don't know what the problem is (probably some CRC error - you can check the log > in interrupt for this) but I'm pretty sure it is not related to this patch. > > I see in the log SD is indeed in HS mode, so not in any DDR mode. > I also see that the SDIO fails as well. There something really weird, which can't > be explained by this patch alone AFAICT. Ok, I think I got it. It is indeed not linked to DDR, but a more trivial mistake. It looks like a writel went AWOL during the final rebase (and after the test campaign). in meson_mmc_set_ios, after: val |= FIELD_PREP(CFG_BUS_WIDTH_MASK, bus_width); Could you try adding writel(val, host->regs + SD_EMMC_CFG); Without this, the bus width is not changed, which explains why eMMC, SD and SDIO fail on init. Let me know if this helps.
Hi Jerome, On Fri, Apr 19, 2019 at 10:53 AM Jerome Brunet <jbrunet@baylibre.com> wrote: > > On Thu, 2019-04-18 at 23:15 +0200, Jerome Brunet wrote: > > On Thu, 2019-04-18 at 22:53 +0200, Martin Blumenstingl wrote: > > > Hi Jerome, > > > > > > On Thu, Apr 18, 2019 at 10:46 PM Jerome Brunet <jbrunet@baylibre.com> wrote: > > > > On Thu, 2019-04-18 at 22:16 +0200, Martin Blumenstingl wrote: > > > > > Hi Jerome, > > > > > > > > > > On Wed, Apr 17, 2019 at 10:44 PM 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> > > > > > it seems that this patch breaks SD card on my Khadas VIM and Khadas VIM2. > > > > > > > > The error I see in your logs is with eMMC and hs200, not SD card. > > > sorry, I should have been more clear that there are two errors: > > > eMMC, this is what I have been seeing for a while on my Khadas VIM2 > > > (it's probably not related to this patch): > > > mmc1: mmc_select_hs200 failed, error -84 > > > mmc1: error -84 whilst initialising MMC card > > > > Following patches were also supposed to help the vim2. > > ... something I tested numerous time on this particular board. > > > > > however, then there's this other error: > > > print_req_error: I/O error, dev mmcblk0, sector 0 flags 0 > > > Buffer I/O error on dev mmcblk0, logical block 0, async page read > > > as result of this the partition table cannot be read and my kernel > > > cannot find the rootfs. > > > > I don't know what the problem is (probably some CRC error - you can check the log > > in interrupt for this) but I'm pretty sure it is not related to this patch. > > > > I see in the log SD is indeed in HS mode, so not in any DDR mode. > > I also see that the SDIO fails as well. There something really weird, which can't > > be explained by this patch alone AFAICT. > > Ok, I think I got it. It is indeed not linked to DDR, but a more trivial mistake. > It looks like a writel went AWOL during the final rebase (and after the test campaign). > > in meson_mmc_set_ios, after: > val |= FIELD_PREP(CFG_BUS_WIDTH_MASK, bus_width); > > Could you try adding > writel(val, host->regs + SD_EMMC_CFG); good catch! > Without this, the bus width is not changed, which explains why eMMC, SD and SDIO fail > on init. I have added that missing writel on top of the whole series and now: - SD card is detected again (same as without this series) - SDIO wifi is detected again (same as without this series) - eMMC is detected (was not working before) I will give my Tested-by on v2 of these patches Martin