Re: [PATCH] [media] staging: atomisp: use clock framework for camera clocks

Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> · Wed, 20 Sep 2017 12:01:14 -0500

On 09/20/2017 04:12 AM, Andy Shevchenko wrote:
On Tue, 2017-09-19 at 15:45 -0500, Pierre-Louis Bossart wrote:
The Atom ISP driver initializes and configures PMC clocks which are
already handled by the clock framework.

Remove all legacy vlv2_platform_clock stuff and move to the clk API to
avoid conflicts, e.g. with audio machine drivers enabling the MCLK for
external codecs

I think it might have a Fixes: tag as well (though I dunno which commit
could be considered as anchor).
The initial integration of the atomisp driver already had this problem, 
i'll add a reference to
'a49d25364dfb9 ("staging/atomisp: Add support for the Intel IPU v2")'

(I doubt Git is so clever to remove files based on information out of
the diff, can you check it and if needed to resend without -D implied?)
Gee, I thought -C -M -D were the standard options to checkpatch, never 
realized it'd prevent patches from applying. Thanks for the tip.


Other than that - nice clean up!

Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
I'll add your Reviewed-by in the v2. Thanks for the review.


Tested-by: Carlo Caione <carlo@xxxxxxxxxxxx>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.
com>
---
  drivers/staging/media/atomisp/Kconfig              |   1 +
  drivers/staging/media/atomisp/platform/Makefile    |   1 -
  .../staging/media/atomisp/platform/clock/Makefile  |   6 -
  .../platform/clock/platform_vlv2_plat_clk.c        |  40 ----
  .../platform/clock/platform_vlv2_plat_clk.h        |  27 ---
  .../media/atomisp/platform/clock/vlv2_plat_clock.c | 247 ------------
---------
  .../platform/intel-mid/atomisp_gmin_platform.c     |  63 +++++-
  7 files changed, 52 insertions(+), 333 deletions(-)
  delete mode 100644
drivers/staging/media/atomisp/platform/clock/Makefile
  delete mode 100644
drivers/staging/media/atomisp/platform/clock/platform_vlv2_plat_clk.c
  delete mode 100644
drivers/staging/media/atomisp/platform/clock/platform_vlv2_plat_clk.h
  delete mode 100644
drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c

diff --git a/drivers/staging/media/atomisp/Kconfig
b/drivers/staging/media/atomisp/Kconfig
index 8eb13c3ba29c..7cdebea35ccf 100644
--- a/drivers/staging/media/atomisp/Kconfig
+++ b/drivers/staging/media/atomisp/Kconfig
@@ -1,6 +1,7 @@
  menuconfig INTEL_ATOMISP
          bool "Enable support to Intel MIPI camera drivers"
          depends on X86 && EFI && MEDIA_CONTROLLER && PCI && ACPI
+	select COMMON_CLK
          help
            Enable support for the Intel ISP2 camera interfaces and
MIPI
            sensor drivers.
diff --git a/drivers/staging/media/atomisp/platform/Makefile
b/drivers/staging/media/atomisp/platform/Makefile
index df157630bda9..0e3b7e1c81c6 100644
--- a/drivers/staging/media/atomisp/platform/Makefile
+++ b/drivers/staging/media/atomisp/platform/Makefile
@@ -2,5 +2,4 @@
  # Makefile for camera drivers.
  #
  
-obj-$(CONFIG_INTEL_ATOMISP) += clock/
  obj-$(CONFIG_INTEL_ATOMISP) += intel-mid/
diff --git a/drivers/staging/media/atomisp/platform/clock/Makefile
b/drivers/staging/media/atomisp/platform/clock/Makefile
deleted file mode 100644
index 82fbe8b6968a..000000000000
diff --git
a/drivers/staging/media/atomisp/platform/clock/platform_vlv2_plat_clk.
c
b/drivers/staging/media/atomisp/platform/clock/platform_vlv2_plat_clk.
c
deleted file mode 100644
index 0aae9b0283bb..000000000000
diff --git
a/drivers/staging/media/atomisp/platform/clock/platform_vlv2_plat_clk.
h
b/drivers/staging/media/atomisp/platform/clock/platform_vlv2_plat_clk.
h
deleted file mode 100644
index b730ab0e8223..000000000000
diff --git
a/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c
b/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c
deleted file mode 100644
index f96789a31819..000000000000
diff --git a/drivers/staging/media/atomisp/platform/intel-
mid/atomisp_gmin_platform.c
b/drivers/staging/media/atomisp/platform/intel-
mid/atomisp_gmin_platform.c
index edaae93af8f9..17b4cfae5abf 100644
--- a/drivers/staging/media/atomisp/platform/intel-
mid/atomisp_gmin_platform.c
+++ b/drivers/staging/media/atomisp/platform/intel-
mid/atomisp_gmin_platform.c
@@ -4,10 +4,10 @@
  #include <linux/efi.h>
  #include <linux/pci.h>
  #include <linux/acpi.h>
+#include <linux/clk.h>
  #include <linux/delay.h>
  #include <media/v4l2-subdev.h>
  #include <linux/mfd/intel_soc_pmic.h>
-#include "../../include/linux/vlv2_plat_clock.h"
  #include <linux/regulator/consumer.h>
  #include <linux/gpio/consumer.h>
  #include <linux/gpio.h>
@@ -17,11 +17,7 @@
  
  #define MAX_SUBDEVS 8
  
-/* Should be defined in vlv2_plat_clock API, isn't: */
-#define VLV2_CLK_PLL_19P2MHZ 1
-#define VLV2_CLK_XTAL_19P2MHZ 0
-#define VLV2_CLK_ON      1
-#define VLV2_CLK_OFF     2
+#define VLV2_CLK_PLL_19P2MHZ 1 /* XTAL on CHT */
  #define ELDO1_SEL_REG	0x19
  #define ELDO1_1P8V	0x16
  #define ELDO1_CTRL_SHIFT 0x00
@@ -33,6 +29,7 @@ struct gmin_subdev {
  	struct v4l2_subdev *subdev;
  	int clock_num;
  	int clock_src;
+	struct clk *pmc_clk;
  	struct gpio_desc *gpio0;
  	struct gpio_desc *gpio1;
  	struct regulator *v1p8_reg;
@@ -344,6 +341,9 @@ static int gmin_platform_deinit(void)
  	return 0;
  }
  
+#define GMIN_PMC_CLK_NAME 14 /* "pmc_plt_clk_[0..5]" */
+static char gmin_pmc_clk_name[GMIN_PMC_CLK_NAME];
+
  static struct gmin_subdev *gmin_subdev_add(struct v4l2_subdev
*subdev)
  {
  	int i, ret;
@@ -377,6 +377,37 @@ static struct gmin_subdev *gmin_subdev_add(struct
v4l2_subdev *subdev)
  	gmin_subdevs[i].gpio0 = gpiod_get_index(dev, NULL, 0,
GPIOD_OUT_LOW);
  	gmin_subdevs[i].gpio1 = gpiod_get_index(dev, NULL, 1,
GPIOD_OUT_LOW);
  
+	/* get PMC clock with clock framework */
+	snprintf(gmin_pmc_clk_name,
+		 sizeof(gmin_pmc_clk_name),
+		 "%s_%d", "pmc_plt_clk", gmin_subdevs[i].clock_num);
+
+	gmin_subdevs[i].pmc_clk = devm_clk_get(dev,
gmin_pmc_clk_name);
+	if (IS_ERR(gmin_subdevs[i].pmc_clk)) {
+		ret = PTR_ERR(gmin_subdevs[i].pmc_clk);
+
+		dev_err(dev,
+			"Failed to get clk from %s : %d\n",
+			gmin_pmc_clk_name,
+			ret);
+
+		return NULL;
+	}
+
+	/*
+	 * The firmware might enable the clock at
+	 * boot (this information may or may not
+	 * be reflected in the enable clock register).
+	 * To change the rate we must disable the clock
+	 * first to cover these cases. Due to common
+	 * clock framework restrictions that do not allow
+	 * to disable a clock that has not been enabled,
+	 * we need to enable the clock first.
+	 */
+	ret = clk_prepare_enable(gmin_subdevs[i].pmc_clk);
+	if (!ret)
+		clk_disable_unprepare(gmin_subdevs[i].pmc_clk);
+
  	if (!IS_ERR(gmin_subdevs[i].gpio0)) {
  		ret = gpiod_direction_output(gmin_subdevs[i].gpio0,
0);
  		if (ret)
@@ -539,13 +570,21 @@ static int gmin_flisclk_ctrl(struct v4l2_subdev
*subdev, int on)
  {
  	int ret = 0;
  	struct gmin_subdev *gs = find_gmin_subdev(subdev);
+	struct i2c_client *client = v4l2_get_subdevdata(subdev);
+
+	if (on) {
+		ret = clk_set_rate(gs->pmc_clk, gs->clock_src);
+
+		if (ret)
+			dev_err(&client->dev, "unable to set PMC rate
%d\n",
+				gs->clock_src);
  
-	if (on)
-		ret = vlv2_plat_set_clock_freq(gs->clock_num, gs-
clock_src);
-	if (ret)
-		return ret;
-	return vlv2_plat_configure_clock(gs->clock_num,
-					 on ? VLV2_CLK_ON :
VLV2_CLK_OFF);
+		ret = clk_prepare_enable(gs->pmc_clk);
+	} else {
+		clk_disable_unprepare(gs->pmc_clk);
+	}
+
+	return ret;
  }
  
  static int gmin_csi_cfg(struct v4l2_subdev *sd, int flag)

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel






