diff mbox

mmc: block: Use the mmc host device index as the mmcblk device index

Message ID 1460015861-14004-1-git-send-email-ulf.hansson@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Ulf Hansson April 7, 2016, 7:57 a.m. UTC
Commit 520bd7a8b415 ("mmc: core: Optimize boot time by detecting cards
simultaneously") causes regressions for some platforms.

These platforms relies on fixed mmcblk device indexes, instead of
deploying the defacto standard with UUID/PARTUUID. In other words their
rootfs needs to be available at hardcoded paths, like /dev/mmcblk0p2.

Such guarantees have never been made by the kernel, but clearly the above
commit changes the behaviour. More precisely, because of that the order
changes of how cards becomes detected, so do their corresponding mmcblk
device indexes.

As the above commit significantly improves boot time for some platforms
(magnitude of seconds), let's avoid reverting this change but instead
restore the behaviour of how mmcblk device indexes becomes picked.

By using the same index for the mmcblk device as for the corresponding mmc
host device, the probe order of mmc host devices decides the index we get
for the mmcblk device.

For those platforms that suffers from a regression, one could expect that
this updated behaviour should be sufficient to meet their expectations of
"fixed" mmcblk device indexes.

Another side effect from this change, is that the same index is used for
the mmc host device, the mmcblk device and the mmc block queue. That
should clarify their relationship.

Reported-by: Peter Hurley <peter@hurleysoftware.com>
Reported-by: Laszlo Fiat <laszlo.fiat@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Fixes: 520bd7a8b415 ("mmc: core: Optimize boot time by detecting cards
simultaneously")
Cc: <stable@vger.kernel.org>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/card/block.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

Comments

Jaehoon Chung April 7, 2016, 12:07 p.m. UTC | #1
On 04/07/2016 04:57 PM, Ulf Hansson wrote:
> Commit 520bd7a8b415 ("mmc: core: Optimize boot time by detecting cards
> simultaneously") causes regressions for some platforms.
> 
> These platforms relies on fixed mmcblk device indexes, instead of
> deploying the defacto standard with UUID/PARTUUID. In other words their
> rootfs needs to be available at hardcoded paths, like /dev/mmcblk0p2.
> 
> Such guarantees have never been made by the kernel, but clearly the above
> commit changes the behaviour. More precisely, because of that the order
> changes of how cards becomes detected, so do their corresponding mmcblk
> device indexes.
> 
> As the above commit significantly improves boot time for some platforms
> (magnitude of seconds), let's avoid reverting this change but instead
> restore the behaviour of how mmcblk device indexes becomes picked.
> 
> By using the same index for the mmcblk device as for the corresponding mmc
> host device, the probe order of mmc host devices decides the index we get
> for the mmcblk device.
> 
> For those platforms that suffers from a regression, one could expect that
> this updated behaviour should be sufficient to meet their expectations of
> "fixed" mmcblk device indexes.
> 
> Another side effect from this change, is that the same index is used for
> the mmc host device, the mmcblk device and the mmc block queue. That
> should clarify their relationship.

I have tested with this patch..but there also are side-effects.
Exynos4 series has the two host controller..one is sdhci, one is dwmmc for eMMC.
In this case, dwmmc controller is probed after sdhci controller.

Then eMMC is always assigned to mmcblk1.

I think it's not good that make another problem for solving something.
It needs more discussion for this..

Best Regards,
Jaehoon Chung

> 
> Reported-by: Peter Hurley <peter@hurleysoftware.com>
> Reported-by: Laszlo Fiat <laszlo.fiat@gmail.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Fixes: 520bd7a8b415 ("mmc: core: Optimize boot time by detecting cards
> simultaneously")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/mmc/card/block.c | 18 +-----------------
>  1 file changed, 1 insertion(+), 17 deletions(-)
> 
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index 3bdbe50..8a0147d 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -86,7 +86,6 @@ static int max_devices;
>  
>  /* TODO: Replace these with struct ida */
>  static DECLARE_BITMAP(dev_use, MAX_DEVICES);
> -static DECLARE_BITMAP(name_use, MAX_DEVICES);
>  
>  /*
>   * There is one mmc_blk_data per slot.
> @@ -105,7 +104,6 @@ struct mmc_blk_data {
>  	unsigned int	usage;
>  	unsigned int	read_only;
>  	unsigned int	part_type;
> -	unsigned int	name_idx;
>  	unsigned int	reset_done;
>  #define MMC_BLK_READ		BIT(0)
>  #define MMC_BLK_WRITE		BIT(1)
> @@ -2202,19 +2200,6 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>  		goto out;
>  	}
>  
> -	/*
> -	 * !subname implies we are creating main mmc_blk_data that will be
> -	 * associated with mmc_card with dev_set_drvdata. Due to device
> -	 * partitions, devidx will not coincide with a per-physical card
> -	 * index anymore so we keep track of a name index.
> -	 */
> -	if (!subname) {
> -		md->name_idx = find_first_zero_bit(name_use, max_devices);
> -		__set_bit(md->name_idx, name_use);
> -	} else
> -		md->name_idx = ((struct mmc_blk_data *)
> -				dev_to_disk(parent)->private_data)->name_idx;
> -
>  	md->area_type = area_type;
>  
>  	/*
> @@ -2264,7 +2249,7 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>  	 */
>  
>  	snprintf(md->disk->disk_name, sizeof(md->disk->disk_name),
> -		 "mmcblk%u%s", md->name_idx, subname ? subname : "");
> +		 "mmcblk%u%s", card->host->index, subname ? subname : "");
>  
>  	if (mmc_card_mmc(card))
>  		blk_queue_logical_block_size(md->queue.queue,
> @@ -2418,7 +2403,6 @@ static void mmc_blk_remove_parts(struct mmc_card *card,
>  	struct list_head *pos, *q;
>  	struct mmc_blk_data *part_md;
>  
> -	__clear_bit(md->name_idx, name_use);
>  	list_for_each_safe(pos, q, &md->part) {
>  		part_md = list_entry(pos, struct mmc_blk_data, part);
>  		list_del(pos);
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson April 7, 2016, 1:07 p.m. UTC | #2
On 7 April 2016 at 14:07, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> On 04/07/2016 04:57 PM, Ulf Hansson wrote:
>> Commit 520bd7a8b415 ("mmc: core: Optimize boot time by detecting cards
>> simultaneously") causes regressions for some platforms.
>>
>> These platforms relies on fixed mmcblk device indexes, instead of
>> deploying the defacto standard with UUID/PARTUUID. In other words their
>> rootfs needs to be available at hardcoded paths, like /dev/mmcblk0p2.
>>
>> Such guarantees have never been made by the kernel, but clearly the above
>> commit changes the behaviour. More precisely, because of that the order
>> changes of how cards becomes detected, so do their corresponding mmcblk
>> device indexes.
>>
>> As the above commit significantly improves boot time for some platforms
>> (magnitude of seconds), let's avoid reverting this change but instead
>> restore the behaviour of how mmcblk device indexes becomes picked.
>>
>> By using the same index for the mmcblk device as for the corresponding mmc
>> host device, the probe order of mmc host devices decides the index we get
>> for the mmcblk device.
>>
>> For those platforms that suffers from a regression, one could expect that
>> this updated behaviour should be sufficient to meet their expectations of
>> "fixed" mmcblk device indexes.
>>
>> Another side effect from this change, is that the same index is used for
>> the mmc host device, the mmcblk device and the mmc block queue. That
>> should clarify their relationship.
>
> I have tested with this patch..but there also are side-effects.
> Exynos4 series has the two host controller..one is sdhci, one is dwmmc for eMMC.
> In this case, dwmmc controller is probed after sdhci controller.
>
> Then eMMC is always assigned to mmcblk1.

Okay, let me follow up with some questions.

1)
What is the sdhci controller used for?

2)
Are you seeing regressions for Exynos for this? I was under the
assumption that your vendor trees contained patches to deal with fixed
mmcblk ids?

3)
You proposed [1] recently to use aliases in DT to support fixed mmcblk
ids. I do realize that as UUID/PARTUUID sometimes can be a bit
cumbersome to use for an embedded system. Using aliases via DT seems
like a very good option. To implement that on top of $subject patch
should be quite easy to fix (I am happy to help with it). Would that
address your concerns?

>
> I think it's not good that make another problem for solving something.
> It needs more discussion for this..

Thanks for testing and your comments!

Kind regards
Uffe

[1]
http://www.spinics.net/lists/linux-mmc/msg35980.html
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Laszlo Fiat April 10, 2016, 6:04 p.m. UTC | #3
Hello,

I have tested this patch on top of v4.6-rc2 + our usual out-of-tree
patches from [1], and there is still no SDIO wifi showing up on
baytrail-t. dmesg is available at [2].

[1] https://github.com/hadess/rtl8723bs/tree/master/patches
[2] https://bugzilla.kernel.org/show_bug.cgi?id=112571#c9


Best regards,

Laszlo Fiat

On Thu, Apr 7, 2016 at 9:57 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> Commit 520bd7a8b415 ("mmc: core: Optimize boot time by detecting cards
> simultaneously") causes regressions for some platforms.
>
> These platforms relies on fixed mmcblk device indexes, instead of
> deploying the defacto standard with UUID/PARTUUID. In other words their
> rootfs needs to be available at hardcoded paths, like /dev/mmcblk0p2.
>
> Such guarantees have never been made by the kernel, but clearly the above
> commit changes the behaviour. More precisely, because of that the order
> changes of how cards becomes detected, so do their corresponding mmcblk
> device indexes.
>
> As the above commit significantly improves boot time for some platforms
> (magnitude of seconds), let's avoid reverting this change but instead
> restore the behaviour of how mmcblk device indexes becomes picked.
>
> By using the same index for the mmcblk device as for the corresponding mmc
> host device, the probe order of mmc host devices decides the index we get
> for the mmcblk device.
>
> For those platforms that suffers from a regression, one could expect that
> this updated behaviour should be sufficient to meet their expectations of
> "fixed" mmcblk device indexes.
>
> Another side effect from this change, is that the same index is used for
> the mmc host device, the mmcblk device and the mmc block queue. That
> should clarify their relationship.
>
> Reported-by: Peter Hurley <peter@hurleysoftware.com>
> Reported-by: Laszlo Fiat <laszlo.fiat@gmail.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Fixes: 520bd7a8b415 ("mmc: core: Optimize boot time by detecting cards
> simultaneously")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/mmc/card/block.c | 18 +-----------------
>  1 file changed, 1 insertion(+), 17 deletions(-)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index 3bdbe50..8a0147d 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -86,7 +86,6 @@ static int max_devices;
>
>  /* TODO: Replace these with struct ida */
>  static DECLARE_BITMAP(dev_use, MAX_DEVICES);
> -static DECLARE_BITMAP(name_use, MAX_DEVICES);
>
>  /*
>   * There is one mmc_blk_data per slot.
> @@ -105,7 +104,6 @@ struct mmc_blk_data {
>         unsigned int    usage;
>         unsigned int    read_only;
>         unsigned int    part_type;
> -       unsigned int    name_idx;
>         unsigned int    reset_done;
>  #define MMC_BLK_READ           BIT(0)
>  #define MMC_BLK_WRITE          BIT(1)
> @@ -2202,19 +2200,6 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>                 goto out;
>         }
>
> -       /*
> -        * !subname implies we are creating main mmc_blk_data that will be
> -        * associated with mmc_card with dev_set_drvdata. Due to device
> -        * partitions, devidx will not coincide with a per-physical card
> -        * index anymore so we keep track of a name index.
> -        */
> -       if (!subname) {
> -               md->name_idx = find_first_zero_bit(name_use, max_devices);
> -               __set_bit(md->name_idx, name_use);
> -       } else
> -               md->name_idx = ((struct mmc_blk_data *)
> -                               dev_to_disk(parent)->private_data)->name_idx;
> -
>         md->area_type = area_type;
>
>         /*
> @@ -2264,7 +2249,7 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>          */
>
>         snprintf(md->disk->disk_name, sizeof(md->disk->disk_name),
> -                "mmcblk%u%s", md->name_idx, subname ? subname : "");
> +                "mmcblk%u%s", card->host->index, subname ? subname : "");
>
>         if (mmc_card_mmc(card))
>                 blk_queue_logical_block_size(md->queue.queue,
> @@ -2418,7 +2403,6 @@ static void mmc_blk_remove_parts(struct mmc_card *card,
>         struct list_head *pos, *q;
>         struct mmc_blk_data *part_md;
>
> -       __clear_bit(md->name_idx, name_use);
>         list_for_each_safe(pos, q, &md->part) {
>                 part_md = list_entry(pos, struct mmc_blk_data, part);
>                 list_del(pos);
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson April 11, 2016, 8:25 a.m. UTC | #4
+ Adrian

On 10 April 2016 at 20:04, Laszlo Fiat <laszlo.fiat@gmail.com> wrote:
> Hello,
>

Hi Laszlo,

Thanks for your report!

> I have tested this patch on top of v4.6-rc2 + our usual out-of-tree
> patches from [1], and there is still no SDIO wifi showing up on
> baytrail-t. dmesg is available at [2].
>
> [1] https://github.com/hadess/rtl8723bs/tree/master/patches
> [2] https://bugzilla.kernel.org/show_bug.cgi?id=112571#c9
>

I understand this issue isn't for an upstream kernel, as you are
carrying out of tree patches to enable sdhci, sdhci-acpi and sdhci-pci
to work for your platform.

As my bandwidth is limited, normally I can't spend time on
non-upstream kernel reported regressions. Anyway, I decided to help
study the kernel log in a bit more detail, but unfortunate I am not
able to figure out what goes wrong.

From looking at the errors and a stack trace that gets dumped in the
kernel log, my guess is that the problem lies within sdhci/sdhci-acpi.
The errors seems related to your out of tree patches.

Regarding commit 520bd7a8b415 ("mmc: core: Optimize boot time by
detecting cards simultaneously"), even if it seems to be triggering
the regression for you, I find it highly unlikely that this is the
root cause.

Someone that knows Baytrail/sdhci-acpi needs to help out with
debugging. Perhaps Adrian or some other Intel folkz are available!?

A final note, $subject patch is suppose to fix problems for platforms
using hardcoded paths to rootfs. Your Baytrail platform don't need it,
as you are using UUID/PARTUUID, which is great!

Kind regards
Uffe

>
> Best regards,
>
> Laszlo Fiat
>
> On Thu, Apr 7, 2016 at 9:57 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> Commit 520bd7a8b415 ("mmc: core: Optimize boot time by detecting cards
>> simultaneously") causes regressions for some platforms.
>>
>> These platforms relies on fixed mmcblk device indexes, instead of
>> deploying the defacto standard with UUID/PARTUUID. In other words their
>> rootfs needs to be available at hardcoded paths, like /dev/mmcblk0p2.
>>
>> Such guarantees have never been made by the kernel, but clearly the above
>> commit changes the behaviour. More precisely, because of that the order
>> changes of how cards becomes detected, so do their corresponding mmcblk
>> device indexes.
>>
>> As the above commit significantly improves boot time for some platforms
>> (magnitude of seconds), let's avoid reverting this change but instead
>> restore the behaviour of how mmcblk device indexes becomes picked.
>>
>> By using the same index for the mmcblk device as for the corresponding mmc
>> host device, the probe order of mmc host devices decides the index we get
>> for the mmcblk device.
>>
>> For those platforms that suffers from a regression, one could expect that
>> this updated behaviour should be sufficient to meet their expectations of
>> "fixed" mmcblk device indexes.
>>
>> Another side effect from this change, is that the same index is used for
>> the mmc host device, the mmcblk device and the mmc block queue. That
>> should clarify their relationship.
>>
>> Reported-by: Peter Hurley <peter@hurleysoftware.com>
>> Reported-by: Laszlo Fiat <laszlo.fiat@gmail.com>
>> Cc: Linus Torvalds <torvalds@linux-foundation.org>
>> Fixes: 520bd7a8b415 ("mmc: core: Optimize boot time by detecting cards
>> simultaneously")
>> Cc: <stable@vger.kernel.org>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> ---
>>  drivers/mmc/card/block.c | 18 +-----------------
>>  1 file changed, 1 insertion(+), 17 deletions(-)
>>
>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>> index 3bdbe50..8a0147d 100644
>> --- a/drivers/mmc/card/block.c
>> +++ b/drivers/mmc/card/block.c
>> @@ -86,7 +86,6 @@ static int max_devices;
>>
>>  /* TODO: Replace these with struct ida */
>>  static DECLARE_BITMAP(dev_use, MAX_DEVICES);
>> -static DECLARE_BITMAP(name_use, MAX_DEVICES);
>>
>>  /*
>>   * There is one mmc_blk_data per slot.
>> @@ -105,7 +104,6 @@ struct mmc_blk_data {
>>         unsigned int    usage;
>>         unsigned int    read_only;
>>         unsigned int    part_type;
>> -       unsigned int    name_idx;
>>         unsigned int    reset_done;
>>  #define MMC_BLK_READ           BIT(0)
>>  #define MMC_BLK_WRITE          BIT(1)
>> @@ -2202,19 +2200,6 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>>                 goto out;
>>         }
>>
>> -       /*
>> -        * !subname implies we are creating main mmc_blk_data that will be
>> -        * associated with mmc_card with dev_set_drvdata. Due to device
>> -        * partitions, devidx will not coincide with a per-physical card
>> -        * index anymore so we keep track of a name index.
>> -        */
>> -       if (!subname) {
>> -               md->name_idx = find_first_zero_bit(name_use, max_devices);
>> -               __set_bit(md->name_idx, name_use);
>> -       } else
>> -               md->name_idx = ((struct mmc_blk_data *)
>> -                               dev_to_disk(parent)->private_data)->name_idx;
>> -
>>         md->area_type = area_type;
>>
>>         /*
>> @@ -2264,7 +2249,7 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>>          */
>>
>>         snprintf(md->disk->disk_name, sizeof(md->disk->disk_name),
>> -                "mmcblk%u%s", md->name_idx, subname ? subname : "");
>> +                "mmcblk%u%s", card->host->index, subname ? subname : "");
>>
>>         if (mmc_card_mmc(card))
>>                 blk_queue_logical_block_size(md->queue.queue,
>> @@ -2418,7 +2403,6 @@ static void mmc_blk_remove_parts(struct mmc_card *card,
>>         struct list_head *pos, *q;
>>         struct mmc_blk_data *part_md;
>>
>> -       __clear_bit(md->name_idx, name_use);
>>         list_for_each_safe(pos, q, &md->part) {
>>                 part_md = list_entry(pos, struct mmc_blk_data, part);
>>                 list_del(pos);
>> --
>> 1.9.1
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Adrian Hunter April 11, 2016, 12:46 p.m. UTC | #5
On 11/04/16 11:25, Ulf Hansson wrote:
> + Adrian
> 
> On 10 April 2016 at 20:04, Laszlo Fiat <laszlo.fiat@gmail.com> wrote:
>> Hello,
>>
> 
> Hi Laszlo,
> 
> Thanks for your report!
> 
>> I have tested this patch on top of v4.6-rc2 + our usual out-of-tree
>> patches from [1], and there is still no SDIO wifi showing up on
>> baytrail-t. dmesg is available at [2].
>>
>> [1] https://github.com/hadess/rtl8723bs/tree/master/patches
>> [2] https://bugzilla.kernel.org/show_bug.cgi?id=112571#c9
>>
> 
> I understand this issue isn't for an upstream kernel, as you are
> carrying out of tree patches to enable sdhci, sdhci-acpi and sdhci-pci
> to work for your platform.
> 
> As my bandwidth is limited, normally I can't spend time on
> non-upstream kernel reported regressions. Anyway, I decided to help
> study the kernel log in a bit more detail, but unfortunate I am not
> able to figure out what goes wrong.
> 
>>From looking at the errors and a stack trace that gets dumped in the
> kernel log, my guess is that the problem lies within sdhci/sdhci-acpi.
> The errors seems related to your out of tree patches.
> 
> Regarding commit 520bd7a8b415 ("mmc: core: Optimize boot time by
> detecting cards simultaneously"), even if it seems to be triggering
> the regression for you, I find it highly unlikely that this is the
> root cause.
> 
> Someone that knows Baytrail/sdhci-acpi needs to help out with
> debugging. Perhaps Adrian or some other Intel folkz are available!?

Need to know exactly what patches you have.  Is it only the ones from [1] above?

You could do an acpidump and also try the failing case with CONFIG_MMC_DEBUG=y.

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson April 12, 2016, 6:59 a.m. UTC | #6
On 7 April 2016 at 09:57, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> Commit 520bd7a8b415 ("mmc: core: Optimize boot time by detecting cards
> simultaneously") causes regressions for some platforms.
>
> These platforms relies on fixed mmcblk device indexes, instead of
> deploying the defacto standard with UUID/PARTUUID. In other words their
> rootfs needs to be available at hardcoded paths, like /dev/mmcblk0p2.
>
> Such guarantees have never been made by the kernel, but clearly the above
> commit changes the behaviour. More precisely, because of that the order
> changes of how cards becomes detected, so do their corresponding mmcblk
> device indexes.
>
> As the above commit significantly improves boot time for some platforms
> (magnitude of seconds), let's avoid reverting this change but instead
> restore the behaviour of how mmcblk device indexes becomes picked.
>
> By using the same index for the mmcblk device as for the corresponding mmc
> host device, the probe order of mmc host devices decides the index we get
> for the mmcblk device.
>
> For those platforms that suffers from a regression, one could expect that
> this updated behaviour should be sufficient to meet their expectations of
> "fixed" mmcblk device indexes.
>
> Another side effect from this change, is that the same index is used for
> the mmc host device, the mmcblk device and the mmc block queue. That
> should clarify their relationship.
>
> Reported-by: Peter Hurley <peter@hurleysoftware.com>
> Reported-by: Laszlo Fiat <laszlo.fiat@gmail.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Fixes: 520bd7a8b415 ("mmc: core: Optimize boot time by detecting cards
> simultaneously")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

To get this more thoroughly tested in linux-next, I have applied this
for my next branch.

I will be monitoring test reports from kernelci, but of course I
appreciate also any other feedback on this.

Kind regards
Uffe

> ---
>  drivers/mmc/card/block.c | 18 +-----------------
>  1 file changed, 1 insertion(+), 17 deletions(-)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index 3bdbe50..8a0147d 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -86,7 +86,6 @@ static int max_devices;
>
>  /* TODO: Replace these with struct ida */
>  static DECLARE_BITMAP(dev_use, MAX_DEVICES);
> -static DECLARE_BITMAP(name_use, MAX_DEVICES);
>
>  /*
>   * There is one mmc_blk_data per slot.
> @@ -105,7 +104,6 @@ struct mmc_blk_data {
>         unsigned int    usage;
>         unsigned int    read_only;
>         unsigned int    part_type;
> -       unsigned int    name_idx;
>         unsigned int    reset_done;
>  #define MMC_BLK_READ           BIT(0)
>  #define MMC_BLK_WRITE          BIT(1)
> @@ -2202,19 +2200,6 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>                 goto out;
>         }
>
> -       /*
> -        * !subname implies we are creating main mmc_blk_data that will be
> -        * associated with mmc_card with dev_set_drvdata. Due to device
> -        * partitions, devidx will not coincide with a per-physical card
> -        * index anymore so we keep track of a name index.
> -        */
> -       if (!subname) {
> -               md->name_idx = find_first_zero_bit(name_use, max_devices);
> -               __set_bit(md->name_idx, name_use);
> -       } else
> -               md->name_idx = ((struct mmc_blk_data *)
> -                               dev_to_disk(parent)->private_data)->name_idx;
> -
>         md->area_type = area_type;
>
>         /*
> @@ -2264,7 +2249,7 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>          */
>
>         snprintf(md->disk->disk_name, sizeof(md->disk->disk_name),
> -                "mmcblk%u%s", md->name_idx, subname ? subname : "");
> +                "mmcblk%u%s", card->host->index, subname ? subname : "");
>
>         if (mmc_card_mmc(card))
>                 blk_queue_logical_block_size(md->queue.queue,
> @@ -2418,7 +2403,6 @@ static void mmc_blk_remove_parts(struct mmc_card *card,
>         struct list_head *pos, *q;
>         struct mmc_blk_data *part_md;
>
> -       __clear_bit(md->name_idx, name_use);
>         list_for_each_safe(pos, q, &md->part) {
>                 part_md = list_entry(pos, struct mmc_blk_data, part);
>                 list_del(pos);
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Laszlo Fiat April 12, 2016, 6:15 p.m. UTC | #7
Adrian, Ulf,

All the mmc related patches are from [1] above. I have one extra patch
for rfkill-gpio to get bluetooth working, and one to ignore efi-bgrt,
but these are unrelated to mmc. Actually we can survive some time
without the out-of-tree mmc patches, but usually we freeze within 1-2
hours, that is why we need them.

I have compiled a v4.6-rc3 vanilla, without any additional patches and
with CONFIG_MMC_DEBUG=y, and created dmesg: [3]

I will compile the same one with the offending commit 520bd7a8b415
reverted, so that we can compare what happens differently.

For acpidump, which format do you need?

Thank you for looking into this.

Best regards,

Laszlo Fiat

[3]: https://bugzilla.kernel.org/show_bug.cgi?id=112571#c10

On Mon, Apr 11, 2016 at 2:46 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 11/04/16 11:25, Ulf Hansson wrote:
>> + Adrian
>>
>> On 10 April 2016 at 20:04, Laszlo Fiat <laszlo.fiat@gmail.com> wrote:
>>> Hello,
>>>
>>
>> Hi Laszlo,
>>
>> Thanks for your report!
>>
>>> I have tested this patch on top of v4.6-rc2 + our usual out-of-tree
>>> patches from [1], and there is still no SDIO wifi showing up on
>>> baytrail-t. dmesg is available at [2].
>>>
>>> [1] https://github.com/hadess/rtl8723bs/tree/master/patches
>>> [2] https://bugzilla.kernel.org/show_bug.cgi?id=112571#c9
>>>
>>
>> I understand this issue isn't for an upstream kernel, as you are
>> carrying out of tree patches to enable sdhci, sdhci-acpi and sdhci-pci
>> to work for your platform.
>>
>> As my bandwidth is limited, normally I can't spend time on
>> non-upstream kernel reported regressions. Anyway, I decided to help
>> study the kernel log in a bit more detail, but unfortunate I am not
>> able to figure out what goes wrong.
>>
>>>From looking at the errors and a stack trace that gets dumped in the
>> kernel log, my guess is that the problem lies within sdhci/sdhci-acpi.
>> The errors seems related to your out of tree patches.
>>
>> Regarding commit 520bd7a8b415 ("mmc: core: Optimize boot time by
>> detecting cards simultaneously"), even if it seems to be triggering
>> the regression for you, I find it highly unlikely that this is the
>> root cause.
>>
>> Someone that knows Baytrail/sdhci-acpi needs to help out with
>> debugging. Perhaps Adrian or some other Intel folkz are available!?
>
> Need to know exactly what patches you have.  Is it only the ones from [1] above?
>
> You could do an acpidump and also try the failing case with CONFIG_MMC_DEBUG=y.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Adrian Hunter April 13, 2016, 11:19 a.m. UTC | #8
On 12/04/16 21:15, Laszlo Fiat wrote:
> Adrian, Ulf,
> 
> All the mmc related patches are from [1] above. I have one extra patch
> for rfkill-gpio to get bluetooth working, and one to ignore efi-bgrt,
> but these are unrelated to mmc. Actually we can survive some time
> without the out-of-tree mmc patches, but usually we freeze within 1-2
> hours, that is why we need them.
> 
> I have compiled a v4.6-rc3 vanilla, without any additional patches and
> with CONFIG_MMC_DEBUG=y, and created dmesg: [3]

Looks like every command to the SDIO card fails.

> 
> I will compile the same one with the offending commit 520bd7a8b415
> reverted, so that we can compare what happens differently.
> 
> For acpidump, which format do you need?

Doesn't matter which format.

> 
> Thank you for looking into this.
> 
> Best regards,
> 
> Laszlo Fiat
> 
> [3]: https://bugzilla.kernel.org/show_bug.cgi?id=112571#c10
> 
> On Mon, Apr 11, 2016 at 2:46 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 11/04/16 11:25, Ulf Hansson wrote:
>>> + Adrian
>>>
>>> On 10 April 2016 at 20:04, Laszlo Fiat <laszlo.fiat@gmail.com> wrote:
>>>> Hello,
>>>>
>>>
>>> Hi Laszlo,
>>>
>>> Thanks for your report!
>>>
>>>> I have tested this patch on top of v4.6-rc2 + our usual out-of-tree
>>>> patches from [1], and there is still no SDIO wifi showing up on
>>>> baytrail-t. dmesg is available at [2].
>>>>
>>>> [1] https://github.com/hadess/rtl8723bs/tree/master/patches
>>>> [2] https://bugzilla.kernel.org/show_bug.cgi?id=112571#c9
>>>>
>>>
>>> I understand this issue isn't for an upstream kernel, as you are
>>> carrying out of tree patches to enable sdhci, sdhci-acpi and sdhci-pci
>>> to work for your platform.
>>>
>>> As my bandwidth is limited, normally I can't spend time on
>>> non-upstream kernel reported regressions. Anyway, I decided to help
>>> study the kernel log in a bit more detail, but unfortunate I am not
>>> able to figure out what goes wrong.
>>>
>>> >From looking at the errors and a stack trace that gets dumped in the
>>> kernel log, my guess is that the problem lies within sdhci/sdhci-acpi.
>>> The errors seems related to your out of tree patches.
>>>
>>> Regarding commit 520bd7a8b415 ("mmc: core: Optimize boot time by
>>> detecting cards simultaneously"), even if it seems to be triggering
>>> the regression for you, I find it highly unlikely that this is the
>>> root cause.
>>>
>>> Someone that knows Baytrail/sdhci-acpi needs to help out with
>>> debugging. Perhaps Adrian or some other Intel folkz are available!?
>>
>> Need to know exactly what patches you have.  Is it only the ones from [1] above?
>>
>> You could do an acpidump and also try the failing case with CONFIG_MMC_DEBUG=y.
>>
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Laszlo Fiat April 13, 2016, 6:19 p.m. UTC | #9
Adrian, Ulf,

Here are the two v4.6-rc3 kernel dmesg with mmc-debug option, both
without additional patches, one working case [1], one failing case
[2], the working case has patch [3] reverted.

I also uploaded the acpidump [4] for Teclast X80h Baytrail-T tablet.

[1]: https://bugzilla.kernel.org/show_bug.cgi?id=112571#c11
[2]: https://bugzilla.kernel.org/show_bug.cgi?id=112571#c10
[3]: patch 520bd7a8b4152aacfbd34eb7f7a447354b631039 ("mmc: core:
Optimize boot time by detecting cards simultaneously")
[4]: https://bugzilla.kernel.org/show_bug.cgi?id=112571#c12

On Wed, Apr 13, 2016 at 1:19 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 12/04/16 21:15, Laszlo Fiat wrote:
>> Adrian, Ulf,
>>
>> All the mmc related patches are from [1] above. I have one extra patch
>> for rfkill-gpio to get bluetooth working, and one to ignore efi-bgrt,
>> but these are unrelated to mmc. Actually we can survive some time
>> without the out-of-tree mmc patches, but usually we freeze within 1-2
>> hours, that is why we need them.
>>
>> I have compiled a v4.6-rc3 vanilla, without any additional patches and
>> with CONFIG_MMC_DEBUG=y, and created dmesg: [3]
>
> Looks like every command to the SDIO card fails.
>
>>
>> I will compile the same one with the offending commit 520bd7a8b415
>> reverted, so that we can compare what happens differently.
>>
>> For acpidump, which format do you need?
>
> Doesn't matter which format.
>
>>
>> Thank you for looking into this.
>>
>> Best regards,
>>
>> Laszlo Fiat
>>
>> [3]: https://bugzilla.kernel.org/show_bug.cgi?id=112571#c10
>>
>> On Mon, Apr 11, 2016 at 2:46 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>> On 11/04/16 11:25, Ulf Hansson wrote:
>>>> + Adrian
>>>>
>>>> On 10 April 2016 at 20:04, Laszlo Fiat <laszlo.fiat@gmail.com> wrote:
>>>>> Hello,
>>>>>
>>>>
>>>> Hi Laszlo,
>>>>
>>>> Thanks for your report!
>>>>
>>>>> I have tested this patch on top of v4.6-rc2 + our usual out-of-tree
>>>>> patches from [1], and there is still no SDIO wifi showing up on
>>>>> baytrail-t. dmesg is available at [2].
>>>>>
>>>>> [1] https://github.com/hadess/rtl8723bs/tree/master/patches
>>>>> [2] https://bugzilla.kernel.org/show_bug.cgi?id=112571#c9
>>>>>
>>>>
>>>> I understand this issue isn't for an upstream kernel, as you are
>>>> carrying out of tree patches to enable sdhci, sdhci-acpi and sdhci-pci
>>>> to work for your platform.
>>>>
>>>> As my bandwidth is limited, normally I can't spend time on
>>>> non-upstream kernel reported regressions. Anyway, I decided to help
>>>> study the kernel log in a bit more detail, but unfortunate I am not
>>>> able to figure out what goes wrong.
>>>>
>>>> >From looking at the errors and a stack trace that gets dumped in the
>>>> kernel log, my guess is that the problem lies within sdhci/sdhci-acpi.
>>>> The errors seems related to your out of tree patches.
>>>>
>>>> Regarding commit 520bd7a8b415 ("mmc: core: Optimize boot time by
>>>> detecting cards simultaneously"), even if it seems to be triggering
>>>> the regression for you, I find it highly unlikely that this is the
>>>> root cause.
>>>>
>>>> Someone that knows Baytrail/sdhci-acpi needs to help out with
>>>> debugging. Perhaps Adrian or some other Intel folkz are available!?
>>>
>>> Need to know exactly what patches you have.  Is it only the ones from [1] above?
>>>
>>> You could do an acpidump and also try the failing case with CONFIG_MMC_DEBUG=y.
>>>
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jaehoon Chung April 14, 2016, 1:25 a.m. UTC | #10
Hi Ulf,

Sorry for replying late.

On 04/07/2016 10:07 PM, Ulf Hansson wrote:
> On 7 April 2016 at 14:07, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> On 04/07/2016 04:57 PM, Ulf Hansson wrote:
>>> Commit 520bd7a8b415 ("mmc: core: Optimize boot time by detecting cards
>>> simultaneously") causes regressions for some platforms.
>>>
>>> These platforms relies on fixed mmcblk device indexes, instead of
>>> deploying the defacto standard with UUID/PARTUUID. In other words their
>>> rootfs needs to be available at hardcoded paths, like /dev/mmcblk0p2.
>>>
>>> Such guarantees have never been made by the kernel, but clearly the above
>>> commit changes the behaviour. More precisely, because of that the order
>>> changes of how cards becomes detected, so do their corresponding mmcblk
>>> device indexes.
>>>
>>> As the above commit significantly improves boot time for some platforms
>>> (magnitude of seconds), let's avoid reverting this change but instead
>>> restore the behaviour of how mmcblk device indexes becomes picked.
>>>
>>> By using the same index for the mmcblk device as for the corresponding mmc
>>> host device, the probe order of mmc host devices decides the index we get
>>> for the mmcblk device.
>>>
>>> For those platforms that suffers from a regression, one could expect that
>>> this updated behaviour should be sufficient to meet their expectations of
>>> "fixed" mmcblk device indexes.
>>>
>>> Another side effect from this change, is that the same index is used for
>>> the mmc host device, the mmcblk device and the mmc block queue. That
>>> should clarify their relationship.
>>
>> I have tested with this patch..but there also are side-effects.
>> Exynos4 series has the two host controller..one is sdhci, one is dwmmc for eMMC.
>> In this case, dwmmc controller is probed after sdhci controller.
>>
>> Then eMMC is always assigned to mmcblk1.
> 
> Okay, let me follow up with some questions.
> 
> 1)
> What is the sdhci controller used for?

Some of Exynos4 series have two MMC host controller. 
As i mentioned, one is sdhci, other is dwmmc.

sdhci controller can be used for eMMC/SD/SDIO. (emmc, T-flash, WiFi)
And dwmmc controller can be only used for eMMC.

It can choose which controller user uses.
In normal, i recommend the dwmmc controller is used for eMMC.
(There are some reason..I/O performance, and functionality.)

> 
> 2)
> Are you seeing regressions for Exynos for this? I was under the
> assumption that your vendor trees contained patches to deal with fixed
> mmcblk ids?

Actually not...but i need to check more..in vendor tress.

> 
> 3)
> You proposed [1] recently to use aliases in DT to support fixed mmcblk
> ids. I do realize that as UUID/PARTUUID sometimes can be a bit
> cumbersome to use for an embedded system. Using aliases via DT seems
> like a very good option. To implement that on top of $subject patch
> should be quite easy to fix (I am happy to help with it). Would that
> address your concerns?

I saw this patch was applied in your repository..So i will test with your tree.
In normal case, i think it's not problem...but i remembered there is a rare case about removable case.
I will try to test with all cases..:)

Best Regards,
Jaehoon Chung

> 
>>
>> I think it's not good that make another problem for solving something.
>> It needs more discussion for this..
> 
> Thanks for testing and your comments!
> 
> Kind regards
> Uffe
> 
> [1]
> http://www.spinics.net/lists/linux-mmc/msg35980.html
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson April 14, 2016, 7:26 a.m. UTC | #11
On 14 April 2016 at 03:25, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Hi Ulf,
>
> Sorry for replying late.
>
> On 04/07/2016 10:07 PM, Ulf Hansson wrote:
>> On 7 April 2016 at 14:07, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>> On 04/07/2016 04:57 PM, Ulf Hansson wrote:
>>>> Commit 520bd7a8b415 ("mmc: core: Optimize boot time by detecting cards
>>>> simultaneously") causes regressions for some platforms.
>>>>
>>>> These platforms relies on fixed mmcblk device indexes, instead of
>>>> deploying the defacto standard with UUID/PARTUUID. In other words their
>>>> rootfs needs to be available at hardcoded paths, like /dev/mmcblk0p2.
>>>>
>>>> Such guarantees have never been made by the kernel, but clearly the above
>>>> commit changes the behaviour. More precisely, because of that the order
>>>> changes of how cards becomes detected, so do their corresponding mmcblk
>>>> device indexes.
>>>>
>>>> As the above commit significantly improves boot time for some platforms
>>>> (magnitude of seconds), let's avoid reverting this change but instead
>>>> restore the behaviour of how mmcblk device indexes becomes picked.
>>>>
>>>> By using the same index for the mmcblk device as for the corresponding mmc
>>>> host device, the probe order of mmc host devices decides the index we get
>>>> for the mmcblk device.
>>>>
>>>> For those platforms that suffers from a regression, one could expect that
>>>> this updated behaviour should be sufficient to meet their expectations of
>>>> "fixed" mmcblk device indexes.
>>>>
>>>> Another side effect from this change, is that the same index is used for
>>>> the mmc host device, the mmcblk device and the mmc block queue. That
>>>> should clarify their relationship.
>>>
>>> I have tested with this patch..but there also are side-effects.
>>> Exynos4 series has the two host controller..one is sdhci, one is dwmmc for eMMC.
>>> In this case, dwmmc controller is probed after sdhci controller.
>>>
>>> Then eMMC is always assigned to mmcblk1.
>>
>> Okay, let me follow up with some questions.
>>
>> 1)
>> What is the sdhci controller used for?
>
> Some of Exynos4 series have two MMC host controller.
> As i mentioned, one is sdhci, other is dwmmc.
>
> sdhci controller can be used for eMMC/SD/SDIO. (emmc, T-flash, WiFi)

I see. So it seems the SDHCI controller can have SD cards (then
assuming those can be removable).

Prior commit 520bd7a8b415 ("mmc: core: Optimize boot time by detecting
cards simultaneously"), the picked mmcblk device index that
corresponded to the dwmmc controller, depended on whether the SD card
was inserted at boot or not.

My point is then, that fixed paths to the rootfs would work only for
one of the two scenarios; 1) The SD card is inserted at boot. 2) The
SD card isn't inserted at boot.

> And dwmmc controller can be only used for eMMC.
>
> It can choose which controller user uses.
> In normal, i recommend the dwmmc controller is used for eMMC.
> (There are some reason..I/O performance, and functionality.)
>
>>
>> 2)
>> Are you seeing regressions for Exynos for this? I was under the
>> assumption that your vendor trees contained patches to deal with fixed
>> mmcblk ids?
>
> Actually not...but i need to check more..in vendor tress.

Okay, thanks!

>
>>
>> 3)
>> You proposed [1] recently to use aliases in DT to support fixed mmcblk
>> ids. I do realize that as UUID/PARTUUID sometimes can be a bit
>> cumbersome to use for an embedded system. Using aliases via DT seems
>> like a very good option. To implement that on top of $subject patch
>> should be quite easy to fix (I am happy to help with it). Would that
>> address your concerns?
>
> I saw this patch was applied in your repository..So i will test with your tree.
> In normal case, i think it's not problem...but i remembered there is a rare case about removable case.
> I will try to test with all cases..:)

Okay, thanks for all you help!

[...]

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson May 16, 2016, 8:47 a.m. UTC | #12
+ Nicholas Krause

On 13 April 2016 at 20:19, Laszlo Fiat <laszlo.fiat@gmail.com> wrote:
> Adrian, Ulf,
>
> Here are the two v4.6-rc3 kernel dmesg with mmc-debug option, both
> without additional patches, one working case [1], one failing case
> [2], the working case has patch [3] reverted.
>
> I also uploaded the acpidump [4] for Teclast X80h Baytrail-T tablet.
>
> [1]: https://bugzilla.kernel.org/show_bug.cgi?id=112571#c11
> [2]: https://bugzilla.kernel.org/show_bug.cgi?id=112571#c10
> [3]: patch 520bd7a8b4152aacfbd34eb7f7a447354b631039 ("mmc: core:
> Optimize boot time by detecting cards simultaneously")
> [4]: https://bugzilla.kernel.org/show_bug.cgi?id=112571#c12
>

Adrian, did you manage to get some time to look into the debug data
from this issue?
I understand if you have been busy and no worries, although I don't
know who else to ping about this.

I don't want to us revert 520bd7a8b4152aacfbd34eb7f7a447354b631039,
because I believe it will just hide the problem in sdhci(-acpi).
Instead, I intend to wait for a proper fix, does that sounds
reasonable to you as well?

Kind regards
Uffe

> On Wed, Apr 13, 2016 at 1:19 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 12/04/16 21:15, Laszlo Fiat wrote:
>>> Adrian, Ulf,
>>>
>>> All the mmc related patches are from [1] above. I have one extra patch
>>> for rfkill-gpio to get bluetooth working, and one to ignore efi-bgrt,
>>> but these are unrelated to mmc. Actually we can survive some time
>>> without the out-of-tree mmc patches, but usually we freeze within 1-2
>>> hours, that is why we need them.
>>>
>>> I have compiled a v4.6-rc3 vanilla, without any additional patches and
>>> with CONFIG_MMC_DEBUG=y, and created dmesg: [3]
>>
>> Looks like every command to the SDIO card fails.
>>
>>>
>>> I will compile the same one with the offending commit 520bd7a8b415
>>> reverted, so that we can compare what happens differently.
>>>
>>> For acpidump, which format do you need?
>>
>> Doesn't matter which format.
>>
>>>
>>> Thank you for looking into this.
>>>
>>> Best regards,
>>>
>>> Laszlo Fiat
>>>
>>> [3]: https://bugzilla.kernel.org/show_bug.cgi?id=112571#c10
>>>
>>> On Mon, Apr 11, 2016 at 2:46 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>> On 11/04/16 11:25, Ulf Hansson wrote:
>>>>> + Adrian
>>>>>
>>>>> On 10 April 2016 at 20:04, Laszlo Fiat <laszlo.fiat@gmail.com> wrote:
>>>>>> Hello,
>>>>>>
>>>>>
>>>>> Hi Laszlo,
>>>>>
>>>>> Thanks for your report!
>>>>>
>>>>>> I have tested this patch on top of v4.6-rc2 + our usual out-of-tree
>>>>>> patches from [1], and there is still no SDIO wifi showing up on
>>>>>> baytrail-t. dmesg is available at [2].
>>>>>>
>>>>>> [1] https://github.com/hadess/rtl8723bs/tree/master/patches
>>>>>> [2] https://bugzilla.kernel.org/show_bug.cgi?id=112571#c9
>>>>>>
>>>>>
>>>>> I understand this issue isn't for an upstream kernel, as you are
>>>>> carrying out of tree patches to enable sdhci, sdhci-acpi and sdhci-pci
>>>>> to work for your platform.
>>>>>
>>>>> As my bandwidth is limited, normally I can't spend time on
>>>>> non-upstream kernel reported regressions. Anyway, I decided to help
>>>>> study the kernel log in a bit more detail, but unfortunate I am not
>>>>> able to figure out what goes wrong.
>>>>>
>>>>> >From looking at the errors and a stack trace that gets dumped in the
>>>>> kernel log, my guess is that the problem lies within sdhci/sdhci-acpi.
>>>>> The errors seems related to your out of tree patches.
>>>>>
>>>>> Regarding commit 520bd7a8b415 ("mmc: core: Optimize boot time by
>>>>> detecting cards simultaneously"), even if it seems to be triggering
>>>>> the regression for you, I find it highly unlikely that this is the
>>>>> root cause.
>>>>>
>>>>> Someone that knows Baytrail/sdhci-acpi needs to help out with
>>>>> debugging. Perhaps Adrian or some other Intel folkz are available!?
>>>>
>>>> Need to know exactly what patches you have.  Is it only the ones from [1] above?
>>>>
>>>> You could do an acpidump and also try the failing case with CONFIG_MMC_DEBUG=y.
>>>>
>>>
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Adrian Hunter May 16, 2016, 9:12 a.m. UTC | #13
On 16/05/16 11:47, Ulf Hansson wrote:
> + Nicholas Krause
> 
> On 13 April 2016 at 20:19, Laszlo Fiat <laszlo.fiat@gmail.com> wrote:
>> Adrian, Ulf,
>>
>> Here are the two v4.6-rc3 kernel dmesg with mmc-debug option, both
>> without additional patches, one working case [1], one failing case
>> [2], the working case has patch [3] reverted.
>>
>> I also uploaded the acpidump [4] for Teclast X80h Baytrail-T tablet.
>>
>> [1]: https://bugzilla.kernel.org/show_bug.cgi?id=112571#c11
>> [2]: https://bugzilla.kernel.org/show_bug.cgi?id=112571#c10
>> [3]: patch 520bd7a8b4152aacfbd34eb7f7a447354b631039 ("mmc: core:
>> Optimize boot time by detecting cards simultaneously")
>> [4]: https://bugzilla.kernel.org/show_bug.cgi?id=112571#c12
>>
> 
> Adrian, did you manage to get some time to look into the debug data
> from this issue?

Not yet.

> I understand if you have been busy and no worries, although I don't
> know who else to ping about this.

I will have a look today.

> 
> I don't want to us revert 520bd7a8b4152aacfbd34eb7f7a447354b631039,
> because I believe it will just hide the problem in sdhci(-acpi).
> Instead, I intend to wait for a proper fix, does that sounds
> reasonable to you as well?

Sure


--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 3bdbe50..8a0147d 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -86,7 +86,6 @@  static int max_devices;
 
 /* TODO: Replace these with struct ida */
 static DECLARE_BITMAP(dev_use, MAX_DEVICES);
-static DECLARE_BITMAP(name_use, MAX_DEVICES);
 
 /*
  * There is one mmc_blk_data per slot.
@@ -105,7 +104,6 @@  struct mmc_blk_data {
 	unsigned int	usage;
 	unsigned int	read_only;
 	unsigned int	part_type;
-	unsigned int	name_idx;
 	unsigned int	reset_done;
 #define MMC_BLK_READ		BIT(0)
 #define MMC_BLK_WRITE		BIT(1)
@@ -2202,19 +2200,6 @@  static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 		goto out;
 	}
 
-	/*
-	 * !subname implies we are creating main mmc_blk_data that will be
-	 * associated with mmc_card with dev_set_drvdata. Due to device
-	 * partitions, devidx will not coincide with a per-physical card
-	 * index anymore so we keep track of a name index.
-	 */
-	if (!subname) {
-		md->name_idx = find_first_zero_bit(name_use, max_devices);
-		__set_bit(md->name_idx, name_use);
-	} else
-		md->name_idx = ((struct mmc_blk_data *)
-				dev_to_disk(parent)->private_data)->name_idx;
-
 	md->area_type = area_type;
 
 	/*
@@ -2264,7 +2249,7 @@  static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 	 */
 
 	snprintf(md->disk->disk_name, sizeof(md->disk->disk_name),
-		 "mmcblk%u%s", md->name_idx, subname ? subname : "");
+		 "mmcblk%u%s", card->host->index, subname ? subname : "");
 
 	if (mmc_card_mmc(card))
 		blk_queue_logical_block_size(md->queue.queue,
@@ -2418,7 +2403,6 @@  static void mmc_blk_remove_parts(struct mmc_card *card,
 	struct list_head *pos, *q;
 	struct mmc_blk_data *part_md;
 
-	__clear_bit(md->name_idx, name_use);
 	list_for_each_safe(pos, q, &md->part) {
 		part_md = list_entry(pos, struct mmc_blk_data, part);
 		list_del(pos);