On Wed, Nov 20, 2013 at 11:17:35AM +0100, Marek Vasut wrote:
> The same code for enabling and disabling SATA clock was found in multiple
> places in the driver. Implement functions that enable/disable the SATA clock
> and use them in such places instead of duplicating the code.
>
> Signed-off-by: Marek Vasut <marex@xxxxxxx>
> Cc: Shawn Guo <shawn.guo@xxxxxxxxxx>
> Cc: Richard Zhu <r65037@xxxxxxxxxxxxx>
> Cc: Tejun Heo <tj@xxxxxxxxxx>
> Cc: Linux-IDE <linux-ide@xxxxxxxxxxxxxxx>
> ---
> drivers/ata/ahci_imx.c | 130 +++++++++++++++++++++++++++----------------------
> 1 file changed, 71 insertions(+), 59 deletions(-)
>
> V2: Move the OR sign from the begining of the line to the end of the line in
> the regmap_update_bits() call.
> V3: Move the PHY configuration programming out of the clock_enable function
> and into the probe() function.
> V4: Rebase on top of new 1/6 patch.
> Remove clk_prepare_enable() being called twice in imx_sata_clock_enable()
> since it was a rebase artifact.
>
> diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
> index 478921b..8e0dbd4 100644
> --- a/drivers/ata/ahci_imx.c
> +++ b/drivers/ata/ahci_imx.c
> @@ -47,6 +47,34 @@ static int ahci_imx_hotplug;
> module_param_named(hotplug, ahci_imx_hotplug, int, 0644);
> MODULE_PARM_DESC(hotplug, "AHCI IMX hot-plug support (0=Don't support, 1=support)");
>
> +static int imx_sata_clock_enable(struct device *dev)
> +{
> + struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
> + int ret;
> +
> + ret = clk_prepare_enable(imxpriv->sata_ref_clk);
> + if (ret < 0) {
> + dev_err(dev, "prepare-enable sata_ref clock err:%d\n", ret);
> + return ret;
> + }
> +
> + regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
> + IMX6Q_GPR13_SATA_MPLL_CLK_EN,
> + IMX6Q_GPR13_SATA_MPLL_CLK_EN);
> +
> + return 0;
> +}
> +
> +static void imx_sata_clock_disable(struct device *dev)
> +{
> + struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
> +
> + regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
> + IMX6Q_GPR13_SATA_MPLL_CLK_EN,
> + !IMX6Q_GPR13_SATA_MPLL_CLK_EN);
> + clk_disable_unprepare(imxpriv->sata_ref_clk);
> +}
> +
> static void ahci_imx_error_handler(struct ata_port *ap)
> {
> u32 reg_val;
> @@ -72,10 +100,7 @@ static void ahci_imx_error_handler(struct ata_port *ap)
> */
> reg_val = readl(mmio + PORT_PHY_CTL);
> writel(reg_val | PORT_PHY_CTL_PDDQ_LOC, mmio + PORT_PHY_CTL);
> - regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
> - IMX6Q_GPR13_SATA_MPLL_CLK_EN,
> - !IMX6Q_GPR13_SATA_MPLL_CLK_EN);
> - clk_disable_unprepare(imxpriv->sata_ref_clk);
> + imx_sata_clock_disable(ap->dev);
> imxpriv->no_device = true;
> }
>
> @@ -97,45 +122,10 @@ static int imx6q_sata_init(struct device *dev, void __iomem *mmio)
> unsigned int reg_val;
> struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
>
> - imxpriv->gpr =
> - syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
> - if (IS_ERR(imxpriv->gpr)) {
> - dev_err(dev, "failed to find fsl,imx6q-iomux-gpr regmap\n");
> - return PTR_ERR(imxpriv->gpr);
> - }
> -
> - ret = clk_prepare_enable(imxpriv->sata_ref_clk);
> - if (ret < 0) {
> - dev_err(dev, "prepare-enable sata_ref clock err:%d\n", ret);
> + ret = imx_sata_clock_enable(dev);
> + if (ret < 0)
> return ret;
> - }
>
> - /*
> - * set PHY Paremeters, two steps to configure the GPR13,
> - * one write for rest of parameters, mask of first write
> - * is 0x07fffffd, and the other one write for setting
> - * the mpll_clk_en.
> - */
> - regmap_update_bits(imxpriv->gpr, 0x34, IMX6Q_GPR13_SATA_RX_EQ_VAL_MASK
> - | IMX6Q_GPR13_SATA_RX_LOS_LVL_MASK
> - | IMX6Q_GPR13_SATA_RX_DPLL_MODE_MASK
> - | IMX6Q_GPR13_SATA_SPD_MODE_MASK
> - | IMX6Q_GPR13_SATA_MPLL_SS_EN
> - | IMX6Q_GPR13_SATA_TX_ATTEN_MASK
> - | IMX6Q_GPR13_SATA_TX_BOOST_MASK
> - | IMX6Q_GPR13_SATA_TX_LVL_MASK
> - | IMX6Q_GPR13_SATA_TX_EDGE_RATE
> - | IMX6Q_GPR13_SATA_MPLL_CLK_EN
> - , IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB
> - | IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2M
> - | IMX6Q_GPR13_SATA_RX_DPLL_MODE_2P_4F
> - | IMX6Q_GPR13_SATA_SPD_MODE_3P0G
> - | IMX6Q_GPR13_SATA_MPLL_SS_EN
> - | IMX6Q_GPR13_SATA_TX_ATTEN_9_16
> - | IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB
> - | IMX6Q_GPR13_SATA_TX_LVL_1_025_V);
> - regmap_update_bits(imxpriv->gpr, 0x34, IMX6Q_GPR13_SATA_MPLL_CLK_EN,
> - IMX6Q_GPR13_SATA_MPLL_CLK_EN);
> usleep_range(100, 200);
For the third time, can this usleep_range() be moved together with
IMX6Q_GPR13_SATA_MPLL_CLK_EN setting into imx_sata_clock_enable(), since
this sleep is specifically for SATA MPLL enabling?
Shawn
>
> /*
> @@ -164,11 +154,7 @@ static int imx6q_sata_init(struct device *dev, void __iomem *mmio)
>
> static void imx6q_sata_exit(struct device *dev)
> {
> - struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
> -
> - regmap_update_bits(imxpriv->gpr, 0x34, IMX6Q_GPR13_SATA_MPLL_CLK_EN,
> - !IMX6Q_GPR13_SATA_MPLL_CLK_EN);
> - clk_disable_unprepare(imxpriv->sata_ref_clk);
> + imx_sata_clock_disable(dev);
> }
>
> static int imx_ahci_suspend(struct device *dev)
> @@ -179,12 +165,8 @@ static int imx_ahci_suspend(struct device *dev)
> * If no_device is set, The CLKs had been gated off in the
> * initialization so don't do it again here.
> */
> - if (!imxpriv->no_device) {
> - regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
> - IMX6Q_GPR13_SATA_MPLL_CLK_EN,
> - !IMX6Q_GPR13_SATA_MPLL_CLK_EN);
> - clk_disable_unprepare(imxpriv->sata_ref_clk);
> - }
> + if (!imxpriv->no_device)
> + imx_sata_clock_disable(dev);
>
> return 0;
> }
> @@ -195,15 +177,10 @@ static int imx_ahci_resume(struct device *dev)
> int ret;
>
> if (!imxpriv->no_device) {
> - ret = clk_prepare_enable(imxpriv->sata_ref_clk);
> - if (ret < 0) {
> - dev_err(dev, "pre-enable sata_ref clock err:%d\n", ret);
> + ret = imx_sata_clock_enable(dev);
> + if (ret < 0)
> return ret;
> - }
>
> - regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
> - IMX6Q_GPR13_SATA_MPLL_CLK_EN,
> - IMX6Q_GPR13_SATA_MPLL_CLK_EN);
> usleep_range(1000, 2000);
> }
>
> @@ -290,6 +267,41 @@ static int imx_ahci_probe(struct platform_device *pdev)
> ahci_dev->dma_mask = &ahci_dev->coherent_dma_mask;
> ahci_dev->of_node = dev->of_node;
>
> + imxpriv->gpr =
> + syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
> +
> + if (IS_ERR(imxpriv->gpr)) {
> + dev_err(dev, "failed to find fsl,imx6q-iomux-gpr regmap\n");
> + ret = PTR_ERR(imxpriv->gpr);
> + goto err_out;
> + }
> +
> + /*
> + * Set PHY Paremeters, two steps to configure the GPR13,
> + * one write for rest of parameters, mask of first write
> + * is 0x07fffffd, and the other one write for setting
> + * the mpll_clk_en happens in imx_sata_clock_enable().
> + */
> + regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
> + IMX6Q_GPR13_SATA_RX_EQ_VAL_MASK |
> + IMX6Q_GPR13_SATA_RX_LOS_LVL_MASK |
> + IMX6Q_GPR13_SATA_RX_DPLL_MODE_MASK |
> + IMX6Q_GPR13_SATA_SPD_MODE_MASK |
> + IMX6Q_GPR13_SATA_MPLL_SS_EN |
> + IMX6Q_GPR13_SATA_TX_ATTEN_MASK |
> + IMX6Q_GPR13_SATA_TX_BOOST_MASK |
> + IMX6Q_GPR13_SATA_TX_LVL_MASK |
> + IMX6Q_GPR13_SATA_TX_EDGE_RATE |
> + IMX6Q_GPR13_SATA_MPLL_CLK_EN,
> + IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB |
> + IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2M |
> + IMX6Q_GPR13_SATA_RX_DPLL_MODE_2P_4F |
> + IMX6Q_GPR13_SATA_SPD_MODE_3P0G |
> + IMX6Q_GPR13_SATA_MPLL_SS_EN |
> + IMX6Q_GPR13_SATA_TX_ATTEN_9_16 |
> + IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB |
> + IMX6Q_GPR13_SATA_TX_LVL_1_025_V);
> +
> ret = platform_device_add_resources(ahci_pdev, res, 2);
> if (ret)
> goto err_out;
> --
> 1.8.4.2
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
