From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751165AbbJXFFN (ORCPT ); Sat, 24 Oct 2015 01:05:13 -0400 Received: from mailout4.samsung.com ([203.254.224.34]:32788 "EHLO mailout4.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750794AbbJXFFK (ORCPT ); Sat, 24 Oct 2015 01:05:10 -0400 X-AuditID: cbfee68d-f79ae6d00000149a-a0-562b1161ff95 Message-id: <562B0F44.1080007@samsung.com> Date: Sat, 24 Oct 2015 10:25:32 +0530 From: Alim Akhtar User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 MIME-version: 1.0 To: Doug Anderson , Krzysztof Kozlowski Cc: Javier Martinez Canillas , "linux-kernel@vger.kernel.org" , Markus Reichl , Anand Moon , linux-samsung-soc , Marek Szyprowski , "linux-mmc@vger.kernel.org" , Alexandre Courbot , Ulf Hansson , =?UTF-8?B?SGVpa28gU3TDvGJuZXI=?= Subject: Re: [PATCH] mmc: pwrseq: Use highest priority for eMMC restart handler References: <1445440540-21525-1-git-send-email-javier@osg.samsung.com> <56282F71.2070408@samsung.com> <562839F7.3040005@osg.samsung.com> <56283F27.9060804@samsung.com> In-reply-to: Content-type: text/plain; charset=UTF-8; format=flowed Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrDIsWRmVeSWpSXmKPExsWyRsSkRjdRUDvMYOJGBYvvD0+xWpxddpDN 4v+j16wWb96uYbJ4/cLQ4vKuOWwWR/73M1rMOL+PyWLdxlvsFi+P/GC0WHvkLrvF8bXhDjwe sxsusnjculPvsXPWXXaPO9f2sHn0Nr9j89jSD+T2bVnF6LH92jxmj8+b5AI4o7hsUlJzMstS i/TtErgyrnw4yVzQIlUx58wG5gbGfpEuRk4OCQETibtb3rFA2GISF+6tZ+ti5OIQEljBKDFz yWwWmKKfi+4zQiRmMUo8+L4OLCEk8IBRoq/NGMTmFdCSuLPsE1icRUBV4srFSWwgNpuAtsTd 6VuYuhg5OEQFIiQeXxCCKBeU+DH5Hli5iECUxPP2bUwgNrPAO2aJCa3xILawgL/Eigcd7BCr PjFKvP/PCWJzCgRLXJp2lw2i3kziUcs6ZghbXmLzmrfMIHdKCPRySKw4PJcV4h4BiW+TD7GA 3CAhICux6QAzxF+SEgdX3GCZwCg2C8lJs5CMnYVk7AJG5lWMoqkFyQXFSelFhnrFibnFpXnp esn5uZsYgZF8+t+z3h2Mtw9YH2IU4GBU4uHVWKQVJsSaWFZcmXuI0RToionMUqLJ+cB0kVcS b2hsZmRhamJqbGRuaaYkzqso9TNYSCA9sSQ1OzW1ILUovqg0J7X4ECMTB6dUA6Nkpqsfww6O vXuEl7bGz9yZHmykZswo17lY1PxmsVbMpsiVtziu2nbWO/nPPJUrWsj64fDmda9eflJgKnTQ vb/KIl+AS3nl69oEhX2Sh+u/vGa2fZtaozhh6qKTNkJljVsPyK+XXznvromO9rG4Lbxd/Ufa MucJZX+YMi8sLvz4Q03Tf0yCDkosxRmJhlrMRcWJAG/vb8DfAgAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprJKsWRmVeSWpSXmKPExsVy+t9jQd1EQe0wg1WnhS2+PzzFanF22UE2 i/+PXrNavHm7hsni9QtDi8u75rBZHPnfz2gx4/w+Jot1G2+xW7w88oPRYu2Ru+wWx9eGO/B4 zG64yOJx6069x85Zd9k97lzbw+bR2/yOzWNLP5Dbt2UVo8f2a/OYPT5vkgvgjGpgtMlITUxJ LVJIzUvOT8nMS7dV8g6Od443NTMw1DW0tDBXUshLzE21VXLxCdB1y8wBulhJoSwxpxQoFJBY XKykb4dpQmiIm64FTGOErm9IEFyPkQEaSFjDmHHlw0nmghapijlnNjA3MPaLdDFyckgImEj8 XHSfEcIWk7hwbz1bFyMXh5DALEaJB9/XsYAkhAQeMEr0tRmD2LwCWhJ3ln0Ci7MIqEpcuTiJ DcRmE9CWuDt9C1MXIweHqECExOMLQhDlghI/Jt8DKxcRiJJ43r6NCcRmFnjHLDGhNR7EFhbw l1jxoIMdYtUnRon3/zlBbE6BYIlL0+6yQdSbSTxqWccMYctLbF7zlnkCI9CRCCtmISmbhaRs ASPzKkaJ1ILkguKk9FzDvNRyveLE3OLSvHS95PzcTYzgZPFMagfjwV3uhxgFOBiVeHgvzNcK E2JNLCuuzD3EKMHBrCTCe3wGUIg3JbGyKrUoP76oNCe1+BCjKTAMJjJLiSbnAxNZXkm8obGJ uamxqaWJhYmZpZI474UMjTAhgfTEktTs1NSC1CKYPiYOTqkGRpbCoDdci9M45a1jX9/g/q9z I6Ov4r5C4YxV23428r78w73v+zthX12zRPej8od6pe6u+xYc8tpwX9uNKbqLCgKYtWTviPA/ uyOwO+Phpm191adaKtaaT9S7NLmS7cCHhX1f46eqaojsF9+voDzlsmHNvq+nvU2u+VWfZv5d 0XO0OCpv7attgUosxRmJhlrMRcWJAH3G4LQsAwAA DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/22/2015 09:04 PM, Doug Anderson wrote: > Krzysztof, > > On Wed, Oct 21, 2015 at 6:43 PM, Krzysztof Kozlowski > wrote: >> I think at least one platform may be affected because it used >> mmc-pwrseq-emmc and gpio-restart. >> >> Look at rk3288-veyron.dtsi. >> >> Both of restart handlers had the priority of 129 which means that the >> order of execution depends on probing sequence. Now you will make the >> sequence strict - first mmc then gpio. >> >> You seems convinced that this is not a problem... I don't know. I would >> prefer test this on affected platforms before risking to break them. >> It's annoying if fix for one SoC breaks another. > > Assuming I'm understanding things properly, veyron isn't using 129 as > a priority. In the dts file: > > gpio-restart { > compatible = "gpio-restart"; > gpios = <&gpio0 13 GPIO_ACTIVE_HIGH>; > pinctrl-names = "default"; > pinctrl-0 = <&ap_warm_reset_h>; > priority = <200>; > }; > > ...so it overrides the default 129 with 200. Ah, but Javier already > pointed that out in his reply. > >>> Since the current mmc_pwrseq_emmc_reset_nb notifier priority is 129, >>> eMMC reset will not work if one of the platforms you mentioned needs >>> this since the system restart handler with prio 192 will be executed >>> before the eMMC one, leaving the eMMC in an unknown state on reboot. >> >> And now you will "fix this" by making eMMC working correctly. So let's >> make it straight: >> 1. Previously the eMMC could be left on these platforms in an unknown >> state (because emmc handler was not executed). >> 2. No one complained! Which could mean that in fact this was working fine... >> 3. Now you will change it. >> 4. Maybe someone will complain? > > On veyron boards the reset shouldn't hurt. The eMMC reset will > actually get asserted at reset anyway since the reset will reset GPIO > states and the default GPIO state for the eMMC line asserts reset. > > OK, I just picked this onto Heiko's somewhat "stable-tree" > (v4.3-rc3-876-g6509232) from > . I put printouts in > __mmc_pwrseq_emmc_reset() to confirm it was getting called. I then > rebooted. I then saw: > > [ 36.175732] reboot: Restarting system > [ 36.179400] DOUG: resetting emmc > [ 36.182829] DOUG: resetting emmc done > > ...and the reboot worked normally (which means that the GPIO restart > got called since otherwise I would have gotten TPM errors). > > So I'd say that for rk3288-veyron-jerry: > > Tested-by: Douglas Anderson > Thank you! > > Note that personally I would only choose the "highest" priority as an > absolute last resort. Leaving a little extra slack in there means > that when the next person comes up with a really good reason to run > before you do that they can do it without changing your code. All > good BASIC programmers know to skip "10" in their first version for > just this reason. ;) > > If I were to pick a number, I suppose I'd pick something like "220", > but that's pretty arbitrary. I would have picked 200 except that it > appears that would race with veyron's choice. > > -Doug >