Leela Krishna Amudala wrote: > > This patch removes the global variables in the driver file and > group them into a structure. > > Signed-off-by: Leela Krishna Amudala <l.krishna@xxxxxxxxxxx> (+ Wim Van Sebroeck) Looks good to me, Acked-by: Kukjin Kim <kgene.kim@xxxxxxxxxxx> Thanks, Kukjin > --- > Note: This patch is rebased on kgene's for-next branch and tested on > SMDK5420. > > Changes since V2: > - Addressed comments given by Tomasz Figa <t.figa@xxxxxxxxxxx> > https://patchwork.kernel.org/patch/2831032/ > - Renamed structure name from s3c2410_watchdog to s3c2410_wdt. > > Changes since V1: > - changed the patch subject. > - indentation correction in s3c2410_watchdog structure. > > drivers/watchdog/s3c2410_wdt.c | 225 +++++++++++++++++++++++------------ > ----- > 1 file changed, 131 insertions(+), 94 deletions(-) > > diff --git a/drivers/watchdog/s3c2410_wdt.c > b/drivers/watchdog/s3c2410_wdt.c > index 6a22cf5..739dbd3 100644 > --- a/drivers/watchdog/s3c2410_wdt.c > +++ b/drivers/watchdog/s3c2410_wdt.c > @@ -84,13 +84,17 @@ MODULE_PARM_DESC(soft_noboot, "Watchdog action, set to > 1 to ignore reboots, " > "0 to reboot (default 0)"); > MODULE_PARM_DESC(debug, "Watchdog debug, set to >1 for debug (default > 0)"); > > -static struct device *wdt_dev; /* platform device attached to */ > -static struct resource *wdt_mem; > -static struct resource *wdt_irq; > -static struct clk *wdt_clock; > -static void __iomem *wdt_base; > -static unsigned int wdt_count; > -static DEFINE_SPINLOCK(wdt_lock); > +struct s3c2410_wdt { > + struct device *dev; > + struct clk *clock; > + void __iomem *reg_base; > + unsigned int count; > + spinlock_t lock; > + unsigned long wtcon_save; > + unsigned long wtdat_save; > + struct watchdog_device wdt_device; > + struct notifier_block freq_transition; > +}; > > /* watchdog control routines */ > > @@ -102,29 +106,43 @@ do { \ > > /* functions */ > > +static inline struct s3c2410_wdt *to_s3c2410_wdt(struct watchdog_device > *wdd) > +{ > + return container_of(wdd, struct s3c2410_wdt, wdt_device); > +} > + > +static inline struct s3c2410_wdt *freq_to_wdt(struct notifier_block *nb) > +{ > + return container_of(nb, struct s3c2410_wdt, freq_transition); > +} > + > static int s3c2410wdt_keepalive(struct watchdog_device *wdd) > { > - spin_lock(&wdt_lock); > - writel(wdt_count, wdt_base + S3C2410_WTCNT); > - spin_unlock(&wdt_lock); > + struct s3c2410_wdt *wdt = to_s3c2410_wdt(wdd); > + > + spin_lock(&wdt->lock); > + writel(wdt->count, wdt->reg_base + S3C2410_WTCNT); > + spin_unlock(&wdt->lock); > > return 0; > } > > -static void __s3c2410wdt_stop(void) > +static void __s3c2410wdt_stop(struct s3c2410_wdt *wdt) > { > unsigned long wtcon; > > - wtcon = readl(wdt_base + S3C2410_WTCON); > + wtcon = readl(wdt->reg_base + S3C2410_WTCON); > wtcon &= ~(S3C2410_WTCON_ENABLE | S3C2410_WTCON_RSTEN); > - writel(wtcon, wdt_base + S3C2410_WTCON); > + writel(wtcon, wdt->reg_base + S3C2410_WTCON); > } > > static int s3c2410wdt_stop(struct watchdog_device *wdd) > { > - spin_lock(&wdt_lock); > - __s3c2410wdt_stop(); > - spin_unlock(&wdt_lock); > + struct s3c2410_wdt *wdt = to_s3c2410_wdt(wdd); > + > + spin_lock(&wdt->lock); > + __s3c2410wdt_stop(wdt); > + spin_unlock(&wdt->lock); > > return 0; > } > @@ -132,12 +150,13 @@ static int s3c2410wdt_stop(struct watchdog_device > *wdd) > static int s3c2410wdt_start(struct watchdog_device *wdd) > { > unsigned long wtcon; > + struct s3c2410_wdt *wdt = to_s3c2410_wdt(wdd); > > - spin_lock(&wdt_lock); > + spin_lock(&wdt->lock); > > - __s3c2410wdt_stop(); > + __s3c2410wdt_stop(wdt); > > - wtcon = readl(wdt_base + S3C2410_WTCON); > + wtcon = readl(wdt->reg_base + S3C2410_WTCON); > wtcon |= S3C2410_WTCON_ENABLE | S3C2410_WTCON_DIV128; > > if (soft_noboot) { > @@ -148,25 +167,26 @@ static int s3c2410wdt_start(struct watchdog_device > *wdd) > wtcon |= S3C2410_WTCON_RSTEN; > } > > - DBG("%s: wdt_count=0x%08x, wtcon=%08lx\n", > - __func__, wdt_count, wtcon); > + DBG("%s: count=0x%08x, wtcon=%08lx\n", > + __func__, wdt->count, wtcon); > > - writel(wdt_count, wdt_base + S3C2410_WTDAT); > - writel(wdt_count, wdt_base + S3C2410_WTCNT); > - writel(wtcon, wdt_base + S3C2410_WTCON); > - spin_unlock(&wdt_lock); > + writel(wdt->count, wdt->reg_base + S3C2410_WTDAT); > + writel(wdt->count, wdt->reg_base + S3C2410_WTCNT); > + writel(wtcon, wdt->reg_base + S3C2410_WTCON); > + spin_unlock(&wdt->lock); > > return 0; > } > > -static inline int s3c2410wdt_is_running(void) > +static inline int s3c2410wdt_is_running(struct s3c2410_wdt *wdt) > { > - return readl(wdt_base + S3C2410_WTCON) & S3C2410_WTCON_ENABLE; > + return readl(wdt->reg_base + S3C2410_WTCON) & S3C2410_WTCON_ENABLE; > } > > static int s3c2410wdt_set_heartbeat(struct watchdog_device *wdd, unsigned > timeout) > { > - unsigned long freq = clk_get_rate(wdt_clock); > + struct s3c2410_wdt *wdt = to_s3c2410_wdt(wdd); > + unsigned long freq = clk_get_rate(wdt->clock); > unsigned int count; > unsigned int divisor = 1; > unsigned long wtcon; > @@ -192,7 +212,7 @@ static int s3c2410wdt_set_heartbeat(struct > watchdog_device *wdd, unsigned timeou > } > > if ((count / divisor) >= 0x10000) { > - dev_err(wdt_dev, "timeout %d too big\n", timeout); > + dev_err(wdt->dev, "timeout %d too big\n", timeout); > return -EINVAL; > } > } > @@ -201,15 +221,15 @@ static int s3c2410wdt_set_heartbeat(struct > watchdog_device *wdd, unsigned timeou > __func__, timeout, divisor, count, count/divisor); > > count /= divisor; > - wdt_count = count; > + wdt->count = count; > > /* update the pre-scaler */ > - wtcon = readl(wdt_base + S3C2410_WTCON); > + wtcon = readl(wdt->reg_base + S3C2410_WTCON); > wtcon &= ~S3C2410_WTCON_PRESCALE_MASK; > wtcon |= S3C2410_WTCON_PRESCALE(divisor-1); > > - writel(count, wdt_base + S3C2410_WTDAT); > - writel(wtcon, wdt_base + S3C2410_WTCON); > + writel(count, wdt->reg_base + S3C2410_WTDAT); > + writel(wtcon, wdt->reg_base + S3C2410_WTCON); > > wdd->timeout = (count * divisor) / freq; > > @@ -242,21 +262,23 @@ static struct watchdog_device s3c2410_wdd = { > > static irqreturn_t s3c2410wdt_irq(int irqno, void *param) > { > - dev_info(wdt_dev, "watchdog timer expired (irq)\n"); > + struct s3c2410_wdt *wdt = platform_get_drvdata(param); > + > + dev_info(wdt->dev, "watchdog timer expired (irq)\n"); > > - s3c2410wdt_keepalive(&s3c2410_wdd); > + s3c2410wdt_keepalive(&wdt->wdt_device); > return IRQ_HANDLED; > } > > - > #ifdef CONFIG_CPU_FREQ > > static int s3c2410wdt_cpufreq_transition(struct notifier_block *nb, > unsigned long val, void *data) > { > int ret; > + struct s3c2410_wdt *wdt = freq_to_wdt(nb); > > - if (!s3c2410wdt_is_running()) > + if (!s3c2410wdt_is_running(wdt)) > goto done; > > if (val == CPUFREQ_PRECHANGE) { > @@ -265,14 +287,15 @@ static int s3c2410wdt_cpufreq_transition(struct > notifier_block *nb, > * the watchdog is running. > */ > > - s3c2410wdt_keepalive(&s3c2410_wdd); > + s3c2410wdt_keepalive(&wdt->wdt_device); > } else if (val == CPUFREQ_POSTCHANGE) { > - s3c2410wdt_stop(&s3c2410_wdd); > + s3c2410wdt_stop(&wdt->wdt_device); > > - ret = s3c2410wdt_set_heartbeat(&s3c2410_wdd, > s3c2410_wdd.timeout); > + ret = s3c2410wdt_set_heartbeat(&wdt->wdt_device, > + wdt->wdt_device.timeout); > > if (ret >= 0) > - s3c2410wdt_start(&s3c2410_wdd); > + s3c2410wdt_start(&wdt->wdt_device); > else > goto err; > } > @@ -281,34 +304,35 @@ done: > return 0; > > err: > - dev_err(wdt_dev, "cannot set new value for timeout %d\n", > - s3c2410_wdd.timeout); > + dev_err(wdt->dev, "cannot set new value for timeout %d\n", > + wdt->wdt_device.timeout); > return ret; > } > > -static struct notifier_block s3c2410wdt_cpufreq_transition_nb = { > - .notifier_call = s3c2410wdt_cpufreq_transition, > -}; > - > -static inline int s3c2410wdt_cpufreq_register(void) > +static inline int s3c2410wdt_cpufreq_register(struct s3c2410_wdt *wdt) > { > - return cpufreq_register_notifier(&s3c2410wdt_cpufreq_transition_nb, > + wdt->freq_transition.notifier_call = s3c2410wdt_cpufreq_transition; > + > + return cpufreq_register_notifier(&wdt->freq_transition, > CPUFREQ_TRANSITION_NOTIFIER); > } > > -static inline void s3c2410wdt_cpufreq_deregister(void) > +static inline void s3c2410wdt_cpufreq_deregister(struct s3c2410_wdt *wdt) > { > - cpufreq_unregister_notifier(&s3c2410wdt_cpufreq_transition_nb, > + wdt->freq_transition.notifier_call = s3c2410wdt_cpufreq_transition; > + > + cpufreq_unregister_notifier(&wdt->freq_transition, > CPUFREQ_TRANSITION_NOTIFIER); > } > > #else > -static inline int s3c2410wdt_cpufreq_register(void) > + > +static inline int s3c2410wdt_cpufreq_register(struct s3c2410_wdt *wdt) > { > return 0; > } > > -static inline void s3c2410wdt_cpufreq_deregister(void) > +static inline void s3c2410wdt_cpufreq_deregister(struct s3c2410_wdt *wdt) > { > } > #endif > @@ -316,6 +340,9 @@ static inline void s3c2410wdt_cpufreq_deregister(void) > static int s3c2410wdt_probe(struct platform_device *pdev) > { > struct device *dev; > + struct s3c2410_wdt *wdt; > + struct resource *wdt_mem; > + struct resource *wdt_irq; > unsigned int wtcon; > int started = 0; > int ret; > @@ -323,8 +350,14 @@ static int s3c2410wdt_probe(struct platform_device > *pdev) > DBG("%s: probe=%p\n", __func__, pdev); > > dev = &pdev->dev; > - wdt_dev = &pdev->dev; > > + wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL); > + if (!wdt) > + return -ENOMEM; > + > + wdt->dev = &pdev->dev; > + spin_lock_init(&wdt->lock); > + wdt->wdt_device = s3c2410_wdd; > wdt_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > if (wdt_mem == NULL) { > dev_err(dev, "no memory resource specified\n"); > @@ -339,24 +372,24 @@ static int s3c2410wdt_probe(struct platform_device > *pdev) > } > > /* get the memory region for the watchdog timer */ > - wdt_base = devm_ioremap_resource(dev, wdt_mem); > - if (IS_ERR(wdt_base)) { > - ret = PTR_ERR(wdt_base); > + wdt->reg_base = devm_ioremap_resource(dev, wdt_mem); > + if (IS_ERR(wdt->reg_base)) { > + ret = PTR_ERR(wdt->reg_base); > goto err; > } > > - DBG("probe: mapped wdt_base=%p\n", wdt_base); > + DBG("probe: mapped reg_base=%p\n", wdt->reg_base); > > - wdt_clock = devm_clk_get(dev, "watchdog"); > - if (IS_ERR(wdt_clock)) { > + wdt->clock = devm_clk_get(dev, "watchdog"); > + if (IS_ERR(wdt->clock)) { > dev_err(dev, "failed to find watchdog clock source\n"); > - ret = PTR_ERR(wdt_clock); > + ret = PTR_ERR(wdt->clock); > goto err; > } > > - clk_prepare_enable(wdt_clock); > + clk_prepare_enable(wdt->clock); > > - ret = s3c2410wdt_cpufreq_register(); > + ret = s3c2410wdt_cpufreq_register(wdt); > if (ret < 0) { > dev_err(dev, "failed to register cpufreq\n"); > goto err_clk; > @@ -365,9 +398,11 @@ static int s3c2410wdt_probe(struct platform_device > *pdev) > /* see if we can actually set the requested timer margin, and if > * not, try the default value */ > > - watchdog_init_timeout(&s3c2410_wdd, tmr_margin, &pdev->dev); > - if (s3c2410wdt_set_heartbeat(&s3c2410_wdd, s3c2410_wdd.timeout)) { > - started = s3c2410wdt_set_heartbeat(&s3c2410_wdd, > + watchdog_init_timeout(&wdt->wdt_device, tmr_margin, &pdev->dev); > + ret = s3c2410wdt_set_heartbeat(&wdt->wdt_device, > + wdt->wdt_device.timeout); > + if (ret) { > + started = s3c2410wdt_set_heartbeat(&wdt->wdt_device, > CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME); > > if (started == 0) > @@ -386,9 +421,9 @@ static int s3c2410wdt_probe(struct platform_device > *pdev) > goto err_cpufreq; > } > > - watchdog_set_nowayout(&s3c2410_wdd, nowayout); > + watchdog_set_nowayout(&wdt->wdt_device, nowayout); > > - ret = watchdog_register_device(&s3c2410_wdd); > + ret = watchdog_register_device(&wdt->wdt_device); > if (ret) { > dev_err(dev, "cannot register watchdog (%d)\n", ret); > goto err_cpufreq; > @@ -396,18 +431,20 @@ static int s3c2410wdt_probe(struct platform_device > *pdev) > > if (tmr_atboot && started == 0) { > dev_info(dev, "starting watchdog timer\n"); > - s3c2410wdt_start(&s3c2410_wdd); > + s3c2410wdt_start(&wdt->wdt_device); > } else if (!tmr_atboot) { > /* if we're not enabling the watchdog, then ensure it is > * disabled if it has been left running from the bootloader > * or other source */ > > - s3c2410wdt_stop(&s3c2410_wdd); > + s3c2410wdt_stop(&wdt->wdt_device); > } > > + platform_set_drvdata(pdev, wdt); > + > /* print out a statement of readiness */ > > - wtcon = readl(wdt_base + S3C2410_WTCON); > + wtcon = readl(wdt->reg_base + S3C2410_WTCON); > > dev_info(dev, "watchdog %sactive, reset %sabled, irq %sabled\n", > (wtcon & S3C2410_WTCON_ENABLE) ? "" : "in", > @@ -417,64 +454,64 @@ static int s3c2410wdt_probe(struct platform_device > *pdev) > return 0; > > err_cpufreq: > - s3c2410wdt_cpufreq_deregister(); > + s3c2410wdt_cpufreq_deregister(wdt); > > err_clk: > - clk_disable_unprepare(wdt_clock); > - wdt_clock = NULL; > + clk_disable_unprepare(wdt->clock); > + wdt->clock = NULL; > > err: > - wdt_irq = NULL; > - wdt_mem = NULL; > return ret; > } > > static int s3c2410wdt_remove(struct platform_device *dev) > { > - watchdog_unregister_device(&s3c2410_wdd); > + struct s3c2410_wdt *wdt = platform_get_drvdata(dev); > > - s3c2410wdt_cpufreq_deregister(); > + watchdog_unregister_device(&wdt->wdt_device); > > - clk_disable_unprepare(wdt_clock); > - wdt_clock = NULL; > + s3c2410wdt_cpufreq_deregister(wdt); > + > + clk_disable_unprepare(wdt->clock); > + wdt->clock = NULL; > > - wdt_irq = NULL; > - wdt_mem = NULL; > return 0; > } > > static void s3c2410wdt_shutdown(struct platform_device *dev) > { > - s3c2410wdt_stop(&s3c2410_wdd); > + struct s3c2410_wdt *wdt = platform_get_drvdata(dev); > + > + s3c2410wdt_stop(&wdt->wdt_device); > } > > #ifdef CONFIG_PM_SLEEP > > -static unsigned long wtcon_save; > -static unsigned long wtdat_save; > - > static int s3c2410wdt_suspend(struct device *dev) > { > + struct s3c2410_wdt *wdt = dev_get_drvdata(dev); > + > /* Save watchdog state, and turn it off. */ > - wtcon_save = readl(wdt_base + S3C2410_WTCON); > - wtdat_save = readl(wdt_base + S3C2410_WTDAT); > + wdt->wtcon_save = readl(wdt->reg_base + S3C2410_WTCON); > + wdt->wtdat_save = readl(wdt->reg_base + S3C2410_WTDAT); > > /* Note that WTCNT doesn't need to be saved. */ > - s3c2410wdt_stop(&s3c2410_wdd); > + s3c2410wdt_stop(&wdt->wdt_device); > > return 0; > } > > static int s3c2410wdt_resume(struct device *dev) > { > - /* Restore watchdog state. */ > + struct s3c2410_wdt *wdt = dev_get_drvdata(dev); > > - writel(wtdat_save, wdt_base + S3C2410_WTDAT); > - writel(wtdat_save, wdt_base + S3C2410_WTCNT); /* Reset count */ > - writel(wtcon_save, wdt_base + S3C2410_WTCON); > + /* Restore watchdog state. */ > + writel(wdt->wtdat_save, wdt->reg_base + S3C2410_WTDAT); > + writel(wdt->wtdat_save, wdt->reg_base + S3C2410_WTCNT);/* Reset > count */ > + writel(wdt->wtcon_save, wdt->reg_base + S3C2410_WTCON); > > dev_info(dev, "watchdog %sabled\n", > - (wtcon_save & S3C2410_WTCON_ENABLE) ? "en" : "dis"); > + (wdt->wtcon_save & S3C2410_WTCON_ENABLE) ? "en" : "dis"); > > return 0; > } > -- > 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
 |