Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 5/5] arm64: defconfig: Enable QCOM IPQ8074 clock and pinctrl
From: Varadarajan Narayanan @ 2017-04-28  9:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1493373403-23462-1-git-send-email-varada@codeaurora.org>

From: Abhishek Sahu <absahu@codeaurora.org>

These configs are required for booting kernel in QCOM
IPQ8074 boards.

Signed-off-by: Abhishek Sahu <absahu@codeaurora.org>
Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
---
 arch/arm64/configs/defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 7c48028..e6aeb69 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -266,6 +266,7 @@ CONFIG_SPI_SPIDEV=m
 CONFIG_SPI_S3C64XX=y
 CONFIG_SPMI=y
 CONFIG_PINCTRL_SINGLE=y
+CONFIG_PINCTRL_IPQ8074=y
 CONFIG_PINCTRL_MAX77620=y
 CONFIG_PINCTRL_MSM8916=y
 CONFIG_PINCTRL_MSM8994=y
@@ -439,6 +440,7 @@ CONFIG_COMMON_CLK_PWM=y
 CONFIG_COMMON_CLK_RK808=y
 CONFIG_CLK_QORIQ=y
 CONFIG_COMMON_CLK_QCOM=y
+CONFIG_IPQ_GCC_8074=y
 CONFIG_MSM_GCC_8916=y
 CONFIG_MSM_GCC_8994=y
 CONFIG_MSM_MMCC_8996=y
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related

* [PATCHv2 2/2] arm64: cpufeature: use static_branch_enable_cpuslocked()
From: Mark Rutland @ 2017-04-28 10:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170427180104.GO1890@arm.com>

On Thu, Apr 27, 2017 at 07:01:04PM +0100, Will Deacon wrote:
> On Thu, Apr 27, 2017 at 06:44:37PM +0100, Mark Rutland wrote:
> > diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> > index f6cc67e..379ad8d 100644
> > --- a/arch/arm64/kernel/cpu_errata.c
> > +++ b/arch/arm64/kernel/cpu_errata.c
> > @@ -175,9 +175,20 @@ void verify_local_cpu_errata_workarounds(void)
> >  		}
> >  }
> >  
> > -void update_cpu_errata_workarounds(void)
> > +/*
> > + * Secondary CPUs are booted with the waker holding the
> > + * CPU hotplug lock, hence we don't need to lock it here again.
> > + */
> > +void update_secondary_cpu_errata_workarounds(void)
> > +{
> > +	update_cpu_capabilities(arm64_errata, "enabling workaround for");
> > +}
> > +
> > +void update_boot_cpu_errata_workarounds(void)
> >  {
> > +	get_online_cpus();
> >  	update_cpu_capabilities(arm64_errata, "enabling workaround for");
> > +	put_online_cpus();
> >  }
> 
> These functions seem to have unhelpful names, especially when compared to
> the naming scheme used by the core code. I'd prefer to have:
> 
> update_cpu_errata_workarounds: just calls update_cpu_capabilities
> 
> update_cpu_errata_workarounds_cpuslocked: does get_online_cpus(), then calls
> update_cpu_errata_workarounds, then does put_online_cpus();

That's the opposite polarity to the other _cpuslocked functions, where
_cpuslocked means that the lock is already held (and should not be taken
by the _cpuslocked function itself.

So I'll make those changes, but I'll swap that so:
update_cpu_errata_workarounds() does:

  get_online_cpus()
  update_cpu_errata_workarounds_cpuslocked()
  put_online_cpus()

> With that change:
> 
> Acked-by: Will Deacon <will.deacon@arm.com>

I assume that will stand with the above change. Please shout if not!

Thanks,
Mark.

^ permalink raw reply

* [PATCH] IMX: Rearm watchdog after loading value for restart
From: Lothar Waßmann @ 2017-04-28 10:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170427131434.17085-1-Frederik.juul@3shape.com>

Hi,

On Thu, 27 Apr 2017 15:14:34 +0200 frederikj at gmail.com wrote:
> When calling the restart function the watchdog is activated and the WDOG
> timeout field is set to 0. This gives a reset time of 500 ms, which is
> consistent with the following delay. However this new time is not loaded
> into the WDOG until the WDOG service routine is run (see IMX6SXRM section
> 70.5.1.1 "Servicing WDOG to reload the counter").
> 
> Not reloading the counter could result in a random delay up to 128 seconds
> before the system restarts, depending on the previous value of the WDOG
> timeout field and when it has last been serviced. That bug is fixed with
> this patch.
>
The routine is actually initiating a softreset by clearing the SRS bit
in WCR. Thus the watchdog timeout should only have any effect, if the
softreset fails in some way. Under normal circumstances the reset
should happen immediately no matter what watchdog timeout is programmed.


Lothar Wa?mann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________

^ permalink raw reply

* [PATCHv2 2/2] arm64: cpufeature: use static_branch_enable_cpuslocked()
From: Will Deacon @ 2017-04-28 10:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170428100230.GA22621@leverpostej>

On Fri, Apr 28, 2017 at 11:02:30AM +0100, Mark Rutland wrote:
> On Thu, Apr 27, 2017 at 07:01:04PM +0100, Will Deacon wrote:
> > On Thu, Apr 27, 2017 at 06:44:37PM +0100, Mark Rutland wrote:
> > > diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> > > index f6cc67e..379ad8d 100644
> > > --- a/arch/arm64/kernel/cpu_errata.c
> > > +++ b/arch/arm64/kernel/cpu_errata.c
> > > @@ -175,9 +175,20 @@ void verify_local_cpu_errata_workarounds(void)
> > >  		}
> > >  }
> > >  
> > > -void update_cpu_errata_workarounds(void)
> > > +/*
> > > + * Secondary CPUs are booted with the waker holding the
> > > + * CPU hotplug lock, hence we don't need to lock it here again.
> > > + */
> > > +void update_secondary_cpu_errata_workarounds(void)
> > > +{
> > > +	update_cpu_capabilities(arm64_errata, "enabling workaround for");
> > > +}
> > > +
> > > +void update_boot_cpu_errata_workarounds(void)
> > >  {
> > > +	get_online_cpus();
> > >  	update_cpu_capabilities(arm64_errata, "enabling workaround for");
> > > +	put_online_cpus();
> > >  }
> > 
> > These functions seem to have unhelpful names, especially when compared to
> > the naming scheme used by the core code. I'd prefer to have:
> > 
> > update_cpu_errata_workarounds: just calls update_cpu_capabilities
> > 
> > update_cpu_errata_workarounds_cpuslocked: does get_online_cpus(), then calls
> > update_cpu_errata_workarounds, then does put_online_cpus();
> 
> That's the opposite polarity to the other _cpuslocked functions, where
> _cpuslocked means that the lock is already held (and should not be taken
> by the _cpuslocked function itself.
> 
> So I'll make those changes, but I'll swap that so:
> update_cpu_errata_workarounds() does:
> 
>   get_online_cpus()
>   update_cpu_errata_workarounds_cpuslocked()
>   put_online_cpus()
> 
> > With that change:
> > 
> > Acked-by: Will Deacon <will.deacon@arm.com>
> 
> I assume that will stand with the above change. Please shout if not!

Haha, yes, I got it downside-up. Thanks for working out what I meant.

Will

^ permalink raw reply

* [PATCH] arm: dts: qcom: add gsbi7 serial to ipq8064 SoC device tree
From: Sven Eckelmann @ 2017-04-28 10:10 UTC (permalink / raw)
  To: linux-arm-kernel

The gsbi_serial7 under gsbi7 is used by the IPQ8068 based board EWS870AP as
main serial console.

Signed-off-by: Sven Eckelmann <sven.eckelmann@openmesh.com>
---
 arch/arm/boot/dts/qcom-ipq8064.dtsi | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom-ipq8064.dtsi
index 76f4e8921d58..f1fbffe59b93 100644
--- a/arch/arm/boot/dts/qcom-ipq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-ipq8064.dtsi
@@ -284,6 +284,29 @@
 			};
 		};
 
+		gsbi7: gsbi at 16600000 {
+			status = "disabled";
+			compatible = "qcom,gsbi-v1.0.0";
+			cell-index = <7>;
+			reg = <0x16600000 0x100>;
+			clocks = <&gcc GSBI7_H_CLK>;
+			clock-names = "iface";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+			syscon-tcsr = <&tcsr>;
+
+			gsbi7_serial: serial at 16640000 {
+				compatible = "qcom,msm-uartdm-v1.3", "qcom,msm-uartdm";
+				reg = <0x16640000 0x1000>,
+				      <0x16600000 0x1000>;
+				interrupts = <0 158 0x0>;
+				clocks = <&gcc GSBI7_UART_CLK>, <&gcc GSBI7_H_CLK>;
+				clock-names = "core", "iface";
+				status = "disabled";
+			};
+		};
+
 		sata_phy: sata-phy at 1b400000 {
 			compatible = "qcom,ipq806x-sata-phy";
 			reg = <0x1b400000 0x200>;
-- 
2.11.0

^ permalink raw reply related

* [PATCH v2] arm64: Add ASM modifier for xN register operands
From: Ard Biesheuvel @ 2017-04-28 10:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170428095349.GA22161@leverpostej>

On 28 April 2017 at 10:53, Mark Rutland <mark.rutland@arm.com> wrote:
> On Fri, Apr 28, 2017 at 08:18:52AM +0100, Ard Biesheuvel wrote:
>> On 27 April 2017 at 23:52, Matthias Kaehlcke <mka@chromium.org> wrote:
>> > El Thu, Apr 27, 2017 at 12:02:56PM +0100 Mark Rutland ha dit:
>> >> On Wed, Apr 26, 2017 at 02:46:16PM -0700, Matthias Kaehlcke wrote:
>
>> >> > -   asm volatile("strb %w0, [%1]" : : "rZ" (val), "r" (addr));
>> >> > +   asm volatile("strb %w0, [%x1]" : : "rZ" (val), "r" (addr));
>> >>
>> >> In general, the '[%xN]' pattern looks *very* suspicious to me. Any
>> >> address must be 64-bit, so this would mask a legitimate warning.
>> >>
>> >> Given the prototype of this function the code if fine either way, but
>> >> were we to refactor things (e.g. making this a macro), that might not be
>> >> true.
>> >>
>> >> ... so I'm not sure it make sense to alter instances used for addresses.
>> >
>> > Good point, I'll leave instances dealing with addresses untouched for now.
>> >
>>
>> OK, I am confused now. We started this thread under the assumption
>> that all unqualified placeholders are warned about by Clang. Given
>> that this appears not to be the case, could we please first find out
>> what causes the warnings?
>
> Yes please.
>
>> Is it necessary at all to add the x modifiers for 64-bit types?
>
> Having delved a little deeper, I think this is actively harmful, and
> clang's warning indicates potential problems even when compiling with
> GCC.
>
> The below test simulates how we might write to control regs and so on,
> with a mov in asm simulating something like an msr.
>
> ---->8----
> #include <stdio.h>
>
> static inline unsigned long generate_val(void)
> {
>         unsigned long val;
>
>         /* hide value generation from GCC */
>         asm (
>                 "movn %0, #0"
>                 : "=r" (val)
>         );
>
>         return val;
> }
>
> static inline unsigned long use_val_32(unsigned int in)
> {
>         unsigned long out;
>
>         /* simulate what we might write to a sysreg */
>         asm (
>                 "mov %x0, %x1"
>                 : "=r" (out)
>                 : "r" (in)
>         );
>
>         return out;
> }
>
> int main(int argc, char *argv)
> {
>         printf("32-bit val is: 0x%016lx\n", use_val_32(generate_val()));
>
>         return 0;
> }
> ---->8----
>
> Depending on optimization level, bits that we do not expect can flow through:
>
> $ gcc test.c -o test
> $ ./test
> 32-bit val is: 0x00000000ffffffff
> $ gcc test.c -O1 -o test
> $ ./test
> 32-bit val is: 0xffffffffffffffff
> $ gcc test.c -O2 -o test
> $ ./test
> 32-bit val is: 0xffffffffffffffff
>
> ... that could be disastrous depending on how the result was used.
>
> With "in" cast to an unsigned long, the compiler realises it needs to perform
> any necessary truncation itself:
>
> $ gcc test.c -o test
> $ ./test
> 32-bit val is: 0x00000000ffffffff
> $ gcc test.c -O1 -o test
> $ ./test
> 32-bit val is: 0x00000000ffffffff
> $ gcc test.c -O2 -o test
> $ ./test
> 32-bit val is: 0x00000000ffffffff
> $ gcc test.c -O3 -o test
> $ ./test
> 32-bit val is: 0x00000000ffffffff
>
> I think that the correct fix is to use intermediate 64-bit variables, or
> casts, so that the compiler *must* use an x register, and consequently
> guarantees that all 64-bits of the register are as we expect.
>

But do we care about those top bits when writing a 32-bit system
register from a X register?

^ permalink raw reply

* [PATCH] dt: gta04: fix polarity of clocks for mcbsp4
From: Andreas Kemnade @ 2017-04-28 10:27 UTC (permalink / raw)
  To: linux-arm-kernel

The clock polarity setting of the mcbsp connected to
the modem was wrong so almost only noise
was received.
With this patch it is also the same as it was on
earlier non-dt kernels where it was working properly

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 arch/arm/boot/dts/omap3-gta04.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi b/arch/arm/boot/dts/omap3-gta04.dtsi
index 16b237d3..c0d9d4b 100644
--- a/arch/arm/boot/dts/omap3-gta04.dtsi
+++ b/arch/arm/boot/dts/omap3-gta04.dtsi
@@ -68,7 +68,8 @@
 		simple-audio-card,bitclock-master = <&telephony_link_master>;
 		simple-audio-card,frame-master = <&telephony_link_master>;
 		simple-audio-card,format = "i2s";
-
+		simple-audio-card,bitclock-inversion;
+		simple-audio-card,frame-inversion;
 		simple-audio-card,cpu {
 			sound-dai = <&mcbsp4>;
 		};
-- 
2.1.4

^ permalink raw reply related

* [PATCH] ARM: qcom_defconfig: Enable IPQ4019 clock and pinctrl
From: Abhishek Sahu @ 2017-04-28 10:27 UTC (permalink / raw)
  To: linux-arm-kernel

These configs are required for booting kernel in QCOM IPQ4019 boards.

Signed-off-by: Abhishek Sahu <absahu@codeaurora.org>
---
 arch/arm/configs/qcom_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/configs/qcom_defconfig b/arch/arm/configs/qcom_defconfig
index 4ffdd60..28c98c6 100644
--- a/arch/arm/configs/qcom_defconfig
+++ b/arch/arm/configs/qcom_defconfig
@@ -113,6 +113,7 @@ CONFIG_SPI_QUP=y
 CONFIG_SPMI=y
 CONFIG_PINCTRL_APQ8064=y
 CONFIG_PINCTRL_APQ8084=y
+CONFIG_PINCTRL_IPQ4019=y
 CONFIG_PINCTRL_IPQ8064=y
 CONFIG_PINCTRL_MSM8660=y
 CONFIG_PINCTRL_MSM8960=y
@@ -182,6 +183,7 @@ CONFIG_COMMON_CLK_QCOM=y
 CONFIG_QCOM_CLK_RPM=y
 CONFIG_QCOM_CLK_SMD_RPM=y
 CONFIG_APQ_MMCC_8084=y
+CONFIG_IPQ_GCC_4019=y
 CONFIG_IPQ_LCC_806X=y
 CONFIG_MSM_GCC_8660=y
 CONFIG_MSM_LCC_8960=y
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related

* [PATCH v2] arm64: Add ASM modifier for xN register operands
From: Mark Rutland @ 2017-04-28 10:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKv+Gu_GP56PL5z4zR7EwD=DeSdVGUs9LKY6sbNzF1k-U+XXUA@mail.gmail.com>

On Fri, Apr 28, 2017 at 11:20:21AM +0100, Ard Biesheuvel wrote:
> On 28 April 2017 at 10:53, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Fri, Apr 28, 2017 at 08:18:52AM +0100, Ard Biesheuvel wrote:
> >> On 27 April 2017 at 23:52, Matthias Kaehlcke <mka@chromium.org> wrote:
> >> > El Thu, Apr 27, 2017 at 12:02:56PM +0100 Mark Rutland ha dit:
> >> >> On Wed, Apr 26, 2017 at 02:46:16PM -0700, Matthias Kaehlcke wrote:
> >
> >> >> > -   asm volatile("strb %w0, [%1]" : : "rZ" (val), "r" (addr));
> >> >> > +   asm volatile("strb %w0, [%x1]" : : "rZ" (val), "r" (addr));
> >> >>
> >> >> In general, the '[%xN]' pattern looks *very* suspicious to me. Any
> >> >> address must be 64-bit, so this would mask a legitimate warning.
> >> >>
> >> >> Given the prototype of this function the code if fine either way, but
> >> >> were we to refactor things (e.g. making this a macro), that might not be
> >> >> true.
> >> >>
> >> >> ... so I'm not sure it make sense to alter instances used for addresses.
> >> >
> >> > Good point, I'll leave instances dealing with addresses untouched for now.
> >> >
> >>
> >> OK, I am confused now. We started this thread under the assumption
> >> that all unqualified placeholders are warned about by Clang. Given
> >> that this appears not to be the case, could we please first find out
> >> what causes the warnings?
> >
> > Yes please.
> >
> >> Is it necessary at all to add the x modifiers for 64-bit types?
> >
> > Having delved a little deeper, I think this is actively harmful, and
> > clang's warning indicates potential problems even when compiling with
> > GCC.
> >
> > The below test simulates how we might write to control regs and so on,
> > with a mov in asm simulating something like an msr.
> >
> > ---->8----
> > #include <stdio.h>
> >
> > static inline unsigned long generate_val(void)
> > {
> >         unsigned long val;
> >
> >         /* hide value generation from GCC */
> >         asm (
> >                 "movn %0, #0"
> >                 : "=r" (val)
> >         );
> >
> >         return val;
> > }
> >
> > static inline unsigned long use_val_32(unsigned int in)
> > {
> >         unsigned long out;
> >
> >         /* simulate what we might write to a sysreg */
> >         asm (
> >                 "mov %x0, %x1"
> >                 : "=r" (out)
> >                 : "r" (in)
> >         );
> >
> >         return out;
> > }
> >
> > int main(int argc, char *argv)
> > {
> >         printf("32-bit val is: 0x%016lx\n", use_val_32(generate_val()));
> >
> >         return 0;
> > }
> > ---->8----
> >
> > Depending on optimization level, bits that we do not expect can flow through:
> >
> > $ gcc test.c -o test
> > $ ./test
> > 32-bit val is: 0x00000000ffffffff
> > $ gcc test.c -O1 -o test
> > $ ./test
> > 32-bit val is: 0xffffffffffffffff
> > $ gcc test.c -O2 -o test
> > $ ./test
> > 32-bit val is: 0xffffffffffffffff
> >
> > ... that could be disastrous depending on how the result was used.
> >
> > With "in" cast to an unsigned long, the compiler realises it needs to perform
> > any necessary truncation itself:
> >
> > $ gcc test.c -o test
> > $ ./test
> > 32-bit val is: 0x00000000ffffffff
> > $ gcc test.c -O1 -o test
> > $ ./test
> > 32-bit val is: 0x00000000ffffffff
> > $ gcc test.c -O2 -o test
> > $ ./test
> > 32-bit val is: 0x00000000ffffffff
> > $ gcc test.c -O3 -o test
> > $ ./test
> > 32-bit val is: 0x00000000ffffffff
> >
> > I think that the correct fix is to use intermediate 64-bit variables, or
> > casts, so that the compiler *must* use an x register, and consequently
> > guarantees that all 64-bits of the register are as we expect.
> 
> But do we care about those top bits when writing a 32-bit system
> register from a X register?

Well, that only means the upper 32 bits are RES0, so yes. They could
gain a meaning on some future HW.

For sysregs, write_sysreg*() already solves this, as there's an implicit
cast to unsigned long via the function prototype. The 'x' modifier there
is only to ensure xzr can be used.

... however, this is a problem for any asm, as it can take input bits we
don't expect, and consequently generate output that we don't expect.
There's a potential functional correctness issue.

We need the precise set of warnings so that for each case we can
determine whether there is a potential issue today, or whether something
else protects us.

Thanks,
Mark.

^ permalink raw reply

* [PATCH v2 1/2] [media] rc-core: Add inlined stubs for core rc_* functions
From: Lee Jones @ 2017-04-28 10:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170412153956.13329-1-lee.jones@linaro.org>

On Wed, 12 Apr 2017, Lee Jones wrote:

> Currently users have to use all sorts of ugly #ifery within
> their drivers in order to avoid linking issues at build time.
> This patch allows users to safely call these functions when
> !CONFIG_RC_CORE and make decisions based on the return value
> instead.  This is a much more common and clean way of doing
> things within the Linux kernel.
> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
> 
> v1 => v2
>   - Use '#if IF_ENABLED()' instead of '#ifdef' in order to do the
>     right thing, even when CONFIG_RC_CORE=m.
>   
> include/media/rc-core.h | 42 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)

