Re: [PATCH] mmc: pwrseq: Use highest priority for eMMC restart handler

[Javier Martinez Canillas]

Hello Marek,

On 10/22/2015 12:07 PM, Marek Szyprowski wrote:
> On 2015-10-22 06:14, Alim Akhtar wrote:
>> On 10/22/2015 08:22 AM, Javier Martinez Canillas wrote:
>>> On 10/22/2015 03:43 AM, Krzysztof Kozlowski wrote:
>>>> On 22.10.2015 10:20, Javier Martinez Canillas wrote:> Hello Krzysztof,

[snip]

>>>>> And $SUBJECT should not cause any regressions for the platforms that
>>>>> are currently using the pwrseq_emmc, since the restart handler was
>>>>> already being called before the system restart handler so bumping
>>>>> the priority should not cause any effect.
>>>>
>>>> I found at least one platform where the sequence *might* change. There
>>>> could be more of them.
>>>>
>>>
>>> Agreed, I missed that rk3288-veyron is using a restart handler with higher
>>> priority and could be other boards too as you said.
>>>
>>> Let's see what is Marek's opinion since he added the pwrseq_emmc support
>>> and also what Ulf thinks about always doing a eMMC reset before reboot.
>>>
>>> I can't think how doing a eMMC card reset before reboot could affect a
>>> board but you are right that we don't know without testing.
>>>
>> git grep result shows:
>> ======
>>  git grep mmc-pwrseq-emmc
>> Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt:- compatible : contains "mmc-pwrseq-emmc".
>> Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt: compatible = "mmc-pwrseq-emmc";
>> arch/arm/boot/dts/am335x-sl50.dts:              compatible = "mmc-pwrseq-emmc";
>> arch/arm/boot/dts/exynos4412-odroid-common.dtsi: compatible = "mmc-pwrseq-emmc";
>> arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi: compatible = "mmc-pwrseq-emmc";
>> arch/arm/boot/dts/rk3288-veyron.dtsi:           compatible = "mmc-pwrseq-emmc";
>> arch/arm64/boot/dts/rockchip/rk3368-r88.dts:            compatible = "mmc-pwrseq-emmc";
>> drivers/mmc/core/pwrseq.c:              .compatible = "mmc-pwrseq-emmc",
>> =====
>> So apart from exynos, there are rk3xxx and am335x which used pwrseq-emmc-restart. So lets wait for the feedback from these guys as well.
>> Thanks.
>>
> 
> The priority was initially chosen in such a way to do the emmc reset just before performing system reboot. I wanted let other possible handlers potentially use mmc if needed (although there is no such case atm). Now it turns that this approach was not the best idea, because there are other board-specific restart handlers with higher priority than the default I was using. IMHO the change proposed by Javier seems to be the best solution for now. The other possibility would be to entirely get rid of restart handler usage and wire this logic to mmc_shutdown(). This makes sense from the logical point of view, but the drawback of this solution is the lack of proper reset sequence in case of emergency reboot (shutdown callbacks are not called on emergency reboot).
>

Thanks a lot for your feedback, I'm glad that you agree with the change then.
 
> Best regards

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/