| Message ID | 1400607830-10989-2-git-send-email-elder@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, May 20, 2014 at 06:43:46PM +0100, Alex Elder wrote: > Broadcom mobile SoCs use a ROM-implemented holding pen for > controlled boot of secondary cores. A special register is > used to communicate to the ROM that a secondary core should > start executing kernel code. This enable method is currently > used for members of the bcm281xx and bcm21664 SoC families. > > The use of an enable method also allows the SMP operation vector to > be assigned as a result of device tree content for these SoCs. > > Signed-off-by: Alex Elder <elder@linaro.org> This is getting out of control, it is absolutely ghastly. I wonder how I can manage to keep cpus.txt updated if anyone with a boot method du jour adds into cpus.txt, and honestly in this specific case it is even hard to understand why. Can't it be done with bindings for the relative register address space (regmap ?) and platform code just calls the registers driver to set-up the jump address ? It is platform specific code anyway there is no way you can make this generic. I really do not see the point in cluttering cpus.txt with this stuff, it is a platform specific hack, and do not belong in generic bindings in my opinion. Thanks, Lorenzo > --- > Documentation/devicetree/bindings/arm/cpus.txt | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt > index 333f4ae..c6a2411 100644 > --- a/Documentation/devicetree/bindings/arm/cpus.txt > +++ b/Documentation/devicetree/bindings/arm/cpus.txt > @@ -185,6 +185,7 @@ nodes to be present and contain the properties described below. > "qcom,gcc-msm8660" > "qcom,kpss-acc-v1" > "qcom,kpss-acc-v2" > + "brcm,bcm11351-cpu-method" > > - cpu-release-addr > Usage: required for systems that have an "enable-method" > @@ -209,6 +210,17 @@ nodes to be present and contain the properties described below. > Value type: <phandle> > Definition: Specifies the ACC[2] node associated with this CPU. > > + - secondary-boot-reg > + Usage: > + Required for systems that have an "enable-method" > + property value of "brcm,bcm11351-cpu-method". > + Value type: <u32> > + Definition: > + Specifies the physical address of the register used to > + request the ROM holding pen code release a secondary > + CPU. The value written to the register is formed by > + encoding the target CPU id into the low bits of the > + physical start address it should jump to. > > Example 1 (dual-cluster big.LITTLE system 32-bit): > > -- > 1.9.1 > >
On 05/27/2014 06:49 AM, Lorenzo Pieralisi wrote: > On Tue, May 20, 2014 at 06:43:46PM +0100, Alex Elder wrote: >> Broadcom mobile SoCs use a ROM-implemented holding pen for >> controlled boot of secondary cores. A special register is >> used to communicate to the ROM that a secondary core should >> start executing kernel code. This enable method is currently >> used for members of the bcm281xx and bcm21664 SoC families. >> >> The use of an enable method also allows the SMP operation vector to >> be assigned as a result of device tree content for these SoCs. >> >> Signed-off-by: Alex Elder <elder@linaro.org> > > This is getting out of control, it is absolutely ghastly. I wonder how > I can manage to keep cpus.txt updated if anyone with a boot method > du jour adds into cpus.txt, and honestly in this specific case it is even > hard to understand why. I concur that it gets out of control to document bindings in "cpus.txt" like this. I posted a separate, independent documentation patch, to address this issue specifically: devicetree: bindings: separate CPU enable method descriptions There I don't even include my new addition but I also do a little work to sort out some stuff--for example only defining "cpu-release-addr" (or "qcom,saw") in the one place where it's relevant. > Can't it be done with bindings for the relative register address space > (regmap ?) and platform code just calls the registers driver to set-up the > jump address ? It is platform specific code anyway there is no way you > can make this generic. This is feedback specific to how I implement this enable method. I am going to address this in a follow-on message, to distinguish it from the broader question of where best to document these enable methods. (I'll be sending that message later today.) > I really do not see the point in cluttering cpus.txt with this stuff, it > is a platform specific hack, and do not belong in generic bindings in my > opinion. Again, I completely agree with this. In order to assign the SMP operations vector for my machine via device tree, I need to define an "enable-method" property in either the "cpus" node or one of the "cpu" nodes. I would prefer to use a generic method, but the method used here is semantically different from the others in existence, and I need to document how it works. The place currently used to do that is "cpus.txt". Please look at the patch I mentioned above. I'd be glad to do it another way; but it is an attempt to address what I saw as a problem that I think you are talking about. Thanks. -Alex > Thanks, > Lorenzo > >> --- >> Documentation/devicetree/bindings/arm/cpus.txt | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt >> index 333f4ae..c6a2411 100644 >> --- a/Documentation/devicetree/bindings/arm/cpus.txt >> +++ b/Documentation/devicetree/bindings/arm/cpus.txt >> @@ -185,6 +185,7 @@ nodes to be present and contain the properties described below. >> "qcom,gcc-msm8660" >> "qcom,kpss-acc-v1" >> "qcom,kpss-acc-v2" >> + "brcm,bcm11351-cpu-method" >> >> - cpu-release-addr >> Usage: required for systems that have an "enable-method" >> @@ -209,6 +210,17 @@ nodes to be present and contain the properties described below. >> Value type: <phandle> >> Definition: Specifies the ACC[2] node associated with this CPU. >> >> + - secondary-boot-reg >> + Usage: >> + Required for systems that have an "enable-method" >> + property value of "brcm,bcm11351-cpu-method". >> + Value type: <u32> >> + Definition: >> + Specifies the physical address of the register used to >> + request the ROM holding pen code release a secondary >> + CPU. The value written to the register is formed by >> + encoding the target CPU id into the low bits of the >> + physical start address it should jump to. >> >> Example 1 (dual-cluster big.LITTLE system 32-bit): >> >> -- >> 1.9.1 >> >> >
On 05/27/2014 06:49 AM, Lorenzo Pieralisi wrote: > On Tue, May 20, 2014 at 06:43:46PM +0100, Alex Elder wrote: >> Broadcom mobile SoCs use a ROM-implemented holding pen for >> controlled boot of secondary cores. A special register is >> used to communicate to the ROM that a secondary core should >> start executing kernel code. This enable method is currently >> used for members of the bcm281xx and bcm21664 SoC families. >> >> The use of an enable method also allows the SMP operation vector to >> be assigned as a result of device tree content for these SoCs. >> >> Signed-off-by: Alex Elder <elder@linaro.org> > > This is getting out of control, it is absolutely ghastly. I wonder how > I can manage to keep cpus.txt updated if anyone with a boot method > du jour adds into cpus.txt, and honestly in this specific case it is even > hard to understand why. OK, in this message I'll focus on the particulars of this proposed binding. > Can't it be done with bindings for the relative register address space > (regmap ?) and platform code just calls the registers driver to set-up the > jump address ? It is platform specific code anyway there is no way you > can make this generic. I want to clarify what you're after here. My aim is to add SMP support for a class of Broadcom SMP machines. To do so, I'm told I need to use the technique of assigning the SMP operations vector as a result of identifying an enable method in the DT. For 32-bit ARM, there are no generic "enable-method" values. (I did attempt to create one for "spin-table" but that was rejected by Russell King.) For the machines I'm trying to enable, secondary CPUS start out spinning in a ROM-based holding pen, and there is no need for a kernel-based one. However, like a spin-table/holding pen enable method, a memory location is required for coordination between the boot CPU running kernel code and secondary CPUs running ROM code. My proposal specifies it using a special numeric property value named "secondary-boot-reg" in the "cpus" node in the DT. And as I understand it, the issue you have relates to how this memory location is specified. You suggest regmap. I'm using a single 32-bit register, only at very early boot time, and thereafter access to it is meaningless. It seems like overkill if it's only used for this purpose. I could hide the register values in the code, but with the exception of that, the code I'm using is generic (in the context of this class of Broadcom machine). I could specify the register differently somehow, in a different node, or with a different property. The bottom line here is I'm not sure whether I understand what you're suggesting, or perhaps why what you suggest is preferable. I'm very open to suggestions, I just need it laid out a bit more detail in order to respond directly. Thanks. -Alex > I really do not see the point in cluttering cpus.txt with this stuff, it > is a platform specific hack, and do not belong in generic bindings in my > opinion. > > Thanks, > Lorenzo > >> --- >> Documentation/devicetree/bindings/arm/cpus.txt | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt >> index 333f4ae..c6a2411 100644 >> --- a/Documentation/devicetree/bindings/arm/cpus.txt >> +++ b/Documentation/devicetree/bindings/arm/cpus.txt >> @@ -185,6 +185,7 @@ nodes to be present and contain the properties described below. >> "qcom,gcc-msm8660" >> "qcom,kpss-acc-v1" >> "qcom,kpss-acc-v2" >> + "brcm,bcm11351-cpu-method" >> >> - cpu-release-addr >> Usage: required for systems that have an "enable-method" >> @@ -209,6 +210,17 @@ nodes to be present and contain the properties described below. >> Value type: <phandle> >> Definition: Specifies the ACC[2] node associated with this CPU. >> >> + - secondary-boot-reg >> + Usage: >> + Required for systems that have an "enable-method" >> + property value of "brcm,bcm11351-cpu-method". >> + Value type: <u32> >> + Definition: >> + Specifies the physical address of the register used to >> + request the ROM holding pen code release a secondary >> + CPU. The value written to the register is formed by >> + encoding the target CPU id into the low bits of the >> + physical start address it should jump to. >> >> Example 1 (dual-cluster big.LITTLE system 32-bit): >> >> -- >> 1.9.1 >> >> >
On Wed, May 28, 2014 at 04:30:47AM +0100, Alex Elder wrote: > On 05/27/2014 06:49 AM, Lorenzo Pieralisi wrote: > > On Tue, May 20, 2014 at 06:43:46PM +0100, Alex Elder wrote: > >> Broadcom mobile SoCs use a ROM-implemented holding pen for > >> controlled boot of secondary cores. A special register is > >> used to communicate to the ROM that a secondary core should > >> start executing kernel code. This enable method is currently > >> used for members of the bcm281xx and bcm21664 SoC families. > >> > >> The use of an enable method also allows the SMP operation vector to > >> be assigned as a result of device tree content for these SoCs. > >> > >> Signed-off-by: Alex Elder <elder@linaro.org> > > > > This is getting out of control, it is absolutely ghastly. I wonder how > > I can manage to keep cpus.txt updated if anyone with a boot method > > du jour adds into cpus.txt, and honestly in this specific case it is even > > hard to understand why. > > OK, in this message I'll focus on the particulars of this > proposed binding. > > > Can't it be done with bindings for the relative register address space > > (regmap ?) and platform code just calls the registers driver to set-up the > > jump address ? It is platform specific code anyway there is no way you > > can make this generic. > > I want to clarify what you're after here. > > My aim is to add SMP support for a class of Broadcom SMP > machines. To do so, I'm told I need to use the technique > of assigning the SMP operations vector as a result of > identifying an enable method in the DT. > > For 32-bit ARM, there are no generic "enable-method" values. > (I did attempt to create one for "spin-table" but that was > rejected by Russell King.) For the machines I'm trying to > enable, secondary CPUS start out spinning in a ROM-based > holding pen, and there is no need for a kernel-based one. > > However, like a spin-table/holding pen enable method, a > memory location is required for coordination between the > boot CPU running kernel code and secondary CPUs running ROM > code. My proposal specifies it using a special numeric > property value named "secondary-boot-reg" in the "cpus" > node in the DT. > > And as I understand it, the issue you have relates to how > this memory location is specified. The issue I have relates to cluttering cpus.txt with all sorts of platform specific SMP boot hacks. > You suggest regmap. I'm using a single 32-bit register, > only at very early boot time, and thereafter access to > it is meaningless. It seems like overkill if it's only > used for this purpose. I could hide the register values > in the code, but with the exception of that, the code I'm > using is generic (in the context of this class of Broadcom > machine). I could specify the register differently somehow, > in a different node, or with a different property. Is that register part of a larger registers block ? What I wanted to say is that you can use a driver "API" (we wish) to write that register, something like eg vexpress does with sysflags: drivers/mfd/vexpress-sysreg.c vexpress_flags_set() instead of grabbing the reg address from a platform specific boot method DT entry. I doubt that register exists on its own, even though I have to say this would force you to write yet another platform specific driver to control a bunch of registers, I do not see any other solution. One thing is for certain: I really do not see the point in adding a boot method per-SoC, and I do not want to end up having a cpus.txt file with a gazillion entries just because every given platform reinvents the wheel when it comes to booting an SMP system, cpus.txt would become a document that describes platform quirks, not a proper binding anymore. At least all platform specific quirks must be moved out of cpus.txt and in platform documentation, I understand it is just a cosmetic change but I want to prevent cpus.txt to become an abomination. > The bottom line here is I'm not sure whether I understand > what you're suggesting, or perhaps why what you suggest is > preferable. I'm very open to suggestions, I just need it > laid out a bit more detail in order to respond directly. See above. Thanks ! Lorenzo > > Thanks. > > -Alex > > > I really do not see the point in cluttering cpus.txt with this stuff, it > > is a platform specific hack, and do not belong in generic bindings in my > > opinion. > > > > Thanks, > > Lorenzo > > > >> --- > >> Documentation/devicetree/bindings/arm/cpus.txt | 12 ++++++++++++ > >> 1 file changed, 12 insertions(+) > >> > >> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt > >> index 333f4ae..c6a2411 100644 > >> --- a/Documentation/devicetree/bindings/arm/cpus.txt > >> +++ b/Documentation/devicetree/bindings/arm/cpus.txt > >> @@ -185,6 +185,7 @@ nodes to be present and contain the properties described below. > >> "qcom,gcc-msm8660" > >> "qcom,kpss-acc-v1" > >> "qcom,kpss-acc-v2" > >> + "brcm,bcm11351-cpu-method" > >> > >> - cpu-release-addr > >> Usage: required for systems that have an "enable-method" > >> @@ -209,6 +210,17 @@ nodes to be present and contain the properties described below. > >> Value type: <phandle> > >> Definition: Specifies the ACC[2] node associated with this CPU. > >> > >> + - secondary-boot-reg > >> + Usage: > >> + Required for systems that have an "enable-method" > >> + property value of "brcm,bcm11351-cpu-method". > >> + Value type: <u32> > >> + Definition: > >> + Specifies the physical address of the register used to > >> + request the ROM holding pen code release a secondary > >> + CPU. The value written to the register is formed by > >> + encoding the target CPU id into the low bits of the > >> + physical start address it should jump to. > >> > >> Example 1 (dual-cluster big.LITTLE system 32-bit): > >> > >> -- > >> 1.9.1 > >> > >> > > > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On 05/28/2014 05:36 AM, Lorenzo Pieralisi wrote: > On Wed, May 28, 2014 at 04:30:47AM +0100, Alex Elder wrote: >> On 05/27/2014 06:49 AM, Lorenzo Pieralisi wrote: >>> On Tue, May 20, 2014 at 06:43:46PM +0100, Alex Elder wrote: >>>> Broadcom mobile SoCs use a ROM-implemented holding pen for >>>> controlled boot of secondary cores. A special register is >>>> used to communicate to the ROM that a secondary core should >>>> start executing kernel code. This enable method is currently >>>> used for members of the bcm281xx and bcm21664 SoC families. >>>> >>>> The use of an enable method also allows the SMP operation vector to >>>> be assigned as a result of device tree content for these SoCs. >>>> >>>> Signed-off-by: Alex Elder <elder@linaro.org> >>> >>> This is getting out of control, it is absolutely ghastly. I wonder how >>> I can manage to keep cpus.txt updated if anyone with a boot method >>> du jour adds into cpus.txt, and honestly in this specific case it is even >>> hard to understand why. >> >> OK, in this message I'll focus on the particulars of this >> proposed binding. >> >>> Can't it be done with bindings for the relative register address space >>> (regmap ?) and platform code just calls the registers driver to set-up the >>> jump address ? It is platform specific code anyway there is no way you >>> can make this generic. >> >> I want to clarify what you're after here. >> >> My aim is to add SMP support for a class of Broadcom SMP >> machines. To do so, I'm told I need to use the technique >> of assigning the SMP operations vector as a result of >> identifying an enable method in the DT. >> >> For 32-bit ARM, there are no generic "enable-method" values. >> (I did attempt to create one for "spin-table" but that was >> rejected by Russell King.) For the machines I'm trying to >> enable, secondary CPUS start out spinning in a ROM-based >> holding pen, and there is no need for a kernel-based one. >> >> However, like a spin-table/holding pen enable method, a >> memory location is required for coordination between the >> boot CPU running kernel code and secondary CPUs running ROM >> code. My proposal specifies it using a special numeric >> property value named "secondary-boot-reg" in the "cpus" >> node in the DT. >> >> And as I understand it, the issue you have relates to how >> this memory location is specified. > > The issue I have relates to cluttering cpus.txt with all > sorts of platform specific SMP boot hacks. OK, as I mentioned in my other message, I totally agree with you here. It's a total (and building) mess. I discussed this with Mark Rutland at ELC last month and suggested splitting that stuff out of "cpus.txt", which I have now proposed with a patch. https://lkml.org/lkml/2014/5/8/545 >> You suggest regmap. I'm using a single 32-bit register, >> only at very early boot time, and thereafter access to >> it is meaningless. It seems like overkill if it's only >> used for this purpose. I could hide the register values >> in the code, but with the exception of that, the code I'm >> using is generic (in the context of this class of Broadcom >> machine). I could specify the register differently somehow, >> in a different node, or with a different property. > > Is that register part of a larger registers block ? What I wanted > to say is that you can use a driver "API" (we wish) to write that > register, something like eg vexpress does with sysflags: > > drivers/mfd/vexpress-sysreg.c > > vexpress_flags_set() > > instead of grabbing the reg address from a platform specific boot > method DT entry. I had this exact thought when I was working on this. Yes, this register *is* part of a register block, "ChipRegs", and I wanted to have a separate driver/library/API to manage access to registers in that block. But the block is sort of a dumping ground for many unrelated things, and lots of them aren't going to be used for normal operation (lots of debug and diagnostic functionality, for example). After some discussion with others on my team I made the explicit decision to *not* have any sort of API in this case, and just specify the register. We decided that if and when we had a need for multiple users of addresses in this range we'd figure out how to do best what was required. > I doubt that register exists on its own, even though I have to say this > would force you to write yet another platform specific driver to control > a bunch of registers, I do not see any other solution. Yes, exactly, and I was prepared to do that. But doing a whole driver for that one address seemed to make less sense. > One thing is for certain: I really do not see the point in adding a boot > method per-SoC, and I do not want to end up having a cpus.txt file with a I'd love to see common, "standard" boot methods. For ARM32 it seems each one was done separately, and in many cases, slightly differently. The one I saw the most was "spin table," but making that "standard" was deemed unacceptable. > gazillion entries just because every given platform reinvents the wheel when > it comes to booting an SMP system, cpus.txt would become a document that > describes platform quirks, not a proper binding anymore. Agreed. > At least all platform specific quirks must be moved out of cpus.txt and > in platform documentation, I understand it is just a cosmetic change but > I want to prevent cpus.txt to become an abomination. Agreed. >> The bottom line here is I'm not sure whether I understand >> what you're suggesting, or perhaps why what you suggest is >> preferable. I'm very open to suggestions, I just need it >> laid out a bit more detail in order to respond directly. > > See above. I really appreciate your responses. One problem with this process is you don't get the benefit of the discussion and design process that went on prior to posting this stuff publicly. If you still feel strongly we should have a custom driver to manage this "ChipRegs" address space, say so, and I can put one together. If my explanations above mitigate your concerns, please say that as well. Either way I just want to make progress toward getting this support upstream. Thanks a lot. -Alex > > Thanks ! > Lorenzo > >> >> Thanks. >> >> -Alex >> >>> I really do not see the point in cluttering cpus.txt with this stuff, it >>> is a platform specific hack, and do not belong in generic bindings in my >>> opinion. >>> >>> Thanks, >>> Lorenzo >>> >>>> --- >>>> Documentation/devicetree/bindings/arm/cpus.txt | 12 ++++++++++++ >>>> 1 file changed, 12 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt >>>> index 333f4ae..c6a2411 100644 >>>> --- a/Documentation/devicetree/bindings/arm/cpus.txt >>>> +++ b/Documentation/devicetree/bindings/arm/cpus.txt >>>> @@ -185,6 +185,7 @@ nodes to be present and contain the properties described below. >>>> "qcom,gcc-msm8660" >>>> "qcom,kpss-acc-v1" >>>> "qcom,kpss-acc-v2" >>>> + "brcm,bcm11351-cpu-method" >>>> >>>> - cpu-release-addr >>>> Usage: required for systems that have an "enable-method" >>>> @@ -209,6 +210,17 @@ nodes to be present and contain the properties described below. >>>> Value type: <phandle> >>>> Definition: Specifies the ACC[2] node associated with this CPU. >>>> >>>> + - secondary-boot-reg >>>> + Usage: >>>> + Required for systems that have an "enable-method" >>>> + property value of "brcm,bcm11351-cpu-method". >>>> + Value type: <u32> >>>> + Definition: >>>> + Specifies the physical address of the register used to >>>> + request the ROM holding pen code release a secondary >>>> + CPU. The value written to the register is formed by >>>> + encoding the target CPU id into the low bits of the >>>> + physical start address it should jump to. >>>> >>>> Example 1 (dual-cluster big.LITTLE system 32-bit): >>>> >>>> -- >>>> 1.9.1 >>>> >>>> >>> >> >> -- >> To unsubscribe from this list: send the line "unsubscribe devicetree" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >
On Wed, May 28, 2014 at 01:22:06PM +0100, Alex Elder wrote: > On 05/28/2014 05:36 AM, Lorenzo Pieralisi wrote: > > On Wed, May 28, 2014 at 04:30:47AM +0100, Alex Elder wrote: > >> On 05/27/2014 06:49 AM, Lorenzo Pieralisi wrote: > >>> On Tue, May 20, 2014 at 06:43:46PM +0100, Alex Elder wrote: > >>>> Broadcom mobile SoCs use a ROM-implemented holding pen for > >>>> controlled boot of secondary cores. A special register is > >>>> used to communicate to the ROM that a secondary core should > >>>> start executing kernel code. This enable method is currently > >>>> used for members of the bcm281xx and bcm21664 SoC families. > >>>> > >>>> The use of an enable method also allows the SMP operation vector to > >>>> be assigned as a result of device tree content for these SoCs. > >>>> > >>>> Signed-off-by: Alex Elder <elder@linaro.org> > >>> > >>> This is getting out of control, it is absolutely ghastly. I wonder how > >>> I can manage to keep cpus.txt updated if anyone with a boot method > >>> du jour adds into cpus.txt, and honestly in this specific case it is even > >>> hard to understand why. > >> > >> OK, in this message I'll focus on the particulars of this > >> proposed binding. > >> > >>> Can't it be done with bindings for the relative register address space > >>> (regmap ?) and platform code just calls the registers driver to set-up the > >>> jump address ? It is platform specific code anyway there is no way you > >>> can make this generic. > >> > >> I want to clarify what you're after here. > >> > >> My aim is to add SMP support for a class of Broadcom SMP > >> machines. To do so, I'm told I need to use the technique > >> of assigning the SMP operations vector as a result of > >> identifying an enable method in the DT. > >> > >> For 32-bit ARM, there are no generic "enable-method" values. > >> (I did attempt to create one for "spin-table" but that was > >> rejected by Russell King.) For the machines I'm trying to > >> enable, secondary CPUS start out spinning in a ROM-based > >> holding pen, and there is no need for a kernel-based one. > >> > >> However, like a spin-table/holding pen enable method, a > >> memory location is required for coordination between the > >> boot CPU running kernel code and secondary CPUs running ROM > >> code. My proposal specifies it using a special numeric > >> property value named "secondary-boot-reg" in the "cpus" > >> node in the DT. > >> > >> And as I understand it, the issue you have relates to how > >> this memory location is specified. > > > > The issue I have relates to cluttering cpus.txt with all > > sorts of platform specific SMP boot hacks. > > OK, as I mentioned in my other message, I totally > agree with you here. It's a total (and building) > mess. I discussed this with Mark Rutland at ELC > last month and suggested splitting that stuff out > of "cpus.txt", which I have now proposed with a > patch. > https://lkml.org/lkml/2014/5/8/545 I think this makes sense, I will review that patchset, and with this approach agreed I am ok with adding a platform specific boot method, since it is split up "nicely", do not bother adding a specific driver to poke a register (it will be fun to see the number of files we have to add to /cpu-enable-method though, big fun). I still think that it is high time we started pushing back on these platform hacks and move towards a common interface like PSCI to boot (and suspend) ARM processors, there is no reason whatsoever why this can't be done on the platforms you are trying to get merged unless I am missing something. Lorenzo
On 05/28/2014 08:34 AM, Lorenzo Pieralisi wrote: > On Wed, May 28, 2014 at 01:22:06PM +0100, Alex Elder wrote: >> On 05/28/2014 05:36 AM, Lorenzo Pieralisi wrote: >>> On Wed, May 28, 2014 at 04:30:47AM +0100, Alex Elder wrote: >>>> On 05/27/2014 06:49 AM, Lorenzo Pieralisi wrote: >>>>> On Tue, May 20, 2014 at 06:43:46PM +0100, Alex Elder wrote: >>>>>> Broadcom mobile SoCs use a ROM-implemented holding pen for >>>>>> controlled boot of secondary cores. A special register is >>>>>> used to communicate to the ROM that a secondary core should >>>>>> start executing kernel code. This enable method is currently >>>>>> used for members of the bcm281xx and bcm21664 SoC families. >>>>>> >>>>>> The use of an enable method also allows the SMP operation vector to >>>>>> be assigned as a result of device tree content for these SoCs. >>>>>> >>>>>> Signed-off-by: Alex Elder <elder@linaro.org> >>>>> >>>>> This is getting out of control, it is absolutely ghastly. I wonder how >>>>> I can manage to keep cpus.txt updated if anyone with a boot method >>>>> du jour adds into cpus.txt, and honestly in this specific case it is even >>>>> hard to understand why. >>>> >>>> OK, in this message I'll focus on the particulars of this >>>> proposed binding. >>>> >>>>> Can't it be done with bindings for the relative register address space >>>>> (regmap ?) and platform code just calls the registers driver to set-up the >>>>> jump address ? It is platform specific code anyway there is no way you >>>>> can make this generic. >>>> >>>> I want to clarify what you're after here. >>>> >>>> My aim is to add SMP support for a class of Broadcom SMP >>>> machines. To do so, I'm told I need to use the technique >>>> of assigning the SMP operations vector as a result of >>>> identifying an enable method in the DT. >>>> >>>> For 32-bit ARM, there are no generic "enable-method" values. >>>> (I did attempt to create one for "spin-table" but that was >>>> rejected by Russell King.) For the machines I'm trying to >>>> enable, secondary CPUS start out spinning in a ROM-based >>>> holding pen, and there is no need for a kernel-based one. >>>> >>>> However, like a spin-table/holding pen enable method, a >>>> memory location is required for coordination between the >>>> boot CPU running kernel code and secondary CPUs running ROM >>>> code. My proposal specifies it using a special numeric >>>> property value named "secondary-boot-reg" in the "cpus" >>>> node in the DT. >>>> >>>> And as I understand it, the issue you have relates to how >>>> this memory location is specified. >>> >>> The issue I have relates to cluttering cpus.txt with all >>> sorts of platform specific SMP boot hacks. >> >> OK, as I mentioned in my other message, I totally >> agree with you here. It's a total (and building) >> mess. I discussed this with Mark Rutland at ELC >> last month and suggested splitting that stuff out >> of "cpus.txt", which I have now proposed with a >> patch. >> https://lkml.org/lkml/2014/5/8/545 > > I think this makes sense, I will review that patchset, and with this > approach agreed I am ok with adding a platform specific boot method, > since it is split up "nicely", do not bother adding a specific driver > to poke a register (it will be fun to see the number of files we have > to add to /cpu-enable-method though, big fun). Great! I used the existing documentation and the code as a guide in crafting the text of those descriptions. Some of them I had to speculate though--especially for ARM64 (for which there is documentation but nothing in the tree that uses it). So it needs some informed feedback. > I still think that it is high time we started pushing back on these > platform hacks and move towards a common interface like PSCI to boot > (and suspend) ARM processors, there is no reason whatsoever why this > can't be done on the platforms you are trying to get merged unless I am > missing something. We have no need for anything other than SMP startup at this point on this platform. If the framework were there for me to use I would have used it. Thanks again for working through this with me. -Alex > Lorenzo >
diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt index 333f4ae..c6a2411 100644 --- a/Documentation/devicetree/bindings/arm/cpus.txt +++ b/Documentation/devicetree/bindings/arm/cpus.txt @@ -185,6 +185,7 @@ nodes to be present and contain the properties described below. "qcom,gcc-msm8660" "qcom,kpss-acc-v1" "qcom,kpss-acc-v2" + "brcm,bcm11351-cpu-method" - cpu-release-addr Usage: required for systems that have an "enable-method" @@ -209,6 +210,17 @@ nodes to be present and contain the properties described below. Value type: <phandle> Definition: Specifies the ACC[2] node associated with this CPU. + - secondary-boot-reg + Usage: + Required for systems that have an "enable-method" + property value of "brcm,bcm11351-cpu-method". + Value type: <u32> + Definition: + Specifies the physical address of the register used to + request the ROM holding pen code release a secondary + CPU. The value written to the register is formed by + encoding the target CPU id into the low bits of the + physical start address it should jump to. Example 1 (dual-cluster big.LITTLE system 32-bit):
Broadcom mobile SoCs use a ROM-implemented holding pen for controlled boot of secondary cores. A special register is used to communicate to the ROM that a secondary core should start executing kernel code. This enable method is currently used for members of the bcm281xx and bcm21664 SoC families. The use of an enable method also allows the SMP operation vector to be assigned as a result of device tree content for these SoCs. Signed-off-by: Alex Elder <elder@linaro.org> --- Documentation/devicetree/bindings/arm/cpus.txt | 12 ++++++++++++ 1 file changed, 12 insertions(+)