Re: [PATCH v2] thermal: ti-soc-thermal: dra7: Implement Workaround for Errata i814 - Bandgap Temperature read Dtemp can be corrupted
Hi Eduardo, Thanks for the review comments. On Wednesday 08 April 2015 01:35 AM, Eduardo Valentin wrote: Hello K, On Tue, Apr 07, 2015 at 08:50:04AM +0530, Keerthy wrote: Hi Eduardo, On Friday 27 March 2015 02:15 PM, Keerthy wrote: Bandgap Temperature read Dtemp can be corrupted DESCRIPTION Read accesses to registers listed below can be corrupted due to incorrect resynchronization between clock domains. Read access to registers below can be corrupted : • CTRL_CORE_DTEMP_MPU/GPU/CORE/DSPEVE/IVA_n (n = 0 to 4) • CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA_n WORKAROUND Multiple reads to CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA[9:0]: BGAP_DTEMPMPU/GPU/CORE/DSPEVE/IVA is needed to discard false value and read right value: 1. Perform two successive reads to BGAP_DTEMP bit field. (a) If read1 returns Val1 and read2 returns Val1, then right value is Val1. (b) If read1 returns Val1, read 2 returns Val2, a third read is needed. 2. Perform third read (a) If read3 returns Val2 then right value is Val2. (b) If read3 returns Val3, then right value is Val3. The third read description (b) is misleading/confusing. Does it mean third read value is always correct or do we need to compare against val1 and val2? if val3 != val2 && val3 != val1, which one is correct? none? That is all is given in the errata description. Seems to assume third one is right value always. Thanks for the detailed errata description. A gentle ping on this. Sorry for the late answer. (minor) Comment below: Signed-off-by: Keerthy --- Read all the temperatures from the 5 sensors and saw that they were sane with time. Ran cpuloadgen and saw that temperatures rising on all the sensors and cooled down as soon as the load was reduced. .../thermal/ti-soc-thermal/dra752-thermal-data.c | 3 +- drivers/thermal/ti-soc-thermal/ti-bandgap.c| 37 +- drivers/thermal/ti-soc-thermal/ti-bandgap.h| 4 +++ 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c b/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c index a492927..58b5c66 100644 --- a/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c +++ b/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c @@ -420,7 +420,8 @@ const struct ti_bandgap_data dra752_data = { TI_BANDGAP_FEATURE_FREEZE_BIT | TI_BANDGAP_FEATURE_TALERT | TI_BANDGAP_FEATURE_COUNTER_DELAY | - TI_BANDGAP_FEATURE_HISTORY_BUFFER, + TI_BANDGAP_FEATURE_HISTORY_BUFFER | + TI_BANDGAP_FEATURE_ERRATA_814, .fclock_name = "l3instr_ts_gclk_div", .div_ck_name = "l3instr_ts_gclk_div", .conv_table = dra752_adc_to_temp, diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c index 74c0e34..baf822e 100644 --- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c +++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c @@ -119,6 +119,37 @@ exit: } /** + * ti_dra7_bandgap_read_temp() - helper function to read dra7 sensor temperature + * @bgp: pointer to ti_bandgap structure + * @reg: desired register (offset) to be read + * + * Function to read dra7 bandgap sensor temperature. This is done separately + * so as to workaround the errata "Bandgap Temperature read Dtemp can be + * corrupted" - Errata ID: i814". + * Read accesses to registers listed below can be corrupted due to incorrect + * resynchronization between clock domains. + * Read access to registers below can be corrupted : + * CTRL_CORE_DTEMP_MPU/GPU/CORE/DSPEVE/IVA_n (n = 0 to 4) + * CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA_n + * + * Return: the register value. + */ +static u32 ti_dra7_bandgap_read_temp(struct ti_bandgap *bgp, u32 reg) Are you sure this bug affects only DRA7? I would prefer you name this not specific to soc version. Maybe bind it to the errata: AFAIK it is impacting DRA7 not sure of OMAP5 and OMAP4. Okay I can name it on the errata number. Yeah makes sense too. If other versions tend to have this then we can just add this errata feature to them also. +static u32 ti_i814_bandgap_read_temp(struct ti_bandgap *bgp, u32 reg) +{ + u32 val1, val2; + + val1 = ti_bandgap_readl(bgp, reg); + val2 = ti_bandgap_readl(bgp, reg); + +/* If both times we read the same value then that is right */ Please, indent the comments too. Okay. + if (val1 == val2) + return val1; + +/* if val1 and val2 are different read it third time */ ditto. Okay + return ti_bandgap_readl(bgp, reg); Actually, if I understood the errata description correctly, you need to: 1. read a third time (val3) 2
Re: [PATCH] mm/migrate: Mark unmap_and_move() "noinline" to avoid ICE in gcc 4.7.3
Andrew Morton writes: > On Tue, 07 Apr 2015 16:27:44 -0700 Kevin Hilman wrote: > >> >> > It should all be there today? >> >> >> >> Nope. >> > >> > huh, I swear I did an mmotm yesterday. >> >> Well, based on the timestamp of the mmotm dir on ozlabs, it appears you >> did. That's why I was confused why the gcc-473 patches from mmots aren't >> there. > > Things look a bit better now. Yup, I can confirm all 4 patches are there now. Things should be in good shape for the next -next. Thanks, Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mm/migrate: Mark unmap_and_move() "noinline" to avoid ICE in gcc 4.7.3
On Tue, 07 Apr 2015 16:27:44 -0700 Kevin Hilman wrote: > >> > It should all be there today? > >> > >> Nope. > > > > huh, I swear I did an mmotm yesterday. > > Well, based on the timestamp of the mmotm dir on ozlabs, it appears you > did. That's why I was confused why the gcc-473 patches from mmots aren't > there. Things look a bit better now. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mm/migrate: Mark unmap_and_move() "noinline" to avoid ICE in gcc 4.7.3
Andrew Morton writes: > On Tue, 07 Apr 2015 15:41:32 -0700 Kevin Hilman wrote: > >> Andrew Morton writes: >> >> > On Tue, 7 Apr 2015 10:57:52 -0700 Kevin Hilman wrote: >> > >> >> > The diff below[2] on top of yours compiles fine here and at least covers >> >> > the compilers I *know* to trigger the ICE. >> >> >> >> I see my fix in your mmots since last Thurs (4/2), but it's not in >> >> mmotm (last updated today) so today's linux-next still has the ICE for >> >> anything other than gcc-4.7.3. Just checking to see when you plan to >> >> update mmotm. >> > >> > It should all be there today? >> >> Nope. > > huh, I swear I did an mmotm yesterday. Well, based on the timestamp of the mmotm dir on ozlabs, it appears you did. That's why I was confused why the gcc-473 patches from mmots aren't there. > Let me see if I can sort out the watchdog mess and produce something > releasable... OK, thanks. Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/14] clkdev: add clkdev_create() helper
On 04/07/15 05:43, Russell King - ARM Linux wrote: > On Mon, Apr 06, 2015 at 01:19:33PM -0700, Stephen Boyd wrote: >> On 04/03/15 10:12, Russell King wrote: >>> @@ -316,6 +329,29 @@ clkdev_alloc(struct clk *clk, const char *con_id, >>> const char *dev_fmt, ...) >>> } >>> EXPORT_SYMBOL(clkdev_alloc); >>> >>> +/** >>> + * clkdev_create - allocate and add a clkdev lookup structure >>> + * @clk: struct clk to associate with all clk_lookups >>> + * @con_id: connection ID string on device >>> + * @dev_fmt: format string describing device name >>> + * >>> + * Returns a clk_lookup structure, which can be later unregistered and >>> + * freed. >> Please add that this returns NULL on failure. > Will do, but please remember that _I'm_ taking the clkdev patches through > my tree, as I'm the maintainer for clkdev. Thanks. > Sounds good to me. Thanks. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mm/migrate: Mark unmap_and_move() "noinline" to avoid ICE in gcc 4.7.3
On Tue, 07 Apr 2015 15:41:32 -0700 Kevin Hilman wrote: > Andrew Morton writes: > > > On Tue, 7 Apr 2015 10:57:52 -0700 Kevin Hilman wrote: > > > >> > The diff below[2] on top of yours compiles fine here and at least covers > >> > the compilers I *know* to trigger the ICE. > >> > >> I see my fix in your mmots since last Thurs (4/2), but it's not in > >> mmotm (last updated today) so today's linux-next still has the ICE for > >> anything other than gcc-4.7.3. Just checking to see when you plan to > >> update mmotm. > > > > It should all be there today? > > Nope. huh, I swear I did an mmotm yesterday. Let me see if I can sort out the watchdog mess and produce something releasable... -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mm/migrate: Mark unmap_and_move() "noinline" to avoid ICE in gcc 4.7.3
Andrew Morton writes: > On Tue, 7 Apr 2015 10:57:52 -0700 Kevin Hilman wrote: > >> > The diff below[2] on top of yours compiles fine here and at least covers >> > the compilers I *know* to trigger the ICE. >> >> I see my fix in your mmots since last Thurs (4/2), but it's not in >> mmotm (last updated today) so today's linux-next still has the ICE for >> anything other than gcc-4.7.3. Just checking to see when you plan to >> update mmotm. > > It should all be there today? Nope. In mmotm, only the original patch plus your first fix is there: $ curl -sO http://www.ozlabs.org/~akpm/mmotm/broken-out.tar.gz $ tar -tavf broken-out.tar.gz |grep gcc-473 -rw-r- akpm/eng 1838 2015-04-01 14:41 broken-out/mm-migrate-mark-unmap_and_move-noinline-to-avoid-ice-in-gcc-473.patch -rw-r- akpm/eng 1309 2015-04-01 14:41 broken-out/mm-migrate-mark-unmap_and_move-noinline-to-avoid-ice-in-gcc-473-fix.patch but in mmots, the additional ptch from me, plus another comment fixup from you are also there: $ curl -sO http://www.ozlabs.org/~akpm/mmots/broken-out.tar.gz $ tar -tavf broken-out.tar.gz |grep gcc-473 -rw-r- akpm/eng 1882 2015-04-06 16:24 broken-out/mm-migrate-mark-unmap_and_move-noinline-to-avoid-ice-in-gcc-473.patch -rw-r- akpm/eng 1271 2015-04-06 16:24 broken-out/mm-migrate-mark-unmap_and_move-noinline-to-avoid-ice-in-gcc-473-fix.patch -rw-r- akpm/eng 1382 2015-04-06 16:24 broken-out/mm-migrate-mark-unmap_and_move-noinline-to-avoid-ice-in-gcc-473-fix-fix.patch -rw-r- akpm/eng968 2015-04-06 16:24 broken-out/mm-migrate-mark-unmap_and_move-noinline-to-avoid-ice-in-gcc-473-fix-fix-fix.patch Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mm/migrate: Mark unmap_and_move() "noinline" to avoid ICE in gcc 4.7.3
On Tue, 7 Apr 2015 10:57:52 -0700 Kevin Hilman wrote: > > The diff below[2] on top of yours compiles fine here and at least covers > > the compilers I *know* to trigger the ICE. > > I see my fix in your mmots since last Thurs (4/2), but it's not in > mmotm (last updated today) so today's linux-next still has the ICE for > anything other than gcc-4.7.3. Just checking to see when you plan to > update mmotm. It should all be there today? -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] thermal: ti-soc-thermal: dra7: Implement Workaround for Errata i814 - Bandgap Temperature read Dtemp can be corrupted
Hello K, On Tue, Apr 07, 2015 at 08:50:04AM +0530, Keerthy wrote: > Hi Eduardo, > > On Friday 27 March 2015 02:15 PM, Keerthy wrote: > >Bandgap Temperature read Dtemp can be corrupted > > > >DESCRIPTION > > Read accesses to registers listed below can be corrupted due to > > incorrect resynchronization between > > clock domains. > > > > Read access to registers below can be corrupted : > > • CTRL_CORE_DTEMP_MPU/GPU/CORE/DSPEVE/IVA_n (n = 0 to 4) > > • CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA_n > > > >WORKAROUND > > Multiple reads to > > CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA[9:0]: > > BGAP_DTEMPMPU/GPU/CORE/DSPEVE/IVA is needed to discard false value > > and read right value: > > 1. Perform two successive reads to BGAP_DTEMP bit field. > > (a) If read1 returns Val1 and read2 returns Val1, > > then right value is Val1. > > (b) If read1 returns Val1, read 2 returns Val2, a > > third read is needed. > > 2. Perform third read > > (a) If read3 returns Val2 then right value is Val2. > > (b) If read3 returns Val3, then right value is Val3. The third read description (b) is misleading/confusing. Does it mean third read value is always correct or do we need to compare against val1 and val2? if val3 != val2 && val3 != val1, which one is correct? none? > > > Thanks for the detailed errata description. > A gentle ping on this. > Sorry for the late answer. (minor) Comment below: > >Signed-off-by: Keerthy > >--- > > > >Read all the temperatures from the 5 sensors and saw that > >they were sane with time. > > > >Ran cpuloadgen and saw that temperatures rising on all the sensors > >and cooled down as soon as the load was reduced. > > > > .../thermal/ti-soc-thermal/dra752-thermal-data.c | 3 +- > > drivers/thermal/ti-soc-thermal/ti-bandgap.c| 37 > > +- > > drivers/thermal/ti-soc-thermal/ti-bandgap.h| 4 +++ > > 3 files changed, 42 insertions(+), 2 deletions(-) > > > >diff --git a/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c > >b/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c > >index a492927..58b5c66 100644 > >--- a/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c > >+++ b/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c > >@@ -420,7 +420,8 @@ const struct ti_bandgap_data dra752_data = { > > TI_BANDGAP_FEATURE_FREEZE_BIT | > > TI_BANDGAP_FEATURE_TALERT | > > TI_BANDGAP_FEATURE_COUNTER_DELAY | > >-TI_BANDGAP_FEATURE_HISTORY_BUFFER, > >+TI_BANDGAP_FEATURE_HISTORY_BUFFER | > >+TI_BANDGAP_FEATURE_ERRATA_814, > > .fclock_name = "l3instr_ts_gclk_div", > > .div_ck_name = "l3instr_ts_gclk_div", > > .conv_table = dra752_adc_to_temp, > >diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c > >b/drivers/thermal/ti-soc-thermal/ti-bandgap.c > >index 74c0e34..baf822e 100644 > >--- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c > >+++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c > >@@ -119,6 +119,37 @@ exit: > > } > > > > /** > >+ * ti_dra7_bandgap_read_temp() - helper function to read dra7 sensor > >temperature > >+ * @bgp: pointer to ti_bandgap structure > >+ * @reg: desired register (offset) to be read > >+ * > >+ * Function to read dra7 bandgap sensor temperature. This is done separately > >+ * so as to workaround the errata "Bandgap Temperature read Dtemp can be > >+ * corrupted" - Errata ID: i814". > >+ * Read accesses to registers listed below can be corrupted due to incorrect > >+ * resynchronization between clock domains. > >+ * Read access to registers below can be corrupted : > >+ * CTRL_CORE_DTEMP_MPU/GPU/CORE/DSPEVE/IVA_n (n = 0 to 4) > >+ * CTRL_CORE_TEMP_SENSOR_MPU/GPU/CORE/DSPEVE/IVA_n > >+ * > >+ * Return: the register value. > >+ */ > >+static u32 ti_dra7_bandgap_read_temp(struct ti_bandgap *bgp, u32 reg) Are you sure this bug affects only DRA7? I would prefer you name this not specific to soc version. Maybe bind it to the errata: +static u32 ti_i814_bandgap_read_temp(struct ti_bandgap *bgp, u32 reg) > >+{ > >+u32 val1, val2; > >+ > >+val1 = ti_bandgap_readl(bgp, reg); > >+val2 = ti_bandgap_readl(bgp, reg); > >+ > >+/* If both times we read the same value then that is right */ Please, indent the comments too. > >+if (val1 == val2) > >+return val1; > >+ > >+/* if val1 and val2 are different read it third time */ ditto. > >+return ti_bandgap_readl(bgp, reg); Actually, if I understood the errata description correctly, you need to: 1. read a third time (val3) 2. compare val3 against val2, and return val2 if they are same If they are different, the errata is not clear. Can you please clarify? > >+} > >+ > >+/** > > * ti_bandgap_read_temp() - helper function to rea
Re: [PATCHv2] ti-soc-thermal: implement omap3 support
Hi Pavel, On Thu, Apr 02, 2015 at 04:49:07PM +0200, Pavel Machek wrote: > > > This adds support for OMAP3 chips to ti-soc-thermal. As requested by > TI people, it is marked unreliable and warning is printed. > > Signed-off-by: Pavel Machek > > --- > > Patch is against thermal linus tree, please apply so that it has > chance for 4.1. I thought of accepting this one the way it and amending the message myself, but you missed the DT example. Actually it is bests, as I already mention, that you send the patch together with the DT changes too. Can you please resend this one with the following (minor) changes: > > v2: fixed min/max temp > > diff --git a/drivers/thermal/ti-soc-thermal/Kconfig > b/drivers/thermal/ti-soc-thermal/Kconfig > index bd4c7be..d414c2d 100644 > --- a/drivers/thermal/ti-soc-thermal/Kconfig > +++ b/drivers/thermal/ti-soc-thermal/Kconfig > @@ -21,6 +21,21 @@ config TI_THERMAL > This includes trip points definitions, extrapolation rules and > CPU cooling device bindings. > > +config OMAP3_THERMAL > + bool "Texas Instruments OMAP3 thermal support" > + depends on TI_SOC_THERMAL > + depends on ARCH_OMAP3 > + help > + If you say yes here you get thermal support for the Texas Instruments > + OMAP3 SoC family. The current chips supported are: > +- OMAP3430 > + > + OMAP3 chips normally don't need thermal management, and sensors in > + this generation are not accurate, nor they are very close to > + the important hotspots. > + > + Say 'N' here. > + > config OMAP4_THERMAL > bool "Texas Instruments OMAP4 thermal support" > depends on TI_SOC_THERMAL > diff --git a/drivers/thermal/ti-soc-thermal/Makefile > b/drivers/thermal/ti-soc-thermal/Makefile > index 1226b24..0f89bdf 100644 > --- a/drivers/thermal/ti-soc-thermal/Makefile > +++ b/drivers/thermal/ti-soc-thermal/Makefile > @@ -2,5 +2,6 @@ obj-$(CONFIG_TI_SOC_THERMAL) += ti-soc-thermal.o > ti-soc-thermal-y := ti-bandgap.o > ti-soc-thermal-$(CONFIG_TI_THERMAL) += ti-thermal-common.o > ti-soc-thermal-$(CONFIG_DRA752_THERMAL) += dra752-thermal-data.o > +ti-soc-thermal-$(CONFIG_OMAP3_THERMAL) += omap3-thermal-data.o > ti-soc-thermal-$(CONFIG_OMAP4_THERMAL) += omap4-thermal-data.o > ti-soc-thermal-$(CONFIG_OMAP5_THERMAL) += omap5-thermal-data.o > diff --git a/drivers/thermal/ti-soc-thermal/omap3-thermal-data.c > b/drivers/thermal/ti-soc-thermal/omap3-thermal-data.c > new file mode 100644 > index 000..86a4e2d > --- /dev/null > +++ b/drivers/thermal/ti-soc-thermal/omap3-thermal-data.c > @@ -0,0 +1,103 @@ > +/* > + * OMAP3 thermal driver. > + * > + * Copyright (C) 2011-2012 Texas Instruments Inc. > + * Copyright (C) 2014 Pavel Machek > + * > + * This software is licensed under the terms of the GNU General Public > + * License version 2, as published by the Free Software Foundation, and > + * may be copied, distributed, and modified under those terms. > + * > + * 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. > + * > + * Note > + * http://www.ti.com/lit/er/sprz278f/sprz278f.pdf "Advisory > + * 3.1.1.186 MMC OCP Clock Not Gated When Thermal Sensor Is Used" > + * > + * Also TI says: > + * Just be careful when you try to make thermal policy like decisions > + * based on this sensor. Placement of the sensor w.r.t the actual logic > + * generating heat has to be a factor as well. If you are just looking > + * for an approximation temperature (thermometerish kind), you might be > + * ok with this. I am not sure we'd find any TI data around this.. just a > + * heads up. > + */ > + > +#include "ti-thermal.h" > +#include "ti-bandgap.h" > + > +/* > + * OMAP34XX has one instance of thermal sensor for MPU > + * need to describe the individual bit fields > + */ > +static struct temp_sensor_registers > +omap34xx_mpu_temp_sensor_registers = { > + .temp_sensor_ctrl = 0, > + .bgap_soc_mask = BIT(8), > + .bgap_eocz_mask = BIT(7), > + .bgap_dtemp_mask = 0x7f, > + > + .bgap_mode_ctrl = 0, > + .mode_ctrl_mask = BIT(9), > +}; > + > +/* Thresholds and limits for OMAP34XX MPU temperature sensor */ > +static struct temp_sensor_data omap34xx_mpu_temp_sensor_data = { > + .min_freq = 32768, > + .max_freq = 32768, > + .max_temp = 125000, > + .min_temp = -4, > + .hyst_val = 5000, > +}; > + > +/* > + * Temperature values in milli degree celsius > + */ > +static const int > +omap34xx_adc_to_temp[128] = { > + -4, -4, -4, -4, -4, -39000, -38000, -36000, > + -34000, -32000, -31000, -29000, -28000, -26000, -25000, -24000, > + -22000, -21000, -19000, -18000, -17000, -15000, -14000, -12000, > + -11000, -9000, -8000, -7000, -5000, -4000, -2000, -1000,
Re: [PATCHv2] ti-soc-thermal: implement omap3 support
On Thu, Apr 02, 2015 at 04:49:07PM +0200, Pavel Machek wrote: > > > This adds support for OMAP3 chips to ti-soc-thermal. As requested by > TI people, it is marked unreliable and warning is printed. > > Signed-off-by: Pavel Machek > > --- > > Patch is against thermal linus tree, please apply so that it has > chance for 4.1. > > v2: fixed min/max temp > > diff --git a/drivers/thermal/ti-soc-thermal/Kconfig > b/drivers/thermal/ti-soc-thermal/Kconfig > index bd4c7be..d414c2d 100644 > --- a/drivers/thermal/ti-soc-thermal/Kconfig > +++ b/drivers/thermal/ti-soc-thermal/Kconfig > @@ -21,6 +21,21 @@ config TI_THERMAL > This includes trip points definitions, extrapolation rules and > CPU cooling device bindings. > > +config OMAP3_THERMAL > + bool "Texas Instruments OMAP3 thermal support" > + depends on TI_SOC_THERMAL > + depends on ARCH_OMAP3 > + help > + If you say yes here you get thermal support for the Texas Instruments > + OMAP3 SoC family. The current chips supported are: > +- OMAP3430 > + > + OMAP3 chips normally don't need thermal management, and sensors in > + this generation are not accurate, nor they are very close to > + the important hotspots. > + > + Say 'N' here. > + > config OMAP4_THERMAL > bool "Texas Instruments OMAP4 thermal support" > depends on TI_SOC_THERMAL > diff --git a/drivers/thermal/ti-soc-thermal/Makefile > b/drivers/thermal/ti-soc-thermal/Makefile > index 1226b24..0f89bdf 100644 > --- a/drivers/thermal/ti-soc-thermal/Makefile > +++ b/drivers/thermal/ti-soc-thermal/Makefile > @@ -2,5 +2,6 @@ obj-$(CONFIG_TI_SOC_THERMAL) += ti-soc-thermal.o > ti-soc-thermal-y := ti-bandgap.o > ti-soc-thermal-$(CONFIG_TI_THERMAL) += ti-thermal-common.o > ti-soc-thermal-$(CONFIG_DRA752_THERMAL) += dra752-thermal-data.o > +ti-soc-thermal-$(CONFIG_OMAP3_THERMAL) += omap3-thermal-data.o > ti-soc-thermal-$(CONFIG_OMAP4_THERMAL) += omap4-thermal-data.o > ti-soc-thermal-$(CONFIG_OMAP5_THERMAL) += omap5-thermal-data.o > diff --git a/drivers/thermal/ti-soc-thermal/omap3-thermal-data.c > b/drivers/thermal/ti-soc-thermal/omap3-thermal-data.c > new file mode 100644 > index 000..86a4e2d > --- /dev/null > +++ b/drivers/thermal/ti-soc-thermal/omap3-thermal-data.c > @@ -0,0 +1,103 @@ > +/* > + * OMAP3 thermal driver. > + * > + * Copyright (C) 2011-2012 Texas Instruments Inc. > + * Copyright (C) 2014 Pavel Machek > + * > + * This software is licensed under the terms of the GNU General Public > + * License version 2, as published by the Free Software Foundation, and > + * may be copied, distributed, and modified under those terms. > + * > + * 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. > + * > + * Note > + * http://www.ti.com/lit/er/sprz278f/sprz278f.pdf "Advisory > + * 3.1.1.186 MMC OCP Clock Not Gated When Thermal Sensor Is Used" > + * > + * Also TI says: > + * Just be careful when you try to make thermal policy like decisions > + * based on this sensor. Placement of the sensor w.r.t the actual logic > + * generating heat has to be a factor as well. If you are just looking > + * for an approximation temperature (thermometerish kind), you might be > + * ok with this. I am not sure we'd find any TI data around this.. just a > + * heads up. > + */ > + > +#include "ti-thermal.h" > +#include "ti-bandgap.h" > + > +/* > + * OMAP34XX has one instance of thermal sensor for MPU > + * need to describe the individual bit fields > + */ > +static struct temp_sensor_registers > +omap34xx_mpu_temp_sensor_registers = { > + .temp_sensor_ctrl = 0, > + .bgap_soc_mask = BIT(8), > + .bgap_eocz_mask = BIT(7), > + .bgap_dtemp_mask = 0x7f, > + > + .bgap_mode_ctrl = 0, > + .mode_ctrl_mask = BIT(9), > +}; > + > +/* Thresholds and limits for OMAP34XX MPU temperature sensor */ > +static struct temp_sensor_data omap34xx_mpu_temp_sensor_data = { > + .min_freq = 32768, > + .max_freq = 32768, > + .max_temp = 125000, > + .min_temp = -4, > + .hyst_val = 5000, > +}; > + > +/* > + * Temperature values in milli degree celsius > + */ > +static const int > +omap34xx_adc_to_temp[128] = { > + -4, -4, -4, -4, -4, -39000, -38000, -36000, > + -34000, -32000, -31000, -29000, -28000, -26000, -25000, -24000, > + -22000, -21000, -19000, -18000, -17000, -15000, -14000, -12000, > + -11000, -9000, -8000, -7000, -5000, -4000, -2000, -1000, , > + 1000, 3000, 4000, 5000, 7000, 8000, 1, 11000, 13000, 14000, > + 15000, 17000, 18000, 2, 21000, 22000, 24000, 25000, 27000, > + 28000, 3, 31000, 32000, 34000, 35000, 37000, 38000, 39000, > + 41000, 42000, 44000, 45000, 47000, 48000, 49000, 51000, 52000, >
Re: [PATCH] mm/migrate: Mark unmap_and_move() "noinline" to avoid ICE in gcc 4.7.3
Hi Andrew, On Wed, Apr 1, 2015 at 2:54 PM, Kevin Hilman wrote: > Andrew Morton writes: > >> On Wed, 01 Apr 2015 10:47:49 +0100 Marc Zyngier wrote: >> >>> > -static int unmap_and_move(new_page_t get_new_page, free_page_t >>> > put_new_page, >>> > - unsigned long private, struct page *page, int force, >>> > - enum migrate_mode mode) >>> > +static noinline int unmap_and_move(new_page_t get_new_page, >>> > + free_page_t put_new_page, >>> > + unsigned long private, struct page *page, >>> > + int force, enum migrate_mode mode) >>> > { >>> >int rc = 0; >>> >int *result = NULL; >>> > >>> >>> Ouch. That's really ugly. And on 32bit ARM, we end-up spilling half of >>> the parameters on the stack, which is not going to help performance >>> either (not that this would be useful on 32bit ARM anyway...). >>> >>> Any chance you could make this dependent on some compiler detection >>> mechanism? >> >> With my arm compiler (gcc-4.4.4) the patch makes no difference - >> unmap_and_move() isn't being inlined anyway. >> >> How does this look? >> >> Kevin, could you please retest? I might have fat-fingered something... > > Your patch on top of Geert's still compiles fine for me with gcc-4.7.3. > However, I'm not sure how specific we can be on the versions. > > /me goes to test a few more compilers... OK... > > ICE: 4.7.1, 4.7.3, 4.8.3 > OK: 4.6.3, 4.9.2, 4.9.3 > > The diff below[2] on top of yours compiles fine here and at least covers > the compilers I *know* to trigger the ICE. I see my fix in your mmots since last Thurs (4/2), but it's not in mmotm (last updated today) so today's linux-next still has the ICE for anything other than gcc-4.7.3. Just checking to see when you plan to update mmotm. Thanks, Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] ARM: OMAP2+: OMAP DRA7xx display-related hwmod data changes for v4.1
On Tue, 7 Apr 2015, Tomi Valkeinen wrote: > Hi, > > No, don't pull this, it doesn't work. > > Paul, the hwmod patch cannot be picked from the DRA7 DSS series. We have > the problem with the DES HDCP clock, without that clock we will see > WARNINGs when booting if the "set DSS submodule parent hwmods" is applied. > > We do see some errors when booting even with the current mainline, > caused by not having that clock enabled: > > [0.194524] omap_hwmod: dss_core: _wait_target_disable failed > [0.194540] omap_hwmod: dss_core: _wait_target_ready failed: -16 > [0.194554] omap_hwmod: dss_core: cannot be enabled for reset (3) > [0.197143] omap_hwmod: dss_dispc: _wait_target_ready failed: -16 > [0.197156] omap_hwmod: dss_dispc: cannot be enabled for reset (3) > [0.199745] omap_hwmod: dss_hdmi: _wait_target_ready failed: -16 > [0.199759] omap_hwmod: dss_hdmi: cannot be enabled for reset (3) > > But that's still much less spam than the above plus ~4 WARNs. > > I'd rather merge the DRA7 DSS patches in one go, to avoid issues related > to the HDPC clock. OK, sorry for the misunderstanding. - Paul -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ARM errata 430973 on multi platform kernels
* Tony Lindgren [150407 08:27]: > * Russell King - ARM Linux [150407 06:58]: > > > > Well, one thing we can do is to tweak the proc-v7*.S such that we detect > > Cortex-A8 separately, and only execute the BTB flush for CA8 processors > > if the errata is enabled. Something like this (untested): > > > > --- a/arch/arm/mm/proc-v7.S > > +++ b/arch/arm/mm/proc-v7.S > > @@ -527,6 +545,16 @@ __v7_ca9mp_proc_info: > > __v7_proc __v7_ca9mp_proc_info, __v7_ca9mp_setup, proc_fns = > > ca9mp_processor_functions > > .size __v7_ca9mp_proc_info, . - __v7_ca9mp_proc_info > > > > + /* > > +* ARM Ltd. Cortex A8 processor. > > +*/ > > + .type __v7_ca8_proc_info, #object > > +__v7_ca8_proc_info: > > + .long 0x410fc080 > > + .long 0xff00 > > + __v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions > > + .size __v7_ca8_proc_info, . - __v7_ca8_proc_info > > + > > #endif /* CONFIG_ARM_LPAE */ > > > > /* > > Works for me. The above needs the following fix folded in to build: > > --- a/arch/arm/mm/proc-v7.S > +++ b/arch/arm/mm/proc-v7.S > @@ -532,7 +532,7 @@ __v7_ca9mp_proc_info: > __v7_ca8_proc_info: > .long 0x410fc080 > .long 0xff00 > - __v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions > + __v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = > ca8_processor_functions > .size __v7_ca8_proc_info, . - __v7_ca8_proc_info > > #endif /* CONFIG_ARM_LPAE */ And then on top of that patch, we can fix the sefaulting issues with the following, what do you guys think? Regards, Tony 8< From: Tony Lindgren Date: Tue, 7 Apr 2015 07:52:39 -0700 Subject: [PATCH] ARM: mm: Fix Cortex-A8 erratum 430973 segfaults for bootloaders and multiarch Looks like apps can be made to segfault easily on armhf distros just by running cpuburn-a8 in the background, then starting apt get update unless erratum 430973 workaround is enabled. This happens on r3p2 also, which has 430973 fixed in hardware. Turns out the reason for this is some bootloaders incorrectly setting the auxilary register IBE bit, which probably causes us to hit erratum 687067 on Cortex-A8 later than r1p2. Now that Cortex-A8 has it's own cpu_ca8_switch_mm, we can fix these issues: 1. If the bootloader incorrectly sets the IBE bit in the auxilary control register for Cortex-A8 revisions with 430973 fixed in hardware, we need to call flush BTAC/BTB to avoid segfaults probably caused by erratum 687067. So let's flush BTAC/BTB unconditionally for Cortex-A8. It won't do anything unless the IBE bit is set. 2. We can and should now keep 430973 enabled for multiarch builds as it no longer causes issues with Cortex-A9 as we have a separate cpu_ca8_switch_mm. Note that SoCs probably should also add checks and print warnings for the misconfigured IBE bit depending on the Cortex-A8 revision so the bootloaders can be fixed Cortex-A8 revisions later than r1p2 to not set the IBE bit. Signed-off-by: Tony Lindgren --- a/arch/arm/mm/proc-v7-2level.S +++ b/arch/arm/mm/proc-v7-2level.S @@ -36,14 +36,16 @@ * * It is assumed that: * - we are not using split page tables + * + * Note that we always need to flush BTAC/BTB if IBE is set + * even on Cortex-A8 revisions not affected by 430973. + * If IBE is not set, the flush BTAC/BTB won't do anything. */ ENTRY(cpu_ca8_switch_mm) #ifdef CONFIG_MMU mov r2, #0 -#ifdef CONFIG_ARM_ERRATA_430973 mcr p15, 0, r2, c7, c5, 6 @ flush BTAC/BTB #endif -#endif ENTRY(cpu_v7_switch_mm) #ifdef CONFIG_MMU mmidr1, r1 @ get mm->context.id --- a/arch/arm/mm/proc-v7.S +++ b/arch/arm/mm/proc-v7.S @@ -352,7 +352,7 @@ __v7_setup: ldr r10, =0x0c08@ Cortex-A8 primary part number teq r0, r10 bne 2f -#if defined(CONFIG_ARM_ERRATA_430973) && !defined(CONFIG_ARCH_MULTIPLATFORM) +#if defined(CONFIG_ARM_ERRATA_430973) teq r5, #0x0010 @ only present in r1p* mrceq p15, 0, r10, c1, c0, 1 @ read aux control register -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ARM errata 430973 on multi platform kernels
* Russell King - ARM Linux [150407 06:58]: > On Mon, Apr 06, 2015 at 10:42:45AM -0700, Tony Lindgren wrote: > > * Ivaylo Dimitrov [150406 10:15]: > > > On 6.04.2015 18:40, Tony Lindgren wrote: > > > > > > > >Oops sorry, wrong numbers for errata above.. s/458693/430973/, here's > > > >a better version: > > > > > > > >1. For cortex-a8 revisions affected by 430973, we can do a custom > > > >cpu_v7_switch_mm function that always does flush BTAC/BTB. > > > > > > > > > > Why custom function, if IBE bit is zero, BTB invalidate instruction is a > > > NOP. Do you think that "mcr p15, 0, r2, c7, c5, 6" executed as a NOP will > > > put so much overhead, that it deserves a custom function? > > > > Hmm but it still seems to do something also on cortex-a8 r3p2 that > > is supposedly not affected by 430973.. Based on my tests so far, at least > > armhf running cpuburn-a8 in the background and doing apt-get update > > segfaults constantly without flush BTAC/BTB. This seems to be the case > > no matter how the aux ctrl reg bits are set.. This should be reproducable > > on any pandboard xm BTW. > > > > > >2. For HS cortex-a8 processors other than n900 affected by 430973, > > > >we need to implement functions similar to rx51_secure_update_aux_cr, > > > >the bootrom on n900 is different from TI HS omaps so the SMC call > > > >numbering may be different. > > > > > > > >3. For later cortex-a8 processors not affected by 430973, we need > > > >to clear IBE bit to avoid erratum 687067. > > > > > > > > > > Maybe it should be implemented something like: > > > > > > 1. if Cortex-A8, always execute invalidate BTB instruction in > > >cpu_v7_switch_mm > > > > This part still seems to need more investigating for why it's still > > needed also r3p2 as I describe above. Otherwise we may be hiding some > > other bug. > > > > > 2. For Cortex-A8 revisions affected by 430973, set IBE bit to 1, set it > > >to 0 for all others. That should happen as soon as possible, > > >otherwise kernel may crash on affected revisions if thumb- > > >compiled. > > > > Yes this makes sense. > > Well, one thing we can do is to tweak the proc-v7*.S such that we detect > Cortex-A8 separately, and only execute the BTB flush for CA8 processors > if the errata is enabled. Something like this (untested): > > --- a/arch/arm/mm/proc-v7.S > +++ b/arch/arm/mm/proc-v7.S > @@ -527,6 +545,16 @@ __v7_ca9mp_proc_info: > __v7_proc __v7_ca9mp_proc_info, __v7_ca9mp_setup, proc_fns = > ca9mp_processor_functions > .size __v7_ca9mp_proc_info, . - __v7_ca9mp_proc_info > > + /* > + * ARM Ltd. Cortex A8 processor. > + */ > + .type __v7_ca8_proc_info, #object > +__v7_ca8_proc_info: > + .long 0x410fc080 > + .long 0xff00 > + __v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions > + .size __v7_ca8_proc_info, . - __v7_ca8_proc_info > + > #endif /* CONFIG_ARM_LPAE */ > > /* Works for me. The above needs the following fix folded in to build: --- a/arch/arm/mm/proc-v7.S +++ b/arch/arm/mm/proc-v7.S @@ -532,7 +532,7 @@ __v7_ca9mp_proc_info: __v7_ca8_proc_info: .long 0x410fc080 .long 0xff00 - __v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions + __v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions .size __v7_ca8_proc_info, . - __v7_ca8_proc_info #endif /* CONFIG_ARM_LPAE */ Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ARM errata 430973 on multi platform kernels
* Matthijs van Duin [150406 20:50]: > On 7 April 2015 at 04:23, Tony Lindgren wrote: > > Oops, sorry user error.. I was trying to clear IBE as a banked register > > like L2 enable bit and of course it did not get cleared.. Clearing it > > with a smc call really clears it. And in that case my test case seems to > > work reliably on r3p2 without erratum 430973 enabled. > > So if I understand correctly, you actually had crashes which only > occurred with IBE enabled and the 430973 workaround disabled? That's right. It seems to happen at least with r3p2 that has 430973 fixed. > That's quite interesting, since it seems to me that can only be the > result of erratum 687067, which means > 1. secrom indeed fails to implement the 687067 workaround. > 2. "BTB invalidate by MVA" is used somewhere in the kernel. > The 430973 workaround would likely conceal this problem due to > regularly flushing the whole BTB, but I'm not sure how wise it is to > rely on that... Yes it seems to be hidden behind 430973. Anyways, we can print some warnings in the kernel for incorrect revision and IBE handling. > > I'm now thinking the kernel should just always set the 430973 specific > > cpu_v7_switch_mm for all cortex-a8 if IBE bit is set. > > And simply take the performance hit if secrom bogusly sets it and the > bootloader doesn't fix it? Yes it seems Russell's patch should do that for cortex-a8. > Sounds reasonable enough to me, given how platform-specific the > appropriate auxcr config is, as well as the means by which it can be > changed. Right, we have quite a few combinations already for omap3.. 34xx/36xx, HS/GP, TI vs Nokia bootrom.. Just proves how useless all these "security" "features" are in the long run :) They will just keep biting people over and over in the long run even if not used. > There's more secrom misconfiguration that the bootloader should fix > anyhow to make optimal use of the processor... Yeah. > > This will work as long as we can read the aux ctrl register IBE > > bit using mrc, which I believe is the case for all cortex-a8 based > > omap variants. > > Aux control is always readable, only write is an issue. OK, hopefully that's the case for 36xx HS version too.. I recall some registers reading as zero on N9 but hopefully not for the aux control register. > On 7 April 2015 at 05:12, Sebastian Reichel wrote: > > If I understood it correctly we can simply call the BTB flush on > > cortex-a8 if IBE bit is not set, since it would be translated as > > nop. > > Indeed you can safely use the BTB-flushing context switch on any cortex-a8. > > There's still value in checking if IBE is set on r2p1 or later, and if > so emit a warning about suboptimal performance. > > > So it should be safe to include the call on all cortex-a8 based > > cpus. We may need a non-BTB-flushing function for non-cortex-a8 > > based cpus, though. > > I just looked it up, apparently BTB-flushing on context switch is not > needed architecturally in ARMv7 (though it was in ARMv6), so that > version should probably indeed only be used for the cortex-a8. OK Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/14] media: omap3isp: remove unused clkdev
Hi Russell, On Tuesday 07 April 2015 13:45:36 Russell King - ARM Linux wrote: > On Tue, Apr 07, 2015 at 12:42:52PM +0300, Laurent Pinchart wrote: > > On Sunday 05 April 2015 15:20:34 Russell King - ARM Linux wrote: > > > On Sat, Apr 04, 2015 at 12:44:35AM +0300, Laurent Pinchart wrote: > > > > Hi Russell, > > > > > > > > Thank you for the patch; > > > > > > > > On Friday 03 April 2015 18:12:58 Russell King wrote: > > > > > No merged platform supplies xclks via platform data. As we want to > > > > > slightly change the clkdev interface, rather than fixing this unused > > > > > code, remove it instead. > > > > > > > > > > Signed-off-by: Russell King > > > > > > > > Acked-by: Laurent Pinchart > > > > > > > > with one caveat though : it conflicts with patches queued for v4.1 in > > > > the media tree. I'll post a rebased version in a reply to your e-mail. > > > > How would you like to handle the conflict ? > > > > > > How bad is the conflict? > > > > It's not too bad, it's mostly a context-related conflict. There are two > > additional lines to remove (plus the associated comment) from > > isp_xclk_init(), as your patch makes a loop now terminate with if > > (condition) continue;. Those two lines could be removed later, keeping > > them doesn't break anything. > > I think it's fine to take it through the media tree as the series doesn't > have any dependencies on this patch. It was merely attempting to get rid > of stuff so that we could move closer to clkdev dealing with a clk_hw > rather than a struct clk - but I never made it that far with the series. > Maybe at a later date... :) :-) I'll take the patch and send a pull request. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/14] ARM: orion: use clkdev_create()
Hi Russell, On 07/04/2015 16:01, Russell King - ARM Linux wrote: > On Tue, Apr 07, 2015 at 03:20:05PM +0200, Gregory CLEMENT wrote: >> Hi Andrew, Russell, >> >> On 04/04/2015 02:17, Andrew Lunn wrote: >>> On Fri, Apr 03, 2015 at 06:13:13PM +0100, Russell King wrote: clkdev_create() is a shorter way to write clkdev_alloc() followed by clkdev_add(). Use this instead. Signed-off-by: Russell King >>> >>> Acked-by: Andrew Lunn >> >> This change makes sens however what about Thomas' comment: removing >> orion_clkdev_add() entirely and directly using lkdev_create() all over >> the place: >> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/327294.html >> >> Then what would be the path for this patch? >> >> As there is a dependency on the 6th patch of this series: "clkdev: add >> clkdev_create() helper" which should be merged through the clk tree, I >> think the best option is that this patch would be also managed by the >> clk tree maintainer (I added them in CC). > > Let me remind people that clkdev is *NOT* part of clk, and that I'm the > maintainer for clkdev. Sorry for the confusion, I quickly had a look on the MAINTAINERS file and didn't realized that the drivers/clk/clkdev.c file was not part of clk (even if actually it was mentioned). > > I'm getting rather pissed off with people taking work away from me, even > when I'm named in the MAINTAINERS file. These patches are going through > my tree unless there's a good reason for them not to. They are _not_ > going through the clk tree. So, as you are going to take care of all the patches it is even simpler. You can take this one too: in mvebu there is no change on this file for this release so there won't be any conflict. Thanks, Gregory -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/14] ARM: orion: use clkdev_create()
On Tue, Apr 07, 2015 at 03:20:05PM +0200, Gregory CLEMENT wrote: > Hi Andrew, Russell, > > On 04/04/2015 02:17, Andrew Lunn wrote: > > On Fri, Apr 03, 2015 at 06:13:13PM +0100, Russell King wrote: > >> clkdev_create() is a shorter way to write clkdev_alloc() followed by > >> clkdev_add(). Use this instead. > >> > >> Signed-off-by: Russell King > > > > Acked-by: Andrew Lunn > > This change makes sens however what about Thomas' comment: removing > orion_clkdev_add() entirely and directly using lkdev_create() all over > the place: > http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/327294.html > > Then what would be the path for this patch? > > As there is a dependency on the 6th patch of this series: "clkdev: add > clkdev_create() helper" which should be merged through the clk tree, I > think the best option is that this patch would be also managed by the > clk tree maintainer (I added them in CC). Let me remind people that clkdev is *NOT* part of clk, and that I'm the maintainer for clkdev. I'm getting rather pissed off with people taking work away from me, even when I'm named in the MAINTAINERS file. These patches are going through my tree unless there's a good reason for them not to. They are _not_ going through the clk tree. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ARM errata 430973 on multi platform kernels
On Mon, Apr 06, 2015 at 08:14:30PM +0200, Matthijs van Duin wrote: > * Ivaylo Dimitrov [150406 10:15]: > > Why custom function, if IBE bit is zero, BTB invalidate instruction is a > > NOP. Do you think that "mcr p15, 0, r2, c7, c5, 6" executed as a NOP will > > put so much overhead, that it deserves a custom function? > > Executing them as nop is Cortex-A8 specific. On other ARMv7 CPUs, BTB > invalidation on context switch may be unnecessary yet expensive. That can be solved (see the patch I just posted) so I wouldn't worry too much about that issue. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ARM errata 430973 on multi platform kernels
On Mon, Apr 06, 2015 at 10:42:45AM -0700, Tony Lindgren wrote: > * Ivaylo Dimitrov [150406 10:15]: > > On 6.04.2015 18:40, Tony Lindgren wrote: > > > > > >Oops sorry, wrong numbers for errata above.. s/458693/430973/, here's > > >a better version: > > > > > >1. For cortex-a8 revisions affected by 430973, we can do a custom > > >cpu_v7_switch_mm function that always does flush BTAC/BTB. > > > > > > > Why custom function, if IBE bit is zero, BTB invalidate instruction is a > > NOP. Do you think that "mcr p15, 0, r2, c7, c5, 6" executed as a NOP will > > put so much overhead, that it deserves a custom function? > > Hmm but it still seems to do something also on cortex-a8 r3p2 that > is supposedly not affected by 430973.. Based on my tests so far, at least > armhf running cpuburn-a8 in the background and doing apt-get update > segfaults constantly without flush BTAC/BTB. This seems to be the case > no matter how the aux ctrl reg bits are set.. This should be reproducable > on any pandboard xm BTW. > > > >2. For HS cortex-a8 processors other than n900 affected by 430973, > > >we need to implement functions similar to rx51_secure_update_aux_cr, > > >the bootrom on n900 is different from TI HS omaps so the SMC call > > >numbering may be different. > > > > > >3. For later cortex-a8 processors not affected by 430973, we need > > >to clear IBE bit to avoid erratum 687067. > > > > > > > Maybe it should be implemented something like: > > > > 1. if Cortex-A8, always execute invalidate BTB instruction in > >cpu_v7_switch_mm > > This part still seems to need more investigating for why it's still > needed also r3p2 as I describe above. Otherwise we may be hiding some > other bug. > > > 2. For Cortex-A8 revisions affected by 430973, set IBE bit to 1, set it > >to 0 for all others. That should happen as soon as possible, > >otherwise kernel may crash on affected revisions if thumb- > >compiled. > > Yes this makes sense. Well, one thing we can do is to tweak the proc-v7*.S such that we detect Cortex-A8 separately, and only execute the BTB flush for CA8 processors if the errata is enabled. Something like this (untested): diff --git a/arch/arm/mm/proc-v7-2level.S b/arch/arm/mm/proc-v7-2level.S index bc86be205c04..fa385140715f 100644 --- a/arch/arm/mm/proc-v7-2level.S +++ b/arch/arm/mm/proc-v7-2level.S @@ -37,15 +37,18 @@ * It is assumed that: * - we are not using split page tables */ -ENTRY(cpu_v7_switch_mm) +ENTRY(cpu_ca8_switch_mm) #ifdef CONFIG_MMU mov r2, #0 - mmidr1, r1 @ get mm->context.id - ALT_SMP(orr r0, r0, #TTB_FLAGS_SMP) - ALT_UP(orr r0, r0, #TTB_FLAGS_UP) #ifdef CONFIG_ARM_ERRATA_430973 mcr p15, 0, r2, c7, c5, 6 @ flush BTAC/BTB #endif +#endif +ENTRY(cpu_v7_switch_mm) +#ifdef CONFIG_MMU + mmidr1, r1 @ get mm->context.id + ALT_SMP(orr r0, r0, #TTB_FLAGS_SMP) + ALT_UP(orr r0, r0, #TTB_FLAGS_UP) #ifdef CONFIG_PID_IN_CONTEXTIDR mrc p15, 0, r2, c13, c0, 1 @ read current context ID lsr r2, r2, #8 @ extract the PID @@ -61,6 +64,7 @@ ENTRY(cpu_v7_switch_mm) #endif bx lr ENDPROC(cpu_v7_switch_mm) +ENDPROC(cpu_ca8_switch_mm) /* * cpu_v7_set_pte_ext(ptep, pte) diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S index b1d19ea5e1af..6bec3cfbea39 100644 --- a/arch/arm/mm/proc-v7.S +++ b/arch/arm/mm/proc-v7.S @@ -153,6 +153,21 @@ ENDPROC(cpu_v7_do_resume) #endif /* + * Cortex-A8 + */ + globl_equ cpu_ca8_proc_init, cpu_v7_proc_init + globl_equ cpu_ca8_proc_fin, cpu_v7_proc_fin + globl_equ cpu_ca8_reset, cpu_v7_reset + globl_equ cpu_ca8_do_idle,cpu_v7_do_idle + globl_equ cpu_ca8_dcache_clean_area, cpu_v7_dcache_clean_area + globl_equ cpu_ca8_set_pte_ext,cpu_v7_set_pte_ext + globl_equ cpu_ca8_suspend_size, cpu_v7_suspend_size +#ifdef CONFIG_ARM_CPU_SUSPEND + globl_equ cpu_ca8_do_suspend, cpu_v7_do_suspend + globl_equ cpu_ca8_do_resume, cpu_v7_do_resume +#endif + +/* * Cortex-A9 processor functions */ globl_equ cpu_ca9mp_proc_init,cpu_v7_proc_init @@ -471,7 +486,10 @@ __v7_setup_stack: @ define struct processor (see and proc-macros.S) define_processor_functions v7, dabort=v7_early_abort, pabort=v7_pabort, suspend=1 +#ifndef CONFIG_ARM_LPAE + define_processor_functions ca8, dabort=v7_early_abort, pabort=v7_pabort, suspend=1 define_processor_functions ca9mp, dabort=v7_early_abort, pabort=v7_pabort, suspend=1 +#endif #ifdef CONFIG_CPU_PJ4B define_processor_functions pj4b, dabort=v7_early_abort, pabort=v7_pabort, suspend=1 #endif @@ -527,6 +545,16 @@ __v7_ca9mp_proc_info: __v7_proc __v7_ca9mp_proc_info, __v7_ca9mp_
Re: [PATCH 10/14] ARM: orion: use clkdev_create()
Hi Andrew, Russell, On 04/04/2015 02:17, Andrew Lunn wrote: > On Fri, Apr 03, 2015 at 06:13:13PM +0100, Russell King wrote: >> clkdev_create() is a shorter way to write clkdev_alloc() followed by >> clkdev_add(). Use this instead. >> >> Signed-off-by: Russell King > > Acked-by: Andrew Lunn This change makes sens however what about Thomas' comment: removing orion_clkdev_add() entirely and directly using lkdev_create() all over the place: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/327294.html Then what would be the path for this patch? As there is a dependency on the 6th patch of this series: "clkdev: add clkdev_create() helper" which should be merged through the clk tree, I think the best option is that this patch would be also managed by the clk tree maintainer (I added them in CC). Thanks, Gregory > > Andrew > >> --- >> arch/arm/plat-orion/common.c | 6 +- >> 1 file changed, 1 insertion(+), 5 deletions(-) >> >> diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c >> index f5b00f41c4f6..2235081a04ee 100644 >> --- a/arch/arm/plat-orion/common.c >> +++ b/arch/arm/plat-orion/common.c >> @@ -28,11 +28,7 @@ >> void __init orion_clkdev_add(const char *con_id, const char *dev_id, >> struct clk *clk) >> { >> -struct clk_lookup *cl; >> - >> -cl = clkdev_alloc(clk, con_id, dev_id); >> -if (cl) >> -clkdev_add(cl); >> +clkdev_create(clk, con_id, "%s", dev_id); >> } >> >> /* Create clkdev entries for all orion platforms except kirkwood. >> -- >> 1.8.3.1 >> >> >> ___ >> linux-arm-kernel mailing list >> linux-arm-ker...@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/14] media: omap3isp: remove unused clkdev
On Tue, Apr 07, 2015 at 12:42:52PM +0300, Laurent Pinchart wrote: > Hello Russell, > > On Sunday 05 April 2015 15:20:34 Russell King - ARM Linux wrote: > > On Sat, Apr 04, 2015 at 12:44:35AM +0300, Laurent Pinchart wrote: > > > Hi Russell, > > > > > > Thank you for the patch; > > > > > > On Friday 03 April 2015 18:12:58 Russell King wrote: > > > > No merged platform supplies xclks via platform data. As we want to > > > > slightly change the clkdev interface, rather than fixing this unused > > > > code, remove it instead. > > > > > > > > Signed-off-by: Russell King > > > > > > Acked-by: Laurent Pinchart > > > > > > with one caveat though : it conflicts with patches queued for v4.1 in the > > > media tree. I'll post a rebased version in a reply to your e-mail. How > > > would you like to handle the conflict ? > > > > How bad is the conflict? > > It's not too bad, it's mostly a context-related conflict. There are two > additional lines to remove (plus the associated comment) from > isp_xclk_init(), > as your patch makes a loop now terminate with if (condition) continue;. Those > two lines could be removed later, keeping them doesn't break anything. I think it's fine to take it through the media tree as the series doesn't have any dependencies on this patch. It was merely attempting to get rid of stuff so that we could move closer to clkdev dealing with a clk_hw rather than a struct clk - but I never made it that far with the series. Maybe at a later date... :) -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/14] clkdev: add clkdev_create() helper
On Mon, Apr 06, 2015 at 01:19:33PM -0700, Stephen Boyd wrote: > On 04/03/15 10:12, Russell King wrote: > > @@ -316,6 +329,29 @@ clkdev_alloc(struct clk *clk, const char *con_id, > > const char *dev_fmt, ...) > > } > > EXPORT_SYMBOL(clkdev_alloc); > > > > +/** > > + * clkdev_create - allocate and add a clkdev lookup structure > > + * @clk: struct clk to associate with all clk_lookups > > + * @con_id: connection ID string on device > > + * @dev_fmt: format string describing device name > > + * > > + * Returns a clk_lookup structure, which can be later unregistered and > > + * freed. > > Please add that this returns NULL on failure. Will do, but please remember that _I'm_ taking the clkdev patches through my tree, as I'm the maintainer for clkdev. Thanks. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/14] clkdev: get rid of redundant clk_add_alias() prototype in linux/clk.h
On Mon, Apr 06, 2015 at 12:04:21PM -0700, Stephen Boyd wrote: > On 04/04/15 05:43, Robert Jarzmik wrote: > > Russell King writes: > > > >> clk_add_alias() is provided by clkdev, and is not part of the clk API. > >> Howver, it is prototyped in two locations: linux/clkdev.h and > >> linux/clk.h. This is a mess. Get rid of the redundant and unnecessary > >> version in linux/clk.h. > >> > >> Signed-off-by: Russell King > > Tested-by: Robert Jarzmik > > > > Actually, this serie fixes a regression I've seen in linux-next, and which > > was > > triggering the Oops in [1] on lubbock. With your serie, the kernel boots > > fine. > > > > > > Is this with the lubbock_defconfig? Is it a regression in 4.0-rc series > or is it due to some pending -next patches interacting with the per-user > clock patches? It looks like the latter because __clk_get_hw() should be > inlined on lubbock_defconfig where CONFIG_COMMON_CLK=n. It's a regression with anything that uses clk_add_alias() or which open codes the aliasing of clocks, or registration of clocks into clkdev. It's quite nasty. The problem is that the way you updated clkdev, by adding __clk_get_hw() at clk_get() time, the struct clk which we're passing in there could well have been already clk_put()'d, and therefore freed, and no longer valid. It's a regression ever since the per-user clk patches went in, caused _entirely_ by those patches. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 03/10] ARM: DRA7: hwmod: set DSS submodule parent hwmods
On 03/04/15 22:03, Paul Walmsley wrote: > On Tue, 24 Mar 2015, Tomi Valkeinen wrote: > >> Set DSS core hwmod as the parent for all the DSS submodules. >> >> Signed-off-by: Tomi Valkeinen > > Thanks, queued. Will send a pull request for this to Tony today, but it > may not make it for v4.1. > > Note that I cannot boot test this since I do not have a DRA7xx board. As I mentioned in the reply to the pull request, we can't cherry pick this patch, we need the HDCP clock. This series should be changed so that the HDCP clock is handled in the beginning of the series to keep bisect working. I'll do that for the next version, which I probably would need to do in any case as Tero's series is still under work. Or if we want to get DRA7 DSS support in earlier, we could just use my earlier simple patch which enables the HDCP clock. Tomi signature.asc Description: OpenPGP digital signature
Re: [GIT PULL] ARM: OMAP2+: OMAP DRA7xx display-related hwmod data changes for v4.1
Hi, No, don't pull this, it doesn't work. Paul, the hwmod patch cannot be picked from the DRA7 DSS series. We have the problem with the DES HDCP clock, without that clock we will see WARNINGs when booting if the "set DSS submodule parent hwmods" is applied. We do see some errors when booting even with the current mainline, caused by not having that clock enabled: [0.194524] omap_hwmod: dss_core: _wait_target_disable failed [0.194540] omap_hwmod: dss_core: _wait_target_ready failed: -16 [0.194554] omap_hwmod: dss_core: cannot be enabled for reset (3) [0.197143] omap_hwmod: dss_dispc: _wait_target_ready failed: -16 [0.197156] omap_hwmod: dss_dispc: cannot be enabled for reset (3) [0.199745] omap_hwmod: dss_hdmi: _wait_target_ready failed: -16 [0.199759] omap_hwmod: dss_hdmi: cannot be enabled for reset (3) But that's still much less spam than the above plus ~4 WARNs. I'd rather merge the DRA7 DSS patches in one go, to avoid issues related to the HDPC clock. Tomi On 04/04/15 02:08, Tony Lindgren wrote: > Hi Arnd & Olof, > > Please feel free to pull this directly from Paul. > > Regards, > > Tony > > * Paul Walmsley [150403 12:55]: >> Hi Tony >> >> The following changes since commit c517d838eb7d07bbe9507871fab3931deccff539: >> >> Linux 4.0-rc1 (2015-02-22 18:21:14 -0800) >> >> are available in the git repository at: >> >> git://git.kernel.org/pub/scm/linux/kernel/git/pjw/omap-pending.git >> tags/for-v4.1/omap-hwmod-b >> >> for you to fetch changes up to 0d75ebdd3230b3d085812df80ec54138828cc76a: >> >> ARM: DRA7: hwmod: set DSS submodule parent hwmods (2015-04-03 13:15:07 >> -0600) >> >> >> OMAP DRA7xx display-related hwmod data changes for v4.1 >> >> Add DMM hwmod data for DRA7xx, and set the parent hwmods fields >> appropriately for the DRA7xx DSS submodules as we've done for >> other OMAP devices. >> >> Note that I do not have a DRA7xx board, and thus cannot boot-test >> these patches. >> >> Basic build, boot, and PM idle test logs are available at: >> >> http://www.pwsan.com/omap/testlogs/omap-hwmod-b-for-v4.1/20150403123702/ >> >> >> Tomi Valkeinen (2): >> ARM: DRA7: hwmod: add DMM hwmod description >> ARM: DRA7: hwmod: set DSS submodule parent hwmods >> >> arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 32 >> +++ >> 1 file changed, 32 insertions(+) >> >> >> >> vmlinux object size >> (delta in bytes from test_v4.0-rc1 >> (c517d838eb7d07bbe9507871fab3931deccff539)): >>text data bsstotal kernel >> 0000 omap1_defconfig >> 0000 omap1_defconfig_1510innovator_only >> 0000 omap1_defconfig_5912osk_only >> 0 +1920 +192 multi_v7_defconfig >> 0 +2160 +216 omap2plus_defconfig >> 0000 omap2plus_defconfig_2430sdp_only >> 0000 omap2plus_defconfig_am33xx_only >> 0000 omap2plus_defconfig_am43xx_only >> 0 +2240 +224 omap2plus_defconfig_cpupm >> 0 +1920 +192 omap2plus_defconfig_dra7xx_only >> 0000 omap2plus_defconfig_n800_multi_omap2xxx >> 0000 omap2plus_defconfig_n800_only_a >> 0 +1920 +192 omap2plus_defconfig_no_pm >> 0000 omap2plus_defconfig_omap2_4_only >> 0000 omap2plus_defconfig_omap3_4_only >> 0000 omap2plus_defconfig_omap5_only >> 0000 rmk_omap3430_ldp_allnoconfig >> 0000 rmk_omap3430_ldp_oldconfig >> +80 -80 rmk_omap4430_sdp_allnoconfig >> 0000 rmk_omap4430_sdp_oldconfig >> >> Boot-time memory difference >> (delta in bytes from test_v4.0-rc1 >> (c517d838eb7d07bbe9507871fab3931deccff539)) >> avail rsrvd high freed board kconfig >> -4k 4k . . 4460pandaesomap2plus_defconfig >>-12k12k . . 4460varsomom omap2plus_defconfig >> -4k 4k . . 5430es2sbct54 omap2plus_defconfig >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > signature.asc Description: OpenPGP digital signature
Re: [PATCH 03/14] clkdev: get rid of redundant clk_add_alias() prototype in linux/clk.h
Stephen Boyd writes: > On 04/04/15 05:43, Robert Jarzmik wrote: >> Russell King writes: >> >>> clk_add_alias() is provided by clkdev, and is not part of the clk API. >>> Howver, it is prototyped in two locations: linux/clkdev.h and >>> linux/clk.h. This is a mess. Get rid of the redundant and unnecessary >>> version in linux/clk.h. >>> >>> Signed-off-by: Russell King >> Tested-by: Robert Jarzmik >> >> Actually, this serie fixes a regression I've seen in linux-next, and which >> was >> triggering the Oops in [1] on lubbock. With your serie, the kernel boots >> fine. >> >> > > Is this with the lubbock_defconfig? Is it a regression in 4.0-rc series > or is it due to some pending -next patches interacting with the per-user > clock patches? It looks like the latter because __clk_get_hw() should be > inlined on lubbock_defconfig where CONFIG_COMMON_CLK=n. It's upon linux-next and the "not yet pulled" pxa tree merged : - http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/331119.html I think it will appear once arm-soc has been prepared for linux-next and pulled. This specific merge window will shift pxa architecture to common clock framework, that's why I saw the regression. Cheers. -- Robert -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/14] media: omap3isp: remove unused clkdev
Hello Russell, On Sunday 05 April 2015 15:20:34 Russell King - ARM Linux wrote: > On Sat, Apr 04, 2015 at 12:44:35AM +0300, Laurent Pinchart wrote: > > Hi Russell, > > > > Thank you for the patch; > > > > On Friday 03 April 2015 18:12:58 Russell King wrote: > > > No merged platform supplies xclks via platform data. As we want to > > > slightly change the clkdev interface, rather than fixing this unused > > > code, remove it instead. > > > > > > Signed-off-by: Russell King > > > > Acked-by: Laurent Pinchart > > > > with one caveat though : it conflicts with patches queued for v4.1 in the > > media tree. I'll post a rebased version in a reply to your e-mail. How > > would you like to handle the conflict ? > > How bad is the conflict? It's not too bad, it's mostly a context-related conflict. There are two additional lines to remove (plus the associated comment) from isp_xclk_init(), as your patch makes a loop now terminate with if (condition) continue;. Those two lines could be removed later, keeping them doesn't break anything. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [rtc-linux] [PATCH] rtc: OMAP: Add external 32k clock feature
Hi, On 04/07/15 06:29, Keerthy wrote: > Hi Andrew, > > Apologies for replying late. > > On Wednesday 25 March 2015 04:29 AM, Andrew Morton wrote: >> On Tue, 3 Mar 2015 15:12:02 +0530 Keerthy wrote: >> >>> Add external 32k clock feature. The internal clock will be gated during >>> suspend. >>> Hence make use of the external 32k clock so that rtc is functional accross >>> suspend/resume. >>> >>> ... >>> >>> @@ -446,6 +449,7 @@ static const struct omap_rtc_device_type >>> omap_rtc_default_type = { >>> >>> static const struct omap_rtc_device_type omap_rtc_am3352_type = { >>> .has_32kclk_en= true, >>> +.has_osc_ext_32k = true, >>> .has_kicker= true, >>> .has_irqwakeen= true, >>> .has_pmic_mode= true, >>> @@ -543,7 +547,16 @@ static int __init omap_rtc_probe(struct >>> platform_device *pdev) >>> if (rtc->type->has_32kclk_en) { >>> reg = rtc_read(rtc, OMAP_RTC_OSC_REG); >>> rtc_writel(rtc, OMAP_RTC_OSC_REG, >>> -reg | OMAP_RTC_OSC_32KCLK_EN); >>> + reg | OMAP_RTC_OSC_32KCLK_EN); >>> +} >>> + >>> +/* Enable External clock as the source */ >>> + >>> +if (rtc->type->has_osc_ext_32k) { >>> +rtc_writel(rtc, OMAP_RTC_OSC_REG, >>> + (OMAP_RTC_OSC_EXT_32K | >>> + rtc_read(rtc, OMAP_RTC_OSC_REG)) & >>> + (~OMAP_RTC_OSC_OSC32K_GZ)); >>> } >> >> How do we know that all systems have this external clock and that it >> works OK? >> > > AM335 and AM43X have the external clock feature which we choose using > RTC_OSC_REG. I verified it works OK by seeing the RTC seconds ticking > even after switching the source to the external 32k Clock. AFAIU, this is related to the external (outside of SoC) oscillator, right? If so, there is a possibility to not assemble it on the board (at least on AM335) and use the internal clock source instead. There are dozens of boards out there that do not have the external oscillator assembled. Am I missing something? -- Regards, Igor. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html