Re: [PATCH] cpufreq: Set policy to non-NULL only after all hotplug online work is done
On 25 February 2014 01:53, Saravana Kannan skan...@codeaurora.org wrote: I was simplifying the scenario that causes it. We change the min/max using ADJUST notifiers for multiple reasons -- thermal being one of them. thermal/cpu_cooling is one example of it. Just to understand the clear picture, you are actually hitting this bug? Or is this only a theoretical bug? So, cpufreq_update_policy() can be called on any CPU. If that races with someone offlining a CPU and onlining it, you'll get this crash. Then shouldn't that be fixed by locks? I think yes. That makes me agree with Srivatsa more here. Though I would say that your argument was also valid that 'policy' shouldn't be up for sale unless it is prepared to. And for that reason only I floated that question earlier: What exactly we need to make sure is initialized in policy? Because policy might keep changing in future as well and that needs locks to protect that stuff. Like min/max/governor/ etc.. So, probably a solution here might be a mix of both. Initialize policy to this minimum level and then make sure locking is used correctly.. The idea would exist, but we can just call cpufreq_generic_get() and pass it policy-clk if it is not NULL. Does that work for you? No. Not all drivers implement clk interface. And so clk doesn't look to be the right parameter. I thought maybe 'policy' can be the right parameter and then people can get use policy-cpu to get cpu id out of it. But even that doesn't look to be a great idea. X86 drivers may share policy structure for CPUs that don't actually share a clock line. And so they do need right CPU number as parameter instead of policy. As they might be doing some tricky stuff there. Also, we need to make sure that -get() returns the frequency at which CPU x is running. So the real question here is: What fields of 'policy' do we need to guarantee to be initialized before policy is made available for others? And probably that is what we need to fix. That's going to be hard to define since future uses could be looking at different fields. What is the API semantics of cpufreq_cpu_get(). I can't ever imagine it being correct for any API to return a partially initialized data structure. I do agree that we can't broadcast 'policy' before it is usable. And that's why I am asking about the right place where we are sure that it is ready.. Even your current solution would break things. For example, below will happen before policy is set in per-cpu variable: - CPUFREQ_CREATE_POLICY notifier will be fired and calls to do cpufreq_cpu_get() there will fail. And hence cpufreq-stats sysfs will break. I did this on 3.12. I'll fix it up to handle this one. This can be moved down without much issues I believe. - Governors also use cpufreq_cpu_get() and so those would also fail as they are started from cpufreq_init_policy().. The only use of this in governors is in update_sampling_rate() and the behavior after this fix would be correct in that case. If the policy is not fully initialized -- that update doesn't get to go through. hmm.. but even that looks odd. We have reached upto governors but policy isn't available to be used? It should have been ready by now? All other uses of this API fall under one of these categories: * CPUs are already fully offline whenever it's called. * Already get the real policy as part of the notifier so don't need to call cpufreq_cpu_get If I find any that doesn't fit the above, I would be glad to fix that if it's pointed out. I have just sent out a patchset to you and others, would be great if you can give it a review/test .. -- 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: linux-next: Tree for Feb 17 (pinctrl-msm)
On Mon, Feb 24, 2014 at 7:41 PM, Josh Cartwright jo...@codeaurora.org wrote: On Mon, Feb 24, 2014 at 10:14:45AM -0800, Randy Dunlap wrote: Without too much effort, I can get this to fail just by making CONFIG_PINCTRL_MSM=m. handle_bad_irq isn't marked EXPORT_SYMBOL*, so hence the warning. Whether or not this is intentional is not clear. Do we support modules installing chained irq handlers? That is a good question to tglx/Grant ... As the kernel looks today, drivers installing chained handlers cannot be modules, and that is it. Yours, Linus Walleij -- 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: linux-next: Tree for Feb 17 (pinctrl-msm)
On Mon, Feb 24, 2014 at 8:28 PM, Bjorn Andersson bjorn.anders...@sonymobile.com wrote: This comes from the request of having everything as a module, to reduce the size of the multi-platform ARM builds. I would say that the important part related to that would be to keep the platform specific tables in modules. But keeping these parts as modules would still mean that it's a module that install the chained irq handler. Yeah that is a bit of double-command is it not :-) @Linus, I'm not sure about what should be module and not in pinctrl, but this part of pinctrl-msm is less important then the others to be able to be compiled as a module. FWIW; Reviewed-by: Bjorn Andersson bjorn.anders...@sonymobile.com Well we can't export that function in the rc series so I applied this patch for fixes with your review tag. We can discuss making chained IRQ handlers in modules for the next merge window... Linus -- 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 2/4] devicetree: bindings: Document Krait CPU/L1 EDAC
Hi Stephen, On Wed, Feb 19, 2014 at 12:20:43AM +, Stephen Boyd wrote: (Sorry, this discussion stalled due to merge window + life events) Sorry for the delay in replying on my side too. On 01/17, Lorenzo Pieralisi wrote: On Thu, Jan 16, 2014 at 07:26:17PM +, Stephen Boyd wrote: On 01/16, Lorenzo Pieralisi wrote: On Thu, Jan 16, 2014 at 06:05:05PM +, Stephen Boyd wrote: On 01/16, Lorenzo Pieralisi wrote: Do we really want to do that ? I am not sure. A cpus node is supposed to be a container node, we should not define this binding just because we know the kernel creates a platform device for it then. This is just copying more of the ePAPR spec into this document. It just so happens that having a compatible field here allows a platform device to be created. I don't see why that's a problem. I do not see why you cannot define a node like pmu or arch-timer and stick a compatible property in there. cpus node does not represent a device, and must not be created as a platform device, that's my opinion. I had what you're suggesting before in the original revision of this patch. Please take a look at the original patch series[1]. I suppose it could be tweaked slightly to still have a cache node for the L2 interrupt and the next-level-cache pointer from the CPUs. Ok, sorry, we are running around in circles here, basically you moved the node to cpus according to reviews. I still think that treating cpus as a device is not a great idea, even though I am in the same position with C-states and probably will add C-state tables in the cpus node. http://comments.gmane.org/gmane.linux.power-management.general/41012 I just would like to see under cpus nodes and properties that apply to all ARM systems, and avoid defining properties (eg interrupts) that have different meanings for different ARM cores. The question related to why the kernel should create a platform device out of cpus is still open. I really do not want to block your series for these simple issues but we have to make a decision and stick to that, I am fine either way if we have a plan. Do you just want a backup plan in case we don't make a platform device out of the cpus node? I believe we can always add code somewhere to create a platform device at runtime if we detect the cpus node has a compatible string equal to qcom,krait. We could probably change this driver's module_init() to scan the DT for such a compatible string and create the platform device right there. If we get more than one interrupt in the cpus node we can add interrupt-names and then have software look for interrupts by name instead of number. As I mentioned, I do not like the idea of adding compatible properties just to force the kernel to create platform devices out of device tree nodes. On top of that I would avoid adding a compatible property to the cpus node (after all properties like enable-method are common for all cpus but still duplicated), my only concern being backward compatibility here (ie if we do that for interrupts, we should do that also for other common cpu nodes properties, otherwise we have different rules for different properties). I think you can then add interrupts to cpu nodes (qcom,krait specific), and as you mentioned create a platform device for that. Thanks, Lorenzo -- 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] hwrng: msm: switch Kconfig to ARCH_QCOM depends
On Wed, Feb 12, 2014 at 11:33:14AM +0200, Stanimir Varbanov wrote: Hi, We've split Qualcomm MSM support into legacy and multiplatform. The RNG driver is only relevant on the multiplatform supported SoCs so switch the Kconfig depends to ARCH_QCOM. CC: Herbert Xu herb...@gondor.apana.org.au Signed-off-by: Kumar Gala ga...@codeaurora.org --- drivers/char/hw_random/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig index 2f2b084..289bd3a 100644 --- a/drivers/char/hw_random/Kconfig +++ b/drivers/char/hw_random/Kconfig @@ -343,7 +343,7 @@ config HW_RANDOM_TPM config HW_RANDOM_MSM tristate Qualcomm MSM Random Number Generator support - depends on HW_RANDOM ARCH_MSM + depends on HW_RANDOM ARCH_QCOM ---help--- This driver provides kernel-side support for the Random Number Generator hardware found on Qualcomm MSM SoCs. IMO, after this change the MSM abbreviation in help clause is irrelevant, could you remove it? Reviewed-by: Stanimir Varbanov svarba...@mm-sol.com Patch applied. -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- 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] cpufreq: Set policy to non-NULL only after all hotplug online work is done
On Tuesday, February 25, 2014 02:20:57 PM Viresh Kumar wrote: On 25 February 2014 01:53, Saravana Kannan skan...@codeaurora.org wrote: I was simplifying the scenario that causes it. We change the min/max using ADJUST notifiers for multiple reasons -- thermal being one of them. thermal/cpu_cooling is one example of it. Just to understand the clear picture, you are actually hitting this bug? Or is this only a theoretical bug? So, cpufreq_update_policy() can be called on any CPU. If that races with someone offlining a CPU and onlining it, you'll get this crash. Then shouldn't that be fixed by locks? I think yes. That makes me agree with Srivatsa more here. Though I would say that your argument was also valid that 'policy' shouldn't be up for sale unless it is prepared to. And for that reason only I floated that question earlier: What exactly we need to make sure is initialized in policy? Because policy might keep changing in future as well and that needs locks to protect that stuff. Like min/max/governor/ etc.. Well, that depends on what the current users expect it to look like initially. It should be initialized to the point in which all of them can handle it correctly. So, probably a solution here might be a mix of both. Initialize policy to this minimum level and then make sure locking is used correctly.. Yes. The idea would exist, but we can just call cpufreq_generic_get() and pass it policy-clk if it is not NULL. Does that work for you? No. Not all drivers implement clk interface. And so clk doesn't look to be the right parameter. I thought maybe 'policy' can be the right parameter and then people can get use policy-cpu to get cpu id out of it. But even that doesn't look to be a great idea. X86 drivers may share policy structure for CPUs that don't actually share a clock line. And so they do need right CPU number as parameter instead of policy. As they might be doing some tricky stuff there. Also, we need to make sure that -get() returns the frequency at which CPU x is running. That's not going to work in at least some cases anyway, because for some types of HW we simply can't retrieve the current frequency in a non-racy way. Thanks! -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- 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] cpufreq: Set policy to non-NULL only after all hotplug online work is done
On 25 February 2014 18:34, Rafael J. Wysocki r...@rjwysocki.net wrote: Well, that depends on what the current users expect it to look like initially. It should be initialized to the point in which all of them can handle it correctly. Exactly. That's not going to work in at least some cases anyway, because for some types of HW we simply can't retrieve the current frequency in a non-racy way. I agree. But this is all that we can guarantee here :) -- 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
[RFC] usb: gadget: f_fs: Add flags to descriptors block
This reworks the way SuperSpeed descriptors are added and instead of
having a magick after full and high speed descriptors, it reworks the
whole descriptors block to include a flags field which lists which
descriptors are present and makes future extensions possible.
Signed-off-by: Michal Nazarewicz min...@mina86.com
---
Just to repeat what I wrote on my phone which is unable to send
text/plain messages:
On Tue, Feb 25 2014, Michał Nazarewicz wrote:
On Feb 25, 2014 5:47 AM, Manu Gautam mgau...@codeaurora.org wrote:
Is this patch good now to be added to your queue (if not already in)?
It applied cleanly on your next branch.
Actually, recent LWN article about flags in syscalls got me thinking and
maybe it's good this patch is not yet in Felipe's queue.
I admit I screwed up when I designed the headers format, but now that we
change it to accommodate SuperSpeed, I think we should go ahead and change
the format by adding flags field.
[…]
Sorry to mention this only now, but I believe that's the right thing to do.
This patch implements this new format and in doing so simplifies, in
my opinion, the code a bit.
It needs to be applied on top of Manu's patch.
It has not been tested at all, not even compile-tested. This is
because whenever I try to compile the kernel, I get this nice message
in my logs:
mce: [Hardware Error]: Machine check events logged
thermal_sys: Critical temperature reached(100 C),shutting down
thermal_sys: Critical temperature reached(100 C),shutting down
Sorry about that.
drivers/usb/gadget/f_fs.c | 136 +++-
drivers/usb/gadget/u_fs.h | 12 ++--
include/uapi/linux/usb/functionfs.h | 49 -
3 files changed, 94 insertions(+), 103 deletions(-)
diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
index 928959d..7d01bd6 100644
--- a/drivers/usb/gadget/f_fs.c
+++ b/drivers/usb/gadget/f_fs.c
@@ -1167,7 +1167,7 @@ static void ffs_data_clear(struct ffs_data *ffs)
if (ffs-epfiles)
ffs_epfiles_destroy(ffs-epfiles, ffs-eps_count);
- kfree(ffs-raw_descs);
+ kfree(ffs-raw_descs_data);
kfree(ffs-raw_strings);
kfree(ffs-stringtabs);
}
@@ -1179,12 +1179,12 @@ static void ffs_data_reset(struct ffs_data *ffs)
ffs_data_clear(ffs);
ffs-epfiles = NULL;
+ ffs-raw_descs_data = NULL;
ffs-raw_descs = NULL;
ffs-raw_strings = NULL;
ffs-stringtabs = NULL;
- ffs-raw_fs_hs_descs_length = 0;
- ffs-raw_ss_descs_length = 0;
+ ffs-real_descs_length = 0;
ffs-fs_descs_count = 0;
ffs-hs_descs_count = 0;
ffs-ss_descs_count = 0;
@@ -1598,89 +1598,76 @@ static int __ffs_data_do_entity(enum ffs_entity_type
type,
static int __ffs_data_got_descs(struct ffs_data *ffs,
char *const _data, size_t len)
{
- unsigned fs_count, hs_count, ss_count = 0;
- int fs_len, hs_len, ss_len, ret = -EINVAL;
- char *data = _data;
+ char *data = _data, *raw_descs;
+ unsigned counts[3], flags;
+ int ret = -EINVAL, i;
ENTER();
- if (unlikely(get_unaligned_le32(data) != FUNCTIONFS_DESCRIPTORS_MAGIC ||
-get_unaligned_le32(data + 4) != len))
+ if (get_unaligned_le32(data + 4) != len)
goto error;
- fs_count = get_unaligned_le32(data + 8);
- hs_count = get_unaligned_le32(data + 12);
-
- data += 16;
- len -= 16;
- if (likely(fs_count)) {
- fs_len = ffs_do_descs(fs_count, data, len,
- __ffs_data_do_entity, ffs);
- if (unlikely(fs_len 0)) {
- ret = fs_len;
+ switch (get_unaligned_le32(data)) {
+ case FUNCTIONFS_DESCRIPTORS_MAGIC:
+ flags = FUNCTIONFS_HAS_FS_DESC | FUNCTIONFS_HAS_HS_DESC;
+ data += 8;
+ len -= 8;
+ break;
+ case FUNCTIONFS_DESCRIPTORS_MAGIC_V2:
+ flags = get_unaligned_le32(data + 8);
+ if (flags ~(FUNCTIONFS_HAS_FS_DESC |
+ FUNCTIONFS_HAS_HS_DESC |
+ FUNCTIONFS_HAS_SS_DESC)) {
+ ret = -ENOSYS;
goto error;
}
-
- data += fs_len;
- len -= fs_len;
- } else {
- fs_len = 0;
+ data += 12;
+ len -= 12;
+ break;
+ default:
+ goto error;
}
- if (likely(hs_count)) {
- hs_len = ffs_do_descs(hs_count, data, len,
- __ffs_data_do_entity, ffs);
- if (unlikely(hs_len 0)) {
- ret = hs_len;
+ /* Read fs_count, hs_count and ss_count (if present) */
+ for (i = 0; i 3; ++i) {
+ if (!(flags (1 i))) {
+
[GIT PULL] qcom driver changes for v3.15
The following changes since commit cf1e8f0cd665e2a9966d2bee4e11ecc0938ff166: ARM: qcom: Rename various msm prefixed functions to qcom (2014-02-06 16:20:41 -0600) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/galak/linux-qcom.git tags/qcom-drivers-for-3.15 for you to fetch changes up to add798a40aec3a768165055477ba8bfee21e06fc: gpio: msm: switch Kconfig to ARCH_QCOM depends (2014-02-25 09:22:10 -0600) Kumar Gala (5): tty: serial: msm: Enable building msm_serial for ARCH_QCOM drm/msm: drop ARCH_MSM Kconfig depend power: reset: msm - switch Kconfig to ARCH_QCOM depends hwrng: msm: switch Kconfig to ARCH_QCOM depends gpio: msm: switch Kconfig to ARCH_QCOM depends drivers/char/hw_random/Kconfig | 6 +++--- drivers/gpio/Kconfig | 2 +- drivers/gpu/drm/msm/Kconfig| 2 +- drivers/power/reset/Kconfig| 2 +- drivers/tty/serial/Kconfig | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ARM: dts: MSM8974: Add pinctrl node
On Feb 24, 2014, at 3:57 AM, Linus Walleij linus.wall...@linaro.org wrote: On Thu, Feb 6, 2014 at 4:28 PM, Ivan T. Ivanov iiva...@mm-sol.com wrote: From: Ivan T. Ivanov iiva...@mm-sol.com Add the pin control node and pin definitions of SPI8. Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com Acked-by: Linus Walleij linus.wall...@linaro.org Kumar, please take this through your qcom tree. applied to qcom/dt - k -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] i2c: New bus driver for the Qualcomm QUP I2C controller
On Fri, Feb 21, 2014 at 3:06 AM, Mark Rutland mark.rutl...@arm.com wrote:
On Fri, Feb 21, 2014 at 12:38:10AM +, Bjorn Andersson wrote:
[...]
+static const struct of_device_id qup_i2c_dt_match[] = {
+ { .compatible = qcom,i2c-qup-v1.1.1 },
+ { .compatible = qcom,i2c-qup-v2.1.1 },
+ { .compatible = qcom,i2c-qup-v2.2.1 },
The all seem to be handled the same.
Are they all compatible with the qcom,i2c-qup-v1.1.1 programming
model, such that it could be used as a fallback in the compatible list
(and the driver would only need to look for it for now)?
The v2 model will get BAM (DMAEngine) support soon, v1 uses an
older DMA core. So there's a difference. I'm not aware what differences
there are between 2.1.1 and 2.2.1.
Question is if next change will be called v3, as we then could skip the
reset of the version.
We could probably skip 2.1.1 as that's supposed to be the first revision
of 8x74, with all the expected HW quirks...I.e. not sure if anyone should
use that.
Regards,
Bjorn
--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] i2c: New bus driver for the Qualcomm QUP I2C controller
On Tue, Feb 25, 2014 at 08:07:13AM -0800, Bjorn Andersson wrote: [snip] The v2 model will get BAM (DMAEngine) support soon, v1 uses an older DMA core. So there's a difference. I'm not aware what differences there are between 2.1.1 and 2.2.1. Difference between 2.1.1 and 2.2.1: - high speed support, up to 3.4MHz - reconfiguration during run state - READ_AND_NACK, STOP tag support - some h/w bug fixes that don't involve software changes. Question is if next change will be called v3, as we then could skip the reset of the version. Next version is 2.4. -- sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2] usb: gadget: f_fs: Add flags to descriptors block
This reworks the way SuperSpeed descriptors are added and instead of
having a magick after full and high speed descriptors, it reworks the
whole descriptors block to include a flags field which lists which
descriptors are present and makes future extensions possible.
Signed-off-by: Michal Nazarewicz min...@mina86.com
---
drivers/usb/gadget/f_fs.c | 136 +++-
drivers/usb/gadget/u_fs.h | 12 ++--
include/uapi/linux/usb/functionfs.h | 49 -
3 files changed, 94 insertions(+), 103 deletions(-)
All right, with some fidling with my fan and limiting CPU frequency to
the mininimum, I've managed to compile this patch and fixed all the
typos.
Manu, if you could include it in your series, adjust your user space
client and test it, it would be wonderful. :]
diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
index 928959d..fab63d3 100644
--- a/drivers/usb/gadget/f_fs.c
+++ b/drivers/usb/gadget/f_fs.c
@@ -1167,7 +1167,7 @@ static void ffs_data_clear(struct ffs_data *ffs)
if (ffs-epfiles)
ffs_epfiles_destroy(ffs-epfiles, ffs-eps_count);
- kfree(ffs-raw_descs);
+ kfree(ffs-raw_descs_data);
kfree(ffs-raw_strings);
kfree(ffs-stringtabs);
}
@@ -1179,12 +1179,12 @@ static void ffs_data_reset(struct ffs_data *ffs)
ffs_data_clear(ffs);
ffs-epfiles = NULL;
+ ffs-raw_descs_data = NULL;
ffs-raw_descs = NULL;
ffs-raw_strings = NULL;
ffs-stringtabs = NULL;
- ffs-raw_fs_hs_descs_length = 0;
- ffs-raw_ss_descs_length = 0;
+ ffs-raw_descs_length = 0;
ffs-fs_descs_count = 0;
ffs-hs_descs_count = 0;
ffs-ss_descs_count = 0;
@@ -1598,89 +1598,76 @@ static int __ffs_data_do_entity(enum ffs_entity_type
type,
static int __ffs_data_got_descs(struct ffs_data *ffs,
char *const _data, size_t len)
{
- unsigned fs_count, hs_count, ss_count = 0;
- int fs_len, hs_len, ss_len, ret = -EINVAL;
- char *data = _data;
+ char *data = _data, *raw_descs;
+ unsigned counts[3], flags;
+ int ret = -EINVAL, i;
ENTER();
- if (unlikely(get_unaligned_le32(data) != FUNCTIONFS_DESCRIPTORS_MAGIC ||
-get_unaligned_le32(data + 4) != len))
+ if (get_unaligned_le32(data + 4) != len)
goto error;
- fs_count = get_unaligned_le32(data + 8);
- hs_count = get_unaligned_le32(data + 12);
-
- data += 16;
- len -= 16;
- if (likely(fs_count)) {
- fs_len = ffs_do_descs(fs_count, data, len,
- __ffs_data_do_entity, ffs);
- if (unlikely(fs_len 0)) {
- ret = fs_len;
+ switch (get_unaligned_le32(data)) {
+ case FUNCTIONFS_DESCRIPTORS_MAGIC:
+ flags = FUNCTIONFS_HAS_FS_DESC | FUNCTIONFS_HAS_HS_DESC;
+ data += 8;
+ len -= 8;
+ break;
+ case FUNCTIONFS_DESCRIPTORS_MAGIC_V2:
+ flags = get_unaligned_le32(data + 8);
+ if (flags ~(FUNCTIONFS_HAS_FS_DESC |
+ FUNCTIONFS_HAS_HS_DESC |
+ FUNCTIONFS_HAS_SS_DESC)) {
+ ret = -ENOSYS;
goto error;
}
-
- data += fs_len;
- len -= fs_len;
- } else {
- fs_len = 0;
+ data += 12;
+ len -= 12;
+ break;
+ default:
+ goto error;
}
- if (likely(hs_count)) {
- hs_len = ffs_do_descs(hs_count, data, len,
- __ffs_data_do_entity, ffs);
- if (unlikely(hs_len 0)) {
- ret = hs_len;
+ /* Read fs_count, hs_count and ss_count (if present) */
+ for (i = 0; i 3; ++i) {
+ if (!(flags (1 i))) {
+ counts[i] = 0;
+ } else if (len 4) {
goto error;
+ } else {
+ counts[i] = get_unaligned_le32(data);
+ data += 4;
+ len -= 4;
}
-
- data += hs_len;
- len -= hs_len;
- } else {
- hs_len = 0;
}
- if (len = 8) {
- /* Check SS_MAGIC for presence of ss_descs and get SS_COUNT */
- if (get_unaligned_le32(data) != FUNCTIONFS_SS_DESC_MAGIC)
- goto einval;
-
- ss_count = get_unaligned_le32(data + 4);
- data += 8;
- len -= 8;
- }
-
- if (!fs_count !hs_count !ss_count)
- goto einval;
-
- if (ss_count) {
- ss_len = ffs_do_descs(ss_count, data, len,
+ /* Read descriptors */
+ raw_descs = data;
+
Re: [GIT PULL] qcom driver changes for v3.15
On Tuesday 25 February 2014, Kumar Gala wrote: The following changes since commit cf1e8f0cd665e2a9966d2bee4e11ecc0938ff166: ARM: qcom: Rename various msm prefixed functions to qcom (2014-02-06 16:20:41 -0600) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/galak/linux-qcom.git tags/qcom-drivers-for-3.15 for you to fetch changes up to add798a40aec3a768165055477ba8bfee21e06fc: gpio: msm: switch Kconfig to ARCH_QCOM depends (2014-02-25 09:22:10 -0600) Applied to the next/drivers branch now, and added a merge description that I adapted from your patch changelogs: Merge qcom driver changes for v3.15 from Kumar Gala: We've split Qualcomm MSM support into legacy and multiplatform. These drivers are only relevant on the multiplatform supported SoCs so switch the Kconfig depends to ARCH_QCOM. It would be helpful if you can in the future always add a description yourself when creating the tag. Arnd -- 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: [PATCHv2] usb: gadget: f_fs: Add flags to descriptors block
Hi, On Tue, Feb 25, 2014 at 06:02:18PM +0100, Michal Nazarewicz wrote: This reworks the way SuperSpeed descriptors are added and instead of having a magick after full and high speed descriptors, it reworks the whole descriptors block to include a flags field which lists which descriptors are present and makes future extensions possible. Signed-off-by: Michal Nazarewicz min...@mina86.com --- drivers/usb/gadget/f_fs.c | 136 +++- drivers/usb/gadget/u_fs.h | 12 ++-- include/uapi/linux/usb/functionfs.h | 49 - 3 files changed, 94 insertions(+), 103 deletions(-) All right, with some fidling with my fan and limiting CPU frequency to the mininimum, I've managed to compile this patch and fixed all the typos. Manu, if you could include it in your series, adjust your user space client and test it, it would be wonderful. :] I'll wait for that then. Note that I want to close my tree next wednesday tops. -- balbi signature.asc Description: Digital signature
Re: [GIT PULL] qcom driver changes for v3.15
On Feb 25, 2014, at 11:14 AM, Arnd Bergmann a...@arndb.de wrote: On Tuesday 25 February 2014, Kumar Gala wrote: The following changes since commit cf1e8f0cd665e2a9966d2bee4e11ecc0938ff166: ARM: qcom: Rename various msm prefixed functions to qcom (2014-02-06 16:20:41 -0600) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/galak/linux-qcom.git tags/qcom-drivers-for-3.15 for you to fetch changes up to add798a40aec3a768165055477ba8bfee21e06fc: gpio: msm: switch Kconfig to ARCH_QCOM depends (2014-02-25 09:22:10 -0600) Applied to the next/drivers branch now, and added a merge description that I adapted from your patch changelogs: Merge qcom driver changes for v3.15 from Kumar Gala: We've split Qualcomm MSM support into legacy and multiplatform. These drivers are only relevant on the multiplatform supported SoCs so switch the Kconfig depends to ARCH_QCOM. It would be helpful if you can in the future always add a description yourself when creating the tag. Arnd Oops, I think when I updated the tag before sending I lost the message in the tag - k -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM / devfreq: Fix out of bounds access of transition table array
On 02/23/2014 11:15 PM, Saravana Kannan wrote: The previous_freq value for a device could be an invalid frequency that results in a error value being returned from devfreq_get_freq_level(). Check for an error value before using that to index into the transition table. Not doing this check will result in memory corruption when previous_freq is not a valid frequency. Signed-off-by: Saravana Kannan skan...@codeaurora.org MyungJoo/Kyungmin, Would either of you have some time to respond to this? Thanks, Saravana -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] cpufreq: Set policy to non-NULL only after all hotplug online work is done
On Tuesday, February 25, 2014 01:11:27 PM Saravana Kannan wrote: On 02/25/2014 05:04 AM, Rafael J. Wysocki wrote: On Tuesday, February 25, 2014 02:20:57 PM Viresh Kumar wrote: On 25 February 2014 01:53, Saravana Kannan skan...@codeaurora.org wrote: I was simplifying the scenario that causes it. We change the min/max using ADJUST notifiers for multiple reasons -- thermal being one of them. thermal/cpu_cooling is one example of it. Just to understand the clear picture, you are actually hitting this bug? Or is this only a theoretical bug? This is a real bug. But the actual caller of cpufreq_update_policy() is a driver that's local to our tree. I'm just giving examples of upstream code that act in a similar way. So, cpufreq_update_policy() can be called on any CPU. If that races with someone offlining a CPU and onlining it, you'll get this crash. Then shouldn't that be fixed by locks? I think yes. That makes me agree with Srivatsa more here. Though I would say that your argument was also valid that 'policy' shouldn't be up for sale unless it is prepared to. And for that reason only I floated that question earlier: What exactly we need to make sure is initialized in policy? Because policy might keep changing in future as well and that needs locks to protect that stuff. Like min/max/governor/ etc.. Well, that depends on what the current users expect it to look like initially. It should be initialized to the point in which all of them can handle it correctly. Yes, so let's not make it available until all of it is initialized. And is fully initialized actually well defined? I don't like the piece meal check. 6 months down the lane someone making changes might not remember this. The problem also applies for drivers that might not be upstreamed, etc. Please don't bring up out-of-the-tree drivers argument in mainline discussions, it is meaningless here. So, probably a solution here might be a mix of both. Initialize policy to this minimum level and then make sure locking is used correctly.. Yes. Rafael, It's not clear what you mean by the yes. Do you want to initialize it partly and make it available. I think that's always wrong. So what actually is your porposal? The idea would exist, but we can just call cpufreq_generic_get() and pass it policy-clk if it is not NULL. Does that work for you? No. Not all drivers implement clk interface. And so clk doesn't look to be the right parameter. I thought maybe 'policy' can be the right parameter and then people can get use policy-cpu to get cpu id out of it. But even that doesn't look to be a great idea. X86 drivers may share policy structure for CPUs that don't actually share a clock line. And so they do need right CPU number as parameter instead of policy. As they might be doing some tricky stuff there. Also, we need to make sure that -get() returns the frequency at which CPU x is running. That's not going to work in at least some cases anyway, because for some types of HW we simply can't retrieve the current frequency in a non-racy way. I think there's been a misunderstanding of what I'm proposing. The reference to policy-clk is only to get rid of the dependency that cpufreq_generic_get() has on the per cpu policy variable being filled. You can do that by just removing calls to get _IF_ clk is filled in. I still have a little problem understanding what you mean exactly. At least please explain the last sentence. Viresh, I'll look at the patches later today and respond. Although, I would have been nice you had helped me fix any issues with my patch than coming up with new ones. Kinda removes to motivation for submitting patches upstream. Everyone is free to post patches at any time during the discussion. Viresh is as well as you are. Thanks! -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- 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] cpufreq: Set policy to non-NULL only after all hotplug online work is done
On 02/25/2014 02:41 PM, Rafael J. Wysocki wrote:
On Tuesday, February 25, 2014 01:11:27 PM Saravana Kannan wrote:
On 02/25/2014 05:04 AM, Rafael J. Wysocki wrote:
On Tuesday, February 25, 2014 02:20:57 PM Viresh Kumar wrote:
On 25 February 2014 01:53, Saravana Kannan skan...@codeaurora.org wrote:
I was simplifying the scenario that causes it. We change the min/max using
ADJUST notifiers for multiple reasons -- thermal being one of them.
thermal/cpu_cooling is one example of it.
Just to understand the clear picture, you are actually hitting this bug? Or
is this only a theoretical bug?
This is a real bug. But the actual caller of cpufreq_update_policy() is
a driver that's local to our tree. I'm just giving examples of upstream
code that act in a similar way.
So, cpufreq_update_policy() can be called on any CPU. If that races with
someone offlining a CPU and onlining it, you'll get this crash.
Then shouldn't that be fixed by locks? I think yes. That makes me agree with
Srivatsa more here.
Though I would say that your argument was also valid that 'policy' shouldn't be
up for sale unless it is prepared to. And for that reason only I
floated that question
earlier: What exactly we need to make sure is initialized in policy? Because
policy might keep changing in future as well and that needs locks to protect
that stuff. Like min/max/governor/ etc..
Well, that depends on what the current users expect it to look like initially.
It should be initialized to the point in which all of them can handle it
correctly.
Yes, so let's not make it available until all of it is initialized.
And is fully initialized actually well defined?
The point in add dev/hot plug path after which we will no longer change
policy fields without sending further CPUFREQ_UPDATE_POLICY_CPU /
CPUFRE_NOTIFY notifiers.
Pretty much the end of __cpufreq_add_dev() so that it's after:
- cpufreq_init_policy()
- And the update of userpolicy fields that after thie init call
I don't like the piece meal check. 6 months down the lane someone making
changes might not remember this. The problem also applies for drivers
that might not be upstreamed, etc.
Please don't bring up out-of-the-tree drivers argument in mainline discussions,
it is meaningless here.
Sure. This also applies to in-tree code. The 6 months down the lane,
what fields are being looked at could still change and we might slip up
on making sure that the policy is not made available before that field
is initialized.
So, probably a solution here might be a mix of both. Initialize policy to this
minimum level and then make sure locking is used correctly..
Yes.
Rafael, It's not clear what you mean by the yes. Do you want to
initialize it partly and make it available. I think that's always wrong.
So what actually is your porposal?
Same as reply to fully initialized. We make the policy after it's
fully initialized. After that point, any changes to the policy can be
tracked by registering for CPUFREQ_UPDATE_POLICY_CPU / CPUFREQ_NOTIFY.
The idea would exist, but we can just call cpufreq_generic_get() and pass it
policy-clk if it is not NULL. Does that work for you?
No. Not all drivers implement clk interface. And so clk doesn't look to be the
right parameter. I thought maybe 'policy' can be the right parameter and
then people can get use policy-cpu to get cpu id out of it.
But even that doesn't look to be a great idea. X86 drivers may share policy
structure for CPUs that don't actually share a clock line. And so they do need
right CPU number as parameter instead of policy. As they might be doing
some tricky stuff there. Also, we need to make sure that -get() returns
the frequency at which CPU x is running.
That's not going to work in at least some cases anyway, because for some types
of HW we simply can't retrieve the current frequency in a non-racy way.
I think there's been a misunderstanding of what I'm proposing. The
reference to policy-clk is only to get rid of the dependency that
cpufreq_generic_get() has on the per cpu policy variable being filled.
You can do that by just removing calls to get _IF_ clk is filled in.
I still have a little problem understanding what you mean exactly. At least
please explain the last sentence.
Ok, here's some pseudo code to explain it better:
Something like, replace all calls to cpufreq_driver-get with
__cpufreq_driver_get() with the fn being something like:
unsigned int __cpufreq_driver_get(cpufreq_policy *policy)
{
if (policy-clk)
return clk_get_rate(policy-clk) / 1000;
else
return cpufreq_driver-get(policy-cpu);
}
That way, we don't have to depend on cpufreq_cpu_get() to return a
policy in cpufreq_generic_get() for the existing add dev code path to
complete without an error (after my patch to advertise the policy struct
after it's fully initialized)
This also avoid unnecessary function calls to -get() when we just as
well know that it's going to call back into
[PATCH 3/3] cpufreq: Set policy to non-NULL only after all hotplug online work is done
The existing code sets the per CPU policy to a non-NULL value before all
the steps performed during the hotplug online path is done. Specifically,
this is done before the policy min/max, governors, etc are initialized for
the policy. This in turn means that calls to cpufreq_cpu_get() return a
non-NULL policy before the policy/CPU is ready to be used.
To fix this, move the update of per CPU policy to a valid value after all
the initialization steps for the policy are completed.
Example kernel panic without this fix:
[ 512.146185] Unable to handle kernel NULL pointer dereference at virtual
address 0020
[ 512.146195] pgd = c0003000
[ 512.146213] [0020] *pgd=804003, *pmd=
[ 512.146228] Internal error: Oops: 206 [#1] PREEMPT SMP ARM
snip
[ 512.146297] PC is at __cpufreq_governor+0x10/0x1ac
[ 512.146312] LR is at cpufreq_update_policy+0x114/0x150
snip
[ 512.149740] ---[ end trace f23a8defea6cd706 ]---
[ 512.149761] Kernel panic - not syncing: Fatal exception
[ 513.152016] CPU0: stopping
[ 513.154710] CPU: 0 PID: 7136 Comm: mpdecision Tainted: G D W
3.10.0-gd727407-00074-g979ede8 #396
snip
[ 513.317224] [c0afe180] (notifier_call_chain+0x40/0x68) from [c02a23ac]
(__blocking_notifier_call_chain+0x40/0x58)
[ 513.327809] [c02a23ac] (__blocking_notifier_call_chain+0x40/0x58) from
[c02a23d8] (blocking_notifier_call_chain+0x14/0x1c)
[ 513.339182] [c02a23d8] (blocking_notifier_call_chain+0x14/0x1c) from
[c0803c68] (cpufreq_set_policy+0xd4/0x2b8)
[ 513.349594] [c0803c68] (cpufreq_set_policy+0xd4/0x2b8) from [c0803e7c]
(cpufreq_init_policy+0x30/0x98)
[ 513.359231] [c0803e7c] (cpufreq_init_policy+0x30/0x98) from [c0805a18]
(__cpufreq_add_dev.isra.17+0x4dc/0x7a4)
[ 513.369560] [c0805a18] (__cpufreq_add_dev.isra.17+0x4dc/0x7a4) from
[c0805d38] (cpufreq_cpu_callback+0x58/0x84)
[ 513.379978] [c0805d38] (cpufreq_cpu_callback+0x58/0x84) from [c0afe180]
(notifier_call_chain+0x40/0x68)
[ 513.389704] [c0afe180] (notifier_call_chain+0x40/0x68) from [c02812dc]
(__cpu_notify+0x28/0x44)
[ 513.398728] [c02812dc] (__cpu_notify+0x28/0x44) from [c0aeed90]
(_cpu_up+0xf4/0x1dc)
[ 513.406797] [c0aeed90] (_cpu_up+0xf4/0x1dc) from [c0aeeed4]
(cpu_up+0x5c/0x78)
[ 513.414357] [c0aeeed4] (cpu_up+0x5c/0x78) from [c0aec808]
(store_online+0x44/0x74)
[ 513.422253] [c0aec808] (store_online+0x44/0x74) from [c03a40f4]
(sysfs_write_file+0x108/0x14c)
[ 513.431195] [c03a40f4] (sysfs_write_file+0x108/0x14c) from [c03517d4]
(vfs_write+0xd0/0x180)
[ 513.439958] [c03517d4] (vfs_write+0xd0/0x180) from [c0351ca8]
(SyS_write+0x38/0x68)
[ 513.447947] [c0351ca8] (SyS_write+0x38/0x68) from [c0205de0]
(ret_fast_syscall+0x0/0x30)
In this specific case, thread A set's CPU1's policy-governor in
cpufreq_init_policy() to NULL while thread B is using the policy-governor in
__cpufreq_governor().
Change-Id: I0f6f4e51ac3b7127a1ea56a1cb8e7ae1bcf8d6b6
Signed-off-by: Saravana Kannan skan...@codeaurora.org
---
drivers/cpufreq/cpufreq.c | 52 ---
1 file changed, 31 insertions(+), 21 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index cb003a6..5caefa9 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -849,7 +849,7 @@ static int cpufreq_add_dev_interface(struct cpufreq_policy
*policy,
goto err_out_kobj_put;
drv_attr++;
}
- if (cpufreq_driver-get) {
+ if (cpufreq_driver-get || policy-clk) {
ret = sysfs_create_file(policy-kobj, cpuinfo_cur_freq.attr);
if (ret)
goto err_out_kobj_put;
@@ -877,6 +877,22 @@ err_out_kobj_put:
return ret;
}
+static unsigned int __cpufreq_get_freq(struct cpufreq_policy *policy)
+{
+ unsigned long freq;
+
+ if (policy-clk) {
+ freq = clk_get_rate(policy-clk);
+ if(!IS_ERR_VALUE(freq))
+ return freq / 1000;
+ }
+
+ if (cpufreq_driver-get)
+ return cpufreq_driver-get(policy-cpu);
+
+ return 0;
+}
+
static void cpufreq_init_policy(struct cpufreq_policy *policy)
{
struct cpufreq_policy new_policy;
@@ -1109,17 +1125,10 @@ static int __cpufreq_add_dev(struct device *dev, struct
subsys_interface *sif,
goto err_set_policy_cpu;
}
- write_lock_irqsave(cpufreq_driver_lock, flags);
- for_each_cpu(j, policy-cpus)
- per_cpu(cpufreq_cpu_data, j) = policy;
- write_unlock_irqrestore(cpufreq_driver_lock, flags);
-
- if (cpufreq_driver-get) {
- policy-cur = cpufreq_driver-get(policy-cpu);
- if (!policy-cur) {
- pr_err(%s: -get() failed\n, __func__);
- goto err_get_freq;
- }
+ policy-cur = __cpufreq_get_freq(policy);
+ if (!policy-cur) {
+ pr_err(%s: get freq failed\n, __func__);
+
