| Message ID | 1446024965-8508-1-git-send-email-p.fedin@samsung.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 28.10.2015 18:36, Pavel Fedin wrote: > Since 8cfc7fdd33080e30b86d21b1a8c9ad0686427ddc ("ARM: EXYNOS: move restart This should be sufficient (although full is okay as well): $ git config core.abbrev 12 > code into pmu driver") PMU support is required in order for the reboot to > work. Unfortunately, there is currently no PMU support for 5410. > > This patch adds exynos5410-pmu to the list of recognized devices. It is > okay for the code to have empty struct exynos_pmu_data, in this case only > minimal support is enabled. This is enough for SWRESET to work. > > Signed-off-by: Pavel Fedin <p.fedin@samsung.com> This definitely looks like a fix for a regression, so: Cc: <stable@vger.kernel.org> Fixes: 8cfc7fdd3308 ("ARM: EXYNOS: move restart code into pmu driver") > --- > arch/arm/mach-exynos/pmu.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c > index de68938..7b9e33c 100644 > --- a/arch/arm/mach-exynos/pmu.c > +++ b/arch/arm/mach-exynos/pmu.c > @@ -908,6 +908,10 @@ static const struct exynos_pmu_data exynos5250_pmu_data = { > .powerdown_conf = exynos5_powerdown_conf, > }; > > +static struct exynos_pmu_data exynos5410_pmu_data = { > + /* Only SWRESET is currently supported on 5410 */ > +}; static const struct With these changes: Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Pavel, On 29 October 2015 at 07:11, Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote: > On 28.10.2015 18:36, Pavel Fedin wrote: >> Since 8cfc7fdd33080e30b86d21b1a8c9ad0686427ddc ("ARM: EXYNOS: move restart > > This should be sufficient (although full is okay as well): > $ git config core.abbrev 12 > >> code into pmu driver") PMU support is required in order for the reboot to >> work. Unfortunately, there is currently no PMU support for 5410. >> >> This patch adds exynos5410-pmu to the list of recognized devices. It is >> okay for the code to have empty struct exynos_pmu_data, in this case only >> minimal support is enabled. This is enough for SWRESET to work. >> >> Signed-off-by: Pavel Fedin <p.fedin@samsung.com> > > This definitely looks like a fix for a regression, so: > Cc: <stable@vger.kernel.org> > Fixes: 8cfc7fdd3308 ("ARM: EXYNOS: move restart code into pmu driver") > >> --- >> arch/arm/mach-exynos/pmu.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c >> index de68938..7b9e33c 100644 >> --- a/arch/arm/mach-exynos/pmu.c >> +++ b/arch/arm/mach-exynos/pmu.c >> @@ -908,6 +908,10 @@ static const struct exynos_pmu_data exynos5250_pmu_data = { >> .powerdown_conf = exynos5_powerdown_conf, >> }; >> >> +static struct exynos_pmu_data exynos5410_pmu_data = { >> + /* Only SWRESET is currently supported on 5410 */ >> +}; > > static const struct > > With these changes: > Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > > Patch looks ok, but after Alim's patch series [1] to handle poweroff and reboot via generic syscon based driver gets merged, this will become redundant entry into PMU driver in the absence of any PMU data for Exynos5410. Alim has taken care of adding power-off and reboot device node in exynos5410.dtsi [2]. [1]: http://www.spinics.net/lists/devicetree/msg98858.html [2]: http://www.spinics.net/lists/devicetree/msg98862.html Thanks, Pankaj Dubey > Best regards, > Krzysztof > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 29.10.2015 11:06, Pankaj Dubey wrote: > Hi Pavel, > > On 29 October 2015 at 07:11, Krzysztof Kozlowski > <k.kozlowski@samsung.com> wrote: >> On 28.10.2015 18:36, Pavel Fedin wrote: >>> Since 8cfc7fdd33080e30b86d21b1a8c9ad0686427ddc ("ARM: EXYNOS: move restart >> >> This should be sufficient (although full is okay as well): >> $ git config core.abbrev 12 >> >>> code into pmu driver") PMU support is required in order for the reboot to >>> work. Unfortunately, there is currently no PMU support for 5410. >>> >>> This patch adds exynos5410-pmu to the list of recognized devices. It is >>> okay for the code to have empty struct exynos_pmu_data, in this case only >>> minimal support is enabled. This is enough for SWRESET to work. >>> >>> Signed-off-by: Pavel Fedin <p.fedin@samsung.com> >> >> This definitely looks like a fix for a regression, so: >> Cc: <stable@vger.kernel.org> >> Fixes: 8cfc7fdd3308 ("ARM: EXYNOS: move restart code into pmu driver") >> >>> --- >>> arch/arm/mach-exynos/pmu.c | 7 +++++++ >>> 1 file changed, 7 insertions(+) >>> >>> diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c >>> index de68938..7b9e33c 100644 >>> --- a/arch/arm/mach-exynos/pmu.c >>> +++ b/arch/arm/mach-exynos/pmu.c >>> @@ -908,6 +908,10 @@ static const struct exynos_pmu_data exynos5250_pmu_data = { >>> .powerdown_conf = exynos5_powerdown_conf, >>> }; >>> >>> +static struct exynos_pmu_data exynos5410_pmu_data = { >>> + /* Only SWRESET is currently supported on 5410 */ >>> +}; >> >> static const struct >> >> With these changes: >> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> >> >> > > Patch looks ok, but after Alim's patch series [1] to handle poweroff > and reboot via generic syscon based driver gets merged, this will > become redundant entry into PMU driver in the absence of any PMU data > for Exynos5410. Alim has taken care of adding power-off and reboot > device node in exynos5410.dtsi [2]. > > [1]: http://www.spinics.net/lists/devicetree/msg98858.html > [2]: http://www.spinics.net/lists/devicetree/msg98862.html > Yes, current patch makes sense only if it gets into fixes for current RC and then backported to stable kernels. Alim's patchset won't be backported. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello! > Patch looks ok, but after Alim's patch series [1] to handle poweroff > and reboot via generic syscon based driver gets merged, this will > become redundant entry into PMU driver in the absence of any PMU data > for Exynos5410. Alim has taken care of adding power-off and reboot > device node in exynos5410.dtsi [2]. > > [1]: http://www.spinics.net/lists/devicetree/msg98858.html > [2]: http://www.spinics.net/lists/devicetree/msg98862.html I looked at those patches, IMHO they are too large for doing too small thing. What they do is basically describing two registers one-by-one and adding a separate per-register driver. Isn't it too much just for pushing a reset pin? May be we should just invent some generic "compatible" string, like "samsung,exynos-pmu" (or "Samsung,exynos-pmu-generic", if you want to be more specific), which would do the same as my patch currently does? This would be only several lines of code, achieving the same result. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Pavel, On Thursday 29 October 2015 12:40 PM, Pavel Fedin wrote: > Hello! > >> Patch looks ok, but after Alim's patch series [1] to handle poweroff >> and reboot via generic syscon based driver gets merged, this will >> become redundant entry into PMU driver in the absence of any PMU data >> for Exynos5410. Alim has taken care of adding power-off and reboot >> device node in exynos5410.dtsi [2]. >> >> [1]: http://www.spinics.net/lists/devicetree/msg98858.html >> [2]: http://www.spinics.net/lists/devicetree/msg98862.html > > I looked at those patches, IMHO they are too large for doing too small thing. What they do is basically describing two registers one-by-one and adding a separate per-register driver. Isn't it too much just for pushing a reset pin? > May be we should just invent some generic "compatible" string, like "samsung,exynos-pmu" (or "Samsung,exynos-pmu-generic", if you want to be more specific), which would do the same as my patch currently does? This would be only several lines of code, achieving the same result. > These patches are not adding any new code, except DT entry. Infact restart handler will be removed from PMU driver and we will be using generic syscon based implementation for reboot, which is already present in Linux kernel. Thus effectively reducing LoC. Only we need to add device node entries in respective DT files, and this is being done for all Exynos SoCs including exynos5410. Thanks, Pankaj Dubey > Kind regards, > Pavel Fedin > Expert Engineer > Samsung Electronics Research center Russia > > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello! > These patches are not adding any new code, except DT entry. Infact > restart handler will be removed from PMU driver and we will be using > generic syscon based implementation for reboot, which is already present > in Linux kernel. Ah, got it. So syscon is a very generic thing, allowing to describe whatever reset register for whatever board, since we are specifying both address and value to write there. Is my understanding correct? If so, then, OK, you can do this too. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday 29 October 2015 02:13 PM, Pavel Fedin wrote: > Hello! > >> These patches are not adding any new code, except DT entry. Infact >> restart handler will be removed from PMU driver and we will be using >> generic syscon based implementation for reboot, which is already present >> in Linux kernel. > > Ah, got it. So syscon is a very generic thing, allowing to describe whatever reset register for whatever board, since we are specifying both address and value to write there. Is my understanding correct? If so, then, OK, you can do this too. > Yes, syscon-reboot does exactly same. We just need to provide "syscon-provider" in our case it's PMU device node, register/offset and value to be written, and syscon-reboot driver will take care from registered restart-handler. Thanks, Pankaj Dubey > Kind regards, > Pavel Fedin > Expert Engineer > Samsung Electronics Research center Russia > > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c index de68938..7b9e33c 100644 --- a/arch/arm/mach-exynos/pmu.c +++ b/arch/arm/mach-exynos/pmu.c @@ -908,6 +908,10 @@ static const struct exynos_pmu_data exynos5250_pmu_data = { .powerdown_conf = exynos5_powerdown_conf, }; +static struct exynos_pmu_data exynos5410_pmu_data = { + /* Only SWRESET is currently supported on 5410 */ +}; + static struct exynos_pmu_data exynos5420_pmu_data = { .pmu_config = exynos5420_pmu_config, .pmu_init = exynos5420_pmu_init, @@ -934,6 +938,9 @@ static const struct of_device_id exynos_pmu_of_device_ids[] = { .compatible = "samsung,exynos5250-pmu", .data = &exynos5250_pmu_data, }, { + .compatible = "samsung,exynos5410-pmu", + .data = &exynos5410_pmu_data, + }, { .compatible = "samsung,exynos5420-pmu", .data = &exynos5420_pmu_data, },
Since 8cfc7fdd33080e30b86d21b1a8c9ad0686427ddc ("ARM: EXYNOS: move restart code into pmu driver") PMU support is required in order for the reboot to work. Unfortunately, there is currently no PMU support for 5410. This patch adds exynos5410-pmu to the list of recognized devices. It is okay for the code to have empty struct exynos_pmu_data, in this case only minimal support is enabled. This is enough for SWRESET to work. Signed-off-by: Pavel Fedin <p.fedin@samsung.com> --- arch/arm/mach-exynos/pmu.c | 7 +++++++ 1 file changed, 7 insertions(+)