[RFC 09/14] MV SATA: Add per channel clk/clkdev support.

[RFC 09/14] MV SATA: Add per channel clk/clkdev support.

[Jason]

On Wed, Mar 07, 2012 at 07:38:58AM +0100, Andrew Lunn wrote:
> > Date: Tue, 6 Mar 2012 14:01:39 -0500
> > From: Jason <jason@lakedaemon.net>
> > To: Andrew Lunn <andrew@lunn.ch>
> > Cc: linux-arm-kernel at lists.infradead.org
> > Subject: Re: [RFC 09/14] MV SATA: Add per channel clk/clkdev support.
> > Message-ID: <20120306190139.GW5050@titan.lakedaemon.net>
> > Content-Type: text/plain; charset=us-ascii
> > 
> > On Tue, Mar 06, 2012 at 07:31:05AM +0100, Andrew Lunn wrote:
> > > The Orion kirkwood chips have a gatable clock per SATA channel. Add
> > 
> > +mv_sata_platform_data *sata_data)
> > >       if (sata_data->n_ports > 1)
> > >               kirkwood_clk_ctrl |= CGC_SATA1;
> > >
> > > +     orion_clkdev_add("0", "sata_mv.0", &clk_sata0);
> > > +     orion_clkdev_add("1", "sata_mv.0", &clk_sata1);
> > > +
> > 
> > This is a problem for devicetree.  With devicetree, neither
> > kirkwood_sata_init(), nor orion_sata_init() are ever called.  The same
> > is true for all the other drivers as well.
> > 
> > Could this be moved into the driver's _probe() function?
> 
> No, this cannot be in the driver. This is setting up the mapping
> between the device name and the clk the device should use. dove,
> orion5x, mv78xx0 need a different clk to kirkwood and this is hidden
> by clkdev, so the driver does not have to care.

Thanks for the clarification.

> BTW: Not all drivers which orion uses are only used by orion. So we
> have to be careful not to add orion specific stuff into some of the
> drivers. This is particularly true for Ethernet and SATA.
> 
> There are two possible solutions to clkdev:
> 
> 1) Add DT bindings for clkdev. I don't know if this is currently
>    supported?

I came to the same conclusion late last night and started poking around.
I didn't see it in any of Mike's patches.

> 2) I can refactor the code. Currently i create the clkdev entries on
>    demand. However i could just create them all just after creating
>    the clocks.

Hmmm, I wouldn't worry about it too much, absent == disabled.  Which is
what I'm thinking for the dt bindings.

> Maybe you can take a look at 1) and see if its possible, while i look
> at refactoring the code.

drivers/clk/of_clk.c ?  I'll start looking at it.

thx,

Jason.

[RFC 09/14] MV SATA: Add per channel clk/clkdev support.

[Andrew Lunn]

On Wed, Mar 07, 2012 at 08:11:37AM -0500, Jason wrote:
> On Wed, Mar 07, 2012 at 07:38:58AM +0100, Andrew Lunn wrote:
> > > Date: Tue, 6 Mar 2012 14:01:39 -0500
> > > From: Jason <jason@lakedaemon.net>
> > > To: Andrew Lunn <andrew@lunn.ch>
> > > Cc: linux-arm-kernel at lists.infradead.org
> > > Subject: Re: [RFC 09/14] MV SATA: Add per channel clk/clkdev support.
> > > Message-ID: <20120306190139.GW5050@titan.lakedaemon.net>
> > > Content-Type: text/plain; charset=us-ascii
> > > 
> > > On Tue, Mar 06, 2012 at 07:31:05AM +0100, Andrew Lunn wrote:
> > > > The Orion kirkwood chips have a gatable clock per SATA channel. Add
> > > 
> > > +mv_sata_platform_data *sata_data)
> > > >       if (sata_data->n_ports > 1)
> > > >               kirkwood_clk_ctrl |= CGC_SATA1;
> > > >
> > > > +     orion_clkdev_add("0", "sata_mv.0", &clk_sata0);
> > > > +     orion_clkdev_add("1", "sata_mv.0", &clk_sata1);
> > > > +
> > > 
> > > This is a problem for devicetree.  With devicetree, neither
> > > kirkwood_sata_init(), nor orion_sata_init() are ever called.  The same
> > > is true for all the other drivers as well.
> > > 
> > > Could this be moved into the driver's _probe() function?
> > 
> > No, this cannot be in the driver. This is setting up the mapping
> > between the device name and the clk the device should use. dove,
> > orion5x, mv78xx0 need a different clk to kirkwood and this is hidden
> > by clkdev, so the driver does not have to care.
> 
> Thanks for the clarification.
> 
> > BTW: Not all drivers which orion uses are only used by orion. So we
> > have to be careful not to add orion specific stuff into some of the
> > drivers. This is particularly true for Ethernet and SATA.
> > 
> > There are two possible solutions to clkdev:
> > 
> > 1) Add DT bindings for clkdev. I don't know if this is currently
> >    supported?
> 
> I came to the same conclusion late last night and started poking around.
> I didn't see it in any of Mike's patches.

