| Message ID | 20240206172845.34316-1-ivan@semenov.dev |
|---|---|
| State | New |
| Headers | show |
| Series | mmc: Fix eMMC initialization with 1-bit bus connection | expand |
On Tue, 6 Feb 2024 at 18:28, Ivan Semenov <ivan@semenov.dev> wrote: > > The Linux mmc driver currently encounters a problem with eMMC chips connected via a 1-bit bus, when the chip supports a 4-bit bus. This regression was introduced in commit 577fb13 after functioning correctly in kernel version 3.15. > > In the function mmc_select_bus_width, the driver attempts to switch to 8-bit and 4-bit modes, but if the 4-bit bus test fails, it does not fall back to 1-bit mode and leaves eMMC in broken 4-bit mode. This results in I/O errors and failure to read the partition table. > > This patch addresses the issue by ensuring that the driver fallback to 1-bit bus mode if the attempt to switch to 4-bit mode fails. > > dmesg log for Samsung eMMC 5.1 chip connected via 1bit bus (only D0 pin) before patch: Wow, that was an old bug you found there. Just to make sure I understand correctly, the platform only supports 1-bit bus, but the DTS doesn't reflect that correctly as it instead indicates that 4 and 8-bit modes are supported? Don't get me wrong, I think the initialization-error-path should be able to cope with that, so we should certainly fix it! > > [134509.044225] mmc0: switch to bus width 4 failed > [134509.044509] mmc0: new high speed MMC card at address 0001 > [134509.054594] mmcblk0: mmc0:0001 BGUF4R 29.1 GiB > [134509.281602] mmc0: switch to bus width 4 failed > [134509.282638] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2 > [134509.282657] Buffer I/O error on dev mmcblk0, logical block 0, async page read > [134509.284598] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2 > [134509.284602] Buffer I/O error on dev mmcblk0, logical block 0, async page read > [134509.284609] ldm_validate_partition_table(): Disk read failed. > [134509.286495] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2 > [134509.286500] Buffer I/O error on dev mmcblk0, logical block 0, async page read > [134509.288303] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2 > [134509.288308] Buffer I/O error on dev mmcblk0, logical block 0, async page read > [134509.289540] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2 > [134509.289544] Buffer I/O error on dev mmcblk0, logical block 0, async page read > [134509.289553] mmcblk0: unable to read partition table > [134509.289728] mmcblk0boot0: mmc0:0001 BGUF4R 31.9 MiB > [134509.290283] mmcblk0boot1: mmc0:0001 BGUF4R 31.9 MiB > [134509.294577] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x80700 phys_seg 1 prio class 2 > [134509.295835] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2 > [134509.295841] Buffer I/O error on dev mmcblk0, logical block 0, async page read > > After patch: > > [134551.089613] mmc0: switch to bus width 4 failed > [134551.090377] mmc0: new high speed MMC card at address 0001 > [134551.102271] mmcblk0: mmc0:0001 BGUF4R 29.1 GiB > [134551.113365] mmcblk0: p1 p2 p3 p4 p5 p6 p7 p8 p9 p10 p11 p12 p13 p14 p15 p16 p17 p18 p19 p20 p21 > [134551.114262] mmcblk0boot0: mmc0:0001 BGUF4R 31.9 MiB > [134551.114925] mmcblk0boot1: mmc0:0001 BGUF4R 31.9 MiB To allow me to apply the patch, you need to provide your Signed-off-by-tag. According to the below: Signed-off-by: Ivan Semenov <ivan@semenov.dev> Please tell me if you are okay with that, then I can amend the patch when applying. No need for you to post a new version. > --- > drivers/mmc/core/mmc.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > index f410bee50132..58ed7193a3ca 100644 > --- a/drivers/mmc/core/mmc.c > +++ b/drivers/mmc/core/mmc.c > @@ -1015,10 +1015,12 @@ static int mmc_select_bus_width(struct mmc_card *card) > static unsigned ext_csd_bits[] = { > EXT_CSD_BUS_WIDTH_8, > EXT_CSD_BUS_WIDTH_4, > + EXT_CSD_BUS_WIDTH_1, > }; > static unsigned bus_widths[] = { > MMC_BUS_WIDTH_8, > MMC_BUS_WIDTH_4, > + MMC_BUS_WIDTH_1, > }; > struct mmc_host *host = card->host; > unsigned idx, bus_width = 0; For my understanding, does your platform support HS200 mode too? Or only high-speed mode? The reason for my question is that it would be interesting to understand whether we end up running the error path in mmc_select_hs200() or not. [...] Kind regards Uffe
On Tue, Feb 13, 2024 at 6:41 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: > > On Tue, 6 Feb 2024 at 18:28, Ivan Semenov <ivan@semenov.dev> wrote: > > > > The Linux mmc driver currently encounters a problem with eMMC chips connected via a 1-bit bus, when the chip supports a 4-bit bus. This regression was introduced in commit 577fb13 after functioning correctly in kernel version 3.15. > > > > In the function mmc_select_bus_width, the driver attempts to switch to 8-bit and 4-bit modes, but if the 4-bit bus test fails, it does not fall back to 1-bit mode and leaves eMMC in broken 4-bit mode. This results in I/O errors and failure to read the partition table. > > > > This patch addresses the issue by ensuring that the driver fallback to 1-bit bus mode if the attempt to switch to 4-bit mode fails. > > > > dmesg log for Samsung eMMC 5.1 chip connected via 1bit bus (only D0 pin) before patch: > > Wow, that was an old bug you found there. > > Just to make sure I understand correctly, the platform only supports > 1-bit bus, but the DTS doesn't reflect that correctly as it instead > indicates that 4 and 8-bit modes are supported? Yes, you are correct, and the second case - with no DTS, when emmc is connected to PC via cardreader, but via 1bit connection only > Don't get me wrong, I think the initialization-error-path should be > able to cope with that, so we should certainly fix it! > > > > > [134509.044225] mmc0: switch to bus width 4 failed > > [134509.044509] mmc0: new high speed MMC card at address 0001 > > [134509.054594] mmcblk0: mmc0:0001 BGUF4R 29.1 GiB > > [134509.281602] mmc0: switch to bus width 4 failed > > [134509.282638] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2 > > [134509.282657] Buffer I/O error on dev mmcblk0, logical block 0, async page read > > [134509.284598] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2 > > [134509.284602] Buffer I/O error on dev mmcblk0, logical block 0, async page read > > [134509.284609] ldm_validate_partition_table(): Disk read failed. > > [134509.286495] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2 > > [134509.286500] Buffer I/O error on dev mmcblk0, logical block 0, async page read > > [134509.288303] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2 > > [134509.288308] Buffer I/O error on dev mmcblk0, logical block 0, async page read > > [134509.289540] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2 > > [134509.289544] Buffer I/O error on dev mmcblk0, logical block 0, async page read > > [134509.289553] mmcblk0: unable to read partition table > > [134509.289728] mmcblk0boot0: mmc0:0001 BGUF4R 31.9 MiB > > [134509.290283] mmcblk0boot1: mmc0:0001 BGUF4R 31.9 MiB > > [134509.294577] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x80700 phys_seg 1 prio class 2 > > [134509.295835] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2 > > [134509.295841] Buffer I/O error on dev mmcblk0, logical block 0, async page read > > > > After patch: > > > > [134551.089613] mmc0: switch to bus width 4 failed > > [134551.090377] mmc0: new high speed MMC card at address 0001 > > [134551.102271] mmcblk0: mmc0:0001 BGUF4R 29.1 GiB > > [134551.113365] mmcblk0: p1 p2 p3 p4 p5 p6 p7 p8 p9 p10 p11 p12 p13 p14 p15 p16 p17 p18 p19 p20 p21 > > [134551.114262] mmcblk0boot0: mmc0:0001 BGUF4R 31.9 MiB > > [134551.114925] mmcblk0boot1: mmc0:0001 BGUF4R 31.9 MiB > > To allow me to apply the patch, you need to provide your > Signed-off-by-tag. According to the below: > > Signed-off-by: Ivan Semenov <ivan@semenov.dev> > > Please tell me if you are okay with that, then I can amend the patch > when applying. No need for you to post a new version. I’m ok with that, is it ok to put it here or I need to send a patch again? Signed-off-by: Ivan Semenov <ivan@semenov.dev> > > > --- > > drivers/mmc/core/mmc.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > > index f410bee50132..58ed7193a3ca 100644 > > --- a/drivers/mmc/core/mmc.c > > +++ b/drivers/mmc/core/mmc.c > > @@ -1015,10 +1015,12 @@ static int mmc_select_bus_width(struct mmc_card *card) > > static unsigned ext_csd_bits[] = { > > EXT_CSD_BUS_WIDTH_8, > > EXT_CSD_BUS_WIDTH_4, > > + EXT_CSD_BUS_WIDTH_1, > > }; > > static unsigned bus_widths[] = { > > MMC_BUS_WIDTH_8, > > MMC_BUS_WIDTH_4, > > + MMC_BUS_WIDTH_1, > > }; > > struct mmc_host *host = card->host; > > unsigned idx, bus_width = 0; > > For my understanding, does your platform support HS200 mode too? Or > only high-speed mode? > > The reason for my question is that it would be interesting to > understand whether we end up running the error path in > mmc_select_hs200() or not. In my case, platform supports only High-Speed mode, so I can’t test in with HS200 mode. > > [...] > > Kind regards > Uffe >
> On Feb 13, 2024, at 18:40, Ulf Hansson <ulf.hansson@linaro.org> wrote: > > On Tue, 6 Feb 2024 at 18:28, Ivan Semenov <ivan@semenov.dev> wrote: >> >> The Linux mmc driver currently encounters a problem with eMMC chips connected via a 1-bit bus, when the chip supports a 4-bit bus. This regression was introduced in commit 577fb13 after functioning correctly in kernel version 3.15. >> >> In the function mmc_select_bus_width, the driver attempts to switch to 8-bit and 4-bit modes, but if the 4-bit bus test fails, it does not fall back to 1-bit mode and leaves eMMC in broken 4-bit mode. This results in I/O errors and failure to read the partition table. >> >> This patch addresses the issue by ensuring that the driver fallback to 1-bit bus mode if the attempt to switch to 4-bit mode fails. >> >> dmesg log for Samsung eMMC 5.1 chip connected via 1bit bus (only D0 pin) before patch: > > Wow, that was an old bug you found there. > > Just to make sure I understand correctly, the platform only supports > 1-bit bus, but the DTS doesn't reflect that correctly as it instead > indicates that 4 and 8-bit modes are supported? Yes, you are correct, and the second case - with no DTS, when emmc is connected to PC via cardreader, but via 1bit connection only > > Don't get me wrong, I think the initialization-error-path should be > able to cope with that, so we should certainly fix it! > >> >> [134509.044225] mmc0: switch to bus width 4 failed >> [134509.044509] mmc0: new high speed MMC card at address 0001 >> [134509.054594] mmcblk0: mmc0:0001 BGUF4R 29.1 GiB >> [134509.281602] mmc0: switch to bus width 4 failed >> [134509.282638] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2 >> [134509.282657] Buffer I/O error on dev mmcblk0, logical block 0, async page read >> [134509.284598] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2 >> [134509.284602] Buffer I/O error on dev mmcblk0, logical block 0, async page read >> [134509.284609] ldm_validate_partition_table(): Disk read failed. >> [134509.286495] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2 >> [134509.286500] Buffer I/O error on dev mmcblk0, logical block 0, async page read >> [134509.288303] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2 >> [134509.288308] Buffer I/O error on dev mmcblk0, logical block 0, async page read >> [134509.289540] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2 >> [134509.289544] Buffer I/O error on dev mmcblk0, logical block 0, async page read >> [134509.289553] mmcblk0: unable to read partition table >> [134509.289728] mmcblk0boot0: mmc0:0001 BGUF4R 31.9 MiB >> [134509.290283] mmcblk0boot1: mmc0:0001 BGUF4R 31.9 MiB >> [134509.294577] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x80700 phys_seg 1 prio class 2 >> [134509.295835] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2 >> [134509.295841] Buffer I/O error on dev mmcblk0, logical block 0, async page read >> >> After patch: >> >> [134551.089613] mmc0: switch to bus width 4 failed >> [134551.090377] mmc0: new high speed MMC card at address 0001 >> [134551.102271] mmcblk0: mmc0:0001 BGUF4R 29.1 GiB >> [134551.113365] mmcblk0: p1 p2 p3 p4 p5 p6 p7 p8 p9 p10 p11 p12 p13 p14 p15 p16 p17 p18 p19 p20 p21 >> [134551.114262] mmcblk0boot0: mmc0:0001 BGUF4R 31.9 MiB >> [134551.114925] mmcblk0boot1: mmc0:0001 BGUF4R 31.9 MiB > > To allow me to apply the patch, you need to provide your > Signed-off-by-tag. According to the below: > > Signed-off-by: Ivan Semenov <ivan@semenov.dev> > > Please tell me if you are okay with that, then I can amend the patch > when applying. No need for you to post a new version. I’m ok with that, is it ok to put it here or I need to send a patch again? Signed-off-by: Ivan Semenov <ivan@semenov.dev> > >> --- >> drivers/mmc/core/mmc.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >> index f410bee50132..58ed7193a3ca 100644 >> --- a/drivers/mmc/core/mmc.c >> +++ b/drivers/mmc/core/mmc.c >> @@ -1015,10 +1015,12 @@ static int mmc_select_bus_width(struct mmc_card *card) >> static unsigned ext_csd_bits[] = { >> EXT_CSD_BUS_WIDTH_8, >> EXT_CSD_BUS_WIDTH_4, >> + EXT_CSD_BUS_WIDTH_1, >> }; >> static unsigned bus_widths[] = { >> MMC_BUS_WIDTH_8, >> MMC_BUS_WIDTH_4, >> + MMC_BUS_WIDTH_1, >> }; >> struct mmc_host *host = card->host; >> unsigned idx, bus_width = 0; > > For my understanding, does your platform support HS200 mode too? Or > only high-speed mode? > > The reason for my question is that it would be interesting to > understand whether we end up running the error path in > mmc_select_hs200() or not. In my case, platform supports only High-Speed mode, so I can’t test in with HS200 mode. > > [...] > > Kind regards > Uffe
On Tue, 13 Feb 2024 at 18:01, Ivan Semenov <ivan@semenov.dev> wrote: > > On Tue, Feb 13, 2024 at 6:41 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: > > > > On Tue, 6 Feb 2024 at 18:28, Ivan Semenov <ivan@semenov.dev> wrote: > > > > > > The Linux mmc driver currently encounters a problem with eMMC chips connected via a 1-bit bus, when the chip supports a 4-bit bus. This regression was introduced in commit 577fb13 after functioning correctly in kernel version 3.15. > > > > > > In the function mmc_select_bus_width, the driver attempts to switch to 8-bit and 4-bit modes, but if the 4-bit bus test fails, it does not fall back to 1-bit mode and leaves eMMC in broken 4-bit mode. This results in I/O errors and failure to read the partition table. > > > > > > This patch addresses the issue by ensuring that the driver fallback to 1-bit bus mode if the attempt to switch to 4-bit mode fails. > > > > > > dmesg log for Samsung eMMC 5.1 chip connected via 1bit bus (only D0 pin) before patch: > > > > Wow, that was an old bug you found there. > > > > Just to make sure I understand correctly, the platform only supports > > 1-bit bus, but the DTS doesn't reflect that correctly as it instead > > indicates that 4 and 8-bit modes are supported? > > Yes, you are correct, and the second case - with no DTS, when emmc is > connected to PC via cardreader, but via 1bit connection only > > > Don't get me wrong, I think the initialization-error-path should be > > able to cope with that, so we should certainly fix it! > > > > > > > > [134509.044225] mmc0: switch to bus width 4 failed > > > [134509.044509] mmc0: new high speed MMC card at address 0001 > > > [134509.054594] mmcblk0: mmc0:0001 BGUF4R 29.1 GiB > > > [134509.281602] mmc0: switch to bus width 4 failed > > > [134509.282638] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2 > > > [134509.282657] Buffer I/O error on dev mmcblk0, logical block 0, async page read > > > [134509.284598] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2 > > > [134509.284602] Buffer I/O error on dev mmcblk0, logical block 0, async page read > > > [134509.284609] ldm_validate_partition_table(): Disk read failed. > > > [134509.286495] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2 > > > [134509.286500] Buffer I/O error on dev mmcblk0, logical block 0, async page read > > > [134509.288303] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2 > > > [134509.288308] Buffer I/O error on dev mmcblk0, logical block 0, async page read > > > [134509.289540] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2 > > > [134509.289544] Buffer I/O error on dev mmcblk0, logical block 0, async page read > > > [134509.289553] mmcblk0: unable to read partition table > > > [134509.289728] mmcblk0boot0: mmc0:0001 BGUF4R 31.9 MiB > > > [134509.290283] mmcblk0boot1: mmc0:0001 BGUF4R 31.9 MiB > > > [134509.294577] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x80700 phys_seg 1 prio class 2 > > > [134509.295835] I/O error, dev mmcblk0, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 2 > > > [134509.295841] Buffer I/O error on dev mmcblk0, logical block 0, async page read > > > > > > After patch: > > > > > > [134551.089613] mmc0: switch to bus width 4 failed > > > [134551.090377] mmc0: new high speed MMC card at address 0001 > > > [134551.102271] mmcblk0: mmc0:0001 BGUF4R 29.1 GiB > > > [134551.113365] mmcblk0: p1 p2 p3 p4 p5 p6 p7 p8 p9 p10 p11 p12 p13 p14 p15 p16 p17 p18 p19 p20 p21 > > > [134551.114262] mmcblk0boot0: mmc0:0001 BGUF4R 31.9 MiB > > > [134551.114925] mmcblk0boot1: mmc0:0001 BGUF4R 31.9 MiB > > > > To allow me to apply the patch, you need to provide your > > Signed-off-by-tag. According to the below: > > > > Signed-off-by: Ivan Semenov <ivan@semenov.dev> > > > > Please tell me if you are okay with that, then I can amend the patch > > when applying. No need for you to post a new version. > > I’m ok with that, is it ok to put it here or I need to send a patch again? > Signed-off-by: Ivan Semenov <ivan@semenov.dev> You don't need to re-send. Instead, I have amended the patch to add your sob-tag and while doing that I took the liberty of making some clarification to the commit message a bit too. Please have a look at my fixes/next branch and let me know if it doesn't look okay to you. So, applied for fixes, thanks! Kind regards Uffe > > > > > > --- > > > drivers/mmc/core/mmc.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > > > index f410bee50132..58ed7193a3ca 100644 > > > --- a/drivers/mmc/core/mmc.c > > > +++ b/drivers/mmc/core/mmc.c > > > @@ -1015,10 +1015,12 @@ static int mmc_select_bus_width(struct mmc_card *card) > > > static unsigned ext_csd_bits[] = { > > > EXT_CSD_BUS_WIDTH_8, > > > EXT_CSD_BUS_WIDTH_4, > > > + EXT_CSD_BUS_WIDTH_1, > > > }; > > > static unsigned bus_widths[] = { > > > MMC_BUS_WIDTH_8, > > > MMC_BUS_WIDTH_4, > > > + MMC_BUS_WIDTH_1, > > > }; > > > struct mmc_host *host = card->host; > > > unsigned idx, bus_width = 0; > > > > For my understanding, does your platform support HS200 mode too? Or > > only high-speed mode? > > > > The reason for my question is that it would be interesting to > > understand whether we end up running the error path in > > mmc_select_hs200() or not. > > In my case, platform supports only High-Speed mode, so I can’t test in > with HS200 mode. > > > > > [...] > > > > Kind regards > > Uffe > >
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index f410bee50132..58ed7193a3ca 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -1015,10 +1015,12 @@ static int mmc_select_bus_width(struct mmc_card *card) static unsigned ext_csd_bits[] = { EXT_CSD_BUS_WIDTH_8, EXT_CSD_BUS_WIDTH_4, + EXT_CSD_BUS_WIDTH_1, }; static unsigned bus_widths[] = { MMC_BUS_WIDTH_8, MMC_BUS_WIDTH_4, + MMC_BUS_WIDTH_1, }; struct mmc_host *host = card->host; unsigned idx, bus_width = 0;