Hi,
On Saturday, February 22, 2014 04:53:31 PM Hans de Goede wrote:
> The allwinner-sun4i AHCI controller needs 2 clocks to be enabled and the
> imx AHCI controller needs 3 clocks to be enabled.
>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
> .../devicetree/bindings/ata/ahci-platform.txt | 1 +
> drivers/ata/ahci.h | 3 +-
> drivers/ata/ahci_platform.c | 119 ++++++++++++++++-----
> include/linux/ahci_platform.h | 4 +
> 4 files changed, 99 insertions(+), 28 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
> index 89de156..3ced07d 100644
> --- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
> +++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
> @@ -10,6 +10,7 @@ Required properties:
>
> Optional properties:
> - dma-coherent : Present if dma operations are coherent
> +- clocks : a list of phandle + clock specifier pairs
>
> Example:
> sata@ffe08000 {
> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
> index 64d1a99..c12862b 100644
> --- a/drivers/ata/ahci.h
> +++ b/drivers/ata/ahci.h
> @@ -51,6 +51,7 @@
>
> enum {
> AHCI_MAX_PORTS = 32,
> + AHCI_MAX_CLKS = 3,
> AHCI_MAX_SG = 168, /* hardware max is 64K */
> AHCI_DMA_BOUNDARY = 0xffffffff,
> AHCI_MAX_CMDS = 32,
> @@ -321,7 +322,7 @@ struct ahci_host_priv {
> u32 em_loc; /* enclosure management location */
> u32 em_buf_sz; /* EM buffer size in byte */
> u32 em_msg_type; /* EM message type */
> - struct clk *clk; /* Only for platforms supporting clk */
> + struct clk *clks[AHCI_MAX_CLKS]; /* Optional */
> void *plat_data; /* Other platform data */
> /*
> * Optional ahci_start_engine override, if not set this gets set to the
> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
> index 4b231ba..609975d 100644
> --- a/drivers/ata/ahci_platform.c
> +++ b/drivers/ata/ahci_platform.c
> @@ -87,6 +87,66 @@ static struct scsi_host_template ahci_platform_sht = {
> AHCI_SHT("ahci_platform"),
> };
>
> +/**
> + * ahci_platform_enable_clks - Enable platform clocks
> + * @hpriv: host private area to store config values
> + *
> + * This function enables all the clks found in hpriv->clks, starting
> + * at index 0. If any clk fails to enable it disables all the clks
> + * already enabled in reverse order, and then returns an error.
> + *
> + * LOCKING:
> + * None.
> + *
> + * RETURNS:
> + * 0 on success otherwise a negative error code
> + */
> +int ahci_platform_enable_clks(struct ahci_host_priv *hpriv)
> +{
> + int c, rc;
> +
> + for (c = 0; c < AHCI_MAX_CLKS && hpriv->clks[c]; c++) {
> + rc = clk_prepare_enable(hpriv->clks[c]);
> + if (rc)
> + goto disable_unprepare_clk;
> + }
> + return 0;
> +
> +disable_unprepare_clk:
> + while (--c >= 0)
> + clk_disable_unprepare(hpriv->clks[c]);
> + return rc;
> +}
> +EXPORT_SYMBOL_GPL(ahci_platform_enable_clks);
> +
> +/**
> + * ahci_platform_disable_clks - Disable platform clocks
> + * @hpriv: host private area to store config values
> + *
> + * This function disables all the clks found in hpriv->clks, in reverse
> + * order of ahci_platform_enable_clks (starting at the end of the array).
> + *
> + * LOCKING:
> + * None.
> + */
> +void ahci_platform_disable_clks(struct ahci_host_priv *hpriv)
> +{
> + int c;
> +
> + for (c = AHCI_MAX_CLKS - 1; c >= 0; c--)
> + if (hpriv->clks[c])
> + clk_disable_unprepare(hpriv->clks[c]);
> +}
> +EXPORT_SYMBOL_GPL(ahci_platform_disable_clks);
> +
> +static void ahci_put_clks(struct ahci_host_priv *hpriv)
> +{
> + int c;
> +
> + for (c = 0; c < AHCI_MAX_CLKS && hpriv->clks[c]; c++)
> + clk_put(hpriv->clks[c]);
> +}
> +
> static int ahci_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -97,6 +157,7 @@ static int ahci_probe(struct platform_device *pdev)
> struct ahci_host_priv *hpriv;
> struct ata_host *host;
> struct resource *mem;
> + struct clk *clk;
> int irq;
> int n_ports;
> int i;
> @@ -131,17 +192,31 @@ static int ahci_probe(struct platform_device *pdev)
> return -ENOMEM;
> }
>
> - hpriv->clk = clk_get(dev, NULL);
> - if (IS_ERR(hpriv->clk)) {
> - dev_err(dev, "can't get clock\n");
> - } else {
> - rc = clk_prepare_enable(hpriv->clk);
> - if (rc) {
> - dev_err(dev, "clock prepare enable failed");
> - goto free_clk;
> + for (i = 0; i < AHCI_MAX_CLKS; i++) {
> + /*
> + * For now we must use clk_get(dev, NULL) for the first clock,
> + * because some platforms (da850, spear13xx) are not yet
> + * converted to use devicetree for clocks. For new platforms
> + * this is equivalent to of_clk_get(dev->of_node, 0).
> + */
> + if (i == 0)
> + clk = clk_get(dev, NULL);
> + else
> + clk = of_clk_get(dev->of_node, i);
> +
> + if (IS_ERR(clk)) {
> + rc = PTR_ERR(clk);
> + if (rc == -EPROBE_DEFER)
> + goto free_clk;
> + break;
> }
> + hpriv->clks[i] = clk;
> }
>
> + rc = ahci_enable_clks(dev, hpriv);
This should be ahci_platform_enable_clks().
> + if (rc)
> + goto free_clk;
> +
> /*
> * Some platforms might need to prepare for mmio region access,
> * which could be done in the following init call. So, the mmio
> @@ -222,11 +297,9 @@ pdata_exit:
> if (pdata && pdata->exit)
> pdata->exit(dev);
> disable_unprepare_clk:
> - if (!IS_ERR(hpriv->clk))
> - clk_disable_unprepare(hpriv->clk);
> + ahci_disable_clks(hpriv);
ahci_platform_disable_clks()
> free_clk:
> - if (!IS_ERR(hpriv->clk))
> - clk_put(hpriv->clk);
> + ahci_put_clks(hpriv);
> return rc;
> }
>
> @@ -239,10 +312,8 @@ static void ahci_host_stop(struct ata_host *host)
> if (pdata && pdata->exit)
> pdata->exit(dev);
>
> - if (!IS_ERR(hpriv->clk)) {
> - clk_disable_unprepare(hpriv->clk);
> - clk_put(hpriv->clk);
> - }
> + ahci_disable_clks(hpriv);
ahci_platform_disable_clks()
> + ahci_put_clks(hpriv);
> }
>
> #ifdef CONFIG_PM_SLEEP
> @@ -277,8 +348,7 @@ static int ahci_suspend(struct device *dev)
> if (pdata && pdata->suspend)
> return pdata->suspend(dev);
>
> - if (!IS_ERR(hpriv->clk))
> - clk_disable_unprepare(hpriv->clk);
> + ahci_disable_clks(hpriv);
ahci_platform_disable_clks()
> return 0;
> }
> @@ -290,13 +360,9 @@ static int ahci_resume(struct device *dev)
> struct ahci_host_priv *hpriv = host->private_data;
> int rc;
>
> - if (!IS_ERR(hpriv->clk)) {
> - rc = clk_prepare_enable(hpriv->clk);
> - if (rc) {
> - dev_err(dev, "clock prepare enable failed");
> - return rc;
> - }
> - }
> + rc = ahci_enable_clks(dev, hpriv);
ahci_platform_enable_clks()
> + if (rc)
> + return rc;
>
> if (pdata && pdata->resume) {
> rc = pdata->resume(dev);
> @@ -317,8 +383,7 @@ static int ahci_resume(struct device *dev)
> return 0;
>
> disable_unprepare_clk:
> - if (!IS_ERR(hpriv->clk))
> - clk_disable_unprepare(hpriv->clk);
> + ahci_disable_clks(hpriv);
ahci_platform_disable_clks()
[...]
All code in question gets rewritten in patch #4/15 so the actual
problem is the broken bisectability between patch #2 and #4.
It seems too late to fix it now but in the future please remember
to build-test each patch separately (in addition to testing the whole
patchset).
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
