[PATCH 0/2] allow cpufreq drivers to export flags
This series is partially in response to a discussion around DT bindings for CPUfreq drivers [0], but it is also needed for on-going work to integrate CPUfreq with the scheduler. In particular a scheduler-driven cpu frequency scaling policy would be well served to know if the underlying CPUfreq driver .target callback might sleep or block for a long time. [0] http://lkml.kernel.org/r/ Mike Turquette (2): cpufreq: add driver flag for sleepable transitions cpufreq: new function to query driver for flags drivers/cpufreq/cpufreq.c | 9 + include/linux/cpufreq.h | 13 + 2 files changed, 22 insertions(+) -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] cpufreq: add driver flag for sleepable transitions
The CPUfreq core does not differentiate between .target & .target_index callbacks that may sleep or block and callbacks that are fast and return immediately. To date this has not mattered much since the typical CPUfreq governor calls the .target callback from process context via a workqueue. When calling the .target callback from a different context, such as a scheduler load balance operation (see Morten's "energy aware scheduler" RFC[0]), this detail matters. This patch introduces a new CPUfreq driver flag for fast .target callbacks that are guaranteed to neither sleep nor block. Setting this flag allows for .target to be called from schedule() and other call sites that have interrupts disabled or other constraints. Drivers may set CPUFREQ_NO_SLEEP at driver registration-time. The default is to not have this flag set, resulting in the need to defer calls to .target and .target_index. [0] http://lkml.kernel.org/r/<1381511957-29776-7-git-send-email-morten.rasmus...@arm.com> Signed-off-by: Mike Turquette --- include/linux/cpufreq.h | 13 + 1 file changed, 13 insertions(+) diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index 138336b..9034573 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -308,6 +308,19 @@ struct cpufreq_driver { */ #define CPUFREQ_NEED_INITIAL_FREQ_CHECK(1 << 5) +/* + * Set by drivers whose .target or .target_index callback will never sleep or + * block. Setting this flag allows for more optimal code by calling .target + * from a context that otherwise would require deferring the work. + * + * An example use of this flag is when scaling cpu frequency from within a call + * to schedule() with interrupts disabled and runqueue locks held. + * + * This flag is not set by default, causing calls to .target to be done in + * process context. + */ +#define CPUFREQ_NO_SLEEP (1 << 6) + int cpufreq_register_driver(struct cpufreq_driver *driver_data); int cpufreq_unregister_driver(struct cpufreq_driver *driver_data); -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] cpufreq: new function to query driver for flags
There are cases for CPUfreq driver flags to be exposed outside of the CPUfreq core code. In particular the CPUFREQ_NO_SLEEP flag can be used by CPUfreq governors to optimize when and how they call that drivers .target callback. In fact this knowledge is a requirement for the on-going work to initiate cpu frequency transitions from the scheduler. This patch implements a simple function to return the CPUfreq driver flags. While currently the flags are a u8, that could grow at a future date and cpufreq_driver_get_flags returns an int. Additionally the function needs to return an error in case no driver is registered. Signed-off-by: Mike Turquette --- drivers/cpufreq/cpufreq.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 9b471b2..f3b9042 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1980,6 +1980,15 @@ int cpufreq_driver_target(struct cpufreq_policy *policy, } EXPORT_SYMBOL_GPL(cpufreq_driver_target); +int cpufreq_driver_get_flags(void) +{ + if (!cpufreq_driver) + return -ENODEV; + + return cpufreq_driver->flags; +} +EXPORT_SYMBOL_GPL(cpufreq_driver_get_flags); + /* * when "event" is CPUFREQ_GOV_LIMITS */ -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] allow cpufreq drivers to export flags
On 8 October 2014 13:18, Mike Turquette wrote: > This series is partially in response to a discussion around DT bindings > for CPUfreq drivers [0], but it is also needed for on-going work to > integrate CPUfreq with the scheduler. In particular a scheduler-driven > cpu frequency scaling policy would be well served to know if the > underlying CPUfreq driver .target callback might sleep or block for a > long time. > > [0] > http://lkml.kernel.org/r/ Firstly this link is broken and then the last comment I gave in Thomas's thread was that he doesn't need this routine anymore. And so your scheduler work is the only user of it.. And so probably we should get this included with the scheduler patches only, isn't it? Just including them without any user wouldn't benefit.. Or am I missing something ? -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] allow cpufreq drivers to export flags
Dear Viresh Kumar, On Wed, 8 Oct 2014 13:24:30 +0530, Viresh Kumar wrote: > On 8 October 2014 13:18, Mike Turquette wrote: > > This series is partially in response to a discussion around DT bindings > > for CPUfreq drivers [0], but it is also needed for on-going work to > > integrate CPUfreq with the scheduler. In particular a scheduler-driven > > cpu frequency scaling policy would be well served to know if the > > underlying CPUfreq driver .target callback might sleep or block for a > > long time. > > > > [0] > > http://lkml.kernel.org/r/ > > Firstly this link is broken and then the last comment I gave in Thomas's > thread was that he doesn't need this routine anymore. And so your scheduler > work is the only user of it.. And so probably we should get this included with > the scheduler patches only, isn't it? Just including them without any user > wouldn't benefit.. > > Or am I missing something ? Well, when one has to merge a large number of changes, we often recommend to merge them piece by piece, which is what Mike is trying to do here. So we cannot at the same time ask developers to merge things in small pieces that are easy to review and to merge everything together because the users of a given API are not there yet. Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] allow cpufreq drivers to export flags
On 8 October 2014 13:41, Thomas Petazzoni wrote: > On Wed, 8 Oct 2014 13:24:30 +0530, Viresh Kumar wrote: >> On 8 October 2014 13:18, Mike Turquette wrote: >> > This series is partially in response to a discussion around DT bindings >> > for CPUfreq drivers [0], but it is also needed for on-going work to >> > integrate CPUfreq with the scheduler. In particular a scheduler-driven > Well, when one has to merge a large number of changes, we often > recommend to merge them piece by piece, which is what Mike is trying to > do here. So we cannot at the same time ask developers to merge things > in small pieces that are easy to review and to merge everything > together because the users of a given API are not there yet. >From the wording of Mike it looks like: - This is required by cpufreq drivers - today - And this will also be useful for scheduler The first point doesn't stand true anymore. Lets wait for Mike's reply and see his opinion. And then the patches are so small and there are no objections against them. I don't think getting them with the scheduler changes is that bad of an idea. In worst case, what if he has to redesign his idea? The new routines will stay without a caller then :) -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 6/7] mfd: qcom-smd-rpm: Driver for the Qualcomm RPM over SMD
On Mon, 29 Sep 2014, Bjorn Andersson wrote: > Driver for the Resource Power Manager (RPM) found in Qualcomm 8974 based > devices. > The driver exposes resources that child drivers can operate on; to > implementing regulator, clock and bus frequency drivers. > > Signed-off-by: Bjorn Andersson > --- > > Note that the qcom_rpm_smd_write is equivalent of qcom_rpm_write and there is > a > possibility of re-using at least the clock implementation on top of this. This > would however require some logic for calling the right implementation so I > have > not done it at this time to keep things as clean as possible. > > An idea for improvement is that in qcom_rpm_smd_write we put the ack_status > and > completion on the stack and register this with idr using the message id, upon > receiving the interrupt we would find the right client and complete this. > Allowing for multiple requests to be in flight at any given time. > > I did not implement this because I haven't done any measurements on what kind > of improvements this could give and it would be a clean iteration ontop of > this. > > drivers/mfd/Kconfig | 14 ++ > drivers/mfd/Makefile |1 + > drivers/mfd/qcom-smd-rpm.c | 299 > ++ > include/linux/mfd/qcom-smd-rpm.h |9 ++ > 4 files changed, 323 insertions(+) > create mode 100644 drivers/mfd/qcom-smd-rpm.c > create mode 100644 include/linux/mfd/qcom-smd-rpm.h > +#define RPM_ERR_INVALID_RESOURCE "resource does not exist" > + > +static bool qcom_rpm_msg_is_invalid_resource(struct qcom_rpm_message *msg) > +{ > + size_t msg_len = sizeof(RPM_ERR_INVALID_RESOURCE) - 1; > + > + if (msg->length != msg_len) > + return false; > + > + if (memcmp(msg->message, RPM_ERR_INVALID_RESOURCE, msg_len)) > + return false; > + > + return true; > +} You can save yourself a hell of a lot of code by just doing: if (memcmp(msg->message, RPM_ERR_INVALID_RESOURCE, min(msg_len, sizeof(RPM_ERR_INVALID_RESOURCE ... in qcom_smd_rpm_callback(). [...] > +static int qcom_smd_rpm_probe(struct qcom_smd_device *sdev) > +{ > + const struct of_device_id *match; > + struct qcom_smd_rpm *rpm; > + > + rpm = devm_kzalloc(&sdev->dev, sizeof(*rpm), GFP_KERNEL); > + if (!rpm) > + return -ENOMEM; > + > + rpm->dev = &sdev->dev; > + mutex_init(&rpm->lock); > + init_completion(&rpm->ack); > + > + match = of_match_device(qcom_smd_rpm_of_match, &sdev->dev); You need to check the return value here. > + rpm->data = match->data; > + rpm->rpm_channel = sdev->channel; > + > + dev_set_drvdata(&sdev->dev, rpm); > + > + dev_info(&sdev->dev, "Qualcomm SMD RPM driver probed\n"); Please remove this line. > + return of_platform_populate(sdev->dev.of_node, NULL, NULL, &sdev->dev); > +} > + > +static void qcom_smd_rpm_remove(struct qcom_smd_device *sdev) > +{ > + dev_set_drvdata(&sdev->dev, NULL); If you use the proper platform device interface you don't have to do this. > + of_platform_depopulate(&sdev->dev); > +} > + > +static struct qcom_smd_driver qcom_smd_rpm_driver = { > + .probe = qcom_smd_rpm_probe, > + .remove = qcom_smd_rpm_remove, > + .callback = qcom_smd_rpm_callback, > + .driver = { > + .name = "qcom_smd_rpm", > + .owner = THIS_MODULE, > + .of_match_table = qcom_smd_rpm_of_match, > + }, > +}; > + > +module_qcom_smd_driver(qcom_smd_rpm_driver); I don't like this. What's wrong with the existing platform driver code? -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] Input: pmic8xxx-keypad - use regmap_field for register access
On Tue, 2014-10-07 at 10:26 -0700, Dmitry Torokhov wrote: > Hi Ivan, > > On Tue, Oct 07, 2014 at 12:50:46PM +0300, Ivan T. Ivanov wrote: > > @@ -527,10 +538,55 @@ static int pmic8xxx_kp_probe(struct platform_device > > *pdev) > > > > + > > + kp->row_hold = devm_regmap_field_alloc(kp->dev, kp->regmap, > > + info->row_hold); > > + if (IS_ERR(kp->row_hold)) > > + return PTR_ERR(kp->row_hold); > > Why do we have to allocate all regmap fields separately instead of > embedding them into keypad structure? > No particular reason. Will rework it. Thank you, Ivan > Thanks. > -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] Input: pmic8xxx-keypad - use regmap_field for register access
On Wed, 2014-10-08 at 12:13 +0300, Ivan T. Ivanov wrote: > On Tue, 2014-10-07 at 10:26 -0700, Dmitry Torokhov wrote: > > Hi Ivan, > > > > On Tue, Oct 07, 2014 at 12:50:46PM +0300, Ivan T. Ivanov wrote: > > > @@ -527,10 +538,55 @@ static int pmic8xxx_kp_probe(struct platform_device > > > *pdev) > > > > > > > + > > > + kp->row_hold = devm_regmap_field_alloc(kp->dev, kp->regmap, > > > +info->row_hold); > > > + if (IS_ERR(kp->row_hold)) > > > + return PTR_ERR(kp->row_hold); > > > > Why do we have to allocate all regmap fields separately instead of > > embedding them into keypad structure? > > > > No particular reason. Will rework it. > Oops. struct regmap_field is opaque. It seems that the allocation is the only way that I could have instance of it. Regards, Ivan -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] Qualcomm PM8941 power key driver
On Tue, 2014-10-07 at 11:46 -0700, Bjorn Andersson wrote: > On Tue 07 Oct 02:01 PDT 2014, Ivan T. Ivanov wrote: > > > > > Hi Bjorn, > > > > On Mon, 2014-10-06 at 18:11 -0700, Bjorn Andersson wrote: > > > These patches add dt bindings and a device driver for the power key block > > > in > > > the Qualcomm PM8941 pmic. > > > > > > Courtney Cavin (2): > > > input: Add Qualcomm PM8941 power key driver > > > input: pm8941-pwrkey: Add DT binding documentation > > > > > > .../bindings/input/qcom,pm8941-pwrkey.txt | 43 + > > > drivers/input/misc/Kconfig | 12 ++ > > > drivers/input/misc/Makefile|1 + > > > drivers/input/misc/pm8941-pwrkey.c | 196 > > > > > > 4 files changed, 252 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt > > > create mode 100644 drivers/input/misc/pm8941-pwrkey.c > > > > Any reason why we cannot reuse pm8xxx-pwrkey driver? It have been > > converted to regmap already. > > > > The boilerplate code is the same, The boilerplate code is almost 100% :-) > but configuration registers have different > layout and values written in them are different. We talk about 3 registers and 2 bit defines. struct regmap_field should be able to help here. > The pm8xxx block have separate > interrupts for press and release events while pm8941 have one interrupt for > both, so the pm8941 must read out the irq status bits to figure out which > event > it was. Optional interrupt property? If both are defined hook old ISR, if its only one hook pm8941 ISR? > > Maybe if we introduce some vagueness related to interrupts in the dt binding > documentation for pm8xxx we could simply reuse that binding. > I would not say vagueness, we just can say that pm8941 did not have second interrupt? Regards, Ivan > Regards, > Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH resend] arm:extend the reserved memory for initrd to be page aligned
On Fri, Sep 19, 2014 at 9:09 AM, Wang, Yalin wrote: > this patch extend the start and end address of initrd to be page aligned, > so that we can free all memory including the un-page aligned head or tail > page of initrd, if the start or end address of initrd are not page > aligned, the page can't be freed by free_initrd_mem() function. > > Signed-off-by: Yalin Wang > --- > arch/arm/mm/init.c | 5 + > arch/arm64/mm/init.c | 8 +++- > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c > index 659c75d..9221645 100644 > --- a/arch/arm/mm/init.c > +++ b/arch/arm/mm/init.c > @@ -636,6 +636,11 @@ static int keep_initrd; > void free_initrd_mem(unsigned long start, unsigned long end) > { > if (!keep_initrd) { > + if (start == initrd_start) > + start = round_down(start, PAGE_SIZE); > + if (end == initrd_end) > + end = round_up(end, PAGE_SIZE); > + > poison_init_mem((void *)start, PAGE_ALIGN(end) - start); > free_reserved_area((void *)start, (void *)end, -1, "initrd"); > } Who guarantees there's no valuable data in [start, initrd_start) and [initrd_end, end) being corrupted? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] ARM: vfp: fix VFPv3 hwcap detection on non-ARM vfp implementations
Hi all, On Wed, Oct 01, 2014 at 06:54:18PM +0100, Stephen Boyd wrote: > On 09/19/14 11:24, Stephen Boyd wrote: > > On 09/18/14 15:46, Russell King - ARM Linux wrote: > >> I know that you're changing this to conform with the ARM ARM, but we > >> have to consider that before VFP was subsumed into the ARM ARM, this > >> register had the format described as per this file, and these other > >> bits may be set for an ARM part. Including these bits in the mask > >> means that we will mis-identify these older parts as VFPv3. > >> > > Do you, or anyone else, know of other implementations? I *hope* that > > this same exercise was done by the VFP architects before they > > re-purposed bits but who knows. If nobody is actually setting these > > higher bits then is there any problem widening the mask (besides it > > being slightly confusing)? > > > > Any thoughts? I've spoken to the architects about this, and it seems that you need to parse FPSID differently depending upon whether you are a v7 CPU or not. However, as you are well aware, detecting whether or not you are v7 isn't as simple as it sounds. cpu_architecture() checks the VMSA/PMSA fields in MMFR0, but that's not quite right for 11MPcore (and 1176 iirc), which are v6 implementations. So, it sounds like the scheme is: if (revised_cpuid_format()) // i.e. MIDR.Architecture == 0xF determine_vfp_arch_using_mvfr_regs(); else determine_vfp_arch_using_fpsid(); which in turn means that we probably need to rethink how we want to advertise floating point hardware features in the future. ARMv8, for example, adds an additional MVFR2 register, which advertises additional floating point instructions without explicitly creating something akin to VFPv5. Will -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" 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 04/15] clk: divider: Make generic for usage elsewhere
On 07/10/14 19:26, Stephen Boyd wrote: On 10/07/2014 10:27 AM, Srinivas Kandagatla wrote: Hi Stephen, Just noticed this regression while testing the patch on Arndale board. https://bugs.linaro.org/show_bug.cgi?id=740 If you return value it works correctly right? I think you are right, .. From last message from bugs.linaro.org: " That seems to be it! This change: --- a/drivers/clk/clk-divider.c +++ b/drivers/clk/clk-divider.c @@ -366,7 +366,7 @@ int divider_get_val(unsigned long rate, unsigned long parent min_t(unsigned int, value, div_mask(width)); - return 0; + return value; } EXPORT_SYMBOL_GPL(divider_get_val); - makes Arndale to boot; rootfs on mmc mounts OK. " thanks, srini -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] ARM: qcom: add description of KPSS WDT for IPQ8064
On Tue, Oct 07, 2014 at 04:07:43PM -0700, Stephen Boyd wrote: > On 10/07/2014 03:10 PM, Josh Cartwright wrote: > >On Thu, Oct 02, 2014 at 12:08:38PM -0700, Stephen Boyd wrote: [..] > >>I'm thinking: > >> > >> timer@200a000 { > >> compatible = "qcom,kpss-timer", "qcom,msm-timer"; > >> interrupts = <1 1 0x301>, > >> <1 2 0x301>, > >> <1 3 0x301>, > >> <1 4 0x301>, > >> <1 5 0x301>; > >> reg = <0x0200a000 0x100>; > >> clock-frequency = <2700>, > >> <32768>; > >> clocks = <&cxo>, <&sleep_clk>; > >> clock-names = "ref", "sleep"; > >> cpu-offset = <0x8>; > >> }; > >Where'd the default timeout configuration go? Or, should we have one > >timeout-sec property and not allow setting the default timeouts per WDT > >instance? Or no configurable timeout at all? > > Ah sorry. How about a timeout-sec-0, timeout-sec-1 property that is per-cpu > and maps to the first and second watchdog timer? Something like: > > timeout-sec-wdt0 = <10 8>; > timeout-sec-wdt1 = <20 15>; Okay, yeah, this is much more concise. I'll take a stab at implementing it. Thanks, Josh -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] Input: pmic8xxx-keypad - use regmap_field for register access
On 10/08/2014 02:30 AM, Ivan T. Ivanov wrote: On Wed, 2014-10-08 at 12:13 +0300, Ivan T. Ivanov wrote: On Tue, 2014-10-07 at 10:26 -0700, Dmitry Torokhov wrote: Hi Ivan, On Tue, Oct 07, 2014 at 12:50:46PM +0300, Ivan T. Ivanov wrote: @@ -527,10 +538,55 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev) + + kp->row_hold = devm_regmap_field_alloc(kp->dev, kp->regmap, + info->row_hold); + if (IS_ERR(kp->row_hold)) + return PTR_ERR(kp->row_hold); Why do we have to allocate all regmap fields separately instead of embedding them into keypad structure? No particular reason. Will rework it. Oops. struct regmap_field is opaque. It seems that the allocation is the only way that I could have instance of it. Maybe we can add an API to allocate an array of fields? -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] Input: pmic8xxx-keypad - use regmap_field for register access
On Wed, Oct 08, 2014 at 11:00:07AM -0700, Stephen Boyd wrote: > On 10/08/2014 02:30 AM, Ivan T. Ivanov wrote: > >On Wed, 2014-10-08 at 12:13 +0300, Ivan T. Ivanov wrote: > >>On Tue, 2014-10-07 at 10:26 -0700, Dmitry Torokhov wrote: > >>>Hi Ivan, > >>> > >>>On Tue, Oct 07, 2014 at 12:50:46PM +0300, Ivan T. Ivanov wrote: > @@ -527,10 +538,55 @@ static int pmic8xxx_kp_probe(struct platform_device > *pdev) > + > + kp->row_hold = devm_regmap_field_alloc(kp->dev, kp->regmap, > +info->row_hold); > + if (IS_ERR(kp->row_hold)) > + return PTR_ERR(kp->row_hold); > >>>Why do we have to allocate all regmap fields separately instead of > >>>embedding them into keypad structure? > >>> > >>No particular reason. Will rework it. > >> > >Oops. struct regmap_field is opaque. It seems that the allocation > >is the only way that I could have instance of it. > > > > Maybe we can add an API to allocate an array of fields? Maybe we could make the structure public instead? I do not see any reason for allocating something separately that has exactly the same lifetime as owning structure. Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] Input: pmic8xxx-keypad - use regmap_field for register access
Adding Mark and Srini On 10/08/2014 11:13 AM, Dmitry Torokhov wrote: On Wed, Oct 08, 2014 at 11:00:07AM -0700, Stephen Boyd wrote: On 10/08/2014 02:30 AM, Ivan T. Ivanov wrote: On Wed, 2014-10-08 at 12:13 +0300, Ivan T. Ivanov wrote: On Tue, 2014-10-07 at 10:26 -0700, Dmitry Torokhov wrote: Hi Ivan, On Tue, Oct 07, 2014 at 12:50:46PM +0300, Ivan T. Ivanov wrote: @@ -527,10 +538,55 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev) + + kp->row_hold = devm_regmap_field_alloc(kp->dev, kp->regmap, + info->row_hold); + if (IS_ERR(kp->row_hold)) + return PTR_ERR(kp->row_hold); Why do we have to allocate all regmap fields separately instead of embedding them into keypad structure? No particular reason. Will rework it. Oops. struct regmap_field is opaque. It seems that the allocation is the only way that I could have instance of it. Maybe we can add an API to allocate an array of fields? Maybe we could make the structure public instead? I do not see any reason for allocating something separately that has exactly the same lifetime as owning structure. Srini/Mark, any reason why the regmap_field structure is opaque? -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" 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 3/3] ARM: dts: qcom: Add SDHC nodes for APQ8084 platform
Georgi, On Tue, Sep 2, 2014 at 5:40 PM, Georgi Djakov wrote: > Enable support for the two SD host controllers on the APQ8084 platform > by adding the required nodes to the DT files. > On the IFC6540 board, the first controller is connected to the onboard > eMMC and the second is connected to a micro-SD card slot. testing this set of patch on IFC6540, i noticed that not all partitions from the eMMC are detected. booting [1] which is basically 3.17-rc7 with a few additional patches, i can see the following relevant mmc traces: [1.162587] mmc0: BKOPS_EN bit is not set [1.315357] mmc0: new HS200 MMC card at address 0001 [1.318098] mmcblk0: mmc0:0001 SEM16G 14.6 GiB [1.330377] mmcblk0boot0: mmc0:0001 SEM16G partition 1 4.00 MiB [1.333652] mmcblk0boot1: mmc0:0001 SEM16G partition 2 4.00 MiB [1.345424] mmcblk0rpmb: mmc0:0001 SEM16G partition 3 4.00 MiB [1.358135] mmcblk0: p1 p2 p3 p4 p5 p6 p7 p8 p9 p10 p11 p12 p13 p14 p15 [1.365902] mmcblk0boot1: p1 [1.367833] mmcblk0boot0: unknown partition table on the same board, booting the board vendor kernel (which is based on QCOM 3.10 tree), i can see much more partitions: [7.662379] mmc0: BKOPS_EN bit = 0 [7.673036] mmc0: new HS200 MMC card at address 0001 [7.673784] mmcblk0: mmc0:0001 SEM16G 14.6 GiB [7.674311] mmcblk0rpmb: mmc0:0001 SEM16G partition 3 4.00 MiB [7.677967] mmcblk0: p1 p2 p3 p4 p5 p6 p7 p8 p9 p10 p11 p12 p13 p14 p15 p16 p17 p18 p19 p20 p21 p22 p23 p24\ p25 the complete dmesg log for can be found in our board lab, at [2] cheers [1] https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/commit/b538bdefecd11bd0278cc31164e1078c1722e5a8 [2] http://armv7.com/scheduler/job/11726/log_file#L_11_11 -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" 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 3/3] ARM: dts: qcom: Add SDHC nodes for APQ8084 platform
On Wed, Oct 08, 2014 at 09:18:44PM +0200, Nicolas Dechesne wrote: > Georgi, > > On Tue, Sep 2, 2014 at 5:40 PM, Georgi Djakov wrote: > > Enable support for the two SD host controllers on the APQ8084 platform > > by adding the required nodes to the DT files. > > On the IFC6540 board, the first controller is connected to the onboard > > eMMC and the second is connected to a micro-SD card slot. > > testing this set of patch on IFC6540, i noticed that not all > partitions from the eMMC are detected. booting [1] which is basically > 3.17-rc7 with a few additional patches, i can see the following > relevant mmc traces: > > [1.162587] mmc0: BKOPS_EN bit is not set > [1.315357] mmc0: new HS200 MMC card at address 0001 > [1.318098] mmcblk0: mmc0:0001 SEM16G 14.6 GiB > [1.330377] mmcblk0boot0: mmc0:0001 SEM16G partition 1 4.00 MiB > [1.333652] mmcblk0boot1: mmc0:0001 SEM16G partition 2 4.00 MiB > [1.345424] mmcblk0rpmb: mmc0:0001 SEM16G partition 3 4.00 MiB > [1.358135] mmcblk0: p1 p2 p3 p4 p5 p6 p7 p8 p9 p10 p11 p12 p13 p14 p15 > [1.365902] mmcblk0boot1: p1 > [1.367833] mmcblk0boot0: unknown partition table This has bitten me more times than I care to admit, but have you checked that you've set CONFIG_MMC_BLOCK_MINORS higher than 16? Perhaps we should be doing that in qcom_defconfig if we aren't already. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] Input: pmic8xxx-keypad - use regmap_field for register access
On Wed, Oct 08, 2014 at 11:20:58AM -0700, Stephen Boyd wrote: > On 10/08/2014 11:13 AM, Dmitry Torokhov wrote: > >>>Oops. struct regmap_field is opaque. It seems that the allocation > >>>is the only way that I could have instance of it. > >>Maybe we can add an API to allocate an array of fields? > >Maybe we could make the structure public instead? I do not see any > >reason for allocating something separately that has exactly the same > >lifetime as owning structure. The lifetime may be different to that of the register map it references, consider MFD function devices for example. > Srini/Mark, any reason why the regmap_field structure is opaque? So you can't peer into it and rely on the contents. I can see it being useful to add a bulk allocator. signature.asc Description: Digital signature
Re: [PATCH 2/4] Input: pmic8xxx-keypad - use regmap_field for register access
On Wed, Oct 08, 2014 at 09:04:26PM +0100, Mark Brown wrote: > On Wed, Oct 08, 2014 at 11:20:58AM -0700, Stephen Boyd wrote: > > On 10/08/2014 11:13 AM, Dmitry Torokhov wrote: > > > >>>Oops. struct regmap_field is opaque. It seems that the allocation > > >>>is the only way that I could have instance of it. > > > >>Maybe we can add an API to allocate an array of fields? > > > >Maybe we could make the structure public instead? I do not see any > > >reason for allocating something separately that has exactly the same > > >lifetime as owning structure. > > The lifetime may be different to that of the register map it references, > consider MFD function devices for example. Exactly, it is different than lifetime of the register map, but the same as device structure of the driver instantiating it. And so instead of doing 10+ separate allocations one could do just one. > > > Srini/Mark, any reason why the regmap_field structure is opaque? > > So you can't peer into it and rely on the contents. I can see it being > useful to add a bulk allocator. And then one have to define offsets in an array and use awkward syntax to access individual fields. Can we just reply on reviews/documentation for users to not do wrong thing? Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 4/7] soc: qcom: Add Shared Memory Manager driver
Here in my initial detailed pass. I still have some "issues" that I want to clarify on my end, but I think I have plenty of comments to start with. On 9/29/2014 6:34 PM, Bjorn Andersson wrote: The Shared Memory Manager driver implements an interface for allocating and accessing items in the memory area shared among all of the processors in a Qualcomm platform. Signed-off-by: Bjorn Andersson --- In later platforms this is extended to support "secure smem", I do unfortunately not have any devices that I could test this with so I have only implemented the "insecure" version for now. I was thinking about implementing an of_xlate as this would make sense for many things. It is however not feasible for SMD, that would require us to list 257 items. It would make sense to enhance this with a method of keeping track of currently consumed items, both to take care of "mutual exclusion" between consumers as well as knowing when it's safe to remove the device and/or unload the driver. By "mutual exclusion" I assume you mean that two different entities are using the same smem id. Such a usecase is outside the purview of this driver, and is essentially unsupported. Any such usecases would need to be handled by the client. In short, smem items support one direct client. I'm not sure it makes sense to unload the driver. I understand this is a qcom specific driver, but it is critical to our devices. Any situation I can think of in which this driver would be unloaded, would ultimately result in a reboot of the device. I did consider exposing the items as regmaps, but for many of the items the regmap context is consuming far more space then the actual data. And we would end up creating 3-400 regmap contexts in a normal system. Also note that the hwspinlock framework does not yet support devicetree based retrieval of spinlock handles, so that part needs to be changed. drivers/soc/qcom/Kconfig |8 + drivers/soc/qcom/Makefile |1 + drivers/soc/qcom/qcom_smem.c | 328 include/linux/soc/qcom/qcom_smem.h | 14 ++ 4 files changed, 351 insertions(+) create mode 100644 drivers/soc/qcom/qcom_smem.c create mode 100644 include/linux/soc/qcom/qcom_smem.h diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig index 7bd2c94..9e6bc56 100644 --- a/drivers/soc/qcom/Kconfig +++ b/drivers/soc/qcom/Kconfig @@ -9,3 +9,11 @@ config QCOM_GSBI functions for connecting the underlying serial UART, SPI, and I2C devices to the output pins. +config QCOM_SMEM + tristate "Qualcomm Shared Memory Interface" + depends on ARCH_QCOM + help + Say y here to enable support for the Qualcomm Shared Memory Manager. + The driver provides an interface to items in a heap shared among all + processors in a Qualcomm platform and is used for exchanging + information as well as a fifo based communication mechanism (SMD). I don't think the reference to SMD is appropriate here. SMD is one of many clients that make use of SMEM. diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile index 4389012..b1f7b18 100644 --- a/drivers/soc/qcom/Makefile +++ b/drivers/soc/qcom/Makefile @@ -1 +1,2 @@ obj-$(CONFIG_QCOM_GSBI) += qcom_gsbi.o +obj-$(CONFIG_QCOM_SMEM) += qcom_smem.o diff --git a/drivers/soc/qcom/qcom_smem.c b/drivers/soc/qcom/qcom_smem.c new file mode 100644 index 000..9766c86 --- /dev/null +++ b/drivers/soc/qcom/qcom_smem.c I see there appears to be a very small ammount of precidence (one other file), but it seems pointlessly redundant to prefix "qcom_" to the file when it exists in a "qcom" directory. What is the other point of view that I am missing? @@ -0,0 +1,328 @@ +/* + * Copyright (c) 2014, Sony Mobile Communications AB. + * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include +#include +#include +#include +#include +#include Alphabetized please. + +#define AUX_BASE_MASK 0xfffc +#define HWSPINLOCK_TIMEOUT 1000 +#define SMEM_MAX_ITEMS 512 + +/** + * struct smem_proc_comm - proc_comm communication struct (legacy) + * @command: current command to be executed + * @status: status of the currently requested command + * @params: parameters to the command + */ +struct smem_proc_comm { + u32 command; + u32 status; + u32 params[2]; +}; + +/** + * struct smem_entry - entry
Re: [RFC 3/7] mfd: devicetree: bindings: Add Qualcomm SMD based RPM DT binding
On 9/30/2014 6:08 PM, Bjorn Andersson wrote: On Tue 30 Sep 16:16 PDT 2014, Jeffrey Hugo wrote: On 9/30/2014 8:37 AM, Bjorn Andersson wrote: On Tue 30 Sep 06:46 PDT 2014, Kumar Gala wrote: On Sep 29, 2014, at 7:34 PM, Bjorn Andersson wrote: diff --git a/Documentation/devicetree/bindings/mfd/qcom-rpm-smd.txt b/Documentation/devicetree/bindings/mfd/qcom-rpm-smd.txt new file mode 100644 index 000..a846101 --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/qcom-rpm-smd.txt @@ -0,0 +1,122 @@ +Qualcomm Resource Power Manager (RPM) over SMD + +This driver is used to interface with the Resource Power Manager (RPM) found in +various Qualcomm platforms. The RPM allows each component in the system to vote +for state of the system resources, such as clocks, regulators and bus +frequencies. + +- compatible: + Usage: required + Value type: + Definition: must be one of: + "qcom,rpm-msm8974” Why not “qcom,rpm-smd”. I’d like to get Jeff H’s input on how what we do here for compat and distinguish the A-family RPM support vs B-family/RPM-SMD support. I don't see anything indicating changes in the actual communication, but as this also encodes what resources are exposed we have to keep this specific. I'm not sure what you mean with distinguish the A and B family, they are completely different and there are no overlap in compatibles or implementation. The overlap is in how children are communicating with the rpm, but this is an implementation detail - and noted in that patch as a future improvement. I forgot to add Jeff, but did send him a separate email asking for his input on all this. Yep. I saw the series. Still doing a deep dive. The rpm-smd driver (and the A family equivalent) is outside of my area of expertise, but I have some familiarity with it as it is a SMD client. Internally we have a singular compat for all of B family, but I haven't been able to figure out how that works to expose all of the resources. I'll talk to my contact later in the week to see what the differences are. That's right, because you have these tables in devicetree in the caf version. You have to have this information somewhere! True, it must exist somewhere. However since its information tied directly to the hardware, it seems like its information that would fit right in DT. I talked to my contact, and it seems like the table attributes vary more than I thought, target to target, so the single table design seems less plausible. Its my understanding you've had an offline discussion with him and some of our regulator guys to discuss the specifics of our needs. I'll let them take over as the experts. Is the per target compat the way to do this though? The way its currently done means we'll have atleast a dozen tables in qcom-smd-rpm.c, and I can't imagine they will have anything more than minor differences from the msm8x74_resource_table that currently exists in patch 6 of the series. Why wouldn't one table that is a superset of all supported targets work? It would work as long as e.g. QCOM_RPM_PM8941_MVS1 always is vsa number 4. But sure, it's a much better fit than the family a and this rpm would reject invalid resources, so it might work. But this works without us knowing about all possible platforms. But if the case is that multiple platforms expose the same table we can always tie the same table to multiple compatibles. Regards, Bjorn -- Jeffrey Hugo Qualcomm Innovation Center, Inc. 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-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] allow cpufreq drivers to export flags
Quoting Viresh Kumar (2014-10-08 01:19:40) > On 8 October 2014 13:41, Thomas Petazzoni > wrote: > > On Wed, 8 Oct 2014 13:24:30 +0530, Viresh Kumar wrote: > >> On 8 October 2014 13:18, Mike Turquette wrote: > > >> > This series is partially in response to a discussion around DT bindings > >> > for CPUfreq drivers [0], but it is also needed for on-going work to > >> > integrate CPUfreq with the scheduler. In particular a scheduler-driven > > > Well, when one has to merge a large number of changes, we often > > recommend to merge them piece by piece, which is what Mike is trying to > > do here. So we cannot at the same time ask developers to merge things > > in small pieces that are easy to review and to merge everything > > together because the users of a given API are not there yet. > > From the wording of Mike it looks like: > - This is required by cpufreq drivers - today > - And this will also be useful for scheduler Hi Viresh and Thomas, Apologies if my wording was confusing. Without getting into a grammar war, I did say that these patches were "in response" to this thread (entirely accurate) and only necessary for the "on-going work" I'm doing with the scheduler. Sorry if any of that came across as me stating that these patches were necessary to solve Thomas' problem. > > The first point doesn't stand true anymore. Lets wait for Mike's reply and > see his opinion. > > And then the patches are so small and there are no objections against > them. I don't think getting them with the scheduler changes is that bad > of an idea. In worst case, what if he has to redesign his idea? The new > routines will stay without a caller then :) Whether you merge the patches now or later is fine by me. I prefer to get my patches out early and often so I can avoid any surprises later on. If you have a fundamental objection to these patches then please let me know. Otherwise it would be wonderful to have an Ack. Thanks! Mike > > -- > viresh -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 1/7] qcom: spm: Add Subsystem Power Manager driver
On 10/07/2014 02:41 PM, Lina Iyer wrote: diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt b/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt index 1505fb8..a18e8fc 100644 --- a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt +++ b/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt @@ -2,11 +2,20 @@ SPM AVS Wrapper 2 (SAW2) The SAW2 is a wrapper around the Subsystem Power Manager (SPM) and the Adaptive Voltage Scaling (AVS) hardware. The SPM is a programmable -micro-controller that transitions a piece of hardware (like a processor or +power-controller that transitions a piece of hardware (like a processor or subsystem) into and out of low power modes via a direct connection to the PMIC. It can also be wired up to interact with other processors in the system, notifying them when a low power state is entered or exited. +Multiple revisions of the SAW hardware is supported using these Device Nodes. s/is/are/ +SAW2 revisions differ in the register offset and configuration data. Also, +same revision of the SAW in different SoCs may have different configuration the same +data due the the differences in hardware capabilities. Hence the SoC name, the +version of the SAW hardware in that SoC and the distinction between cpu (big +or Little) or cache, may be needed to uniquely identify the SAW register +configuration and initialization data. The compatible string is used to +indicate this parameter. + PROPERTIES - compatible: diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile index 70d52ed..20b329f 100644 --- a/drivers/soc/qcom/Makefile +++ b/drivers/soc/qcom/Makefile @@ -1,3 +1,4 @@ obj-$(CONFIG_QCOM_GSBI) += qcom_gsbi.o +obj-$(CONFIG_QCOM_PM) += spm.o CFLAGS_scm.o :=$(call as-instr,.arch_extension sec,-DREQUIRES_SEC=1) obj-$(CONFIG_QCOM_SCM) += scm.o scm-boot.o diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c new file mode 100644 index 000..c1dd04b --- /dev/null +++ b/drivers/soc/qcom/spm.c @@ -0,0 +1,223 @@ +/* Copyright (c) 2011-2014, The Linux Foundation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#define MAX_PMIC_DATA 3 +#define MAX_SEQ_DATA 64 + +enum { + SPM_REG_CFG, + SPM_REG_SPM_CTL, + SPM_REG_DLY, + SPM_REG_PMIC_DLY, + SPM_REG_PMIC_DATA_0, + SPM_REG_VCTL, + SPM_REG_SEQ_ENTRY, + SPM_REG_SPM_STS, + SPM_REG_PMIC_STS, + SPM_REG_NR, +}; + +struct spm_reg_data { + /* Register position */ + const u8 *reg_offset; + + /* Register initialization values */ + u32 spm_cfg; + u32 spm_dly; + u32 pmic_dly; + u32 pmic_data[MAX_PMIC_DATA]; + + /* Sequences and start indices */ + u8 seq[MAX_SEQ_DATA]; + u8 start_index[PM_SLEEP_MODE_NR]; + +}; + +struct spm_driver_data { + void __iomem *reg_base_addr; It's not really an address, more like a reg_base or just base. + const struct spm_reg_data *reg_data; +}; + +static const u8 spm_reg_offset_v2_1[SPM_REG_NR] = { + [SPM_REG_CFG] = 0x08, + [SPM_REG_SPM_CTL] = 0x30, + [SPM_REG_DLY] = 0x34, + [SPM_REG_SEQ_ENTRY] = 0x80, +}; + +/* SPM register data for 8974, 8084 */ +static const struct spm_reg_data spm_reg_8974_8084_cpu = { + .reg_offset = spm_reg_offset_v2_1, + .spm_cfg = 0x1, + .spm_dly = 0x3C102800, + .seq = { 0x03, 0x0B, 0x0F, 0x00, 0x20, 0x80, 0x10, 0xE8, 0x5B, 0x03, + 0x3B, 0xE8, 0x5B, 0x82, 0x10, 0x0B, 0x30, 0x06, 0x26, 0x30, + 0x0F }, + .start_index[PM_SLEEP_MODE_STBY] = 0, + .start_index[PM_SLEEP_MODE_SPC] = 3, +}; + +static DEFINE_PER_CPU_SHARED_ALIGNED(struct spm_driver_data, cpu_spm_drv); + +/** + * spm_set_low_power_mode() - Configure SPM start address for low power mode + * @mode: SPM LPM mode to enter + */ +int qcom_spm_set_low_power_mode(enum pm_sleep_mode mode) +{ + struct spm_driver_data *drv = &__get_cpu_var(cpu_spm_drv); this_cpu_ptr() + u32 start_index; + u32 ctl_val; + + if (!drv->reg_base_addr) + return -ENXIO; + + start_index = drv->reg_data->start_index[mode]; + + ctl_val = readl_relaxed(drv->reg_base_addr + + drv->reg_data->reg_offset[SPM_REG_SPM_CTL]); + start_index &= 0x7F; Why are we statically defining numbers large
Re: [PATCH v8 4/7] qcom: pm: Add cpu low power mode functions
On 10/07/2014 02:41 PM, Lina Iyer wrote: + +static struct platform_device qcom_cpuidle_device = { + .name = "qcom_cpuidle", + .id= -1, + .dev.platform_data = qcom_cpu_pm_enter_sleep, +}; + Same comment as last time, doesn't need to be static. +static int __init qcom_pm_device_init(void) +{ + platform_device_register(&qcom_cpuidle_device); + This is wrong. We're going to register a platform device whenever this file is included in a kernel. This is then going to try and probe the qcom_cpuidle device which is going to fail and print an error message if we're not running on a qcom device. This is one reason why I've been arguing to get rid of this file and just put it inside the spm driver. That way we don't ever add the cpuidle device and: a) We know the SPM hardware is configured and ready to be used b) We don't get this annoying warning and have some weird device in sysfs on non-qcom devices -- 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-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 5/7] qcom: cpuidle: Add cpuidle driver for QCOM cpus
On 10/07/2014 02:41 PM, Lina Iyer wrote: + +static int qcom_cpuidle_probe(struct platform_device *pdev) +{ + struct cpuidle_driver *drv = &qcom_cpuidle_driver; + int ret; + + qcom_idle_enter = pdev->dev.platform_data; + if (!qcom_idle_enter) + return -EFAULT; Is this ever true? Let's just drop the whole check. -- 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-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] allow cpufreq drivers to export flags
On 9 October 2014 05:31, Mike Turquette wrote: > Whether you merge the patches now or later is fine by me. I prefer to > get my patches out early and often so I can avoid any surprises later > on. If you have a fundamental objection to these patches then please let > me know. Otherwise it would be wonderful to have an Ack. Acked-by: Viresh Kumar But I would still ask you to get these merged with scheduler changes. :) -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" 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 3/3] ARM: dts: qcom: Add SDHC nodes for APQ8084 platform
On Wed, Oct 8, 2014 at 9:45 PM, Josh Cartwright wrote: > This has bitten me more times than I care to admit, but have you checked > that you've set CONFIG_MMC_BLOCK_MINORS higher than 16? Perhaps we > should be doing that in qcom_defconfig if we aren't already. ouch... i was using multi_v7_defconfig which indeed has: CONFIG_MMC_BLOCK_MINORS=16 note that you can override the default CONFIG with bootargs with mmcblk.perdev_minors=32 thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html