| Message ID | 20220429105229.368728-4-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | mtd: rawnand: renesas: Runtime PM use | expand |
Hi Miquel, On Fri, Apr 29, 2022 at 12:52 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > This NAND controller is part of a well defined power domain handled by > the runtime PM core. Let's keep the harmony with the other RZ/N1 drivers > and exclusively use the runtime PM API to enable/disable the clocks. > > We still need to retrieve the external clock rate in order to derive the > NAND timings, but that is not a big deal, we can still do that in the > probe and just save this value to reuse it later. > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> Thanks for your patch! > --- a/drivers/mtd/nand/raw/renesas-nand-controller.c > +++ b/drivers/mtd/nand/raw/renesas-nand-controller.c > @@ -1319,6 +1319,7 @@ static int rnandc_chips_init(struct rnandc *rnandc) > static int rnandc_probe(struct platform_device *pdev) > { > struct rnandc *rnandc; > + struct clk *eclk; > int irq, ret; > > rnandc = devm_kzalloc(&pdev->dev, sizeof(*rnandc), GFP_KERNEL); > @@ -1335,29 +1336,10 @@ static int rnandc_probe(struct platform_device *pdev) > if (IS_ERR(rnandc->regs)) > return PTR_ERR(rnandc->regs); > > - /* APB clock */ > - rnandc->hclk = devm_clk_get(&pdev->dev, "hclk"); > - if (IS_ERR(rnandc->hclk)) > - return PTR_ERR(rnandc->hclk); > - > - /* External NAND bus clock */ > - rnandc->eclk = devm_clk_get(&pdev->dev, "eclk"); > - if (IS_ERR(rnandc->eclk)) > - return PTR_ERR(rnandc->eclk); > - > - ret = clk_prepare_enable(rnandc->hclk); > - if (ret) > - return ret; > - > - ret = clk_prepare_enable(rnandc->eclk); > - if (ret) > - goto disable_hclk; > - > rnandc_dis_interrupts(rnandc); > irq = platform_get_irq_optional(pdev, 0); > if (irq == -EPROBE_DEFER) { > - ret = irq; > - goto disable_eclk; > + return irq; > } else if (irq < 0) { > dev_info(&pdev->dev, "No IRQ found, fallback to polling\n"); > rnandc->use_polling = true; > @@ -1365,12 +1347,25 @@ static int rnandc_probe(struct platform_device *pdev) > ret = devm_request_irq(&pdev->dev, irq, rnandc_irq_handler, 0, > "renesas-nand-controller", rnandc); > if (ret < 0) > - goto disable_eclk; > + return ret; > } > > ret = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32)); > if (ret) > - goto disable_eclk; > + return ret; > + > + pm_runtime_enable(&pdev->dev); > + pm_runtime_get_sync(&pdev->dev); ret = pm_runtime_resume_and_get)...); if (ret < 0) ... > + > + /* The external NAND bus clock rate is needed for computing timings */ > + eclk = clk_get(&pdev->dev, "eclk"); > + if (IS_ERR(eclk)) { > + ret = PTR_ERR(eclk); > + goto dis_runtime_pm; > + } > + > + rnandc->ext_clk_rate = clk_get_rate(eclk); > + clk_put(eclk); > > rnandc_clear_fifo(rnandc); > > @@ -1378,14 +1373,13 @@ static int rnandc_probe(struct platform_device *pdev) > > ret = rnandc_chips_init(rnandc); > if (ret) > - goto disable_eclk; > + goto dis_runtime_pm; > > return 0; > > -disable_eclk: > - clk_disable_unprepare(rnandc->eclk); > -disable_hclk: > - clk_disable_unprepare(rnandc->hclk); > +dis_runtime_pm: > + pm_runtime_put_sync(&pdev->dev); pm_runtime_put() > + pm_runtime_disable(&pdev->dev); > > return ret; > } > @@ -1396,8 +1390,8 @@ static int rnandc_remove(struct platform_device *pdev) > > rnandc_chips_cleanup(rnandc); > > - clk_disable_unprepare(rnandc->eclk); > - clk_disable_unprepare(rnandc->hclk); > + pm_runtime_put_sync(&pdev->dev); pm_runtime_put() > + pm_runtime_disable(&pdev->dev); Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, > > ret = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32)); > > if (ret) > > - goto disable_eclk; > > + return ret; > > + > > + pm_runtime_enable(&pdev->dev); > > + pm_runtime_get_sync(&pdev->dev); > > ret = pm_runtime_resume_and_get)...); > if (ret < 0) ... I also changed the enable call to use devm_pm_runtime_enable() and dropped the calls to pm_runtime_disable() below (same in the RTC driver). > > > + > > + /* The external NAND bus clock rate is needed for computing timings */ > > + eclk = clk_get(&pdev->dev, "eclk"); > > + if (IS_ERR(eclk)) { > > + ret = PTR_ERR(eclk); > > + goto dis_runtime_pm; > > + } > > + > > + rnandc->ext_clk_rate = clk_get_rate(eclk); > > + clk_put(eclk); > > Thanks, Miquèl
diff --git a/drivers/mtd/nand/raw/renesas-nand-controller.c b/drivers/mtd/nand/raw/renesas-nand-controller.c index 6db063b230a9..72dfbc7fd424 100644 --- a/drivers/mtd/nand/raw/renesas-nand-controller.c +++ b/drivers/mtd/nand/raw/renesas-nand-controller.c @@ -16,6 +16,7 @@ #include <linux/mtd/rawnand.h> #include <linux/of.h> #include <linux/platform_device.h> +#include <linux/pm_runtime.h> #include <linux/slab.h> #define COMMAND_REG 0x00 @@ -216,8 +217,7 @@ struct rnandc { struct nand_controller controller; struct device *dev; void __iomem *regs; - struct clk *hclk; - struct clk *eclk; + unsigned long ext_clk_rate; unsigned long assigned_cs; struct list_head chips; struct nand_chip *selected_chip; @@ -891,7 +891,7 @@ static int rnandc_setup_interface(struct nand_chip *chip, int chipnr, { struct rnand_chip *rnand = to_rnand(chip); struct rnandc *rnandc = to_rnandc(chip->controller); - unsigned int period_ns = 1000000000 / clk_get_rate(rnandc->eclk); + unsigned int period_ns = 1000000000 / rnandc->ext_clk_rate; const struct nand_sdr_timings *sdr; unsigned int cyc, cle, ale, bef_dly, ca_to_data; @@ -1319,6 +1319,7 @@ static int rnandc_chips_init(struct rnandc *rnandc) static int rnandc_probe(struct platform_device *pdev) { struct rnandc *rnandc; + struct clk *eclk; int irq, ret; rnandc = devm_kzalloc(&pdev->dev, sizeof(*rnandc), GFP_KERNEL); @@ -1335,29 +1336,10 @@ static int rnandc_probe(struct platform_device *pdev) if (IS_ERR(rnandc->regs)) return PTR_ERR(rnandc->regs); - /* APB clock */ - rnandc->hclk = devm_clk_get(&pdev->dev, "hclk"); - if (IS_ERR(rnandc->hclk)) - return PTR_ERR(rnandc->hclk); - - /* External NAND bus clock */ - rnandc->eclk = devm_clk_get(&pdev->dev, "eclk"); - if (IS_ERR(rnandc->eclk)) - return PTR_ERR(rnandc->eclk); - - ret = clk_prepare_enable(rnandc->hclk); - if (ret) - return ret; - - ret = clk_prepare_enable(rnandc->eclk); - if (ret) - goto disable_hclk; - rnandc_dis_interrupts(rnandc); irq = platform_get_irq_optional(pdev, 0); if (irq == -EPROBE_DEFER) { - ret = irq; - goto disable_eclk; + return irq; } else if (irq < 0) { dev_info(&pdev->dev, "No IRQ found, fallback to polling\n"); rnandc->use_polling = true; @@ -1365,12 +1347,25 @@ static int rnandc_probe(struct platform_device *pdev) ret = devm_request_irq(&pdev->dev, irq, rnandc_irq_handler, 0, "renesas-nand-controller", rnandc); if (ret < 0) - goto disable_eclk; + return ret; } ret = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32)); if (ret) - goto disable_eclk; + return ret; + + pm_runtime_enable(&pdev->dev); + pm_runtime_get_sync(&pdev->dev); + + /* The external NAND bus clock rate is needed for computing timings */ + eclk = clk_get(&pdev->dev, "eclk"); + if (IS_ERR(eclk)) { + ret = PTR_ERR(eclk); + goto dis_runtime_pm; + } + + rnandc->ext_clk_rate = clk_get_rate(eclk); + clk_put(eclk); rnandc_clear_fifo(rnandc); @@ -1378,14 +1373,13 @@ static int rnandc_probe(struct platform_device *pdev) ret = rnandc_chips_init(rnandc); if (ret) - goto disable_eclk; + goto dis_runtime_pm; return 0; -disable_eclk: - clk_disable_unprepare(rnandc->eclk); -disable_hclk: - clk_disable_unprepare(rnandc->hclk); +dis_runtime_pm: + pm_runtime_put_sync(&pdev->dev); + pm_runtime_disable(&pdev->dev); return ret; } @@ -1396,8 +1390,8 @@ static int rnandc_remove(struct platform_device *pdev) rnandc_chips_cleanup(rnandc); - clk_disable_unprepare(rnandc->eclk); - clk_disable_unprepare(rnandc->hclk); + pm_runtime_put_sync(&pdev->dev); + pm_runtime_disable(&pdev->dev); return 0; }
This NAND controller is part of a well defined power domain handled by the runtime PM core. Let's keep the harmony with the other RZ/N1 drivers and exclusively use the runtime PM API to enable/disable the clocks. We still need to retrieve the external clock rate in order to derive the NAND timings, but that is not a big deal, we can still do that in the probe and just save this value to reuse it later. Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- .../mtd/nand/raw/renesas-nand-controller.c | 58 +++++++++---------- 1 file changed, 26 insertions(+), 32 deletions(-)