* [PATCH v3] clk: x86: Do not gate clocks enabled by the firmware @ 2017-07-14 8:23 Carlo Caione 2017-07-18 23:24 ` Stephen Boyd 2017-09-07 9:22 ` Carlo Caione 0 siblings, 2 replies; 10+ messages in thread From: Carlo Caione @ 2017-07-14 8:23 UTC (permalink / raw) To: mturquette, linux-clk, dvhart, pierre-louis.bossart, sboyd, linux, eballetbo, andriy.shevchenko Cc: Carlo Caione From: Carlo Caione <carlo@endlessm.com> Read the enable register to determine if the clock is already in use by the firmware. In this case avoid gating the clock. Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Acked-by: Darren Hart (VMware) <dvhart@infradead.org> Signed-off-by: Carlo Caione <carlo@endlessm.com> --- drivers/clk/x86/clk-pmc-atom.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/clk/x86/clk-pmc-atom.c b/drivers/clk/x86/clk-pmc-atom.c index 2b60577703ef..be8d821ce625 100644 --- a/drivers/clk/x86/clk-pmc-atom.c +++ b/drivers/clk/x86/clk-pmc-atom.c @@ -185,6 +185,13 @@ static struct clk_plt *plt_clk_register(struct platform_device *pdev, int id, pclk->reg = base + PMC_CLK_CTL_OFFSET + id * PMC_CLK_CTL_SIZE; spin_lock_init(&pclk->lock); + /* + * If the clock was already enabled by the firmware mark it as critical + * to avoid it being gated by the clock framework if no driver owns it. + */ + if (plt_clk_is_enabled(&pclk->hw)) + init.flags |= CLK_IS_CRITICAL; + ret = devm_clk_hw_register(&pdev->dev, &pclk->hw); if (ret) { pclk = ERR_PTR(ret); -- 2.13.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3] clk: x86: Do not gate clocks enabled by the firmware 2017-07-14 8:23 [PATCH v3] clk: x86: Do not gate clocks enabled by the firmware Carlo Caione @ 2017-07-18 23:24 ` Stephen Boyd 2017-09-07 9:22 ` Carlo Caione 1 sibling, 0 replies; 10+ messages in thread From: Stephen Boyd @ 2017-07-18 23:24 UTC (permalink / raw) To: Carlo Caione Cc: mturquette, linux-clk, dvhart, pierre-louis.bossart, linux, eballetbo, andriy.shevchenko, Carlo Caione On 07/14, Carlo Caione wrote: > From: Carlo Caione <carlo@endlessm.com> > > Read the enable register to determine if the clock is already in use by > the firmware. In this case avoid gating the clock. > > Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> > Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Acked-by: Darren Hart (VMware) <dvhart@infradead.org> > Signed-off-by: Carlo Caione <carlo@endlessm.com> > --- Applied to clk-fixes and added a Fixes tag. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] clk: x86: Do not gate clocks enabled by the firmware 2017-07-14 8:23 [PATCH v3] clk: x86: Do not gate clocks enabled by the firmware Carlo Caione 2017-07-18 23:24 ` Stephen Boyd @ 2017-09-07 9:22 ` Carlo Caione 2017-09-07 11:59 ` Pierre-Louis Bossart 1 sibling, 1 reply; 10+ messages in thread From: Carlo Caione @ 2017-09-07 9:22 UTC (permalink / raw) To: alan Cc: Michael Turquette, open list:COMMON CLK FRAMEWORK, Darren Hart, Pierre-Louis Bossart, Stephen Boyd, Linux Upstreaming Team, Enric Balletbo Serra, andriy.shevchenko, Carlo Caione, Carlo Caione On Fri, Jul 14, 2017 at 10:23 AM, Carlo Caione <carlo@caione.org> wrote: > From: Carlo Caione <carlo@endlessm.com> > > Read the enable register to determine if the clock is already in use by > the firmware. In this case avoid gating the clock. > > Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> > Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Acked-by: Darren Hart (VMware) <dvhart@infradead.org> > Signed-off-by: Carlo Caione <carlo@endlessm.com> > --- > drivers/clk/x86/clk-pmc-atom.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/clk/x86/clk-pmc-atom.c b/drivers/clk/x86/clk-pmc-atom.c > index 2b60577703ef..be8d821ce625 100644 > --- a/drivers/clk/x86/clk-pmc-atom.c > +++ b/drivers/clk/x86/clk-pmc-atom.c > @@ -185,6 +185,13 @@ static struct clk_plt *plt_clk_register(struct platform_device *pdev, int id, > pclk->reg = base + PMC_CLK_CTL_OFFSET + id * PMC_CLK_CTL_SIZE; > spin_lock_init(&pclk->lock); > > + /* > + * If the clock was already enabled by the firmware mark it as critical > + * to avoid it being gated by the clock framework if no driver owns it. > + */ > + if (plt_clk_is_enabled(&pclk->hw)) > + init.flags |= CLK_IS_CRITICAL; > + > ret = devm_clk_hw_register(&pdev->dev, &pclk->hw); > if (ret) { > pclk = ERR_PTR(ret); Hi, I just discovered that this fix is not working anymore on my platform (with a baytrail processor). This fix was needed on my setup because the clock pmc_plt_clk_4 used by the Ethernet and enabled by the firmware, was being gated by the clock framework since the r8169 driver was failing to claim it. You can read the whole discussion about this in [0]. This fix was also needed for some other audio machine drivers. I bisected the problem down to commit 49d25364df ("staging/atomisp: Add support for the Intel IPU v2"). The clock driver is basically shutting down all the clocks (see [1]) at probe time, so the clk-pmc-atom driver is seeing all the clocks already disabled when going to check. Any idea how to proper fix this? Cheers, [0] https://www.spinics.net/lists/platform-driver-x86/msg12092.html [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c?h=v4.13#n200 -- Carlo Caione ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] clk: x86: Do not gate clocks enabled by the firmware 2017-09-07 9:22 ` Carlo Caione @ 2017-09-07 11:59 ` Pierre-Louis Bossart 2017-09-07 21:51 ` Pierre-Louis Bossart 0 siblings, 1 reply; 10+ messages in thread From: Pierre-Louis Bossart @ 2017-09-07 11:59 UTC (permalink / raw) To: Carlo Caione, alan Cc: Michael Turquette, open list:COMMON CLK FRAMEWORK, Darren Hart, Stephen Boyd, Linux Upstreaming Team, Enric Balletbo Serra, andriy.shevchenko, Carlo Caione On 9/7/17 4:22 AM, Carlo Caione wrote: > On Fri, Jul 14, 2017 at 10:23 AM, Carlo Caione <carlo@caione.org> wrote: >> From: Carlo Caione <carlo@endlessm.com> >> >> Read the enable register to determine if the clock is already in use by >> the firmware. In this case avoid gating the clock. >> >> Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> >> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >> Acked-by: Darren Hart (VMware) <dvhart@infradead.org> >> Signed-off-by: Carlo Caione <carlo@endlessm.com> >> --- >> drivers/clk/x86/clk-pmc-atom.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/drivers/clk/x86/clk-pmc-atom.c b/drivers/clk/x86/clk-pmc-atom.c >> index 2b60577703ef..be8d821ce625 100644 >> --- a/drivers/clk/x86/clk-pmc-atom.c >> +++ b/drivers/clk/x86/clk-pmc-atom.c >> @@ -185,6 +185,13 @@ static struct clk_plt *plt_clk_register(struct platform_device *pdev, int id, >> pclk->reg = base + PMC_CLK_CTL_OFFSET + id * PMC_CLK_CTL_SIZE; >> spin_lock_init(&pclk->lock); >> >> + /* >> + * If the clock was already enabled by the firmware mark it as critical >> + * to avoid it being gated by the clock framework if no driver owns it. >> + */ >> + if (plt_clk_is_enabled(&pclk->hw)) >> + init.flags |= CLK_IS_CRITICAL; >> + >> ret = devm_clk_hw_register(&pdev->dev, &pclk->hw); >> if (ret) { >> pclk = ERR_PTR(ret); > > Hi, > > I just discovered that this fix is not working anymore on my platform > (with a baytrail processor). > This fix was needed on my setup because the clock pmc_plt_clk_4 used > by the Ethernet and enabled by the firmware, was being gated by the > clock framework since the r8169 driver was failing to claim it. You > can read the whole discussion about this in [0]. This fix was also > needed for some other audio machine drivers. > > I bisected the problem down to commit 49d25364df ("staging/atomisp: > Add support for the Intel IPU v2"). The clock driver is basically > shutting down all the clocks (see [1]) at probe time, so the > clk-pmc-atom driver is seeing all the clocks already disabled when > going to check. > > Any idea how to proper fix this? Looks like the same code that was initially used as a reference for support of PMC clocks in the clock framework, so now we have 2 drivers programming the same PMC hardware, not so good. We'd probably have to move the atomisp driver to move to clk_get/prepare/enable instead of the old vlv2_clck_get? > > Cheers, > > [0] https://www.spinics.net/lists/platform-driver-x86/msg12092.html > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c?h=v4.13#n200 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] clk: x86: Do not gate clocks enabled by the firmware 2017-09-07 11:59 ` Pierre-Louis Bossart @ 2017-09-07 21:51 ` Pierre-Louis Bossart 2017-09-08 8:05 ` Carlo Caione 2017-09-18 8:05 ` Andy Shevchenko 0 siblings, 2 replies; 10+ messages in thread From: Pierre-Louis Bossart @ 2017-09-07 21:51 UTC (permalink / raw) To: Carlo Caione, alan Cc: Michael Turquette, open list:COMMON CLK FRAMEWORK, Darren Hart, Stephen Boyd, Linux Upstreaming Team, Enric Balletbo Serra, andriy.shevchenko, Carlo Caione [-- Attachment #1: Type: text/plain, Size: 3052 bytes --] On 09/07/2017 06:59 AM, Pierre-Louis Bossart wrote: > On 9/7/17 4:22 AM, Carlo Caione wrote: >> On Fri, Jul 14, 2017 at 10:23 AM, Carlo Caione <carlo@caione.org> wrote: >>> From: Carlo Caione <carlo@endlessm.com> >>> >>> Read the enable register to determine if the clock is already in use by >>> the firmware. In this case avoid gating the clock. >>> >>> Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> >>> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >>> Acked-by: Darren Hart (VMware) <dvhart@infradead.org> >>> Signed-off-by: Carlo Caione <carlo@endlessm.com> >>> --- >>> drivers/clk/x86/clk-pmc-atom.c | 7 +++++++ >>> 1 file changed, 7 insertions(+) >>> >>> diff --git a/drivers/clk/x86/clk-pmc-atom.c >>> b/drivers/clk/x86/clk-pmc-atom.c >>> index 2b60577703ef..be8d821ce625 100644 >>> --- a/drivers/clk/x86/clk-pmc-atom.c >>> +++ b/drivers/clk/x86/clk-pmc-atom.c >>> @@ -185,6 +185,13 @@ static struct clk_plt *plt_clk_register(struct >>> platform_device *pdev, int id, >>> pclk->reg = base + PMC_CLK_CTL_OFFSET + id * PMC_CLK_CTL_SIZE; >>> spin_lock_init(&pclk->lock); >>> >>> + /* >>> + * If the clock was already enabled by the firmware mark it >>> as critical >>> + * to avoid it being gated by the clock framework if no >>> driver owns it. >>> + */ >>> + if (plt_clk_is_enabled(&pclk->hw)) >>> + init.flags |= CLK_IS_CRITICAL; >>> + >>> ret = devm_clk_hw_register(&pdev->dev, &pclk->hw); >>> if (ret) { >>> pclk = ERR_PTR(ret); >> >> Hi, >> >> I just discovered that this fix is not working anymore on my platform >> (with a baytrail processor). >> This fix was needed on my setup because the clock pmc_plt_clk_4 used >> by the Ethernet and enabled by the firmware, was being gated by the >> clock framework since the r8169 driver was failing to claim it. You >> can read the whole discussion about this in [0]. This fix was also >> needed for some other audio machine drivers. >> >> I bisected the problem down to commit 49d25364df ("staging/atomisp: >> Add support for the Intel IPU v2"). The clock driver is basically >> shutting down all the clocks (see [1]) at probe time, so the >> clk-pmc-atom driver is seeing all the clocks already disabled when >> going to check. >> >> Any idea how to proper fix this? > > Looks like the same code that was initially used as a reference for > support of PMC clocks in the clock framework, so now we have 2 drivers > programming the same PMC hardware, not so good. We'd probably have to > move the atomisp driver to move to clk_get/prepare/enable instead of > the old vlv2_clck_get? See attached an untested change to show the idea on moving to the clk API and get feedback (I am not a camera guy, just trying to help - compile-tested/checkpatch only). This code *should* enable the same functionality using existing hooks in the kernel (and handle the BYT/CHT clocking difference as a bonus). I am sure Andy will have plenty of comments :-) [-- Attachment #2: 0001-media-staging-atomisp-use-clock-framework-for-camera.patch --] [-- Type: text/x-patch, Size: 6216 bytes --] >From 48bbe216d947eecc2fda97d6ca4b5d3225430e82 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Date: Thu, 7 Sep 2017 16:15:46 -0500 Subject: [PATCH] [media] staging: atomisp: use clock framework for camera clocks 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 [Compile-tested only - for comments-feedback] Reported-by: Carlo Caione <carlo@endlessm.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> --- 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 +++++- 6 files changed, 51 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/platform/Makefile b/drivers/staging/media/atomisp/platform/Makefile index df15763..0e3b7e1 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 82fbe8b..0000000 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 0aae9b0..0000000 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 b730ab0..0000000 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 f96789a..0000000 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 edaae93..17b4cfa 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) -- 2.9.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3] clk: x86: Do not gate clocks enabled by the firmware 2017-09-07 21:51 ` Pierre-Louis Bossart @ 2017-09-08 8:05 ` Carlo Caione 2017-09-18 8:05 ` Andy Shevchenko 1 sibling, 0 replies; 10+ messages in thread From: Carlo Caione @ 2017-09-08 8:05 UTC (permalink / raw) To: Pierre-Louis Bossart Cc: Carlo Caione, alan, Michael Turquette, open list:COMMON CLK FRAMEWORK, Darren Hart, Stephen Boyd, Linux Upstreaming Team, Enric Balletbo Serra, Andy Shevchenko On Thu, Sep 7, 2017 at 11:51 PM, Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> wrote: > > See attached an untested change to show the idea on moving to the clk API > and get feedback (I am not a camera guy, just trying to help - > compile-tested/checkpatch only). > This code *should* enable the same functionality using existing hooks in the > kernel (and handle the BYT/CHT clocking difference as a bonus). > I am sure Andy will have plenty of comments :-) FWIW tested-by: Carlo Caione <carlo@endlessm.com> Thanks, -- Carlo Caione | +39.340.80.30.096 | Endless ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] clk: x86: Do not gate clocks enabled by the firmware 2017-09-07 21:51 ` Pierre-Louis Bossart 2017-09-08 8:05 ` Carlo Caione @ 2017-09-18 8:05 ` Andy Shevchenko 2017-09-22 21:36 ` Darren Hart 1 sibling, 1 reply; 10+ messages in thread From: Andy Shevchenko @ 2017-09-18 8:05 UTC (permalink / raw) To: Pierre-Louis Bossart, Carlo Caione, alan Cc: Michael Turquette, open list:COMMON CLK FRAMEWORK, Darren Hart, Stephen Boyd, Linux Upstreaming Team, Enric Balletbo Serra, Carlo Caione On Thu, 2017-09-07 at 16:51 -0500, Pierre-Louis Bossart wrote: > > > > > > Looks like the same code that was initially used as a reference for > > support of PMC clocks in the clock framework, so now we have 2 > > drivers > > programming the same PMC hardware, not so good. We'd probably have > > to > > move the atomisp driver to move to clk_get/prepare/enable instead > > of > > the old vlv2_clck_get? > > See attached an untested change to show the idea on moving to the clk > API and get feedback (I am not a camera guy, just trying to help - > compile-tested/checkpatch only). > This code *should* enable the same functionality using existing hooks > in > the kernel (and handle the BYT/CHT clocking difference as a bonus). > I am sure Andy will have plenty of comments :-) I have one comment: nice clean up! AtomISP is a *horrible* mess. -- Andy Shevchenko <andriy.shevchenko@linux.intel.com> Intel Finland Oy ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] clk: x86: Do not gate clocks enabled by the firmware 2017-09-18 8:05 ` Andy Shevchenko @ 2017-09-22 21:36 ` Darren Hart 2017-09-22 21:47 ` Carlo Caione 2017-09-22 22:17 ` Pierre-Louis Bossart 0 siblings, 2 replies; 10+ messages in thread From: Darren Hart @ 2017-09-22 21:36 UTC (permalink / raw) To: Andy Shevchenko Cc: Pierre-Louis Bossart, Carlo Caione, alan, Michael Turquette, open list:COMMON CLK FRAMEWORK, Stephen Boyd, Linux Upstreaming Team, Enric Balletbo Serra, Carlo Caione On Mon, Sep 18, 2017 at 11:05:17AM +0300, Andy Shevchenko wrote: > On Thu, 2017-09-07 at 16:51 -0500, Pierre-Louis Bossart wrote: > > > > > > > > > > > Looks like the same code that was initially used as a reference for > > > support of PMC clocks in the clock framework, so now we have 2 > > > drivers > > > programming the same PMC hardware, not so good. We'd probably have > > > to > > > move the atomisp driver to move to clk_get/prepare/enable instead > > > of > > > the old vlv2_clck_get? > > > > See attached an untested change to show the idea on moving to the clk > > API and get feedback (I am not a camera guy, just trying to help - > > compile-tested/checkpatch only). > > This code *should* enable the same functionality using existing hooks > > in > > the kernel (and handle the BYT/CHT clocking difference as a bonus). > > I am sure Andy will have plenty of comments :-) > > I have one comment: nice clean up! > > AtomISP is a *horrible* mess. > Pierre, will you be submitting this patch officially now that you have a tested-by? -- Darren Hart VMware Open Source Technology Center ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] clk: x86: Do not gate clocks enabled by the firmware 2017-09-22 21:36 ` Darren Hart @ 2017-09-22 21:47 ` Carlo Caione 2017-09-22 22:17 ` Pierre-Louis Bossart 1 sibling, 0 replies; 10+ messages in thread From: Carlo Caione @ 2017-09-22 21:47 UTC (permalink / raw) To: Darren Hart Cc: Andy Shevchenko, Pierre-Louis Bossart, Carlo Caione, alan, Michael Turquette, open list:COMMON CLK FRAMEWORK, Stephen Boyd, Linux Upstreaming Team, Enric Balletbo Serra On Fri, Sep 22, 2017 at 10:36 PM, Darren Hart <dvhart@infradead.org> wrote: > On Mon, Sep 18, 2017 at 11:05:17AM +0300, Andy Shevchenko wrote: >> On Thu, 2017-09-07 at 16:51 -0500, Pierre-Louis Bossart wrote: >> > >> > >> >> > > >> > > Looks like the same code that was initially used as a reference for >> > > support of PMC clocks in the clock framework, so now we have 2 >> > > drivers >> > > programming the same PMC hardware, not so good. We'd probably have >> > > to >> > > move the atomisp driver to move to clk_get/prepare/enable instead >> > > of >> > > the old vlv2_clck_get? >> > >> > See attached an untested change to show the idea on moving to the clk >> > API and get feedback (I am not a camera guy, just trying to help - >> > compile-tested/checkpatch only). >> > This code *should* enable the same functionality using existing hooks >> > in >> > the kernel (and handle the BYT/CHT clocking difference as a bonus). >> > I am sure Andy will have plenty of comments :-) >> >> I have one comment: nice clean up! >> >> AtomISP is a *horrible* mess. >> > > Pierre, will you be submitting this patch officially now that you have a > tested-by? Just to clarify that my tested-by was to assert that the fix proposed by Pierre makes my system bootable again but I cannot say for sure if the atomisp driver keeps working fine after that. -- Carlo Caione | +39.340.80.30.096 | Endless ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] clk: x86: Do not gate clocks enabled by the firmware 2017-09-22 21:36 ` Darren Hart 2017-09-22 21:47 ` Carlo Caione @ 2017-09-22 22:17 ` Pierre-Louis Bossart 1 sibling, 0 replies; 10+ messages in thread From: Pierre-Louis Bossart @ 2017-09-22 22:17 UTC (permalink / raw) To: Darren Hart, Andy Shevchenko Cc: Carlo Caione, alan, Michael Turquette, open list:COMMON CLK FRAMEWORK, Stephen Boyd, Linux Upstreaming Team, Enric Balletbo Serra, Carlo Caione On 09/22/2017 04:36 PM, Darren Hart wrote: > On Mon, Sep 18, 2017 at 11:05:17AM +0300, Andy Shevchenko wrote: >> On Thu, 2017-09-07 at 16:51 -0500, Pierre-Louis Bossart wrote: >>> >>>> Looks like the same code that was initially used as a reference for >>>> support of PMC clocks in the clock framework, so now we have 2 >>>> drivers >>>> programming the same PMC hardware, not so good. We'd probably have >>>> to >>>> move the atomisp driver to move to clk_get/prepare/enable instead >>>> of >>>> the old vlv2_clck_get? >>> See attached an untested change to show the idea on moving to the clk >>> API and get feedback (I am not a camera guy, just trying to help - >>> compile-tested/checkpatch only). >>> This code *should* enable the same functionality using existing hooks >>> in >>> the kernel (and handle the BYT/CHT clocking difference as a bonus). >>> I am sure Andy will have plenty of comments :-) >> I have one comment: nice clean up! >> >> AtomISP is a *horrible* mess. >> > Pierre, will you be submitting this patch officially now that you have a > tested-by? it was merged by Sakari this morning in the media tree https://git.linuxtv.org/sailus/media_tree.git/log/?h=atomisp > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-09-22 22:17 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-07-14 8:23 [PATCH v3] clk: x86: Do not gate clocks enabled by the firmware Carlo Caione 2017-07-18 23:24 ` Stephen Boyd 2017-09-07 9:22 ` Carlo Caione 2017-09-07 11:59 ` Pierre-Louis Bossart 2017-09-07 21:51 ` Pierre-Louis Bossart 2017-09-08 8:05 ` Carlo Caione 2017-09-18 8:05 ` Andy Shevchenko 2017-09-22 21:36 ` Darren Hart 2017-09-22 21:47 ` Carlo Caione 2017-09-22 22:17 ` Pierre-Louis Bossart
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).