Hans,

What are your plans for this patch set?  Removal of #ifery from C
files has to be seen as an improvement, thus I see no reason why they
wouldn't be accepted.

Please advise.

> diff --git a/include/media/rc-core.h b/include/media/rc-core.h
> index 73ddd721..f176a9e 100644
> --- a/include/media/rc-core.h
> +++ b/include/media/rc-core.h
> @@ -209,7 +209,14 @@ struct rc_dev {
>   * @rc_driver_type: specifies the type of the RC output to be allocated
>   * returns a pointer to struct rc_dev.
>   */
> +#if IS_ENABLED(CONFIG_RC_CORE)
>  struct rc_dev *rc_allocate_device(enum rc_driver_type);
> +#else
> +static inline struct rc_dev *rc_allocate_device(int unused)
> +{
> +	return NULL;
> +}
> +#endif
>  
>  /**
>   * devm_rc_allocate_device - Managed RC device allocation
> @@ -218,21 +225,42 @@ struct rc_dev *rc_allocate_device(enum rc_driver_type);
>   * @rc_driver_type: specifies the type of the RC output to be allocated
>   * returns a pointer to struct rc_dev.
>   */
> +#if IS_ENABLED(CONFIG_RC_CORE)
>  struct rc_dev *devm_rc_allocate_device(struct device *dev, enum rc_driver_type);
> +#else
> +static inline struct rc_dev *devm_rc_allocate_device(struct device *dev, int unused)
> +{
> +	return NULL;
> +}
> +#endif
>  
>  /**
>   * rc_free_device - Frees a RC device
>   *
>   * @dev: pointer to struct rc_dev.
>   */
> +#if IS_ENABLED(CONFIG_RC_CORE)
>  void rc_free_device(struct rc_dev *dev);
> +#else
> +static inline void rc_free_device(struct rc_dev *dev)
> +{
> +	return;
> +}
> +#endif
>  
>  /**
>   * rc_register_device - Registers a RC device
>   *
>   * @dev: pointer to struct rc_dev.
>   */
> +#if IS_ENABLED(CONFIG_RC_CORE)
>  int rc_register_device(struct rc_dev *dev);
> +#else
> +static inline int rc_register_device(struct rc_dev *dev)
> +{
> +	return -EOPNOTSUPP;
> +}
> +#endif
>  
>  /**
>   * devm_rc_register_device - Manageded registering of a RC device
> @@ -240,14 +268,28 @@ int rc_register_device(struct rc_dev *dev);
>   * @parent: pointer to struct device.
>   * @dev: pointer to struct rc_dev.
>   */
> +#if IS_ENABLED(CONFIG_RC_CORE)
>  int devm_rc_register_device(struct device *parent, struct rc_dev *dev);
> +#else
> +static inline int devm_rc_register_device(struct device *parent, struct rc_dev *dev)
> +{
> +	return -EOPNOTSUPP;
> +}
> +#endif
>  
>  /**
>   * rc_unregister_device - Unregisters a RC device
>   *
>   * @dev: pointer to struct rc_dev.
>   */
> +#if IS_ENABLED(CONFIG_RC_CORE)
>  void rc_unregister_device(struct rc_dev *dev);
> +#else
> +static inline void rc_unregister_device(struct rc_dev *dev)
> +{
> +	return;
> +}
> +#endif
>  
>  /**
>   * rc_open - Opens a RC device

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH v5 17/22] KVM: arm64: vgic-its: Collection table save/restore
From: Christoffer Dall @ 2017-04-28 10:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1492164934-988-18-git-send-email-eric.auger@redhat.com>

On Fri, Apr 14, 2017 at 12:15:29PM +0200, Eric Auger wrote:
> The save path copies the collection entries into guest RAM
> at the GPA specified in the BASER register. This obviously
> requires the BASER to be set. The last written element is a
> dummy collection table entry.
> 
> We do not index by collection ID as the collection entry
> can fit into 8 bytes while containing the collection ID.
> 
> On restore path we re-allocate the collection objects.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> ---
> v4 -> v5:
> - add macros for field encoding/decoding
> - use abi->cte_esz
> - rename flush into save
> - check the target_addr is valid
> 
> v3 -> v4:
> - replaced u64 *ptr by gpa_t gpa
> - check the collection does not exist before allocating it
> 
> v1 -> v2:
> - reword commit message and directly use 8 as entry size
> - no kvm parameter anymore
> - add helper for flush/restore cte
> - table size computed here
> - add le64/cpu conversions
> ---
>  virt/kvm/arm/vgic/vgic-its.c | 109 ++++++++++++++++++++++++++++++++++++++++++-
>  virt/kvm/arm/vgic/vgic.h     |   9 ++++
>  2 files changed, 116 insertions(+), 2 deletions(-)
> 
> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> index c22b35d..484e541 100644
> --- a/virt/kvm/arm/vgic/vgic-its.c
> +++ b/virt/kvm/arm/vgic/vgic-its.c
> @@ -1785,13 +1785,97 @@ static int vgic_its_restore_device_tables(struct vgic_its *its)
>  	return -ENXIO;
>  }
>  
> +static int vgic_its_save_cte(struct vgic_its *its,
> +			     struct its_collection *collection,
> +			     gpa_t gpa, int esz)
> +{
> +	u64 val;
> +	int ret;
> +
> +	val = (1ULL << KVM_ITS_CTE_VALID_SHIFT |
> +	       ((u64)collection->target_addr << KVM_ITS_CTE_RDBASE_SHIFT) |
> +	       collection->collection_id);
> +	val = cpu_to_le64(val);
> +	ret = kvm_write_guest(its->dev->kvm, gpa, &val, esz);
> +	return ret;
> +}
> +
> +static int vgic_its_restore_cte(struct vgic_its *its, gpa_t gpa,
> +				int esz, bool *valid)
> +{
> +	struct its_collection *collection;
> +	struct kvm *kvm = its->dev->kvm;
> +	u32 target_addr;
> +	u32 coll_id;
> +	u64 val;
> +	int ret;
> +
> +	*valid = false;

I don't see why you need this.

> +
> +	ret = kvm_read_guest(kvm, gpa, &val, esz);

hmm, we better not have an esz larger than sizeof(u64) here then.

> +	if (ret)
> +		return ret;
> +	val = le64_to_cpu(val);
> +	*valid = val & KVM_ITS_CTE_VALID_MASK;
> +
> +	if (!*valid)
> +		return 0;

I think this would be nicer with just a bool on the stack and then set
*valid = in the end of the function.  Alternatively, you could just have
the return value return > 0 when there are more entries.

> +
> +	target_addr = (u32)(val >> KVM_ITS_CTE_RDBASE_SHIFT);
> +	coll_id = val & KVM_ITS_CTE_ICID_MASK;
> +
> +	if (target_addr >= atomic_read(&kvm->online_vcpus))
> +		return -EINVAL;
> +
> +	collection = find_collection(its, coll_id);
> +	if (collection)
> +		return -EEXIST;
> +	ret = vgic_its_alloc_collection(its, &collection, coll_id);
> +	if (ret)
> +		return ret;
> +	collection->target_addr = target_addr;
> +	return 0;
> +}
> +
>  /**
>   * vgic_its_save_collection_table - Save the collection table into
>   * guest RAM
>   */
>  static int vgic_its_save_collection_table(struct vgic_its *its)
>  {
> -	return -ENXIO;
> +	const struct vgic_its_abi *abi = vgic_its_get_abi(its);
> +	struct its_collection *collection;
> +	u64 val;
> +	gpa_t gpa;
> +	size_t max_size, filled = 0;
> +	int ret, cte_esz = abi->cte_esz;
> +
> +	gpa = BASER_ADDRESS(its->baser_coll_table);
> +	if (!gpa)
> +		return 0;
> +
> +	max_size = GITS_BASER_NR_PAGES(its->baser_coll_table) * SZ_64K;
> +
> +	list_for_each_entry(collection, &its->collection_list, coll_list) {
> +		if (filled == max_size)
> +			return -ENOSPC;
> +		ret = vgic_its_save_cte(its, collection, gpa, cte_esz);
> +		if (ret)
> +			return ret;
> +		gpa += cte_esz;
> +		filled += cte_esz;
> +	}
> +
> +	if (filled == max_size)
> +		return 0;
> +
> +	/*
> +	 * table is not fully filled, add a last dummy element
> +	 * with valid bit unset
> +	 */
> +	val = 0;
> +	ret = kvm_write_guest(its->dev->kvm, gpa, &val, cte_esz);
> +	return ret;
>  }
>  
>  /**
> @@ -1801,7 +1885,28 @@ static int vgic_its_save_collection_table(struct vgic_its *its)
>   */
>  static int vgic_its_restore_collection_table(struct vgic_its *its)
>  {
> -	return -ENXIO;
> +	const struct vgic_its_abi *abi = vgic_its_get_abi(its);
> +	size_t max_size, read = 0;
> +	gpa_t gpa;
> +	int ret, cte_esz = abi->cte_esz;
> +
> +	gpa = BASER_ADDRESS(its->baser_coll_table);
> +
> +	if (!gpa)
> +		return 0;

I don't understand this check.

Don't you want to do do
	if (!(its->baser_coll_table) & GITS_BASER_VALID)
		return 0;

Or what am I missing?

Thanks,
-Christoffer

> +
> +	max_size = GITS_BASER_NR_PAGES(its->baser_coll_table) * SZ_64K;
> +
> +	while (read < max_size) {
> +		bool valid;
> +
> +		ret = vgic_its_restore_cte(its, gpa, cte_esz, &valid);
> +		if (!valid || ret)
> +			break;
> +		gpa += cte_esz;
> +		read += cte_esz;
> +	}
> +	return ret;
>  }
>  
>  /**
> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
> index b87f1c6..56e57c1 100644
> --- a/virt/kvm/arm/vgic/vgic.h
> +++ b/virt/kvm/arm/vgic/vgic.h
> @@ -73,6 +73,15 @@
>  				      KVM_REG_ARM_VGIC_SYSREG_CRM_MASK | \
>  				      KVM_REG_ARM_VGIC_SYSREG_OP2_MASK)
>  
> +/*
> + * As per Documentation/virtual/kvm/devices/arm-vgic-its.txt,
> + * below macros are defined for ITS table entry encoding.
> + */
> +#define KVM_ITS_CTE_VALID_SHIFT		63
> +#define KVM_ITS_CTE_VALID_MASK		BIT_ULL(63)
> +#define KVM_ITS_CTE_RDBASE_SHIFT	16
> +#define KVM_ITS_CTE_ICID_MASK		GENMASK_ULL(15, 0)
> +
>  static inline bool irq_is_pending(struct vgic_irq *irq)
>  {
>  	if (irq->config == VGIC_CONFIG_EDGE)
> -- 
> 2.5.5
> 

^ permalink raw reply

* [PATCHv3 0/2] arm64: fix hotplug rwsem boot fallout
From: Mark Rutland @ 2017-04-28 11:01 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

These patches fix a boot issue seen on some arm64 platforms as a result of the
hotplug rwsem rework.

Thomas, would you be able to take these into the tip smp/hotplug branch?

Will has acked the arm64 part, and is happy for this to go via tip [1].

I've tested this atop of the tip smp/hotplug branch, and with the arm64
for-next/core branch merged in, which git handles automatically. In both cases,
it builds cleanly and boots fine on Juno R1.

Thanks,
Mark.

Since v1 [2]:
* Fix update_cpu_capabilities() by splitting boot/secondary

Since v2 [3]:
* Align on _cpuslocked naming
* Avoid duplicating workaround string
* Add Will's Acked-by

[1] https://lkml.kernel.org/r/20170427180104.GO1890 at arm.
[2] https://lkml.kernel.org/r/20170427154806.GA6646 at leverpostej
[3] https://lkml.kernel.org/r/1493315077-19496-3-git-send-email-mark.rutland at arm.com

Mark Rutland (1):
  arm64: cpufeature: use static_branch_enable_cpuslocked()

Sebastian Andrzej Siewior (1):
  jump_label: Provide static_key_[enable|/slow_inc]_cpuslocked()

 arch/arm64/include/asm/cpufeature.h |  3 ++-
 arch/arm64/kernel/cpu_errata.c      |  9 ++++++++-
 arch/arm64/kernel/cpufeature.c      |  5 ++++-
 include/linux/jump_label.h          |  7 +++++++
 kernel/jump_label.c                 | 10 ++++++++++
 5 files changed, 31 insertions(+), 3 deletions(-)

-- 
1.9.1

^ permalink raw reply

* [PATCHv3 1/2] jump_label: Provide static_key_[enable|/slow_inc]_cpuslocked()
From: Mark Rutland @ 2017-04-28 11:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1493377266-2205-1-git-send-email-mark.rutland@arm.com>

From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Provide static_key_[enable|slow_inc]_cpuslocked() variant that
don't take cpu_hotplug_lock().

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sebastian Siewior <bigeasy@linutronix.de>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: jbaron at akamai.com
---
 include/linux/jump_label.h |  7 +++++++
 kernel/jump_label.c        | 10 ++++++++++
 2 files changed, 17 insertions(+)

diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index d7b17d1..c80d8b1 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -164,6 +164,7 @@ extern void arch_jump_label_transform_static(struct jump_entry *entry,
 extern void jump_label_apply_nops(struct module *mod);
 extern int static_key_count(struct static_key *key);
 extern void static_key_enable(struct static_key *key);
+extern void static_key_enable_cpuslocked(struct static_key *key);
 extern void static_key_disable(struct static_key *key);
 extern void static_key_disable_cpuslocked(struct static_key *key);
 
@@ -252,6 +253,11 @@ static inline void static_key_enable(struct static_key *key)
 		static_key_slow_inc(key);
 }
 
+static inline void static_key_enable_cpuslocked(struct static_key *key)
+{
+	static_key_enable(key);
+}
+
 static inline void static_key_disable(struct static_key *key)
 {
 	int count = static_key_count(key);
@@ -429,6 +435,7 @@ struct static_key_false {
  */
 
 #define static_branch_enable(x)			static_key_enable(&(x)->key)
+#define static_branch_enable_cpuslocked(x)	static_key_enable_cpuslocked(&(x)->key)
 #define static_branch_disable(x)		static_key_disable(&(x)->key)
 #define static_branch_disable_cpuslocked(x)	static_key_disable_cpuslocked(&(x)->key)
 
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index d71124e..6343f4c 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -90,6 +90,16 @@ void static_key_enable(struct static_key *key)
 }
 EXPORT_SYMBOL_GPL(static_key_enable);
 
+void static_key_enable_cpuslocked(struct static_key *key)
+{
+	int count = static_key_count(key);
+
+	WARN_ON_ONCE(count < 0 || count > 1);
+
+	if (!count)
+		static_key_slow_inc_cpuslocked(key);
+}
+
 void static_key_disable(struct static_key *key)
 {
 	int count = static_key_count(key);
-- 
1.9.1

^ permalink raw reply related

* [PATCHv3 2/2] arm64: cpufeature: use static_branch_enable_cpuslocked()
From: Mark Rutland @ 2017-04-28 11:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1493377266-2205-1-git-send-email-mark.rutland@arm.com>

Recently, the hotplug locking was conveted to use a percpu rwsem. Unlike
the existing {get,put}_online_cpus() logic, this can't nest.
Unfortunately, in arm64's secondary boot path we can end up nesting via
static_branch_enable() in cpus_set_cap() when we detect an erratum.

This leads to a stream of messages as below, where the secondary
attempts to schedule before it has been fully onlined. As the CPU
orchestrating the onlining holds the rswem, this hangs the system.

[    0.250334] BUG: scheduling while atomic: swapper/1/0/0x00000002
[    0.250337] Modules linked in:
[    0.250346] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.11.0-rc7-next-20170424 #2
[    0.250349] Hardware name: ARM Juno development board (r1) (DT)
[    0.250353] Call trace:
[    0.250365] [<ffff000008088510>] dump_backtrace+0x0/0x238
[    0.250371] [<ffff00000808880c>] show_stack+0x14/0x20
[    0.250377] [<ffff00000839d854>] dump_stack+0x9c/0xc0
[    0.250384] [<ffff0000080e3540>] __schedule_bug+0x50/0x70
[    0.250391] [<ffff000008932ecc>] __schedule+0x52c/0x5a8
[    0.250395] [<ffff000008932f80>] schedule+0x38/0xa0
[    0.250400] [<ffff000008935e8c>] rwsem_down_read_failed+0xc4/0x108
[    0.250407] [<ffff0000080fe8e0>] __percpu_down_read+0x100/0x118
[    0.250414] [<ffff0000080c0b60>] get_online_cpus+0x70/0x78
[    0.250420] [<ffff0000081749e8>] static_key_enable+0x28/0x48
[    0.250425] [<ffff00000808de90>] update_cpu_capabilities+0x78/0xf8
[    0.250430] [<ffff00000808d14c>] update_cpu_errata_workarounds+0x1c/0x28
[    0.250435] [<ffff00000808e004>] check_local_cpu_capabilities+0xf4/0x128
[    0.250440] [<ffff00000808e894>] secondary_start_kernel+0x8c/0x118
[    0.250444] [<000000008093d1b4>] 0x8093d1b4

We call cpus_set_cap() from update_cpu_capabilities(), which is called
from the secondary boot path (where the CPU orchestrating the onlining
holds the hotplug rwsem), and in the primary boot path, where this is
not held.

This patch makes cpus_set_cap() use static_branch_enable_cpuslocked(),
and updates all the callers of update_cpu_capabilities() consistent with
the change.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reported-by: Catalin Marinas <catalin.marinas@arm.com>
Suggested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
[Mark: minor fixups]
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/include/asm/cpufeature.h | 3 ++-
 arch/arm64/kernel/cpu_errata.c      | 9 ++++++++-
 arch/arm64/kernel/cpufeature.c      | 5 ++++-
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index f31c48d..a8d1b5b 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -145,7 +145,7 @@ static inline void cpus_set_cap(unsigned int num)
 			num, ARM64_NCAPS);
 	} else {
 		__set_bit(num, cpu_hwcaps);
-		static_branch_enable(&cpu_hwcap_keys[num]);
+		static_branch_enable_cpuslocked(&cpu_hwcap_keys[num]);
 	}
 }
 
@@ -223,6 +223,7 @@ void update_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
 void check_local_cpu_capabilities(void);
 
 void update_cpu_errata_workarounds(void);
+void update_cpu_errata_workarounds_cpuslocked(void);
 void __init enable_errata_workarounds(void);
 void verify_local_cpu_errata_workarounds(void);
 
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index f6cc67e..e650cf8 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -175,11 +175,18 @@ void verify_local_cpu_errata_workarounds(void)
 		}
 }
 
-void update_cpu_errata_workarounds(void)
+void update_cpu_errata_workarounds_cpuslocked(void)
 {
 	update_cpu_capabilities(arm64_errata, "enabling workaround for");
 }
 
+void update_cpu_errata_workarounds(void)
+{
+	get_online_cpus();
+	update_cpu_errata_workarounds_cpuslocked();
+	put_online_cpus();
+}
+
 void __init enable_errata_workarounds(void)
 {
 	enable_cpu_capabilities(arm64_errata);
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index abda8e8..113c06d 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -956,6 +956,7 @@ static void __init setup_elf_hwcaps(const struct arm64_cpu_capabilities *hwcaps)
 			cap_set_elf_hwcap(hwcaps);
 }
 
+/* Should be called with CPU hotplug lock held */
 void update_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
 			    const char *info)
 {
@@ -1075,14 +1076,16 @@ void check_local_cpu_capabilities(void)
 	 * advertised capabilities.
 	 */
 	if (!sys_caps_initialised)
-		update_cpu_errata_workarounds();
+		update_cpu_errata_workarounds_cpuslocked();
 	else
 		verify_local_cpu_capabilities();
 }
 
 static void __init setup_feature_capabilities(void)
 {
+	get_online_cpus();
 	update_cpu_capabilities(arm64_features, "detected feature:");
+	put_online_cpus();
 	enable_cpu_capabilities(arm64_features);
 }
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH v5 17/22] KVM: arm64: vgic-its: Collection table save/restore
From: Auger Eric @ 2017-04-28 11:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170428104448.GB1439@lvm>