You are missing up clk and clkdev. Mike's patches are for clk. You
need clkdev bindings.

> > 2) I can refactor the code. Currently i create the clkdev entries on
> >    demand. However i could just create them all just after creating
> >    the clocks.

> drivers/clk/of_clk.c ?  I'll start looking at it.

drivers/clk/of_clkdev.c, or maybe drivers/of/of_clkdev.c. You should
probably run this by Grant before investing too much effort. It might
be a stupid idea...

Also, most of the refactoring is done. Needs some testing and then i
can send RFCv2 patches.

    Andrew

[RFC 00/14] Generic clk for Orion platforms.

[Andrew Lunn]

This patch set starts the process of changing Orion based platforms to
use the generic clk framework. The first patch fixes compile problems
with the framework, and is expected to be dropped once fixed upstream.
Then clks are added. This is a fixed rate clk for tclk and for
kirkwood, most of the gated clocks also get a clk. The following
patches then modified the drivers to make use of these clocks,
getting, prepareing, enabling at probe time, and disableing,
unprepareing, and putting at remove time. Rather than pass the clock
frequency as platform_data, the driver now uses clk_get_rate(). This
results in some platform_data structures becoming redundant, and so
are removed.

TODO:

 0) Cleanup the white space changes, and hunks in the wrong patches.
 1) PCIe needs adopting to use clks.
 2) Strip out all references to kirkwood_clk_ctrl
 3) Find a solution for turning off unused clks - Probably needs
    framework support, which Mike has already suggested.
 4) Find a solution for tuning off unused SATA and PCIe PHYs. Maybe
    use the unprepare() method, but the current basic clk providers 
    don't support this.

If we are willing to accept a regression in power usage, we could skip
3) and 4) for the moment in order to get this code into use. That
would help the device tree work which benefits from not having to deal
with clock frequencies when setting up device driver bindings.

Boot tested on kirkwood. No testing what so ever on other Orion
platforms.

Based on 

git://git.linaro.org/people/mturquette/linux.git v3.3-rc5-clkv5


Andrew Lunn (14):
  [clk] Fix compile errors in DEFINE_CLK_GATE()
  ARM: Orion: Add clocks using the generic clk infrastructure.
  Arm: Orion: spi: Add clk/clkdev support.
  orion: spi: remove enable_clock_fix which is not used
  Arm: Orion: eth: Add clk/clkdev support.
  ARM: Orion: wdt: Add clk/clkdev support
  ARM: Orion: uart: Get the clock rate via clk_get_rate().
  ARM: Kirkwood: Remove tclk from kirkwood_asoc_platform_data.
  MV SATA: Add per channel clk/clkdev support.
  [Orion ehci] Add support for enabling clocks
  [Orion nand] Add support for clk, if there is one.
  [Orion sdio] Add support for clk.
  [Orion crypto] Add support for clk
  [Orion xor] Add support for clk

 arch/arm/Kconfig                                  |    3 +
 arch/arm/mach-dove/common.c                       |   37 ++++---
 arch/arm/mach-dove/dove-db-setup.c                |    1 -
 arch/arm/mach-kirkwood/common.c                   |  119 +++++++++++++++++----
 arch/arm/mach-kirkwood/include/mach/bridge-regs.h |   16 +++
 arch/arm/mach-kirkwood/mv88f6281gtw_ge-setup.c    |    1 -
 arch/arm/mach-kirkwood/rd88f6192-nas-setup.c      |    1 -
 arch/arm/mach-kirkwood/t5325-setup.c              |    1 -
 arch/arm/mach-kirkwood/tsx1x-common.c             |    1 -
 arch/arm/mach-mv78xx0/common.c                    |   44 +++++---
 arch/arm/mach-orion5x/common.c                    |   26 ++++-
 arch/arm/mach-orion5x/rd88f6183ap-ge-setup.c      |    1 -
 arch/arm/plat-orion/common.c                      |   88 ++++++++-------
 arch/arm/plat-orion/include/plat/audio.h          |    1 -
 arch/arm/plat-orion/include/plat/common.h         |   26 +++--
 arch/arm/plat-orion/include/plat/orion_wdt.h      |   18 ---
 drivers/ata/sata_mv.c                             |   43 +++++++-
 drivers/clk/clk.c                                 |    1 +
 drivers/crypto/mv_cesa.c                          |   15 +++
 drivers/dma/mv_xor.c                              |   16 +++
 drivers/dma/mv_xor.h                              |    1 +
 drivers/mmc/host/mvsdio.c                         |   14 +++
 drivers/mtd/nand/orion_nand.c                     |   18 +++
 drivers/net/ethernet/marvell/mv643xx_eth.c        |   46 ++++++--
 drivers/spi/spi-orion.c                           |   37 +++++--
 drivers/usb/host/ehci-orion.c                     |   16 +++
 drivers/watchdog/orion_wdt.c                      |   18 ++-
 include/linux/clk-private.h                       |    4 +-
 include/linux/mv643xx_eth.h                       |    1 -
 include/linux/spi/orion_spi.h                     |   18 ---
 30 files changed, 443 insertions(+), 189 deletions(-)
 delete mode 100644 arch/arm/plat-orion/include/plat/orion_wdt.h
 delete mode 100644 include/linux/spi/orion_spi.h

-- 
1.7.9.1

[RFC 09/14] MV SATA: Add per channel clk/clkdev support.

[Jason]

On Tue, Mar 06, 2012 at 07:31:05AM +0100, Andrew Lunn wrote:
> The Orion kirkwood chips have a gatable clock per SATA channel. Add
> code to get and enable this clk if it exists.
> 
> Signed-of-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  arch/arm/mach-kirkwood/common.c           |    3 ++
>  arch/arm/plat-orion/common.c              |    4 +-
>  arch/arm/plat-orion/include/plat/common.h |    3 ++
>  drivers/ata/sata_mv.c                     |   43 +++++++++++++++++++++++++---
>  4 files changed, 46 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/mach-kirkwood/common.c b/arch/arm/mach-kirkwood/common.c
> index 7588812..855540a 100644
> --- a/arch/arm/mach-kirkwood/common.c
> +++ b/arch/arm/mach-kirkwood/common.c
> @@ -244,6 +244,9 @@ void __init kirkwood_sata_init(struct mv_sata_platform_data *sata_data)
>  	if (sata_data->n_ports > 1)
>  		kirkwood_clk_ctrl |= CGC_SATA1;
>  
> +	orion_clkdev_add("0", "sata_mv.0", &clk_sata0);
> +	orion_clkdev_add("1", "sata_mv.0", &clk_sata1);
> +

This is a problem for devicetree.  With devicetree, neither
kirkwood_sata_init(), nor orion_sata_init() are ever called.  The same
is true for all the other drivers as well.

Could this be moved into the driver's _probe() function?

thx,

Jason.

>  	orion_sata_init(sata_data, SATA_PHYS_BASE, IRQ_KIRKWOOD_SATA);
>  }
>  
> diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
> index 05f078a..06f1e7a 100644
> --- a/arch/arm/plat-orion/common.c
> +++ b/arch/arm/plat-orion/common.c
> @@ -23,8 +23,8 @@
>  #include <plat/ehci-orion.h>
>  
>  /* Create a clkdev entry for a given device/clk */
> -static void orion_clkdev_add(const char *con_id, const char *dev_id,
> -			     struct clk *clk)
> +void orion_clkdev_add(const char *con_id, const char *dev_id,
> +		      struct clk *clk)
>  {
>  	struct clk_lookup *cl;
>  
> diff --git a/arch/arm/plat-orion/include/plat/common.h b/arch/arm/plat-orion/include/plat/common.h
> index a70976e..b739343 100644
> --- a/arch/arm/plat-orion/include/plat/common.h
> +++ b/arch/arm/plat-orion/include/plat/common.h
> @@ -107,4 +107,7 @@ void __init orion_crypto_init(unsigned long mapbase,
>  			      unsigned long srambase,
>  			      unsigned long sram_size,
>  			      unsigned long irq);
> +
> +void orion_clkdev_add(const char *con_id, const char *dev_id,
> +		      struct clk *clk);
>  #endif
> diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
> index 38950ea..0f4a7e9 100644
> --- a/drivers/ata/sata_mv.c
> +++ b/drivers/ata/sata_mv.c
> @@ -553,6 +553,7 @@ struct mv_host_priv {
>  
>  #if defined(CONFIG_HAVE_CLK)
>  	struct clk		*clk;
> +	struct clk              **port_clks;
>  #endif
>  	/*
>  	 * These consistent DMA memory pools give us guaranteed
> @@ -4026,6 +4027,9 @@ static int mv_platform_probe(struct platform_device *pdev)
>  	struct mv_host_priv *hpriv;
>  	struct resource *res;
>  	int n_ports, rc;
> +#if defined(CONFIG_HAVE_CLK)
> +	int port;
> +#endif
>  
>  	ata_print_version_once(&pdev->dev, DRV_VERSION);
>  
> @@ -4053,6 +4057,13 @@ 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;
> @@ -4065,9 +4076,18 @@ static int mv_platform_probe(struct platform_device *pdev)
>  #if defined(CONFIG_HAVE_CLK)
>  	hpriv->clk = clk_get(&pdev->dev, NULL);
>  	if (IS_ERR(hpriv->clk))
> -		dev_notice(&pdev->dev, "cannot get clkdev\n");
> -	else
> -		clk_enable(hpriv->clk);
> +		dev_notice(&pdev->dev, "cannot get optional clkdev\n");
> +	else {
> +		clk_prepare_enable(hpriv->clk);
> +	}
> +	for (port = 0; port < n_ports; port++) {
> +		char port_number[16];
> +		sprintf(port_number, "%d", port);
> +		hpriv->port_clks[port] = clk_get(&pdev->dev, port_number);
> +		if (!IS_ERR(hpriv->port_clks[port])) {
> +			clk_prepare_enable(hpriv->port_clks[port]);
> +		}
> +	}
>  #endif
>  
>  	/*
> @@ -4097,9 +4117,15 @@ static int mv_platform_probe(struct platform_device *pdev)
>  err:
>  #if defined(CONFIG_HAVE_CLK)
>  	if (!IS_ERR(hpriv->clk)) {
> -		clk_disable(hpriv->clk);
> +		clk_disable_unprepare(hpriv->clk);
>  		clk_put(hpriv->clk);
>  	}
> +	for (port = 0; port < n_ports; port++) {
> +		if (!IS_ERR(hpriv->port_clks[port])) {
> +			clk_disable_unprepare(hpriv->port_clks[port]);
> +			clk_put(hpriv->port_clks[port]);
> +		}
> +	}
>  #endif
>  
>  	return rc;
> @@ -4118,14 +4144,21 @@ static int __devexit 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(hpriv->clk);
> +		clk_disable_unprepare(hpriv->clk);
>  		clk_put(hpriv->clk);
>  	}
> +	for (port = 0; port < host->n_ports; port++) {
> +		if (!IS_ERR(hpriv->port_clks[port])) {
> +			clk_disable_unprepare(hpriv->port_clks[port]);
> +			clk_put(hpriv->port_clks[port]);
> +		}
> +	}
>  #endif
>  	return 0;
>  }
> -- 
> 1.7.9.1
>