Convert MSM to common clock framework

Add calls to clk_prepare and unprepare so that MSM can migrate to the common clock framework.

Cc Felipe Balbi <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

---
 drivers/usb/otg/msm_otg.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

drivers/usb/otg/msm_otg.c +0 -0

index 9f5fc90..2ae0639 100644
@@ -514,13 +514,13 @@ static int msm_otg_suspend(struct msm_otg *motg)
                        motg->pdata->otg_control == OTG_PMIC_CONTROL)

Quoted 8 lines · Stephen Boyd

drivers/usb/otg/msm_otg.c +0 -0

index 9f5fc90..2ae0639 100644
@@ -514,13 +514,13 @@ static int msm_otg_suspend(struct msm_otg *motg)
                        motg->pdata->otg_control == OTG_PMIC_CONTROL)
                writel(readl(USB_PHY_CTRL) | PHY_RETEN, USB_PHY_CTRL);
 

drivers/usb/otg/msm_otg.c static int msm_otg_suspend(struct msm_otg *motg) · line 521

Quoted 8 lines

drivers/usb/otg/msm_otg.c +3 -3

-       clk_disable(motg->pclk);
-       clk_disable(motg->clk);
+       clk_disable_unprepare(motg->pclk);
+       clk_disable_unprepare(motg->clk);
        if (motg->core_clk)
-               clk_disable(motg->core_clk);
+               clk_disable_unprepare(motg->core_clk);
 

I was under assumption that system suspend/resume might be called from the interrupt context. If that is case then its not appropriate to call clk_prepare/unprepare here right? or my understanding is not correct????

Quoted 9 lines · Pankaj Jangra

Quoted 8 lines · Stephen Boyd

drivers/usb/otg/msm_otg.c +0 -0

index 9f5fc90..2ae0639 100644
@@ -514,13 +514,13 @@ static int msm_otg_suspend(struct msm_otg *motg)
                        motg->pdata->otg_control == OTG_PMIC_CONTROL)
                writel(readl(USB_PHY_CTRL) | PHY_RETEN, USB_PHY_CTRL);
 

Quoted 11 lines

drivers/usb/otg/msm_otg.c static int msm_otg_suspend(struct msm_otg *motg) · line 521

Quoted 8 lines

drivers/usb/otg/msm_otg.c +3 -3

-       clk_disable(motg->pclk);
-       clk_disable(motg->clk);
+       clk_disable_unprepare(motg->pclk);
+       clk_disable_unprepare(motg->clk);
        if (motg->core_clk)
-               clk_disable(motg->core_clk);
+               clk_disable_unprepare(motg->core_clk);
 

I was under assumption that system suspend/resume might be called from the interrupt context. If that is case then its not appropriate to call clk_prepare/unprepare here right? or my understanding is not correct????

Well we call disable_irq() at the top of this suspend function so I suspect your understanding is incorrect. disable_irq() can call schedule(). There are also a bunch of regulator calls in msm_hsusb_ldo_set_mode() that this suspend function calls (almost all regulator calls grab a mutex).

Add calls to clk_prepare and unprepare so that MSM can migrate to the common clock framework. We never unprepare the clocks until driver remove because the clocks are enabled and disabled in irq context. Finer grained power management is possible in the future via runtime power management techniques.

Signed-off-by Stephen Boyd<[email protected]>

---
  drivers/iommu/msm_iommu_dev.c | 17 ++++++++++-------
  1 file changed, 10 insertions(+), 7 deletions(-)

drivers/iommu/msm_iommu_dev.c +3 -3

index 8e8fb07..d344f6a 100644
@@ -160,7 +160,7 @@ static int msm_iommu_probe(struct platform_device *pdev)
  		goto fail;
  	}
 
-	ret = clk_enable(iommu_pclk);
+	ret = clk_prepare_enable(iommu_pclk);
  	if (ret)
  		goto fail_enable;
 

@@ -170,7 +170,7 @@ static int msm_iommu_probe(struct platform_device *pdev)
  		if (clk_get_rate(iommu_clk) == 0)
  			clk_set_min_rate(iommu_clk, 1);
 
-		ret = clk_enable(iommu_clk);
+		ret = clk_prepare_enable(iommu_clk);
  		if (ret) {
  			clk_put(iommu_clk);
  			goto fail_pclk;

@@ -261,7 +261,7 @@ fail_clk:
  		clk_put(iommu_clk);
  	}
  fail_pclk:
-	clk_disable(iommu_pclk);
+	clk_disable_unprepare(iommu_pclk);
  fail_enable:
  	clk_put(iommu_pclk);
  fail:

