| Message ID | 20220507100147.5802-4-andriy.shevchenko@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/4] powerpc/52xx: Remove dead code, i.e. mpc52xx_get_xtal_freq() | expand |
On Mon, May 16, 2022 at 11:48:05PM +1000, Michael Ellerman wrote: > Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes: > > We may convert the GPT driver to use fwnode API for the sake > > of consistency of the used APIs inside the driver. > > I'm not sure about this one. > > It's more consistent to use fwnode in this driver, but it's very > inconsistent with the rest of the powerpc code. We have basically no > uses of the fwnode APIs at the moment. Fair point! > It seems like a pretty straight-forward conversion, but there could > easily be a bug in there, I don't have any way to test it. Do you? Nope, only compile testing. The important part of this series is to clean up of_node from GPIO library, so since here it's a user of it I want to do that. This patch is just ad-hoc conversion that I noticed is possible. But there is no any requirement to do so. Lemme drop this from v3.
Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes: > On Mon, May 16, 2022 at 05:05:12PM +0300, Andy Shevchenko wrote: >> On Mon, May 16, 2022 at 11:48:05PM +1000, Michael Ellerman wrote: >> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes: >> > > We may convert the GPT driver to use fwnode API for the sake >> > > of consistency of the used APIs inside the driver. >> > >> > I'm not sure about this one. >> > >> > It's more consistent to use fwnode in this driver, but it's very >> > inconsistent with the rest of the powerpc code. We have basically no >> > uses of the fwnode APIs at the moment. >> >> Fair point! >> >> > It seems like a pretty straight-forward conversion, but there could >> > easily be a bug in there, I don't have any way to test it. Do you? >> >> Nope, only compile testing. The important part of this series is to >> clean up of_node from GPIO library, so since here it's a user of >> it I want to do that. This patch is just ad-hoc conversion that I >> noticed is possible. But there is no any requirement to do so. >> >> Lemme drop this from v3. > > I just realize that there is no point to send a v3. You can just apply > first 3 patches. Or is your comment against entire series? No, my comment is just about this patch. I don't mind converting to new APIs when it's blocking some other cleanup. But given the age of this code I think it's probably better to just leave the rest of it as-is, unless someone volunteers to test it. So yeah I'll just take patches 1-3 of this v2 series, no need to resend. cheers
On Tue, May 17, 2022 at 09:38:56AM +1000, Michael Ellerman wrote: > Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes: > > On Mon, May 16, 2022 at 05:05:12PM +0300, Andy Shevchenko wrote: > >> On Mon, May 16, 2022 at 11:48:05PM +1000, Michael Ellerman wrote: > >> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes: > >> > > We may convert the GPT driver to use fwnode API for the sake > >> > > of consistency of the used APIs inside the driver. > >> > > >> > I'm not sure about this one. > >> > > >> > It's more consistent to use fwnode in this driver, but it's very > >> > inconsistent with the rest of the powerpc code. We have basically no > >> > uses of the fwnode APIs at the moment. > >> > >> Fair point! > >> > >> > It seems like a pretty straight-forward conversion, but there could > >> > easily be a bug in there, I don't have any way to test it. Do you? > >> > >> Nope, only compile testing. The important part of this series is to > >> clean up of_node from GPIO library, so since here it's a user of > >> it I want to do that. This patch is just ad-hoc conversion that I > >> noticed is possible. But there is no any requirement to do so. > >> > >> Lemme drop this from v3. > > > > I just realize that there is no point to send a v3. You can just apply > > first 3 patches. Or is your comment against entire series? > > No, my comment is just about this patch. > > I don't mind converting to new APIs when it's blocking some other > cleanup. But given the age of this code I think it's probably better to > just leave the rest of it as-is, unless someone volunteers to test it. > > So yeah I'll just take patches 1-3 of this v2 series, no need to resend. Thanks! One note though, the fwnode_for_each_parent_node() is not yet available in upstream, but will be after v5.19-rc1. It means the patch 3 can't be applied without that. That's why LKP complained on patch 4 in this series. That said, the easiest way is to postpone it till v5.19-rc1 is out.
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c index ae47fdcc8a96..58c3651034bd 100644 --- a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c +++ b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c @@ -53,10 +53,9 @@ #include <linux/interrupt.h> #include <linux/io.h> #include <linux/list.h> +#include <linux/mod_devicetable.h> +#include <linux/module.h> #include <linux/mutex.h> -#include <linux/of.h> -#include <linux/of_platform.h> -#include <linux/of_gpio.h> #include <linux/kernel.h> #include <linux/property.h> #include <linux/slab.h> @@ -64,7 +63,7 @@ #include <linux/watchdog.h> #include <linux/miscdevice.h> #include <linux/uaccess.h> -#include <linux/module.h> + #include <asm/div64.h> #include <asm/mpc52xx.h> @@ -235,18 +234,17 @@ static const struct irq_domain_ops mpc52xx_gpt_irq_ops = { .xlate = mpc52xx_gpt_irq_xlate, }; -static void -mpc52xx_gpt_irq_setup(struct mpc52xx_gpt_priv *gpt, struct device_node *node) +static void mpc52xx_gpt_irq_setup(struct mpc52xx_gpt_priv *gpt) { int cascade_virq; unsigned long flags; u32 mode; - cascade_virq = irq_of_parse_and_map(node, 0); - if (!cascade_virq) + cascade_virq = platform_get_irq(to_platform_device(gpt->dev), 0); + if (cascade_virq < 0) return; - gpt->irqhost = irq_domain_add_linear(node, 1, &mpc52xx_gpt_irq_ops, gpt); + gpt->irqhost = irq_domain_create_linear(dev_fwnode(gpt->dev), 1, &mpc52xx_gpt_irq_ops, gpt); if (!gpt->irqhost) { dev_err(gpt->dev, "irq_domain_add_linear() failed\n"); return; @@ -670,8 +668,7 @@ static int mpc52xx_gpt_wdt_init(void) return err; } -static int mpc52xx_gpt_wdt_setup(struct mpc52xx_gpt_priv *gpt, - const u32 *period) +static int mpc52xx_gpt_wdt_setup(struct mpc52xx_gpt_priv *gpt, const u32 period) { u64 real_timeout; @@ -679,14 +676,14 @@ static int mpc52xx_gpt_wdt_setup(struct mpc52xx_gpt_priv *gpt, mpc52xx_gpt_wdt = gpt; /* configure the wdt if the device tree contained a timeout */ - if (!period || *period == 0) + if (period == 0) return 0; - real_timeout = (u64) *period * 1000000000ULL; + real_timeout = (u64)period * 1000000000ULL; if (mpc52xx_gpt_do_start(gpt, real_timeout, 0, 1)) dev_warn(gpt->dev, "starting as wdt failed\n"); else - dev_info(gpt->dev, "watchdog set to %us timeout\n", *period); + dev_info(gpt->dev, "watchdog set to %us timeout\n", period); return 0; } @@ -697,8 +694,7 @@ static int mpc52xx_gpt_wdt_init(void) return 0; } -static inline int mpc52xx_gpt_wdt_setup(struct mpc52xx_gpt_priv *gpt, - const u32 *period) +static inline int mpc52xx_gpt_wdt_setup(struct mpc52xx_gpt_priv *gpt, const u32 period) { return 0; } @@ -726,25 +722,26 @@ static int mpc52xx_gpt_probe(struct platform_device *ofdev) dev_set_drvdata(&ofdev->dev, gpt); mpc52xx_gpt_gpio_setup(gpt); - mpc52xx_gpt_irq_setup(gpt, ofdev->dev.of_node); + mpc52xx_gpt_irq_setup(gpt); mutex_lock(&mpc52xx_gpt_list_mutex); list_add(&gpt->list, &mpc52xx_gpt_list); mutex_unlock(&mpc52xx_gpt_list_mutex); /* check if this device could be a watchdog */ - if (of_get_property(ofdev->dev.of_node, "fsl,has-wdt", NULL) || - of_get_property(ofdev->dev.of_node, "has-wdt", NULL)) { - const u32 *on_boot_wdt; + if (device_property_present(gpt->dev, "fsl,has-wdt") || + device_property_present(gpt->dev, "has-wdt")) { + u32 on_boot_wdt = 0; + int ret; gpt->wdt_mode = MPC52xx_GPT_CAN_WDT; - on_boot_wdt = of_get_property(ofdev->dev.of_node, - "fsl,wdt-on-boot", NULL); - if (on_boot_wdt) { + ret = device_property_read_u32(gpt->dev, "fsl,wdt-on-boot", &on_boot_wdt); + if (ret) { + dev_info(gpt->dev, "can function as watchdog\n"); + } else { dev_info(gpt->dev, "used as watchdog\n"); gpt->wdt_mode |= MPC52xx_GPT_IS_WDT; - } else - dev_info(gpt->dev, "can function as watchdog\n"); + } mpc52xx_gpt_wdt_setup(gpt, on_boot_wdt); }
We may convert the GPT driver to use fwnode API for the sake of consistency of the used APIs inside the driver. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- v2: no changes arch/powerpc/platforms/52xx/mpc52xx_gpt.c | 47 +++++++++++------------ 1 file changed, 22 insertions(+), 25 deletions(-)