Re: [PATCH 2/4] ARM: msm: Remove 7x00 support
On Wed, Oct 30, 2013 at 7:45 PM, Daniel Walker wrote: > On Wed, Oct 30, 2013 at 05:36:58PM -0700, Olof Johansson wrote: >> On Wed, Oct 30, 2013 at 4:25 PM, Daniel Walker wrote: >> >> > So the current users of those platforms are, what SOL ? >> >> What users? Show me one. > > What am I chop liver ? Ah, right. So, what platforms do you currently rely on mainline support for? Please be precise so we can figure out what needs to be kept and not. -Olof -- 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
On Wed, Oct 30, 2013 at 05:36:58PM -0700, Olof Johansson wrote: > On Wed, Oct 30, 2013 at 4:25 PM, Daniel Walker wrote: > > > So the current users of those platforms are, what SOL ? > > What users? Show me one. > What am I chop liver ? Daniel -- 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
On Wed, Oct 30, 2013 at 4:25 PM, Daniel Walker wrote: > So the current users of those platforms are, what SOL ? What users? Show me one. -Olof -- 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
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 ? Daniel -- 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: [PATCH v2 4/6] edac: Document Krait L1/L2 EDAC driver binding
On Oct 30, 2013, at 4:58 PM, Stephen Boyd wrote: > On 10/30/13 14:56, Kumar Gala wrote: >> On Oct 30, 2013, at 4:48 PM, Stephen Boyd wrote: >> >>> On 10/30/13 14:45, Kumar Gala wrote: On Oct 30, 2013, at 3:25 PM, Stephen Boyd wrote: > +l2-cache node containing the following properties: Is the L1 interrupt not per core L1 cache (even if they are OR together at PIC)? >>> Yes it is per CPU. That is what the 0xf part of the cpus interrupts >>> property is showing. >> Than why not have it in each cpu node? > > Because that duplicates things unnecessarily? The cpus node can hold > things that are common to all CPUs to avoid duplication. If it was a > different PPI for each CPU then I would agree that we need to put it in > each cpu node. Ok, I'll accept that as the binding is specific to Krait (and I assume all SoCs w/Krait wire this up to a common interrupt) - 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 v2 4/6] edac: Document Krait L1/L2 EDAC driver binding
On 10/30/13 14:56, Kumar Gala wrote: > On Oct 30, 2013, at 4:48 PM, Stephen Boyd wrote: > >> On 10/30/13 14:45, Kumar Gala wrote: >>> On Oct 30, 2013, at 3:25 PM, Stephen Boyd wrote: +l2-cache node containing the following properties: >>> Is the L1 interrupt not per core L1 cache (even if they are OR together at >>> PIC)? >> Yes it is per CPU. That is what the 0xf part of the cpus interrupts >> property is showing. > Than why not have it in each cpu node? Because that duplicates things unnecessarily? The cpus node can hold things that are common to all CPUs to avoid duplication. If it was a different PPI for each CPU then I would agree that we need to put it in each cpu node. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/6] edac: Document Krait L1/L2 EDAC driver binding
On Oct 30, 2013, at 4:48 PM, Stephen Boyd wrote:
> On 10/30/13 14:45, Kumar Gala wrote:
>> On Oct 30, 2013, at 3:25 PM, Stephen Boyd wrote:
>>
>>> @@ -75,3 +77,50 @@ Example:
>>> reg = <0x101>;
>>> };
>>> };
>>> +
>>> +If the compatible string contains "qcom,krait" there shall be an interrupts
>>> +property containing the L1/CPU error interrupt number. There shall also be
>>> an
>> 'also be a'
>
> ok
>
>>
>>> +l2-cache node containing the following properties:
>> Is the L1 interrupt not per core L1 cache (even if they are OR together at
>> PIC)?
>
> Yes it is per CPU. That is what the 0xf part of the cpus interrupts
> property is showing.
Than why not have it in each cpu node?
>>>
>>> + - compatible: Shall contain at least "cache"
>>> + - cache-level: Must be 2
>>> + - interrupts: Shall contain the L2 error interrupt
>>> +
>>> +Example:
>>> +
>>> + cpus {
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> + interrupts = <1 9 0xf04>;
>>> + compatible = "qcom,krait";
>>>
- 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 v2 4/6] edac: Document Krait L1/L2 EDAC driver binding
On 10/30/13 14:45, Kumar Gala wrote:
> On Oct 30, 2013, at 3:25 PM, Stephen Boyd wrote:
>
>> @@ -75,3 +77,50 @@ Example:
>> reg = <0x101>;
>> };
>> };
>> +
>> +If the compatible string contains "qcom,krait" there shall be an interrupts
>> +property containing the L1/CPU error interrupt number. There shall also be
>> an
> 'also be a'
ok
>
>> +l2-cache node containing the following properties:
> Is the L1 interrupt not per core L1 cache (even if they are OR together at
> PIC)?
Yes it is per CPU. That is what the 0xf part of the cpus interrupts
property is showing.
>
>> +
>> + - compatible: Shall contain at least "cache"
>> + - cache-level: Must be 2
>> + - interrupts: Shall contain the L2 error interrupt
>> +
>> +Example:
>> +
>> +cpus {
>> +#address-cells = <1>;
>> +#size-cells = <0>;
>> +interrupts = <1 9 0xf04>;
>> +compatible = "qcom,krait";
>>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/6] edac: Document Krait L1/L2 EDAC driver binding
On Oct 30, 2013, at 3:25 PM, Stephen Boyd wrote:
> The Krait L1/L2 error reporting device is made up of two
> interrupts, one per-CPU interrupt for the L1 caches and one
> interrupt for the L2 cache.
>
> Cc: Mark Rutland
> Cc: Kumar Gala
> Cc:
> Signed-off-by: Stephen Boyd
> ---
> Documentation/devicetree/bindings/arm/cpus.txt | 49 ++
> 1 file changed, 49 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt
> b/Documentation/devicetree/bindings/arm/cpus.txt
> index f32494d..0f7b27f 100644
> --- a/Documentation/devicetree/bindings/arm/cpus.txt
> +++ b/Documentation/devicetree/bindings/arm/cpus.txt
> @@ -44,6 +44,8 @@ For the ARM architecture every CPU node must contain the
> following properties:
> "marvell,mohawk"
> "marvell,xsc3"
> "marvell,xscale"
> + "qcom,scorpion"
> + "qcom,krait"
>
> Example:
>
> @@ -75,3 +77,50 @@ Example:
> reg = <0x101>;
> };
> };
> +
> +If the compatible string contains "qcom,krait" there shall be an interrupts
> +property containing the L1/CPU error interrupt number. There shall also be an
'also be a'
> +l2-cache node containing the following properties:
Is the L1 interrupt not per core L1 cache (even if they are OR together at PIC)?
> +
> + - compatible: Shall contain at least "cache"
> + - cache-level: Must be 2
> + - interrupts: Shall contain the L2 error interrupt
> +
> +Example:
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + interrupts = <1 9 0xf04>;
> + compatible = "qcom,krait";
> +
> + cpu@0 {
> + device_type = "cpu";
> + reg = <0>;
> + next-level-cache = <&L2>;
> + };
> +
> + cpu@1 {
> + device_type = "cpu";
> + reg = <1>;
> + next-level-cache = <&L2>;
> + };
> +
> + cpu@2 {
> + device_type = "cpu";
> + reg = <2>;
> + next-level-cache = <&L2>;
> + };
> +
> + cpu@3 {
> + device_type = "cpu";
> + reg = <3>;
> + next-level-cache = <&L2>;
> + };
> +
> + L2: l2-cache {
> + compatible = "cache";
> + cache-level = <2>;
> + interrupts = <0 2 0x4>;
> + };
> + };
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
>
--
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 1/2] dmaengine: add msm bam dma driver
On 10/30/13 13:31, Andy Gross wrote:
> On Tue, Oct 29, 2013 at 10:56:03AM -0700, Stephen Boyd wrote:
>> On 10/25, Andy Gross wrote:
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +
>>> +#include "dmaengine.h"
>>> +#include "msm_bam_dma_priv.h"
>> Why do we need this file? Can't we just put the #defines in this
>> file?
>
> There were enough definitions and structures to warrant another file.
>
Ah ok. I find it annoying to flip between two files but I guess that's
my problem.
>>> + if (!bdev) {
>>> + dev_err(&pdev->dev, "insufficient memory for private data\n");
>> kmalloc calls already print errors when they fail, so this can be
>> removed.
>
> has this always been the case?
The warning in the page allocator seems to have been there since pre-git
days (see __alloc_pages_slowpath() and how it calls warn_alloc_failed())
. Other warnings in the sl*b allocators seem to have come later (see
8bdec192b40cf7f7eec170b317c76089eb5eeddb for example).
--
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 1/2] dmaengine: add msm bam dma driver
On Tue, Oct 29, 2013 at 10:56:03AM -0700, Stephen Boyd wrote:
> On 10/25, Andy Gross wrote:
> > diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> > index f238cfd..a71b415 100644
> > --- a/drivers/dma/Kconfig
> > +++ b/drivers/dma/Kconfig
> > @@ -364,4 +364,13 @@ config DMATEST
> > Simple DMA test client. Say N unless you're debugging a
> > DMA Device driver.
> >
> > +
> > +config MSM_BAM_DMA
> > + tristate "MSM BAM DMA support"
> > + depends on ARCH_MSM
>
> It would be nice if we didn't have to rely on ARCH_MSM here so we
> get more build coverage.
I can remove that. There is nothing that forces this depend option.
> > + select DMA_ENGINE
> > + ---help---
> > + Enable support for the MSM BAM DMA controller. This controller
> > + provides DMA capabilities for a variety of on-chip devices.
> > +
> > endif
> > diff --git a/drivers/dma/msm_bam_dma.c b/drivers/dma/msm_bam_dma.c
> > new file mode 100644
> > index 000..d16bf94
> > --- /dev/null
> > +++ b/drivers/dma/msm_bam_dma.c
> > @@ -0,0 +1,840 @@
> > +/*
> > + * MSM BAM DMA engine driver
> > + *
> > + * Copyright (c) 2013, The Linux Foundation. All rights reserved.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 and
> > + * only version 2 as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
>
> interrupt.h is here twice
Will remove.
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include "dmaengine.h"
> > +#include "msm_bam_dma_priv.h"
>
> Why do we need this file? Can't we just put the #defines in this
> file?
There were enough definitions and structures to warrant another file.
> > +
> > +
> > +/*
>
> There should be two '*'s here for kernel-doc.
Ok. I'll conform.
> > + * bam_alloc_chan - Allocate channel resources for DMA channel.
> > + * @chan: specified channel
> > + *
> > + * This function allocates the FIFO descriptor memory and resets the
> > channel
> > + */
> > +static int bam_alloc_chan(struct dma_chan *chan)
> > +{
> > + struct bam_chan *bchan = to_bam_chan(chan);
> > + struct bam_device *bdev = bchan->device;
> > + u32 val;
> > + union bam_pipe_ctrl pctrl;
> > +
> > + /* check for channel activity */
> > + pctrl.value = ioread32(bdev->regs + BAM_P_CTRL(bchan->id));
>
> I recall io{read,write}*() being used for PCI and other ioport
> devices. If that's right then readl/writel would be more
> appropriate, and the *_relaxed variants would be even better if
> the correct memory barriers are also put in place.
I swear I ran across something about readl going legacy, which is why I went
with io{read,write}. The relaxed variants would definitely be better. I'll
switch over.
> > + if (pctrl.bits.p_en) {
> > + dev_err(bdev->dev, "channel already active\n");
> > + return -EINVAL;
> > + }
> > +
> > + /* allocate FIFO descriptor space */
> > + bchan->fifo_virt = (struct bam_desc_hw *)dma_alloc_coherent(bdev->dev,
>
> There isn't any need to cast from void *.
Missed this one and a couple of others. will fix.
> > + BAM_DESC_FIFO_SIZE, &bchan->fifo_phys,
> > + GFP_KERNEL);
> > +
> > + if (!bchan->fifo_virt) {
> > + dev_err(bdev->dev, "Failed to allocate descriptor fifo\n");
> > + return -ENOMEM;
> > + }
> > +
> > + /* reset channel */
> > + iowrite32(1, bdev->regs + BAM_P_RST(bchan->id));
> > + iowrite32(0, bdev->regs + BAM_P_RST(bchan->id));
> > +
> > + /* configure fifo address/size in bam channel registers */
> > + iowrite32(bchan->fifo_phys, bdev->regs +
> > + BAM_P_DESC_FIFO_ADDR(bchan->id));
> > + iowrite32(BAM_DESC_FIFO_SIZE, bdev->regs +
> > + BAM_P_FIFO_SIZES(bchan->id));
> > +
> > + /* unmask and enable interrupts for defined EE, bam and error irqs */
> > + iowrite32(BAM_IRQ_MSK, bdev->regs + BAM_IRQ_SRCS_EE(bchan->ee));
> > +
> > + /* enable the per pipe interrupts, enable EOT and INT irqs */
> > + iowrite32(P_DEFAULT_IRQS_EN, bdev->regs + BAM_P_IRQ_EN(bchan->id));
> > +
> > + /* unmask the specific pipe and EE combo */
> > + val = ioread32(bdev->regs + BAM_IRQ_SRCS_MSK_EE(bchan->ee));
> > + val |= 1 << bchan->id;
> > + iowrite32(val, bdev->regs + BAM_IRQ_SRCS_MSK_EE(bchan->ee));
> > +
> > + /* set fixed direction and mode, then enable channel */
> > + pctrl.value = 0;
> > + pctrl.bit
[PATCH v2 0/6] Krait L1/L2 EDAC driver
This patchset adds support for the Krait L1/L2 cache error detection hardware. The first patch fixes a generic framework bug. The next two patches lay the groundwork for this driver to be added by exporting percpu irq functions as well as adding the Krait l2 indirection register code. The next two patches add the driver and the binding and the final patch hooks it all up by adding the device tree node. I'm not sure which tree this is supposed to go through. Ideally we could send the first 3 plus the 5th one through an edac tree. The final dts changes could go through arm-soc via davidb's tree and the Documentation patch could go through the devicetree tree. Changes since v1: * Moved binding into cpus node * Picked up acks on first two patches * Commented krait l2 accessor functions Stephen Boyd (6): edac: Don't try to cancel workqueue when it's never setup genirq: export percpu irq functions for module usage ARM: Add Krait L2 accessor functions edac: Document Krait L1/L2 EDAC driver binding edac: Add support for Krait CPU cache error detection ARM: dts: msm: Add Krait CPU/L2 nodes Documentation/devicetree/bindings/arm/cpus.txt | 49 arch/arm/boot/dts/qcom-msm8974.dtsi| 37 +++ arch/arm/common/Kconfig| 3 + arch/arm/common/Makefile | 1 + arch/arm/common/krait-l2-accessors.c | 58 + arch/arm/include/asm/krait-l2-accessors.h | 20 ++ drivers/edac/Kconfig | 8 + drivers/edac/Makefile | 2 + drivers/edac/edac_device.c | 3 + drivers/edac/krait_edac.c | 346 + kernel/irq/manage.c| 2 + 11 files changed, 529 insertions(+) create mode 100644 arch/arm/common/krait-l2-accessors.c create mode 100644 arch/arm/include/asm/krait-l2-accessors.h create mode 100644 drivers/edac/krait_edac.c -- 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 v2 6/6] ARM: dts: msm: Add Krait CPU/L2 nodes
On Wed, Oct 30, 2013 at 01:25:36PM -0700, Stephen Boyd wrote: This allows us to probe the krait-edac driver. Cc: David Brown Signed-off-by: Stephen Boyd --- arch/arm/boot/dts/qcom-msm8974.dtsi | 37 + 1 file changed, 37 insertions(+) Acked-by: David Brown -- 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
[PATCH v2 4/6] edac: Document Krait L1/L2 EDAC driver binding
The Krait L1/L2 error reporting device is made up of two
interrupts, one per-CPU interrupt for the L1 caches and one
interrupt for the L2 cache.
Cc: Mark Rutland
Cc: Kumar Gala
Cc:
Signed-off-by: Stephen Boyd
---
Documentation/devicetree/bindings/arm/cpus.txt | 49 ++
1 file changed, 49 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/cpus.txt
b/Documentation/devicetree/bindings/arm/cpus.txt
index f32494d..0f7b27f 100644
--- a/Documentation/devicetree/bindings/arm/cpus.txt
+++ b/Documentation/devicetree/bindings/arm/cpus.txt
@@ -44,6 +44,8 @@ For the ARM architecture every CPU node must contain the
following properties:
"marvell,mohawk"
"marvell,xsc3"
"marvell,xscale"
+ "qcom,scorpion"
+ "qcom,krait"
Example:
@@ -75,3 +77,50 @@ Example:
reg = <0x101>;
};
};
+
+If the compatible string contains "qcom,krait" there shall be an interrupts
+property containing the L1/CPU error interrupt number. There shall also be an
+l2-cache node containing the following properties:
+
+ - compatible: Shall contain at least "cache"
+ - cache-level: Must be 2
+ - interrupts: Shall contain the L2 error interrupt
+
+Example:
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ interrupts = <1 9 0xf04>;
+ compatible = "qcom,krait";
+
+ cpu@0 {
+ device_type = "cpu";
+ reg = <0>;
+ next-level-cache = <&L2>;
+ };
+
+ cpu@1 {
+ device_type = "cpu";
+ reg = <1>;
+ next-level-cache = <&L2>;
+ };
+
+ cpu@2 {
+ device_type = "cpu";
+ reg = <2>;
+ next-level-cache = <&L2>;
+ };
+
+ cpu@3 {
+ device_type = "cpu";
+ reg = <3>;
+ next-level-cache = <&L2>;
+ };
+
+ L2: l2-cache {
+ compatible = "cache";
+ cache-level = <2>;
+ interrupts = <0 2 0x4>;
+ };
+ };
--
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
[PATCH v2 2/6] genirq: export percpu irq functions for module usage
In the near future we're going to use these percpu irq functions in the Krait CPU EDAC driver. Export them so that the EDAC driver can be compiled as a module. Acked-by: Thomas Gleixner Signed-off-by: Stephen Boyd --- kernel/irq/manage.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c index 514bcfd..c6009d1 100644 --- a/kernel/irq/manage.c +++ b/kernel/irq/manage.c @@ -1623,6 +1623,7 @@ void free_percpu_irq(unsigned int irq, void __percpu *dev_id) kfree(__free_percpu_irq(irq, dev_id)); chip_bus_sync_unlock(desc); } +EXPORT_SYMBOL_GPL(free_percpu_irq); /** * setup_percpu_irq - setup a per-cpu interrupt @@ -1693,3 +1694,4 @@ int request_percpu_irq(unsigned int irq, irq_handler_t handler, return retval; } +EXPORT_SYMBOL_GPL(request_percpu_irq); -- 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
[PATCH v2 5/6] edac: Add support for Krait CPU cache error detection
Add support for the Krait CPU cache error detection. This is a
simplified version of the code originally written by Stepan
Moskovchenko[1] ported to the EDAC device framework.
[1]
https://www.codeaurora.org/cgit/quic/la/kernel/msm/tree/arch/arm/mach-msm/cache_erp.c?h=msm-3.4
Cc: Stepan Moskovchenko
Signed-off-by: Stephen Boyd
---
drivers/edac/Kconfig | 8 ++
drivers/edac/Makefile | 2 +
drivers/edac/krait_edac.c | 346 ++
3 files changed, 356 insertions(+)
create mode 100644 drivers/edac/krait_edac.c
diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 878f090..68f612d 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -368,4 +368,12 @@ config EDAC_OCTEON_PCI
Support for error detection and correction on the
Cavium Octeon family of SOCs.
+config EDAC_KRAIT_CACHE
+ tristate "Krait L1/L2 Cache"
+ depends on EDAC_MM_EDAC && ARCH_MSM
+ select KRAIT_L2_ACCESSORS
+ help
+ Support for error detection and correction on the
+ Krait L1/L2 cache controller.
+
endif # EDAC
diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
index 4154ed6..b6ea505 100644
--- a/drivers/edac/Makefile
+++ b/drivers/edac/Makefile
@@ -64,3 +64,5 @@ obj-$(CONFIG_EDAC_OCTEON_PC) += octeon_edac-pc.o
obj-$(CONFIG_EDAC_OCTEON_L2C) += octeon_edac-l2c.o
obj-$(CONFIG_EDAC_OCTEON_LMC) += octeon_edac-lmc.o
obj-$(CONFIG_EDAC_OCTEON_PCI) += octeon_edac-pci.o
+
+obj-$(CONFIG_EDAC_KRAIT_CACHE) += krait_edac.o
diff --git a/drivers/edac/krait_edac.c b/drivers/edac/krait_edac.c
new file mode 100644
index 000..b1d9f52
--- /dev/null
+++ b/drivers/edac/krait_edac.c
@@ -0,0 +1,346 @@
+/* Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#include "edac_core.h"
+
+#define CESR_DCTPE BIT(0)
+#define CESR_DCDPE BIT(1)
+#define CESR_ICTPE BIT(2)
+#define CESR_ICDPE BIT(3)
+#define CESR_DCTE (BIT(4) | BIT(5))
+#define CESR_ICTE (BIT(6) | BIT(7))
+#define CESR_TLBMH BIT(16)
+#define CESR_I_MASK0x00cc
+/* Print a message for everything but TLB MH events */
+#define CESR_PRINT_MASK0x00ff
+
+#define L2ESR 0x204
+#define L2ESR_MPDCDBIT(0)
+#define L2ESR_MPSLVBIT(1)
+#define L2ESR_TSESBBIT(2)
+#define L2ESR_TSEDBBIT(3)
+#define L2ESR_DSESBBIT(4)
+#define L2ESR_DSEDBBIT(5)
+#define L2ESR_MSE BIT(6)
+#define L2ESR_MPLDREXNOK BIT(8)
+#define L2ESR_CPU_MASK 0xf
+#define L2ESR_CPU_SHIFT16
+#define L2ESR_SP BIT(20)
+
+#define L2ESYNR0 0x208
+#define L2ESYNR1 0x209
+#define L2EAR0 0x20c
+#define L2EAR1 0x20d
+
+struct krait_edac {
+ int l1_irq;
+ struct edac_device_ctl_info * __percpu *edev;
+ struct notifier_block notifier;
+};
+
+struct krait_edac_error {
+ const char * const msg;
+ void (*func)(struct edac_device_ctl_info *edac_dev,
+ int inst_nr, int block_nr, const char *msg);
+};
+
+static unsigned int read_cesr(void)
+{
+ unsigned int cesr;
+
+ asm volatile ("mrc p15, 7, %0, c15, c0, 1 @ cesr" : "=r" (cesr));
+ return cesr;
+}
+
+static void write_cesr(unsigned int cesr)
+{
+ asm volatile ("mcr p15, 7, %0, c15, c0, 1 @ cesr" : : "r" (cesr));
+}
+
+static unsigned int read_cesynr(void)
+{
+ unsigned int cesynr;
+
+ asm volatile ("mrc p15, 7, %0, c15, c0, 3 @ cesynr" : "=r" (cesynr));
+ return cesynr;
+}
+
+static irqreturn_t krait_l1_irq(int irq, void *dev_id)
+{
+ struct edac_device_ctl_info *edac = dev_id;
+ unsigned int cesr = read_cesr();
+ unsigned int i_cesynr, d_cesynr;
+ unsigned int cpu = smp_processor_id();
+ int print_regs = cesr & CESR_PRINT_MASK;
+ int i;
+ static const struct krait_edac_error errors[] = {
+ { "D-cache tag parity error", edac_device_handle_ue },
+ { "D-cache data parity error", edac_device_handle_ue },
+ { "I-cache tag parity error", edac_device_handle_ce },
+ { "I-cache data parity error", edac_device_handle_ce },
+ { "
[PATCH v2 1/6] edac: Don't try to cancel workqueue when it's never setup
We only setup a workqueue for edac devices that use the polling
method. We still try to cancel the workqueue if an edac_device
uses the irq method though. This causes a warning from debug
objects when we remove an edac device:
WARNING: CPU: 0 PID: 56 at lib/debugobjects.c:260 debug_print_object+0x98/0xc0()
ODEBUG: assert_init not available (active state 0) object type: timer_list
hint: stub_timer+0x0/0x28
Modules linked in: krait_edac(-)
CPU: 0 PID: 56 Comm: rmmod Not tainted 3.12.0-rc2-00035-g15292a0 #69
(unwind_backtrace+0x0/0x144)
(show_stack+0x20/0x24)
(dump_stack+0x74/0xb4)
(warn_slowpath_common+0x78/0x9c)
(warn_slowpath_fmt+0x40/0x48)
(debug_print_object+0x98/0xc0)
(debug_object_assert_init+0xdc/0xec)
(del_timer+0x24/0x7c)
(try_to_grab_pending+0xc0/0x1b0)
(cancel_delayed_work+0x2c/0xa0)
(edac_device_workq_teardown+0x1c/0x38)
(edac_device_del_device+0xb8/0xe4)
(krait_edac_remove+0x50/0x70 [krait_edac])
(platform_drv_remove+0x24/0x28)
(__device_release_driver+0x68/0xc0)
(driver_detach+0xc4/0xc8)
(bus_remove_driver+0xac/0x114)
(driver_unregister+0x38/0x58)
(platform_driver_unregister+0x1c/0x20)
(krait_edac_driver_exit+0x14/0x1c [krait_edac])
(SyS_delete_module+0x178/0x2b4)
Fix it by skipping the workqueue teardown for such devices.
Acked-by: Borislav Petkov
Signed-off-by: Stephen Boyd
---
drivers/edac/edac_device.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/edac/edac_device.c b/drivers/edac/edac_device.c
index 211021d..203561b 100644
--- a/drivers/edac/edac_device.c
+++ b/drivers/edac/edac_device.c
@@ -437,6 +437,9 @@ void edac_device_workq_teardown(struct edac_device_ctl_info
*edac_dev)
{
int status;
+ if (!edac_dev->edac_check)
+ return;
+
status = cancel_delayed_work(&edac_dev->work);
if (status == 0) {
/* workq instance might be running, wait for it */
--
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
[PATCH v2 6/6] ARM: dts: msm: Add Krait CPU/L2 nodes
This allows us to probe the krait-edac driver.
Cc: David Brown
Signed-off-by: Stephen Boyd
---
arch/arm/boot/dts/qcom-msm8974.dtsi | 37 +
1 file changed, 37 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi
b/arch/arm/boot/dts/qcom-msm8974.dtsi
index 152879d..bfe8b9c 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -9,6 +9,43 @@
compatible = "qcom,msm8974";
interrupt-parent = <&intc>;
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ interrupts = <1 9 0xf04>;
+ compatible = "qcom,krait";
+
+ cpu@0 {
+ device_type = "cpu";
+ reg = <0>;
+ next-level-cache = <&L2>;
+ };
+
+ cpu@1 {
+ device_type = "cpu";
+ reg = <1>;
+ next-level-cache = <&L2>;
+ };
+
+ cpu@2 {
+ device_type = "cpu";
+ reg = <2>;
+ next-level-cache = <&L2>;
+ };
+
+ cpu@3 {
+ device_type = "cpu";
+ reg = <3>;
+ next-level-cache = <&L2>;
+ };
+
+ L2: l2-cache {
+ compatible = "cache";
+ cache-level = <2>;
+ interrupts = <0 2 0x4>;
+ };
+ };
+
soc: soc {
#address-cells = <1>;
#size-cells = <1>;
--
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
[PATCH v2 3/6] ARM: Add Krait L2 accessor functions
Qualcomm's Krait CPUs have a handful of L2 cache controller
registers that live behind a cp15 based indirection register.
First you program the indirection register (l2cpselr) to point
the L2 'window' register (l2cpdr) at what you want to read/write.
Then you read/write the 'window' register to do what you want.
The l2cpselr register is not banked per-cpu so we must lock
around accesses to it to prevent other CPUs from re-pointing
l2cpdr underneath us.
Cc: Mark Rutland
Cc: Russell King
Signed-off-by: Stephen Boyd
---
arch/arm/common/Kconfig | 3 ++
arch/arm/common/Makefile | 1 +
arch/arm/common/krait-l2-accessors.c | 58 +++
arch/arm/include/asm/krait-l2-accessors.h | 20 +++
4 files changed, 82 insertions(+)
create mode 100644 arch/arm/common/krait-l2-accessors.c
create mode 100644 arch/arm/include/asm/krait-l2-accessors.h
diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig
index c3a4e9c..9da52dc 100644
--- a/arch/arm/common/Kconfig
+++ b/arch/arm/common/Kconfig
@@ -9,6 +9,9 @@ config DMABOUNCE
bool
select ZONE_DMA
+config KRAIT_L2_ACCESSORS
+ bool
+
config SHARP_LOCOMO
bool
diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
index 8c60f47..606131a 100644
--- a/arch/arm/common/Makefile
+++ b/arch/arm/common/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_ICST) += icst.o
obj-$(CONFIG_SA) += sa.o
obj-$(CONFIG_PCI_HOST_VIA82C505) += via82c505.o
obj-$(CONFIG_DMABOUNCE)+= dmabounce.o
+obj-$(CONFIG_KRAIT_L2_ACCESSORS) += krait-l2-accessors.o
obj-$(CONFIG_SHARP_LOCOMO) += locomo.o
obj-$(CONFIG_SHARP_PARAM) += sharpsl_param.o
obj-$(CONFIG_SHARP_SCOOP) += scoop.o
diff --git a/arch/arm/common/krait-l2-accessors.c
b/arch/arm/common/krait-l2-accessors.c
new file mode 100644
index 000..f17c361
--- /dev/null
+++ b/arch/arm/common/krait-l2-accessors.c
@@ -0,0 +1,58 @@
+/*
+ * Copyright (c) 2011-2013, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include
+
+#include
+#include
+
+static DEFINE_RAW_SPINLOCK(krait_l2_lock);
+
+void set_l2_indirect_reg(u32 addr, u32 val)
+{
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&krait_l2_lock, flags);
+ /*
+* Select the L2 window by poking l2cpselr, then write to the window
+* via l2cpdr.
+*/
+ asm volatile ("mcr p15, 3, %0, c15, c0, 6 @ l2cpselr" : : "r" (addr));
+ isb();
+ asm volatile ("mcr p15, 3, %0, c15, c0, 7 @ l2cpdr" : : "r" (val));
+ isb();
+
+ raw_spin_unlock_irqrestore(&krait_l2_lock, flags);
+}
+EXPORT_SYMBOL(set_l2_indirect_reg);
+
+u32 get_l2_indirect_reg(u32 addr)
+{
+ u32 val;
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&krait_l2_lock, flags);
+ /*
+* Select the L2 window by poking l2cpselr, then read from the window
+* via l2cpdr.
+*/
+ asm volatile ("mcr p15, 3, %0, c15, c0, 6 @ l2cpselr" : : "r" (addr));
+ isb();
+ asm volatile ("mrc p15, 3, %0, c15, c0, 7 @ l2cpdr" : "=r" (val));
+
+ raw_spin_unlock_irqrestore(&krait_l2_lock, flags);
+
+ return val;
+}
+EXPORT_SYMBOL(get_l2_indirect_reg);
diff --git a/arch/arm/include/asm/krait-l2-accessors.h
b/arch/arm/include/asm/krait-l2-accessors.h
new file mode 100644
index 000..d5305c4
--- /dev/null
+++ b/arch/arm/include/asm/krait-l2-accessors.h
@@ -0,0 +1,20 @@
+/*
+ * Copyright (c) 2011-2013, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __ASMARM_KRAIT_L2_ACCESSORS_H
+#define __ASMARM_KRAIT_L2_ACCESSORS_H
+
+extern void set_l2_indirect_reg(u32 addr, u32 val);
+extern u32 get_l2_indirect_reg(u32 addr);
+
+#endif
--
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.
Re: [PATCH v3 02/10] spmi: Linux driver framework for SPMI
On 10/30/13 12:37, Josh Cartwright wrote:
> On Tue, Oct 29, 2013 at 09:52:15AM -0700, Stephen Boyd wrote:
>> On 10/28/13 11:12, Josh Cartwright wrote:
>>> +int spmi_register_read(struct spmi_device *sdev, u8 addr, u8 *buf)
>>> +{
>>> + /* 5-bit register address */
>>> + if (addr > 0x1F)
>> Perhaps 0x1f should be a #define.
> Is 0x1F with the comment above it not clear enough?
It triggered my 'magic number used twice' alarm. I'm ok with it either way.
>>> +struct spmi_controller *spmi_controller_alloc(struct device *parent,
>>> + size_t size)
>>> +{
>>> + struct spmi_controller *ctrl;
>>> + int id;
>>> +
>>> + if (WARN_ON(!parent))
>>> + return NULL;
>>> +
>>> + ctrl = kzalloc(sizeof(*ctrl) + size, GFP_KERNEL);
>>> + if (!ctrl)
>>> + return NULL;
>>> +
>>> + device_initialize(&ctrl->dev);
>>> + ctrl->dev.type = &spmi_ctrl_type;
>>> + ctrl->dev.bus = &spmi_bus_type;
>>> + ctrl->dev.parent = parent;
>>> + ctrl->dev.of_node = parent->of_node;
>>> + spmi_controller_set_drvdata(ctrl, &ctrl[1]);
>>> +
>>> + idr_preload(GFP_KERNEL);
>>> + mutex_lock(&ctrl_idr_lock);
>>> + id = idr_alloc(&ctrl_idr, ctrl, 0, 0, GFP_NOWAIT);
>>> + mutex_unlock(&ctrl_idr_lock);
>>> + idr_preload_end();
>> This can just be:
>>
>> mutex_lock(&ctrl_idr_lock);
>> id = idr_alloc(&ctrl_idr, ctrl, 0, 0, GFP_KERNEL);
>> mutex_unlock(&ctrl_idr_lock);
> Actually, I'm wondering if it's just easier to leverage the simpler
> ida_simple_* APIs instead.
Yes that also works.
--
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 02/10] spmi: Linux driver framework for SPMI
On Tue, Oct 29, 2013 at 09:52:15AM -0700, Stephen Boyd wrote:
> On 10/28/13 11:12, Josh Cartwright wrote:
> > diff --git a/drivers/spmi/Kconfig b/drivers/spmi/Kconfig
> > new file mode 100644
> > index 000..a03835f
> > --- /dev/null
> > +++ b/drivers/spmi/Kconfig
> > @@ -0,0 +1,9 @@
> > +#
> > +# SPMI driver configuration
> > +#
> > +menuconfig SPMI
> > + bool "SPMI support"
>
> Can we do tristate?
I don't think there is any reason why we can't do tristate here. I do
foresee in the future, however, that we'll run into ordering/dependency
problems when we get the regulator drivers in place.
I suppose we'll wait until we get there to deal with those.
> > + help
> > + SPMI (System Power Management Interface) is a two-wire
> > + serial interface between baseband and application processors
> > + and Power Management Integrated Circuits (PMIC).
> > diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
> > new file mode 100644
> > index 000..0780bd4
> > --- /dev/null
> > +++ b/drivers/spmi/spmi.c
> > @@ -0,0 +1,491 @@
> > +/* Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 and
> > + * only version 2 as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include
> > +
> > +static DEFINE_MUTEX(ctrl_idr_lock);
> > +static DEFINE_IDR(ctrl_idr);
> > +
> > +static void spmi_dev_release(struct device *dev)
> > +{
> > + struct spmi_device *sdev = to_spmi_device(dev);
> > + kfree(sdev);
> > +}
> > +
> > +static struct device_type spmi_dev_type = {
> > + .release= spmi_dev_release,
>
> const?
>
[..]
> > +static struct device_type spmi_ctrl_type = {
>
>
> const?
Yep. Thanks.
[..]
> > +int spmi_register_read(struct spmi_device *sdev, u8 addr, u8 *buf)
> > +{
> > + /* 5-bit register address */
> > + if (addr > 0x1F)
>
> Perhaps 0x1f should be a #define.
Is 0x1F with the comment above it not clear enough?
> > + return -EINVAL;
> > +
> > + return spmi_read_cmd(sdev->ctrl, SPMI_CMD_READ, sdev->usid, addr, 0,
> > +buf);
> > +}
> > +EXPORT_SYMBOL_GPL(spmi_register_read);
> > +
> [...]
> > +struct spmi_controller *spmi_controller_alloc(struct device *parent,
> > + size_t size)
> > +{
> > + struct spmi_controller *ctrl;
> > + int id;
> > +
> > + if (WARN_ON(!parent))
> > + return NULL;
> > +
> > + ctrl = kzalloc(sizeof(*ctrl) + size, GFP_KERNEL);
> > + if (!ctrl)
> > + return NULL;
> > +
> > + device_initialize(&ctrl->dev);
> > + ctrl->dev.type = &spmi_ctrl_type;
> > + ctrl->dev.bus = &spmi_bus_type;
> > + ctrl->dev.parent = parent;
> > + ctrl->dev.of_node = parent->of_node;
> > + spmi_controller_set_drvdata(ctrl, &ctrl[1]);
> > +
> > + idr_preload(GFP_KERNEL);
> > + mutex_lock(&ctrl_idr_lock);
> > + id = idr_alloc(&ctrl_idr, ctrl, 0, 0, GFP_NOWAIT);
> > + mutex_unlock(&ctrl_idr_lock);
> > + idr_preload_end();
>
> This can just be:
>
> mutex_lock(&ctrl_idr_lock);
> id = idr_alloc(&ctrl_idr, ctrl, 0, 0, GFP_KERNEL);
> mutex_unlock(&ctrl_idr_lock);
Actually, I'm wondering if it's just easier to leverage the simpler
ida_simple_* APIs instead.
> > +
> > + if (id < 0) {
> > + dev_err(parent,
> > + "unable to allocate SPMI controller identifier.\n");
> > + spmi_controller_put(ctrl);
> > + return NULL;
> > + }
> > +
> > + ctrl->nr = id;
> > + dev_set_name(&ctrl->dev, "spmi-%d", id);
> > +
> > + dev_dbg(&ctrl->dev, "allocated controller 0x%p id %d\n", ctrl, id);
> > + return ctrl;
> > +}
> > +EXPORT_SYMBOL_GPL(spmi_controller_alloc);
> > +
> > +static int of_spmi_register_devices(struct spmi_controller *ctrl)
> > +{
> > + struct device_node *node;
> > + int err;
> > +
> > + dev_dbg(&ctrl->dev, "of_spmi_register_devices\n");
>
> Could be
>
> dev_dbg(&ctrl->dev, "%s", __func__);
>
> so that function renaming is transparent.
Some of these dev_dbg()'s (like this one) can probably be just be
removed, especially because we have an additional dev_dbg() right below
this one...
> > +
> > + if (!ctrl->dev.of_node)
> > + return -ENODEV;
> > +
> > + dev_dbg(&ctrl->dev, "looping through children\n");
> > +
> > + for_each_available_child_of_node(ctrl->dev.of_node, node) {
> > + struct spmi_device *sdev;
> > + u32 reg[2];
> > +
> > +
Re: [PATCH v3 04/10] spmi: Add MSM PMIC Arbiter SPMI controller
On Wed, Oct 30, 2013 at 11:05:36AM -0700, Stephen Boyd wrote:
> On 10/28, Josh Cartwright wrote:
> > +
> > +/**
> > + * pa_write_data: write 1..4 bytes from buf to pmic-arb's register
> > + * @bc byte-count -1. range: 0..3
> > + * @reg register's address
> > + * @buf buffer to write. length must be bc+1
>
> Missing colon between variable and description.
I'll clean the documentation up a bit and push it all to the
implementation (as suggested in a previous review).
[..]
> > + if (bc >= PMIC_ARB_MAX_TRANS_BYTES) {
> > + dev_err(&ctrl->dev,
> > + "pmic-arb supports 1..%d bytes per trans, but:%d
> > requested",
>
> Nitpick: Please replace the colon between but and %d with a space.
>
> > + PMIC_ARB_MAX_TRANS_BYTES, bc+1);
>
> Space around that '+' please.
Sure.
> > + return -EINVAL;
> > + }
> > + dev_dbg(&ctrl->dev,
> > + "op:0x%x sid:%d bc:%d addr:0x%x\n", opc, sid, bc, addr);
> > +
> > + /* Check the opcode */
> > + if (opc >= 0x60 && opc <= 0x7F)
> > + opc = PMIC_ARB_OP_READ;
> > + else if (opc >= 0x20 && opc <= 0x2F)
> > + opc = PMIC_ARB_OP_EXT_READ;
> > + else if (opc >= 0x38 && opc <= 0x3F)
> > + opc = PMIC_ARB_OP_EXT_READL;
> > + else
> > + return -EINVAL;
> > +
> > + cmd = (opc << 27) | ((sid & 0xf) << 20) | (addr << 4) | (bc & 0x7);
> > +
> > + spin_lock_irqsave(&pmic_arb->lock, flags);
> > + pmic_arb_base_write(pmic_arb, PMIC_ARB_CMD(pmic_arb->channel), cmd);
> > + rc = pmic_arb_wait_for_done(ctrl);
> > + if (rc)
> > + goto done;
> > +
> > + /* Read from FIFO, note 'bc' is actually number of bytes minus 1 */
> > + pa_read_data(pmic_arb, buf, PMIC_ARB_RDATA0(pmic_arb->channel)
> > + , min_t(u8, bc, 3));
>
> Nitpick: Weird comma starting a line here.
Okay.
[..]
> > +static int __exit spmi_pmic_arb_remove(struct platform_device *pdev)
>
> __exit shouldn't be here. We want this function in modules.
>
> > +{
> > + struct spmi_controller *ctrl = platform_get_drvdata(pdev);
> > + spmi_controller_remove(ctrl);
> > + return 0;
> > +}
> > +
> > +static struct of_device_id spmi_pmic_arb_match_table[] = {
> > + { .compatible = "qcom,spmi-pmic-arb", },
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(of, spmi_pmic_arb_match_table);
> > +
> > +static struct platform_driver spmi_pmic_arb_driver = {
> > + .probe = spmi_pmic_arb_probe,
> > + .remove = __exit_p(spmi_pmic_arb_remove),
>
> Please drop this __exit_p() usage as well.
>
> > + .driver = {
> > + .name = "spmi_pmic_arb",
> > + .owner = THIS_MODULE,
> > + .of_match_table = spmi_pmic_arb_match_table,
> > + },
> > +};
> > +module_platform_driver(spmi_pmic_arb_driver);
>
> MODULE_LICENSE()
> MODULE_ALIAS()
Yep, will re-add. Not sure why I dropped these...
--
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 05/10] spmi: pmic_arb: add support for interrupt handling
Stephen-
Thanks for all the comments.
On Wed, Oct 30, 2013 at 11:17:55AM -0700, Stephen Boyd wrote:
> On 10/28, Josh Cartwright wrote:
> > @@ -108,12 +111,17 @@ struct spmi_pmic_arb_dev {
> > void __iomem*base;
> > void __iomem*intr;
> > void __iomem*cnfg;
> > + unsigned intirq;
> > boolallow_wakeup;
> > spinlock_t lock;
> > u8 channel;
> > u8 min_apid;
> > u8 max_apid;
> > u32 mapping_table[SPMI_MAPPING_TABLE_LEN];
> > + int bus_nr;
>
> This looks unused.
Indeed, will remove (I think this is a holdout from the split qpnpint
handling that exists in the vendor tree).
> > + struct irq_domain *domain;
> > + struct spmi_controller *spmic;
> > + u16 apid_to_ppid[256];
> > };
> >
> > static inline u32 pmic_arb_base_read(struct spmi_pmic_arb_dev *dev, u32
> > offset)
> [...]
> > +
> > +static void pmic_arb_chained_irq(unsigned int irq, struct irq_desc *desc)
> > +{
> > + struct spmi_pmic_arb_dev *pa = irq_get_handler_data(irq);
> > + struct irq_chip *chip = irq_get_chip(irq);
> > + void __iomem *intr = pa->intr;
> > + int first = pa->min_apid >> 5;
> > + int last = pa->max_apid >> 5;
> > + int i, id;
> > + u8 ee = 0; /*pa->owner;*/
>
> TODO?
Indeed, I removed this for some reason awhile back, but this really
should be put back into place, and the EE should be a required property
in the bindings.
[..]
> > +static int qpnpint_irq_domain_dt_translate(struct irq_domain *d,
> > + struct device_node *controller,
> > + const u32 *intspec,
> > + unsigned int intsize,
> > + unsigned long *out_hwirq,
> > + unsigned int *out_type)
> > +{
> > + struct spmi_pmic_arb_dev *pa = d->host_data;
> > + struct spmi_pmic_arb_irq_spec spec;
> > + int err;
> > + u8 apid;
> > +
> > + dev_dbg(&pa->spmic->dev,
> > + "intspec[0] 0x%1x intspec[1] 0x%02x intspec[2] 0x%02x\n",
> > + intspec[0], intspec[1], intspec[2]);
> > +
> > + if (d->of_node != controller)
> > + return -EINVAL;
> > + if (intsize != 4)
> > + return -EINVAL;
> > + if (intspec[0] > 0xF || intspec[1] > 0xFF || intspec[2] > 0x7)
> > + return -EINVAL;
> > +
> > + spec.slave = intspec[0];
> > + spec.per = intspec[1];
> > + spec.irq = intspec[2];
> > +
> > + err = search_mapping_table(pa, &spec, &apid);
> > + if (err)
> > + return err;
> > +
> > + pa->apid_to_ppid[apid] = spec.slave << 8 | spec.per;
> > +
> > + /* Keep track of {max,min}_apid for bounding search during interrupt */
> > + if (apid > pa->max_apid)
> > + pa->max_apid = apid;
> > + if (apid < pa->min_apid)
> > + pa->min_apid = apid;
>
> Ah makes sense now why we set this to the opposite values in
> probe. Please put a comment in patch 4 and maybe move that setup
> in patch 4 to this patch.
Indeed, I'll move it and add a comment at init-time.
> > +
> > + *out_hwirq = spec.slave << 24
> > + | spec.per << 16
> > + | spec.irq << 8
> > + | apid;
> > + *out_type = intspec[3] & IRQ_TYPE_SENSE_MASK;
> > +
> > + dev_dbg(&pa->spmic->dev, "out_hwirq = %lu\n", *out_hwirq);
> > +
> > + return 0;
> > +}
> > +
> [...]
> > static int spmi_pmic_arb_probe(struct platform_device *pdev)
> > {
> > struct spmi_pmic_arb_dev *pa;
> > struct spmi_controller *ctrl;
> > struct resource *res;
> > - int err, i;
> > + int err = 0, i;
> >
> > ctrl = spmi_controller_alloc(&pdev->dev, sizeof(*pa));
> > if (!ctrl)
> > return -ENOMEM;
> >
> > pa = spmi_controller_get_drvdata(ctrl);
> > + pa->spmic = ctrl;
> >
> > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "core");
> > pa->base = devm_ioremap_resource(&ctrl->dev, res);
> > @@ -349,6 +679,12 @@ static int spmi_pmic_arb_probe(struct platform_device
> > *pdev)
> > goto err_put_ctrl;
> > }
> >
> > + pa->irq = platform_get_irq(pdev, 0);
> > + if (pa->irq < 0) {
> > + err = pa->irq;
> > + goto err_put_ctrl;
> > + }
> > +
> > for (i = 0; i < ARRAY_SIZE(pa->mapping_table); ++i)
> > pa->mapping_table[i] = readl_relaxed(
> > pa->cnfg + SPMI_MAPPING_TABLE_REG(i));
> > @@ -364,15 +700,30 @@ static int spmi_pmic_arb_probe(struct platform_device
> > *pdev)
> > ctrl->read_cmd = pmic_arb_read_cmd;
> > ctrl->write_cmd = pmic_arb_write_cmd;
> >
> > + dev_dbg(&pdev->dev, "adding irq domain\n");
> > + pa->domain = irq_domain_add_tree(pdev->dev.of_node,
> > +&pmic_arb_irq_domain_ops, pa);
> > + if
Re: [PATCH v3 05/10] spmi: pmic_arb: add support for interrupt handling
On 10/28, Josh Cartwright wrote:
> @@ -108,12 +111,17 @@ struct spmi_pmic_arb_dev {
> void __iomem*base;
> void __iomem*intr;
> void __iomem*cnfg;
> + unsigned intirq;
> boolallow_wakeup;
> spinlock_t lock;
> u8 channel;
> u8 min_apid;
> u8 max_apid;
> u32 mapping_table[SPMI_MAPPING_TABLE_LEN];
> + int bus_nr;
This looks unused.
> + struct irq_domain *domain;
> + struct spmi_controller *spmic;
> + u16 apid_to_ppid[256];
> };
>
> static inline u32 pmic_arb_base_read(struct spmi_pmic_arb_dev *dev, u32
> offset)
[...]
> +
> +static void pmic_arb_chained_irq(unsigned int irq, struct irq_desc *desc)
> +{
> + struct spmi_pmic_arb_dev *pa = irq_get_handler_data(irq);
> + struct irq_chip *chip = irq_get_chip(irq);
> + void __iomem *intr = pa->intr;
> + int first = pa->min_apid >> 5;
> + int last = pa->max_apid >> 5;
> + int i, id;
> + u8 ee = 0; /*pa->owner;*/
TODO?
> + u32 status;
> +
> + chained_irq_enter(chip, desc);
> +
> + for (i = first; i <= last; ++i) {
> + status = readl_relaxed(intr + SPMI_PIC_OWNER_ACC_STATUS(ee, i));
> + while (status) {
> + id = ffs(status) - 1;
> + status &= ~(1 << id);
> + periph_interrupt(pa, id + i * 32);
> + }
> + }
> +
> + chained_irq_exit(chip, desc);
> +}
> +
[...]
> +static int qpnpint_irq_domain_dt_translate(struct irq_domain *d,
> +struct device_node *controller,
> +const u32 *intspec,
> +unsigned int intsize,
> +unsigned long *out_hwirq,
> +unsigned int *out_type)
> +{
> + struct spmi_pmic_arb_dev *pa = d->host_data;
> + struct spmi_pmic_arb_irq_spec spec;
> + int err;
> + u8 apid;
> +
> + dev_dbg(&pa->spmic->dev,
> + "intspec[0] 0x%1x intspec[1] 0x%02x intspec[2] 0x%02x\n",
> + intspec[0], intspec[1], intspec[2]);
> +
> + if (d->of_node != controller)
> + return -EINVAL;
> + if (intsize != 4)
> + return -EINVAL;
> + if (intspec[0] > 0xF || intspec[1] > 0xFF || intspec[2] > 0x7)
> + return -EINVAL;
> +
> + spec.slave = intspec[0];
> + spec.per = intspec[1];
> + spec.irq = intspec[2];
> +
> + err = search_mapping_table(pa, &spec, &apid);
> + if (err)
> + return err;
> +
> + pa->apid_to_ppid[apid] = spec.slave << 8 | spec.per;
> +
> + /* Keep track of {max,min}_apid for bounding search during interrupt */
> + if (apid > pa->max_apid)
> + pa->max_apid = apid;
> + if (apid < pa->min_apid)
> + pa->min_apid = apid;
Ah makes sense now why we set this to the opposite values in
probe. Please put a comment in patch 4 and maybe move that setup
in patch 4 to this patch.
> +
> + *out_hwirq = spec.slave << 24
> +| spec.per << 16
> +| spec.irq << 8
> +| apid;
> + *out_type = intspec[3] & IRQ_TYPE_SENSE_MASK;
> +
> + dev_dbg(&pa->spmic->dev, "out_hwirq = %lu\n", *out_hwirq);
> +
> + return 0;
> +}
> +
[...]
> static int spmi_pmic_arb_probe(struct platform_device *pdev)
> {
> struct spmi_pmic_arb_dev *pa;
> struct spmi_controller *ctrl;
> struct resource *res;
> - int err, i;
> + int err = 0, i;
>
> ctrl = spmi_controller_alloc(&pdev->dev, sizeof(*pa));
> if (!ctrl)
> return -ENOMEM;
>
> pa = spmi_controller_get_drvdata(ctrl);
> + pa->spmic = ctrl;
>
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "core");
> pa->base = devm_ioremap_resource(&ctrl->dev, res);
> @@ -349,6 +679,12 @@ static int spmi_pmic_arb_probe(struct platform_device
> *pdev)
> goto err_put_ctrl;
> }
>
> + pa->irq = platform_get_irq(pdev, 0);
> + if (pa->irq < 0) {
> + err = pa->irq;
> + goto err_put_ctrl;
> + }
> +
> for (i = 0; i < ARRAY_SIZE(pa->mapping_table); ++i)
> pa->mapping_table[i] = readl_relaxed(
> pa->cnfg + SPMI_MAPPING_TABLE_REG(i));
> @@ -364,15 +700,30 @@ static int spmi_pmic_arb_probe(struct platform_device
> *pdev)
> ctrl->read_cmd = pmic_arb_read_cmd;
> ctrl->write_cmd = pmic_arb_write_cmd;
>
> + dev_dbg(&pdev->dev, "adding irq domain\n");
> + pa->domain = irq_domain_add_tree(pdev->dev.of_node,
> + &pmic_arb_irq_domain_ops, pa);
> + if (!pa->domain) {
> + dev_err(&pdev-
Re: [PATCH v3 04/10] spmi: Add MSM PMIC Arbiter SPMI controller
On 10/28, Josh Cartwright wrote:
> +
> +/**
> + * pa_write_data: write 1..4 bytes from buf to pmic-arb's register
> + * @bc byte-count -1. range: 0..3
> + * @reg register's address
> + * @buf buffer to write. length must be bc+1
Missing colon between variable and description.
> + */
> +static void
> +pa_write_data(struct spmi_pmic_arb_dev *dev, const u8 *buf, u32 reg, u8 bc)
> +{
> + u32 data = 0;
> + memcpy(&data, buf, (bc & 3) + 1);
> + pmic_arb_base_write(dev, reg, data);
> +}
> +
[...]
> +static int pmic_arb_read_cmd(struct spmi_controller *ctrl,
> + u8 opc, u8 sid, u16 addr, u8 bc, u8 *buf)
> +{
> + struct spmi_pmic_arb_dev *pmic_arb = spmi_controller_get_drvdata(ctrl);
> + unsigned long flags;
> + u32 cmd;
> + int rc;
> +
> + if (bc >= PMIC_ARB_MAX_TRANS_BYTES) {
> + dev_err(&ctrl->dev,
> + "pmic-arb supports 1..%d bytes per trans, but:%d
> requested",
Nitpick: Please replace the colon between but and %d with a space.
> + PMIC_ARB_MAX_TRANS_BYTES, bc+1);
Space around that '+' please.
> + return -EINVAL;
> + }
> + dev_dbg(&ctrl->dev,
> + "op:0x%x sid:%d bc:%d addr:0x%x\n", opc, sid, bc, addr);
> +
> + /* Check the opcode */
> + if (opc >= 0x60 && opc <= 0x7F)
> + opc = PMIC_ARB_OP_READ;
> + else if (opc >= 0x20 && opc <= 0x2F)
> + opc = PMIC_ARB_OP_EXT_READ;
> + else if (opc >= 0x38 && opc <= 0x3F)
> + opc = PMIC_ARB_OP_EXT_READL;
> + else
> + return -EINVAL;
> +
> + cmd = (opc << 27) | ((sid & 0xf) << 20) | (addr << 4) | (bc & 0x7);
> +
> + spin_lock_irqsave(&pmic_arb->lock, flags);
> + pmic_arb_base_write(pmic_arb, PMIC_ARB_CMD(pmic_arb->channel), cmd);
> + rc = pmic_arb_wait_for_done(ctrl);
> + if (rc)
> + goto done;
> +
> + /* Read from FIFO, note 'bc' is actually number of bytes minus 1 */
> + pa_read_data(pmic_arb, buf, PMIC_ARB_RDATA0(pmic_arb->channel)
> + , min_t(u8, bc, 3));
Nitpick: Weird comma starting a line here.
> +
> + if (bc > 3)
> + pa_read_data(pmic_arb, buf + 4,
> + PMIC_ARB_RDATA1(pmic_arb->channel), bc - 4);
> +
> +done:
> + spin_unlock_irqrestore(&pmic_arb->lock, flags);
> + return rc;
> +}
> +
[...]
> +static int spmi_pmic_arb_probe(struct platform_device *pdev)
> +{
> + struct spmi_pmic_arb_dev *pa;
> + struct spmi_controller *ctrl;
> + struct resource *res;
> + int err, i;
> +
> + ctrl = spmi_controller_alloc(&pdev->dev, sizeof(*pa));
> + if (!ctrl)
> + return -ENOMEM;
> +
> + pa = spmi_controller_get_drvdata(ctrl);
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "core");
> + pa->base = devm_ioremap_resource(&ctrl->dev, res);
> + if (IS_ERR(pa->base)) {
> + err = PTR_ERR(pa->base);
> + goto err_put_ctrl;
> + }
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "intr");
> + pa->intr = devm_ioremap_resource(&ctrl->dev, res);
> + if (IS_ERR(pa->intr)) {
> + err = PTR_ERR(pa->intr);
> + goto err_put_ctrl;
> + }
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cnfg");
> + pa->cnfg = devm_ioremap_resource(&ctrl->dev, res);
> + if (IS_ERR(pa->cnfg)) {
> + err = PTR_ERR(pa->cnfg);
> + goto err_put_ctrl;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(pa->mapping_table); ++i)
> + pa->mapping_table[i] = readl_relaxed(
> + pa->cnfg + SPMI_MAPPING_TABLE_REG(i));
> +
> + platform_set_drvdata(pdev, ctrl);
> + spin_lock_init(&pa->lock);
> +
> + pa->channel = 0;
> + pa->max_apid = 0;
> + pa->min_apid = PMIC_ARB_MAX_PERIPHS - 1;
That looks backwards. Is this right?
> +
> + ctrl->cmd = pmic_arb_cmd;
> + ctrl->read_cmd = pmic_arb_read_cmd;
> + ctrl->write_cmd = pmic_arb_write_cmd;
> +
> + err = spmi_controller_add(ctrl);
> + if (err)
> + goto err_put_ctrl;
> +
> + dev_dbg(&ctrl->dev, "PMIC Arb Version 0x%x\n",
> + pmic_arb_base_read(pa, PMIC_ARB_VERSION));
> +
> + return 0;
> +
> +err_put_ctrl:
> + spmi_controller_put(ctrl);
> + return err;
> +}
> +
> +static int __exit spmi_pmic_arb_remove(struct platform_device *pdev)
__exit shouldn't be here. We want this function in modules.
> +{
> + struct spmi_controller *ctrl = platform_get_drvdata(pdev);
> + spmi_controller_remove(ctrl);
> + return 0;
> +}
> +
> +static struct of_device_id spmi_pmic_arb_match_table[] = {
> + { .compatible = "qcom,spmi-pmic-arb", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, spmi_pmic_arb_match_table);
> +
> +static struct platform_driver spmi_pmic_arb_driver = {
> + .probe = spmi_pmic_arb_probe,
> + .
Re: [PATCH 4/4] ARM: msm: Remove 8x50 support
On Wed, Oct 30, 2013 at 02:30:23PM +0100, Arnd Bergmann wrote: > On Tuesday 29 October 2013, Daniel Walker wrote: > > Isn't this the Nexus one platform ? Same as the last one , why don't you > > just update it to use the device tree? This doesn't seem like it would > > be all that difficult. > > Please don't top-post. Seriously ? I was responding to the whole comment, don't be so pedantic .. > > > On Mon, Oct 28, 2013 at 01:43:26PM -0700, David Brown wrote: > > > > > > This code has not been converted to device tree, and is hindering > > > support for the multi-platform kernel on ARM. If someone wishes to > > > continue support for this target, patches that provide devicetree and > > > multi-platform support can start by re-adding these files. > > ^ Here is your answer. > > > While I'd definitely love to see support for the Nexus One upstream, > it is evident that nobody has been working on this for a long time. I have a device with this chip in it .. I would like that device not to disappear from Linux .. Doing this puts an un-do burden on users who would like to see this support go in, but if it's removed like this that will never happen. No one has an much information on these chips as Qualcomm .. It's not realistic that any user would be able to do a lot without their support.. By allowing them to remove code for these device it effectively ends and chance of them have more code support added.. Yet the devices exist in mass amounts. To be honest your not an MSM maintainer, are there are political issue related to this.. This isn't particularly technical because converting these relatively simple platform to the device tree shouldn't be all that difficult. To me Your involvement is puzzling .. Myself and my co-maintainers need to be the ones discussing this.. G1 code is also getting removed in this series, and the code for this creates a usable device .. The problem here is that you really can't start randomly removing support for platforms that people have and want them to continue working. We have a process for that, and typically user objections weight heavily in that process (read, feature-removal-schedule.txt) Daniel -- 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 4/4] ARM: msm: Remove 8x50 support
On Tuesday 29 October 2013, Daniel Walker wrote: > Isn't this the Nexus one platform ? Same as the last one , why don't you > just update it to use the device tree? This doesn't seem like it would > be all that difficult. Please don't top-post. > On Mon, Oct 28, 2013 at 01:43:26PM -0700, David Brown wrote: > > > > This code has not been converted to device tree, and is hindering > > support for the multi-platform kernel on ARM. If someone wishes to > > continue support for this target, patches that provide devicetree and > > multi-platform support can start by re-adding these files. ^ Here is your answer. While I'd definitely love to see support for the Nexus One upstream, it is evident that nobody has been working on this for a long time. 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: [PATCH 3/4] ARM: msm: Remove 7x30 supporty
On Tuesday 29 October 2013, Daniel Walker wrote: > Why wouldn't you just update it to use the device tree ? There are lots > of phones our there using 7x30 .. > > This is one that Qualcomm specifically upstreamed, so what was the point > of upstreaming it ? Things like this happen a lot: you start with great ambitions, then real life takes over and everybody who was once working on it has moved on to other projects before the code actually works. We have the policy to let new code for incomplete platforms into the tree very liberally, but the flip side is that we have to remove them as swiftly if progress stops. Note that this is completely different from most of the older platforms like the StrongARM based machines that are way more outdated but are actually working just fine and getting fixed when they don't. The MSM platform is currently a mess with most SoCs broken most of the time. I definitely agree with David's approach of removing the ones that are not being worked on and finally getting the newer ones to work properly, and on the same level as the other platforms. Once that work is complete, adding back support for the older SoCs should become much easier if anyone is still interested, since the required changes (multiplatform, DT, ...) are targetted at reducing the effort of maintaining platforms. 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: [PATCH 4/6] edac: Document Krait L1/L2 EDAC driver binding
On Oct 29, 2013, at 7:38 PM, Mark Rutland wrote:
> On Tue, Oct 29, 2013 at 06:00:59PM +, Stephen Boyd wrote:
>> On 10/29/13 01:21, Kumar Gala wrote:
>>> On Oct 28, 2013, at 7:31 PM, Stephen Boyd wrote:
>>>
The Krait L1/L2 error reporting device is made up of two
interrupts, one per-CPU interrupt for the L1 caches and one
interrupt for the L2 cache.
Cc:
Signed-off-by: Stephen Boyd
---
.../devicetree/bindings/arm/qcom,krait-cache-erp.txt | 16
1 file changed, 16 insertions(+)
create mode 100644
Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
diff --git
a/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
b/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
new file mode 100644
index 000..01fe8a8
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
@@ -0,0 +1,16 @@
+* Qualcomm Krait L1 / L2 cache error reporting
+
+Required properties:
+- compatible: Should be "qcom,krait-cache-erp"
+- interrupts: Should contain the L1/CPU error interrupt number and
+ then the L2 cache error interrupt number
+
+Optional properties:
+- interrupt-names: Should contain the interrupt names "l1_irq" and
+ "l2_irq"
+
+Example:
+ edac {
+ compatible = "qcom,krait-cache-erp";
+ interrupts = <1 9 0xf04>, <0 2 0x4>;
+ };
>>> Why wouldn't we have these as part of cache nodes in the dts? (which begs
>>> the question why we don't have cache nodes?)
>>>
>>
>> I can certainly add cache nodes and cpu nodes and then put the
>> interrupts in those nodes. I was thinking along those same lines when I
>> ported this driver but figured it would be good to get something out
>> there. The only question I have is how am I supposed to hook that up
>> into the linux device model? Will the edac driver bind to the device
>> created for the cpus node and the cache node? I guess it will have to be
>> a driver that binds to two devices.
>>
>> One could argue that we should put the cp15 based architected timers in
>> the cpus node also but so far nobody has done that and I think there was
>> some reasoning behind that, Mark?
>
> The architected timer binding was created at a time I wasn't involved in
> kernel
> development, and I'm not aware of any particular reasoning. I've heard that
> there was a decision to not duplicate banked resources, which would explain
> not
> having the timer under /cpus/cpu@N, but doesn't imply that having it under
> /cpus is bad.
>
> Do we have precedent for putting any devices other than CPUs in /cpus?
On PPC we had core cache's (depending on topology) that would be there, and
thus why I raised the suggestion.
- 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
