lkml.org 
[lkml]   [2017]   [Sep]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] [media] staging: atomisp: use clock framework for camera clocks
From
Date


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@linux.intel.com>
I'll add your Reviewed-by in the v2. Thanks for the review.
>
>
>> Tested-by: Carlo Caione <carlo@endlessm.com>
>> 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)

\
 
 \ /
  Last update: 2017-09-20 19:02    [W:0.084 / U:29.304 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site