* Re: [PATCH 1/2] dt-bindings: watchdog: Add arm,smc-wdt watchdog arm,smc-wdt compatible [not found] ` <CAODwPW8WwntWb_=dg2J3AMy-gHw2QvNj_g98SufN13+AuGnUSg@mail.gmail.com> @ 2020-02-25 7:44 ` Xingyu Chen 2020-03-10 1:00 ` Evan Benn 0 siblings, 1 reply; 4+ messages in thread From: Xingyu Chen @ 2020-02-25 7:44 UTC (permalink / raw) To: Julius Werner Cc: Mark Rutland, Rob Herring, linux-watchdog, Jianxin Pan, devicetree, Greg Kroah-Hartman, LKML, Yonghui Yu, Evan Benn, Jonathan Cameron, Mauro Carvalho Chehab, open list:ARM/Amlogic Meson..., Wim Van Sebroeck, David S. Miller, Guenter Roeck Hi, Julius On 2020/2/25 9:23, Julius Werner wrote: >> The SMC function ID may be solved by the DTS, but the wdt indexs(Eg: >> SMCWD_INFO) are also different >> for each vendor. The imx_sc_wdt.c is also use the SMC to operate the >> WDT, but the wdt indexs(Eg: IMX_SIP_TIMER_START_WDOG) >> are different from ours. IMO, If the ATF can implement a common hal >> interface and index for watchdog, then writing a >> common smc wdt driver will be easier to compatible with all vendors. > The MediaTek driver is still in flux (e.g. still being reviewed in > Trusted Firmware here: > https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/3405), > we can still change it. So if we can now decide on making this a > "standard" driver, we can change the MediaTek interface to match IMX > and standardize on that. (There are existing Chromebooks shipped with > a different interface, but we could handle those separately with > downstream patches. I think having a unified interface that will > prevent this problem in the future would be worth some extra > complication right now.) If the ATF provides a common watchdog hal interface and index, I am happy to match the generic sec wdt driver. Compared to the current MTK wdt index [0], the following indexes need to be supported for meson wdt [1]. - *_INIT. - *_GETTIMEOUT. - *_RESETNOW. It is used to reset the system right now, similar to your SOFT RESET. For another platform-specific parameter "SMC function ID", the generic sec wdt driver can get it from the dts, but if the driver want to compatible with more vendors in the future, maybe we should consider Guenter's suggestion at [2] [0]: https://patchwork.kernel.org/patch/11395579/ [1]: https://patchwork.kernel.org/patch/11331271/ [2]: https://lore.kernel.org/linux-watchdog/20200220155159.GB29658@roeck-us.net/T/#md00328548222965054cd19ec7dda074f8fc09fe2 Best Regards > . _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] dt-bindings: watchdog: Add arm,smc-wdt watchdog arm,smc-wdt compatible 2020-02-25 7:44 ` [PATCH 1/2] dt-bindings: watchdog: Add arm,smc-wdt watchdog arm,smc-wdt compatible Xingyu Chen @ 2020-03-10 1:00 ` Evan Benn [not found] ` <ad28aa47-0490-937f-898f-0e4695ef6ec6@amlogic.com> 0 siblings, 1 reply; 4+ messages in thread From: Evan Benn @ 2020-03-10 1:00 UTC (permalink / raw) To: Xingyu Chen Cc: Mark Rutland, devicetree, LINUX-WATCHDOG, Jianxin Pan, Rob Herring, Greg Kroah-Hartman, LKML, Yonghui Yu, Jonathan Cameron, Mauro Carvalho Chehab, Julius Werner, open list:ARM/Amlogic Meson..., Wim Van Sebroeck, David S. Miller, Guenter Roeck Hi Xingyu, I am trying to establish some clarity about what to do here. The trusted firmware review has now been accepted https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/3405. I could try to add your mentioned extra operation indexes to the ATF watchdog, to try to establish a standard ATF smc watchdog interface. Hypothetically then your linux driver could connect to any of the ATF watchdogs, apart from the meson indirection layer. I do not quite understand the meson layer to be honest, would we run the meson layer on non-amlogic SOCs? It looks feasible to strip the meson part from your driver so that it works on more socs, please correct me if I am wrong. Alternatively we could also add these extra operation indexes to this linux driver. Unfortunately I would not have a way to test that. Thanks Evan On Tue, Feb 25, 2020 at 6:43 PM Xingyu Chen <xingyu.chen@amlogic.com> wrote: > > Hi, Julius > > On 2020/2/25 9:23, Julius Werner wrote: > >> The SMC function ID may be solved by the DTS, but the wdt indexs(Eg: > >> SMCWD_INFO) are also different > >> for each vendor. The imx_sc_wdt.c is also use the SMC to operate the > >> WDT, but the wdt indexs(Eg: IMX_SIP_TIMER_START_WDOG) > >> are different from ours. IMO, If the ATF can implement a common hal > >> interface and index for watchdog, then writing a > >> common smc wdt driver will be easier to compatible with all vendors. > > The MediaTek driver is still in flux (e.g. still being reviewed in > > Trusted Firmware here: > > https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/3405), > > we can still change it. So if we can now decide on making this a > > "standard" driver, we can change the MediaTek interface to match IMX > > and standardize on that. (There are existing Chromebooks shipped with > > a different interface, but we could handle those separately with > > downstream patches. I think having a unified interface that will > > prevent this problem in the future would be worth some extra > > complication right now.) > If the ATF provides a common watchdog hal interface and index, I am > happy to match > the generic sec wdt driver. Compared to the current MTK wdt index [0], > the following > indexes need to be supported for meson wdt [1]. > - *_INIT. > - *_GETTIMEOUT. > - *_RESETNOW. It is used to reset the system right now, similar to your > SOFT RESET. > > For another platform-specific parameter "SMC function ID", the generic > sec wdt driver can get it from the dts, but if > the driver want to compatible with more vendors in the future, maybe we > should consider Guenter's suggestion at [2] > > [0]: https://patchwork.kernel.org/patch/11395579/ > [1]: https://patchwork.kernel.org/patch/11331271/ > [2]: > https://lore.kernel.org/linux-watchdog/20200220155159.GB29658@roeck-us.net/T/#md00328548222965054cd19ec7dda074f8fc09fe2 > > Best Regards > > . _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <ad28aa47-0490-937f-898f-0e4695ef6ec6@amlogic.com>]
* Re: [PATCH 1/2] dt-bindings: watchdog: Add arm,smc-wdt watchdog arm,smc-wdt compatible [not found] ` <ad28aa47-0490-937f-898f-0e4695ef6ec6@amlogic.com> @ 2020-03-11 19:24 ` Julius Werner 2020-03-13 16:13 ` Xingyu Chen 0 siblings, 1 reply; 4+ messages in thread From: Julius Werner @ 2020-03-11 19:24 UTC (permalink / raw) To: Xingyu Chen Cc: Mark Rutland, devicetree, LINUX-WATCHDOG, Jianxin Pan, Rob Herring, Greg Kroah-Hartman, LKML, Mauro Carvalho Chehab, Evan Benn, Jonathan Cameron, Yonghui Yu, Julius Werner, open list:ARM/Amlogic Meson..., Wim Van Sebroeck, David S. Miller, Guenter Roeck > - *_INIT and *GETTIMEOUT. Although your driver does not need them, could you take them as options in your driver ? The driver already has SMCWD_INFO which is used during probe to retrieve the minimum and maximum timeout values supported by the hardware at probe time. Maybe it would make sense to rename that to INIT (which would still return those values, but can also do whatever initialization needs to be done in TF)? GETTIMELEFT I agree we can implement optionally, and other platforms would just return a PSCI_RET_NOT_SUPPORTED for that. > - *_RESETNOW. It is used to reset the system right now, similar to your SOFT RESET. could you reserve an operation index in ATF ? Just curious, why do you need this? Shouldn't you use the PSCI standard SYSTEM_RESET SMC for that? (If you want to control exactly how the platform is reset, you could also use SYSTEM_RESET2 with a vendor-defined reset_type.) _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] dt-bindings: watchdog: Add arm,smc-wdt watchdog arm,smc-wdt compatible 2020-03-11 19:24 ` Julius Werner @ 2020-03-13 16:13 ` Xingyu Chen 0 siblings, 0 replies; 4+ messages in thread From: Xingyu Chen @ 2020-03-13 16:13 UTC (permalink / raw) To: Julius Werner Cc: Mark Rutland, devicetree, LINUX-WATCHDOG, Jianxin Pan, Rob Herring, Greg Kroah-Hartman, LKML, Mauro Carvalho Chehab, Evan Benn, Jonathan Cameron, Yonghui Yu, open list:ARM/Amlogic Meson..., Wim Van Sebroeck, David S. Miller, Guenter Roeck Hi, Julius On 2020/3/12 3:24, Julius Werner wrote: >> - *_INIT and *GETTIMEOUT. Although your driver does not need them, could you take them as options in your driver ? > The driver already has SMCWD_INFO which is used during probe to > retrieve the minimum and maximum timeout values supported by the > hardware at probe time. Maybe it would make sense to rename that to > INIT (which would still return those values, but can also do whatever > initialization needs to be done in TF)? Yes,INIT would make sense for me. > GETTIMELEFT I agree we can > implement optionally, and other platforms would just return a > PSCI_RET_NOT_SUPPORTED for that. > >> - *_RESETNOW. It is used to reset the system right now, similar to your SOFT RESET. could you reserve an operation index in ATF ? > Just curious, why do you need this? Shouldn't you use the PSCI > standard SYSTEM_RESET SMC for that? (If you want to control exactly > how the platform is reset, you could also use SYSTEM_RESET2 with a > vendor-defined reset_type.) I just wanted it to be compatible with other OS,and I think it over, maybe I can also use the PSCI interface to execuate the system reset on the other OS. Anyway, please ignore this request. Thanks. > > . _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-13 16:13 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20200214062637.216209-1-evanbenn@chromium.org> [not found] ` <20200214172512.1.I02ebc5b8743b1a71e0e15f68ea77e506d4e6f840@changeid> [not found] ` <20200219223046.GA16537@bogus> [not found] ` <CAODwPW8JspiUtyU4CC95w9rbNRyUF-Aeb9TuPm1PzmP6u=y1EA@mail.gmail.com> [not found] ` <20200219232005.GA9737@roeck-us.net> [not found] ` <CAKz_xw2hvHL=a4s37dmuCTWDbxefQFR3rfcaNiWYJY4T+jqabA@mail.gmail.com> [not found] ` <e42320b8-266f-0b0e-b20b-b72228510e81@amlogic.com> [not found] ` <CAODwPW94KX46PzSrf_uuEFPKudXor=26d=g3Qta5veRfxmMDUA@mail.gmail.com> [not found] ` <1326f594-3cfd-c03d-4f2c-50eeb75724b2@amlogic.com> [not found] ` <CAODwPW8WwntWb_=dg2J3AMy-gHw2QvNj_g98SufN13+AuGnUSg@mail.gmail.com> 2020-02-25 7:44 ` [PATCH 1/2] dt-bindings: watchdog: Add arm,smc-wdt watchdog arm,smc-wdt compatible Xingyu Chen 2020-03-10 1:00 ` Evan Benn [not found] ` <ad28aa47-0490-937f-898f-0e4695ef6ec6@amlogic.com> 2020-03-11 19:24 ` Julius Werner 2020-03-13 16:13 ` Xingyu Chen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).