Re: [PATCH] ARM: defconfig: multi_v7: Enable Qualcomm options for Honami
Kevin Hilman writes: > Bjorn Andersson writes: > >> The Honami device does not boot due to the lack of pinctrl support >> compiled in with the multi_v7_defconfig. This patch correct this and >> enables coincell, sdhci, smsm, smp2p and wcnss_ctrl as well, all found >> on a variety of Qualcomm boards. >> >> Reported-by: Kevin Hilman >> Signed-off-by: Bjorn Andersson > > Tested-by: Kevin Hilman > > Verified that this patch on top of next-20151127 gets my z1 booting > again. ... and applied to next/defconfig on top of Andy's patch. Thanks, Kevin -- 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] ARM: defconfig: multi_v7: Enable Qualcomm options for Honami
Bjorn Andersson writes: > The Honami device does not boot due to the lack of pinctrl support > compiled in with the multi_v7_defconfig. This patch correct this and > enables coincell, sdhci, smsm, smp2p and wcnss_ctrl as well, all found > on a variety of Qualcomm boards. > > Reported-by: Kevin Hilman > Signed-off-by: Bjorn Andersson Tested-by: Kevin Hilman Verified that this patch on top of next-20151127 gets my z1 booting again. Kevin -- 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 RFC 16/27] ARM: cpuidle: Record the next wakeup event of the CPU
Lina Iyer writes:
> On Thu, Nov 19 2015 at 16:35 -0700, Kevin Hilman wrote:
>>Lina Iyer writes:
>>
>>> Reading the next wakeup of the CPU can only be realiably done only from
>>> that CPU. In the idle enter path record the next wake up of the CPU. The
>>> information is useful to determine the sleep time left for the CPU.
>>>
>>> Signed-off-by: Lina Iyer
>>> ---
>>> drivers/cpuidle/cpuidle-arm.c | 5 +
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c
>>> index 8e72a23..b3133ef 100644
>>> --- a/drivers/cpuidle/cpuidle-arm.c
>>> +++ b/drivers/cpuidle/cpuidle-arm.c
>>> @@ -18,9 +18,11 @@
>>> #include
>>> #include
>>> #include
>>> +#include
>>> #include
>>> #include
>>> #include
>>> +#include
>>>
>>> #include
>>>
>>> @@ -49,7 +51,9 @@ static int arm_enter_idle_state(struct cpuidle_device
>>> *dev,
>>> ret = cpu_pm_enter();
>>> if (!ret) {
>>> struct device *cpu_dev = get_cpu_device(dev->cpu);
>>> + struct generic_pm_domain_data *gpd = dev_gpd_data(cpu_dev);
>>>
>>> + gpd->td.next_wakeup = tick_nohz_get_next_wakeup();
>>> RCU_NONIDLE(pm_runtime_put_sync_suspend(cpu_dev));
>>
>>Maybe set this back to zero atomicaly, after wakeup?
>>
>>Checking for non-zero that might be another way for the domain goveror that
>>there
>>haven't been any CPU wakeups since the CPUs have gone idle.
>>
> I set it to 0 below.
Oops, you're right. Not sure how I missed that.
> The reference counting by the GET_PUT tells the domain if a device is
> suspended. I see that as a redudant information. May be I am not getting
> your point.
It's redundant, but may be helpful in detecting races. e.g. domain is
being powered off and CPU wakes up and sets this zero, but runtime PM
layers haven't been called yet so dev->power->usage_count is still zero.
Kevin
--
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 RFC 18/27] drivers: cpu-pd: Add PM Domain governor for CPUs
Lina Iyer writes:
> A PM domain comprising of CPUs may be powered off when all the CPUs in
> the domain are powered down. Powering down a CPU domain is generally a
> expensive operation and therefore the power performance trade offs
> should be considered. The time between the last CPU powering down and
> the first CPU powering up in a domain, is the time available for the
> domain to sleep. Ideally, the sleep time of the domain should fulfill
> the residency requirement of the domains' idle state.
>
> To do this effectively, read the time before the wakeup of the cluster's
> CPUs and ensure that the domain's idle state sleep time guarantees the
> QoS requirements of each of the CPU, the PM QoS CPU_DMA_LATENCY and the
> state's residency.
>
> Signed-off-by: Lina Iyer
[...]
> +static bool cpu_stop_ok(struct device *dev)
> +{
> + return true;
> +}
> +
> +struct dev_power_governor cpu_pd_gov = {
> + .power_down_ok = cpu_pd_down_ok,
> + .stop_ok = cpu_stop_ok,
> +};
If stop_ok is unconditionally true, it should probably just be removed
(IOW cpu_pd_gov->stop_ok == NULL), and that will avoid an unnecessary
function call.
Kevin
--
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 RFC 18/27] drivers: cpu-pd: Add PM Domain governor for CPUs
Lorenzo Pieralisi writes: > On Tue, Nov 17, 2015 at 03:37:42PM -0700, Lina Iyer wrote: >> A PM domain comprising of CPUs may be powered off when all the CPUs in >> the domain are powered down. Powering down a CPU domain is generally a >> expensive operation and therefore the power performance trade offs >> should be considered. The time between the last CPU powering down and >> the first CPU powering up in a domain, is the time available for the >> domain to sleep. Ideally, the sleep time of the domain should fulfill >> the residency requirement of the domains' idle state. >> >> To do this effectively, read the time before the wakeup of the cluster's >> CPUs and ensure that the domain's idle state sleep time guarantees the >> QoS requirements of each of the CPU, the PM QoS CPU_DMA_LATENCY and the >> state's residency. > > To me this information should be part of the CPUidle governor (it is > already there), we should not split the decision into multiple layers. > > The problem you are facing is that the CPUidle governor(s) do not take > cross cpus relationship into account, I do not think that adding another > decision layer in the power domain subsystem helps, you are doing that > just because adding it to the existing CPUidle governor(s) is invasive. > > Why can't we use the power domain work you put together to eg disable > idle states that share multiple cpus and make them "visible" only > when the power domain that encompass them is actually going down ? > > You could use the power domains information to detect states that > are shared between cpus. > > It is just an idea, what I am saying is that having another governor in > the power domain subsytem does not make much sense, you split the > decision in two layers while there is actually one, the existing > CPUidle governor and that's where the decision should be taken. Hmm, considering "normal" devices in "normal" power domains, and following the same logic, the equivalent would be to say that the decision to gate the power domain belongs to the individual drivers in the domain instead of in the power domain layer. I disagree. IMO, there are different decision layers because there are different hardware layers. Devices (including CPUs) are reponsible for handling device-local idle states, based on device-local conditions (e.g. local wakeups, timers, etc.) and domains are responsible for handling decisions based on conditions of the whole domain. Kevin -- 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 RFC 16/27] ARM: cpuidle: Record the next wakeup event of the CPU
Lina Iyer writes:
> Reading the next wakeup of the CPU can only be realiably done only from
> that CPU. In the idle enter path record the next wake up of the CPU. The
> information is useful to determine the sleep time left for the CPU.
>
> Signed-off-by: Lina Iyer
> ---
> drivers/cpuidle/cpuidle-arm.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c
> index 8e72a23..b3133ef 100644
> --- a/drivers/cpuidle/cpuidle-arm.c
> +++ b/drivers/cpuidle/cpuidle-arm.c
> @@ -18,9 +18,11 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -49,7 +51,9 @@ static int arm_enter_idle_state(struct cpuidle_device *dev,
> ret = cpu_pm_enter();
> if (!ret) {
> struct device *cpu_dev = get_cpu_device(dev->cpu);
> + struct generic_pm_domain_data *gpd = dev_gpd_data(cpu_dev);
>
> + gpd->td.next_wakeup = tick_nohz_get_next_wakeup();
> RCU_NONIDLE(pm_runtime_put_sync_suspend(cpu_dev));
Maybe set this back to zero atomicaly, after wakeup?
Checking for non-zero that might be another way for the domain goveror that
there
haven't been any CPU wakeups since the CPUs have gone idle.
Kevin
--
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 RFC 15/27] PM / Domains: Add next_wakeup to device's timing data
Lina Iyer writes:
> Allow devices that know when their next wakeup event is, to record save
> it as part of timing data. A genpd governor may use this data to
> determine if suspending the domain is going to affect the QoS of its
> devices.
>
> Signed-off-by: Lina Iyer
> ---
> include/linux/pm_domain.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index f1329ea..9ac089d 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -16,6 +16,7 @@
> #include
> #include
> #include
> +#include
>
> /* Defines used for the flags field in the struct generic_pm_domain */
> #define GENPD_FLAG_PM_CLK(1U << 0) /* PM domain uses PM clk */
> @@ -104,6 +105,7 @@ struct gpd_timing_data {
> s64 effective_constraint_ns;
> bool constraint_changed;
> bool cached_stop_ok;
> + ktime_t next_wakeup;
> };
The addition of a new field should be combined with the patch that uses
it (e.g. PATCH 16/27)
Kevin
--
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 RFC 13/27] ARM: cpuidle: Add runtime PM support for CPU idle
Lina Iyer writes:
> Notify runtime PM when the CPU is going to be powered off in the idle
> state. This allows for runtime PM suspend/resume of the CPU as well as
> its PM domain.
>
> Cc: Daniel Lezcano
> Cc: Lorenzo Pieralisi
> Signed-off-by: Lina Iyer
> ---
> drivers/cpuidle/cpuidle-arm.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c
> index 545069d..8e72a23 100644
> --- a/drivers/cpuidle/cpuidle-arm.c
> +++ b/drivers/cpuidle/cpuidle-arm.c
> @@ -11,13 +11,16 @@
>
> #define pr_fmt(fmt) "CPUidle arm: " fmt
>
> +#include
> #include
> #include
> #include
> #include
> #include
> #include
> +#include
> #include
> +#include
>
> #include
>
> @@ -45,6 +48,10 @@ static int arm_enter_idle_state(struct cpuidle_device *dev,
>
> ret = cpu_pm_enter();
> if (!ret) {
> + struct device *cpu_dev = get_cpu_device(dev->cpu);
> +
> + RCU_NONIDLE(pm_runtime_put_sync_suspend(cpu_dev));
> +
A comment in the changelog about the use of _put_sync_suspend() instead
of a normal _put_sync() would be helpful for this patch.
Kevin
--
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 RFC 03/27] PM / Domain: Add additional state specific param
Lina Iyer writes: > Allow domain states to hold additional state related data in a u32 > value. This may be used by the platform driver. Should probably expand the changelog here to give some examples how how this might be used by the platform driver. Kevin -- 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] clk: qcom: Specify LE device endianness
Stephen Boyd writes: > All these clock controllers are little endian devices, but so far > we've been relying on the regmap mmio bus handling this for us > without explicitly stating that fact. After commit 4a98da2164cf > (regmap-mmio: Use native endianness for read/write, 2015-10-29), > the regmap mmio bus will read/write with the __raw_*() IO > accessors, instead of using the readl/writel() APIs that do > proper byte swapping for little endian devices. > > So if we're running on a big endian processor and haven't > specified the endianness explicitly in the regmap config or in > DT, we're going to switch from doing little endian byte swapping > to big endian accesses without byte swapping, leading to some > confusing results. On my apq8074 dragonboard, this causes the > device to fail to boot as we access the clock controller with > big endian IO accesses even though the device is little endian. > > Specify the endianness explicitly so that the regmap core > properly byte swaps the accesses for us. > > Reported-by: Kevin Hilman > Cc: Simon Arlott > Cc: Mark Brown > Signed-off-by: Stephen Boyd Tested-by: Kevin Hilman Verified that this patch on top of next-20151118 gets my 2 8x94 boards booting again with big-endian kernel. Kevin -- 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/3] Remove the need for qcom,{msm-id,board-id,pmic-id}
Hi Stephen, Stephen Boyd writes: > This patchset documents a compatible string format that encodes > all the information that was being encoded in qcom specific DT > properties in downstream msm kernels. The goal being to come > up with a format that will allow us to express the information > we want to express without requiring the use of vendor specific > properties. An updated dtbTool will be released after these new > bindings are accepted so that users can work with non-updateable > bootloaders. > > This is an attempt to resolve a discussion around March of this year[1] > where the arm-soc maintainers suggested we express this information > through the board's compatible string. Thanks for working on this. This indeed looks like the right way to handle this, including updating dtbTool. Very nice. I guess the DT maintainers should have a crack at the bindings, but otherwise it looks good. Kevin -- 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 09/14] ARM: dts: ifc6410: Add missing pinctrl to gsbi7 uart
Hi Srini, On Fri, Sep 18, 2015 at 5:31 AM, Srinivas Kandagatla wrote: > This patch adds missing 2pin uart pinctrl property to gsbi7 uart on > IFC6410. > > Signed-off-by: Srinivas Kandagatla kernelci.org found some new boot failures in -next on 8064[1]. I bisected it down to this patch, which landed in linux next in the form of commit d3a578ed4310 ARM: dts: ifc6410: Add missing pinctrl to gsbi7 uart. Kevin [1] http://kernelci.org/boot/qcom-apq8064-ifc6410/job/next/kernel/next-20150925/defconfig/multi_v7_defconfig/lab/lab-khilman/?_id=5605083c59b51466d26c3250 -- 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: [GIT PULL] qcom dt changes for 4.3-rc2
Hi Andy, Andy Gross writes: > The following changes since commit 4040f37692ffbc602117d316c574a68a0fbfed93: > > Merge tag 'qcom-dt-for-4.3' into for-dt (2015-08-27 12:07:21 -0500) > > are available in the git repository at: > > > git://codeaurora.org/quic/kernel/agross-msm.git tags/qcom-dt-for-4.3-rc2 > > for you to fetch changes up to cecd36b9c847b451e8ae297b53b094445b5352e4: > > ARM: dts: qcom: msm8974-sony-xperia-honami: Use stdout-path (2015-08-27 > 12:11:22 -0500) > > Can you respin this on v4.2-rc2? Your current branch is on -rc8, which causes some conflicts with other drivers/cpufreq stuff we have in the late branch. Similar request for your soc branch. Kevin -- 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 v9 0/9] Add simple NVMEM Framework via regmap.
On Fri, Aug 7, 2015 at 9:18 AM, Srinivas Kandagatla wrote: > Thanks Kevin, > > On 07/08/15 17:13, Kevin Hilman wrote: >> >> Srinivas Kandagatla writes: >> >>> This patchset adds a new simple NVMEM framework to kernel, and it is >>> tested >>> with various drivers like "QCOM thermal sensors", "QCOM cpr driver", >>> "begal bone cape manager" and few more on the way. >>> >>> Can you please consider this as 4.3 material, AFAIK there are more than 3 >>> drivers >>> depending on this framework which are wating since last 2 merge windows. >>> >>> Should these patches go via ARM-SOC tree or via Greg's tree? >> >> >> With Greg's ack, we can take this through the arm-soc tree (unless Greg >> wants to queue it, which is fine also.) >> > > Greg already merged these patches yesterday on to > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git > in the char-misc-next branch. Ah, great. I missed that (still catching up on post-vacation mail backlog.) Thanks, Kevin -- 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 v9 0/9] Add simple NVMEM Framework via regmap.
Srinivas Kandagatla writes: > This patchset adds a new simple NVMEM framework to kernel, and it is tested > with various drivers like "QCOM thermal sensors", "QCOM cpr driver", > "begal bone cape manager" and few more on the way. > > Can you please consider this as 4.3 material, AFAIK there are more than 3 > drivers > depending on this framework which are wating since last 2 merge windows. > > Should these patches go via ARM-SOC tree or via Greg's tree? With Greg's ack, we can take this through the arm-soc tree (unless Greg wants to queue it, which is fine also.) Can you collect the final Tested-by's and prepare a pull request? Thanks, Kevin -- 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 12/12] msm: msm_fb: Remove dead code
Stephen Boyd writes: > On 03/13/2015 11:51 AM, Jean-Christophe PLAGNIOL-VILLARD wrote: >> On 11:09 Fri 13 Mar , Stephen Boyd wrote: >>> This code is no longer used now that mach-msm has been removed. >>> Delete it. >>> >>> Cc: Jean-Christophe Plagniol-Villard >>> Cc: Tomi Valkeinen >>> Cc: David Brown >>> Cc: Bryan Huntsman >>> Cc: Daniel Walker >>> Signed-off-by: Stephen Boyd >> Acked-by: Jean-Christophe PLAGNIOL-VILLARD >> > > Thanks again. Should this go through arm-soc (Cced)? Or some other tree > besides the fb tree? This is the last patch that hasn't been applied in > this set of 12 patches. It's all drivers/video/fbdev/*, so it's not arm-soc material. Looks to me like it should go through the fbdev tree. Kevin -- 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 1/2] ARM: Add cpu_resume_arm() for firmwares that resume in ARM state
Stephen Boyd writes: > On 06/08/2015 02:33 PM, Kevin Hilman wrote: >> Stephen Boyd writes: >> >>> On 06/03, Russell King - ARM Linux wrote: >>>> On Tue, Jun 02, 2015 at 12:12:57PM -0700, Stephen Boyd wrote: >>>>> Some platforms always enter the kernel in the ARM state even if >>>>> the kernel is compiled for THUMB2. Add a small wrapper on top of >>>>> cpu_resume() that switches into THUMB2 state. >>>>> >>>>> This fixes a problem reported by Kevin Hilman on next-20150601 >>>>> where the ifc6410 fails to boot a THUMB2 kernel because the >>>>> platform's firmware always enters the kernel in ARM mode from >>>>> deep idle states. >>>> I think this bit of the commit message isn't quite correct: this will >>>> only resolve the boot problem with a T2 kernel when the failing platform >>>> is converted to use the cpu_resume_arm() entry point. >>>> >>>> Apart from that, the patch looks good to me, thanks. >>> Sure, it could say "This provides the functionality to fix a problem >>> reported by..." >> Stephen, are you planning to submit this through Russell's patch >> tracker? or... >> >> Russell, should we take this through arm-soc along with PATCH 2/2 which >> depends on this one? >> > > I'm happy to see it be applied by arm-soc maintainers directly given > that Russell said "the patch looks good to me". The commit text needs a > slight reword here though, so I can resubmit the patches to arm-soc if > you like. Go ahead and respin/resubmit, and I can get them ready, but I'll wait for an official word or acked-by from Russell before taking them both through arm-soc. Kevin -- 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 1/2] ARM: Add cpu_resume_arm() for firmwares that resume in ARM state
Stephen Boyd writes: > On 06/03, Russell King - ARM Linux wrote: >> On Tue, Jun 02, 2015 at 12:12:57PM -0700, Stephen Boyd wrote: >> > Some platforms always enter the kernel in the ARM state even if >> > the kernel is compiled for THUMB2. Add a small wrapper on top of >> > cpu_resume() that switches into THUMB2 state. >> > >> > This fixes a problem reported by Kevin Hilman on next-20150601 >> > where the ifc6410 fails to boot a THUMB2 kernel because the >> > platform's firmware always enters the kernel in ARM mode from >> > deep idle states. >> >> I think this bit of the commit message isn't quite correct: this will >> only resolve the boot problem with a T2 kernel when the failing platform >> is converted to use the cpu_resume_arm() entry point. >> >> Apart from that, the patch looks good to me, thanks. > > Sure, it could say "This provides the functionality to fix a problem > reported by..." Stephen, are you planning to submit this through Russell's patch tracker? or... Russell, should we take this through arm-soc along with PATCH 2/2 which depends on this one? Kevin -- 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 0/7] qcom: Add support for tsens driver
Rajendra Nayak writes: > Hi Kevin, > >> Curious how are you testing this? with which thermal governor? > > I have been mostly testing this with step wise on the db410c target > board. > >> >> I'm experimenting with the new thermal power_allocator governor >> currently in linux-next, and was hoping to use this to test it, but I'm >> noticing it doesn't work with the power_allocator governor. >> >> The first thing is that the power_allator goveror expects each thermal >> zone to have a sustainable-power property in the DT, which this series >> doesn't provide. Hacking some dummy values in there to see what happens >> next, I then hit the "thermal zone cpu-thermal0 has wrong trip setup for >> power allocator" error from the power_allocator driver. > > Right, the RFC I posted wasn't tested with IPA and hence does not have > some of the additional properties in DT which IPA expects. > The wrong trip setup error that you are seeing is because IPA expects > 2 passive trip points, a "switch on" trip point and a "desired > temperature" trip point and the dts I posted with the RFC had > just one passive trip point. > > I am in the process of testing this with IPA myself (on db410c mainly, > but will also test on db8074 and IFC6410/6540 as well) and will repost > in a few days with updated dts entries and a few other minor fixes > to the driver itself. Let me know what target board you are > interested to get this working on and I will make sure I test on it > before I repost. Great, thanks! I tried on qcom-apq8064-ifc6410 and qcom-msm8974-sony-xperia-honami, and would have also tried qcom-apq8084-ifc6540 but saw that Lina mentioned there was missing support for 8084. Kevin -- 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 0/7] qcom: Add support for tsens driver
[+ Javi, Punit for power_allocator questions ] Hi Rajendra, Rajendra Nayak writes: > This is an attempt to have a single tsens driver for > the different versions of the tsens IP that exist, on > different qcom msm/apq SoCs. > Support is added for msm8916, msm8960 and msm8974 families. > Based on top of the latest eeprom framework patches [1] Curious how are you testing this? with which thermal governor? I'm experimenting with the new thermal power_allocator governor currently in linux-next, and was hoping to use this to test it, but I'm noticing it doesn't work with the power_allocator governor. The first thing is that the power_allator goveror expects each thermal zone to have a sustainable-power property in the DT, which this series doesn't provide. Hacking some dummy values in there to see what happens next, I then hit the "thermal zone cpu-thermal0 has wrong trip setup for power allocator" error from the power_allocator driver. I stopped there as that made it clear this isn't yet ready to be used with the power_allocator governor. Kevin -- 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 1/7] thermal: qcom: tsens: Add a skeletal tsens drivers
Hi Rajendra,
Rajendra Nayak writes:
> tsens is qualcomms' thermal temperature sensor device. It
> supports reading temperatures from multiple thermal sensors
> present on various QCOM SoCs.
> Calibration data is generally read from a eeprom device.
>
> Add a skeleton driver with all the necessary abstractions so
> a variety of qcom device families which support tsens can
> add driver extensions.
>
> Also add the required device tree bindings which can be used
> to descibe the tsens device in DT.
>
> Signed-off-by: Rajendra Nayak
[...]
> + ret = tmdev->ops->calibrate(tmdev);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "tsens calibration failed\n");
> + return ret;
> + }
I was trying this series on linux-next with Srini's eeprom v4 and my
first attempt was to build the eeprom, qfprom and tsens drivers as
built-in. Unfortunately, the tsens driver probed before the
eeprom/qfprom driver so failed here with the "calibration failed"
message.
Building things as modules worked better, but you might want to take a
look at possibly using -EPROBE_DEFER here?
Kevin
--
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 0/2] Fix cpuidle on qcom's THUMB2 kernels
Stephen Boyd writes: > I split this patch into two now that we have the cpu_resume_arm() > wrapper. The first patch could go through rmk's tree and then be > provided at some stable branch so that the second patch could go > through arm-soc. Or the two patches could go through arm-soc and > be squashed into one patch or stay split out. Either way, we have a > cross maintainer dependency here that needs to be resolved. Tested-by: Kevin Hilman Boot tested on top of next-20150602 using multi_v7_defconfig + CONFIG_THUMB2_KERNEL=y on a handful of qcom 8x[678]4 platforms[1] These platforms were all failing to boot this config with plain next-20150602[2] and are all booting fine with this series. Kevin [1] qcom-apq8064-ifc6410 qcom-apq8064-cm-qs600 qcom-apq8074-dragonboard qcom-msm8974-sony-xperia-honami qcom-apq8084-ifc6540 [2] http://kernelci.org/boot/all/job/next/kernel/next-20150602/ -- 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/RFT 2/6] clk: qcom: Add runtime support to handle clocks using PM clocks
Geert Uytterhoeven writes: > On Fri, Apr 24, 2015 at 12:58 PM, Rajendra Nayak > wrote: >>> Second, I want to see less users of pm_clk_add_notifier() since it's >>> not the proper/long-term way of how to assign PM domain pointers to a >>> device. Instead that shall be done at device registration point. In >>> your case while parsing the DT nodes and adding devices onto to their >>> buses. >> >> but these are devices which do not really have a controllable power >> domain, they just have controllable clocks. >> >>> Yes, I understand that it will requires quite some work to adopt to >>> this behaviour - but that how it shall be done. >>> >>> Finally, an important note, you don't need to use PM domains for these >>> devices at all and thus you don't need to fix what I propose. Instead >>> you only have to implement the runtime PM callbacks for each driver >>> and manage the clocks from there. That is probably also a easier >>> solution. >> >> But that would mean I repeat the same code in all drivers to do a >> clk_get/prepare/enable/disable/unprepare of the same "core" and "iface" >> clocks. I thought we have clock_ops.c just to avoid that (atleast up >> until we have a better way of doing it) >> And there are atleast a few architecture which have used it to avoid the >> duplication across all drivers (omap1/davinci/sh/keystone) > > At least for arm/shmobile, we're planning to move away from this, cfr. > http://www.spinics.net/lists/linux-sh/msg41114.html Just to clarify for Rajendra's sake... SH is moving away from the pm_clk_add_notifier(), but not duplicating the clk_get/prepare/enable/disable/unprepare across all the drivers. Rather, they're using a genpd to model the clock domain, and taking advantage of the pm_domain speciic attach callback to attach the PM clocks. This should work for qcom also assuming that these device nodes don't also need to belong to a different PM domain. Kevin -- 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/RFT 5/6] i2c: qup: Get rid of clock handling as its done using runtime callbacks
Rajendra Nayak writes: > Remove all clock handling from the driver as this is not handled from > within platform runtime callbacks. s/not/now/ ?? Kevin -- 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/RFT 4/6] mmc: sdhci-msm: convert driver to use runtime PM apis
"Rajendra Nayak" writes:
>> On 23 April 2015 at 10:45, Rajendra Nayak wrote:
>>> With platform support now in place to manage clocks from within runtime
>>> PM
>>> callbacks, get rid of all clock handling from the driver and convert the
>>> driver to use runtime PM apis.
>>>
>>> Signed-off-by: Rajendra Nayak
>>> Cc:
>>> ---
>>> drivers/mmc/host/sdhci-msm.c | 46
>>> +++-
>>> 1 file changed, 11 insertions(+), 35 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>>> index 4a09f76..3c62a77 100644
>>> --- a/drivers/mmc/host/sdhci-msm.c
>>> +++ b/drivers/mmc/host/sdhci-msm.c
>>> @@ -19,6 +19,7 @@
>>> #include
>>> #include
>>> #include
>>> +#include
>>>
>>> #include "sdhci-pltfm.h"
>>>
>>> @@ -56,8 +57,6 @@
>>> struct sdhci_msm_host {
>>> struct platform_device *pdev;
>>> void __iomem *core_mem; /* MSM SDCC mapped address */
>>> - struct clk *clk;/* main SD/MMC bus clock */
>>> - struct clk *pclk; /* SDHC peripheral bus clock */
>>> struct clk *bus_clk;/* SDHC bus voter clock */
>>> struct mmc_host *mmc;
>>> struct sdhci_pltfm_data sdhci_msm_pdata;
>>> @@ -469,29 +468,8 @@ static int sdhci_msm_probe(struct platform_device
>>> *pdev)
>>> goto pltfm_free;
>>> }
>>>
>>> - /* Setup main peripheral bus clock */
>>> - msm_host->pclk = devm_clk_get(&pdev->dev, "iface");
>>> - if (IS_ERR(msm_host->pclk)) {
>>> - ret = PTR_ERR(msm_host->pclk);
>>> - dev_err(&pdev->dev, "Perpheral clk setup failed (%d)\n",
>>> ret);
>>> - goto bus_clk_disable;
>>> - }
>>> -
>>> - ret = clk_prepare_enable(msm_host->pclk);
>>> - if (ret)
>>> - goto bus_clk_disable;
>>> -
>>> - /* Setup SDC MMC clock */
>>> - msm_host->clk = devm_clk_get(&pdev->dev, "core");
>>> - if (IS_ERR(msm_host->clk)) {
>>> - ret = PTR_ERR(msm_host->clk);
>>> - dev_err(&pdev->dev, "SDC MMC clk setup failed (%d)\n",
>>> ret);
>>> - goto pclk_disable;
>>> - }
>>> -
>>> - ret = clk_prepare_enable(msm_host->clk);
>>> - if (ret)
>>> - goto pclk_disable;
>>> + pm_runtime_enable(&pdev->dev);
>>> + pm_runtime_get_sync(&pdev->dev);
>>>
>>> core_memres = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>>> msm_host->core_mem = devm_ioremap_resource(&pdev->dev,
>>> core_memres);
>>> @@ -499,7 +477,7 @@ static int sdhci_msm_probe(struct platform_device
>>> *pdev)
>>> if (IS_ERR(msm_host->core_mem)) {
>>> dev_err(&pdev->dev, "Failed to remap registers\n");
>>> ret = PTR_ERR(msm_host->core_mem);
>>> - goto clk_disable;
>>> + goto err;
>>> }
>>>
>>> /* Reset the core and Enable SDHC mode */
>>> @@ -511,7 +489,7 @@ static int sdhci_msm_probe(struct platform_device
>>> *pdev)
>>> if (readl(msm_host->core_mem + CORE_POWER) & CORE_SW_RST) {
>>> dev_err(&pdev->dev, "Stuck in reset\n");
>>> ret = -ETIMEDOUT;
>>> - goto clk_disable;
>>> + goto err;
>>> }
>>>
>>> /* Set HC_MODE_EN bit in HC_MODE register */
>>> @@ -545,15 +523,13 @@ static int sdhci_msm_probe(struct platform_device
>>> *pdev)
>>>
>>> ret = sdhci_add_host(host);
>>> if (ret)
>>> - goto clk_disable;
>>> + goto err;
>>>
>>> return 0;
>>>
>>> -clk_disable:
>>> - clk_disable_unprepare(msm_host->clk);
>>> -pclk_disable:
>>> - clk_disable_unprepare(msm_host->pclk);
>>> -bus_clk_disable:
>>> +err:
>>> + pm_runtime_put_sync(&pdev->dev);
>>> + pm_runtime_disable(&pdev->dev);
>>> if (!IS_ERR(msm_host->bus_clk))
>>> clk_disable_unprepare(msm_host->bus_clk);
>>> pltfm_free:
>>> @@ -571,8 +547,8 @@ static int sdhci_msm_remove(struct platform_device
>>> *pdev)
>>>
>>> sdhci_remove_host(host, dead);
>>> sdhci_pltfm_free(pdev);
>>> - clk_disable_unprepare(msm_host->clk);
>>> - clk_disable_unprepare(msm_host->pclk);
>>> + pm_runtime_put_sync(&pdev->dev);
>>> + pm_runtime_disable(&pdev->dev);
>>> if (!IS_ERR(msm_host->bus_clk))
>>> clk_disable_unprepare(msm_host->bus_clk);
>>> return 0;
>>> --
>>> QUALCOMM INDIA, on behalf of 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-pm" in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>> This all looks wrong. The driver will no longer work unless CONFIG_PM is
>> set.
>
> Right, I seem to have completely ignored the !CONFIG_PM case.
> Will look at ho
Re: [RFC 0/5] PM / clock_ops: provide default runtime ops and cleanup users
Rajendra Nayak writes: > Most users of PM clocks do the exact same thing in runtime callbacks. Probably because they were all copied from mach-davinci. ;) > Provide default callbacks and cleanup the existing users > (keystone/davinci/omap1/sh) Very nice cleanup, Thanks! For the series: Reviewed-by: Kevin Hilman Kevin -- 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 v18 01/11] ARM: qcom: Add Subsystem Power Manager (SPM) driver
Lina Iyer writes: > SPM is a hardware block that controls the peripheral logic surrounding > the application cores (cpu/l$). When the core executes WFI instruction, > the SPM takes over the putting the core in low power state as > configured. The wake up for the SPM is an interrupt at the GIC, which > then completes the rest of low power mode sequence and brings the core > out of low power mode. > > The SPM has a set of control registers that configure the SPMs > individually based on the type of the core and the runtime conditions. > SPM is a finite state machine block to which a sequence is provided and > it interprets the bytes and executes them in sequence. Each low power > mode that the core can enter into is provided to the SPM as a sequence. > > Configure the SPM to set the core (cpu or L2) into its low power mode, > the index of the first command in the sequence is set in the SPM_CTL > register. When the core executes ARM wfi instruction, it triggers the > SPM state machine to start executing from that index. The SPM state > machine waits until the interrupt occurs and starts executing the rest > of the sequence until it hits the end of the sequence. The end of the > sequence jumps the core out of its low power mode. > > Add support for an idle driver to set up the SPM to place the core in > Standby or Standalone power collapse mode when the core is idle. > > Based on work by: Mahesh Sivasubramanian , > Ai Li , Praveen Chidambaram > Original tree available at - > git://codeaurora.org/quic/la/kernel/msm-3.10.git > > Cc: Stephen Boyd > Cc: Arnd Bergmann > Cc: Kevin Hilman > Cc: Daniel Lezcano > Signed-off-by: Lina Iyer Acked-by: Kevin Hilman and also tested on qcom-apq8064-ifc6410 and qcom-msm8974-sony-xperia-honami and saw the various states being entered on all CPUs. Tested-by: Kevin Hilman Kevin -- 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 1/3] devicetree: bindings: Document qcom,msm-id and qcom,board-id
Kumar Gala writes: > The top level qcom,msm-id and qcom,board-id are utilized by bootloaders > on Qualcomm MSM platforms to determine which device tree should be > utilized and passed to the kernel. > > Cc: > Signed-off-by: Kumar Gala Is this the special magic that allows qcom bootloaders to take a kernel plus multiple DTBs and figure out which DTB to pass? Kevin -- 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/2] qcom: scm: Add scm_set_warm_boot_addr function
Lina Iyer writes:
> A core can be powered down for cpuidle or when it is hotplugged of.
s/of/off/ ?
> In
> either case, the warmboot return address would be different. Allow
> setting the warmboot address for a specific cpu, optimize and write to
> the firmware if the address is different than the previously set
> address.
>
> Also we do not need to export the warmboot flags. Move them into the
> implementation file.
>
> Signed-off-by: Lina Iyer
> ---
> arch/arm/mach-qcom/scm-boot.c | 38 ++
> arch/arm/mach-qcom/scm-boot.h | 7 +++
> 2 files changed, 41 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-qcom/scm-boot.c b/arch/arm/mach-qcom/scm-boot.c
> index 45cee3e..cb73134 100644
> --- a/arch/arm/mach-qcom/scm-boot.c
> +++ b/arch/arm/mach-qcom/scm-boot.c
> @@ -1,4 +1,5 @@
> /* Copyright (c) 2010, Code Aurora Forum. All rights reserved.
> + * Copyright (c) 2014, Linaro Ltd.
> *
> * 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
> @@ -21,6 +22,23 @@
> #include "scm.h"
> #include "scm-boot.h"
>
> +#define SCM_FLAG_WARMBOOT_CPU0 0x04
> +#define SCM_FLAG_WARMBOOT_CPU1 0x02
> +#define SCM_FLAG_WARMBOOT_CPU2 0x10
> +#define SCM_FLAG_WARMBOOT_CPU3 0x40
> +
> +struct scm_warmboot {
> + int flag;
> + void *entry;
> +};
> +
> +static struct scm_warmboot scm_flags[] = {
> + { .flag = SCM_FLAG_WARMBOOT_CPU0 },
> + { .flag = SCM_FLAG_WARMBOOT_CPU1 },
> + { .flag = SCM_FLAG_WARMBOOT_CPU2 },
> + { .flag = SCM_FLAG_WARMBOOT_CPU3 },
> +};
> +
> /*
> * Set the cold/warm boot address for one of the CPU cores.
> */
> @@ -31,9 +49,29 @@ int scm_set_boot_addr(phys_addr_t addr, int flags)
> phys_addr_t addr;
> } cmd;
>
> + might_sleep();
> +
Unrelated change?
Looking closer at scm_call(), looks like the might_sleep() should go
inside scm_call() where it uses the mutex.
> cmd.addr = addr;
> cmd.flags = flags;
> return scm_call(SCM_SVC_BOOT, SCM_BOOT_ADDR,
> &cmd, sizeof(cmd), NULL, 0);
> }
> EXPORT_SYMBOL(scm_set_boot_addr);
> +
> +int scm_set_warm_boot_addr(void *entry, int cpu)
> +{
> + int ret;
> +
> + /*
> + * Reassign only if we are switching from hotplug entry point
> + * to cpuidle entry point or vice versa.
> + */
> + if (entry == scm_flags[cpu].entry)
> + return 0;
> +
> + ret = scm_set_boot_addr(virt_to_phys(entry), scm_flags[cpu].flag);
> + if (!ret)
> + scm_flags[cpu].entry = entry;
> +
> + return ret;
> +}
> diff --git a/arch/arm/mach-qcom/scm-boot.h b/arch/arm/mach-qcom/scm-boot.h
> index 02b445c..97dbf58 100644
> --- a/arch/arm/mach-qcom/scm-boot.h
> +++ b/arch/arm/mach-qcom/scm-boot.h
> @@ -1,4 +1,5 @@
> /* Copyright (c) 2010, Code Aurora Forum. All rights reserved.
> + * Copyright (c) 2014, Linaro Ltd.
> *
> * 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
> @@ -9,6 +10,7 @@
> * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> * GNU General Public License for more details.
> */
> +
> #ifndef __MACH_SCM_BOOT_H
> #define __MACH_SCM_BOOT_H
>
> @@ -16,11 +18,8 @@
> #define SCM_FLAG_COLDBOOT_CPU1 0x01
> #define SCM_FLAG_COLDBOOT_CPU2 0x08
> #define SCM_FLAG_COLDBOOT_CPU3 0x20
> -#define SCM_FLAG_WARMBOOT_CPU0 0x04
> -#define SCM_FLAG_WARMBOOT_CPU1 0x02
> -#define SCM_FLAG_WARMBOOT_CPU2 0x10
> -#define SCM_FLAG_WARMBOOT_CPU3 0x40
You just added these in patch 1, and now moved them. I would just
squash the 2 patches together.
Kevin
--
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 v14 00/10] cpuidle driver for QCOM SoCs: 8064, 8074, 8084
Lina Iyer writes: > Dependent patchsets - > https://lkml.org/lkml/2014/8/4/767 > http://www.spinics.net/lists/linux-arm-msm/msg10799.html > http://www.spinics.net/lists/linux-arm-msm/msg10795.html > > Changes since v13: > - Return values for idle states propagated back to the cpuidle driver. > - Remove static bool cpuidle_drv_init. > - Register cpuidle driver and cpuidle device separately. > - cpuidle device registered only when the SPM for the cpu is probed. > - Initialization changes to ensure dynamic cpuidle devices are registered only > after the cpuidle driver is registered. > - Removed wmb, replaced with a poll loop to ensure that the SPM registers are > written before executing wfi. > - Added spm_register_write_sync() for write guarantees. > - Removed irrelevant return value for spm_set_low_power_mode(). > - Added comments, updated module description. > - Removed Reviewed-by and Acked-by on the spm and cpuidle-qcom patches. I think this series should also include updates to qcom_defconfig that enable QCOM_PM and the cpuidle driver. Then it will be enabled by default and get more broad testing. Kevin -- 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 v12 03/10] qcom: spm: Add Subsystem Power Manager driver
Lina Iyer writes: > SPM is a hardware block that controls the peripheral logic surrounding > the application cores (cpu/l$). When the core executes WFI instruction, > the SPM takes over the putting the core in low power state as > configured. The wake up for the SPM is an interrupt at the GIC, which > then completes the rest of low power mode sequence and brings the core > out of low power mode. > > The SPM has a set of control registers that configure the SPMs > individually based on the type of the core and the runtime conditions. > SPM is a finite state machine block to which a sequence is provided and > it interprets the bytes and executes them in sequence. Each low power > mode that the core can enter into is provided to the SPM as a sequence. > > Configure the SPM to set the core (cpu or L2) into its low power mode, > the index of the first command in the sequence is set in the SPM_CTL > register. When the core executes ARM wfi instruction, it triggers the > SPM state machine to start executing from that index. The SPM state > machine waits until the interrupt occurs and starts executing the rest > of the sequence until it hits the end of the sequence. The end of the > sequence jumps the core out of its low power mode. > > Add support for an idle driver to set up the SPM to place the core in > Standby or Standalone power collapse mode when the core is idle. > > Based on work by: Mahesh Sivasubramanian , > Ai Li , Praveen Chidambaram > Original tree available at - > git://codeaurora.org/quic/la/kernel/msm-3.10.git > > Signed-off-by: Lina Iyer [...] > diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c > new file mode 100644 > index 000..e5d3ebd > --- /dev/null > +++ b/drivers/soc/qcom/spm.c > @@ -0,0 +1,328 @@ > +/* Copyright (c) 2011-2014, The Linux Foundation. All rights reserved. nit: one more multi-line coding style missed (no need to repost just for this, feel free to just updated it locally) After that, feel free to add: Reviewed-by: Kevin Hilman -- 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 v10 07/10] qcom: cpuidle: Add cpuidle driver for QCOM cpus
Daniel Lezcano writes:
> On 11/21/2014 07:03 PM, Lina Iyer wrote:
>> Add cpuidle driver interface to allow cpus to go into C-States. Use the
>> cpuidle DT interface, common across ARM architectures, to provide the
>> idle state information to the cpuidle framework.
>>
>> Supported modes at this time are Standby and Standalone Power Collapse.
>>
>> Signed-off-by: Lina Iyer
>
> One nit and one comment below. Other than that:
>
> Acked-by: Daniel Lezcano
>
> [ ... ]
>
>> +static int qcom_cpu_stby(struct cpuidle_device *dev,
>> +struct cpuidle_driver *drv, int index)
>> +{
>> +lpm_ops->standby(NULL);
>
> In my last comment I was referring about a check for entering
> successfully the idle state:
>
> if (lpm_ops->standby(NULL))
> return -1;
>
>> +return index;
>> +}
>> +
>> +static int qcom_cpu_spc(struct cpuidle_device *dev,
>> +struct cpuidle_driver *drv, int index)
>> +{
>> +lpm_ops->spc(NULL);
>> +
>> +return index;
Similar to Daniel's comment above. if lpm_ops->spc() fails, do you want
to fall back to standby.
Hmm, using the DT idle states, it doesn't look as straight forward as it
used to be to fall back to a "safe state."
Kevin
--
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 v9 2/9] qcom: spm: Add Subsystem Power Manager driver
Oops, I thought I had sent this, but it was sitting in the drafts
folder. Sending anyways because it looks like most of these issues
still exist in v10.
Lina Iyer writes:
> SPM is a hardware block that controls the peripheral logic surrounding
> the application cores (cpu/l$). When the core executes WFI instruction,
> the SPM takes over the putting the core in low power state as
> configured. The wake up for the SPM is an interrupt at the GIC, which
> then completes the rest of low power mode sequence and brings the core
> out of low power mode.
>
> The SPM has a set of control registers that configure the SPMs
> individually based on the type of the core and the runtime conditions.
> SPM is a finite state machine block to which a sequence is provided and
> it interprets the bytes and executes them in sequence. Each low power
> mode that the core can enter into is provided to the SPM as a sequence.
>
> Configure the SPM to set the core (cpu or L2) into its low power mode,
> the index of the first command in the sequence is set in the SPM_CTL
> register. When the core executes ARM wfi instruction, it triggers the
> SPM state machine to start executing from that index. The SPM state
> machine waits until the interrupt occurs and starts executing the rest
> of the sequence until it hits the end of the sequence. The end of the
> sequence jumps the core out of its low power mode.
>
> Add support for an idle driver to set up the SPM to place the core in
> Standby or Standalone power collapse mode when the core is idle.
>
> Based on work by: Mahesh Sivasubramanian ,
> Ai Li , Praveen Chidambaram
> Original tree available at -
> git://codeaurora.org/quic/la/kernel/msm-3.10.git
>
> Signed-off-by: Lina Iyer
A few coding-style, readablity comments below...
> ---
> .../devicetree/bindings/arm/msm/qcom,saw2.txt | 31 +-
> drivers/soc/qcom/Kconfig | 8 +
> drivers/soc/qcom/Makefile | 1 +
> drivers/soc/qcom/spm.c | 334
> +
> include/soc/qcom/pm.h | 31 ++
> 5 files changed, 399 insertions(+), 6 deletions(-)
> create mode 100644 drivers/soc/qcom/spm.c
> create mode 100644 include/soc/qcom/pm.h
>
> diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
> b/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
> index 1505fb8..690c3c0 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 are supported using these Device
> Nodes.
> +SAW2 revisions differ in the register offset and configuration data. Also,
> the
> +same revision of the SAW in different SoCs may have different configuration
> +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:
> @@ -14,10 +23,13 @@ PROPERTIES
> Value type:
> Definition: shall contain "qcom,saw2". A more specific value should be
> one of:
> - "qcom,saw2-v1"
> - "qcom,saw2-v1.1"
> - "qcom,saw2-v2"
> - "qcom,saw2-v2.1"
> + "qcom,saw2-v1"
> + "qcom,saw2-v1.1"
> + "qcom,saw2-v2"
> + "qcom,saw2-v2.1"
> + "qcom,apq8064-saw2-v1.1-cpu"
> + "qcom,msm8974-saw2-v2.1-cpu"
> + "qcom,apq8084-saw2-v2.1-cpu"
>
> - reg:
> Usage: required
> @@ -26,10 +38,17 @@ PROPERTIES
> the register region. An optional second element specifies
> the base address and size of the alias register region.
>
> +- regulator:
> + Usage: optional
> + Value type: boolean
> + Definition: Indicates that this SPM device acts as a regulator device
> + device for the core (CPU or Cache) the SPM is attached
> + to.
>
> Example:
>
> - regulator@2099000 {
> + power-controller@2099000 {
> compa
Re: [PATCH] tty: serial: msm_serial: Don't require DT aliases
Greg, On Fri, Nov 14, 2014 at 10:39 AM, Stephen Boyd wrote: > If there isn't a DT alias then of_alias_get_id() will return > -ENODEV. This will cause the msm_serial driver to fail probe, > when we want to keep the previous behavior where we generated a > dynamic line number at probe time. Restore this behavior by > generating a dynamic id if the line number is still negative > after checking for an alias or in the non-DT case looking at the > .id field of the platform device. > > Reported-by: Kevin Hilman > Tested-by: Kevin Hilman > Signed-off-by: Stephen Boyd Can you add this to tty-next. Without this, all the ARM/qcom boards do not boot in linux-next. Thanks, Kevin -- 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] tty: serial: msm_serial: Use DT aliases
Stephen Boyd writes: > On 11/13/2014 04:46 PM, Frank Rowand wrote: >> On 11/13/2014 11:31 AM, Stephen Boyd wrote: >>> Sorry, I'm sort of lost. If there are serial aliases in the dts file, >>> then we should alias all of the serial ports. If there aren't aliases >>> then we're backwards compatible with the dts we have now and we'll do >>> dynamic generation. Putting code into the driver to validate that >>> this is true is not the job of the driver. If anything, it should >>> validated when the dts file is created. If one day we screw up and >>> have a dts file with such a bad configuration we'll have to work >>> around it, but until that day comes I'd rather not think about it. >> Maybe I did not understand when you said "Perhaps we should use an ida". >> That sentence led me to think the driver should check for misconfiguration. >> The case I was trying to handle was if there was at least one serialN >> alias and at least one UART without an alias. For example, if there >> are three UARTs (serial_a, serial_b, serial_c, probed in that order) >> and one alias (serial0 = &serial_c;) then the result would be: >> >>serial_a line 0 (from msm_uart_next_id) >>serial_b line 1 (from msm_uart_next_id) >>serial_c line 0 (from the alias) >> >>Two UARTs probed with line == 0. This is an error. >> >> Most of the serial drivers don't check for this type of bad configuration. >> Some drivers keep a bit map of which lines have been used. I'm not sure >> what they do in case of a conflict (I did not read to that level of detail). >> >> I thought you were suggesting the driver check for the bad configuration, >> so I was proposing a somewhat simple way of forcing a boot error for the >> bad configuration. >> >> Since you are not suggesting the driver check for the bad configuration, >> you can ignore my proposal. I agree that it is ok for the driver to >> expect the board dts to be correct. The problem should be detected by >> the dts author on first boot as part of normal bring up testing, and >> then corrected. >> > > Ah ok. I was just saying we could use an ida instead of an atomic > increment so that this driver works properly with driver > binding/unbinding, otherwise the line number keeps increasing and > quickly goes beyond the static array of ports (which I still don't > understand why we have at all btw). Due to the length of the thread, I haven't followed all the details, and I suspect Greg hasn't either, so I'm not sure if you're discssuing what the right fix is for what's in -next (still broken[1], or what should be done with the device board files. If the fix from earlier in this thread is still the right one for fixing -next, could you repost it separately for Greg to queue/squash and for me to re-test (if needed.) Thanks, Kevin [1] http://lists.linaro.org/pipermail/kernel-build-reports/2014-November/006298.html -- 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] tty: serial: msm_serial: Use DT aliases
Stephen Boyd writes: > On 11/10/2014 10:54 AM, Kevin Hilman wrote: >> On Wed, Oct 22, 2014 at 5:33 PM, Stephen Boyd wrote: >>> We rely on probe order of this driver to determine the line number for >>> the uart port. This makes it impossible to know the line number >>> when these devices are populated via DT. Use the DT alias >>> mechanism to assign the line based on the aliases node. >>> >>> Signed-off-by: Stephen Boyd >> FYI... this patch hit linux-next and caused multiple boot failures on >> qcom platforms[1] as of next-20141110. I'm assuming this is because >> the corresponding DTS changes have not hit linux-next yet. >> >> Kevin >> >> [1] http://status.armcloud.us/boot/?qcom > > Hmm the intention was to make it optional so that dts changes aren't > necessary unless you want deterministic numbering. I screwed that up > badly :/ Thanks for finding this. > > Greg, can you also apply this patch or squash it into the bad one? > > 8<- > > From: Stephen Boyd > Subject: [PATCH] tty: serial: msm_serial: Don't required DT aliases > > If there isn't a DT alias then of_alias_get_id() will return > -ENODEV. This will cause the msm_serial driver to fail probe, > when we want to keep the previous behavior where we generated a > dynamic line number at probe time. Restore this behavior by > generating a dynamic id if the line number is still negative > after checking for an alias or (in the non-DT case) looking at the > .id field of the platform device. > > Reported-by: Kevin Hilman > Signed-off-by: Stephen Boyd Tested-by: Kevin Hilman I confirm that this patch gets things booting again for the msm8974/xperia-z1 and the apq8064/ifc6410. Kevin -- 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] tty: serial: msm_serial: Use DT aliases
On Wed, Oct 22, 2014 at 5:33 PM, Stephen Boyd wrote: > We rely on probe order of this driver to determine the line number for > the uart port. This makes it impossible to know the line number > when these devices are populated via DT. Use the DT alias > mechanism to assign the line based on the aliases node. > > Signed-off-by: Stephen Boyd FYI... this patch hit linux-next and caused multiple boot failures on qcom platforms[1] as of next-20141110. I'm assuming this is because the corresponding DTS changes have not hit linux-next yet. Kevin [1] http://status.armcloud.us/boot/?qcom -- 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 v3] ARM: dts: Add initial DTS file for Sony Xperia Z1 phone
Hi Kumar, Kumar Gala writes: > Will not make the 3.18 merge window. I will queue it up for 3.19. Yes, it's too late for the merge window proper, but since it's DT only support for new platform, you can submit it as a fix to us (arm-soc) for v3.18-rc. In fact, if you chose (and if it's known to work for v3.17 and/or earlier), you can even mark it for stable. Greg has said he's OK with DT-only changes like this that only affect a specific platform to go into stable. Kevin -- 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 0/4] ARM: dts: apq8064 DT patches
Srinivas Kandagatla writes: > These patches adds dt nodes for RPM, USB and SATA IPs found in APQ8064. > All the driver support for these IPs are already available in v3.17, except > the RPM driver which is now accepted by Lee and Mark. > > These patches depend on the header file from > http://www.spinics.net/lists/arm-kernel/msg364310.html > for regulators. > > Now that the RPM patch is accepted, should RPM patch go via arm-soc tree? > so that we can get more functionality on IFC6410 board in v3.18 mainline. > > All these patches are tested on IFC6410 board. > Am hoping to get these patches for v3.18, as these patches were sitting in > my dev tree since last 3 merge windows. A nit on process, for future reference: You only need to send to a...@kernel.org if you're expecting the arm-soc maintainers to directly apply the patches or pull request. In this case, it seems like stuff that the qcom maintainers will queue up in their DT branch, and then submit to arm-soc maintainers. Kevin -- 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 v7 5/7] qcom: cpuidle: Add cpuidle driver for QCOM cpus
Nicolas Pitre writes: > On Tue, 30 Sep 2014, Kevin Hilman wrote: > >> On Tue, Sep 30, 2014 at 10:51 AM, Nicolas Pitre >> wrote: >> > On Tue, 30 Sep 2014, Kevin Hilman wrote: >> > >> >> Lorenzo Pieralisi writes: [...] >> >> > This may be misleading. Call it PlatformWFI or something like that, not >> >> > WFI if >> >> > that's not what it is. >> >> >> >> This gets at a little pet peeve of mine: >> >> >> >> IMO, naming any state with "WFI" is a bit confusing, because typically >> >> *every* idle state is entered by one (or more) CPU executing WFI, no? >> > >> > Agreed. >> > >> > The only state called "WFI" should be the one that only executes the WFI >> > instruction without any other hardware setup around it. >> >> Well, I would go even further in that none of the states should be >> called WFI, because WFI is used to enter all of them. > > Fair enough. > > So let's fix this by finding a name for that state that consists of only > executing WFI and that every SOC has. > > Suggestions? The DT idle-states binding doc (though seemingly written more with arm64 and SBSA in mind) uses "standby" for the shallowest idle. Kevin -- 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 v7 5/7] qcom: cpuidle: Add cpuidle driver for QCOM cpus
On Tue, Sep 30, 2014 at 10:51 AM, Nicolas Pitre wrote: > On Tue, 30 Sep 2014, Kevin Hilman wrote: > >> Lorenzo Pieralisi writes: >> >> > Hi Lina, >> > >> > On Sat, Sep 27, 2014 at 01:58:13AM +0100, Lina Iyer wrote: >> >> Add cpuidle driver interface to allow cpus to go into C-States. Use the >> >> cpuidle DT interface common across ARM architectures to provide the >> >> C-State information to the cpuidle framework. >> >> >> >> Supported modes at this time are clock gating (wfi) and cpu power down >> >> (Standalone PC or spc). >> >> >> >> Signed-off-by: Lina Iyer >> >> --- >> >> .../bindings/arm/msm/qcom,idle-state.txt | 72 + >> >> drivers/cpuidle/Kconfig.arm| 7 ++ >> >> drivers/cpuidle/Makefile | 1 + >> >> drivers/cpuidle/cpuidle-qcom.c | 89 >> >> ++ >> >> 4 files changed, 169 insertions(+) >> >> create mode 100644 >> >> Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt >> >> create mode 100644 drivers/cpuidle/cpuidle-qcom.c >> >> >> >> diff --git >> >> a/Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt >> >> b/Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt >> >> new file mode 100644 >> >> index 000..47095b9 >> >> --- /dev/null >> >> +++ b/Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt >> >> @@ -0,0 +1,72 @@ >> >> +QCOM Idle States for cpuidle driver >> >> + >> >> +ARM provides idle-state node to define the cpuidle states, as defined in >> >> [1]. >> >> +cpuidle-qcom is the cpuidle driver for Qualcomm SoCs and uses these idle >> >> +states. Idle states have different enter/exit latency and residency >> >> values. >> >> +The idle states supported by the QCOM SoC are defined as - >> >> + >> >> +* WFI >> >> +* Retention >> >> +* Standalone Power Collapse (Standalone PC or SPC) >> >> +* Power Collapse (PC) >> >> + >> >> +WFI: WFI does a little more in addition to architectural clock gating. >> >> ARM >> > >> > This may be misleading. Call it PlatformWFI or something like that, not >> > WFI if >> > that's not what it is. >> >> This gets at a little pet peeve of mine: >> >> IMO, naming any state with "WFI" is a bit confusing, because typically >> *every* idle state is entered by one (or more) CPU executing WFI, no? > > Agreed. > > The only state called "WFI" should be the one that only executes the WFI > instruction without any other hardware setup around it. Well, I would go even further in that none of the states should be called WFI, because WFI is used to enter all of them. Kevin -- 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 v7 5/7] qcom: cpuidle: Add cpuidle driver for QCOM cpus
Lorenzo Pieralisi writes: > Hi Lina, > > On Sat, Sep 27, 2014 at 01:58:13AM +0100, Lina Iyer wrote: >> Add cpuidle driver interface to allow cpus to go into C-States. Use the >> cpuidle DT interface common across ARM architectures to provide the >> C-State information to the cpuidle framework. >> >> Supported modes at this time are clock gating (wfi) and cpu power down >> (Standalone PC or spc). >> >> Signed-off-by: Lina Iyer >> --- >> .../bindings/arm/msm/qcom,idle-state.txt | 72 + >> drivers/cpuidle/Kconfig.arm| 7 ++ >> drivers/cpuidle/Makefile | 1 + >> drivers/cpuidle/cpuidle-qcom.c | 89 >> ++ >> 4 files changed, 169 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt >> create mode 100644 drivers/cpuidle/cpuidle-qcom.c >> >> diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt >> b/Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt >> new file mode 100644 >> index 000..47095b9 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt >> @@ -0,0 +1,72 @@ >> +QCOM Idle States for cpuidle driver >> + >> +ARM provides idle-state node to define the cpuidle states, as defined in >> [1]. >> +cpuidle-qcom is the cpuidle driver for Qualcomm SoCs and uses these idle >> +states. Idle states have different enter/exit latency and residency values. >> +The idle states supported by the QCOM SoC are defined as - >> + >> +* WFI >> +* Retention >> +* Standalone Power Collapse (Standalone PC or SPC) >> +* Power Collapse (PC) >> + >> +WFI: WFI does a little more in addition to architectural clock gating. ARM > > This may be misleading. Call it PlatformWFI or something like that, not WFI if > that's not what it is. This gets at a little pet peeve of mine: IMO, naming any state with "WFI" is a bit confusing, because typically *every* idle state is entered by one (or more) CPU executing WFI, no? Kevin -- 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 v7 4/7] qcom: pm: Add cpu low power mode functions
Lina Iyer writes: > On Mon, Sep 29 2014 at 17:37 -0600, Stephen Boyd wrote: >>On 09/26/14 17:58, Lina Iyer wrote: >>> Based on work by many authors, available at codeaurora.org >>> >> >>> Signed-off-by: Lina Iyer >>> [lina: simplify the driver for an initial submission, add commit text >>> description of idle states] >> >>Maintainer tags don't really make sense unless there is another author. >> > Hmm.. Since this patch is a derivative work, I wanted to clarify, what > changed seems important. The work was done by many authors. Adding > signed-off from everybody who could have contributed to the patch > downstream is confusing. > I would be okay removing it. Yes, you should remove it, but move the "Based on work by many authors" part to just before your signoff. Also, it's considered good manners to name any significant contributors if there are a few standouts. Also, the "available at codeaurora.org" is wanting a link, ideally to a specific commit, or a branch so original code can easily be found. Kevin -- 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 v7 3/7] arm: dts: qcom: Add SPM device bindings for 8084
Lina Iyer writes: > Add SPM device bindings for QCOM 8084 based cpus. SPM is the sub-system > power manager and controls the logic around the cores (cpu and L2). > > Each core has an instance of SPM and controls only that core. Each cpu > SPM is configured to support WFI and SPC (standalone-power collapse). > > Signed-off-by: Lina Iyer nit: This will need some more description, IMO. The changelog talks about SPM bindings, but all the stuff added refers to SAW. Kevin -- 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 v7 1/7] qcom: spm: Add Subsystem Power Manager driver
Lina Iyer writes: > Based on work by many authors, available at codeaurora.org > > SPM is a hardware block that controls the peripheral logic surrounding > the application cores (cpu/l$). When the core executes WFI instruction, > the SPM takes over the putting the core in low power state as > configured. The wake up for the SPM is an interrupt at the GIC, which > then completes the rest of low power mode sequence and brings the core > out of low power mode. > > The SPM has a set of control registers that configure the SPMs > individually based on the type of the core and the runtime conditions. > SPM is a finite state machine block to which a sequence is provided and > it interprets the bytes and executes them in sequence. Each low power > mode that the core can enter into is provided to the SPM as a sequence. > > Configure the SPM to set the core (cpu or L2) into its low power mode, > the index of the first command in the sequence is set in the SPM_CTL > register. When the core executes ARM wfi instruction, it triggers the > SPM state machine to start executing from that index. The SPM state > machine waits until the interrupt occurs and starts executing the rest > of the sequence until it hits the end of the sequence. The end of the > sequence jumps the core out of its low power mode. > > Signed-off-by: Lina Iyer [...] > diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt > b/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt > index 1505fb8..9a9cc99 100644 > --- a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt > +++ b/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt > @@ -14,10 +14,9 @@ PROPERTIES > Value type: > Definition: shall contain "qcom,saw2". A more specific value should be > one of: > - "qcom,saw2-v1" > - "qcom,saw2-v1.1" > - "qcom,saw2-v2" > - "qcom,saw2-v2.1" > + "qcom,apq8064-saw2-v1.1-cpu" > + "qcom,msm8974-saw2-v2.1-cpu" > + "qcom,apq8084-saw2-v2.1-cpu" This looks odd (to me.) Why are the SoC name and the SAW2 version both needed? Kevin -- 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 v6 1/5] qcom: spm: Add Subsystem Power Manager driver
Lina Iyer writes: > On Fri, Sep 26 2014 at 13:04 -0600, Kevin Hilman wrote: >>Lina Iyer writes: >> >>> On Wed, Sep 24 2014 at 12:07 -0600, Kumar Gala wrote: [...] >>>> >>>>General comment, lets use qcom instead of msm for various things. >>>> >>>>[snip] >>>> >>> OK, Done. I renamed all msm_ functions to qcom_ functions as well. >>> >> >>Does that apply to the other parts of this series too? like the msm-pm >>and cpuidle layers? >> > Yes, there are no more msm's anymore :) > What about Documentation/devicetree/bindings/arm/msm? -- 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 v6 1/5] qcom: spm: Add Subsystem Power Manager driver
Lina Iyer writes: > On Wed, Sep 24 2014 at 12:07 -0600, Kumar Gala wrote: >> >>On Sep 23, 2014, at 6:51 PM, Lina Iyer wrote: >> >>> Based on work by many authors, available at codeaurora.org >>> >>> SPM is a hardware block that controls the peripheral logic surrounding >>> the application cores (cpu/l$). When the core executes WFI instruction, >>> the SPM takes over the putting the core in low power state as >>> configured. The wake up for the SPM is an interrupt at the GIC, which >>> then completes the rest of low power mode sequence and brings the core >>> out of low power mode. >>> >>> The SPM has a set of control registers that configure the SPMs >>> individually based on the type of the core and the runtime conditions. >>> SPM is a finite state machine block to which a sequence is provided and >>> it interprets the bytes and executes them in sequence. Each low power >>> mode that the core can enter into is provided to the SPM as a sequence. >>> >>> Configure the SPM to set the core (cpu or L2) into its low power mode, >>> the index of the first command in the sequence is set in the SPM_CTL >>> register. When the core executes ARM wfi instruction, it triggers the >>> SPM state machine to start executing from that index. The SPM state >>> machine waits until the interrupt occurs and starts executing the rest >>> of the sequence until it hits the end of the sequence. The end of the >>> sequence jumps the core out of its low power mode. >>> >>> Signed-off-by: Lina Iyer >>> [lina: simplify the driver for initial submission, clean up and update >>> commit text] >>> --- >>> Documentation/devicetree/bindings/arm/msm/spm.txt | 43 +++ >>> drivers/soc/qcom/Kconfig | 8 + >>> drivers/soc/qcom/Makefile | 1 + >>> drivers/soc/qcom/spm.c| 388 >>> ++ >>> include/soc/qcom/spm.h| 38 +++ >>> 5 files changed, 478 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/arm/msm/spm.txt >>> create mode 100644 drivers/soc/qcom/spm.c >>> create mode 100644 include/soc/qcom/spm.h >> >>General comment, lets use qcom instead of msm for various things. >> >>[snip] >> > OK, Done. I renamed all msm_ functions to qcom_ functions as well. > Does that apply to the other parts of this series too? like the msm-pm and cpuidle layers? Kevin -- 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 v6 1/5] qcom: spm: Add Subsystem Power Manager driver
Lina Iyer writes:
> Based on work by many authors, available at codeaurora.org
>
> SPM is a hardware block that controls the peripheral logic surrounding
> the application cores (cpu/l$). When the core executes WFI instruction,
> the SPM takes over the putting the core in low power state as
> configured. The wake up for the SPM is an interrupt at the GIC, which
> then completes the rest of low power mode sequence and brings the core
> out of low power mode.
>
> The SPM has a set of control registers that configure the SPMs
> individually based on the type of the core and the runtime conditions.
> SPM is a finite state machine block to which a sequence is provided and
> it interprets the bytes and executes them in sequence. Each low power
> mode that the core can enter into is provided to the SPM as a sequence.
>
> Configure the SPM to set the core (cpu or L2) into its low power mode,
> the index of the first command in the sequence is set in the SPM_CTL
> register. When the core executes ARM wfi instruction, it triggers the
> SPM state machine to start executing from that index. The SPM state
> machine waits until the interrupt occurs and starts executing the rest
> of the sequence until it hits the end of the sequence. The end of the
> sequence jumps the core out of its low power mode.
>
> Signed-off-by: Lina Iyer
[...]
> +enum {
> + MSM_SPM_REG_SAW2_CFG,
> + MSM_SPM_REG_SAW2_AVS_CTL,
> + MSM_SPM_REG_SAW2_AVS_HYSTERESIS,
> + MSM_SPM_REG_SAW2_SPM_CTL,
> + MSM_SPM_REG_SAW2_PMIC_DLY,
> + MSM_SPM_REG_SAW2_AVS_LIMIT,
> + MSM_SPM_REG_SAW2_AVS_DLY,
> + MSM_SPM_REG_SAW2_SPM_DLY,
> + MSM_SPM_REG_SAW2_PMIC_DATA_0,
> + MSM_SPM_REG_SAW2_PMIC_DATA_1,
> + MSM_SPM_REG_SAW2_PMIC_DATA_2,
> + MSM_SPM_REG_SAW2_PMIC_DATA_3,
> + MSM_SPM_REG_SAW2_PMIC_DATA_4,
> + MSM_SPM_REG_SAW2_PMIC_DATA_5,
> + MSM_SPM_REG_SAW2_PMIC_DATA_6,
> + MSM_SPM_REG_SAW2_PMIC_DATA_7,
> + MSM_SPM_REG_SAW2_RST,
> +
> + MSM_SPM_REG_NR_INITIALIZE = MSM_SPM_REG_SAW2_RST,
> +
> + MSM_SPM_REG_SAW2_ID,
> + MSM_SPM_REG_SAW2_SECURE,
> + MSM_SPM_REG_SAW2_STS0,
> + MSM_SPM_REG_SAW2_STS1,
> + MSM_SPM_REG_SAW2_STS2,
> + MSM_SPM_REG_SAW2_VCTL,
> + MSM_SPM_REG_SAW2_SEQ_ENTRY,
> + MSM_SPM_REG_SAW2_SPM_STS,
> + MSM_SPM_REG_SAW2_AVS_STS,
> + MSM_SPM_REG_SAW2_PMIC_STS,
> + MSM_SPM_REG_SAW2_VERSION,
> +
> + MSM_SPM_REG_NR,
> +};
Do you really need the MSM_SPM_REG_ prefix on all of these? As these
names are all local to the driver, the extra prefix isn't needed IMO.
Also, consdiering that SPM seems to be sub-block of SAW2, the fact that
the SPM shows up twice in some of the names is a bit confusing.
Kevin
--
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] soc: qcom: do not disable the iface clock in probe
Srinivas Kandagatla writes:
> On 24/09/14 16:22, Kevin Hilman wrote:
>> Srinivas Kandagatla writes:
>>
>>> since commit 31964ffebbb9 ("tty: serial: msm: Remove direct access to
>>> GSBI")'
>>> serial hangs if earlyprintk are enabled.
>>>
>>> This hang is noticed only when the GSBI driver is probed and all the
>>> earlyprintks before gsbi probe are seen on the console.
>>> The reason why it hangs is because GSBI driver disables hclk in its
>>> probe function without realizing that the serial IP might be in use by
>>> a bootconsole. As gsbi driver disables the clock in probe the
>>> bootconsole locks up.
>>
>>> Turning off hclk's could be dangerous if there are system components
>>> like earlyprintk using the hclk.
>>
>> This seems rather fragile. Isn't the right fix for these other
>> components to use the clk api to ensure the clock does not get enabled?
> Here we are depending on the bootloader to setup the clocks.
>
> Am not sure if we can really use clk apis at that early stage of bootstrap.
Not that early, but all that matters is that you clk_enable() sometime
before the GSBI driver calls clk_disable().
If the clocks are always enabled by the bootloader, the platforms clock
driver might check for those and ensure they are enabled, so that linux
is aware of it, and bugs like this one are avoided.
IMO, clock enable/disable shuffling in the GSBI driver to avoid UART
console problems is just papering over a bug that's waiting to pop up
elsewhere.
Kevin
--
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] ARM: dts: Add initial DTS file for Sony Xperia Z1
Hi Tim, Tim Bird writes: > This DTS has support for the Sony Xperia Z1 phone (codenamed Honami). > This first version of the DTS supports just a serial console. > > changes from v1 > - removed qcom,msm-id attribute > - removed mis-capitalized Qualcomm word > - changed dts file name > - added board-specific compatible-string > > Signed-off-by: Tim Bird [...] > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile > index b8c5cd3..0c2702e 100644 > --- a/arch/arm/boot/dts/Makefile > +++ b/arch/arm/boot/dts/Makefile > @@ -343,7 +343,8 @@ dtb-$(CONFIG_ARCH_QCOM) += \ > qcom-apq8074-dragonboard.dtb \ > qcom-apq8084-mtp.dtb \ > qcom-msm8660-surf.dtb \ > - qcom-msm8960-cdp.dtb > + qcom-msm8960-cdp.dtb \ > + qcom-msm8974-sony-honami-xperia.dtb This doesn't compile becase the filename added is xperia-honami (not honami-xperia) After fixing that up, I was able to test it on my Z1 (Thanks Tim/Bjorn!) but found I needed to add in the qcom,msm-id stuff first. I guess that will be remedied with the updated mkbootimg tool, correct? Anyways, after fixing the filename, feel free to add: Tested-by: Kevin Hilman Kevin -- 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] soc: qcom: do not disable the iface clock in probe
Srinivas Kandagatla writes:
> since commit 31964ffebbb9 ("tty: serial: msm: Remove direct access to GSBI")'
> serial hangs if earlyprintk are enabled.
>
> This hang is noticed only when the GSBI driver is probed and all the
> earlyprintks before gsbi probe are seen on the console.
> The reason why it hangs is because GSBI driver disables hclk in its
> probe function without realizing that the serial IP might be in use by
> a bootconsole. As gsbi driver disables the clock in probe the
> bootconsole locks up.
> Turning off hclk's could be dangerous if there are system components
> like earlyprintk using the hclk.
This seems rather fragile. Isn't the right fix for these other
components to use the clk api to ensure the clock does not get enabled?
Kevin
--
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 v1] soc: qcom: do not disable the iface clock in probe
On Tue, Sep 23, 2014 at 10:42 AM, Kumar Gala wrote: > > On Aug 15, 2014, at 12:41 AM, Srinivas Kandagatla > wrote: > >> The use case here is when we have a bootconsole which is printing the >> characters on serial console and gsbi driver comes up after some time. >> As gsbi driver disables the clock in probe the bootconsole locks up. >> >> This patch fixes the problem by disabling the clock in platform remove >> rather than in probe. >> >> Tested-by: Linus Walleij >> Signed-off-by: Srinivas Kandagatla >> --- >> Changes since RFC: >> - none, just a subject line change. > > Srini, > > We need some changes to the commit message for this to get into v3.17. > > Probably some references to: > > http://www.spinics.net/lists/linux-arm-msm/msg10589.html > > Adding Kevin to provide details on what else should be in commit message > w/regards to the regression we introduced. At this point of the cycle, we're rather strict about what qualifies as a fix. It should be fixing a specific regression. Looking at the above link, it looks like it is, so could you re-spin this with changelog, summarizing the regression and how this fixes the regression, and we should still be able to get it in for v3.17. Also, it looks like the original was only posted to the msm list. For future reference, for patches that need to go via the arm-soc maintainers, make sure to Cc a...@kernel.org. Thanks, Kevin -- 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 v5 7/7] arm: dts: qcom: Add idle states device nodes for 8974
Lina Iyer writes:
> Add allowable C-States for each cpu using the cpu-idle-states node.
> ARM spec dictates WFI as the default idle state at 0. Support standalone
> power collapse (power down that does not affect any SoC idle states) for
> each cpu.
>
> Signed-off-by: Lina Iyer
[...]
> @@ -55,6 +59,22 @@
> cache-level = <2>;
> qcom,saw = <&saw_l2>;
> };
> +
> + idle-states {
> + CPU_WFI: cpu-idle-state-0 {
Minor nit:
This name is what's being used to populate the idle state's ->name field
in the CPUidle driver, which in turn is used by tools like powertop to
display state names, etc.
So first, that name field has to be short (it currently gets truncated,
dropping the zero), and second that field should be more descriptive,
like 'WFI', so viewing in the various tools shows something useful.
Thanks,
Kevin
--
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 v3 1/8] msm: scm: Move scm-boot files to drivers/soc and include/soc
Lina Iyer writes: > On Wed, Aug 27, 2014 at 10:18:24AM -0700, Kevin Hilman wrote: >>Lina Iyer writes: >> >>> Signed-off-by: Lina Iyer >> >>Why the move? It looks like scm-boot.c still calls into the main >>scm_call() stuff that still lives under arch/arm/mach-qcom, and not in >>arm64, so I'm not seeing the motiviation for the change (and the >>changelog didn't help explain either :) >> >>Kevin > > I dint intend on moving the scm-boot for 64 bit purposes, but that > would be a good argument :) My motivation was to move the scm-boot.h, but > figured might as well clean up the mach-qcom in the process, per > guidance. Thanks for clarifying. I suspect that should be a separate cleanup series with its own descriptive changelog, because to me it only looks like a partial cleanup since only some of the stuff was moved out, and it's not abvious what it will be sharing that code. Kevin -- 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 v4 7/8] qcom: cpuidle: Add cpuidle driver for QCOM cpus
Lina Iyer writes: > On Thu, Aug 21, 2014 at 03:24:44AM +0200, Daniel Lezcano wrote: >>On 08/20/2014 12:15 AM, Lina Iyer wrote: [...] >> >> So IIUC, if you specify the index 1, that means the state[0] will be >> the default WFI. But you override the callback below in the loop. >> >> I recommend you use the default arm wfi callback but you implement >> the cpu_do_idle for your platform. > > Yes, it was intended. I dont want to define two WFI states. The > architectural WFI does not buy us enough compared to WFI that SoC can > do. L2 can go into low power modes when the core is in WFI and for that > I would like to have all WFI's enter SoC framework. If the L2 is going into low-power, that means there will be higher latency coming out compared to the architectural WFI, correct? If you have both, and the latency/residency numbers are accurate, the governor is then left to pick the right one. So, what's wrong with having both? Kevin -- 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 v3 1/8] msm: scm: Move scm-boot files to drivers/soc and include/soc
Lina Iyer writes:
> Signed-off-by: Lina Iyer
Why the move? It looks like scm-boot.c still calls into the main
scm_call() stuff that still lives under arch/arm/mach-qcom, and not in
arm64, so I'm not seeing the motiviation for the change (and the
changelog didn't help explain either :)
Kevin
> arch/arm/mach-qcom/Makefile | 1 -
> arch/arm/mach-qcom/platsmp.c| 2 +-
> drivers/soc/qcom/Makefile | 2 +-
> {arch/arm/mach-qcom => drivers/soc/qcom}/scm-boot.c | 4 ++--
> {arch/arm/mach-qcom => include/soc/qcom}/scm-boot.h | 0
> 5 files changed, 4 insertions(+), 5 deletions(-)
> rename {arch/arm/mach-qcom => drivers/soc/qcom}/scm-boot.c (97%)
> rename {arch/arm/mach-qcom => include/soc/qcom}/scm-boot.h (100%)
>
> diff --git a/arch/arm/mach-qcom/Makefile b/arch/arm/mach-qcom/Makefile
> index db41e8c..e324375 100644
> --- a/arch/arm/mach-qcom/Makefile
> +++ b/arch/arm/mach-qcom/Makefile
> @@ -1,3 +1,2 @@
> obj-y:= board.o
> obj-$(CONFIG_SMP)+= platsmp.o
> -obj-$(CONFIG_QCOM_SCM) += scm-boot.o
> diff --git a/arch/arm/mach-qcom/platsmp.c b/arch/arm/mach-qcom/platsmp.c
> index d690856..a692bcb 100644
> --- a/arch/arm/mach-qcom/platsmp.c
> +++ b/arch/arm/mach-qcom/platsmp.c
> @@ -20,7 +20,7 @@
>
> #include
>
> -#include "scm-boot.h"
> +#include
>
> #define VDD_SC1_ARRAY_CLAMP_GFS_CTL 0x35a0
> #define SCSS_CPU1CORE_RESET 0x2d80
> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> index a39446d..70d52ed 100644
> --- a/drivers/soc/qcom/Makefile
> +++ b/drivers/soc/qcom/Makefile
> @@ -1,3 +1,3 @@
> obj-$(CONFIG_QCOM_GSBI) += qcom_gsbi.o
> CFLAGS_scm.o :=$(call as-instr,.arch_extension sec,-DREQUIRES_SEC=1)
> -obj-$(CONFIG_QCOM_SCM) += scm.o
> +obj-$(CONFIG_QCOM_SCM) += scm.o scm-boot.o
> diff --git a/arch/arm/mach-qcom/scm-boot.c b/drivers/soc/qcom/scm-boot.c
> similarity index 97%
> rename from arch/arm/mach-qcom/scm-boot.c
> rename to drivers/soc/qcom/scm-boot.c
> index 5add20e..60ff7b4 100644
> --- a/arch/arm/mach-qcom/scm-boot.c
> +++ b/drivers/soc/qcom/scm-boot.c
> @@ -17,9 +17,9 @@
>
> #include
> #include
> -#include
>
> -#include "scm-boot.h"
> +#include
> +#include
>
> /*
> * Set the cold/warm boot address for one of the CPU cores.
> diff --git a/arch/arm/mach-qcom/scm-boot.h b/include/soc/qcom/scm-boot.h
> similarity index 100%
> rename from arch/arm/mach-qcom/scm-boot.h
> rename to include/soc/qcom/scm-boot.h
--
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 v3 0/3] Qualcomm Resource Power Manager driver
Kumar Gala writes: > On Jun 18, 2014, at 10:53 AM, Kevin Hilman wrote: > >> Bjorn Andersson writes: >> >>> On Tue, Jun 17, 2014 at 10:07 AM, Kevin Hilman wrote: >>>> +Paul Walmsley >>>> >>>> Bjorn Andersson writes: >>>> >>>>> This series adds a regulator driver for the Resource Power Manager found >>>>> in >>>>> Qualcomm 8660, 8960 and 8064 based devices. >>>>> >>>>> The RPM driver exposes resources to its child devices, that can be >>>>> accessed to >>>>> implement drivers for the regulators, clocks and bus frequency control >>>>> that's >>>>> owned by the RPM in these devices. >>>>> >>>>> Changes since v2: >>>>> - Fix copy-paste error in dt binding >>>>> - Correct incomplete move from mfd to soc >>>>> - Correct const mistake in regulator driver >>>>> >>>>> Changes since v1: >>>>> - Moved rpm driver to drivers/soc >>>> >>>> I'm not sure I follow the motivation for having this under drivers/soc? >>>> >>> Hi Kevin, >>> >>> I've made the argument that to me this is conceptually a black box >>> handling regulators, clocks and other stuff; hence similar to a PMIC, >>> which would fit nicely into drivers/mfd. >>> >>> I still think this is the case and now that I look back I didn't get >>> any pushback from Lee Jones so maybe the move was premature? >> >> Yes, IMO, the move was premature, but hopefully the drivers/soc folks >> can chime in an clarify the criteria for inclusion there. >> >> Kevin > > I dont agree, I think having this in drivers/soc means that we can > clearly go through drivers/soc in the future and look for patterns > across SoCs that should be re-factored. I don't believe that was the goal in creating drivers/soc. > Where MFD seems like its become the new drivers misc. Well, I don't think that drivers/soc wants to be the new drivers/misc either. ;) Thinking more about what this RPM driver actually does, and since you mentioned patterns across SoCs, it seems to me the RPM driver bascially just doing the IPC. So, rather than MFD or drivers/soc, it seems to me that it should be implmented as a controller in the new common mailbox framwork[1] being worked on by Jassi Brar (added to Cc.) IIUC, RPM is actually only doing one-way IPC (it only exposes a write() interface to clients) so it seems like a rather simple implementation of a mailbox controller. I believe Bjorn is already on the list of folks Cc'd on the common mailbox framework, so it would be good to hear from him why RPM wouldn't fit under that framework. Thanks, Kevin [1] https://lkml.org/lkml/2014/6/12/470 -- 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 v3 0/3] Qualcomm Resource Power Manager driver
Bjorn Andersson writes: > On Tue, Jun 17, 2014 at 10:07 AM, Kevin Hilman wrote: >> +Paul Walmsley >> >> Bjorn Andersson writes: >> >>> This series adds a regulator driver for the Resource Power Manager found in >>> Qualcomm 8660, 8960 and 8064 based devices. >>> >>> The RPM driver exposes resources to its child devices, that can be accessed >>> to >>> implement drivers for the regulators, clocks and bus frequency control >>> that's >>> owned by the RPM in these devices. >>> >>> Changes since v2: >>> - Fix copy-paste error in dt binding >>> - Correct incomplete move from mfd to soc >>> - Correct const mistake in regulator driver >>> >>> Changes since v1: >>> - Moved rpm driver to drivers/soc >> >> I'm not sure I follow the motivation for having this under drivers/soc? >> > Hi Kevin, > > I've made the argument that to me this is conceptually a black box > handling regulators, clocks and other stuff; hence similar to a PMIC, > which would fit nicely into drivers/mfd. > > I still think this is the case and now that I look back I didn't get > any pushback from Lee Jones so maybe the move was premature? Yes, IMO, the move was premature, but hopefully the drivers/soc folks can chime in an clarify the criteria for inclusion there. Kevin -- 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 v3 0/3] Qualcomm Resource Power Manager driver
+Paul Walmsley Bjorn Andersson writes: > This series adds a regulator driver for the Resource Power Manager found in > Qualcomm 8660, 8960 and 8064 based devices. > > The RPM driver exposes resources to its child devices, that can be accessed to > implement drivers for the regulators, clocks and bus frequency control that's > owned by the RPM in these devices. > > Changes since v2: > - Fix copy-paste error in dt binding > - Correct incomplete move from mfd to soc > - Correct const mistake in regulator driver > > Changes since v1: > - Moved rpm driver to drivers/soc I'm not sure I follow the motivation for having this under drivers/soc? In fact, my (possibly incorrect) understanding of this Resource Power Manager is that it's actually to manage communication/control of resources that are actually handled off the SoC by a microcontroller. I added Paul W. to Cc so he can correct me, but I think this is a ways off from the goal of what drivers/soc was for. Kevin -- 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] mmc: sdhci-msm: Fix fallout from sdhci refactoring
Stephen Boyd writes: > The sdhci core was refactored recently and some of those > refactorings required changes in every sdhci platform driver. > Those updates happened around the same time as when the msm > driver was merged so the refactorings missed the msm driver. > Hook in the basic library functions so that we can boot apq8074 > dragonboards again instead of crashing when we try to jump to > NULL function pointers. > > Reported-by: Kevin Hilman > Cc: Georgi Djakov > Cc: Russell King > Signed-off-by: Stephen Boyd Tested-by: Kevin Hilman I verifed that this patch on top of next-20140610 gets the qcom dragonboard booting again. Kevin -- 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] ARM: qcom: Enable basic support for Qualcomm platforms in multi_v7_defconfig
Kevin Hilman writes: > Stephen Boyd writes: > >> On 02/28, Kevin Hilman wrote: >>> Hi Kumar, >>> >>> Kevin Hilman writes: >>> >>> > Kumar Gala writes: >>> > >>> >> Enable support for the MSM8x60, MSM8960, and MSM8974 SoCs, clocks and >>> >> serial console as part of the standard multi_v7_defconfig. >>> >> >>> >> Signed-off-by: Kumar Gala >>> >> --- >>> >> >>> >> I leave this to the arm-soc guys to apply because of possible conflicts >>> >> with other updates to multi_v7_defconfig. >>> > >>> > Seem to be a handful of !qcom changes here, probably resulting from a >>> > run of 'make savedefconfig' without all the new changes in. >>> > >>> > I've thinned this out to only the qcom changes and applied the following >>> > to next/boards. I think it's better to do any 'make savedefconfig' >>> > cleanups as a separate, cleanup-only patch as needed. >>> >>> Bisecting a new boot failure on u8500/snowball came down to this patch. >>> Simply disabling CONFIG_ARCH_QCOM in mutli_v7_defconfig gets the >>> u8500/snowball booting again, so it looks like there may still be some >>> qcom specifics that are being run on non-qcom platforms with a >>> multi-platform config. >>> >>> Could you audit for initcalls/drivers being initialized on non-qcom >>> SoCs? >>> >> >> Can you try disabling CONFIG_ARCH_MSM8X60 and >> CONFIG_ARCH_MSM8960? MY suspicion is the text offset movement is >> causing problems, but I hope I'm wrong. > > Sorry to dash your hopes, but you're right. > > Starting from a fresh multi_v7_defconfig and disabling those two Kconfig > options gets the snowball booting again on multi_v7_defconfig. For the record... After some discussion on IRC, we discovered this is because those options add a 2Mb offset to TEXT_OFFSET, and my boot tool wasn't taking that into account when creating the uImage to boot on snowball. I guess it's time to upgrade u-boot on snowball with bootz support so it can directly boot a zImage. Kevin -- 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] ARM: qcom: Enable basic support for Qualcomm platforms in multi_v7_defconfig
Stephen Boyd writes: > On 02/28, Kevin Hilman wrote: >> Hi Kumar, >> >> Kevin Hilman writes: >> >> > Kumar Gala writes: >> > >> >> Enable support for the MSM8x60, MSM8960, and MSM8974 SoCs, clocks and >> >> serial console as part of the standard multi_v7_defconfig. >> >> >> >> Signed-off-by: Kumar Gala >> >> --- >> >> >> >> I leave this to the arm-soc guys to apply because of possible conflicts >> >> with other updates to multi_v7_defconfig. >> > >> > Seem to be a handful of !qcom changes here, probably resulting from a >> > run of 'make savedefconfig' without all the new changes in. >> > >> > I've thinned this out to only the qcom changes and applied the following >> > to next/boards. I think it's better to do any 'make savedefconfig' >> > cleanups as a separate, cleanup-only patch as needed. >> >> Bisecting a new boot failure on u8500/snowball came down to this patch. >> Simply disabling CONFIG_ARCH_QCOM in mutli_v7_defconfig gets the >> u8500/snowball booting again, so it looks like there may still be some >> qcom specifics that are being run on non-qcom platforms with a >> multi-platform config. >> >> Could you audit for initcalls/drivers being initialized on non-qcom >> SoCs? >> > > Can you try disabling CONFIG_ARCH_MSM8X60 and > CONFIG_ARCH_MSM8960? MY suspicion is the text offset movement is > causing problems, but I hope I'm wrong. Sorry to dash your hopes, but you're right. Starting from a fresh multi_v7_defconfig and disabling those two Kconfig options gets the snowball booting again on multi_v7_defconfig. Kevin -- 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] ARM: qcom: Enable basic support for Qualcomm platforms in multi_v7_defconfig
Hi Kumar, Kevin Hilman writes: > Kumar Gala writes: > >> Enable support for the MSM8x60, MSM8960, and MSM8974 SoCs, clocks and >> serial console as part of the standard multi_v7_defconfig. >> >> Signed-off-by: Kumar Gala >> --- >> >> I leave this to the arm-soc guys to apply because of possible conflicts >> with other updates to multi_v7_defconfig. > > Seem to be a handful of !qcom changes here, probably resulting from a > run of 'make savedefconfig' without all the new changes in. > > I've thinned this out to only the qcom changes and applied the following > to next/boards. I think it's better to do any 'make savedefconfig' > cleanups as a separate, cleanup-only patch as needed. Bisecting a new boot failure on u8500/snowball came down to this patch. Simply disabling CONFIG_ARCH_QCOM in mutli_v7_defconfig gets the u8500/snowball booting again, so it looks like there may still be some qcom specifics that are being run on non-qcom platforms with a multi-platform config. Could you audit for initcalls/drivers being initialized on non-qcom SoCs? Thanks, Kevin -- 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] ARM: qcom: Enable basic support for Qualcomm platforms in multi_v7_defconfig
Kumar Gala writes: > Enable support for the MSM8x60, MSM8960, and MSM8974 SoCs, clocks and > serial console as part of the standard multi_v7_defconfig. > > Signed-off-by: Kumar Gala > --- > > I leave this to the arm-soc guys to apply because of possible conflicts > with other updates to multi_v7_defconfig. Seem to be a handful of !qcom changes here, probably resulting from a run of 'make savedefconfig' without all the new changes in. I've thinned this out to only the qcom changes and applied the following to next/boards. I think it's better to do any 'make savedefconfig' cleanups as a separate, cleanup-only patch as needed. Kevin >From 60476e4ad5a1369467ab8a061ae8ce601478646d Mon Sep 17 00:00:00 2001 From: Kumar Gala Date: Tue, 25 Feb 2014 14:34:05 -0600 Subject: [PATCH] ARM: qcom: Enable basic support for Qualcomm platforms in multi_v7_defconfig Enable support for the MSM8x60, MSM8960, and MSM8974 SoCs, clocks and serial console as part of the standard multi_v7_defconfig. Signed-off-by: Kumar Gala [khilman: removed non-qcom changes] Signed-off-by: Kevin Hilman --- arch/arm/configs/multi_v7_defconfig | 10 ++ 1 file changed, 10 insertions(+) diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig index 56549f178fe3..64e850d4896c 100644 --- a/arch/arm/configs/multi_v7_defconfig +++ b/arch/arm/configs/multi_v7_defconfig @@ -33,6 +33,10 @@ CONFIG_SOC_OMAP5=y CONFIG_SOC_AM33XX=y CONFIG_SOC_DRA7XX=y CONFIG_SOC_AM43XX=y +CONFIG_ARCH_QCOM=y +CONFIG_ARCH_MSM8X60=y +CONFIG_ARCH_MSM8960=y +CONFIG_ARCH_MSM8974=y CONFIG_ARCH_ROCKCHIP=y CONFIG_ARCH_SOCFPGA=y CONFIG_PLAT_SPEAR=y @@ -148,6 +152,8 @@ CONFIG_SERIAL_SIRFSOC_CONSOLE=y CONFIG_SERIAL_TEGRA=y CONFIG_SERIAL_IMX=y CONFIG_SERIAL_IMX_CONSOLE=y +CONFIG_SERIAL_MSM=y +CONFIG_SERIAL_MSM_CONSOLE=y CONFIG_SERIAL_VT8500=y CONFIG_SERIAL_VT8500_CONSOLE=y CONFIG_SERIAL_OF_PLATFORM=y @@ -298,6 +304,10 @@ CONFIG_MFD_NVEC=y CONFIG_KEYBOARD_NVEC=y CONFIG_SERIO_NVEC_PS2=y CONFIG_NVEC_POWER=y +CONFIG_COMMON_CLK_QCOM=y +CONFIG_MSM_GCC_8660=y +CONFIG_MSM_MMCC_8960=y +CONFIG_MSM_MMCC_8974=y CONFIG_TEGRA_IOMMU_GART=y CONFIG_TEGRA_IOMMU_SMMU=y CONFIG_MEMORY=y -- 1.8.3 -- 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: Split Qualcomm support into legacy and multiplatform
Kumar Gala writes: > On Feb 26, 2014, at 8:22 AM, Kevin Hilman wrote: > >> Kumar Gala writes: >> >>> Introduce a new mach-qcom that will support SoCs that intend to be >>> multiplatform compatiable while keeping mach-msm to legacy SoC/board >>> support that will not transition over to multiplatform. >>> >>> As part of this, we move support for MSM8X60, MSM8960 and MSM8974 over >>> to mach-qcom. >>> >>> Signed-off-by: Kumar Gala >> >> Having a closer look at this after seeing the multi_v7_defconfig change, >> I have a minor nit on the new ARCH_QCOM name. >> >> For new additions, we've been trying to move towards using SOC_foo >> instead of ARCH_foo. Any reason not to do that here also? >> >> Kevin > > No reason, just wasn’t aware and no one said anything til now. Yeah, sorry for not reviewiing sooner. > Hmm, how to handle this now, can we do this as a 3.16 cleanup? Yes, we've already merged it, so doing it as a cleanup should be fine. Thanks, Kevin -- 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: Split Qualcomm support into legacy and multiplatform
Kumar Gala writes: > Introduce a new mach-qcom that will support SoCs that intend to be > multiplatform compatiable while keeping mach-msm to legacy SoC/board > support that will not transition over to multiplatform. > > As part of this, we move support for MSM8X60, MSM8960 and MSM8974 over > to mach-qcom. > > Signed-off-by: Kumar Gala Having a closer look at this after seeing the multi_v7_defconfig change, I have a minor nit on the new ARCH_QCOM name. For new additions, we've been trying to move towards using SOC_foo instead of ARCH_foo. Any reason not to do that here also? Kevin -- 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] Enable clock controllers on MSM
Stephen Boyd writes: > These patches add the clock controller nodes, enable the clock drivers > on MSM based platforms, and hook it up enough to get the serial console > working. This is based on the merge of Mike's clk-next branch with > linux-next-20140116. The changes need the clk-next branch because that's > where the DTS include files landed. I forgot to repond to this earlier, but I tested this on top of -next and it gets the dragonboard booting w/mainline. Yay! > Perhaps this can be applied after 3.14-rc1 is out? Yeah, sounds good. Kevin > Stephen Boyd (2): > ARM: dts: msm: Add clock controller nodes and hook into uart > ARM: msm_defconfig: Enable MSM clock drivers > > arch/arm/boot/dts/qcom-msm8660-surf.dts | 11 +++ > arch/arm/boot/dts/qcom-msm8960-cdp.dts | 18 ++ > arch/arm/boot/dts/qcom-msm8974.dtsi | 24 > arch/arm/configs/msm_defconfig | 4 > 4 files changed, 57 insertions(+) -- 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] usb: phy: msm: fix compilation errors when !CONFIG_PM_SLEEP
Josh Cartwright writes:
> Both the PM_RUNTIME and PM_SLEEP callbacks call into the common
> msm_otg_{suspend,resume} routines, however these routines are only being
> built when CONFIG_PM_SLEEP. In addition, msm_otg_{suspend,resume} also
> depends on msm_hsusb_config_vddcx(), which is only built when
> CONFIG_PM_SLEEP.
>
> Fix the CONFIG_PM_RUNTIME, !CONFIG_PM_SLEEP case by changing the
> preprocessor conditional, and moving msm_hsusb_config_vddcx().
>
> While we're here, eliminate the CONFIG_PM conditional for setting
> up the dev_pm_ops.
>
> This address the following errors Russell King has hit doing randconfig
> builds:
>
> drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_suspend':
> drivers/usb/phy/phy-msm-usb.c:1691:2: error: implicit declaration of function
> 'msm_otg_suspend'
> drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_resume':
> drivers/usb/phy/phy-msm-usb.c:1699:2: error: implicit declaration of function
> 'msm_otg_resume'
>
> Cc: Ivan T. Ivanov
> Reported-by: Russell King
> Signed-off-by: Josh Cartwright
[...]
> @@ -440,7 +414,32 @@ static int msm_otg_reset(struct usb_phy *phy)
> #define PHY_SUSPEND_TIMEOUT_USEC (500 * 1000)
> #define PHY_RESUME_TIMEOUT_USEC (100 * 1000)
>
> -#ifdef CONFIG_PM_SLEEP
> +#if defined(CONFIG_PM_SLEEP) || defined(CONFIG_PM_RUNTIME)
nit: you should just use CONFIG_PM here since that is what it's for.
c.f. kernel/power/Kconfig:
config PM
def_bool y
depends on PM_SLEEP || PM_RUNTIME
Kevin
--
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] ARM: msm_defconfig: Update for multi-platform
Stephen Boyd writes: > ARCH_MSM is a hidden config option now so this defconfig needs to > be updated to select ARCH_MSM_DT instead. While we're here, > remove dead symbols (SSBI), drop selected symbols (ZRELADDR, > PHYLIB, USB_PHY) and enable the MSM random driver (HW_RANDOM_MSM). > > Cc: Kevin Hilman > Signed-off-by: Stephen Boyd Applied to next/soc, merged into for-next and pushed out. Thanks for the quick fixup. Kevin -- 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: msm: Remove 7x00 support
Daniel Walker writes: > On Thu, Oct 31, 2013 at 10:12:03AM -0700, Kevin Hilman wrote: >> Daniel Walker writes: >> >> >> No. The idea behind splitting them is to allow current platforms with >> active maintainers to progress without being held back. The older >> platforms can stay and have an opportunity to modernize. >> >> The kernel is a moving target, without some minimal effort to keep >> platforms up to date, the effort to continue to maintain/modernize them >> can become more of a pain than it's worth. If maintainers of these older >> platforms are willing to put in the work, nobody will be SOL. If >> nobody shows interest in modernizing these older platforms (which seems >> to be the case based on the last couple years), then it is reasonable >> IMO for them to fade away slowly. > > > According to a prior email Tony suggested that OMAP was split for purely > technical reasons.. If code is shared in some way , or has synergies, and > there's no > technical reason to split a sub-architecture, then to me there's no win in > splitting > things.. The wins have already been well described in this thread in terms of maintenance of newer platforms using modern kernel infrastructure. > It's just more directories, more confusion etc.. The confusion > would come from someone wanting to find the code related to a platform, > but woops there's a bunch of directories, or code flow and how the > sub-architecture is strung together .. Personally I found OMAP very > confusing in that regard. > > ARM and the sub-architectures is already confusing I don't think we need > to start compounding the problem by allowing random whatever-you-want > sub-directories from every sub-architecture. Randomness is quite a bit of an exaggeration of what's been proposed here. These decisions are made on a case-by-case basis and is this case is being done for ease of maintainence for newer platforms, which may not be a "technical reason" for you, but is important for overall maintenance of arm-soc. If we do this split, you are more than welcome to demonstrate the commonality by modernizing mach-msm, combining it with mach-qcom, removing mach-msm, and then removing all the "confusion." Kevin -- 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: msm: Remove 7x00 support
Daniel Walker writes: > On Wed, Oct 30, 2013 at 04:08:27PM -0700, Kevin Hilman wrote: >> Olof Johansson writes: >> >> > I would be very happy to take more code for the older Qualcomm chipset >> > to enable full functionality for them, but it's been my impression >> > that far from all that is needed to make it a useful platform is in >> > the upstream kernel, and there's been no signs of more of it showing >> > up at least in the last two years. >> > >> > So we have a bit of a stalemate here -- the current Qualcomm team >> > wants to avoid having to deal too much with the legacy platforms -- >> > they are technically quite different from the current platforms and >> > the divergence makes it hard to deal with supporting it all in a >> > modern way without risking regressions. I tend to agree with them. >> >> As do I. >> >> > Just like omap split between omap1 and omap2plus, I think it's a time >> > to create a mach-qcom instead, and move the modern (v7, most likely) >> > platforms there -- enable them with device tree, modern framework >> > infrastructure, etc. That way you can keep older platforms in mach-msm >> > without risk of regressions, and they have a clean base to start on >> > with their later platforms. >> >> I think this split approach is a good compromise. >> >> If the maintainers of the current older platforms wish to bring them up >> to modern frameworks, we can consider combining again. If not, they the >> older platforms will take the same path as the rest of the older >> platforms that slowly fade away. >> > > So the current users of those platforms are, what SOL ? No. The idea behind splitting them is to allow current platforms with active maintainers to progress without being held back. The older platforms can stay and have an opportunity to modernize. The kernel is a moving target, without some minimal effort to keep platforms up to date, the effort to continue to maintain/modernize them can become more of a pain than it's worth. If maintainers of these older platforms are willing to put in the work, nobody will be SOL. If nobody shows interest in modernizing these older platforms (which seems to be the case based on the last couple years), then it is reasonable IMO for them to fade away slowly. Kevin -- 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: msm: Remove 7x00 support
Olof Johansson writes: > I would be very happy to take more code for the older Qualcomm chipset > to enable full functionality for them, but it's been my impression > that far from all that is needed to make it a useful platform is in > the upstream kernel, and there's been no signs of more of it showing > up at least in the last two years. > > So we have a bit of a stalemate here -- the current Qualcomm team > wants to avoid having to deal too much with the legacy platforms -- > they are technically quite different from the current platforms and > the divergence makes it hard to deal with supporting it all in a > modern way without risking regressions. I tend to agree with them. As do I. > Just like omap split between omap1 and omap2plus, I think it's a time > to create a mach-qcom instead, and move the modern (v7, most likely) > platforms there -- enable them with device tree, modern framework > infrastructure, etc. That way you can keep older platforms in mach-msm > without risk of regressions, and they have a clean base to start on > with their later platforms. I think this split approach is a good compromise. If the maintainers of the current older platforms wish to bring them up to modern frameworks, we can consider combining again. If not, they the older platforms will take the same path as the rest of the older platforms that slowly fade away. Kevin -- 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: [GIT PULL] msm cleanups for v3.13
David Brown writes: > The following changes since commit 4a10c2ac2f368583138b774ca41fac4207911983: > > Linux 3.12-rc2 (2013-09-23 15:41:09 -0700) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/davidb/linux-msm.git > tags/msm-cleanup-for-3.13 > > for you to fetch changes up to 81cf1e061d001fab44dbaa85fc4fbfb6da6713a1: > > ARM: msm: Rename msm devicetrees to have standard 'qcom' prefix (2013-09-25 > 13:02:56 -0700) > > > General cleanups for MSM for 3.13 > > Some small cleanups for MSM. Removes extraneous irq definitions that > aren't used on DT targets, moves the single existing board file to > board-dt.c in antipication of additional targets, and renames the > existing DT files to have a common 'qcom' prefix. > > Pulled into next/cleanup, Thanks, Kevin -- 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 v4 05/17] arch_timer: Move to generic sched_clock framework
Stephen Boyd writes:
> Register with the generic sched_clock framework now that it
> supports 64 bits. This fixes two problems with the current
> sched_clock support for machines using the architected timers.
> First off, we don't subtract the start value from subsequent
> sched_clock calls so we can potentially start off with
> sched_clock returning gigantic numbers. Second, there is no
> support for suspend/resume handling so problems such as discussed
> in 6a4dae5 (ARM: 7565/1: sched: stop sched_clock() during
> suspend, 2012-10-23) can happen without this patch. Finally, it
> allows us to move the sched_clock setup into drivers clocksource
> out of the arch ports.
>
> Cc: Christopher Covington
> Cc: Catalin Marinas
> Signed-off-by: Stephen Boyd
A boot failure on Exynos5/Arndale showed up in next-20131014[1], and a
subsequent bisect has fingered this patch as the culprit.
I haven't had a chance to debug this any further, but wanted to share in
case someone else can debug.
The console log is below, but don't think there is much useful there as
it shows nothing after the 'Starting kernel ...' from u-boot.
Kevin
[1]
http://lists.linaro.org/pipermail/kernel-build-reports/2013-October/000651.html
Connected to arndale console [channel connected] (~$quit to exit)
(user:khilman) is already connected
~$hardreset
Command(arndale console)> hardreset
(user:khilman) Reboot arndale
Reboot: arndale ; phidget 2 0 : off, sleep, on
U-Boot 2013.01.-rc1-dirty (Jun 28 2013 - 07:14:48) for ARNDALE5250
CPU:Exynos5250@1000MHz
Board: for ARNDALE5250
I2C: ready
DRAM: 2 GiB
WARNING: Caches not enabled
Checking Boot Mode ... SDMMC
MMC: EXYNOS DWMMC: 0, EXYNOS DWMMC: 1, EXYNOS DWMMC: 2
In:serial
Out: serial
Err: serial
Net: No ethernet found.
(Re)start USB...
USB0: USB EHCI 1.00
scanning bus 0 for devices... 4 USB Device(s) found
scanning usb for storage devices... 0 Storage Device(s) found
scanning usb for ethernet devices... 1 Ethernet Device(s) found
Hit any key to stop autoboot:
3 0
ARNDALE5250 # version
version
U-Boot 2013.01.-rc1-dirty (Jun 28 2013 - 07:14:48) for ARNDALE5250
arm-linux-gnueabi-gcc (Ubuntu/Linaro 4.7.2-1ubuntu1) 4.7.2
GNU ld (GNU Binutils for Ubuntu) 2.22.90.20120919
ARNDALE5250 # setenv bootargs console=tty0 console=ttySAC2,115200n8 rw
root=/dev/mmcblk1p3 rootwait rootfstype=ext4
setenv bootargs console=tty0 console=ttySAC2,115200n8 rw root=/dev/mmcblk1p3
rootwait rootfstype=ext4
ARNDALE5250 # setenv netargs 'setenv bootargs ${bootargs}
ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}192.168.1.254:none'
setenv netargs 'setenv bootargs ${bootargs}
ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}192.168.1.254:none'
ARNDALE5250 # if test -n ${initenv}; then run initenv; fi
if test -n ${initenv}; then run initenv; fi
ARNDALE5250 # if test -n ${preboot}; then run preboot; fi
if test -n ${preboot}; then run preboot; fi
(Re)start USB...
USB0: USB EHCI 1.00
scanning bus 0 for devices... 4 USB Device(s) found
scanning usb for storage devices... 0 Storage Device(s) found
scanning usb for ethernet devices... 1 Ethernet Device(s) found
ARNDALE5250 #setenv autoload no; setenv autoboot no
setenv autoload no; setenv autoboot no
ARNDALE5250 # dhcp
dhcp
Waiting for Ethernet connection... done.
BOOTP broadcast 1
DHCP client bound to address 192.168.1.171
ARNDALE5250 # setenv serverip 192.168.1.2
setenv serverip 192.168.1.2
ARNDALE5250 # if test -n ${netargs}; then run netargs; fi
if test -n ${netargs}; then run netargs; fi
ARNDALE5250 # tftp 0x4080 tmp/arndale-lNb7OG/zImage
tftp 0x4080 tmp/arndale-lNb7OG/zImage
Waiting for Ethernet connection... done.
Using asx0 device
TFTP from server 192.168.1.2; our IP address is 192.168.1.171
Filename 'tmp/arndale-lNb7OG/zImage'.
Load address: 0x4080
Loading: *#
#
###
done
Bytes transferred = 2709912 (295998 hex)
ARNDALE5250 # tftp 0x407c tmp/arndale-lNb7OG/exynos5250-arndale.dtb
tftp 0x407c tmp/arndale-lNb7OG/exynos5250-arndale.dtb
Waiting for Ethernet connection... done.
Using asx0 device
TFTP from server 192.168.1.2; our IP address is 192.168.1.171
Filename 'tmp/arndale-lNb7OG/exynos5250-arndale.dtb'.
Load address: 0x407c
Loading: *###
done
Bytes transferred = 32949 (80b5 hex)
ARNDALE5250 # printenv bootargs
printenv bootargs
bootargs=console=tty0 console=ttySAC2,115200n8 rw root=/dev/mmcblk1p3 rootwait
rootfstype=ext4
ip=192.168.1.171:192.168.1.2:192.168.1.254:255.255.255.0192.168.1.254:none
ARNDALE5250 # bootz 0x4080 - 0x407c
bootz 0x4080 - 0x407c
## Flattened Device Tree blob at 407c
Booting using the fdt blob at 0x407c
Using Device Tree in place at 407c, end 407cb0b4
Starting kernel ...
~$off
# PYBOOT: Exception: kernel: ERROR
Re: [GIT PULL v2] msm cleanups for v3.12
David Brown writes: > The following changes since commit 3b2f64d00c46e1e4e9bd0bb9bb12619adac27a4b: > > Linux 3.11-rc2 (2013-07-21 12:05:29 -0700) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/davidb/linux-msm.git > tags/msm-cleanup3-for-3.12 > > for you to fetch changes up to 3294a7e7c3a55bb4eb9d98b0af5643a7ea44afe1: > > ARM: msm: Only compile io.c on platforms that use it (2013-08-06 11:18:04 > -0700) > > > General cleanups for MSM for 3.12 > > This is a series of cleanups to the MSM code. These are a healthy > start to bringing the MSM code closer to the state of current ARM code > and getting more of it ready for conversion to devicetree. Pulled. Thanks for the nice cleanups and getting closer to multi-platform support. Kevin -- 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: Shared regulator usage
Anirudh Ghayal writes:
> On 11/26/2012 7:17 PM, Mark Brown wrote:
>> On Mon, Nov 26, 2012 at 05:13:37AM -0800, agha...@codeaurora.org wrote:
>>
>>> For example:
>>> Consumer (A) cpu-freq sets the voltage range to {1.275v, 1.375v}. The
>>> regulator framework eventually sets the regulator to 1.275v. Consumer (B)
>>> recommends a lower the voltage say (1.25v) and executes set_volatge on
>>> {1.25v, 1.25v}. This is where regulator_check_consumers() fails as it does
>>> not meet the (A)'s constraint.
>>
>> Well, of course. What else would you expect to happen in this case?
>>
>>> We are looking for the right way to handle such a situation using
>>> regulator framework, considering this to be a valid usecase. One way we
>>> could think of is having one of the driver (say cpu-freq) register a
>>> virtual regulator device and have the other driver be its consumer. This
>>> way all the regulator calls to the physical hardware will be routed though
>>> one primary driver which can take care of the adjustments. The problem
>>> with such approach would be scalability for a new consumer, i.e. adding
>>> another consumer for the primary driver would present a similar problem as
>>> the original one.
>>
>> Why not just fix your consumers to request the voltage ranges they
>> actually want? Clearly in your above example one of the consumers can
>> support a wider voltage range than it is actually requesting so it
>> should just request that voltage range.
>>
> Mark,
>
> Let me try to explain this further.
>
> Both our cpu-freq and the core-power-reduction (CPR) driver manipulate
> the voltage to the same physical device (apps core) and both these
> drivers are independent. The cpu-freq driver has predefined static
> mappings and accordingly sets the nominal voltage
> based on the target frequency. The CPR then does micro adjustments to
> this voltage making it more optimal to save power. This is very
> similar to the SmartReflex design on the OMAP
> (http://omappedia.org/wiki/SR_Voltage_Control_Migration,
> http://lists.linuxfoundation.org/pipermail/linux-pm/2012-April/034117.html),
> the only difference being that in our case both the consumer drivers
> use the regulator framework to manipulate the voltage. I suppose on
> OMAP they handle such a case inside the OMAP-specific voltage layer.
>
> A typical example in our case is --
>
> cpu-freq mappings
> <1.4Ghz, 1.3v>
> <1.2Ghz, 1.2v>
> <1.1Ghz, 1.1v>
>
> At 1.4Ghz the cpu-freq driver votes for 1.3v, then the CPR kicks in
> and recommends a voltage of 1.275v. Now a set_voltage with this new
> level (1.275v, 1.275) fails as it does not satisfy the limits of the
> cpu-freq driver. It is not possible to tweak this range any further as
> it would not achieve the goal of micro-adjusting the voltage to save
> power.
>
> Such scenarios are very likely to occur in the future on embedded
> systems where there is a need to conserve power by introducing some
> adaptive voltage scaling techniques based on various parameters such
> as temperature/sensitivity.
FWIW, you're correct about the design on OMAP.
We handle AVS below the regulator framework. CPUfreq manages "nominal"
voltages, and requests those from the OMAP voltage layer. If AVS is
enabled, then the voltage layer (actually, the SmartReflex driver)
overrides the nominal voltage with the actual voltage.
Kevin
--
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 11/13] ARM: gpio: consolidate trivial gpiolib implementations
Russell King - ARM Linux writes: > Consolidate 24 trivial gpiolib implementions out of mach/gpio.h > into asm/gpio.h. This is basically the include of asm-generic/gpio.h > and the definition of gpio_get_value, gpio_set_value, and gpio_cansleep > as described in Documentation/gpio.txt > > Signed-off-by: Russell King For OMAP: Tested-by: Kevin Hilman Acked-by: Kevin Hilman Also goes for patches 12 & 13 also. Kevin -- 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 03/13] ARM: gpio: omap: convert drivers to use asm/gpio.h rather than mach/gpio.h
Russell King - ARM Linux writes: > Signed-off-by: Russell King Acked-by: Kevin Hilman -- 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/40] Complete set of clocksource/sched_clock patches
Russell King - ARM Linux writes: > Here is the entire set of clocksource and sched_clock patches which > have been previously posted. There's a couple of small tweaks in a > few of the patches (such as adding notrace to OMAP clocksource read > functions), and this also shows the proper ordering of these patches. > > Still looking for acks or tested-by's for these patches. Tested-by: Kevin Hilman on a few davinci platforms. Kevin -- 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] pm_qos: Add system bus performance parameter
mark gross writes: > On Fri, Aug 27, 2010 at 07:55:37PM -0700, Saravana Kannan wrote: >> mark gross wrote: >> >On Fri, Aug 27, 2010 at 01:10:55AM -0700, skan...@codeaurora.org wrote: >> >>Ignoring other details for now, the biggest problem with throughput/KBps >> >>units is that PM QoS can't handle it well in its current state. For KBps >> >>the requests should be added together before it's "enforced". Just picking >> >>the maximum won't work optimally. >> > >> >well then current pm_qos code for network throughput takes the max. >> >> I don't know how the network throughput is enforced, but if the unit >> is KBps and it's just doing a Max, then I think it's broken. If two >> clients request 50 KBps and your network can go till 200 KBps, you >> would still be requesting 50 KBps when you could have requested 100 >> KBps. >> >> Any specific reason PM QoS doesn't support a "summation" "comparitor"? > > PM_QoS could do a summation, but keep in mind it pm_qos not qos. pm_qos > is a best effort thing to constrain power management throttling, not > provide a true quality of service or deadline scheduling support. For me (and I think Saravana too), this is still all about power, but it's closely tied to QoS. For things like busses, which are inherently shared, PM is tightly coupled with "true" QoS, so I'm not sure I fully follow the distinction being made between PM QoS and QoS. Seems like the tradeoff is always between power and performance. > If you stick to the full up quality of service mentality you quickly get > into discussions just like those around memory over commit. Its really > hard to know when best effort or hard QoS is appropriate. > > If you are trying to use pm_qos as a true qos interface then, its > definitely not up to the task. > > example: you have one 100Mb NIC in your box. With PM QoS you could > have 4 user mode applications requesting 100Mbs PM_Q0S. In this case > the right thing to do is to constrain the NIC PM to keep it full on and > the PHY going as fast as it can. But you'll never get 400Mbs out of the > thing. > > So far only max and min really have made sense for pm_qos but, if a case > pops up where summation makes more sense for aggregating the pm_qos > requests then I'm open to it. Using your example above, what if the 4 apps all request 10Mb/s? What is best effort? Leave the NIC in 10Mb/s mode, or bump up the power state to 100Mb/s mode? This decision is both QoS and PM related. Without summation, the 'max' request is still 10Mb/s so you would keep the lower power state. But you also know that none of the clients will get their requested rate. There's some gray area here since there is a choice. Was the point of the request to keep the NIC at the *power-state* needed for 10Mb/s (a PM request) or was the request saying the app wanted at least 10Mb/s (a QoS request.) My understanding is that PM QoS is intended to limit power-state throttling. IOW, in the absence of PM QoS requests, the PM core code is free to throttle the power of the devices/subsystems/busses etc. If requests are present, it is no longer free to throttle blindly. The question here seems to be whether or not the PM core code should also be free to increase the power state to meet a combination of PM QoS requests. To me this is still PM related. Just like in race-to-idle for the CPU, it might be better for overall power to go to the highter state for a burst and then be able to fully throttle again. Kevin -- 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] pm_qos: Add system bus performance parameter
Saravana Kannan writes:
> Some drivers/devices might need some minimum system bus performance to
> provide acceptable service. Provide a PM QoS parameter to send these requests
> to.
>
> The new parameter is named "system bus performance" since it is generic enough
> for the unit of the request to be frequency, bandwidth or something else that
> might be appropriate. It's up to each implementation of the QoS provider to
> define what the unit of the request would be.
>
> Signed-off-by: Saravana Kannan
With this current design, only one system-wide bus would be managed.
What if a platform has more than one independently scalable bus?
I think the only scalable way to handle this kind of thing is to have
per-device QoS constraints that can then be combined/aggregated by parent
devices/busses.
At LPC this year, I've proposed per-device QoS constraints[1] as a topic
for the PM mini-conf. I hope some folks from the MSM camp can be there
for these discussions.
Kevin
[1] http://www.linuxplumbersconf.org/2010/ocw/proposals/819
> ---
> kernel/pm_qos_params.c |9 +
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
> index 996a4de..1a44a67 100644
> --- a/kernel/pm_qos_params.c
> +++ b/kernel/pm_qos_params.c
> @@ -93,12 +93,21 @@ static struct pm_qos_object network_throughput_pm_qos = {
> .type = PM_QOS_MAX,
> };
>
> +static BLOCKING_NOTIFIER_HEAD(system_bus_performance_notifier);
> +static struct pm_qos_object system_bus_performance_pm_qos = {
> + .requests = PLIST_HEAD_INIT(system_bus_performance_pm_qos.requests,
> pm_qos_lock),
> + .notifiers = &system_bus_performance_notifier,
> + .name = "system_bus_performance",
> + .default_value = 0,
> + .type = PM_QOS_MAX,
> +};
>
> static struct pm_qos_object *pm_qos_array[] = {
> &null_pm_qos,
> &cpu_dma_pm_qos,
> &network_lat_pm_qos,
> &network_throughput_pm_qos
> + &system_bus_performance_pm_qos
> };
>
> static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf,
--
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/2] platform: Facilitate the creation of pseudo-platform buses
Grant Likely writes: >>> If the override is global to the platform_bus_type, then the model >>> will not scale. >> >> It will not scale any more (or less) than the current functionality of >> the driver model which handles this globally. Again, if scalabilty >> becomes a problem down the road, lets fix it then. > > Alright, I can agree to that. I do agree that the runtime override is > far and away better than the weak symbol approach. However, there > needs to be some very clear rules in place for users of the override, > namely that users must understand that they are stewards of the > platform_bus_type, and must take care to preserve the default > behaviour for "uninteresting" devices. Completely agree here. Any overrides of the dev_pm_ops functions that do not also call the pm_generic_* functions (or their equivalents) should be suspect. I'll post a proposal for a runtime override shortly. > Also, the expectation should be that it is a temporary measure until a > better abstraction is implemented. It might be that a separate > bus_type is the way to go (if the multiple driver registration problem > can be solved) or it might be a way to differentiate pm_ops either > per-device or per-parent. I'm not sure, but I'm starting on an OMAP > project soon, so I may very well end up working on this. Yes, I'll be glad to work on this too, but first I need to get through reviewing the backlog of all the OMAP drivers we're converting to runtime PM. Kevin -- 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/2] platform: Facilitate the creation of pseudo-platform buses
Grant Likely writes: [...] >>> >>> My fears on this point may very well be unfounded. This isn't the >>> hill I'm going to die on either. Show me an implementation of driver >>> sharing that is clean and prove me wrong! :-) >> >> IMHO, simply overriding the few dev_pm_ops methods was the cleanest and >> simplest. >> >> Since we seem to be in agreement now that the a new bus may not the >> right abstraction for this (since we want it to be completely >> transparent to the drivers), I'll go back to the original design. No new >> bus types, keep the platform_bus as is, but simply override the few >> dev_pm_ops methods I care about. This is what is done on SH, >> SH-Mobile[1] and my original version for OMAP that started this >> conversation. >> >> Yes, the weak-symbol method of overriding is not scalable, but that's a >> separate issue from whether or not to create a new bus. I have a >> proposed fix for the weak which I'll post shortly. > > Okay. > > One constraint remains though: If you can override the dev_pm_ops on > a per-device or per-device-parent basis, then you've got my support. hmm, a new requirement?, and one that would require some significant changes to the driver model. Currently, dev_pm_ops for a bus applies globally to *all* devices on that bus (or class or type) and changing that would require changing the platform_bus code to start having per-device (or per-parent-device) checks. > If the override (even when fixed to work at runtime) applies to every > device on the platform_bus_type, then I'll nack it. /me can't help but wonder why the OMAP implementation is getting all the negative attention while the other identical implementations are already upstream. > My concern here is that the SoC or platform support code doesn't get > to "own" the platform_bus_type. Well, I'm not proposing that here. This "feature" already exists in mainline (albeit using the less-than-optimal weak symbol approach.) All I'm proposing is fixing it to make it multi-arch friendly. If you think this behavior should be changed to non global, that should be done a separate series since it is not directly related to runtime PM support for a given platform, IMO. > Other drivers/code can register their own set of > platform_devices, which may also need to perform their own dev_pm_ops > override. IMHO, we should deal with such hypothetical, future problems *if* they arise down the road. > If the override is global to the platform_bus_type, then the model > will not scale. It will not scale any more (or less) than the current functionality of the driver model which handles this globally. Again, if scalabilty becomes a problem down the road, lets fix it then. Kevin -- 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/2] platform: Facilitate the creation of pseudo-platform buses
Grant Likely writes: [...] >> One of the primary goals of this (at least for me and it seems Magnus also) >> is to keep drivers ignorant of their bus type, and any bus-type-specific >> code should be done in the bus_type implementation. > > Heh; which just screams to me that bus_type is the wrong level to be > abstracting the behaviour Heh, now I feel like we're going around in circles. Remember, I never wanted to create add a new bus_type. Someone else [ahem] suggested doing the abstraction at the bus_type level. ;) > (but I also understand your need for the > "omap_device" wrapper around platform_device which also requires some > method to sort out when a platform_device really is an omap_device > without an unsafe dereference). Yes, I'm working on the devres approach to that now, as is Magnus for the sh-mobile version (proposed for .36-rc1[1]) >> Both for SH and OMAP, we've been using the (admiteddly broken) >> weak-symbol-override approach to getting a custom bus-type based on the >> platform_bus. We've been using that in OMAP for a while now and have >> not seen any need to for the drivers to know if they are on the vanilla >> platform_bus or the customized one. >> >> I'm very curious to hear what type of impact you expect to the drivers. > > My fears on this point may very well be unfounded. This isn't the > hill I'm going to die on either. Show me an implementation of driver > sharing that is clean and prove me wrong! :-) IMHO, simply overriding the few dev_pm_ops methods was the cleanest and simplest. Since we seem to be in agreement now that the a new bus may not the right abstraction for this (since we want it to be completely transparent to the drivers), I'll go back to the original design. No new bus types, keep the platform_bus as is, but simply override the few dev_pm_ops methods I care about. This is what is done on SH, SH-Mobile[1] and my original version for OMAP that started this conversation. Yes, the weak-symbol method of overriding is not scalable, but that's a separate issue from whether or not to create a new bus. I have a proposed fix for the weak which I'll post shortly. Kevin [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2010-August/022411.html -- 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/2] platform: Facilitate the creation of pseudo-platform buses
Grant Likely writes: > I'm not convinced (yet) that this is the right approach, and I'd like > to see a few sample drivers converted to the new approach. Creating > new bus_types that "inherit" from the platform_bus is actually not a > bad idea, and it is an elegant way to change the behaviour (for > example, how power management is implemented) for devices connected in > a different way. > > A problem with the approach that Kevin pointed out is that drivers > that need to work on both the platform_bus_type and the new > soc_bus_type must explicitly register themselves on both bus types. And potentially more than 2 if a driver exists for a hardware block on different SoCs. Magnus raised this issue for drivers used across SH and ARM/SH-Mobile, and the same issue exists between TI OMAP and TI DaVinci SoCs. > There is no mechanism to allow drivers from one bus type to also be > made available to another bus type. Certainly it would be possible to > invent a mechanism, but the more I think about it, them more I think > it will be a bad idea. The runtime-PM use-case that kicked this > discussion off makes the assumption that a driver will behave > identically when attached to either the platform_bus_type or the > soc_bus_type. However, I think that in the general case that > assumption will prove to be false. I strongly suspect that the new > bus type will turn out to be not as similar to the platform_bus_type > as originally assumed and that there will still be bus-type-specific > impact on device drivers What makes you suspect that? Maybe Patrick has other use-cases in mind, but from a PM perspective, I have not seen any impact. One of the primary goals of this (at least for me and it seems Magnus also) is to keep drivers ignorant of their bus type, and any bus-type-specific code should be done in the bus_type implementation. Both for SH and OMAP, we've been using the (admiteddly broken) weak-symbol-override approach to getting a custom bus-type based on the platform_bus. We've been using that in OMAP for a while now and have not seen any need to for the drivers to know if they are on the vanilla platform_bus or the customized one. I'm very curious to hear what type of impact you expect to the drivers. Kevin -- 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] platform: Facilitate the creation of pseduo-platform busses
Patrick Pannuto writes: >> >>> You had indicated that you were going on vacation for a month and I >>> had not seen any more follow-up on this issue, so I forged ahead. >> >> Great, I'm glad you forged ahead. There is definitely a broader need >> for something like this, and I have no personal attachment to the code. >> >> I have no problems with you continuing the work (in fact, I'd prefer it. >> I have lots of other things to catch up on after my vacation.) >> >> In the future though, it's common (and kind) to note the original author >> in the changelog when basing a patch on previous work. Something like >> "originally written by..." or "based on the work of..." etc. > > Ok, I can do that; that was the intention of the "original inspiration from" > line at the beginning. Maybe we need an 'Inspired-by:' tag. :) > Is there a more formal way of indicating this in the > next version of the patch? Should I add you as a "From:" or an "Author:"? I don't know if there is an official way of doing this. I typically add something like "based on the idea/code originally proposed/written by John Doe" in the changelog, and then add Cc: John Doe <...> in the changelog too, but AFAIK, none of this is formalized. Kevin -- 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] platform: Facilitate the creation of pseduo-platform busses
Patrick Pannuto writes:
> On 08/04/2010 05:16 PM, Kevin Hilman wrote:
>> Patrick Pannuto writes:
>>
>>> Inspiration for this comes from:
>>> http://www.mail-archive.com/linux-o...@vger.kernel.org/msg31161.html
>>
>> Also, later in that thread I also wrote[1] what seems to be the core of
>> what you've done here: namely, allow platform_devices and
>> platform_drivers to to be used on custom busses. Patch is at the end of
>> this mail with a more focused changelog. As Greg suggested in his reply
>> to your first version, this part could be merged today, and the
>> platform_bus_init stuff could be added later, after some more review.
>> Some comments below...
>>
>
> I can split this into 2 patches.
Yes, I think that would be better.
> Was your patch sent to linux-kernel or just linux-omap? I'm not on
> linux-omap...
That thread was on linux-arm-kernel and linux-omap
>
>>> [snip]
>>>
>>> Which will allow the same driver to easily to used on either
>>> the platform bus or the newly defined bus type.
>>
>> Except it requires a re-compile.
>>
>> Rather than doing this at compile time, it would be better to support
>> legacy devices at runtime. You could handle this by simply registering
>> the driver on the custom bus and the platform_bus and let the bus
>> matching code handle it. Then, the same binary would work on both
>> legacy and updated SoCs.
>>
>
> Can you safely register a driver on more than one bus? I didn't think
> that was safe -- normally it's impossible since you're calling
>
> struct BUS_TYPE_driver mydriver;
> BUS_TYPE_driver_register(&mydriver)
>
> but now we have multiple "bus types" that are all actually platform type;
> still,
> at a minimum you would need:
> struct platform_driver mydrvier1 = {
> .driver.bus = &sub_bus1,
> };
> struct platform_driver mydrvier2 = {
> .driver.bus = &sub_bus2,
> };
> which would all point to the same driver functions, yet the respective devices
> attached for the "same" driver would be on different buses. I fear this might
> confuse some drivers. I don't think dynamic bus assignment is this easy
>
> In short: I do not believe the same driver can be registered on multiple
> different buses -- if this is wrong, please correct me.
It is possible, and currently done in powerpc land where some
drivers handle devices on the platform_bus and the custom OF bus.
However, as noted by Magnus, what we really need here is a way for
drivers to not care at all what kind of bus they are on. There are an
increasing number of drivers that are re-used not just across different
SoCs in the same family, but across totally different SoCs (e.g. drivers
for hardware shared between TI OMAP and TI DaVinci, or SH and SH-Mobile/ARM)
>>
>> Up to here, this looks exactly what I wrote in thread referenced
>> above.
>>
>
> It is, you just went on vacation :)
>
Ah, OK. The changelog was missing credits to that affect, but I was
more concerned that you hadn't seen my example and didn't want to be
duplicating work.
>>>
>>> if (code != retval)
>>> platform_driver_unregister(drv);
>>> @@ -1017,6 +1019,26 @@ struct bus_type platform_bus_type = {
>>> };
>>> EXPORT_SYMBOL_GPL(platform_bus_type);
>>>
>>> +/** platform_bus_type_init - fill in a pseudo-platform-bus
>>> + * @bus: foriegn bus type
>>> + *
>>> + * This init is basically a selective memcpy that
>>> + * won't overwrite any user-defined attributes and
>>> + * only copies things that platform bus defines anyway
>>> + */
>>
>> minor nit: kernel doc style has wrong indentation
>>
>
> will fix
>
>>> +void platform_bus_type_init(struct bus_type *bus)
>>> +{
>>> + if (!bus->dev_attrs)
>>> + bus->dev_attrs = platform_bus_type.dev_attrs;
>>> + if (!bus->match)
>>> + bus->match = platform_bus_type.match;
>>> + if (!bus->uevent)
>>> + bus->uevent = platform_bus_type.uevent;
>>> + if (!bus->pm)
>>> + bus->pm = platform_bus_type.pm;
>>> +}
>>> +EXPORT_SYMBOL_GPL(platform_bus_type_init);
>>
>> With this approach, you should note in the comments/changelog that
>> any selective customization of the bus PM methods must be done after
>> calling platform_bus_type_init().
>
> No they don't. If you call platform_bus_type_init
Re: [PATCH] platform: Facilitate the creation of pseduo-platform busses
Magnus Damm writes:
> On Thu, Aug 5, 2010 at 7:14 AM, Patrick Pannuto
> wrote:
>> Inspiration for this comes from:
>> http://www.mail-archive.com/linux-o...@vger.kernel.org/msg31161.html
>>
>> RFC: http://lkml.org/lkml/2010/8/3/496
>> Patch is unchanged from the RFC. Reviews seemed generally positive
>> and it seemed this was desired functionality.
>
> Thanks for your patch, it's really nice to see work done in this area!
> I'd like to see something like this merged in the not so distant
> future. At this point I'm not so concerned about the details, so I'll
> restrict myself to this:
>
>> /drivers/my_driver.c
>> static struct platform_driver my_driver = {
>> .driver = {
>> .name = "my-driver",
>> .owner = THIS_MODULE,
>> .bus = &my_bus_type,
>> },
>> };
>
> I would really prefer not to have the bus type in the here. I
> understand it's needed at this point, but I wonder if it's possible to
> adjust the device<->driver matching for platform devices to allow any
> type of pseudo-platform bus_type.
I totally agree here. Keeping the drivers ignorant of the bus (or SoC)
they are on will make them much more portable.
Kevin
--
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] platform: Facilitate the creation of pseduo-platform busses
|-- my-driver
>
>
> Signed-off-by: Patrick Pannuto
> ---
> drivers/base/platform.c | 30 ++
> include/linux/platform_device.h |2 ++
> 2 files changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 4d99c8b..c86be03 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -241,7 +241,8 @@ int platform_device_add(struct platform_device *pdev)
> if (!pdev->dev.parent)
> pdev->dev.parent = &platform_bus;
>
> - pdev->dev.bus = &platform_bus_type;
> + if (!pdev->dev.bus)
> + pdev->dev.bus = &platform_bus_type;
>
> if (pdev->id != -1)
> dev_set_name(&pdev->dev, "%s.%d", pdev->name, pdev->id);
> @@ -482,7 +483,8 @@ static void platform_drv_shutdown(struct device *_dev)
> */
> int platform_driver_register(struct platform_driver *drv)
> {
> - drv->driver.bus = &platform_bus_type;
> + if (!drv->driver.bus)
> + drv->driver.bus = &platform_bus_type;
> if (drv->probe)
> drv->driver.probe = platform_drv_probe;
> if (drv->remove)
> @@ -539,12 +541,12 @@ int __init_or_module platform_driver_probe(struct
> platform_driver *drv,
>* if the probe was successful, and make sure any forced probes of
>* new devices fail.
>*/
> - spin_lock(&platform_bus_type.p->klist_drivers.k_lock);
> + spin_lock(&drv->driver.bus->p->klist_drivers.k_lock);
> drv->probe = NULL;
> if (code == 0 && list_empty(&drv->driver.p->klist_devices.k_list))
> retval = -ENODEV;
> drv->driver.probe = platform_drv_probe_fail;
> - spin_unlock(&platform_bus_type.p->klist_drivers.k_lock);
> + spin_unlock(&drv->driver.bus->p->klist_drivers.k_lock);
Up to here, this looks exactly what I wrote in thread referenced above.
>
> if (code != retval)
> platform_driver_unregister(drv);
> @@ -1017,6 +1019,26 @@ struct bus_type platform_bus_type = {
> };
> EXPORT_SYMBOL_GPL(platform_bus_type);
>
> +/** platform_bus_type_init - fill in a pseudo-platform-bus
> + * @bus: foriegn bus type
> + *
> + * This init is basically a selective memcpy that
> + * won't overwrite any user-defined attributes and
> + * only copies things that platform bus defines anyway
> + */
minor nit: kernel doc style has wrong indentation
> +void platform_bus_type_init(struct bus_type *bus)
> +{
> + if (!bus->dev_attrs)
> + bus->dev_attrs = platform_bus_type.dev_attrs;
> + if (!bus->match)
> + bus->match = platform_bus_type.match;
> + if (!bus->uevent)
> + bus->uevent = platform_bus_type.uevent;
> + if (!bus->pm)
> + bus->pm = platform_bus_type.pm;
> +}
> +EXPORT_SYMBOL_GPL(platform_bus_type_init);
With this approach, you should note in the comments/changelog that
any selective customization of the bus PM methods must be done after
calling platform_bus_type_init().
Also, You've left out the legacy PM methods here. That implies that
moving a driver from the platform_bus to the custom bus is not entirely
transparent. If the driver still has legacy PM methods, it would stop
working on the custom bus.
While this is good motivation for converting a driver to dev_pm_ops, at
a minimum it should be clear in the changelog that the derivative busses
do not support legacy PM methods. However, since it's quite easy to do,
and you want the derivative busses to be *exactly* like the platform bus
except where explicitly changed, I'd suggest you also check/copy the
legacy PM methods.
In addition, you've missed several fields in 'struct bus_type'
(bus_attr, drv_attr, p, etc.) Without digging deeper into the driver
core, I'm not sure if they are all needed at init time, but it should be
clear in the comments why they can be excluded.
Kevin
[1] http://www.mail-archive.com/linux-o...@vger.kernel.org/msg31289.html
>From b784009af1d0a7065dc5e58a13ce29afa3432d3e Mon Sep 17 00:00:00 2001
From: Kevin Hilman
Date: Mon, 28 Jun 2010 16:08:14 -0700
Subject: [PATCH] driver core: allow platform_devices and platform_drivers on
custom busses
This allows platform_devices and platform_drivers to be registered onto
custom busses that are compatible with the platform_bus.
Signed-off-by: Kevin Hilman
---
drivers/base/platform.c | 10 ++
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 4d99c8b..2cf55e2 100644
--- a/drivers/base/p
