Messages in this thread | | | Date | Tue, 14 Feb 2023 04:10:00 -0800 | Subject | Re: [PATCH v6 3/3] watchdog: mt7621-wdt: avoid ralink architecture dependent code | From | Guenter Roeck <> |
| |
On 2/14/23 00:31, Krzysztof Kozlowski wrote: > On 13/02/2023 21:05, Sergio Paracuellos wrote: >> MT7621 SoC has a system controller node. Watchdog need to access to reset >> status register. Ralink architecture and related driver are old and from >> the beggining they are using some architecture dependent operations for >> accessing this shared registers through 'asm/mach-ralink/ralink_regs.h' >> header file. However this is not ideal from a driver perspective which can >> just access to the system controller registers in an arch independent way >> using regmap syscon APIs. Update Kconfig accordingly to select new added >> dependencies and allow driver to be compile tested. >> >> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com> >> --- >> drivers/watchdog/Kconfig | 4 +++- >> drivers/watchdog/mt7621_wdt.c | 18 +++++++++++++----- >> 2 files changed, 16 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig >> index b64bc49c7..cf752ad64 100644 >> --- a/drivers/watchdog/Kconfig >> +++ b/drivers/watchdog/Kconfig >> @@ -1865,7 +1865,9 @@ config GXP_WATCHDOG >> config MT7621_WDT >> tristate "Mediatek SoC watchdog" >> select WATCHDOG_CORE >> - depends on SOC_MT7620 || SOC_MT7621 >> + select REGMAP_MMIO >> + select MFD_SYSCON >> + depends on SOC_MT7620 || SOC_MT7621 || COMPILE_TEST >> help >> Hardware driver for the Mediatek/Ralink MT7621/8 SoC Watchdog Timer. >> >> diff --git a/drivers/watchdog/mt7621_wdt.c b/drivers/watchdog/mt7621_wdt.c >> index 40fb2c9ba..22e979bdd 100644 >> --- a/drivers/watchdog/mt7621_wdt.c >> +++ b/drivers/watchdog/mt7621_wdt.c >> @@ -15,8 +15,8 @@ >> #include <linux/moduleparam.h> >> #include <linux/platform_device.h> >> #include <linux/mod_devicetable.h> >> - >> -#include <asm/mach-ralink/ralink_regs.h> >> +#include <linux/mfd/syscon.h> >> +#include <linux/regmap.h> >> >> #define SYSC_RSTSTAT 0x38 >> #define WDT_RST_CAUSE BIT(1) >> @@ -34,6 +34,7 @@ >> struct mt7621_wdt_data { >> void __iomem *base; >> struct reset_control *rst; >> + struct regmap *sysc; >> struct watchdog_device wdt; >> }; >> >> @@ -104,9 +105,12 @@ static int mt7621_wdt_stop(struct watchdog_device *w) >> return 0; >> } >> >> -static int mt7621_wdt_bootcause(void) >> +static int mt7621_wdt_bootcause(struct mt7621_wdt_data *d) >> { >> - if (rt_sysc_r32(SYSC_RSTSTAT) & WDT_RST_CAUSE) >> + u32 val; >> + >> + regmap_read(d->sysc, SYSC_RSTSTAT, &val); >> + if (val & WDT_RST_CAUSE) >> return WDIOF_CARDRESET; >> >> return 0; >> @@ -143,6 +147,10 @@ static int mt7621_wdt_probe(struct platform_device *pdev) >> if (!drvdata) >> return -ENOMEM; >> >> + drvdata->sysc = syscon_regmap_lookup_by_compatible("mediatek,mt7621-sysc"); >> + if (IS_ERR(drvdata->sysc)) >> + return PTR_ERR(drvdata->sysc); > > This should be the backup/error path for original code using syscon > property. Looking up by compatible is really not portable/re-usable. >
I really disagree here.
$ git grep syscon_regmap_lookup_by_compatible | wc 90 326 8940
I have not yet reviewed this code, but I do not accept this argument against it.
Guenter
| |