Hi Christoffer,

On 28/04/2017 12:44, Christoffer Dall wrote:
> On Fri, Apr 14, 2017 at 12:15:29PM +0200, Eric Auger wrote:
>> The save path copies the collection entries into guest RAM
>> at the GPA specified in the BASER register. This obviously
>> requires the BASER to be set. The last written element is a
>> dummy collection table entry.
>>
>> We do not index by collection ID as the collection entry
>> can fit into 8 bytes while containing the collection ID.
>>
>> On restore path we re-allocate the collection objects.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>> v4 -> v5:
>> - add macros for field encoding/decoding
>> - use abi->cte_esz
>> - rename flush into save
>> - check the target_addr is valid
>>
>> v3 -> v4:
>> - replaced u64 *ptr by gpa_t gpa
>> - check the collection does not exist before allocating it
>>
>> v1 -> v2:
>> - reword commit message and directly use 8 as entry size
>> - no kvm parameter anymore
>> - add helper for flush/restore cte
>> - table size computed here
>> - add le64/cpu conversions
>> ---
>>  virt/kvm/arm/vgic/vgic-its.c | 109 ++++++++++++++++++++++++++++++++++++++++++-
>>  virt/kvm/arm/vgic/vgic.h     |   9 ++++
>>  2 files changed, 116 insertions(+), 2 deletions(-)
>>
>> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
>> index c22b35d..484e541 100644
>> --- a/virt/kvm/arm/vgic/vgic-its.c
>> +++ b/virt/kvm/arm/vgic/vgic-its.c
>> @@ -1785,13 +1785,97 @@ static int vgic_its_restore_device_tables(struct vgic_its *its)
>>  	return -ENXIO;
>>  }
>>  
>> +static int vgic_its_save_cte(struct vgic_its *its,
>> +			     struct its_collection *collection,
>> +			     gpa_t gpa, int esz)
>> +{
>> +	u64 val;
>> +	int ret;
>> +
>> +	val = (1ULL << KVM_ITS_CTE_VALID_SHIFT |
>> +	       ((u64)collection->target_addr << KVM_ITS_CTE_RDBASE_SHIFT) |
>> +	       collection->collection_id);
>> +	val = cpu_to_le64(val);
>> +	ret = kvm_write_guest(its->dev->kvm, gpa, &val, esz);
>> +	return ret;
>> +}
>> +
>> +static int vgic_its_restore_cte(struct vgic_its *its, gpa_t gpa,
>> +				int esz, bool *valid)
>> +{
>> +	struct its_collection *collection;
>> +	struct kvm *kvm = its->dev->kvm;
>> +	u32 target_addr;
>> +	u32 coll_id;
>> +	u64 val;
>> +	int ret;
>> +
>> +	*valid = false;
> 
> I don't see why you need this.
I initialized it here in case kvm_read_guest() fails
> 
>> +
>> +	ret = kvm_read_guest(kvm, gpa, &val, esz);
> 
> hmm, we better not have an esz larger than sizeof(u64) here then.
Yes this could be part if the ABI ops but is that worth the effort now?
I can add a comment somewhere to mention that trap.
> 
>> +	if (ret)
>> +		return ret;
>> +	val = le64_to_cpu(val);
>> +	*valid = val & KVM_ITS_CTE_VALID_MASK;
>> +
>> +	if (!*valid)
>> +		return 0;
> 
> I think this would be nicer with just a bool on the stack and then set
> *valid = in the end of the function.
OK
  Alternatively, you could just have
> the return value return > 0 when there are more entries.
> 
>> +
>> +	target_addr = (u32)(val >> KVM_ITS_CTE_RDBASE_SHIFT);
>> +	coll_id = val & KVM_ITS_CTE_ICID_MASK;
>> +
>> +	if (target_addr >= atomic_read(&kvm->online_vcpus))
>> +		return -EINVAL;
>> +
>> +	collection = find_collection(its, coll_id);
>> +	if (collection)
>> +		return -EEXIST;
>> +	ret = vgic_its_alloc_collection(its, &collection, coll_id);
>> +	if (ret)
>> +		return ret;
>> +	collection->target_addr = target_addr;
>> +	return 0;
>> +}
>> +
>>  /**
>>   * vgic_its_save_collection_table - Save the collection table into
>>   * guest RAM
>>   */
>>  static int vgic_its_save_collection_table(struct vgic_its *its)
>>  {
>> -	return -ENXIO;
>> +	const struct vgic_its_abi *abi = vgic_its_get_abi(its);
>> +	struct its_collection *collection;
>> +	u64 val;
>> +	gpa_t gpa;
>> +	size_t max_size, filled = 0;
>> +	int ret, cte_esz = abi->cte_esz;
>> +
>> +	gpa = BASER_ADDRESS(its->baser_coll_table);
>> +	if (!gpa)
>> +		return 0;
>> +
>> +	max_size = GITS_BASER_NR_PAGES(its->baser_coll_table) * SZ_64K;
>> +
>> +	list_for_each_entry(collection, &its->collection_list, coll_list) {
>> +		if (filled == max_size)
>> +			return -ENOSPC;
>> +		ret = vgic_its_save_cte(its, collection, gpa, cte_esz);
>> +		if (ret)
>> +			return ret;
>> +		gpa += cte_esz;
>> +		filled += cte_esz;
>> +	}
>> +
>> +	if (filled == max_size)
>> +		return 0;
>> +
>> +	/*
>> +	 * table is not fully filled, add a last dummy element
>> +	 * with valid bit unset
>> +	 */
>> +	val = 0;
>> +	ret = kvm_write_guest(its->dev->kvm, gpa, &val, cte_esz);
>> +	return ret;
>>  }
>>  
>>  /**
>> @@ -1801,7 +1885,28 @@ static int vgic_its_save_collection_table(struct vgic_its *its)
>>   */
>>  static int vgic_its_restore_collection_table(struct vgic_its *its)
>>  {
>> -	return -ENXIO;
>> +	const struct vgic_its_abi *abi = vgic_its_get_abi(its);
>> +	size_t max_size, read = 0;
>> +	gpa_t gpa;
>> +	int ret, cte_esz = abi->cte_esz;
>> +
>> +	gpa = BASER_ADDRESS(its->baser_coll_table);
>> +
>> +	if (!gpa)
>> +		return 0;
> 
> I don't understand this check.
> 
> Don't you want to do do
> 	if (!(its->baser_coll_table) & GITS_BASER_VALID)
> 		return 0;
> 
> Or what am I missing?
Yes that's correct I will fix that.

Thanks

Eric
> 
> Thanks,
> -Christoffer
> 
>> +
>> +	max_size = GITS_BASER_NR_PAGES(its->baser_coll_table) * SZ_64K;
>> +
>> +	while (read < max_size) {
>> +		bool valid;
>> +
>> +		ret = vgic_its_restore_cte(its, gpa, cte_esz, &valid);
>> +		if (!valid || ret)
>> +			break;
>> +		gpa += cte_esz;
>> +		read += cte_esz;
>> +	}
>> +	return ret;
>>  }
>>  
>>  /**
>> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
>> index b87f1c6..56e57c1 100644
>> --- a/virt/kvm/arm/vgic/vgic.h
>> +++ b/virt/kvm/arm/vgic/vgic.h
>> @@ -73,6 +73,15 @@
>>  				      KVM_REG_ARM_VGIC_SYSREG_CRM_MASK | \
>>  				      KVM_REG_ARM_VGIC_SYSREG_OP2_MASK)
>>  
>> +/*
>> + * As per Documentation/virtual/kvm/devices/arm-vgic-its.txt,
>> + * below macros are defined for ITS table entry encoding.
>> + */
>> +#define KVM_ITS_CTE_VALID_SHIFT		63
>> +#define KVM_ITS_CTE_VALID_MASK		BIT_ULL(63)
>> +#define KVM_ITS_CTE_RDBASE_SHIFT	16
>> +#define KVM_ITS_CTE_ICID_MASK		GENMASK_ULL(15, 0)
>> +
>>  static inline bool irq_is_pending(struct vgic_irq *irq)
>>  {
>>  	if (irq->config == VGIC_CONFIG_EDGE)
>> -- 
>> 2.5.5
>>
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

^ permalink raw reply

* [PATCH 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe
From: Arnd Bergmann @ 2017-04-28 11:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1493347596.29314.55.camel@mtkswgap22>

On Fri, Apr 28, 2017 at 4:46 AM, Ryder Lee <ryder.lee@mediatek.com> wrote:
> On Thu, 2017-04-27 at 21:06 +0200, Arnd Bergmann wrote:
>> On Wed, Apr 26, 2017 at 10:10 AM, Ryder Lee <ryder.lee@mediatek.com> wrote:
>> > On Tue, 2017-04-25 at 14:18 +0200, Arnd Bergmann wrote:
>> >> On Sun, Apr 23, 2017 at 10:19 AM, Ryder Lee <ryder.lee@mediatek.com> wrote:
>> Are any of the registers the same at all, e.g. for MSI handling?
>
> No, It doesn't support MSI. All I can do is using the registers that designer provide
> to me. The others are inviable for software. So I treat it as different hardware.
> Furthermore, we hope that we can put all mediatek drivers together
> regardless of in-house IP or lincense IP
>
> We have no particular IP name but just use chip name to call it. So I
> will temporarily use "mediatek,gen2v1-pcie" in patch v1.

I think using the chip name as in the first version of your patch name is
better then, in particular since the 'gen2v1' would not be an actual version
number but just say which variant got merged into mainline first.

A related question would be on how general we want the binding to be.
Your binding text starts out by describing that there are three root ports
and what their capabilities are.

If you think there might be other (existing or future) chips that use the
same binding and driver, then being a little more abstract could help
in the long run.

       Arnd

^ permalink raw reply

* [PATCH v2] clk: sunxi-ng: Fix dependency on SUNXI_CCU_GATE
From: Corentin Labbe @ 2017-04-28 12:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170428011533.GZ7065@codeaurora.org>

On Thu, Apr 27, 2017 at 06:15:33PM -0700, Stephen Boyd wrote:
> On 04/27, Corentin Labbe wrote:
> > On Thu, Apr 27, 2017 at 09:04:36AM +0200, Maxime Ripard wrote:
> > > On Wed, Apr 26, 2017 at 01:53:19PM +0200, Corentin Labbe wrote:
> > > > When CONFIG_SUNXI_CCU is set but no other SUNXI_CCU is selected i got
> > > > the following build error:
> > > > drivers/built-in.o: In function `ccu_pll_notifier_cb':
> > > > drivers/clk/sunxi-ng/ccu_common.c:71: undefined reference to `ccu_gate_helper_disable'
> > > > drivers/clk/sunxi-ng/ccu_common.c:73: undefined reference to `ccu_gate_helper_enable'
> > > > 
> > > > The problem is the function ccu_pll_notifier_cb in ccu_common.c need
> > > > some function from ccu_gate.c which is not compiled since SUNXI_CCU_GATE
> > > > is not selected.
> > > > 
> > > > This patch remove SUNXI_CCU_GATE and compile ccu_gate.c unconditionnaly
> > > > since all other combination of options select SUNXI_CCU_GATE finally.
> > > > 
> > > > Fixes: 02ae2bc6febd ("clk: sunxi-ng: Add clk notifier to gate then ungate PLL clocks")
> > > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> > > 
> > > I think Chen-Yu had some comments that you didn't address.
> > > 
> > 
> > I have changed subject as requested
> 
> There were more comments than just fixing the subject. And now
> Arnd has sent a patch. This is a problem in mainline, and the
> release is days away. I'm inclined to go with Arnd's patch unless
> someone sends something else and forward it directly to Linus.

Sorry, I havent see the rest of wens's comments.
Go for Arnd's patch.

^ permalink raw reply

* [RFC/RFT PATCH 01/18] PCI: Initialize bridge release function at bridge allocation
From: Arnd Bergmann @ 2017-04-28 12:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170426111809.19922-2-lorenzo.pieralisi@arm.com>

On Wed, Apr 26, 2017 at 1:17 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> The introduction of pci_register_host_bridge() kernel interface
> allows PCI host controller drivers to create the struct pci_host_bridge
> object, initialize it and register it with the kernel so that its
> corresponding PCI bus can be scanned and its devices probed.
>
> The host bridge device release function pci_release_host_bridge_dev is a
> static function common for all struct pci_host_bridge allocated objects,
> so in its current form cannot be used by PCI host bridge controllers
> drivers to initialize the allocated struct pci_host_bridge, which
> leaves struct pci_host_bridge devices release function uninitialized.
>
> Since pci_release_host_bridge_dev is a function common to all PCI
> host bridge objects, initialize it in pci_alloc_host_bridge() (ie
> common host bridge allocation interface) so that all struct
> pci_host_bridge objects have their release function initialized by
> default at allocation time, removing the need for exporting the
> common pci_release_host_bridge_dev function to other compilation
> units.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>

Makes sense. I think originally I did not have the bridge->release_fn
callback, so drivers would have to provide their won release callback,
but this is no longer needed as they can override the release_fn instead.

Acked-by: Arnd Bergmann <arnd@arndb.de>

^ permalink raw reply

* [RFC/RFT PATCH 02/18] PCI: Add pci_free_host_bridge interface
From: Arnd Bergmann @ 2017-04-28 12:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170426111809.19922-3-lorenzo.pieralisi@arm.com>

On Wed, Apr 26, 2017 at 1:17 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> Commit a52d1443bba1 ("PCI: Export host bridge registration interface")
> exported the pci_alloc_host_bridge() interface so that PCI host
> controllers drivers can make use of it.
>
> Introduce pci_alloc_host_bridge() kernel counterpart to free the
> host bridge data structures, pci_free_host_bridge(), export it
> and update kernel functions releasing host bridge objects allocated
> memory to make use of it.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>

Acked-by: Arnd Bergmann <arnd@arndb.de>

^ permalink raw reply

* [PATCH v3 1/9] bus: fsl-mc: add DPCON object APIs
From: Greg KH @ 2017-04-28 12:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170428095034.8842-2-ruxandra.radulescu@nxp.com>

On Fri, Apr 28, 2017 at 04:50:26AM -0500, Ioana Radulescu wrote:
> From: Ioana Radulescu <ruxandra.radulescu@freescale.com>
> 
> This patch adds the command building/parsing wrapper functions
> for the DPCON object. The binary interface version is v3.2.

The subject should have "staging: fsl-mc: bus: ..." to match the other
ones (and to catch my filters properly...)

I'll take this as-is for now.

thanks,

greg k-h

^ permalink raw reply

* [RFC/RFT PATCH 03/18] PCI: Introduce pci_scan_root_bus_bridge()
From: Arnd Bergmann @ 2017-04-28 12:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170426111809.19922-4-lorenzo.pieralisi@arm.com>

On Wed, Apr 26, 2017 at 1:17 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> Current pci_scan_root_bus() interface is made up of two main
> code paths:
>
> - pci_create_root_bus()
> - pci_scan_child_bus()
>
> pci_create_root_bus() is a wrapper function that allows to create
> a struct pci_host_bridge structure, initialize it with the passed
> parameters and register it with the kernel.
>
> As the struct pci_host_bridge require additional struct members,
> pci_create_root_bus() parameters list has grown in time, making
> it unwieldy to add further parameters to it in case the struct
> pci_host_bridge gains more members fields to augment its functionality.
>
> Since PCI core code provides functions to allocate struct
> pci_host_bridge, instead of forcing the pci_create_root_bus() interface
> to add new parameters to cater for new struct pci_host_bridge
> functionality, it is more suitable to add an interface in PCI
> core code to scan a PCI bus straight from a struct pci_host_bridge
> created and customized by each specific PCI host controller driver.
>
> Add a pci_scan_root_bus_bridge() function to allow PCI host controller
> drivers to create and initialize struct pci_host_bridge and scan
> the resulting bus.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>

Good idea, yes. To avoid growing the number of interfaces too
much, should we change the existing users of pci_register_host_bridge
in host drivers over to this entry point, and make the other one
local to probe.c then?

> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 7e4ffc4..c7a7f72 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2412,6 +2412,44 @@ void pci_bus_release_busn_res(struct pci_bus *b)
>                         res, ret ? "can not be" : "is");
>  }
>
> +int pci_scan_root_bus_bridge(struct pci_host_bridge *bridge)
> +{
> +       struct resource_entry *window;
> +       bool found = false;
> +       struct pci_bus *b;
> +       int max, bus, ret;
> +
> +       if (!bridge)
> +               return -EINVAL;
> +
> +       resource_list_for_each_entry(window, &bridge->windows)
> +               if (window->res->flags & IORESOURCE_BUS) {
> +                       found = true;
> +                       break;
> +               }
> +
> +       ret = pci_register_host_bridge(bridge);
> +       if (ret < 0)
> +               return ret;
> +
> +       b = bridge->bus;
> +       bus = bridge->busnr;
> +
> +       if (!found) {
> +               dev_info(&b->dev,
> +                "No busn resource found for root bus, will use [bus %02x-ff]\n",
> +                       bus);
> +               pci_bus_insert_busn_res(b, bus, 255);
> +       }
> +
> +       max = pci_scan_child_bus(b);
> +
> +       if (!found)
> +               pci_bus_update_busn_res_end(b, max);
> +
> +       return 0;
> +}
> +

We probably want an EXPORT_SYMBOL() here as well.

     Arnd

^ permalink raw reply

* [PATCH/RFC 0/5] arm64: dts: renesas: Break out common board support
From: Geert Uytterhoeven @ 2017-04-28 12:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170428073256.GG10196@verge.net.au>

Hi Simon,

On Fri, Apr 28, 2017 at 9:32 AM, Simon Horman <horms@verge.net.au> wrote:
> On Fri, Apr 28, 2017 at 09:11:36AM +0200, Geert Uytterhoeven wrote:
>> On Fri, Apr 28, 2017 at 9:04 AM, Simon Horman <horms@verge.net.au> wrote:
>> > On Thu, Apr 27, 2017 at 03:32:49PM +0200, Geert Uytterhoeven wrote:
>> >> On Wed, Apr 26, 2017 at 10:11 AM, Geert Uytterhoeven
>> >> <geert@linux-m68k.org> wrote:
>> >> > CC Vladimir (which I forgot to CC initially, sorry for that)
>> >> >
>> >> > On Wed, Apr 26, 2017 at 10:06 AM, Simon Horman <horms@verge.net.au> wrote:
>> >> >> On Fri, Apr 21, 2017 at 02:55:16PM +0200, Geert Uytterhoeven wrote:
>> >> >>> The Renesas Salvator-X and ULCB development board can be equipped with
>> >> >>> either an R-Car H3 or M3-W SiP, which are pin-compatible.  All boards
>> >> >>> use separate DTBs, but currently there's no sharing of board-specific
>> >> >>> devices in DTS.
>> >> >>>
>> >> >>> This series reduces duplication by extracting common board support into
>> >> >>> their own .dtsi files.  As the level of support varies across boards and
>> >> >>> SoCs, this requires the addition of a few external clocks and
>> >> >>> placeholder devices on R-Car M3-W, so the common board support DTS can
>> >> >>> refer to them.
>> >> >>>
>> >> >>>   - Patches 1 and 2 add the external audio and PCIe bus clocks on R-Car
>> >> >>>     M3-W, which are present in r8a7795.dtsi, and used in
>> >> >>>     r8a7795-salvator-x.dts,
>> >> >>>   - RFC patch 3 adds placeholders for devices that are not yet supported
>> >> >>>     and/or tested on R-Car M3-W, but used on R-Car H3,
>> >> >>>   - RFC patch 4 extracts common Salvator-X board support,
>> >> >>>   - RFC patch 5 extracts common ULCB board support.
>> >> >>>
>> >> >>> For R-Car H3 based boards, there are no functional changes.
>> >> >>> For R-Car M3-W based boards, some new devices are now described in DT.
>> >> >>>
>> >> >>> Dependencies:
>> >> >>>   - renesas-devel-20170420-v4.11-rc7,
>> >> >>>   - Patches 1 and 2 can be applied as-is,
>> >> >>>   - Patches 4 and 5 depend on "[PATCH 0/8] arm64: dts: renesas: Break
>> >> >>>     out R-Car H3 and M3-W SiP"
>> >> >>>     (http://www.spinics.net/lists/devicetree/msg173820.html).
>>            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>
>> >> >>> DTB changes have been inspected using scripts/dtc/dtx_diff.
>> >> >>> This has been tested on Salvator-X (both H3 and M3-W).
>> >> >>> This has not been tested on H3ULCB and M3ULCB due to lack of hardware.
>> >> >>>
>> >> >>> Thanks for your comments!
>> >> >>
>> >> >> Thanks for tackling this important problem. I have looked over the changes
>> >> >> and they seem nice to me. I would, however, be more comfortable applying
>> >> >> them if they were rested on the ULCB boards.
>> >> >
>> >> > tested?
>> >> >
>> >> > I've pushed a branch for testing to topic/rcar3-dtsi-sharing in
>> >> > git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git.
>> >>
>> >> I managed to test it on the new H3ULCB and M3ULCB baords in Magnus' farm.
>> >> No issues detected.
>> >
>> > Great! Any objections to me queuing this up?
>>
>> The dependency above (no feedback about the SiP types yet).
>>
>> I can respin without that dependency, if that is preferred...
>
> It seems to me that it would be nice to get these in sooner than later - in
> particular earlier rather than later in the (v4.13) development cycle. But
> I defer to your judgement on what is best.

Agreed.  Stay tuned for v2...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH v3 1/9] bus: fsl-mc: add DPCON object APIs
From: Ruxandra Ioana Radulescu @ 2017-04-28 12:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170428122332.GB18081@kroah.com>



> -----Original Message-----
> From: Greg KH [mailto:gregkh at linuxfoundation.org]
> Sent: Friday, April 28, 2017 3:24 PM
> To: Ruxandra Ioana Radulescu <ruxandra.radulescu@nxp.com>
> Cc: devel at driverdev.osuosl.org; stuyoder at gmail.com; arnd at arndb.de;
> Haiying Wang <haiying.wang@nxp.com>; Roy Pledge
> <roy.pledge@nxp.com>; Alexandru Marginean
> <alexandru.marginean@nxp.com>; linux-kernel at vger.kernel.org;
> agraf at suse.de; Bogdan Hamciuc <bogdan.hamciuc@nxp.com>; Bogdan
> Purcareata <bogdan.purcareata@nxp.com>; linux-arm-
> kernel at lists.infradead.org; Laurentiu Tudor <laurentiu.tudor@nxp.com>
> Subject: Re: [PATCH v3 1/9] bus: fsl-mc: add DPCON object APIs
> 
> On Fri, Apr 28, 2017 at 04:50:26AM -0500, Ioana Radulescu wrote:
> > From: Ioana Radulescu <ruxandra.radulescu@freescale.com>
> >
> > This patch adds the command building/parsing wrapper functions
> > for the DPCON object. The binary interface version is v3.2.
> 
> The subject should have "staging: fsl-mc: bus: ..." to match the other
> ones (and to catch my filters properly...)

My bad, I must have missed this one when I changed the subject
lines for v2.
 
> I'll take this as-is for now.
 
Thanks a lot!
Ioana

^ permalink raw reply

* [PATCH v5 1/4] printk/nmi: generic solution for safe printk in NMI
From: Petr Mladek @ 2017-04-28 12:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170428012530.GA383@jagdpanzerIV.localdomain>

On Fri 2017-04-28 10:25:30, Sergey Senozhatsky wrote:
> 
> On (04/20/17 15:11), Petr Mladek wrote:
> [..]
> >  void printk_nmi_enter(void)
> >  {
> > -	this_cpu_or(printk_context, PRINTK_NMI_CONTEXT_MASK);
> > +	/*
> > +	 * The size of the extra per-CPU buffer is limited. Use it
> > +	 * only when really needed.
> > +	 */
> > +	if (this_cpu_read(printk_context) & PRINTK_SAFE_CONTEXT_MASK ||
> > +	    raw_spin_is_locked(&logbuf_lock)) {
> 
> can we please have && here?

OK, it sounds reasonable after all.

> [..]
> > diff --git a/lib/nmi_backtrace.c b/lib/nmi_backtrace.c
> > index 4e8a30d1c22f..0bc0a3535a8a 100644
> > --- a/lib/nmi_backtrace.c
> > +++ b/lib/nmi_backtrace.c
> > @@ -86,9 +86,11 @@ void nmi_trigger_cpumask_backtrace(const cpumask_t *mask,
> >  
> >  bool nmi_cpu_backtrace(struct pt_regs *regs)
> >  {
> > +	static arch_spinlock_t lock = __ARCH_SPIN_LOCK_UNLOCKED;
> >  	int cpu = smp_processor_id();
> >  
> >  	if (cpumask_test_cpu(cpu, to_cpumask(backtrace_mask))) {
> > +		arch_spin_lock(&lock);
> >  		if (regs && cpu_in_idle(instruction_pointer(regs))) {
> >  			pr_warn("NMI backtrace for cpu %d skipped: idling at pc %#lx\n",
> >  				cpu, instruction_pointer(regs));
> > @@ -99,6 +101,7 @@ bool nmi_cpu_backtrace(struct pt_regs *regs)
> >  			else
> >  				dump_stack();
> >  		}
> > +		arch_spin_unlock(&lock);
> >  		cpumask_clear_cpu(cpu, to_cpumask(backtrace_mask));
> >  		return true;
> >  	}
> 
> can the nmi_backtrace part be a patch on its own?

I would prefer to keep it in the same patch. The backtrace from
all CPUs is completely unusable when all CPUs push to the global
log buffer in parallel. Single patch might safe hair of some
poor bisectors.

Best Regards,
Petr

^ permalink raw reply

* [RFC/RFT PATCH 05/18] ARM: PCI: dove: Convert PCI scan API to pci_scan_root_bus_bridge()
From: Arnd Bergmann @ 2017-04-28 12:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170426111809.19922-6-lorenzo.pieralisi@arm.com>

On Wed, Apr 26, 2017 at 1:17 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> The introduction of pci_scan_root_bus_bridge() provides a PCI core
> API to scan a PCI root bus backed by an already initialized
> struct pci_host_bridge object, which simplifies the bus scan
> interface and makes the PCI scan root bus interface easier to
> generalize as members are added to the struct pci_host_bridge().
>
> Convert ARM dove platform code to pci_scan_root_bus_bridge() to improve
> the PCI root bus scanning interface.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

Is this patch required for one of the later steps in the series?

As non-DT dove uses the traditional pci_common_init() helper rather
than registering its own driver, I wonder if there is anything to gain here.

        Arnd

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox