* [PATCH] ATA: sata_mv: Remove unneeded forward declaration @ 2013-07-29 15:21 Ezequiel Garcia 2013-07-29 15:21 ` [PATCH] ATA: sata_mv: Remove uneeded CONFIG_HAVE_CLK ifdefs Ezequiel Garcia 2013-07-29 15:59 ` [PATCH] ATA: sata_mv: Remove unneeded forward declaration Tejun Heo 0 siblings, 2 replies; 7+ messages in thread From: Ezequiel Garcia @ 2013-07-29 15:21 UTC (permalink / raw) To: linux-ide, linux-kernel Cc: Andrew Lunn, tj, Thomas Petazzoni, Gregory Clement, Lior Amsalem, Ezequiel Garcia These forward declarations are no longer needed, and are probably historical left-over. Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> --- drivers/ata/sata_mv.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c index 35c6b6d..10ea99a 100644 --- a/drivers/ata/sata_mv.c +++ b/drivers/ata/sata_mv.c @@ -4428,9 +4428,6 @@ static int mv_pci_device_resume(struct pci_dev *pdev) #endif #endif -static int mv_platform_probe(struct platform_device *pdev); -static int mv_platform_remove(struct platform_device *pdev); - static int __init mv_init(void) { int rc = -ENODEV; -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH] ATA: sata_mv: Remove uneeded CONFIG_HAVE_CLK ifdefs 2013-07-29 15:21 [PATCH] ATA: sata_mv: Remove unneeded forward declaration Ezequiel Garcia @ 2013-07-29 15:21 ` Ezequiel Garcia 2013-07-29 16:00 ` Tejun Heo 2013-07-29 19:04 ` Andrew Lunn 2013-07-29 15:59 ` [PATCH] ATA: sata_mv: Remove unneeded forward declaration Tejun Heo 1 sibling, 2 replies; 7+ messages in thread From: Ezequiel Garcia @ 2013-07-29 15:21 UTC (permalink / raw) To: linux-ide, linux-kernel Cc: Andrew Lunn, tj, Thomas Petazzoni, Gregory Clement, Lior Amsalem, Ezequiel Garcia If CONFIG_HAVE_CLK is not selected, then all the clk API turn out into stubs, so there's no need to have the ifdefs. The only side-effect of this patch is the extra tiny kmalloc, but that's not enough reason to have such ugly ifdefs all around the code. Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> --- drivers/ata/sata_mv.c | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c index 10ea99a..3519987 100644 --- a/drivers/ata/sata_mv.c +++ b/drivers/ata/sata_mv.c @@ -553,10 +553,8 @@ struct mv_host_priv { u32 irq_mask_offset; u32 unmask_all_irqs; -#if defined(CONFIG_HAVE_CLK) struct clk *clk; struct clk **port_clks; -#endif /* * These consistent DMA memory pools give us guaranteed * alignment for hardware-accessed data structures, @@ -4032,9 +4030,7 @@ static int mv_platform_probe(struct platform_device *pdev) struct resource *res; int n_ports = 0, irq = 0; int rc; -#if defined(CONFIG_HAVE_CLK) int port; -#endif ata_print_version_once(&pdev->dev, DRV_VERSION); @@ -4068,13 +4064,11 @@ static int mv_platform_probe(struct platform_device *pdev) if (!host || !hpriv) return -ENOMEM; -#if defined(CONFIG_HAVE_CLK) hpriv->port_clks = devm_kzalloc(&pdev->dev, sizeof(struct clk *) * n_ports, GFP_KERNEL); if (!hpriv->port_clks) return -ENOMEM; -#endif host->private_data = hpriv; hpriv->n_ports = n_ports; hpriv->board_idx = chip_soc; @@ -4084,7 +4078,6 @@ static int mv_platform_probe(struct platform_device *pdev) resource_size(res)); hpriv->base -= SATAHC0_REG_BASE; -#if defined(CONFIG_HAVE_CLK) hpriv->clk = clk_get(&pdev->dev, NULL); if (IS_ERR(hpriv->clk)) dev_notice(&pdev->dev, "cannot get optional clkdev\n"); @@ -4098,7 +4091,6 @@ static int mv_platform_probe(struct platform_device *pdev) if (!IS_ERR(hpriv->port_clks[port])) clk_prepare_enable(hpriv->port_clks[port]); } -#endif /* * (Re-)program MBUS remapping windows if we are asked to. @@ -4124,7 +4116,6 @@ static int mv_platform_probe(struct platform_device *pdev) return 0; err: -#if defined(CONFIG_HAVE_CLK) if (!IS_ERR(hpriv->clk)) { clk_disable_unprepare(hpriv->clk); clk_put(hpriv->clk); @@ -4135,7 +4126,6 @@ err: clk_put(hpriv->port_clks[port]); } } -#endif return rc; } @@ -4151,13 +4141,10 @@ err: static int mv_platform_remove(struct platform_device *pdev) { struct ata_host *host = platform_get_drvdata(pdev); -#if defined(CONFIG_HAVE_CLK) struct mv_host_priv *hpriv = host->private_data; int port; -#endif ata_host_detach(host); -#if defined(CONFIG_HAVE_CLK) if (!IS_ERR(hpriv->clk)) { clk_disable_unprepare(hpriv->clk); clk_put(hpriv->clk); @@ -4168,7 +4155,6 @@ static int mv_platform_remove(struct platform_device *pdev) clk_put(hpriv->port_clks[port]); } } -#endif return 0; } -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] ATA: sata_mv: Remove uneeded CONFIG_HAVE_CLK ifdefs 2013-07-29 15:21 ` [PATCH] ATA: sata_mv: Remove uneeded CONFIG_HAVE_CLK ifdefs Ezequiel Garcia @ 2013-07-29 16:00 ` Tejun Heo 2013-07-29 16:51 ` Ezequiel Garcia 2013-07-29 19:04 ` Andrew Lunn 1 sibling, 1 reply; 7+ messages in thread From: Tejun Heo @ 2013-07-29 16:00 UTC (permalink / raw) To: Ezequiel Garcia Cc: linux-ide, linux-kernel, Andrew Lunn, Thomas Petazzoni, Gregory Clement, Lior Amsalem On Mon, Jul 29, 2013 at 12:21:22PM -0300, Ezequiel Garcia wrote: > If CONFIG_HAVE_CLK is not selected, then all the clk API turn out > into stubs, so there's no need to have the ifdefs. > The only side-effect of this patch is the extra tiny kmalloc, > but that's not enough reason to have such ugly ifdefs all around > the code. > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> Can you please add a comment in the hpriv definition explaining what that clk is for and how it's optional and becomes noops when not needed? Thanks. -- tejun ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ATA: sata_mv: Remove uneeded CONFIG_HAVE_CLK ifdefs 2013-07-29 16:00 ` Tejun Heo @ 2013-07-29 16:51 ` Ezequiel Garcia 0 siblings, 0 replies; 7+ messages in thread From: Ezequiel Garcia @ 2013-07-29 16:51 UTC (permalink / raw) To: Tejun Heo Cc: linux-ide, linux-kernel, Andrew Lunn, Thomas Petazzoni, Gregory Clement, Lior Amsalem On Mon, Jul 29, 2013 at 12:00:16PM -0400, Tejun Heo wrote: > On Mon, Jul 29, 2013 at 12:21:22PM -0300, Ezequiel Garcia wrote: > > If CONFIG_HAVE_CLK is not selected, then all the clk API turn out > > into stubs, so there's no need to have the ifdefs. > > The only side-effect of this patch is the extra tiny kmalloc, > > but that's not enough reason to have such ugly ifdefs all around > > the code. > > > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > > Can you please add a comment in the hpriv definition explaining what > that clk is for and how it's optional and becomes noops when not > needed? > Sure. Thanks for the feedback, -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ATA: sata_mv: Remove uneeded CONFIG_HAVE_CLK ifdefs 2013-07-29 15:21 ` [PATCH] ATA: sata_mv: Remove uneeded CONFIG_HAVE_CLK ifdefs Ezequiel Garcia 2013-07-29 16:00 ` Tejun Heo @ 2013-07-29 19:04 ` Andrew Lunn 2013-07-29 19:44 ` Ezequiel Garcia 1 sibling, 1 reply; 7+ messages in thread From: Andrew Lunn @ 2013-07-29 19:04 UTC (permalink / raw) To: Ezequiel Garcia Cc: linux-ide, linux-kernel, Andrew Lunn, tj, Thomas Petazzoni, Gregory Clement, Lior Amsalem On Mon, Jul 29, 2013 at 12:21:22PM -0300, Ezequiel Garcia wrote: > If CONFIG_HAVE_CLK is not selected, then all the clk API turn out > into stubs, so there's no need to have the ifdefs. > The only side-effect of this patch is the extra tiny kmalloc, > but that's not enough reason to have such ugly ifdefs all around > the code. > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> Hi Ezequiel What architectures did you compile this for? PowerPC and x86 as well as ARM? I _think_ this driver is also used there, and i _think_ without some #ifdef, it failed to build. But maybe since then, the common clock is now available on all architectures? Andrew > --- > drivers/ata/sata_mv.c | 14 -------------- > 1 file changed, 14 deletions(-) > > diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c > index 10ea99a..3519987 100644 > --- a/drivers/ata/sata_mv.c > +++ b/drivers/ata/sata_mv.c > @@ -553,10 +553,8 @@ struct mv_host_priv { > u32 irq_mask_offset; > u32 unmask_all_irqs; > > -#if defined(CONFIG_HAVE_CLK) > struct clk *clk; > struct clk **port_clks; > -#endif > /* > * These consistent DMA memory pools give us guaranteed > * alignment for hardware-accessed data structures, > @@ -4032,9 +4030,7 @@ static int mv_platform_probe(struct platform_device *pdev) > struct resource *res; > int n_ports = 0, irq = 0; > int rc; > -#if defined(CONFIG_HAVE_CLK) > int port; > -#endif > > ata_print_version_once(&pdev->dev, DRV_VERSION); > > @@ -4068,13 +4064,11 @@ static int mv_platform_probe(struct platform_device *pdev) > > if (!host || !hpriv) > return -ENOMEM; > -#if defined(CONFIG_HAVE_CLK) > hpriv->port_clks = devm_kzalloc(&pdev->dev, > sizeof(struct clk *) * n_ports, > GFP_KERNEL); > if (!hpriv->port_clks) > return -ENOMEM; > -#endif > host->private_data = hpriv; > hpriv->n_ports = n_ports; > hpriv->board_idx = chip_soc; > @@ -4084,7 +4078,6 @@ static int mv_platform_probe(struct platform_device *pdev) > resource_size(res)); > hpriv->base -= SATAHC0_REG_BASE; > > -#if defined(CONFIG_HAVE_CLK) > hpriv->clk = clk_get(&pdev->dev, NULL); > if (IS_ERR(hpriv->clk)) > dev_notice(&pdev->dev, "cannot get optional clkdev\n"); > @@ -4098,7 +4091,6 @@ static int mv_platform_probe(struct platform_device *pdev) > if (!IS_ERR(hpriv->port_clks[port])) > clk_prepare_enable(hpriv->port_clks[port]); > } > -#endif > > /* > * (Re-)program MBUS remapping windows if we are asked to. > @@ -4124,7 +4116,6 @@ static int mv_platform_probe(struct platform_device *pdev) > return 0; > > err: > -#if defined(CONFIG_HAVE_CLK) > if (!IS_ERR(hpriv->clk)) { > clk_disable_unprepare(hpriv->clk); > clk_put(hpriv->clk); > @@ -4135,7 +4126,6 @@ err: > clk_put(hpriv->port_clks[port]); > } > } > -#endif > > return rc; > } > @@ -4151,13 +4141,10 @@ err: > static int mv_platform_remove(struct platform_device *pdev) > { > struct ata_host *host = platform_get_drvdata(pdev); > -#if defined(CONFIG_HAVE_CLK) > struct mv_host_priv *hpriv = host->private_data; > int port; > -#endif > ata_host_detach(host); > > -#if defined(CONFIG_HAVE_CLK) > if (!IS_ERR(hpriv->clk)) { > clk_disable_unprepare(hpriv->clk); > clk_put(hpriv->clk); > @@ -4168,7 +4155,6 @@ static int mv_platform_remove(struct platform_device *pdev) > clk_put(hpriv->port_clks[port]); > } > } > -#endif > return 0; > } > > -- > 1.8.1.5 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ATA: sata_mv: Remove uneeded CONFIG_HAVE_CLK ifdefs 2013-07-29 19:04 ` Andrew Lunn @ 2013-07-29 19:44 ` Ezequiel Garcia 0 siblings, 0 replies; 7+ messages in thread From: Ezequiel Garcia @ 2013-07-29 19:44 UTC (permalink / raw) To: Andrew Lunn Cc: linux-ide, linux-kernel, tj, Thomas Petazzoni, Gregory Clement, Lior Amsalem Hi Andrew, On Mon, Jul 29, 2013 at 09:04:31PM +0200, Andrew Lunn wrote: > On Mon, Jul 29, 2013 at 12:21:22PM -0300, Ezequiel Garcia wrote: > > If CONFIG_HAVE_CLK is not selected, then all the clk API turn out > > into stubs, so there's no need to have the ifdefs. > > The only side-effect of this patch is the extra tiny kmalloc, > > but that's not enough reason to have such ugly ifdefs all around > > the code. > > > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > > What architectures did you compile this for? PowerPC and x86 as well > as ARM? I _think_ this driver is also used there, and i _think_ > without some #ifdef, it failed to build. But maybe since then, the > common clock is now available on all architectures? > Using COMPILE_TEST=y, I've compiled with a working ARM config, as well as with a MMU-less ARM (with COMMON_CLK=n). And now that you mention x86, I've compiled it with an x86 config (with COMMON_CLK=n). I'm not sure this particular change is architecture dependent (at least nowadays), since by looking at include/linux/clk.h it seems pretty straightforward that all the operations are no-ops if HAVE_CLK=n. Am I missing something? -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ATA: sata_mv: Remove unneeded forward declaration 2013-07-29 15:21 [PATCH] ATA: sata_mv: Remove unneeded forward declaration Ezequiel Garcia 2013-07-29 15:21 ` [PATCH] ATA: sata_mv: Remove uneeded CONFIG_HAVE_CLK ifdefs Ezequiel Garcia @ 2013-07-29 15:59 ` Tejun Heo 1 sibling, 0 replies; 7+ messages in thread From: Tejun Heo @ 2013-07-29 15:59 UTC (permalink / raw) To: Ezequiel Garcia Cc: linux-ide, linux-kernel, Andrew Lunn, Thomas Petazzoni, Gregory Clement, Lior Amsalem On Mon, Jul 29, 2013 at 12:21:21PM -0300, Ezequiel Garcia wrote: > These forward declarations are no longer needed, and are probably > historical left-over. > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> Applied to libata/for-3.12. Thanks. -- tejun ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-07-29 19:44 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-07-29 15:21 [PATCH] ATA: sata_mv: Remove unneeded forward declaration Ezequiel Garcia 2013-07-29 15:21 ` [PATCH] ATA: sata_mv: Remove uneeded CONFIG_HAVE_CLK ifdefs Ezequiel Garcia 2013-07-29 16:00 ` Tejun Heo 2013-07-29 16:51 ` Ezequiel Garcia 2013-07-29 19:04 ` Andrew Lunn 2013-07-29 19:44 ` Ezequiel Garcia 2013-07-29 15:59 ` [PATCH] ATA: sata_mv: Remove unneeded forward declaration Tejun Heo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).