drivers/iommu/msm_iommu_dev.c +3 -3

  		clk_put(drv->pclk);
  		memset(drv, 0, sizeof(*drv));
  		kfree(drv);

@@ -314,14 +317,14 @@ static int msm_iommu_ctx_probe(struct platform_device *pdev)
  	INIT_LIST_HEAD(&ctx_drvdata->attached_elm);
  	platform_set_drvdata(pdev, ctx_drvdata);
 
-	ret = clk_enable(drvdata->pclk);
+	ret = clk_prepare_enable(drvdata->pclk);
  	if (ret)
  		goto fail;
 
  	if (drvdata->clk) {
-		ret = clk_enable(drvdata->clk);
+		ret = clk_prepare_enable(drvdata->clk);
  		if (ret) {
-			clk_disable(drvdata->pclk);
+			clk_disable_unprepare(drvdata->pclk);
  			goto fail;
  		}
  	}

drivers/iommu/msm_iommu_dev.c static int msm_iommu_remove(struct · line 283

Quoted 11 lines

drivers/iommu/msm_iommu_dev.c +4 -1

@@ -275,8 +275,11 @@ static int msm_iommu_remove(struct
 platform_device *pdev)
 
      drv = platform_get_drvdata(pdev);
      if (drv) {
-        if (drv->clk)
+        if (drv->clk) {
+            clk_unprepare(drv->clk);
              clk_put(drv->clk);
+        }
+        clk_unprepare(drv->pclk);

Are these changes right? Every other clk API change in this patch is using the combined prepare_enable/disable_unprepare() calls. So, when would we end up at this location with the clocks prepared but not enabled?

Also, what if the device gets probed and then immediately removed. Will it work correctly?

Nobody is using this API upstream and it's just contributing cruft. Remove it so the MSM clock API is closer to the generic struct clock API.

Cc Saravana Kannan<[email protected]>

Signed-off-by Stephen Boyd<[email protected]>

---
  arch/arm/mach-msm/clock-pcom.c       | 10 ----------
  arch/arm/mach-msm/clock.c            |  8 --------
  arch/arm/mach-msm/clock.h            |  6 ------
  arch/arm/mach-msm/include/mach/clk.h |  3 ---
  4 files changed, 27 deletions(-)

arch/arm/mach-msm/clock-pcom.c +0 -10

index a52c970..bb75f8e 100644
@@ -85,15 +85,6 @@ static int pc_clk_set_max_rate(unsigned id, unsigned rate)
  		return (int)id<  0 ? -EINVAL : 0;
  }
 
-static int pc_clk_set_flags(unsigned id, unsigned flags)
-{
-	int rc = msm_proc_comm(PCOM_CLKCTL_RPC_SET_FLAGS,&id,&flags);
-	if (rc<  0)
-		return rc;
-	else
-		return (int)id<  0 ? -EINVAL : 0;
-}
-
  static unsigned pc_clk_get_rate(unsigned id)
  {
  	if (msm_proc_comm(PCOM_CLKCTL_RPC_RATE,&id, NULL))

@@ -130,7 +121,6 @@ struct clk_ops clk_ops_pcom = {
  	.set_rate = pc_clk_set_rate,
  	.set_min_rate = pc_clk_set_min_rate,
  	.set_max_rate = pc_clk_set_max_rate,
-	.set_flags = pc_clk_set_flags,
  	.get_rate = pc_clk_get_rate,
  	.is_enabled = pc_clk_is_enabled,
  	.round_rate = pc_clk_round_rate,

arch/arm/mach-msm/clock.c +0 -8

index d9145df..5fac2df 100644
@@ -121,14 +121,6 @@ struct clk *clk_get_parent(struct clk *clk)
  }
  EXPORT_SYMBOL(clk_get_parent);
 
-int clk_set_flags(struct clk *clk, unsigned long flags)
-{
-	if (clk == NULL || IS_ERR(clk))
-		return -EINVAL;
-	return clk->ops->set_flags(clk->id, flags);
-}
-EXPORT_SYMBOL(clk_set_flags);
-
  /* EBI1 is the only shared clock that several clients want to vote on as of
   * this commit. If this changes in the future, then it might be better to
   * make clk_min_rate handle the voting or make ebi1_clk_set_min_rate more

arch/arm/mach-msm/clock.h +0 -6

index 2c007f6..a25ff58 100644
@@ -21,11 +21,6 @@
  #include<linux/list.h>
  #include<mach/clk.h>
 
-#define CLKFLAG_INVERT			0x00000001
-#define CLKFLAG_NOINVERT		0x00000002
-#define CLKFLAG_NONEST			0x00000004
-#define CLKFLAG_NORESET			0x00000008
-
  #define CLK_FIRST_AVAILABLE_FLAG	0x00000100
  #define CLKFLAG_AUTO_OFF		0x00000200
  #define CLKFLAG_MIN			0x00000400

@@ -39,7 +34,6 @@ struct clk_ops {
  	int (*set_rate)(unsigned id, unsigned rate);
  	int (*set_min_rate)(unsigned id, unsigned rate);
  	int (*set_max_rate)(unsigned id, unsigned rate);
-	int (*set_flags)(unsigned id, unsigned flags);
  	unsigned (*get_rate)(unsigned id);
  	unsigned (*is_enabled)(unsigned id);
  	long (*round_rate)(unsigned id, unsigned rate);

arch/arm/mach-msm/include/mach/clk.h +0 -0

index e8d3842..5f1c37d 100644

There are no users of this API anymore so let's just remove it. If a need arises in the future we can extend the common clock API to handle it.

Cc Saravana Kannan<[email protected]>

Signed-off-by Stephen Boyd<[email protected]>

---
  arch/arm/mach-msm/clock-debug.c      |  9 +--------
  arch/arm/mach-msm/clock.c            | 12 ------------
  arch/arm/mach-msm/include/mach/clk.h |  6 ------
  3 files changed, 1 insertion(+), 26 deletions(-)

arch/arm/mach-msm/clock-debug.c +0 -0

index 4886404..c4b34d7 100644
@@ -25,14 +25,7 @@ static int clock_debug_rate_set(void *data, u64 val)
  	struct clk *clock = data;
  	int ret;

arch/arm/mach-msm/clock-debug.c +0 -0

  	if (ret != 0)
  		printk(KERN_ERR "clk_set%s_rate failed (%d)\n",
  			(clock->flags&  CLK_MIN) ? "_min" : "", ret);

arch/arm/mach-msm/clock.c +0 -12

index 5fac2df..8d07b25 100644
@@ -97,18 +97,6 @@ long clk_round_rate(struct clk *clk, unsigned long rate)
  }
  EXPORT_SYMBOL(clk_round_rate);
 
-int clk_set_min_rate(struct clk *clk, unsigned long rate)
-{
-	return clk->ops->set_min_rate(clk->id, rate);
-}
-EXPORT_SYMBOL(clk_set_min_rate);
-
-int clk_set_max_rate(struct clk *clk, unsigned long rate)
-{
-	return clk->ops->set_max_rate(clk->id, rate);
-}
-EXPORT_SYMBOL(clk_set_max_rate);
-
  int clk_set_parent(struct clk *clk, struct clk *parent)
  {
  	return -ENOSYS;

arch/arm/mach-msm/include/mach/clk.h +0 -6

index 5f1c37d..fd4f4a7 100644
@@ -25,12 +25,6 @@ enum clk_reset_action {
 
  struct clk;
 
-/* Rate is minimum clock rate in Hz */
-int clk_set_min_rate(struct clk *clk, unsigned long rate);
-
-/* Rate is maximum clock rate in Hz */
-int clk_set_max_rate(struct clk *clk, unsigned long rate);
-
  /* Assert/Deassert reset to a hardware block associated with a clock */
  int clk_reset(struct clk *clk, enum clk_reset_action action);
 

This file is not used outside of the two users in the clock-7x30 array. Those two clocks are virtual "source" clocks that don't really need to exist outside of the clock driver. Let's remove them from the array, since they're not doing anything anyway, and then remove the clock-7x30.h include file along with it.

Cc Saravana Kannan <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

---
 arch/arm/mach-msm/clock-7x30.h      | 155 ------------------------------------
 arch/arm/mach-msm/devices-msm7x30.c |   2 -
 2 files changed, 157 deletions(-)
 delete mode 100644 arch/arm/mach-msm/clock-7x30.h

arch/arm/mach-msm/clock-7x30.h deleted +0 -155

deleted file mode 100644
index 1410445..0000000
@@ -1,155 +0,0 @@
-/* Copyright (c) 2009, Code Aurora Forum. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#ifndef __ARCH_ARM_MACH_MSM_CLOCK_7X30_H
-#define __ARCH_ARM_MACH_MSM_CLOCK_7X30_H
-
-enum {
-       L_7X30_NONE_CLK = -1,
-       L_7X30_ADM_CLK,
-       L_7X30_I2C_CLK,
-       L_7X30_I2C_2_CLK,
-       L_7X30_QUP_I2C_CLK,
-       L_7X30_UART1DM_CLK,
-       L_7X30_UART1DM_P_CLK,
-       L_7X30_UART2DM_CLK,
-       L_7X30_UART2DM_P_CLK,
-       L_7X30_EMDH_CLK,
-       L_7X30_EMDH_P_CLK,
-       L_7X30_PMDH_CLK,
-       L_7X30_PMDH_P_CLK,
-       L_7X30_GRP_2D_CLK,
-       L_7X30_GRP_2D_P_CLK,
-       L_7X30_GRP_3D_SRC_CLK,
-       L_7X30_GRP_3D_CLK,
-       L_7X30_GRP_3D_P_CLK,
-       L_7X30_IMEM_CLK,
-       L_7X30_SDC1_CLK,
-       L_7X30_SDC1_P_CLK,
-       L_7X30_SDC2_CLK,
-       L_7X30_SDC2_P_CLK,
-       L_7X30_SDC3_CLK,
-       L_7X30_SDC3_P_CLK,
-       L_7X30_SDC4_CLK,
-       L_7X30_SDC4_P_CLK,
-       L_7X30_MDP_CLK,
-       L_7X30_MDP_P_CLK,
-       L_7X30_MDP_LCDC_PCLK_CLK,
-       L_7X30_MDP_LCDC_PAD_PCLK_CLK,
-       L_7X30_MDP_VSYNC_CLK,
-       L_7X30_MI2S_CODEC_RX_M_CLK,
-       L_7X30_MI2S_CODEC_RX_S_CLK,
-       L_7X30_MI2S_CODEC_TX_M_CLK,
-       L_7X30_MI2S_CODEC_TX_S_CLK,
-       L_7X30_MI2S_M_CLK,
-       L_7X30_MI2S_S_CLK,
-       L_7X30_LPA_CODEC_CLK,
-       L_7X30_LPA_CORE_CLK,
-       L_7X30_LPA_P_CLK,
-       L_7X30_MIDI_CLK,
-       L_7X30_MDC_CLK,
-       L_7X30_ROTATOR_IMEM_CLK,
-       L_7X30_ROTATOR_P_CLK,
-       L_7X30_SDAC_M_CLK,
-       L_7X30_SDAC_CLK,
-       L_7X30_UART1_CLK,
-       L_7X30_UART2_CLK,
-       L_7X30_UART3_CLK,
-       L_7X30_TV_CLK,
-       L_7X30_TV_DAC_CLK,
-       L_7X30_TV_ENC_CLK,
-       L_7X30_HDMI_CLK,
-       L_7X30_TSIF_REF_CLK,
-       L_7X30_TSIF_P_CLK,
-       L_7X30_USB_HS_SRC_CLK,
-       L_7X30_USB_HS_CLK,
-       L_7X30_USB_HS_CORE_CLK,
-       L_7X30_USB_HS_P_CLK,
-       L_7X30_USB_HS2_CLK,
-       L_7X30_USB_HS2_CORE_CLK,
-       L_7X30_USB_HS2_P_CLK,
-       L_7X30_USB_HS3_CLK,
-       L_7X30_USB_HS3_CORE_CLK,
-       L_7X30_USB_HS3_P_CLK,
-       L_7X30_VFE_CLK,
-       L_7X30_VFE_P_CLK,
-       L_7X30_VFE_MDC_CLK,
-       L_7X30_VFE_CAMIF_CLK,
-       L_7X30_CAMIF_PAD_P_CLK,
-       L_7X30_CAM_M_CLK,
-       L_7X30_JPEG_CLK,
-       L_7X30_JPEG_P_CLK,
-       L_7X30_VPE_CLK,
-       L_7X30_MFC_CLK,
-       L_7X30_MFC_DIV2_CLK,
-       L_7X30_MFC_P_CLK,
-       L_7X30_SPI_CLK,
-       L_7X30_SPI_P_CLK,
-       L_7X30_CSI0_CLK,
-       L_7X30_CSI0_VFE_CLK,
-       L_7X30_CSI0_P_CLK,
-       L_7X30_CSI1_CLK,
-       L_7X30_CSI1_VFE_CLK,
-       L_7X30_CSI1_P_CLK,
-       L_7X30_GLBL_ROOT_CLK,
-
-       L_7X30_AXI_LI_VG_CLK,
-       L_7X30_AXI_LI_GRP_CLK,
-       L_7X30_AXI_LI_JPEG_CLK,
-       L_7X30_AXI_GRP_2D_CLK,
-       L_7X30_AXI_MFC_CLK,
-       L_7X30_AXI_VPE_CLK,
-       L_7X30_AXI_LI_VFE_CLK,
-       L_7X30_AXI_LI_APPS_CLK,
-       L_7X30_AXI_MDP_CLK,
-       L_7X30_AXI_IMEM_CLK,
-       L_7X30_AXI_LI_ADSP_A_CLK,
-       L_7X30_AXI_ROTATOR_CLK,
-
-       L_7X30_NR_CLKS
-};
-
-struct clk_ops;
-extern struct clk_ops clk_ops_7x30;
-
-struct clk_ops *clk_7x30_is_local(uint32_t id);
-int clk_7x30_init(void);
-
-void pll_enable(uint32_t pll);
-void pll_disable(uint32_t pll);
-
-extern int internal_pwr_rail_ctl_auto(unsigned rail_id, bool enable);
-
-#define CLK_7X30(clk_name, clk_id, clk_dev, clk_flags) {       \
-       .con_id = clk_name, \
-       .dev_id = clk_dev, \
-       .clk = &(struct clk){ \
-               .id = L_7X30_##clk_id, \
-               .remote_id = P_##clk_id, \
-               .flags = clk_flags, \
-               .dbg_name = #clk_id, \
-       }, \
-       }
-
-#define CLK_7X30S(clk_name, l_id, r_id, clk_dev, clk_flags) {  \
-       .con_id = clk_name, \
-       .dev_id = clk_dev, \
-       .clk = &(struct clk){ \
-               .id = L_7X30_##l_id, \
-               .remote_id = P_##r_id, \
-               .flags = clk_flags, \
-               .dbg_name = #l_id, \
-               .ops = &clk_ops_pcom, \
-       }, \
-       }
-
-#endif

arch/arm/mach-msm/devices-msm7x30.c +0 -0

index 09b4f14..1597d43 100644

arch/arm/mach-msm/devices-msm7x30.c struct clk_lookup msm_clocks_7x30[] = { · line 150

Quoted 11 lines · Stephen Boyd

arch/arm/mach-msm/devices-msm7x30.c +0 -1

index 09b4f14..1597d43 100644
@@ -146,7 +146,6 @@ struct clk_lookup msm_clocks_7x30[] = {
        CLK_PCOM("grp_2d_pclk", GRP_2D_P_CLK,   NULL, 0),
        CLK_PCOM("grp_clk",     GRP_3D_CLK,     NULL, 0),
        CLK_PCOM("grp_pclk",    GRP_3D_P_CLK,   NULL, 0),
-       CLK_7X30S("grp_src_clk", GRP_3D_SRC_CLK, GRP_3D_CLK,    NULL, 0),
        CLK_PCOM("hdmi_clk",    HDMI_CLK,       NULL, 0),
        CLK_PCOM("imem_clk",    IMEM_CLK,       NULL, OFF),

I suppose you forget to remove the include header clock-7x30.h from this file. :)

Quoted 14 lines · Pankaj Jangra

arch/arm/mach-msm/devices-msm7x30.c struct clk_lookup msm_clocks_7x30[] = { · line 150

Quoted 11 lines · Stephen Boyd

arch/arm/mach-msm/devices-msm7x30.c +0 -1

index 09b4f14..1597d43 100644
@@ -146,7 +146,6 @@ struct clk_lookup msm_clocks_7x30[] = {
        CLK_PCOM("grp_2d_pclk", GRP_2D_P_CLK,   NULL, 0),
        CLK_PCOM("grp_clk",     GRP_3D_CLK,     NULL, 0),
        CLK_PCOM("grp_pclk",    GRP_3D_P_CLK,   NULL, 0),
-       CLK_7X30S("grp_src_clk", GRP_3D_SRC_CLK, GRP_3D_CLK,    NULL, 0),
        CLK_PCOM("hdmi_clk",    HDMI_CLK,       NULL, 0),
        CLK_PCOM("imem_clk",    IMEM_CLK,       NULL, OFF),

I suppose you forget to remove the include header clock-7x30.h from this file. :)

Thanks, fixed. BTW, you could have trimmed this mail down so I didn't have to scroll to the bottom.

In the near future we'll be moving clock-pcom to a platform driver, in which case these two users of clk_get() in mach-msm need to be updated. Have board-trout-panel.c make the proc_comm call directly so that we don't have to port this board specific code to the driver right now and reorder the initcall order of dma.c so that it initializes after the clock driver probes but before any drivers use dma APIs.

Signed-off-by Stephen Boyd <[email protected]>

---
 arch/arm/mach-msm/Makefile            |  9 ++++++---
 arch/arm/mach-msm/board-trout-panel.c | 19 +++++++------------
 arch/arm/mach-msm/dma.c               |  5 ++---
 3 files changed, 15 insertions(+), 18 deletions(-)

arch/arm/mach-msm/Makefile +0 -2

index 17519fa..3dbae74 100644
@@ -5,12 +5,15 @@ obj-$(CONFIG_DEBUG_FS) += clock-debug.o
 obj-$(CONFIG_MSM_VIC) += irq-vic.o
 obj-$(CONFIG_MSM_IOMMU) += devices-iommu.o
 
-obj-$(CONFIG_ARCH_MSM7X00A) += dma.o irq.o
-obj-$(CONFIG_ARCH_MSM7X30) += dma.o

Hi,

Quoted 29 lines · Stephen Boyd

In the near future we'll be moving clock-pcom to a platform driver, in which case these two users of clk_get() in mach-msm need to be updated. Have board-trout-panel.c make the proc_comm call directly so that we don't have to port this board specific code to the driver right now and reorder the initcall order of dma.c so that it initializes after the clock driver probes but before any drivers use dma APIs.

Signed-off-by Stephen Boyd <[email protected]>

---
 arch/arm/mach-msm/Makefile            |  9 ++++++---
 arch/arm/mach-msm/board-trout-panel.c | 19 +++++++------------
 arch/arm/mach-msm/dma.c               |  5 ++---
 3 files changed, 15 insertions(+), 18 deletions(-)

arch/arm/mach-msm/Makefile +2 -3

index 17519fa..3dbae74 100644
@@ -5,12 +5,15 @@ obj-$(CONFIG_DEBUG_FS) += clock-debug.o
 obj-$(CONFIG_MSM_VIC) += irq-vic.o
 obj-$(CONFIG_MSM_IOMMU) += devices-iommu.o
 
-obj-$(CONFIG_ARCH_MSM7X00A) += dma.o irq.o
-obj-$(CONFIG_ARCH_MSM7X30) += dma.o
-obj-$(CONFIG_ARCH_QSD8X50) += dma.o sirc.o
+obj-$(CONFIG_ARCH_MSM7X00A) += irq.o
+obj-$(CONFIG_ARCH_QSD8X50) += sirc.o
 

arch/arm/mach-msm/Makefile obj-$(CONFIG_DEBUG_FS) += clock-debug.o · line 16

Quoted 6 lines

arch/arm/mach-msm/Makefile +4 -0

 obj-$(CONFIG_MSM_PROC_COMM) += proc_comm.o clock-pcom.o vreg.o
 
+obj-$(CONFIG_ARCH_MSM7X00A) += dma.o
+obj-$(CONFIG_ARCH_MSM7X30) += dma.o
+obj-$(CONFIG_ARCH_QSD8X50) += dma.o
+

I did not understand what is the need of this change in Makefile?

To move closer to the generic struct clock framework move the proc_comm based clock code to a platform driver. The data describing the struct clks still live in the devices-$ARCH file, but the clock initialization is done at driver binding time.

Cc Saravana Kannan <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

---

 arch/arm/mach-msm/board-halibut.c      |  2 +-
 arch/arm/mach-msm/board-msm7x30.c      |  2 +-
 arch/arm/mach-msm/board-qsd8x50.c      |  2 +-
 arch/arm/mach-msm/board-trout.c        |  3 +--
 arch/arm/mach-msm/clock-pcom.c         | 23 +++++++++++++++++++++--
 arch/arm/mach-msm/clock-pcom.h         |  5 +++++
 arch/arm/mach-msm/clock.c              |  2 +-
 arch/arm/mach-msm/clock.h              |  3 +++
 arch/arm/mach-msm/devices-msm7x00.c    | 12 ++++++++++--
 arch/arm/mach-msm/devices-msm7x30.c    | 11 +++++++++--
 arch/arm/mach-msm/devices-qsd8x50.c    | 11 +++++++++--
 arch/arm/mach-msm/devices.h            | 15 +++------------
 arch/arm/mach-msm/include/mach/board.h |  5 -----
 13 files changed, 65 insertions(+), 31 deletions(-)

arch/arm/mach-msm/board-halibut.c +0 -0

index 6ce542e..3c8cfe4 100644

arch/arm/mach-msm/clock-pcom.c struct clk_ops clk_ops_pcom = { · line 133

Quoted 5 lines

arch/arm/mach-msm/clock-pcom.c +4 -0

+
+static int __devinit msm_clock_pcom_probe(struct platform_device *pdev)
+{
+       struct pcom_clk_pdata *pdata = pdev->dev.platform_data;

Should'nt it be const struct?

arch/arm/mach-msm/clock-pcom.c +8 -6

-static int pc_clk_set_rate(unsigned id, unsigned rate)
+static int pc_clk_set_rate(struct clk_hw *hw, unsigned long new_rate,
+                          unsigned long p_rate)
 {
-       /* The rate _might_ be rounded off to the nearest KHz value by the
+       struct clk_pcom *p = to_clk_pcom(hw);
+       unsigned id = p->id, rate = new_rate;
+       int rc;
+
+       /*
+        * The rate _might_ be rounded off to the nearest KHz value by the
         * remote function. So a return value of 0 doesn't necessarily mean
         * that the exact rate was set successfully.
         */
-       int rc = msm_proc_comm(PCOM_CLKCTL_RPC_SET_RATE, &id, &rate);
-       if (rc < 0)
-               return rc;
-       else

+               rc = msm_proc_comm(PCOM_CLKCTL_RPC_SET_RATE, &id, &rate);
        if (rc < 0)
                return rc;
        else
                return (int)id < 0 ? -EINVAL : 0;

}

Hi Stephen,

Quoted 18 lines · Stephen Boyd

arch/arm/mach-msm/clock-pcom.c +8 -6

-static int pc_clk_set_rate(unsigned id, unsigned rate)
+static int pc_clk_set_rate(struct clk_hw *hw, unsigned long new_rate,
+                          unsigned long p_rate)
 {
-       /* The rate _might_ be rounded off to the nearest KHz value by the
+       struct clk_pcom *p = to_clk_pcom(hw);
+       unsigned id = p->id, rate = new_rate;
+       int rc;
+
+       /*
+        * The rate _might_ be rounded off to the nearest KHz value by the
         * remote function. So a return value of 0 doesn't necessarily mean
         * that the exact rate was set successfully.
         */
-       int rc = msm_proc_comm(PCOM_CLKCTL_RPC_SET_RATE, &id, &rate);
-       if (rc < 0)
-               return rc;
-       else

arch/arm/mach-msm/clock-pcom.c int pc_clk_reset(unsigned id, enum clk_reset_action action) · line 85

Quoted 10 lines

arch/arm/mach-msm/clock-pcom.c +2 -8

-               return (int)id < 0 ? -EINVAL : 0;
-}
-
-static int pc_clk_set_min_rate(unsigned id, unsigned rate)
-{
-       int rc = msm_proc_comm(PCOM_CLKCTL_RPC_MIN_RATE, &id, &rate);
-       if (rc < 0)
-               return rc;
+       if (p->flags & CLKFLAG_MIN)
+               rc = msm_proc_comm(PCOM_CLKCTL_RPC_MIN_RATE, &id, &rate);

You are missing if condition here checking the rc ?

arch/arm/mach-msm/clock-pcom.c int pc_clk_reset(unsigned id, enum clk_reset_action action) · line 82

Quoted 7 lines

else

arch/arm/mach-msm/clock-pcom.c +0 -6

-               return (int)id < 0 ? -EINVAL : 0;
-}
-
-static int pc_clk_set_max_rate(unsigned id, unsigned rate)
-{
-       int rc = msm_proc_comm(PCOM_CLKCTL_RPC_MAX_RATE, &id, &rate);

and else here i think for the MIN_FLAG if check.

Quoted 6 lines

+               rc = msm_proc_comm(PCOM_CLKCTL_RPC_SET_RATE, &id, &rate);
        if (rc < 0)
                return rc;
        else
                return (int)id < 0 ? -EINVAL : 0;

}

Quoted 34 lines · Pankaj Jangra

Hi Stephen,

Quoted 18 lines · Stephen Boyd

arch/arm/mach-msm/clock-pcom.c +8 -6

-static int pc_clk_set_rate(unsigned id, unsigned rate)
+static int pc_clk_set_rate(struct clk_hw *hw, unsigned long new_rate,
+                          unsigned long p_rate)
 {
-       /* The rate _might_ be rounded off to the nearest KHz value by the
+       struct clk_pcom *p = to_clk_pcom(hw);
+       unsigned id = p->id, rate = new_rate;
+       int rc;
+
+       /*
+        * The rate _might_ be rounded off to the nearest KHz value by the
         * remote function. So a return value of 0 doesn't necessarily mean
         * that the exact rate was set successfully.
         */
-       int rc = msm_proc_comm(PCOM_CLKCTL_RPC_SET_RATE, &id, &rate);
-       if (rc < 0)
-               return rc;
-       else

arch/arm/mach-msm/clock-pcom.c int pc_clk_reset(unsigned id, enum clk_reset_action action) · line 85

Quoted 10 lines

arch/arm/mach-msm/clock-pcom.c +2 -8

-               return (int)id < 0 ? -EINVAL : 0;
-}
-
-static int pc_clk_set_min_rate(unsigned id, unsigned rate)
-{
-       int rc = msm_proc_comm(PCOM_CLKCTL_RPC_MIN_RATE, &id, &rate);
-       if (rc < 0)
-               return rc;
+       if (p->flags & CLKFLAG_MIN)
+               rc = msm_proc_comm(PCOM_CLKCTL_RPC_MIN_RATE, &id, &rate);

You are missing if condition here checking the rc ?

Quoted 15 lines

arch/arm/mach-msm/clock-pcom.c int pc_clk_reset(unsigned id, enum clk_reset_action action) · line 82

Quoted 7 lines

else

arch/arm/mach-msm/clock-pcom.c +0 -6

-               return (int)id < 0 ? -EINVAL : 0;
-}
-
-static int pc_clk_set_max_rate(unsigned id, unsigned rate)
-{
-       int rc = msm_proc_comm(PCOM_CLKCTL_RPC_MAX_RATE, &id, &rate);

and else here i think for the MIN_FLAG if check.

Quoted 6 lines

+               rc = msm_proc_comm(PCOM_CLKCTL_RPC_SET_RATE, &id, &rate);
        if (rc < 0)
                return rc;
        else
                return (int)id < 0 ? -EINVAL : 0;

}

This is the resulting code:

        if (p->flags & CLKFLAG_MIN)
                rc = msm_proc_comm(PCOM_CLKCTL_RPC_MIN_RATE, &id, &rate);
        else
                rc = msm_proc_comm(PCOM_CLKCTL_RPC_SET_RATE, &id, &rate);
        if (rc < 0)
                return rc;
        else
                return (int)id < 0 ? -EINVAL : 0;

So we check the rc for both cases in the same if condition. Is there anything wrong?

This patchset moves the existing MSM clock code and affected drivers to the common clock framework. A prerequisite of moving to the common clock framework is to use clk_prepare() and clk_enable() so the first few patches migrate drivers to that call (clk_prepare() is a no-op on MSM right now). It also removes some custom clock APIs that MSM provides and finally moves the proc_comm clock code to the common struct clock.

This patch series will be used as the foundation of the MSM 8660/8960 clock code that I plan to send out soon.

These patches have one dependency on the devm_clk_register() function which I already posted to the list[1]. They're based on linux-next-20120917.

Stephen Boyd (10):
usb: otg: msm: Convert to clk_prepare/unprepare
msm_sdcc: Convert to clk_prepare/unprepare
msm: iommu: Convert to clk_prepare/unprepare
msm: iommu: Use clk_set_rate() instead of clk_set_min_rate()
ARM: msm: Remove custom clk_set_flags() API
ARM: msm: Remove custom clk_set_{max,min}_rate() API
ARM: msm: Remove clock-7x30.h include file
ARM: msm: Prepare clk_get() users in mach-msm for clock-pcom driver
ARM: msm: Make proc_comm clock control into a platform driver
ARM: msm: Migrate to common clock framework

 arch/arm/Kconfig                       |   1 +
 arch/arm/mach-msm/Makefile             |  10 +-
 arch/arm/mach-msm/board-halibut.c      |   2 +-
 arch/arm/mach-msm/board-msm7x30.c      |   2 +-
 arch/arm/mach-msm/board-qsd8x50.c      |   2 +-
 arch/arm/mach-msm/board-trout-panel.c  |  19 ++--
 arch/arm/mach-msm/board-trout.c        |   3 +-
 arch/arm/mach-msm/clock-7x30.h         | 155 ------------------------------
 arch/arm/mach-msm/clock-debug.c        | 130 --------------------------
 arch/arm/mach-msm/clock-pcom.c         | 149 ++++++++++++++++++-----------
 arch/arm/mach-msm/clock-pcom.h         |  31 +++---
 arch/arm/mach-msm/clock.c              | 166 +--------------------------------
 arch/arm/mach-msm/clock.h              |  51 +++-------
 arch/arm/mach-msm/devices-msm7x00.c    |  12 ++-
 arch/arm/mach-msm/devices-msm7x30.c    |  13 ++-
 arch/arm/mach-msm/devices-qsd8x50.c    |  11 ++-
 arch/arm/mach-msm/devices.h            |  15 +--
 arch/arm/mach-msm/dma.c                |   5 +-
 arch/arm/mach-msm/include/mach/board.h |   5 -
 arch/arm/mach-msm/include/mach/clk.h   |   9 --
 drivers/iommu/msm_iommu_dev.c          |  20 ++--
 drivers/mmc/host/msm_sdcc.c            |  14 ++-
 drivers/usb/otg/msm_otg.c              |  38 ++++----
 23 files changed, 222 insertions(+), 641 deletions(-)
 delete mode 100644 arch/arm/mach-msm/clock-7x30.h
 delete mode 100644 arch/arm/mach-msm/clock-debug.c

[1] https://lkml.org/lkml/2012/9/19/28

Signature · 8 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel