| Message ID | 1446844273-6460-2-git-send-email-kapilh@broadcom.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Nov 6, 2015 at 3:11 PM, Kapil Hali <kapilh@broadcom.com> wrote: > Add a compatible string "brcm,bcm-nsp-smp" for Broadcom's > Northstar Plus CPU to the 32-bit ARM CPU device tree binding > documentation file and create a new binding documentation for > Northstar Plus CPU. > > Signed-off-by: Kapil Hali <kapilh@broadcom.com> > --- > .../bindings/arm/bcm/brcm,nsp-cpu-method.txt | 36 ++++++++++++++++++++++ > Documentation/devicetree/bindings/arm/cpus.txt | 1 + > 2 files changed, 37 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt > > diff --git a/Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt b/Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt > new file mode 100644 > index 0000000..8506da7 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt > @@ -0,0 +1,36 @@ > +Broadcom Northstar Plus SoC CPU Enable Method > +--------------------------------------------- > +This binding defines the enable method used for starting secondary > +CPUs in the following Broadcom SoCs: > + BCM58522, BCM58525, BCM58535, BCM58622, BCM58623, BCM58625, BCM88312 > + > +The enable method is specified by defining the following required > +properties in the "cpus" device tree node: > + - enable-method = "brcm,bcm-nsp-smp"; As I said already, this is supposed to be per cpu. > + - secondary-boot-reg = <...>; And then you might as well move this too. > + > +The secondary-boot-reg property is a u32 value that specifies the > +physical address of the register used to request the ROM holding pen > +code release a secondary CPU. > + > +Example: > + cpus { > + #address-cells = <1>; > + #size-cells = <0>; > + enable-method = "brcm,bcm-nsp-smp"; > + secondary-boot-reg = <0xffff042c>; > + > + cpu0: cpu@0 { > + device_type = "cpu"; > + compatible = "arm,cortex-a9"; > + next-level-cache = <&L2>; > + reg = <0>; > + }; > + > + cpu1: cpu@1 { > + device_type = "cpu"; > + compatible = "arm,cortex-a9"; > + next-level-cache = <&L2>; > + reg = <1>; > + }; > + }; > diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt > index 91e6e5c..6abe3f3 100644 > --- a/Documentation/devicetree/bindings/arm/cpus.txt > +++ b/Documentation/devicetree/bindings/arm/cpus.txt > @@ -191,6 +191,7 @@ nodes to be present and contain the properties described below. > "allwinner,sun8i-a23" > "arm,psci" > "brcm,brahma-b15" > + "brcm,bcm-nsp-smp" > "marvell,armada-375-smp" > "marvell,armada-380-smp" > "marvell,armada-390-smp" > -- > 2.1.0 >
Le 06/11/2015 13:11, Kapil Hali a écrit : > Add a compatible string "brcm,bcm-nsp-smp" for Broadcom's > Northstar Plus CPU to the 32-bit ARM CPU device tree binding > documentation file and create a new binding documentation for > Northstar Plus CPU. > > Signed-off-by: Kapil Hali <kapilh@broadcom.com> > --- > .../bindings/arm/bcm/brcm,nsp-cpu-method.txt | 36 ++++++++++++++++++++++ > Documentation/devicetree/bindings/arm/cpus.txt | 1 + > 2 files changed, 37 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt > > diff --git a/Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt b/Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt > new file mode 100644 > index 0000000..8506da7 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt > @@ -0,0 +1,36 @@ > +Broadcom Northstar Plus SoC CPU Enable Method > +--------------------------------------------- > +This binding defines the enable method used for starting secondary > +CPUs in the following Broadcom SoCs: > + BCM58522, BCM58525, BCM58535, BCM58622, BCM58623, BCM58625, BCM88312 > + > +The enable method is specified by defining the following required > +properties in the "cpus" device tree node: > + - enable-method = "brcm,bcm-nsp-smp"; > + - secondary-boot-reg = <...>; > + > +The secondary-boot-reg property is a u32 value that specifies the > +physical address of the register used to request the ROM holding pen > +code release a secondary CPU. Is it really how the ROM code is implemented, as a pen holding/release mechanism (which sounds like how this was implemented previously in the kernel actually) or should this be described in a more generic way as the physical address of the register where the secondary CPUs reset vector address must be written to? Or something along these lines. > + > +Example: > + cpus { > + #address-cells = <1>; > + #size-cells = <0>; > + enable-method = "brcm,bcm-nsp-smp"; Just a nit, but if NSP and NS are sharing the same mechanism, would not a more "NS-centric" property be more appropriate because NS came before NSP? > + secondary-boot-reg = <0xffff042c>; > + > + cpu0: cpu@0 { > + device_type = "cpu"; > + compatible = "arm,cortex-a9"; > + next-level-cache = <&L2>; > + reg = <0>; > + }; > + > + cpu1: cpu@1 { > + device_type = "cpu"; > + compatible = "arm,cortex-a9"; > + next-level-cache = <&L2>; > + reg = <1>; > + }; > + }; > diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt > index 91e6e5c..6abe3f3 100644 > --- a/Documentation/devicetree/bindings/arm/cpus.txt > +++ b/Documentation/devicetree/bindings/arm/cpus.txt > @@ -191,6 +191,7 @@ nodes to be present and contain the properties described below. > "allwinner,sun8i-a23" > "arm,psci" > "brcm,brahma-b15" > + "brcm,bcm-nsp-smp" > "marvell,armada-375-smp" > "marvell,armada-380-smp" > "marvell,armada-390-smp" >
On Sat, Nov 07, 2015 at 01:40:23PM -0800, Florian Fainelli wrote: > Le 06/11/2015 13:11, Kapil Hali a écrit : > > Add a compatible string "brcm,bcm-nsp-smp" for Broadcom's > > Northstar Plus CPU to the 32-bit ARM CPU device tree binding > > documentation file and create a new binding documentation for > > Northstar Plus CPU. > > > > Signed-off-by: Kapil Hali <kapilh@broadcom.com> > > --- > > .../bindings/arm/bcm/brcm,nsp-cpu-method.txt | 36 ++++++++++++++++++++++ > > Documentation/devicetree/bindings/arm/cpus.txt | 1 + > > 2 files changed, 37 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt > > > > diff --git a/Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt b/Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt > > new file mode 100644 > > index 0000000..8506da7 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt > > @@ -0,0 +1,36 @@ > > +Broadcom Northstar Plus SoC CPU Enable Method > > +--------------------------------------------- > > +This binding defines the enable method used for starting secondary > > +CPUs in the following Broadcom SoCs: > > + BCM58522, BCM58525, BCM58535, BCM58622, BCM58623, BCM58625, BCM88312 > > + > > +The enable method is specified by defining the following required > > +properties in the "cpus" device tree node: > > + - enable-method = "brcm,bcm-nsp-smp"; > > + - secondary-boot-reg = <...>; > > + > > +The secondary-boot-reg property is a u32 value that specifies the > > +physical address of the register used to request the ROM holding pen > > +code release a secondary CPU. > > Is it really how the ROM code is implemented, as a pen holding/release > mechanism (which sounds like how this was implemented previously in the > kernel actually) or should this be described in a more generic way as > the physical address of the register where the secondary CPUs reset > vector address must be written to? Or something along these lines. Why do people insist on using holding pens to bring their secondary CPUs into existence? I hope the hardware people aren't being dumb and have no way to hold in reset or power down their secondary CPUs, either of which is a vital feature for things like kexec and the like. If they do have a way to hold secondary CPUs in reset or powered down, why aren't they using that at boot instead of implementing the stupid Versatile scheme, which exists because Versatile _can't_ hold its CPUs in reset or power them down... It's times like this that I wonder what kind of drugs the hardware SoC people are on, but I'm well aware that people contributing SMP bringup solutions are also dumb idiots who copy the Versatile scheme with very little thought... (as you can see, I'm not mincing my words here - if people want to be lazy in this regard despite this having been brought up multiple times, and the lead developers having said that the versatile pen_release stuff should not be used, they earn themselves the right to be called dumb idiots. Simple solution to avoid that title: don't be a dumb idiot by copy the Versatile SMP bring up code! It's not a sane model for any SoC sane SoC to follow.) Is this clear enough?
2015-11-08 9:31 GMT-08:00 Russell King - ARM Linux <linux@arm.linux.org.uk>: >> Is it really how the ROM code is implemented, as a pen holding/release >> mechanism (which sounds like how this was implemented previously in the >> kernel actually) or should this be described in a more generic way as >> the physical address of the register where the secondary CPUs reset >> vector address must be written to? Or something along these lines. > > Why do people insist on using holding pens to bring their secondary CPUs > into existence? I hope the hardware people aren't being dumb and have no > way to hold in reset or power down their secondary CPUs, either of which > is a vital feature for things like kexec and the like. If they do have > a way to hold secondary CPUs in reset or powered down, why aren't they > using that at boot instead of implementing the stupid Versatile scheme, > which exists because Versatile _can't_ hold its CPUs in reset or power > them down... There are few implementations out there which suffer from this same mistake (mostly MIPS implementations) but that is not really relevant here. Most of the time this comes from not understanding software models and/or not properly taking into account a complex (too complex) reset model. > > It's times like this that I wonder what kind of drugs the hardware SoC > people are on, but I'm well aware that people contributing SMP bringup > solutions are also dumb idiots who copy the Versatile scheme with very > little thought... (as you can see, I'm not mincing my words here - if > people want to be lazy in this regard despite this having been brought > up multiple times, and the lead developers having said that the versatile > pen_release stuff should not be used, they earn themselves the right to > be called dumb idiots. Simple solution to avoid that title: don't be a > dumb idiot by copy the Versatile SMP bring up code! It's not a sane > model for any SoC sane SoC to follow.) > > Is this clear enough? The actual implementation of the SMP code in the next patches do not use a pen holding/release mechanism anymore as it used in the previous iterations of these same patches, so I would say, lesson learned. My question was whether the binding was documenting the hardware implementation (which it should) or the software implementation (which it should not).
Hi Russel, On 11/8/2015 11:01 PM, Russell King - ARM Linux wrote: > On Sat, Nov 07, 2015 at 01:40:23PM -0800, Florian Fainelli wrote: >> Le 06/11/2015 13:11, Kapil Hali a écrit : >>> Add a compatible string "brcm,bcm-nsp-smp" for Broadcom's >>> Northstar Plus CPU to the 32-bit ARM CPU device tree binding >>> documentation file and create a new binding documentation for >>> Northstar Plus CPU. >>> >>> Signed-off-by: Kapil Hali <kapilh@broadcom.com> >>> --- >>> .../bindings/arm/bcm/brcm,nsp-cpu-method.txt | 36 ++++++++++++++++++++++ >>> Documentation/devicetree/bindings/arm/cpus.txt | 1 + >>> 2 files changed, 37 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt >>> >>> diff --git a/Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt b/Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt >>> new file mode 100644 >>> index 0000000..8506da7 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt >>> @@ -0,0 +1,36 @@ >>> +Broadcom Northstar Plus SoC CPU Enable Method >>> +--------------------------------------------- >>> +This binding defines the enable method used for starting secondary >>> +CPUs in the following Broadcom SoCs: >>> + BCM58522, BCM58525, BCM58535, BCM58622, BCM58623, BCM58625, BCM88312 >>> + >>> +The enable method is specified by defining the following required >>> +properties in the "cpus" device tree node: >>> + - enable-method = "brcm,bcm-nsp-smp"; >>> + - secondary-boot-reg = <...>; >>> + >>> +The secondary-boot-reg property is a u32 value that specifies the >>> +physical address of the register used to request the ROM holding pen >>> +code release a secondary CPU. >> >> Is it really how the ROM code is implemented, as a pen holding/release >> mechanism (which sounds like how this was implemented previously in the >> kernel actually) or should this be described in a more generic way as >> the physical address of the register where the secondary CPUs reset >> vector address must be written to? Or something along these lines. > > Why do people insist on using holding pens to bring their secondary CPUs > into existence? I hope the hardware people aren't being dumb and have no > way to hold in reset or power down their secondary CPUs, either of which > is a vital feature for things like kexec and the like. If they do have > a way to hold secondary CPUs in reset or powered down, why aren't they > using that at boot instead of implementing the stupid Versatile scheme, > which exists because Versatile _can't_ hold its CPUs in reset or power > them down... > > It's times like this that I wonder what kind of drugs the hardware SoC > people are on, but I'm well aware that people contributing SMP bringup > solutions are also dumb idiots who copy the Versatile scheme with very > little thought... (as you can see, I'm not mincing my words here - if > people want to be lazy in this regard despite this having been brought > up multiple times, and the lead developers having said that the versatile > pen_release stuff should not be used, they earn themselves the right to > be called dumb idiots. Simple solution to avoid that title: don't be a > dumb idiot by copy the Versatile SMP bring up code! It's not a sane > model for any SoC sane SoC to follow.) > > Is this clear enough? > It was clear the very first time itself as pointed out by you and the lead developers and hence the change was readily sent in the very next patch set. I didn't change a comment in this patch, which is misleading about the SMP enable-method used in the patch set, it is my mistake and I apologies for the same. I will change it and send the next patch set. Also, before sending out the patch set, I had asked for a clarification about the method: https://lkml.org/lkml/2015/11/6/234 For my understanding, I am repeating my query- In case of simple method of waking up secondary core, smp_boot_secondary() will always return success indicating secondary core successfully started. I understand that in __cpu_up(), primary core waits for completion till secondary core comes online or time outs. However, is it appropriate to return successful start of secondary core without knowing if it really did? Thanks, Kapil Hali
Hi Florian, On 11/8/2015 3:10 AM, Florian Fainelli wrote: > Le 06/11/2015 13:11, Kapil Hali a écrit : >> Add a compatible string "brcm,bcm-nsp-smp" for Broadcom's >> Northstar Plus CPU to the 32-bit ARM CPU device tree binding >> documentation file and create a new binding documentation for >> Northstar Plus CPU. >> >> Signed-off-by: Kapil Hali <kapilh@broadcom.com> >> --- >> .../bindings/arm/bcm/brcm,nsp-cpu-method.txt | 36 ++++++++++++++++++++++ >> Documentation/devicetree/bindings/arm/cpus.txt | 1 + >> 2 files changed, 37 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt >> >> diff --git a/Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt b/Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt >> new file mode 100644 >> index 0000000..8506da7 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt >> @@ -0,0 +1,36 @@ >> +Broadcom Northstar Plus SoC CPU Enable Method >> +--------------------------------------------- >> +This binding defines the enable method used for starting secondary >> +CPUs in the following Broadcom SoCs: >> + BCM58522, BCM58525, BCM58535, BCM58622, BCM58623, BCM58625, BCM88312 >> + >> +The enable method is specified by defining the following required >> +properties in the "cpus" device tree node: >> + - enable-method = "brcm,bcm-nsp-smp"; >> + - secondary-boot-reg = <...>; >> + >> +The secondary-boot-reg property is a u32 value that specifies the >> +physical address of the register used to request the ROM holding pen >> +code release a secondary CPU. > > Is it really how the ROM code is implemented, as a pen holding/release > mechanism (which sounds like how this was implemented previously in the > kernel actually) or should this be described in a more generic way as > the physical address of the register where the secondary CPUs reset > vector address must be written to? Or something along these lines. > I overlooked this patch and didn't change the description. It is a physical address of a register which holds the address of the secondary core's entry point. >> + >> +Example: >> + cpus { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + enable-method = "brcm,bcm-nsp-smp"; > > Just a nit, but if NSP and NS are sharing the same mechanism, would not > a more "NS-centric" property be more appropriate because NS came before NSP? > >> + secondary-boot-reg = <0xffff042c>; >> + >> + cpu0: cpu@0 { >> + device_type = "cpu"; >> + compatible = "arm,cortex-a9"; >> + next-level-cache = <&L2>; >> + reg = <0>; >> + }; >> + >> + cpu1: cpu@1 { >> + device_type = "cpu"; >> + compatible = "arm,cortex-a9"; >> + next-level-cache = <&L2>; >> + reg = <1>; >> + }; >> + }; >> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt >> index 91e6e5c..6abe3f3 100644 >> --- a/Documentation/devicetree/bindings/arm/cpus.txt >> +++ b/Documentation/devicetree/bindings/arm/cpus.txt >> @@ -191,6 +191,7 @@ nodes to be present and contain the properties described below. >> "allwinner,sun8i-a23" >> "arm,psci" >> "brcm,brahma-b15" >> + "brcm,bcm-nsp-smp" >> "marvell,armada-375-smp" >> "marvell,armada-380-smp" >> "marvell,armada-390-smp" >> > > Thanks, Kapil Hali
On Tue, Nov 10, 2015 at 09:33:12PM +0530, Kapil Hali wrote: > Hi Russel, Wrong. Look at my name as sent in the From: and as quoted in the very next line. As far as I'm concerned (and I don't care what other people say) it's disrespectful to spell people's names incorrectly. > It was clear the very first time itself as pointed out by you and the > lead developers and hence the change was readily sent in the very next > patch set. I didn't change a comment in this patch, which is misleading > about the SMP enable-method used in the patch set, it is my mistake and > I apologies for the same. I will change it and send the next patch set. Thanks. > Also, before sending out the patch set, I had asked for a clarification > about the method: https://lkml.org/lkml/2015/11/6/234 Sorry, I don't read every single email irrespective of how it's marked. There's way too much email, and way too much mail with improperly classified recipient lists to be able to usefully sort this mail. (If you do the math, the email rate during a 12 hour working day from just linux-arm-kernel is one email every 2.5 minutes, assuming 300 emails a day. It takes way longer than that to compose a proper reply to an email - I've spent around 15 minutes on this one alone. Hence, if I'm busy, I more or less totally ignore email now, and rarely bother to "catch up" with missed emails.) > For my understanding, I am repeating my query- In case of simple method of > waking up secondary core, smp_boot_secondary() will always return success > indicating secondary core successfully started. I understand that in > __cpu_up(), primary core waits for completion till secondary core comes > online or time outs. However, is it appropriate to return successful start > of secondary core without knowing if it really did? Yes, because all that your smp_boot_secondary() should be doing is trying to start the core. If you encounter an error trying to do so, that's what the error return is for. If you just set a bit somewhere to tell the hardware to release the secondary core's reset, then if you set the bit and return success, that's prefectly acceptable. The core ARM SMP code will then wait up to one second for the secondary core to become known to the kernel before declaring that the CPU failed to come online. If it fails to appear, we assume that it will never appear - and actually at that point the system is in an unknown state: if the secondary CPU crashed during its boot, it could start scribbling into memory or touching devices in an unpredictable way: the only sane answer is to reboot the whole system to ensure that it's back to a known good state. Hence why we don't provide any cleanup at the point of a failed secondary CPU (I've been debating about tainting the kernel at that point, so we know when things have gone bad.) Hope this helps.
Hi Rob, On 11/7/2015 11:33 PM, Rob Herring wrote: > On Fri, Nov 6, 2015 at 3:11 PM, Kapil Hali <kapilh@broadcom.com> wrote: >> Add a compatible string "brcm,bcm-nsp-smp" for Broadcom's >> Northstar Plus CPU to the 32-bit ARM CPU device tree binding >> documentation file and create a new binding documentation for >> Northstar Plus CPU. >> >> Signed-off-by: Kapil Hali <kapilh@broadcom.com> >> --- >> .../bindings/arm/bcm/brcm,nsp-cpu-method.txt | 36 ++++++++++++++++++++++ >> Documentation/devicetree/bindings/arm/cpus.txt | 1 + >> 2 files changed, 37 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt >> >> diff --git a/Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt b/Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt >> new file mode 100644 >> index 0000000..8506da7 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt >> @@ -0,0 +1,36 @@ >> +Broadcom Northstar Plus SoC CPU Enable Method >> +--------------------------------------------- >> +This binding defines the enable method used for starting secondary >> +CPUs in the following Broadcom SoCs: >> + BCM58522, BCM58525, BCM58535, BCM58622, BCM58623, BCM58625, BCM88312 >> + >> +The enable method is specified by defining the following required >> +properties in the "cpus" device tree node: >> + - enable-method = "brcm,bcm-nsp-smp"; > > As I said already, this is supposed to be per cpu. > >> + - secondary-boot-reg = <...>; > > And then you might as well move this too. > NS/NSP family of SoCs have maximum of two cores. There would not be a need for another boot-reg in this family of SoCs. However, I agree, it should go to individual CPU nodes. I will do the change in the next patch. >> + >> +The secondary-boot-reg property is a u32 value that specifies the >> +physical address of the register used to request the ROM holding pen >> +code release a secondary CPU. >> + >> +Example: >> + cpus { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + enable-method = "brcm,bcm-nsp-smp"; >> + secondary-boot-reg = <0xffff042c>; >> + >> + cpu0: cpu@0 { >> + device_type = "cpu"; >> + compatible = "arm,cortex-a9"; >> + next-level-cache = <&L2>; >> + reg = <0>; >> + }; >> + >> + cpu1: cpu@1 { >> + device_type = "cpu"; >> + compatible = "arm,cortex-a9"; >> + next-level-cache = <&L2>; >> + reg = <1>; >> + }; >> + }; >> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt >> index 91e6e5c..6abe3f3 100644 >> --- a/Documentation/devicetree/bindings/arm/cpus.txt >> +++ b/Documentation/devicetree/bindings/arm/cpus.txt >> @@ -191,6 +191,7 @@ nodes to be present and contain the properties described below. >> "allwinner,sun8i-a23" >> "arm,psci" >> "brcm,brahma-b15" >> + "brcm,bcm-nsp-smp" >> "marvell,armada-375-smp" >> "marvell,armada-380-smp" >> "marvell,armada-390-smp" >> -- >> 2.1.0 >> Thanks, Kapil Hali
Hi Russell, On 11/10/2015 9:55 PM, Russell King - ARM Linux wrote: > On Tue, Nov 10, 2015 at 09:33:12PM +0530, Kapil Hali wrote: >> Hi Russel, > > Wrong. Look at my name as sent in the From: and as quoted in the very > next line. As far as I'm concerned (and I don't care what other people > say) it's disrespectful to spell people's names incorrectly. > I am sincerely apologetic about it. It was a deviation that will not repeat again. >> It was clear the very first time itself as pointed out by you and the >> lead developers and hence the change was readily sent in the very next >> patch set. I didn't change a comment in this patch, which is misleading >> about the SMP enable-method used in the patch set, it is my mistake and >> I apologies for the same. I will change it and send the next patch set. > > Thanks. > >> Also, before sending out the patch set, I had asked for a clarification >> about the method: https://lkml.org/lkml/2015/11/6/234 > > Sorry, I don't read every single email irrespective of how it's marked. > There's way too much email, and way too much mail with improperly > classified recipient lists to be able to usefully sort this mail. > (If you do the math, the email rate during a 12 hour working day from > just linux-arm-kernel is one email every 2.5 minutes, assuming 300 emails > a day. It takes way longer than that to compose a proper reply to an > email - I've spent around 15 minutes on this one alone. Hence, if I'm > busy, I more or less totally ignore email now, and rarely bother to > "catch up" with missed emails.) > >> For my understanding, I am repeating my query- In case of simple method of >> waking up secondary core, smp_boot_secondary() will always return success >> indicating secondary core successfully started. I understand that in >> __cpu_up(), primary core waits for completion till secondary core comes >> online or time outs. However, is it appropriate to return successful start >> of secondary core without knowing if it really did? > > Yes, because all that your smp_boot_secondary() should be doing is > trying to start the core. If you encounter an error trying to do so, > that's what the error return is for. > > If you just set a bit somewhere to tell the hardware to release the > secondary core's reset, then if you set the bit and return success, > that's prefectly acceptable. The core ARM SMP code will then wait > up to one second for the secondary core to become known to the kernel > before declaring that the CPU failed to come online. > > If it fails to appear, we assume that it will never appear - and > actually at that point the system is in an unknown state: if the > secondary CPU crashed during its boot, it could start scribbling > into memory or touching devices in an unpredictable way: the only > sane answer is to reboot the whole system to ensure that it's back > to a known good state. Hence why we don't provide any cleanup at > the point of a failed secondary CPU (I've been debating about > tainting the kernel at that point, so we know when things have > gone bad.) > > Hope this helps. > Surely it has helped and many thanks for your detailed explanation. Thanks, Kapil Hali
diff --git a/Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt b/Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt new file mode 100644 index 0000000..8506da7 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt @@ -0,0 +1,36 @@ +Broadcom Northstar Plus SoC CPU Enable Method +--------------------------------------------- +This binding defines the enable method used for starting secondary +CPUs in the following Broadcom SoCs: + BCM58522, BCM58525, BCM58535, BCM58622, BCM58623, BCM58625, BCM88312 + +The enable method is specified by defining the following required +properties in the "cpus" device tree node: + - enable-method = "brcm,bcm-nsp-smp"; + - secondary-boot-reg = <...>; + +The secondary-boot-reg property is a u32 value that specifies the +physical address of the register used to request the ROM holding pen +code release a secondary CPU. + +Example: + cpus { + #address-cells = <1>; + #size-cells = <0>; + enable-method = "brcm,bcm-nsp-smp"; + secondary-boot-reg = <0xffff042c>; + + cpu0: cpu@0 { + device_type = "cpu"; + compatible = "arm,cortex-a9"; + next-level-cache = <&L2>; + reg = <0>; + }; + + cpu1: cpu@1 { + device_type = "cpu"; + compatible = "arm,cortex-a9"; + next-level-cache = <&L2>; + reg = <1>; + }; + }; diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt index 91e6e5c..6abe3f3 100644 --- a/Documentation/devicetree/bindings/arm/cpus.txt +++ b/Documentation/devicetree/bindings/arm/cpus.txt @@ -191,6 +191,7 @@ nodes to be present and contain the properties described below. "allwinner,sun8i-a23" "arm,psci" "brcm,brahma-b15" + "brcm,bcm-nsp-smp" "marvell,armada-375-smp" "marvell,armada-380-smp" "marvell,armada-390-smp"
Add a compatible string "brcm,bcm-nsp-smp" for Broadcom's Northstar Plus CPU to the 32-bit ARM CPU device tree binding documentation file and create a new binding documentation for Northstar Plus CPU. Signed-off-by: Kapil Hali <kapilh@broadcom.com> --- .../bindings/arm/bcm/brcm,nsp-cpu-method.txt | 36 ++++++++++++++++++++++ Documentation/devicetree/bindings/arm/cpus.txt | 1 + 2 files changed, 37 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/bcm/brcm,nsp-cpu-method.txt