| Message ID | 20230213200519.889503-1-sergio.paracuellos@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | watchdog: mt7621-wdt: avoid globals and arch dependencies | expand |
On 13.02.2023 23:05, Sergio Paracuellos wrote: > Hi all, > > This series make an update in the MT7621 SoC's watchdog driver code. This > SoC already provides a system controller node to access reset status > register needed for the watchdog. Instead of using MIPS architecture > dependent operations in header 'asm/mach-ralink/ralink_regs.h' get > node syscon regmap using 'syscon_regmap_lookup_by_compatible()' and use. > regmap APIS. Driver is also using some globals that are not needed at > all if a driver data structure is used along the code. Hence, add all > new needed stuff inside a new driver data structure. With this changes > driver can be properly compile tested. > > Thanks in advance for reviewing this! > > v1 of this series here [0]. > v2 of this series here [1]. > v3 of this series here [2]. > v4 of this series here [3]. > v5 of this series here [4]. > > Changes in v6: > - Get regmap syscon using 'syscon_regmap_lookup_by_compatible()' > - Add COMPILE_TEST and dependencies to Kconfig. > - Collect Philippe Mathieu-Daudé 'Reviewed-by' tag for watchdog node > warning fix. > > Changes in v5: > - Drop patches related with device tree ABI breakage and only > maintain the rest. This makes me regret taking mt7621-dts out of drivers/staging. Clearly there were improvements to be made on the MT7621 drivers that would affect the devicetree binding. I don't think one would bat an eye to make dt-binding changes on a devicetree on drivers/staging. I also want to make a similar change that would break the ABI. I want to split the MediaTek MIPS (currently called Ralink) pinctrl subdrivers further, namely mt7620.c, to split MT7628/MT7688 pinmux data from MT7620, so I can properly document which function can be given to which group(s) for the MT7628/MT7688 SoCs. This means I've got to introduce a new compatible string for the MT76X8 SoCs which would cause the pinctrl driver to stop working with an old DT for the MT76X8 SoCs. By the nature of the change, I can't make a way around with the code like Sergio did in v6. Arınç
On 14.02.2023 11:12, Arınç ÜNAL wrote: > On 13.02.2023 23:05, Sergio Paracuellos wrote: >> Hi all, >> >> This series make an update in the MT7621 SoC's watchdog driver code. This >> SoC already provides a system controller node to access reset status >> register needed for the watchdog. Instead of using MIPS architecture >> dependent operations in header 'asm/mach-ralink/ralink_regs.h' get >> node syscon regmap using 'syscon_regmap_lookup_by_compatible()' and use. >> regmap APIS. Driver is also using some globals that are not needed at >> all if a driver data structure is used along the code. Hence, add all >> new needed stuff inside a new driver data structure. With this changes >> driver can be properly compile tested. >> >> Thanks in advance for reviewing this! >> >> v1 of this series here [0]. >> v2 of this series here [1]. >> v3 of this series here [2]. >> v4 of this series here [3]. >> v5 of this series here [4]. >> >> Changes in v6: >> - Get regmap syscon using 'syscon_regmap_lookup_by_compatible()' >> - Add COMPILE_TEST and dependencies to Kconfig. >> - Collect Philippe Mathieu-Daudé 'Reviewed-by' tag for watchdog node >> warning fix. >> >> Changes in v5: >> - Drop patches related with device tree ABI breakage and only >> maintain the rest. > > This makes me regret taking mt7621-dts out of drivers/staging. Clearly > there were improvements to be made on the MT7621 drivers that would > affect the devicetree binding. I don't think one would bat an eye to > make dt-binding changes on a devicetree on drivers/staging. > > I also want to make a similar change that would break the ABI. I want to > split the MediaTek MIPS (currently called Ralink) pinctrl subdrivers > further, namely mt7620.c, to split MT7628/MT7688 pinmux data from > MT7620, so I can properly document which function can be given to which > group(s) for the MT7628/MT7688 SoCs. > > This means I've got to introduce a new compatible string for the MT76X8 > SoCs which would cause the pinctrl driver to stop working with an old DT > for the MT76X8 SoCs. By the nature of the change, I can't make a way > around with the code like Sergio did in v6. Now that I think about it, I think there's a way I can introduce a new compatible string for the sake of better documentation, and keep the driver working with old DTs. I'd not like to convolute this series with this conversation. I'll send a mail to the related mailing list when the time is right. Arınç
Hi all, This series make an update in the MT7621 SoC's watchdog driver code. This SoC already provides a system controller node to access reset status register needed for the watchdog. Instead of using MIPS architecture dependent operations in header 'asm/mach-ralink/ralink_regs.h' get node syscon regmap using 'syscon_regmap_lookup_by_compatible()' and use. regmap APIS. Driver is also using some globals that are not needed at all if a driver data structure is used along the code. Hence, add all new needed stuff inside a new driver data structure. With this changes driver can be properly compile tested. Thanks in advance for reviewing this! v1 of this series here [0]. v2 of this series here [1]. v3 of this series here [2]. v4 of this series here [3]. v5 of this series here [4]. Changes in v6: - Get regmap syscon using 'syscon_regmap_lookup_by_compatible()' - Add COMPILE_TEST and dependencies to Kconfig. - Collect Philippe Mathieu-Daudé 'Reviewed-by' tag for watchdog node warning fix. Changes in v5: - Drop patches related with device tree ABI breakage and only maintain the rest. - Collect Arinc 'Reviewed-by' tag for watchdog node warning fix. Changes in v4: - Add a patch to fix a watchdog node warning with 'make dtbs_check' because of a wrong node name. - Collect Guenter 'Reviewed-by' tags for watchdog driver code. - Add a missing 'COMPILE_TEST' to Kconfig which was lost when driver code was split in two patches in v2. Changes in v3: - rename phandler from 'ralink,sysctl' into 'mediatek,sysctl'. - Drop error message added in PATCH 3 that modifies functionality and we only want to maintain current functionaloty by now. Changes in v2: - Remove no needed compatible 'syscon' from bindings. - Rewrite new syscon phandle description in bindings. - Remove 'syscon' from compatible in 'mt7621.dtsi'. - Split PATCH 3 into two different patches: - PATCH 3: removes static globals using a driver data structure. - PATCH 4: remove ralink architecture dependent includes and code. Best regards, Sergio Paracuellos [0]: https://lore.kernel.org/linux-watchdog/20230210065621.598120-1-sergio.paracuellos@gmail.com/T/#t [1]: https://lore.kernel.org/linux-watchdog/20230210121735.639089-1-sergio.paracuellos@gmail.com/T/#t [2]: https://lore.kernel.org/linux-watchdog/20230210173841.705783-1-sergio.paracuellos@gmail.com/T/#t [3]: https://lore.kernel.org/linux-watchdog/20230211073357.755893-1-sergio.paracuellos@gmail.com/T/#t [4]: https://lore.kernel.org/linux-watchdog/20230213120638.850612-1-sergio.paracuellos@gmail.com/T/#t Sergio Paracuellos (2): Sergio Paracuellos (3): mips: dts: ralink: mt7621: rename watchdog node from 'wdt' into 'watchdog' watchdog: mt7621-wdt: avoid static global declarations watchdog: mt7621-wdt: avoid ralink architecture dependent code arch/mips/boot/dts/ralink/mt7621.dtsi | 2 +- drivers/watchdog/Kconfig | 4 +- drivers/watchdog/mt7621_wdt.c | 118 +++++++++++++++++--------- 3 files changed, 81 insertions(+), 43 deletions(-)