lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-id: <562B0F44.1080007@samsung.com> Date: Sat, 24 Oct 2015 10:25:32 +0530 From: Alim Akhtar <alim.akhtar@...sung.com> To: Doug Anderson <dianders@...omium.org>, Krzysztof Kozlowski <k.kozlowski@...sung.com> Cc: Javier Martinez Canillas <javier@....samsung.com>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Markus Reichl <m.reichl@...etechno.de>, Anand Moon <linux.amoon@...il.com>, linux-samsung-soc <linux-samsung-soc@...r.kernel.org>, Marek Szyprowski <m.szyprowski@...sung.com>, "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>, Alexandre Courbot <acourbot@...dia.com>, Ulf Hansson <ulf.hansson@...aro.org>, Heiko Stübner <heiko@...ech.de> Subject: Re: [PATCH] mmc: pwrseq: Use highest priority for eMMC restart handler On 10/22/2015 09:04 PM, Doug Anderson wrote: > Krzysztof, > > On Wed, Oct 21, 2015 at 6:43 PM, Krzysztof Kozlowski > <k.kozlowski@...sung.com> 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 > <https://github.com/mmind/linux-rockchip/>. 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 <dianders@...omium.org> > 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists