* Re: [PATCH v2] PCI: aardvark: Wait for endpoint to be ready before training link
From: Remi Pommarel @ 2019-08-06 18:49 UTC (permalink / raw)
To: Thomas Petazzoni, Lorenzo Pieralisi, Bjorn Helgaas
Cc: linux-pci, Ellie Reeves, linux-kernel, linux-arm-kernel
In-Reply-To: <20190522213351.21366-2-repk@triplefau.lt>
On Wed, May 22, 2019 at 11:33:50PM +0200, Remi Pommarel wrote:
> When configuring pcie reset pin from gpio (e.g. initially set by
> u-boot) to pcie function this pin goes low for a brief moment
> asserting the PERST# signal. Thus connected device enters fundamental
> reset process and link configuration can only begin after a minimal
> 100ms delay (see [1]).
>
> Because the pin configuration comes from the "default" pinctrl it is
> implicitly configured before the probe callback is called:
>
> driver_probe_device()
> really_probe()
> ...
> pinctrl_bind_pins() /* Here pin goes from gpio to PCIE reset
> function and PERST# is asserted */
> ...
> drv->probe()
>
> [1] "PCI Express Base Specification", REV. 4.0
> PCI Express, February 19 2014, 6.6.1 Conventional Reset
>
> Signed-off-by: Remi Pommarel <repk@triplefau.lt>
> ---
> Changes since v1:
> - Add a comment about pinctrl implicit pin configuration
> - Use more legible msleep
> - Use PCI_PM_D3COLD_WAIT macro
>
> Please note that I will unlikely be able to answer any comments from May
> 24th to June 10th.
> ---
> drivers/pci/controller/pci-aardvark.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> index 134e0306ff00..d998c2b9cd04 100644
> --- a/drivers/pci/controller/pci-aardvark.c
> +++ b/drivers/pci/controller/pci-aardvark.c
> @@ -324,6 +324,14 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)
> reg |= PIO_CTRL_ADDR_WIN_DISABLE;
> advk_writel(pcie, reg, PIO_CTRL);
>
> + /*
> + * PERST# signal could have been asserted by pinctrl subsystem before
> + * probe() callback has been called, making the endpoint going into
> + * fundamental reset. As required by PCI Express spec a delay for at
> + * least 100ms after such a reset before link training is needed.
> + */
> + msleep(PCI_PM_D3COLD_WAIT);
> +
> /* Start link training */
> reg = advk_readl(pcie, PCIE_CORE_LINK_CTRL_STAT_REG);
> reg |= PCIE_CORE_LINK_TRAINING;
> --
> 2.20.1
Gentle ping.
--
Remi
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 5/6] tty: serial: Add linflexuart driver for S32V234
From: gregkh @ 2019-08-06 18:40 UTC (permalink / raw)
To: Stefan-gabriel Mirea
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, corbet@lwn.net,
catalin.marinas@arm.com, linux-doc@vger.kernel.org,
Larisa Ileana Grigore, linux-kernel@vger.kernel.org, Leo Li,
Cosmin Stefan Stoica, robh+dt@kernel.org,
linux-serial@vger.kernel.org, jslaby@suse.com,
shawnguo@kernel.org, will@kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <HE1PR0402MB28579034C09EB49A76A4F8E7DFD50@HE1PR0402MB2857.eurprd04.prod.outlook.com>
On Tue, Aug 06, 2019 at 05:11:17PM +0000, Stefan-gabriel Mirea wrote:
> On 8/5/2019 6:31 PM, gregkh@linuxfoundation.org wrote:
> > On Fri, Aug 02, 2019 at 07:47:23PM +0000, Stefan-gabriel Mirea wrote:
> >>
> >> +/* Freescale Linflex UART */
> >> +#define PORT_LINFLEXUART 121
> >
> > Do you really need this modified?
>
> Hello Greg,
>
> This macro is meant to be assigned to port->type in the config_port
> method from uart_ops, in order for verify_port to know if the received
> serial_struct structure was really targeted for a LINFlex port. It
> needs to be defined outside, to avoid "collisions" with other drivers.
Yes, I know what it goes to, but does anyone in userspace actually use
it?
> As far as I see, uart_set_info() will actually fail at the
> "baud_base < 9600" check[1], right after calling verify_port(), when
> performing an ioctl() on "/dev/console" with TIOCSSERIAL using a
> serial_struct obtained with TIOCGSERIAL. This happens because this
> reduced version of the LINFlex UART driver will not touch the uartclk
> field of the uart_port (as there is currently no clock support).
> Therefore, the linflex_config/verify_port() functions, along with the
> PORT_LINFLEXUART macro, may be indeed unnecessary at this point (and
> should be added later). Is this what you mean?
No, see below.
> Other than that, I do not see anything wrong with the addition of a
> define in serial_core.h for this purpose (which is also what most of the
> serial drivers do, including amba-pl011.c, mentioned in
> Documentation/driver-api/serial/driver.rst as providing the reference
> implementation), so please be more specific.
I am getting tired of dealing with merge issues with that list, and no
one seems to be able to find where they are really needed for userspace,
especially for new devices. What happens if you do not have use it?
thanks,
greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] arm64: Disable big endian builds with clang
From: Mark Brown @ 2019-08-06 18:39 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, Nick Desaulniers, Nathan Chancellor,
Tri Vo
Cc: clang-built-linux, Mark Brown, linux-arm-kernel
Current boot tests with clang built big endian kernels in KernelCI are
showing problems with the kernel being unable to interpret big endian
userspace. This is a bug and should be fixed but for now let's prevent
these kernels being built, we may end up needing to add a version
dependency on the compiler anyway.
Signed-off-by: Mark Brown <broonie@kernel.org>
---
The clang people (CCed) are aware and looking into this.
arch/arm64/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 990fdcbf05c7..1c32d9889e0f 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -813,6 +813,7 @@ config ARM64_PA_BITS
config CPU_BIG_ENDIAN
bool "Build big-endian kernel"
+ depends on !CC_IS_CLANG
help
Say Y if you plan on running a kernel in big-endian mode.
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [linux-sunxi] [PATCH 0/3] Add basic support for RTC on Allwinner H6 SoC
From: Ondřej Jirman @ 2019-08-06 18:30 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Mark Rutland, Alessandro Zummo, Alexandre Belloni, devicetree,
Maxime Ripard, linux-kernel, linux-sunxi, Rob Herring,
linux-arm-kernel, linux-rtc
In-Reply-To: <CAGb2v66cbpsoHJoiFJkBwhZ5SbO+uO+Kf6gtnA3kPFQZq0329Q@mail.gmail.com>
On Mon, Apr 15, 2019 at 04:18:12PM +0800, Chen-Yu Tsai wrote:
> On Fri, Apr 12, 2019 at 8:07 PM megous via linux-sunxi
> <linux-sunxi@googlegroups.com> wrote:
> >
> > From: Ondrej Jirman <megous@megous.com>
> >
> > I went through the datasheets for H6 and H5, and compared the differences.
> > RTCs are largely similar, but not entirely compatible. Incompatibilities
> > are in details not yet implemented by the rtc driver though.
> >
> > I also corrected the clock tree in H6 DTSI.
>
> Please also add DCXO clock input/output and XO clock input to the bindings
> and DT, and also fix up the clock tree. You can skip them in the driver for
> now, but please add a TODO. As long as you don't change the clock-output-name
> of osc24M, everything should work as before.
>
> We just want the DT to describe what is actually there. For the XO input,
> you could just directly reference the external crystal node. The gate for
> it is likely somewhere in the PRCM block, which we don't have docs for.
So I was thinking about this for a while, and came up with this:
----------------- arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi -----------------
index 64c39f663d22..ac99ddbebe5c 100644
@@ -627,14 +635,15 @@
rtc: rtc@7000000 {
compatible = "allwinner,sun50i-h6-rtc";
reg = <0x07000000 0x400>;
interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>;
- clock-output-names = "osc32k", "osc32k-out", "iosc";
- clocks = <&ext_osc32k>;
+ clock-output-names = "osc32k", "osc32k-out", "iosc", "hosc";
+ clock-names = "losc", "dcxo";
+ clocks = <&ext_osc32k>, <&osc24M>;
#clock-cells = <1>;
};
r_ccu: clock@7010000 {
compatible = "allwinner,sun50i-h6-r-ccu";
reg = <0x07010000 0x400>;
I'm not completely sure how (or why?) to describe in DTSI which oscillator the
designer used (XO vs DCXO). This information is signalled by the pad voltage and
can be determined at runtime from DCXO_CTRL_REG's OSC_CLK_SRC_SEL (bit 3). It's
not possible to change at runtime.
HOSC source selection is only material to the CPUS (ARISC) firmware when it
wants to turn off all PLLs and the main crystal oscillator so that it knows
which one to turn off. I don't see any other use for it. It's just
informational. I don't think (future) crust firmware has space to be reading
DTBs, so the detection will be using OSC_CLK_SRC_SEL anyway.
Maybe whether XO or DCXO is used also matters if you want to do some fine
tunning of DCXO (control register has pletny of options), but that's probably
better done in u-boot. And there's still no need to read HOSC source from DT.
The driver can just check compatible, and if it is H6 and OSC_CLK_SRC_SEL is 1,
it can do it's DCXO tunning, or whatever. But neither OS nor bootloader will
be using this info to gate/disable the osciallator.
If we really want this in DT, maybe we can model it by having just two input
clocks to RTC described in DTSI, and the DTSI for H6 would have this by default:
clock-names = "losc", "dcxo";
clocks = <&ext_osc32k>, <&osc24M>;
And the board designer could change it from a board file, like this:
&rtc {
clock-names = "losc", "xo";
clocks = <&ext_osc32k>, <&osc24M>;
};
The driver could decide which oscillator is used by the presence of either
dcxo or xo input clock.
But in any case, the driver can also get this info from DCXO_CTRL_REG's
OSC_CLK_SRC_SEL, so it doesn't need to read this from DT at all. So it's a bit
pointless.
So I see two options:
1) skip adding dcxo/xo to input clocks of RTC completely
2) the above
What do you think?
regards,
o.
> > There's a small detail here, that's not described absolutely correctly in
> > DTSI, but the difference is not really that material. ext_osc32k is
> > originally modelled as a fixed clock that feeds into RTC module, but in
> > reality it's the RTC module that implements via its registers enabling and
> > disabling of this oscillator/clock.
> >
> > Though:
> > - there's no other possible user of ext_osc32k than RTC module
> > - there's no other possible external configuration for the crystal
> > circuit that would need to be handled in the dts per board
> >
> > So I guess, while the description is not perfect, this patch series still
> > improves the current situation. Or maybe I'm misunderstanding something,
> > and &ext_osc32k node just describes a fact that there's a crystal on
> > the board. Then, everything is perhaps fine. :)
>
> Correct. The external clock nodes are modeling the crystal, not the internal
> clock gate / distributor.
>
> Were the vendor to not include the crystal (for whatever reasons), the DT
> should be able to describe it via the absence of the clock input, and the
> driver should correctly use the internal (inaccurate) oscillator. I realize
> the clocks property is required, and the driver doesn't handle this case
> either, so we might have to fix that if it were to appear in the wild.
>
> > For now, the enable bit for this oscillator is toggled by the re-parenting
> > code automatically, as needed.
>
> That's fine. No need to increase the clock tree depth.
>
> ChenYu
>
> > This patchset is necessary for implementing the WiFi/Bluetooth support
> > on boards using H6 SoC.
> >
> > Please take a look.
> >
> > Thank you and regards,
> > Ondrej Jirman
> >
> > Ondrej Jirman (3):
> > dt-bindings: Add compatible for H6 RTC
> > rtc: sun6i: Add support for H6 RTC
> > arm64: dts: sun50i-h6: Add support for RTC and fix the clock tree
> >
> > .../devicetree/bindings/rtc/sun6i-rtc.txt | 1 +
> > arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 30 +++++++-------
> > drivers/rtc/rtc-sun6i.c | 40 ++++++++++++++++++-
> > 3 files changed, 55 insertions(+), 16 deletions(-)
> >
> > --
> > 2.21.0
> >
> > --
> > You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> > For more options, visit https://groups.google.com/d/optout.
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH net] net: ethernet: sun4i-emac: Support phy-handle property for finding PHYs
From: David Miller @ 2019-08-06 18:29 UTC (permalink / raw)
To: wens; +Cc: netdev, wens, linux-arm-kernel, mripard, linux-kernel
In-Reply-To: <20190806073539.32519-1-wens@kernel.org>
From: Chen-Yu Tsai <wens@kernel.org>
Date: Tue, 6 Aug 2019 15:35:39 +0800
> From: Chen-Yu Tsai <wens@csie.org>
>
> The sun4i-emac uses the "phy" property to find the PHY it's supposed to
> use. This property was deprecated in favor of "phy-handle" in commit
> 8c5b09447625 ("dt-bindings: net: sun4i-emac: Convert the binding to a
> schemas").
>
> Add support for this new property name, and fall back to the old one in
> case the device tree hasn't been updated.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Applied.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 0/6] hwspinlock: allow sharing of hwspinlocks
From: Bjorn Andersson @ 2019-08-06 18:21 UTC (permalink / raw)
To: Suman Anna
Cc: Ohad Ben-Cohen, Mark Rutland, Alexandre TORGUE, Jonathan Corbet,
linux-doc@vger.kernel.org, linux-remoteproc@vger.kernel.org,
linux-kernel@vger.kernel.org, Fabien DESSENNE,
devicetree@vger.kernel.org, Rob Herring, Maxime Coquelin,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, Benjamin GAIGNARD
In-Reply-To: <dcd1aeea-cffe-d5fb-af5a-e52efcc2e046@ti.com>
On Tue 06 Aug 10:38 PDT 2019, Suman Anna wrote:
> Hi Fabien,
>
> On 8/5/19 12:46 PM, Bjorn Andersson wrote:
> > On Mon 05 Aug 01:48 PDT 2019, Fabien DESSENNE wrote:
> >
> >>
> >> On 01/08/2019 9:14 PM, Bjorn Andersson wrote:
> >>> On Wed 13 Mar 08:50 PDT 2019, Fabien Dessenne wrote:
[..]
> >> B/ This would introduce some inconsistency between the two 'request' API
> >> which are hwspin_lock_request() and hwspin_lock_request_specific().
> >> hwspin_lock_request() looks for an unused lock, so requests for an exclusive
> >> usage. On the other side, request_specific() would request shared locks.
> >> Worst the following sequence can transform an exclusive usage into a shared
> >>
> >
> > There is already an inconsistency in between these; as with above any
> > system that uses both request() and request_specific() will be suffering
> > from intermittent failures due to probe ordering.
> >
> >> one:
> >> -hwspin_lock_request() -> returns Id#0 (exclusive)
> >> -hwspin_lock_request() -> returns Id#1 (exclusive)
> >> -hwspin_lock_request_specific(0) -> returns Id#0 and makes Id#0 shared
> >> Honestly I am not sure that this is a real issue, but it's better to have it
> >> in mind before we take ay decision
>
> Wouldn't it be actually simpler to just introduce a new specific API
> variant for this, similar to the reset core for example (it uses a
> separate exclusive API), without having to modify the bindings at all.
> It is just a case of your driver using the right API, and the core can
> be modified to use the additional tag semantics based on the API. It
> should avoid any confusion with say using a different second cell value
> for the same lock in two different nodes.
>
But this implies that there is an actual need to hold these locks
exclusively. Given that they are (except for the raw case) all wrapped
by Linux locking primitives there shouldn't be a problem sharing a lock
(except possibly for the raw case).
I agree that we shouldn't specify this property in DT - if anything it
should be a variant of the API.
> If you are sharing a hwlock on the Linux side, surely your driver should
> be aware that it is a shared lock. The tag can be set during the first
> request API, and you look through both tags when giving out a handle.
>
Why would the driver need to know about it?
> Obviously, the hwspin_lock_request() API usage semantics always had the
> implied additional need for communicating the lock id to the other peer
> entity, so a realistic usage is most always the specific API variant. I
> doubt this API would be of much use for the shared driver usage. This
> also implies that the client user does not care about specifying a lock
> in DT.
>
Afaict if the lock are shared then there shouldn't be a problem with
some clients using the request API and others request_specific(). As any
collisions would simply mean that there are more contention on the lock.
With the current exclusive model that is not possible and the success of
the request_specific will depend on probe order.
But perhaps it should be explicitly prohibited to use both APIs on the
same hwspinlock instance?
Regards,
Bjorn
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 3/8] of/fdt: add function to get the SoC wide DMA addressable memory size
From: Nicolas Saenz Julienne @ 2019-08-06 18:12 UTC (permalink / raw)
To: Rob Herring
Cc: phill, devicetree,
moderated list:BROADCOM BCM2835 ARM ARCHITECTURE,
Florian Fainelli, Will Deacon, Eric Anholt, Marc Zyngier,
Catalin Marinas, Frank Rowand, linux-kernel@vger.kernel.org,
linux-mm, Linux IOMMU, Matthias Brugger, wahrenst, Andrew Morton,
Robin Murphy, Christoph Hellwig,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Marek Szyprowski
In-Reply-To: <CAL_Jsq+LjsRmFg-xaLgpVx3miXN3hid3aD+mgTW__j0SbEFYjQ@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 5141 bytes --]
Hi Rob,
On Mon, 2019-08-05 at 13:23 -0600, Rob Herring wrote:
> On Mon, Aug 5, 2019 at 10:03 AM Nicolas Saenz Julienne
> <nsaenzjulienne@suse.de> wrote:
> > Hi Rob,
> > Thanks for the review!
> >
> > On Fri, 2019-08-02 at 11:17 -0600, Rob Herring wrote:
> > > On Wed, Jul 31, 2019 at 9:48 AM Nicolas Saenz Julienne
> > > <nsaenzjulienne@suse.de> wrote:
> > > > Some SoCs might have multiple interconnects each with their own DMA
> > > > addressing limitations. This function parses the 'dma-ranges' on each of
> > > > them and tries to guess the maximum SoC wide DMA addressable memory
> > > > size.
> > > >
> > > > This is specially useful for arch code in order to properly setup CMA
> > > > and memory zones.
> > >
> > > We already have a way to setup CMA in reserved-memory, so why is this
> > > needed for that?
> >
> > Correct me if I'm wrong but I got the feeling you got the point of the patch
> > later on.
>
> No, for CMA I don't. Can't we already pass a size and location for CMA
> region under /reserved-memory. The only advantage here is perhaps the
> CMA range could be anywhere in the DMA zone vs. a fixed location.
Now I get it, sorry I wasn't aware of that interface.
Still, I'm not convinced it matches RPi's use case as this would hard-code
CMA's size. Most people won't care, but for the ones that do, it's nicer to
change the value from the kernel command line than editing the dtb. I get that
if you need to, for example, reserve some memory for the video to work, it's
silly not to hard-code it. Yet due to the board's nature and users base I say
it's important to favor flexibility. It would also break compatibility with
earlier versions of the board and diverge from the downstream kernel behaviour.
Which is a bigger issue than it seems as most users don't always understand
which kernel they are running and unknowingly copy configuration options from
forums.
As I also need to know the DMA addressing limitations to properly configure
memory zones and dma-direct. Setting up the proper CMA constraints during the
arch's init will be trivial anyway.
> > > IMO, I'd just do:
> > >
> > > if (of_fdt_machine_is_compatible(blob, "brcm,bcm2711"))
> > > dma_zone_size = XX;
> > >
> > > 2 lines of code is much easier to maintain than 10s of incomplete code
> > > and is clearer who needs this. Maybe if we have dozens of SoCs with
> > > this problem we should start parsing dma-ranges.
> >
> > FYI that's what arm32 is doing at the moment and was my first instinct. But
> > it
> > seems that arm64 has been able to survive so far without any machine
> > specific
> > code and I have the feeling Catalin and Will will not be happy about this
> > solution. Am I wrong?
>
> No doubt. I'm fine if the 2 lines live in drivers/of/.
>
> Note that I'm trying to reduce the number of early_init_dt_scan_*
> calls from arch code into the DT code so there's more commonality
> across architectures in the early DT scans. So ideally, this can all
> be handled under early_init_dt_scan() call.
How does this look? (I'll split it in two patches and add a comment explaining
why dt_dma_zone_size is needed)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index f2444c61a136..1395be40b722 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -30,6 +30,8 @@
#include "of_private.h"
+u64 dt_dma_zone_size __ro_after_init;
+
/*
* of_fdt_limit_memory - limit the number of regions in the /memory node
* @limit: maximum entries
@@ -802,6 +805,11 @@ const char * __init of_flat_dt_get_machine_name(void)
return name;
}
+static const int __init of_fdt_machine_is_compatible(char *name)
+{
+ return of_compat_cmp(of_flat_dt_get_machine_name(), name, strlen(name));
+}
+
/**
* of_flat_dt_match_machine - Iterate match tables to find matching machine.
*
@@ -1260,6 +1268,14 @@ void __init early_init_dt_scan_nodes(void)
of_scan_flat_dt(early_init_dt_scan_memory, NULL);
}
+void __init early_init_dt_get_dma_zone_size(void)
+{
+ dt_dma_zone_size = 0;
+
+ if (of_fdt_machine_is_compatible("brcm,bcm2711"))
+ dt_dma_zone_size = 0x3c000000;
+}
+
bool __init early_init_dt_scan(void *params)
{
bool status;
@@ -1269,6 +1285,7 @@ bool __init early_init_dt_scan(void *params)
return false;
early_init_dt_scan_nodes();
+ early_init_dt_get_dma_zone_size();
return true;
}
diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
index 2ad36b7bd4fa..b5a9f685de14 100644
--- a/include/linux/of_fdt.h
+++ b/include/linux/of_fdt.h
@@ -27,6 +27,8 @@ extern void *of_fdt_unflatten_tree(const unsigned long *blob,
struct device_node *dad,
struct device_node **mynodes);
+extern u64 dt_dma_zone_size __ro_after_init;
+
/* TBD: Temporary export of fdt globals - remove when code fully merged */
extern int __initdata dt_root_addr_cells;
extern int __initdata dt_root_size_cells;
Regards,
Nicolas
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v2 01/34] mm/gup: add make_dirty arg to put_user_pages_dirty_lock()
From: Ira Weiny @ 2019-08-06 17:39 UTC (permalink / raw)
To: john.hubbard
Cc: linux-fbdev, Jan Kara, kvm, Dave Hansen, Dave Chinner, dri-devel,
linux-mm, Matthew Wilcox, sparclinux, Dan Williams, devel,
rds-devel, linux-rdma, x86, amd-gfx, Christoph Hellwig,
Christoph Hellwig, Jason Gunthorpe, xen-devel, devel, linux-media,
John Hubbard, intel-gfx, linux-block, Jérôme Glisse,
linux-rpi-kernel, ceph-devel, linux-arm-kernel, linux-nfs, netdev,
LKML, linux-xfs, linux-crypto, linux-fsdevel, Andrew Morton
In-Reply-To: <20190804224915.28669-2-jhubbard@nvidia.com>
On Sun, Aug 04, 2019 at 03:48:42PM -0700, john.hubbard@gmail.com wrote:
> From: John Hubbard <jhubbard@nvidia.com>
>
> Provide a more capable variation of put_user_pages_dirty_lock(),
> and delete put_user_pages_dirty(). This is based on the
> following:
>
> 1. Lots of call sites become simpler if a bool is passed
> into put_user_page*(), instead of making the call site
> choose which put_user_page*() variant to call.
>
> 2. Christoph Hellwig's observation that set_page_dirty_lock()
> is usually correct, and set_page_dirty() is usually a
> bug, or at least questionable, within a put_user_page*()
> calling chain.
>
> This leads to the following API choices:
>
> * put_user_pages_dirty_lock(page, npages, make_dirty)
>
> * There is no put_user_pages_dirty(). You have to
> hand code that, in the rare case that it's
> required.
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Ira Weiny <ira.weiny@intel.com>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
> drivers/infiniband/core/umem.c | 5 +-
> drivers/infiniband/hw/hfi1/user_pages.c | 5 +-
> drivers/infiniband/hw/qib/qib_user_pages.c | 13 +--
> drivers/infiniband/hw/usnic/usnic_uiom.c | 5 +-
> drivers/infiniband/sw/siw/siw_mem.c | 19 +---
> include/linux/mm.h | 5 +-
> mm/gup.c | 115 +++++++++------------
> 7 files changed, 61 insertions(+), 106 deletions(-)
>
> diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
> index 08da840ed7ee..965cf9dea71a 100644
> --- a/drivers/infiniband/core/umem.c
> +++ b/drivers/infiniband/core/umem.c
> @@ -54,10 +54,7 @@ static void __ib_umem_release(struct ib_device *dev, struct ib_umem *umem, int d
>
> for_each_sg_page(umem->sg_head.sgl, &sg_iter, umem->sg_nents, 0) {
> page = sg_page_iter_page(&sg_iter);
> - if (umem->writable && dirty)
> - put_user_pages_dirty_lock(&page, 1);
> - else
> - put_user_page(page);
> + put_user_pages_dirty_lock(&page, 1, umem->writable && dirty);
> }
>
> sg_free_table(&umem->sg_head);
> diff --git a/drivers/infiniband/hw/hfi1/user_pages.c b/drivers/infiniband/hw/hfi1/user_pages.c
> index b89a9b9aef7a..469acb961fbd 100644
> --- a/drivers/infiniband/hw/hfi1/user_pages.c
> +++ b/drivers/infiniband/hw/hfi1/user_pages.c
> @@ -118,10 +118,7 @@ int hfi1_acquire_user_pages(struct mm_struct *mm, unsigned long vaddr, size_t np
> void hfi1_release_user_pages(struct mm_struct *mm, struct page **p,
> size_t npages, bool dirty)
> {
> - if (dirty)
> - put_user_pages_dirty_lock(p, npages);
> - else
> - put_user_pages(p, npages);
> + put_user_pages_dirty_lock(p, npages, dirty);
>
> if (mm) { /* during close after signal, mm can be NULL */
> atomic64_sub(npages, &mm->pinned_vm);
> diff --git a/drivers/infiniband/hw/qib/qib_user_pages.c b/drivers/infiniband/hw/qib/qib_user_pages.c
> index bfbfbb7e0ff4..26c1fb8d45cc 100644
> --- a/drivers/infiniband/hw/qib/qib_user_pages.c
> +++ b/drivers/infiniband/hw/qib/qib_user_pages.c
> @@ -37,15 +37,6 @@
>
> #include "qib.h"
>
> -static void __qib_release_user_pages(struct page **p, size_t num_pages,
> - int dirty)
> -{
> - if (dirty)
> - put_user_pages_dirty_lock(p, num_pages);
> - else
> - put_user_pages(p, num_pages);
> -}
> -
> /**
> * qib_map_page - a safety wrapper around pci_map_page()
> *
> @@ -124,7 +115,7 @@ int qib_get_user_pages(unsigned long start_page, size_t num_pages,
>
> return 0;
> bail_release:
> - __qib_release_user_pages(p, got, 0);
> + put_user_pages_dirty_lock(p, got, false);
> bail:
> atomic64_sub(num_pages, ¤t->mm->pinned_vm);
> return ret;
> @@ -132,7 +123,7 @@ int qib_get_user_pages(unsigned long start_page, size_t num_pages,
>
> void qib_release_user_pages(struct page **p, size_t num_pages)
> {
> - __qib_release_user_pages(p, num_pages, 1);
> + put_user_pages_dirty_lock(p, num_pages, true);
>
> /* during close after signal, mm can be NULL */
> if (current->mm)
> diff --git a/drivers/infiniband/hw/usnic/usnic_uiom.c b/drivers/infiniband/hw/usnic/usnic_uiom.c
> index 0b0237d41613..62e6ffa9ad78 100644
> --- a/drivers/infiniband/hw/usnic/usnic_uiom.c
> +++ b/drivers/infiniband/hw/usnic/usnic_uiom.c
> @@ -75,10 +75,7 @@ static void usnic_uiom_put_pages(struct list_head *chunk_list, int dirty)
> for_each_sg(chunk->page_list, sg, chunk->nents, i) {
> page = sg_page(sg);
> pa = sg_phys(sg);
> - if (dirty)
> - put_user_pages_dirty_lock(&page, 1);
> - else
> - put_user_page(page);
> + put_user_pages_dirty_lock(&page, 1, dirty);
> usnic_dbg("pa: %pa\n", &pa);
> }
> kfree(chunk);
> diff --git a/drivers/infiniband/sw/siw/siw_mem.c b/drivers/infiniband/sw/siw/siw_mem.c
> index 67171c82b0c4..1e197753bf2f 100644
> --- a/drivers/infiniband/sw/siw/siw_mem.c
> +++ b/drivers/infiniband/sw/siw/siw_mem.c
> @@ -60,20 +60,6 @@ struct siw_mem *siw_mem_id2obj(struct siw_device *sdev, int stag_index)
> return NULL;
> }
>
> -static void siw_free_plist(struct siw_page_chunk *chunk, int num_pages,
> - bool dirty)
> -{
> - struct page **p = chunk->plist;
> -
> - while (num_pages--) {
> - if (!PageDirty(*p) && dirty)
> - put_user_pages_dirty_lock(p, 1);
> - else
> - put_user_page(*p);
> - p++;
> - }
> -}
> -
> void siw_umem_release(struct siw_umem *umem, bool dirty)
> {
> struct mm_struct *mm_s = umem->owning_mm;
> @@ -82,8 +68,9 @@ void siw_umem_release(struct siw_umem *umem, bool dirty)
> for (i = 0; num_pages; i++) {
> int to_free = min_t(int, PAGES_PER_CHUNK, num_pages);
>
> - siw_free_plist(&umem->page_chunk[i], to_free,
> - umem->writable && dirty);
> + put_user_pages_dirty_lock(umem->page_chunk[i].plist,
> + to_free,
> + umem->writable && dirty);
> kfree(umem->page_chunk[i].plist);
> num_pages -= to_free;
> }
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 0334ca97c584..9759b6a24420 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1057,8 +1057,9 @@ static inline void put_user_page(struct page *page)
> put_page(page);
> }
>
> -void put_user_pages_dirty(struct page **pages, unsigned long npages);
> -void put_user_pages_dirty_lock(struct page **pages, unsigned long npages);
> +void put_user_pages_dirty_lock(struct page **pages, unsigned long npages,
> + bool make_dirty);
> +
> void put_user_pages(struct page **pages, unsigned long npages);
>
> #if defined(CONFIG_SPARSEMEM) && !defined(CONFIG_SPARSEMEM_VMEMMAP)
> diff --git a/mm/gup.c b/mm/gup.c
> index 98f13ab37bac..7fefd7ab02c4 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -29,85 +29,70 @@ struct follow_page_context {
> unsigned int page_mask;
> };
>
> -typedef int (*set_dirty_func_t)(struct page *page);
> -
> -static void __put_user_pages_dirty(struct page **pages,
> - unsigned long npages,
> - set_dirty_func_t sdf)
> -{
> - unsigned long index;
> -
> - for (index = 0; index < npages; index++) {
> - struct page *page = compound_head(pages[index]);
> -
> - /*
> - * Checking PageDirty at this point may race with
> - * clear_page_dirty_for_io(), but that's OK. Two key cases:
> - *
> - * 1) This code sees the page as already dirty, so it skips
> - * the call to sdf(). That could happen because
> - * clear_page_dirty_for_io() called page_mkclean(),
> - * followed by set_page_dirty(). However, now the page is
> - * going to get written back, which meets the original
> - * intention of setting it dirty, so all is well:
> - * clear_page_dirty_for_io() goes on to call
> - * TestClearPageDirty(), and write the page back.
> - *
> - * 2) This code sees the page as clean, so it calls sdf().
> - * The page stays dirty, despite being written back, so it
> - * gets written back again in the next writeback cycle.
> - * This is harmless.
> - */
> - if (!PageDirty(page))
> - sdf(page);
> -
> - put_user_page(page);
> - }
> -}
> -
> /**
> - * put_user_pages_dirty() - release and dirty an array of gup-pinned pages
> - * @pages: array of pages to be marked dirty and released.
> + * put_user_pages_dirty_lock() - release and optionally dirty gup-pinned pages
> + * @pages: array of pages to be maybe marked dirty, and definitely released.
Better would be.
@pages: array of pages to be put
> * @npages: number of pages in the @pages array.
> + * @make_dirty: whether to mark the pages dirty
> *
> * "gup-pinned page" refers to a page that has had one of the get_user_pages()
> * variants called on that page.
> *
> * For each page in the @pages array, make that page (or its head page, if a
> - * compound page) dirty, if it was previously listed as clean. Then, release
> - * the page using put_user_page().
> + * compound page) dirty, if @make_dirty is true, and if the page was previously
> + * listed as clean. In any case, releases all pages using put_user_page(),
> + * possibly via put_user_pages(), for the non-dirty case.
I don't think users of this interface need this level of detail. I think
something like.
* For each page in the @pages array, release the page. If @make_dirty is
* true, mark the page dirty prior to release.
> *
> * Please see the put_user_page() documentation for details.
> *
> - * set_page_dirty(), which does not lock the page, is used here.
> - * Therefore, it is the caller's responsibility to ensure that this is
> - * safe. If not, then put_user_pages_dirty_lock() should be called instead.
> + * set_page_dirty_lock() is used internally. If instead, set_page_dirty() is
> + * required, then the caller should a) verify that this is really correct,
> + * because _lock() is usually required, and b) hand code it:
> + * set_page_dirty_lock(), put_user_page().
> *
> */
> -void put_user_pages_dirty(struct page **pages, unsigned long npages)
> +void put_user_pages_dirty_lock(struct page **pages, unsigned long npages,
> + bool make_dirty)
> {
> - __put_user_pages_dirty(pages, npages, set_page_dirty);
> -}
> -EXPORT_SYMBOL(put_user_pages_dirty);
> + unsigned long index;
>
> -/**
> - * put_user_pages_dirty_lock() - release and dirty an array of gup-pinned pages
> - * @pages: array of pages to be marked dirty and released.
> - * @npages: number of pages in the @pages array.
> - *
> - * For each page in the @pages array, make that page (or its head page, if a
> - * compound page) dirty, if it was previously listed as clean. Then, release
> - * the page using put_user_page().
> - *
> - * Please see the put_user_page() documentation for details.
> - *
> - * This is just like put_user_pages_dirty(), except that it invokes
> - * set_page_dirty_lock(), instead of set_page_dirty().
> - *
> - */
> -void put_user_pages_dirty_lock(struct page **pages, unsigned long npages)
> -{
> - __put_user_pages_dirty(pages, npages, set_page_dirty_lock);
> + /*
> + * TODO: this can be optimized for huge pages: if a series of pages is
> + * physically contiguous and part of the same compound page, then a
> + * single operation to the head page should suffice.
> + */
I think this comment belongs to the for loop below... or just something about
how to make this and put_user_pages() more efficient. It is odd, that this is
the same comment as in put_user_pages()...
The code is good. So... Other than the comments.
Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Ira
> +
> + if (!make_dirty) {
> + put_user_pages(pages, npages);
> + return;
> + }
> +
> + for (index = 0; index < npages; index++) {
> + struct page *page = compound_head(pages[index]);
> + /*
> + * Checking PageDirty at this point may race with
> + * clear_page_dirty_for_io(), but that's OK. Two key
> + * cases:
> + *
> + * 1) This code sees the page as already dirty, so it
> + * skips the call to set_page_dirty(). That could happen
> + * because clear_page_dirty_for_io() called
> + * page_mkclean(), followed by set_page_dirty().
> + * However, now the page is going to get written back,
> + * which meets the original intention of setting it
> + * dirty, so all is well: clear_page_dirty_for_io() goes
> + * on to call TestClearPageDirty(), and write the page
> + * back.
> + *
> + * 2) This code sees the page as clean, so it calls
> + * set_page_dirty(). The page stays dirty, despite being
> + * written back, so it gets written back again in the
> + * next writeback cycle. This is harmless.
> + */
> + if (!PageDirty(page))
> + set_page_dirty_lock(page);
> + put_user_page(page);
> + }
> }
> EXPORT_SYMBOL(put_user_pages_dirty_lock);
>
> --
> 2.22.0
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 0/6] hwspinlock: allow sharing of hwspinlocks
From: Suman Anna @ 2019-08-06 17:38 UTC (permalink / raw)
To: Bjorn Andersson, Fabien DESSENNE
Cc: Ohad Ben-Cohen, Mark Rutland, Alexandre TORGUE, Jonathan Corbet,
linux-doc@vger.kernel.org, linux-remoteproc@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
Rob Herring, Maxime Coquelin,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, Benjamin GAIGNARD
In-Reply-To: <20190805174659.GA23928@tuxbook-pro>
Hi Fabien,
On 8/5/19 12:46 PM, Bjorn Andersson wrote:
> On Mon 05 Aug 01:48 PDT 2019, Fabien DESSENNE wrote:
>
>>
>> On 01/08/2019 9:14 PM, Bjorn Andersson wrote:
>>> On Wed 13 Mar 08:50 PDT 2019, Fabien Dessenne wrote:
>>>
>>>> The current implementation does not allow two different devices to use
>>>> a common hwspinlock. This patch set proposes to have, as an option, some
>>>> hwspinlocks shared between several users.
>>>>
>>>> Below is an example that explain the need for this:
>>>> exti: interrupt-controller@5000d000 {
>>>> compatible = "st,stm32mp1-exti", "syscon";
>>>> interrupt-controller;
>>>> #interrupt-cells = <2>;
>>>> reg = <0x5000d000 0x400>;
>>>> hwlocks = <&hsem 1>;
>>>> };
>>>> The two drivers (stm32mp1-exti and syscon) refer to the same hwlock.
>>>> With the current hwspinlock implementation, only the first driver succeeds
>>>> in requesting (hwspin_lock_request_specific) the hwlock. The second request
>>>> fails.
Help me understand the problem that you are trying to solve here. Is
this a case of you having two clients on Linux-side needing to use the
same lock but still requiring the arbitration with software running on
some other remote processor? Are they talking to the same entity on the
remote-side or different peers.
I see the series is all about getting a handle so that they can use the
API, and is the expected usage that the same entity will lock and unlock
before the other driver can lock it.
>>>>
>>>>
>>>> The proposed approach does not modify the API, but extends the DT 'hwlocks'
>>>> property with a second optional parameter (the first one identifies an
>>>> hwlock) that specifies whether an hwlock is requested for exclusive usage
>>>> (current behavior) or can be shared between several users.
>>>> Examples:
>>>> hwlocks = <&hsem 8>; Ref to hwlock #8 for exclusive usage
>>>> hwlocks = <&hsem 8 0>; Ref to hwlock #8 for exclusive (0) usage
>>>> hwlocks = <&hsem 8 1>; Ref to hwlock #8 for shared (1) usage
>>>>
>>>> As a constraint, the #hwlock-cells value must be 1 or 2.
>>>> In the current implementation, this can have theorically any value but:
>>>> - all of the exisiting drivers use the same value : 1.
>>>> - the framework supports only one value : 1 (see implementation of
>>>> of_hwspin_lock_simple_xlate())
>>>> Hence, it shall not be a problem to restrict this value to 1 or 2 since
>>>> it won't break any driver.
>>>>
>>> Hi Fabien,
>>>
>>> Your series looks good, but it makes me wonder why the hardware locks
>>> should be an exclusive resource.
>>>
>>> How about just making all (specific) locks shared?
>>
>> Hi Bjorn,
>>
>> Making all locks shared is a possible implementation (my first
>> implementation
>> was going this way) but there are some drawbacks we must be aware of:
>>
>> A/ This theoretically break the legacy behavior (the legacy works with
>> exclusive (UNUSED radix tag) usage). As a consequence, an existing driver
>> that is currently failing to request a lock (already claimed by another
>> user) would now work fine. Not sure that there are such drivers, so this
>> point is probably not a real issue.
>>
>
> Right, it's possible that a previously misconfigured system now
> successfully probes more than one device that uses a particular
> spinlock. But such system would be suffering from issues related to e.g.
> probe ordering.
>
> So I think we should ignore this issue.
>
>> B/ This would introduce some inconsistency between the two 'request' API
>> which are hwspin_lock_request() and hwspin_lock_request_specific().
>> hwspin_lock_request() looks for an unused lock, so requests for an exclusive
>> usage. On the other side, request_specific() would request shared locks.
>> Worst the following sequence can transform an exclusive usage into a shared
>>
>
> There is already an inconsistency in between these; as with above any
> system that uses both request() and request_specific() will be suffering
> from intermittent failures due to probe ordering.
>
>> one:
>> -hwspin_lock_request() -> returns Id#0 (exclusive)
>> -hwspin_lock_request() -> returns Id#1 (exclusive)
>> -hwspin_lock_request_specific(0) -> returns Id#0 and makes Id#0 shared
>> Honestly I am not sure that this is a real issue, but it's better to have it
>> in mind before we take ay decision
Wouldn't it be actually simpler to just introduce a new specific API
variant for this, similar to the reset core for example (it uses a
separate exclusive API), without having to modify the bindings at all.
It is just a case of your driver using the right API, and the core can
be modified to use the additional tag semantics based on the API. It
should avoid any confusion with say using a different second cell value
for the same lock in two different nodes.
If you are sharing a hwlock on the Linux side, surely your driver should
be aware that it is a shared lock. The tag can be set during the first
request API, and you look through both tags when giving out a handle.
Obviously, the hwspin_lock_request() API usage semantics always had the
implied additional need for communicating the lock id to the other peer
entity, so a realistic usage is most always the specific API variant. I
doubt this API would be of much use for the shared driver usage. This
also implies that the client user does not care about specifying a lock
in DT.
regards
Suman
>
> The case where I can see a
> problem with this would be if the two clients somehow would nest their
> locking regions.
>
> But generally I think this could consider this an improvement, because
> the request_specific() would now be able to acquire its hwlock, with
> some additional contention due to the multiple use.
>
>> I could not find any driver using the hwspin_lock_request() API, we
>> may decide to remove (or to make deprecated) this API, having
>> everything 'shared without any conditions'.
>>
>
> It would be nice to have an upstream user of this API.
>
>>
>> I can see three options:
>> 1- Keep my initial proposition
>> 2- Have hwspin_lock_request_specific() using shared locks and
>> hwspin_lock_request() using unused (so 'initially' exclusive) locks.
>> 3- Have hwspin_lock_request_specific() using shared locks and
>> remove/make deprecated hwspin_lock_request().
>>
>> Just let me know what is your preference.
>>
>
> I think we should start with #2 and would like input from e.g. Suman
> regarding #3.
>
> Regards,
> Bjorn
>
>> BR
>>
>> Fabien
>>
>>>
>>> Regards,
>>> Bjorn
>>>
>>>> Fabien Dessenne (6):
>>>> dt-bindings: hwlock: add support of shared locks
>>>> hwspinlock: allow sharing of hwspinlocks
>>>> dt-bindings: hwlock: update STM32 #hwlock-cells value
>>>> ARM: dts: stm32: Add hwspinlock node for stm32mp157 SoC
>>>> ARM: dts: stm32: Add hwlock for irqchip on stm32mp157
>>>> ARM: dts: stm32: hwlocks for GPIO for stm32mp157
>>>>
>>>> .../devicetree/bindings/hwlock/hwlock.txt | 27 +++++--
>>>> .../bindings/hwlock/st,stm32-hwspinlock.txt | 6 +-
>>>> Documentation/hwspinlock.txt | 10 ++-
>>>> arch/arm/boot/dts/stm32mp157-pinctrl.dtsi | 2 +
>>>> arch/arm/boot/dts/stm32mp157c.dtsi | 10 +++
>>>> drivers/hwspinlock/hwspinlock_core.c | 82 +++++++++++++++++-----
>>>> drivers/hwspinlock/hwspinlock_internal.h | 2 +
>>>> 7 files changed, 108 insertions(+), 31 deletions(-)
>>>>
>>>> --
>>>> 2.7.4
>>>>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3] tracing: Function stack size and its name mismatch in arm64
From: Joel Fernandes @ 2019-08-06 17:25 UTC (permalink / raw)
To: Steven Rostedt
Cc: Jiping Ma, catalin.marinas, will.deacon, linux-kernel, mingo,
linux-arm-kernel
In-Reply-To: <20190806123455.487ac02b@gandalf.local.home>
On Tue, Aug 06, 2019 at 12:34:55PM -0400, Steven Rostedt wrote:
> On Tue, 6 Aug 2019 11:48:11 -0400
> Joel Fernandes <joel@joelfernandes.org> wrote:
>
>
> > > diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h
> > > index 5ab5200b2bdc..13a4832cfb00 100644
> > > --- a/arch/arm64/include/asm/ftrace.h
> > > +++ b/arch/arm64/include/asm/ftrace.h
> > > @@ -13,6 +13,7 @@
> > > #define HAVE_FUNCTION_GRAPH_FP_TEST
> > > #define MCOUNT_ADDR ((unsigned long)_mcount)
> > > #define MCOUNT_INSN_SIZE AARCH64_INSN_SIZE
> > > +#define ARCH_RET_ADDR_AFTER_LOCAL_VARS 1
> > >
> > > #ifndef __ASSEMBLY__
> > > #include <linux/compat.h>
> > > diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
> > > index 5d16f73898db..050c6bd9beac 100644
> > > --- a/kernel/trace/trace_stack.c
> > > +++ b/kernel/trace/trace_stack.c
> > > @@ -158,6 +158,18 @@ static void check_stack(unsigned long ip, unsigned long *stack)
> > > i++;
> > > }
> > >
> > > +#ifdef ARCH_RET_ADDR_AFTER_LOCAL_VARS
> > > + /*
> > > + * Most archs store the return address before storing the
> > > + * function's local variables. But some archs do this backwards.
> > > + */
> > > + if (x > 1) {
> > > + memmove(&stack_trace_index[0], &stack_trace_index[1],
> > > + sizeof(stack_trace_index[0]) * (x - 1));
> > > + x--;
> > > + }
> > > +#endif
> > > +
> > > stack_trace_nr_entries = x;
> > >
> > > if (task_stack_end_corrupted(current)) {
> >
> >
> > I am not fully understanding the fix :(. If the positions of the data and
> > FP/LR are swapped, then there should be a loop of some sort where the FP/LR
> > are copied repeatedly to undo the mess we are discussing. But in this patch
> > I see only one copy happening. May be I just don't understand this code well
> > enough. Are there any more clues for helping understand the fix?
>
> Here's the best way to explain this. The code is using the stack trace
> to figure out which function is the stack hog. Or perhaps a serious of
> stack hogs. On x86, a call stores the return address as it calls the
> next function. Then that function allocates its stack frame for its
> local variables and saving of registers.
This makes perfect sense, (probably also makes sense to push this whole
explanation into either the changelog or the kernel documentation)
Thanks a lot, Steve!
- Joel
> on x86:
>
> [ top of stack ]
> 0: sys call entry frame
> 10: return addr to entry code
> 11: start of sys_foo frame
> 20: return addr to sys_foo
> 21: start of kernel_func_bar frame
> 30: return addr to kernel_func_bar
> 31: [ do trace stack here ]
>
>
> Then we do a save_stack_trace which returns the addresses of the
> functions it finds. Which would be (from the bottom of the stack to the
> top)
>
> return addr to kernel_func_bar
> return addr to sys_foo
> return addr to entry code
>
> What we do here is try to figure out how much stack each of theses
> functions have. So we loop through the stack looking for the addresses
> returned by the save_stack trace, and see where on the stack this is.
> This gives us:
>
> return addr to kernel_func_bar [ 30 ]
> return addr to sys_foo [ 20 ]
> return addr to entry frame [ 10 ]
>
> From this, we can conclude (on x86) that the size of the stack used for
> kernel_func_bar is 30 - 20 = 10. Because on the stack we have:
>
> 20: return addr to sys_foo
> 21: start of kernel_func_bar frame <<-- kernel_func_bar stack frame
> 30: return addr to kernel_func_bar
>
>
> Now, what Jiping reported, is that on arm64, it saves the link register
> (the return address) when it is needed to, which is after the stack
> frame for the current function has been saved. That means we have
> something that looks like this:
>
> on arm64:
>
> [ top of stack ]
> 0: sys call entry frame
> 10: start of sys_foo_frame
> 19: return addr to entry code << lr saved before calling kern_func_bar
> 20: start of kernel_func_bar frame
> 29: return addr to sys_foo_frame << lr saved before calling next function
> 30: [ do trace stack here ]
>
> Now, I have a question. To call the mcount code (ftrace and the stack
> tracing), you need to save the return address of kern_func_bar
> somewhere, otherwise the call to mcount will overwrite the lr. But
> let's say it does and then forgets it, so we have:
>
> 30: return addr of kernel_func_bar frame
> 31: [ do trace stack here ]
>
> Now save_stack_trace gives us the same result:
>
> return addr to kernel_func bar
> return addr to sys_foo
> return addr to entry frame
>
> But we get a different result when finding them in the location of the
> stack.
>
> return addr to kernel_func_bar [ 30 ]
> return addr to sys foo [ 29 ]
> return addr to entry frame [ 19 ]
>
> The simple subtractions will be off:
>
> kernel_func_bar stack size = 30 - 29 = 1
> Or even, sys_foo 29 - 19 = 10, but if we look at the stack:
>
> 10: start of sys_foo_frame
> 19: return addr to entry_code
> 20: start of kernel_func_bar frame
> 29: return addr to sys_foo
>
> We are measuring the kernel_func_bar frame for sys_foo!
>
> We are off by one here.
>
> stack_trace_index[] is an array of the offsets mapping to the function
> return addresses found. If we shift it by one, then we then sync the
> functions found with their frames:
>
> stack_trace_index[0] = 30
> stack_trace_index[1] = 29
> stack_trace_index[2] = 19
>
> memmove((&stack_trace_index[0], &stack_trace_index[1],
> sizeof(stack_trace_index[0]) * (x - 1));
>
> Makes that:
>
> stack_trace_index[0] = 29
> stack_trace_index[1] = 19
>
> And we do x-- to lose the last frame.
>
> With the stack_dump_trace being:
>
> stack_dump_trace[0] = return addr kernel_func_bar
> stack_dump_trace[1] = return addr sys_foo
>
> we then match which frame size belongs to which function better.
>
>
> >
> > Also, this stack trace loop (original code) is a bit hairy :) It appears
> > there is a call to stack_trace_save() followed by another loop that goes
> > through the returned entries from there and tries to generate a set of
> > indexes. Isn't the real issue that the entries returned by stack_trace_save()
> > are a out of whack? I am curious also if other users of stack_trace_save()
> > will suffer from the same issue.
>
> No, the order is fine. The issue is that we are using the location of
> the return address in the stack to find out what function has the
> biggest stack usage, and our assumption for arm64 is incorrect in that
> location.
>
> -- Steve
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v19 00/15] arm64: untag user pointers passed to the kernel
From: Will Deacon @ 2019-08-06 17:13 UTC (permalink / raw)
To: Will Deacon
Cc: Mark Rutland, kvm, Christian Koenig, Szabolcs Nagy,
Catalin Marinas, dri-devel, Kostya Serebryany, Khalid Aziz,
Lee Smith, open list:KERNEL SELFTEST FRAMEWORK, Vincenzo Frascino,
Jacob Bramley, Leon Romanovsky, linux-rdma, amd-gfx,
Christoph Hellwig, Jason Gunthorpe, Dmitry Vyukov, Dave Martin,
Evgeniy Stepanov, linux-media, Kees Cook, Ruben Ayrapetyan,
Andrey Konovalov, Kevin Brodsky, Alex Williamson,
Mauro Carvalho Chehab, Linux ARM, Linux Memory Management List,
Greg Kroah-Hartman, Felix Kuehling, LKML, Jens Wiklander,
Ramana Radhakrishnan, Alexander Deucher, Andrew Morton, enh,
Robin Murphy, Yishai Hadas, Luc Van Oostenryck
In-Reply-To: <20190724142059.GC21234@fuggles.cambridge.arm.com>
On Wed, Jul 24, 2019 at 03:20:59PM +0100, Will Deacon wrote:
> On Wed, Jul 24, 2019 at 04:16:49PM +0200, Andrey Konovalov wrote:
> > On Wed, Jul 24, 2019 at 4:02 PM Will Deacon <will@kernel.org> wrote:
> > > On Tue, Jul 23, 2019 at 08:03:29PM +0200, Andrey Konovalov wrote:
> > > > Should this go through the mm or the arm tree?
> > >
> > > I would certainly prefer to take at least the arm64 bits via the arm64 tree
> > > (i.e. patches 1, 2 and 15). We also need a Documentation patch describing
> > > the new ABI.
> >
> > Sounds good! Should I post those patches together with the
> > Documentation patches from Vincenzo as a separate patchset?
>
> Yes, please (although as you say below, we need a new version of those
> patches from Vincenzo to address the feedback on v5). The other thing I
> should say is that I'd be happy to queue the other patches in the series
> too, but some of them are missing acks from the relevant maintainers (e.g.
> the mm/ and fs/ changes).
Ok, I've queued patches 1, 2, and 15 on a stable branch here:
https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=for-next/tbi
which should find its way into -next shortly via our for-next/core branch.
If you want to make changes, please send additional patches on top.
This is targetting 5.4, but I will drop it before the merge window if
we don't have both of the following in place:
* Updated ABI documentation with Acks from Catalin and Kevin
* The other patches in the series either Acked (so I can pick them up)
or queued via some other tree(s) for 5.4.
Make sense?
Cheers,
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 5/6] tty: serial: Add linflexuart driver for S32V234
From: Stefan-gabriel Mirea @ 2019-08-06 17:11 UTC (permalink / raw)
To: gregkh@linuxfoundation.org
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, corbet@lwn.net,
catalin.marinas@arm.com, linux-doc@vger.kernel.org,
Larisa Ileana Grigore, linux-kernel@vger.kernel.org, Leo Li,
Cosmin Stefan Stoica, robh+dt@kernel.org,
linux-serial@vger.kernel.org, jslaby@suse.com,
shawnguo@kernel.org, will@kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190805153114.GA16836@kroah.com>
On 8/5/2019 6:31 PM, gregkh@linuxfoundation.org wrote:
> On Fri, Aug 02, 2019 at 07:47:23PM +0000, Stefan-gabriel Mirea wrote:
>>
>> +/* Freescale Linflex UART */
>> +#define PORT_LINFLEXUART 121
>
> Do you really need this modified?
Hello Greg,
This macro is meant to be assigned to port->type in the config_port
method from uart_ops, in order for verify_port to know if the received
serial_struct structure was really targeted for a LINFlex port. It
needs to be defined outside, to avoid "collisions" with other drivers.
As far as I see, uart_set_info() will actually fail at the
"baud_base < 9600" check[1], right after calling verify_port(), when
performing an ioctl() on "/dev/console" with TIOCSSERIAL using a
serial_struct obtained with TIOCGSERIAL. This happens because this
reduced version of the LINFlex UART driver will not touch the uartclk
field of the uart_port (as there is currently no clock support).
Therefore, the linflex_config/verify_port() functions, along with the
PORT_LINFLEXUART macro, may be indeed unnecessary at this point (and
should be added later). Is this what you mean?
Other than that, I do not see anything wrong with the addition of a
define in serial_core.h for this purpose (which is also what most of the
serial drivers do, including amba-pl011.c, mentioned in
Documentation/driver-api/serial/driver.rst as providing the reference
implementation), so please be more specific.
Regards,
Stefan
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/serial_core.c?h=v5.3-rc1#n872
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3] tracing: Function stack size and its name mismatch in arm64
From: Steven Rostedt @ 2019-08-06 17:03 UTC (permalink / raw)
To: Joel Fernandes
Cc: Jiping Ma, catalin.marinas, will.deacon, linux-kernel, mingo,
linux-arm-kernel
In-Reply-To: <20190806123455.487ac02b@gandalf.local.home>
On Tue, 6 Aug 2019 12:34:55 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> Here's the best way to explain this. The code is using the stack trace
> to figure out which function is the stack hog. Or perhaps a serious of
Why so serious? .... s/serious/series/
-- Steve
> stack hogs. On x86, a call stores the return address as it calls the
> next function. Then that function allocates its stack frame for its
> local variables and saving of registers.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2 5/5] reset: Add support for resets provided by SCMI
From: Sudeep Holla @ 2019-08-06 17:02 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Peng Fan, Etienne Carriere, Souvik Chakravarty, Philipp Zabel,
wesleys, aidapala, linux-kernel, Saeed Nowshadi, Bo Zhang,
Felix Burton, Jim Quinlan, Sudeep Holla, pajay, Gaku Inami,
Volodymyr Babchuk
In-Reply-To: <20190806170208.6787-1-sudeep.holla@arm.com>
On some ARM based systems, a separate Cortex-M based System Control
Processor(SCP) provides the overall power, clock, reset and system
control. System Control and Management Interface(SCMI) Message Protocol
is defined for the communication between the Application Cores(AP)
and the SCP.
Adds support for the resets provided using SCMI protocol for performing
reset management of various devices present on the SoC. Various reset
functionalities are achieved by the means of different ARM SCMI device
operations provided by the ARM SCMI framework.
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
MAINTAINERS | 1 +
drivers/reset/Kconfig | 11 ++++
drivers/reset/Makefile | 1 +
drivers/reset/reset-scmi.c | 126 +++++++++++++++++++++++++++++++++++++
4 files changed, 139 insertions(+)
create mode 100644 drivers/reset/reset-scmi.c
v1->v2:
- Renamed RESET_ARM_SCMI to RESET_SCMI and reworded Kconfig text
- Dropped unused struct device pointer from scmi_reset_data
- Added to_scmi_handle which helped to remove some repetitive code
- Fixed some doxygen comments
- Initialised rcdev.nr_resets
- Fixed MODULE_DESCRIPTION
diff --git a/MAINTAINERS b/MAINTAINERS
index 6426db5198f0..f4af5c59c116 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15545,6 +15545,7 @@ F: drivers/clk/clk-sc[mp]i.c
F: drivers/cpufreq/sc[mp]i-cpufreq.c
F: drivers/firmware/arm_scpi.c
F: drivers/firmware/arm_scmi/
+F: drivers/reset/reset-scmi.c
F: include/linux/sc[mp]i_protocol.h
SYSTEM RESET/SHUTDOWN DRIVERS
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 21efb7d39d62..4178ac11ba85 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -116,6 +116,17 @@ config RESET_QCOM_PDC
to control reset signals provided by PDC for Modem, Compute,
Display, GPU, Debug, AOP, Sensors, Audio, SP and APPS.
+config RESET_SCMI
+ tristate "Reset driver controlled via ARM SCMI interface"
+ depends on ARM_SCMI_PROTOCOL || COMPILE_TEST
+ default ARM_SCMI_PROTOCOL
+ help
+ This driver provides support for reset signal/domains that are
+ controlled by firmware that implements the SCMI interface.
+
+ This driver uses SCMI Message Protocol to interact with the
+ firmware controlling all the reset signals.
+
config RESET_SIMPLE
bool "Simple Reset Controller Driver" if COMPILE_TEST
default ARCH_STM32 || ARCH_STRATIX10 || ARCH_SUNXI || ARCH_ZX || ARCH_ASPEED || ARCH_BITMAIN
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 61456b8f659c..cf60ce526064 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
obj-$(CONFIG_RESET_QCOM_AOSS) += reset-qcom-aoss.o
obj-$(CONFIG_RESET_QCOM_PDC) += reset-qcom-pdc.o
+obj-$(CONFIG_RESET_SCMI) += reset-scmi.o
obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
obj-$(CONFIG_RESET_STM32MP157) += reset-stm32mp1.o
obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
diff --git a/drivers/reset/reset-scmi.c b/drivers/reset/reset-scmi.c
new file mode 100644
index 000000000000..5e976a02a6cc
--- /dev/null
+++ b/drivers/reset/reset-scmi.c
@@ -0,0 +1,126 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ARM System Control and Management Interface (ARM SCMI) reset driver
+ *
+ * Copyright (C) 2019 ARM Ltd.
+ */
+
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/scmi_protocol.h>
+
+/**
+ * struct scmi_reset_data - reset controller information structure
+ * @rcdev: reset controller entity
+ * @handle: ARM SCMI handle used for communication with system controller
+ * @dev: reset controller device pointer
+ */
+struct scmi_reset_data {
+ struct reset_controller_dev rcdev;
+ const struct scmi_handle *handle;
+};
+
+#define to_scmi_reset_data(p) container_of((p), struct scmi_reset_data, rcdev)
+#define to_scmi_handle(p) (to_scmi_reset_data(p)->handle)
+
+/**
+ * scmi_reset_assert() - assert device reset
+ * @rcdev: reset controller entity
+ * @id: ID of the reset to be asserted
+ *
+ * This function implements the reset driver op to assert a device's reset
+ * using the ARM SCMI protocol.
+ *
+ * Return: 0 for successful request, else a corresponding error value
+ */
+static int
+scmi_reset_assert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+ const struct scmi_handle *handle = to_scmi_handle(rcdev);
+
+ return handle->reset_ops->assert(handle, id);
+}
+
+/**
+ * scmi_reset_deassert() - deassert device reset
+ * @rcdev: reset controller entity
+ * @id: ID of the reset to be deasserted
+ *
+ * This function implements the reset driver op to deassert a device's reset
+ * using the ARM SCMI protocol.
+ *
+ * Return: 0 for successful request, else a corresponding error value
+ */
+static int
+scmi_reset_deassert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+ const struct scmi_handle *handle = to_scmi_handle(rcdev);
+
+ return handle->reset_ops->deassert(handle, id);
+}
+
+/**
+ * scmi_reset_reset() - reset the device
+ * @rcdev: reset controller entity
+ * @id: ID of the reset signal to be reset(assert + deassert)
+ *
+ * This function implements the reset driver op to trigger a device's
+ * reset signal using the ARM SCMI protocol.
+ *
+ * Return: 0 for successful request, else a corresponding error value
+ */
+static int
+scmi_reset_reset(struct reset_controller_dev *rcdev, unsigned long id)
+{
+ const struct scmi_handle *handle = to_scmi_handle(rcdev);
+
+ return handle->reset_ops->reset(handle, id);
+}
+
+static const struct reset_control_ops scmi_reset_ops = {
+ .assert = scmi_reset_assert,
+ .deassert = scmi_reset_deassert,
+ .reset = scmi_reset_reset,
+};
+
+static int scmi_reset_probe(struct scmi_device *sdev)
+{
+ struct scmi_reset_data *data;
+ struct device *dev = &sdev->dev;
+ struct device_node *np = dev->of_node;
+ const struct scmi_handle *handle = sdev->handle;
+
+ if (!handle || !handle->reset_ops)
+ return -ENODEV;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->rcdev.ops = &scmi_reset_ops;
+ data->rcdev.owner = THIS_MODULE;
+ data->rcdev.of_node = np;
+ data->rcdev.nr_resets = handle->reset_ops->num_domains_get(handle);
+
+ return devm_reset_controller_register(dev, &data->rcdev);
+}
+
+static const struct scmi_device_id scmi_id_table[] = {
+ { SCMI_PROTOCOL_RESET },
+ { },
+};
+MODULE_DEVICE_TABLE(scmi, scmi_id_table);
+
+static struct scmi_driver scmi_reset_driver = {
+ .name = "scmi-reset",
+ .probe = scmi_reset_probe,
+ .id_table = scmi_id_table,
+};
+module_scmi_driver(scmi_reset_driver);
+
+MODULE_AUTHOR("Sudeep Holla <sudeep.holla@arm.com>");
+MODULE_DESCRIPTION("ARM SCMI reset controller driver");
+MODULE_LICENSE("GPL v2");
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 4/5] firmware: arm_scmi: Add RESET protocol in SCMI v2.0
From: Sudeep Holla @ 2019-08-06 17:02 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Peng Fan, Etienne Carriere, Souvik Chakravarty, Philipp Zabel,
wesleys, aidapala, linux-kernel, Saeed Nowshadi, Bo Zhang,
Felix Burton, Jim Quinlan, Sudeep Holla, pajay, Gaku Inami,
Volodymyr Babchuk
In-Reply-To: <20190806170208.6787-1-sudeep.holla@arm.com>
SCMIv2.0 adds a new Reset Management Protocol to manage various reset
states a given device or domain can enter. Device(s) that can be
collectively reset through a common reset signal constitute a reset
domain for the firmware.
A reset domain can be reset autonomously or explicitly through assertion
and de-assertion of the signal. When autonomous reset is chosen, the
firmware is responsible for taking the necessary steps to reset the
domain and to subsequently bring it out of reset. When explicit reset is
chosen, the caller has to specifically assert and then de-assert the
reset signal by issuing two separate RESET commands.
Add the basic SCMI reset infrastructure that can be used by Linux
reset controller driver.
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/firmware/arm_scmi/Makefile | 2 +-
drivers/firmware/arm_scmi/reset.c | 231 +++++++++++++++++++++++++++++
include/linux/scmi_protocol.h | 26 ++++
3 files changed, 258 insertions(+), 1 deletion(-)
create mode 100644 drivers/firmware/arm_scmi/reset.c
diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
index c47d28d556b6..5f298f00a82e 100644
--- a/drivers/firmware/arm_scmi/Makefile
+++ b/drivers/firmware/arm_scmi/Makefile
@@ -2,5 +2,5 @@
obj-y = scmi-bus.o scmi-driver.o scmi-protocols.o
scmi-bus-y = bus.o
scmi-driver-y = driver.o
-scmi-protocols-y = base.o clock.o perf.o power.o sensors.o
+scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o
obj-$(CONFIG_ARM_SCMI_POWER_DOMAIN) += scmi_pm_domain.o
diff --git a/drivers/firmware/arm_scmi/reset.c b/drivers/firmware/arm_scmi/reset.c
new file mode 100644
index 000000000000..11cb8b5ccf34
--- /dev/null
+++ b/drivers/firmware/arm_scmi/reset.c
@@ -0,0 +1,231 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * System Control and Management Interface (SCMI) Reset Protocol
+ *
+ * Copyright (C) 2019 ARM Ltd.
+ */
+
+#include "common.h"
+
+enum scmi_reset_protocol_cmd {
+ RESET_DOMAIN_ATTRIBUTES = 0x3,
+ RESET = 0x4,
+ RESET_NOTIFY = 0x5,
+};
+
+enum scmi_reset_protocol_notify {
+ RESET_ISSUED = 0x0,
+};
+
+#define NUM_RESET_DOMAIN_MASK 0xffff
+#define RESET_NOTIFY_ENABLE BIT(0)
+
+struct scmi_msg_resp_reset_domain_attributes {
+ __le32 attributes;
+#define SUPPORTS_ASYNC_RESET(x) ((x) & BIT(31))
+#define SUPPORTS_NOTIFY_RESET(x) ((x) & BIT(30))
+ __le32 latency;
+ u8 name[SCMI_MAX_STR_SIZE];
+};
+
+struct scmi_msg_reset_domain_reset {
+ __le32 domain_id;
+ __le32 flags;
+#define AUTONOMOUS_RESET BIT(0)
+#define EXPLICIT_RESET_ASSERT BIT(1)
+#define ASYNCHRONOUS_RESET BIT(2)
+ __le32 reset_state;
+#define ARCH_RESET_TYPE BIT(31)
+#define COLD_RESET_STATE BIT(0)
+#define ARCH_COLD_RESET (ARCH_RESET_TYPE | COLD_RESET_STATE)
+};
+
+struct reset_dom_info {
+ bool async_reset;
+ bool reset_notify;
+ u32 latency_us;
+ char name[SCMI_MAX_STR_SIZE];
+};
+
+struct scmi_reset_info {
+ int num_domains;
+ struct reset_dom_info *dom_info;
+};
+
+static int scmi_reset_attributes_get(const struct scmi_handle *handle,
+ struct scmi_reset_info *pi)
+{
+ int ret;
+ struct scmi_xfer *t;
+ u32 *attr;
+
+ ret = scmi_xfer_get_init(handle, PROTOCOL_ATTRIBUTES,
+ SCMI_PROTOCOL_RESET, 0, sizeof(*attr), &t);
+ if (ret)
+ return ret;
+
+ attr = t->rx.buf;
+
+ ret = scmi_do_xfer(handle, t);
+ if (!ret)
+ pi->num_domains = le32_to_cpu(*attr) & NUM_RESET_DOMAIN_MASK;
+
+ scmi_xfer_put(handle, t);
+ return ret;
+}
+
+static int
+scmi_reset_domain_attributes_get(const struct scmi_handle *handle, u32 domain,
+ struct reset_dom_info *dom_info)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct scmi_msg_resp_reset_domain_attributes *attr;
+
+ ret = scmi_xfer_get_init(handle, RESET_DOMAIN_ATTRIBUTES,
+ SCMI_PROTOCOL_RESET, sizeof(domain),
+ sizeof(*attr), &t);
+ if (ret)
+ return ret;
+
+ *(__le32 *)t->tx.buf = cpu_to_le32(domain);
+ attr = t->rx.buf;
+
+ ret = scmi_do_xfer(handle, t);
+ if (!ret) {
+ u32 attributes = le32_to_cpu(attr->attributes);
+
+ dom_info->async_reset = SUPPORTS_ASYNC_RESET(attributes);
+ dom_info->reset_notify = SUPPORTS_NOTIFY_RESET(attributes);
+ dom_info->latency_us = le32_to_cpu(attr->latency);
+ if (dom_info->latency_us == U32_MAX)
+ dom_info->latency_us = 0;
+ strlcpy(dom_info->name, attr->name, SCMI_MAX_STR_SIZE);
+ }
+
+ scmi_xfer_put(handle, t);
+ return ret;
+}
+
+static int scmi_reset_num_domains_get(const struct scmi_handle *handle)
+{
+ struct scmi_reset_info *pi = handle->reset_priv;
+
+ return pi->num_domains;
+}
+
+static char *scmi_reset_name_get(const struct scmi_handle *handle, u32 domain)
+{
+ struct scmi_reset_info *pi = handle->reset_priv;
+ struct reset_dom_info *dom = pi->dom_info + domain;
+
+ return dom->name;
+}
+
+static int scmi_reset_latency_get(const struct scmi_handle *handle, u32 domain)
+{
+ struct scmi_reset_info *pi = handle->reset_priv;
+ struct reset_dom_info *dom = pi->dom_info + domain;
+
+ return dom->latency_us;
+}
+
+static int scmi_domain_reset(const struct scmi_handle *handle, u32 domain,
+ u32 flags, u32 state)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct scmi_msg_reset_domain_reset *dom;
+ struct scmi_reset_info *pi = handle->reset_priv;
+ struct reset_dom_info *rdom = pi->dom_info + domain;
+
+ if (rdom->async_reset)
+ flags |= ASYNCHRONOUS_RESET;
+
+ ret = scmi_xfer_get_init(handle, RESET, SCMI_PROTOCOL_RESET,
+ sizeof(*dom), 0, &t);
+ if (ret)
+ return ret;
+
+ dom = t->tx.buf;
+ dom->domain_id = cpu_to_le32(domain);
+ dom->flags = cpu_to_le32(flags);
+ dom->domain_id = cpu_to_le32(state);
+
+ if (rdom->async_reset)
+ ret = scmi_do_xfer_with_response(handle, t);
+ else
+ ret = scmi_do_xfer(handle, t);
+
+ scmi_xfer_put(handle, t);
+ return ret;
+}
+
+static int scmi_reset_domain_reset(const struct scmi_handle *handle, u32 domain)
+{
+ return scmi_domain_reset(handle, domain, AUTONOMOUS_RESET,
+ ARCH_COLD_RESET);
+}
+
+static int
+scmi_reset_domain_assert(const struct scmi_handle *handle, u32 domain)
+{
+ return scmi_domain_reset(handle, domain, EXPLICIT_RESET_ASSERT,
+ ARCH_COLD_RESET);
+}
+
+static int
+scmi_reset_domain_deassert(const struct scmi_handle *handle, u32 domain)
+{
+ return scmi_domain_reset(handle, domain, 0, ARCH_COLD_RESET);
+}
+
+static struct scmi_reset_ops reset_ops = {
+ .num_domains_get = scmi_reset_num_domains_get,
+ .name_get = scmi_reset_name_get,
+ .latency_get = scmi_reset_latency_get,
+ .reset = scmi_reset_domain_reset,
+ .assert = scmi_reset_domain_assert,
+ .deassert = scmi_reset_domain_deassert,
+};
+
+static int scmi_reset_protocol_init(struct scmi_handle *handle)
+{
+ int domain;
+ u32 version;
+ struct scmi_reset_info *pinfo;
+
+ scmi_version_get(handle, SCMI_PROTOCOL_RESET, &version);
+
+ dev_dbg(handle->dev, "Reset Version %d.%d\n",
+ PROTOCOL_REV_MAJOR(version), PROTOCOL_REV_MINOR(version));
+
+ pinfo = devm_kzalloc(handle->dev, sizeof(*pinfo), GFP_KERNEL);
+ if (!pinfo)
+ return -ENOMEM;
+
+ scmi_reset_attributes_get(handle, pinfo);
+
+ pinfo->dom_info = devm_kcalloc(handle->dev, pinfo->num_domains,
+ sizeof(*pinfo->dom_info), GFP_KERNEL);
+ if (!pinfo->dom_info)
+ return -ENOMEM;
+
+ for (domain = 0; domain < pinfo->num_domains; domain++) {
+ struct reset_dom_info *dom = pinfo->dom_info + domain;
+
+ scmi_reset_domain_attributes_get(handle, domain, dom);
+ }
+
+ handle->reset_ops = &reset_ops;
+ handle->reset_priv = pinfo;
+
+ return 0;
+}
+
+static int __init scmi_reset_init(void)
+{
+ return scmi_protocol_register(SCMI_PROTOCOL_RESET,
+ &scmi_reset_protocol_init);
+}
+subsys_initcall(scmi_reset_init);
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index f0f2b53a1dac..881fea47c83d 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -187,6 +187,26 @@ struct scmi_sensor_ops {
u64 *value);
};
+/**
+ * struct scmi_reset_ops - represents the various operations provided
+ * by SCMI Reset Protocol
+ *
+ * @num_domains_get: get the count of reset domains provided by SCMI
+ * @name_get: gets the name of a reset domain
+ * @latency_get: gets the reset latency for the specified reset domain
+ * @reset: resets the specified reset domain
+ * @assert: explicitly assert reset signal of the specified reset domain
+ * @deassert: explicitly deassert reset signal of the specified reset domain
+ */
+struct scmi_reset_ops {
+ int (*num_domains_get)(const struct scmi_handle *handle);
+ char *(*name_get)(const struct scmi_handle *handle, u32 domain);
+ int (*latency_get)(const struct scmi_handle *handle, u32 domain);
+ int (*reset)(const struct scmi_handle *handle, u32 domain);
+ int (*assert)(const struct scmi_handle *handle, u32 domain);
+ int (*deassert)(const struct scmi_handle *handle, u32 domain);
+};
+
/**
* struct scmi_handle - Handle returned to ARM SCMI clients for usage.
*
@@ -196,6 +216,7 @@ struct scmi_sensor_ops {
* @perf_ops: pointer to set of performance protocol operations
* @clk_ops: pointer to set of clock protocol operations
* @sensor_ops: pointer to set of sensor protocol operations
+ * @reset_ops: pointer to set of reset protocol operations
* @perf_priv: pointer to private data structure specific to performance
* protocol(for internal use only)
* @clk_priv: pointer to private data structure specific to clock
@@ -204,6 +225,8 @@ struct scmi_sensor_ops {
* protocol(for internal use only)
* @sensor_priv: pointer to private data structure specific to sensors
* protocol(for internal use only)
+ * @reset_priv: pointer to private data structure specific to reset
+ * protocol(for internal use only)
*/
struct scmi_handle {
struct device *dev;
@@ -212,11 +235,13 @@ struct scmi_handle {
struct scmi_clk_ops *clk_ops;
struct scmi_power_ops *power_ops;
struct scmi_sensor_ops *sensor_ops;
+ struct scmi_reset_ops *reset_ops;
/* for protocol internal use */
void *perf_priv;
void *clk_priv;
void *power_priv;
void *sensor_priv;
+ void *reset_priv;
};
enum scmi_std_protocol {
@@ -226,6 +251,7 @@ enum scmi_std_protocol {
SCMI_PROTOCOL_PERF = 0x13,
SCMI_PROTOCOL_CLOCK = 0x14,
SCMI_PROTOCOL_SENSOR = 0x15,
+ SCMI_PROTOCOL_RESET = 0x16,
};
struct scmi_device {
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 3/5] dt-bindings: arm: Extend SCMI to support new reset protocol
From: Sudeep Holla @ 2019-08-06 17:02 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mark Rutland, devicetree, Peng Fan, Etienne Carriere,
Souvik Chakravarty, Philipp Zabel, wesleys, aidapala,
linux-kernel, Rob Herring, Saeed Nowshadi, Bo Zhang, Felix Burton,
Jim Quinlan, Sudeep Holla, pajay, Gaku Inami, Volodymyr Babchuk
In-Reply-To: <20190806170208.6787-1-sudeep.holla@arm.com>
SCMIv2.0 adds a new Reset Management Protocol to manage various reset
states a given device or domain can enter. Extend the existing SCMI
bindings to add reset protocol support by re-using the reset bindings
for bothe reset providers and consumers.
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: devicetree@vger.kernel.org
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
.../devicetree/bindings/arm/arm,scmi.txt | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt b/Documentation/devicetree/bindings/arm/arm,scmi.txt
index 317a2fc3667a..083dbf96ee00 100644
--- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
+++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
@@ -73,6 +73,16 @@ SCMI provides an API to access the various sensors on the SoC.
as used by the firmware. Refer to platform details
for your implementation for the IDs to use.
+Reset signal bindings for the reset domains based on SCMI Message Protocol
+------------------------------------------------------------
+
+This binding for the SCMI reset domain providers uses the generic reset
+signal binding[5].
+
+Required properties:
+ - #reset-cells : Should be 1. Contains the reset domain ID value used
+ by SCMI commands.
+
SRAM and Shared Memory for SCMI
-------------------------------
@@ -93,6 +103,7 @@ Each sub-node represents the reserved area for SCMI.
[2] Documentation/devicetree/bindings/power/power_domain.txt
[3] Documentation/devicetree/bindings/thermal/thermal.txt
[4] Documentation/devicetree/bindings/sram/sram.txt
+[5] Documentation/devicetree/bindings/reset/reset.txt
Example:
@@ -152,6 +163,11 @@ firmware {
reg = <0x15>;
#thermal-sensor-cells = <1>;
};
+
+ scmi_reset: protocol@16 {
+ reg = <0x16>;
+ #reset-cells = <1>;
+ };
};
};
@@ -166,6 +182,7 @@ hdlcd@7ff60000 {
reg = <0 0x7ff60000 0 0x1000>;
clocks = <&scmi_clk 4>;
power-domains = <&scmi_devpd 1>;
+ resets = <&scmi_reset 10>;
};
thermal-zones {
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 2/5] firmware: arm_scmi: Make use SCMI v2.0 fastchannel for performance protocol
From: Sudeep Holla @ 2019-08-06 17:02 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Peng Fan, Etienne Carriere, Souvik Chakravarty, wesleys,
Ionela Voinescu, aidapala, linux-kernel, Saeed Nowshadi,
Quentin Perret, Bo Zhang, Felix Burton, Jim Quinlan, Sudeep Holla,
pajay, Gaku Inami, Volodymyr Babchuk, Chris Redpath
In-Reply-To: <20190806170208.6787-1-sudeep.holla@arm.com>
SCMI v2.0 adds support for "FastChannel" which do not use a message
header as they are specialized for a single message.
Only PERFORMANCE_LIMITS_{SET,GET} and PERFORMANCE_LEVEL_{SET,GET}
commands are supported over fastchannels. As they are optional, they
need to be discovered by PERFORMANCE_DESCRIBE_FASTCHANNEL command.
Further {LIMIT,LEVEL}_SET commands can have optional doorbell support.
Add support for making use of these fastchannels.
Cc: Ionela Voinescu <Ionela.Voinescu@arm.com>
Cc: Chris Redpath <Chris.Redpath@arm.com>
Cc: Quentin Perret <Quentin.Perret@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/firmware/arm_scmi/perf.c | 104 +++++++++++++++++++++++++++++--
1 file changed, 100 insertions(+), 4 deletions(-)
v1->v2:
- Changed the macro SCMI_PERF_FC_RING_DB to use do {} while(0)
diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
index 6cce3e82e81e..fb7f6cab2c11 100644
--- a/drivers/firmware/arm_scmi/perf.c
+++ b/drivers/firmware/arm_scmi/perf.c
@@ -8,6 +8,7 @@
#include <linux/bits.h>
#include <linux/of.h>
#include <linux/io.h>
+#include <linux/io-64-nonatomic-hi-lo.h>
#include <linux/platform_device.h>
#include <linux/pm_opp.h>
#include <linux/sort.h>
@@ -293,7 +294,42 @@ scmi_perf_describe_levels_get(const struct scmi_handle *handle, u32 domain,
return ret;
}
-static int scmi_perf_limits_set(const struct scmi_handle *handle, u32 domain,
+#define SCMI_PERF_FC_RING_DB(doorbell, w) \
+do { \
+ u##w val = 0; \
+ struct scmi_fc_db_info *db = doorbell; \
+ \
+ if (db->mask) \
+ val = ioread##w(db->addr) & db->mask; \
+ iowrite##w((u##w)db->set | val, db->addr); \
+} while(0)
+
+static void scmi_perf_fc_ring_db(struct scmi_fc_db_info *db)
+{
+ if (!db || !db->addr)
+ return;
+
+ if (db->width == 1)
+ SCMI_PERF_FC_RING_DB(db, 8);
+ else if (db->width == 2)
+ SCMI_PERF_FC_RING_DB(db, 16);
+ else if (db->width == 4)
+ SCMI_PERF_FC_RING_DB(db, 32);
+ else /* db->width == 8 */
+#ifdef CONFIG_64BIT
+ SCMI_PERF_FC_RING_DB(db, 64);
+#else
+ {
+ u64 val = 0;
+
+ if (db->mask)
+ val = ioread64_hi_lo(db->addr) & db->mask;
+ iowrite64_hi_lo(db->set, db->addr);
+ }
+#endif
+}
+
+static int scmi_perf_mb_limits_set(const struct scmi_handle *handle, u32 domain,
u32 max_perf, u32 min_perf)
{
int ret;
@@ -316,7 +352,23 @@ static int scmi_perf_limits_set(const struct scmi_handle *handle, u32 domain,
return ret;
}
-static int scmi_perf_limits_get(const struct scmi_handle *handle, u32 domain,
+static int scmi_perf_limits_set(const struct scmi_handle *handle, u32 domain,
+ u32 max_perf, u32 min_perf)
+{
+ struct scmi_perf_info *pi = handle->perf_priv;
+ struct perf_dom_info *dom = pi->dom_info + domain;
+
+ if (dom->fc_info && dom->fc_info->limit_set_addr) {
+ iowrite32(max_perf, dom->fc_info->limit_set_addr);
+ iowrite32(min_perf, dom->fc_info->limit_set_addr + 4);
+ scmi_perf_fc_ring_db(dom->fc_info->limit_set_db);
+ return 0;
+ }
+
+ return scmi_perf_mb_limits_set(handle, domain, max_perf, min_perf);
+}
+
+static int scmi_perf_mb_limits_get(const struct scmi_handle *handle, u32 domain,
u32 *max_perf, u32 *min_perf)
{
int ret;
@@ -342,7 +394,22 @@ static int scmi_perf_limits_get(const struct scmi_handle *handle, u32 domain,
return ret;
}
-static int scmi_perf_level_set(const struct scmi_handle *handle, u32 domain,
+static int scmi_perf_limits_get(const struct scmi_handle *handle, u32 domain,
+ u32 *max_perf, u32 *min_perf)
+{
+ struct scmi_perf_info *pi = handle->perf_priv;
+ struct perf_dom_info *dom = pi->dom_info + domain;
+
+ if (dom->fc_info && dom->fc_info->limit_get_addr) {
+ *max_perf = ioread32(dom->fc_info->limit_get_addr);
+ *min_perf = ioread32(dom->fc_info->limit_get_addr + 4);
+ return 0;
+ }
+
+ return scmi_perf_mb_limits_get(handle, domain, max_perf, min_perf);
+}
+
+static int scmi_perf_mb_level_set(const struct scmi_handle *handle, u32 domain,
u32 level, bool poll)
{
int ret;
@@ -365,7 +432,22 @@ static int scmi_perf_level_set(const struct scmi_handle *handle, u32 domain,
return ret;
}
-static int scmi_perf_level_get(const struct scmi_handle *handle, u32 domain,
+static int scmi_perf_level_set(const struct scmi_handle *handle, u32 domain,
+ u32 level, bool poll)
+{
+ struct scmi_perf_info *pi = handle->perf_priv;
+ struct perf_dom_info *dom = pi->dom_info + domain;
+
+ if (dom->fc_info && dom->fc_info->level_set_addr) {
+ iowrite32(level, dom->fc_info->level_set_addr);
+ scmi_perf_fc_ring_db(dom->fc_info->level_set_db);
+ return 0;
+ }
+
+ return scmi_perf_mb_level_set(handle, domain, level, poll);
+}
+
+static int scmi_perf_mb_level_get(const struct scmi_handle *handle, u32 domain,
u32 *level, bool poll)
{
int ret;
@@ -387,6 +469,20 @@ static int scmi_perf_level_get(const struct scmi_handle *handle, u32 domain,
return ret;
}
+static int scmi_perf_level_get(const struct scmi_handle *handle, u32 domain,
+ u32 *level, bool poll)
+{
+ struct scmi_perf_info *pi = handle->perf_priv;
+ struct perf_dom_info *dom = pi->dom_info + domain;
+
+ if (dom->fc_info && dom->fc_info->level_get_addr) {
+ *level = ioread32(dom->fc_info->level_get_addr);
+ return 0;
+ }
+
+ return scmi_perf_mb_level_get(handle, domain, level, poll);
+}
+
static bool scmi_perf_fc_size_is_valid(u32 msg, u32 size)
{
if ((msg == PERF_LEVEL_GET || msg == PERF_LEVEL_SET) && size == 4)
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 1/5] firmware: arm_scmi: Add discovery of SCMI v2.0 performance fastchannels
From: Sudeep Holla @ 2019-08-06 17:02 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Peng Fan, Etienne Carriere, Souvik Chakravarty, wesleys,
Ionela Voinescu, aidapala, linux-kernel, Saeed Nowshadi,
Quentin Perret, Bo Zhang, Felix Burton, Jim Quinlan, Sudeep Holla,
pajay, Gaku Inami, Volodymyr Babchuk, Chris Redpath
In-Reply-To: <20190806170208.6787-1-sudeep.holla@arm.com>
SCMI v2.0 adds support for "FastChannel", a lightweight unidirectional
channel that is dedicated to a single SCMI message type for controlling
a specific platform resource. They do not use a message header as they
are specialized for a single message.
Only PERFORMANCE_LIMITS_{SET,GET} and PERFORMANCE_LEVEL_{SET,GET}
commands are supported over fastchannels. As they are optional, they
need to be discovered by PERFORMANCE_DESCRIBE_FASTCHANNEL command.
Further {LIMIT,LEVEL}_SET commands can have optional doorbell support.
Add support for discovery of these fastchannels.
Cc: Ionela Voinescu <Ionela.Voinescu@arm.com>
Cc: Chris Redpath <Chris.Redpath@arm.com>
Cc: Quentin Perret <Quentin.Perret@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/firmware/arm_scmi/perf.c | 153 ++++++++++++++++++++++++++++++-
1 file changed, 149 insertions(+), 4 deletions(-)
diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
index 3c8ae7cc35de..6cce3e82e81e 100644
--- a/drivers/firmware/arm_scmi/perf.c
+++ b/drivers/firmware/arm_scmi/perf.c
@@ -5,7 +5,9 @@
* Copyright (C) 2018 ARM Ltd.
*/
+#include <linux/bits.h>
#include <linux/of.h>
+#include <linux/io.h>
#include <linux/platform_device.h>
#include <linux/pm_opp.h>
#include <linux/sort.h>
@@ -21,6 +23,7 @@ enum scmi_performance_protocol_cmd {
PERF_LEVEL_GET = 0x8,
PERF_NOTIFY_LIMITS = 0x9,
PERF_NOTIFY_LEVEL = 0xa,
+ PERF_DESCRIBE_FASTCHANNEL = 0xb,
};
struct scmi_opp {
@@ -44,6 +47,7 @@ struct scmi_msg_resp_perf_domain_attributes {
#define SUPPORTS_SET_PERF_LVL(x) ((x) & BIT(30))
#define SUPPORTS_PERF_LIMIT_NOTIFY(x) ((x) & BIT(29))
#define SUPPORTS_PERF_LEVEL_NOTIFY(x) ((x) & BIT(28))
+#define SUPPORTS_PERF_FASTCHANNELS(x) ((x) & BIT(27))
__le32 rate_limit_us;
__le32 sustained_freq_khz;
__le32 sustained_perf_level;
@@ -87,17 +91,56 @@ struct scmi_msg_resp_perf_describe_levels {
} opp[0];
};
+struct scmi_perf_get_fc_info {
+ __le32 domain;
+ __le32 message_id;
+};
+
+struct scmi_msg_resp_perf_desc_fc {
+ __le32 attr;
+#define SUPPORTS_DOORBELL(x) ((x) & BIT(0))
+#define DOORBELL_REG_WIDTH(x) FIELD_GET(GENMASK(2, 1), (x))
+ __le32 rate_limit;
+ __le32 chan_addr_low;
+ __le32 chan_addr_high;
+ __le32 chan_size;
+ __le32 db_addr_low;
+ __le32 db_addr_high;
+ __le32 db_set_lmask;
+ __le32 db_set_hmask;
+ __le32 db_preserve_lmask;
+ __le32 db_preserve_hmask;
+};
+
+struct scmi_fc_db_info {
+ int width;
+ u64 set;
+ u64 mask;
+ void __iomem *addr;
+};
+
+struct scmi_fc_info {
+ void __iomem *level_set_addr;
+ void __iomem *limit_set_addr;
+ void __iomem *level_get_addr;
+ void __iomem *limit_get_addr;
+ struct scmi_fc_db_info *level_set_db;
+ struct scmi_fc_db_info *limit_set_db;
+};
+
struct perf_dom_info {
bool set_limits;
bool set_perf;
bool perf_limit_notify;
bool perf_level_notify;
+ bool perf_fastchannels;
u32 opp_count;
u32 sustained_freq_khz;
u32 sustained_perf_level;
u32 mult_factor;
char name[SCMI_MAX_STR_SIZE];
struct scmi_opp opp[MAX_OPPS];
+ struct scmi_fc_info *fc_info;
};
struct scmi_perf_info {
@@ -162,6 +205,7 @@ scmi_perf_domain_attributes_get(const struct scmi_handle *handle, u32 domain,
dom_info->set_perf = SUPPORTS_SET_PERF_LVL(flags);
dom_info->perf_limit_notify = SUPPORTS_PERF_LIMIT_NOTIFY(flags);
dom_info->perf_level_notify = SUPPORTS_PERF_LEVEL_NOTIFY(flags);
+ dom_info->perf_fastchannels = SUPPORTS_PERF_FASTCHANNELS(flags);
dom_info->sustained_freq_khz =
le32_to_cpu(attr->sustained_freq_khz);
dom_info->sustained_perf_level =
@@ -250,7 +294,7 @@ scmi_perf_describe_levels_get(const struct scmi_handle *handle, u32 domain,
}
static int scmi_perf_limits_set(const struct scmi_handle *handle, u32 domain,
- u32 max_perf, u32 min_perf)
+ u32 max_perf, u32 min_perf)
{
int ret;
struct scmi_xfer *t;
@@ -273,7 +317,7 @@ static int scmi_perf_limits_set(const struct scmi_handle *handle, u32 domain,
}
static int scmi_perf_limits_get(const struct scmi_handle *handle, u32 domain,
- u32 *max_perf, u32 *min_perf)
+ u32 *max_perf, u32 *min_perf)
{
int ret;
struct scmi_xfer *t;
@@ -299,7 +343,7 @@ static int scmi_perf_limits_get(const struct scmi_handle *handle, u32 domain,
}
static int scmi_perf_level_set(const struct scmi_handle *handle, u32 domain,
- u32 level, bool poll)
+ u32 level, bool poll)
{
int ret;
struct scmi_xfer *t;
@@ -322,7 +366,7 @@ static int scmi_perf_level_set(const struct scmi_handle *handle, u32 domain,
}
static int scmi_perf_level_get(const struct scmi_handle *handle, u32 domain,
- u32 *level, bool poll)
+ u32 *level, bool poll)
{
int ret;
struct scmi_xfer *t;
@@ -343,6 +387,104 @@ static int scmi_perf_level_get(const struct scmi_handle *handle, u32 domain,
return ret;
}
+static bool scmi_perf_fc_size_is_valid(u32 msg, u32 size)
+{
+ if ((msg == PERF_LEVEL_GET || msg == PERF_LEVEL_SET) && size == 4)
+ return true;
+ if ((msg == PERF_LIMITS_GET || msg == PERF_LIMITS_SET) && size == 8)
+ return true;
+ return false;
+}
+
+static void
+scmi_perf_domain_desc_fc(const struct scmi_handle *handle, u32 domain,
+ u32 message_id, void __iomem **p_addr,
+ struct scmi_fc_db_info **p_db)
+{
+ int ret;
+ u32 flags;
+ u64 phys_addr;
+ u8 size;
+ void __iomem *addr;
+ struct scmi_xfer *t;
+ struct scmi_fc_db_info *db;
+ struct scmi_perf_get_fc_info *info;
+ struct scmi_msg_resp_perf_desc_fc *resp;
+
+ if (!p_addr)
+ return;
+
+ ret = scmi_xfer_get_init(handle, PERF_DESCRIBE_FASTCHANNEL,
+ SCMI_PROTOCOL_PERF,
+ sizeof(*info), sizeof(*resp), &t);
+ if (ret)
+ return;
+
+ info = t->tx.buf;
+ info->domain = cpu_to_le32(domain);
+ info->message_id = cpu_to_le32(message_id);
+
+ ret = scmi_do_xfer(handle, t);
+ if (ret)
+ goto err_xfer;
+
+ resp = t->rx.buf;
+ flags = le32_to_cpu(resp->attr);
+ size = le32_to_cpu(resp->chan_size);
+ if (!scmi_perf_fc_size_is_valid(message_id, size))
+ goto err_xfer;
+
+ phys_addr = le32_to_cpu(resp->chan_addr_low);
+ phys_addr |= (u64)le32_to_cpu(resp->chan_addr_high) << 32;
+ addr = devm_ioremap(handle->dev, phys_addr, size);
+ if (!addr)
+ goto err_xfer;
+ *p_addr = addr;
+
+ if (p_db && SUPPORTS_DOORBELL(flags)) {
+ db = devm_kzalloc(handle->dev, sizeof(*db), GFP_KERNEL);
+ if (!db)
+ goto err_xfer;
+
+ size = 1 << DOORBELL_REG_WIDTH(flags);
+ phys_addr = le32_to_cpu(resp->db_addr_low);
+ phys_addr |= (u64)le32_to_cpu(resp->db_addr_high) << 32;
+ addr = devm_ioremap(handle->dev, phys_addr, size);
+ if (!addr)
+ goto err_xfer;
+
+ db->addr = addr;
+ db->width = size;
+ db->set = le32_to_cpu(resp->db_set_lmask);
+ db->set |= (u64)le32_to_cpu(resp->db_set_hmask) << 32;
+ db->mask = le32_to_cpu(resp->db_preserve_lmask);
+ db->mask |= (u64)le32_to_cpu(resp->db_preserve_hmask) << 32;
+ *p_db = db;
+ }
+err_xfer:
+ scmi_xfer_put(handle, t);
+}
+
+static void scmi_perf_domain_init_fc(const struct scmi_handle *handle,
+ u32 domain, struct scmi_fc_info **p_fc)
+{
+ struct scmi_fc_info *fc;
+
+ fc = devm_kzalloc(handle->dev, sizeof(*fc), GFP_KERNEL);
+ if (!fc)
+ return;
+
+ scmi_perf_domain_desc_fc(handle, domain, PERF_LEVEL_SET,
+ &fc->level_set_addr, &fc->level_set_db);
+ scmi_perf_domain_desc_fc(handle, domain, PERF_LEVEL_GET,
+ &fc->level_get_addr, NULL);
+ scmi_perf_domain_desc_fc(handle, domain, PERF_LIMITS_SET,
+ &fc->limit_set_addr, &fc->limit_set_db);
+ scmi_perf_domain_desc_fc(handle, domain, PERF_LIMITS_GET,
+ &fc->limit_get_addr, NULL);
+ *p_fc = fc;
+}
+
/* Device specific ops */
static int scmi_dev_domain_id(struct device *dev)
{
@@ -494,6 +636,9 @@ static int scmi_perf_protocol_init(struct scmi_handle *handle)
scmi_perf_domain_attributes_get(handle, domain, dom);
scmi_perf_describe_levels_get(handle, domain, dom);
+
+ if (dom->perf_fastchannels)
+ scmi_perf_domain_init_fc(handle, domain, &dom->fc_info);
}
handle->perf_ops = &perf_ops;
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 0/5] firmware: arm_scmi: add SCMI v2.0 fastchannels and reset protocol support
From: Sudeep Holla @ 2019-08-06 17:02 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Peng Fan, Etienne Carriere, Souvik Chakravarty, wesleys, aidapala,
linux-kernel, Saeed Nowshadi, Bo Zhang, Felix Burton, Jim Quinlan,
Sudeep Holla, pajay, Gaku Inami, Volodymyr Babchuk
SCMI v2.0[1] released recently adds support for:
1. Performance protocol fast channels
2. Reset Management Protocol
among several other features.
This series adds support for the above 2.
The code is based on the cleanup[2] and Rx/async/delayed response series[3]
and is available @[4]
--
Regards,
Sudeep
v1[5]->v2:
- Changed the macro SCMI_PERF_FC_RING_DB to use do {} while(0)
- Renamed RESET_ARM_SCMI to RESET_SCMI and reworded Kconfig text
- Dropped unused struct device pointer from scmi_reset_data
- Added to_scmi_handle which helped to remove some repetitive code
- Fixed some doxygen comments
- Initialised rcdev.nr_resets
- Fixed MODULE_DESCRIPTION
[1] http://infocenter.arm.com/help/topic/com.arm.doc.den0056b/DEN0056B_System_Control_and_Management_Interface_v2_0.pdf
[2] https://lore.kernel.org/lkml/20190726134531.8928-1-sudeep.holla@arm.com
[3] https://lore.kernel.org/lkml/20190726135138.9858-1-sudeep.holla@arm.com/
[4] git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git scmi_updates
[5] https://lore.kernel.org/lkml/20190726135954.11078-1-sudeep.holla@arm.com/
Sudeep Holla (5):
firmware: arm_scmi: Add discovery of SCMI v2.0 performance fastchannels
firmware: arm_scmi: Make use SCMI v2.0 fastchannel for performance protocol
dt-bindings: arm: Extend SCMI to support new reset protocol
firmware: arm_scmi: Add RESET protocol in SCMI v2.0
reset: Add support for resets provided by SCMI
.../devicetree/bindings/arm/arm,scmi.txt | 17 ++
MAINTAINERS | 1 +
drivers/firmware/arm_scmi/Makefile | 2 +-
drivers/firmware/arm_scmi/perf.c | 257 +++++++++++++++++-
drivers/firmware/arm_scmi/reset.c | 231 ++++++++++++++++
drivers/reset/Kconfig | 11 +
drivers/reset/Makefile | 1 +
drivers/reset/reset-scmi.c | 126 +++++++++
include/linux/scmi_protocol.h | 26 ++
9 files changed, 663 insertions(+), 9 deletions(-)
create mode 100644 drivers/firmware/arm_scmi/reset.c
create mode 100644 drivers/reset/reset-scmi.c
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] arm64: Clarify when cpu_enable() is called
From: Mark Brown @ 2019-08-06 17:00 UTC (permalink / raw)
To: Will Deacon, Catalin Marinas; +Cc: Mark Brown, linux-arm-kernel
Strengthen the wording in the documentation for cpu_enable() to make it
more obvious to readers not already familiar with the code when the core
will call this callback and that this is intentional.
Signed-off-by: Mark Brown <broonie@kernel.org>
---
arch/arm64/include/asm/cpufeature.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index cf65a47ee6b4..3d8afcf687d9 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -289,9 +289,9 @@ struct arm64_cpu_capabilities {
u16 type;
bool (*matches)(const struct arm64_cpu_capabilities *caps, int scope);
/*
- * Take the appropriate actions to enable this capability for this CPU.
- * For each successfully booted CPU, this method is called for each
- * globally detected capability.
+ * Take the appropriate actions to configure this capability for this
+ * CPU. This will be called on all CPUs in the system if the
+ * capability is detected anywhere in the system.
*/
void (*cpu_enable)(const struct arm64_cpu_capabilities *cap);
union {
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH] ARM: dts: add touchkey nodes for midas
From: Denis 'GNUtoo' Carikli @ 2019-08-06 16:57 UTC (permalink / raw)
To: Mark Rutland
Cc: Simon Shields, Krzysztof Kozlowski, Paul Kocialkowski,
Rob Herring, Denis 'GNUtoo' Carikli, linux-arm-kernel
From: Simon Shields <simon@lineageos.org>
this patch adds the fixed VTOUCH_3.3V regulator and configures
the touchkey node + i2c-gpio node.
Signed-off-by: Simon Shields <simon@lineageos.org>
GNUtoo@cyberdimension.org: Fixed keycodes.
Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>
---
arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi | 4 +++
arch/arm/boot/dts/exynos4412-midas.dtsi | 29 +++++++++++++++++++++
arch/arm/boot/dts/exynos4412-n710x.dts | 4 +++
3 files changed, 37 insertions(+)
diff --git a/arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi b/arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi
index ce87d2ff27aa..e71f103ab940 100644
--- a/arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi
+++ b/arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi
@@ -166,5 +166,9 @@
&s5c73m3 {
standby-gpios = <&gpm0 1 GPIO_ACTIVE_LOW>; /* ISP_STANDBY */
vdda-supply = <&ldo17_reg>;
+};
+
+&touchkey_reg {
+ gpio = <&gpm0 0 GPIO_ACTIVE_HIGH>;
status = "okay";
};
diff --git a/arch/arm/boot/dts/exynos4412-midas.dtsi b/arch/arm/boot/dts/exynos4412-midas.dtsi
index 83be3a797411..797e8de40580 100644
--- a/arch/arm/boot/dts/exynos4412-midas.dtsi
+++ b/arch/arm/boot/dts/exynos4412-midas.dtsi
@@ -13,6 +13,7 @@
#include "exynos4412.dtsi"
#include "exynos4412-ppmu-common.dtsi"
#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
#include <dt-bindings/interrupt-controller/irq.h>
#include <dt-bindings/clock/maxim,max77686.h>
#include <dt-bindings/pinctrl/samsung.h>
@@ -92,6 +93,15 @@
enable-active-high;
};
+ touchkey_reg: voltage-regulator-4 {
+ compatible = "regulator-fixed";
+ regulator-name = "VTOUCH_3.3V";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ enable-active-high;
+ status = "disabled";
+ };
+
gpio-keys {
compatible = "gpio-keys";
pinctrl-names = "default";
@@ -197,6 +207,25 @@
};
};
+ i2c_touchkey: i2c-gpio-4 {
+ compatible = "i2c-gpio";
+ sda-gpios = <&gpl0 2 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+ scl-gpios = <&gpl0 1 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+ i2c-gpio,delay-us = <2>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ touchkey@20 {
+ compatible = "cypress,midas-touchkey";
+ reg = <0x20>;
+ vdd-supply = <&touchkey_reg>;
+ vcc-supply = <&ldo5_reg>;
+ interrupt-parent = <&gpj0>;
+ interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
+ linux,keycodes = <KEY_BACK KEY_MENU>;
+ };
+ };
+
i2c-mhl {
compatible = "i2c-gpio";
gpios = <&gpf0 4 GPIO_ACTIVE_HIGH>, <&gpf0 6 GPIO_ACTIVE_HIGH>;
diff --git a/arch/arm/boot/dts/exynos4412-n710x.dts b/arch/arm/boot/dts/exynos4412-n710x.dts
index fe2bfd76cc4e..6acb19d2bae2 100644
--- a/arch/arm/boot/dts/exynos4412-n710x.dts
+++ b/arch/arm/boot/dts/exynos4412-n710x.dts
@@ -71,5 +71,9 @@
&s5c73m3 {
standby-gpios = <&gpm0 6 GPIO_ACTIVE_LOW>; /* ISP_STANDBY */
vdda-supply = <&cam_vdda_reg>;
+};
+
+&touchkey_reg {
+ gpio = <&gpm0 5 GPIO_ACTIVE_HIGH>;
status = "okay";
};
--
2.22.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH 04/22] media: Move v4l2_fwnode_parse_link from v4l2 to driver base
From: Steve Longerbeam @ 2019-08-06 16:53 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Heikki Krogerus, Rafael J. Wysocki, Laurent Pinchart, Shawn Guo,
Fabio Estevam, open list:STAGING SUBSYSTEM, Mauro Carvalho Chehab,
Michal Simek, open list:ACPI, NXP Linux Team, Philipp Zabel,
Len Brown, Sakari Ailus, Sascha Hauer, Thomas Gleixner,
Andy Shevchenko, moderated list:ARM/ZYNQ ARCHITECTURE, Hyun Kwon,
Greg Kroah-Hartman, open list, Jacopo Mondi,
Pengutronix Kernel Team, Hans Verkuil, Linux Media Mailing List,
Enrico Weigelt
In-Reply-To: <CAHp75VcOh8bOf_s6t0ehwGtcYn64QFGj303SVvpHrztEOhTRgg@mail.gmail.com>
Hi Andy,
On 8/5/19 11:41 PM, Andy Shevchenko wrote:
> On Tue, Aug 6, 2019 at 2:37 AM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>> There is nothing v4l2-specific about v4l2_fwnode_{parse|put}_link().
>> Make these functions more generally available by moving them to driver
>> base, with the appropriate name changes to the functions and struct.
>>
>> In the process embed a 'struct fwnode_endpoint' in 'struct fwnode_link'
>> for both sides of the link, and make use of fwnode_graph_parse_endpoint()
>> to fully parse both endpoints. Rename members local_node and
>> remote_node to more descriptive local_port_parent and
>> remote_port_parent.
>>
> May I ask if it's going to be used outside of v4l2?
It could be, I can see fwnode_graph_parse_link() being useful in DRM,
for example. But at this, only media core and v4l2 are making use of it.
This patch was created so that fwnode links can be parsed in a new media
core function that forms media links from fwnode links.
The full patchset doesn't seem to be up yet, but see [1] for the cover
letter.
Steve
[1] https://www.mail-archive.com/linux-media@vger.kernel.org/msg148910.html
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] dma-mapping: fix page attributes for dma_mmap_*
From: Russell King - ARM Linux admin @ 2019-08-06 16:48 UTC (permalink / raw)
To: Will Deacon
Cc: Shawn Anastasio, Michael Ellerman, linux-kernel, iommu,
Catalin Marinas, linuxppc-dev, Christoph Hellwig,
linux-arm-kernel, Robin Murphy
In-Reply-To: <20190806164503.GD1330@shell.armlinux.org.uk>
On Tue, Aug 06, 2019 at 05:45:03PM +0100, Russell King - ARM Linux admin wrote:
> On Tue, Aug 06, 2019 at 05:08:54PM +0100, Will Deacon wrote:
> > On Sat, Aug 03, 2019 at 08:48:12AM +0200, Christoph Hellwig wrote:
> > > On Fri, Aug 02, 2019 at 11:38:03AM +0100, Will Deacon wrote:
> > > >
> > > > So this boils down to a terminology mismatch. The Arm architecture doesn't have
> > > > anything called "write combine", so in Linux we instead provide what the Arm
> > > > architecture calls "Normal non-cacheable" memory for pgprot_writecombine().
> > > > Amongst other things, this memory type permits speculation, unaligned accesses
> > > > and merging of writes. I found something in the architecture spec about
> > > > non-cachable memory, but it's written in Armglish[1].
> > > >
> > > > pgprot_noncached(), on the other hand, provides what the architecture calls
> > > > Strongly Ordered or Device-nGnRnE memory. This is intended for mapping MMIO
> > > > (i.e. PCI config space) and therefore forbids speculation, preserves access
> > > > size, requires strict alignment and also forces write responses to come from
> > > > the endpoint.
> > > >
> > > > I think the naming mismatch is historical, but on arm64 we wanted to use the
> > > > same names as arm32 so that any drivers using these things directly would get
> > > > the same behaviour.
> > >
> > > That all makes sense, but it totally needs a comment. I'll try to draft
> > > one based on this. I've also looked at the arm32 code a bit more, and
> > > it seems arm always (?) supported Normal non-cacheable attribute, but
> > > Linux only optionally uses it for arm v6+ because of fears of drivers
> > > missing barriers.
> >
> > I think it was also to do with aliasing, but I don't recall all of the
> > details.
>
> ARMv6+ is where the architecture significantly changed to introduce
> the idea of [Normal, Device, Strongly Ordered] where Normal has the
> cache attributes.
>
> Before that, we had just "uncached/unbuffered, uncached/buffered,
> cached/unbuffered, cached/buffered" modes.
>
> The write buffer (enabled by buffered modes) has no architected
> guarantees about how long writes will sit in it, and there is only
> the "drain write buffer" instruction to push writes out.
>
> Up to and including ARMv5, we took the easy approach of just using
> the "uncached/unbuffered" mode since that is (a) the safest, and (b)
> avoids write buffers that alias when there are multiple different
> mappings.
>
> We could have used a different approach, making all IO writes contain
> a "drain write buffer" instruction, and map DMA memory as "buffered",
> but as there were no Linux barriers defined to order memory accesses
> to DMA memory (so, for example, ring buffers can be updated in the
> correct order) back in those days, using the uncached/unbuffered mode
> was the sanest and most reliable solution.
>
> >
> > > The other really weird things is that in arm32
> > > pgprot_dmacoherent incudes the L_PTE_XN bit, which from my understanding
> > > is the no-execture bit, but pgprot_writecombine does not. This seems to
> > > not very unintentional. So minus that the whole DMA_ATTR_WRITE_COMBІNE
> > > seems to be about flagging old arm specific drivers as having the proper
> > > barriers in places and otherwise is a no-op.
> >
> > I think it only matters for Armv7 CPUs, but yes, we should probably be
> > setting L_PTE_XN for both of these memory types.
>
> Conventionally, pgprot_writecombine() has only been used to change
> the memory type and not the permissions. Since writecombine memory
> is still capable of being executed, I don't see any reason to set XN
> for it.
>
> If the user wishes to mmap() using PROT_READ|PROT_EXEC, then is there
> really a reason for writecombine to set XN overriding the user?
>
> That said, pgprot_writecombine() is mostly used for framebuffers, which
> arguably shouldn't be executable anyway - but who'd want to mmap() the
> framebuffer with PROT_EXEC?
>
> >
> > > Here is my tentative plan:
> > >
> > > - respin this patch with a small fix to handle the
> > > DMA_ATTR_NON_CONSISTENT (as in ignore it unless actually supported),
> > > but keep the name as-is to avoid churn. This should allow 5.3
> > > inclusion and backports
> > > - remove DMA_ATTR_WRITE_COMBINE support from mips, probably also 5.3
> > > material.
> > > - move all architectures but arm over to just define
> > > pgprot_dmacoherent, including a comment with the above explanation
> > > for arm64.
> >
> > That would be great, thanks.
> >
> > > - make DMA_ATTR_WRITE_COMBINE a no-op and schedule it for removal,
> > > thus removing the last instances of arch_dma_mmap_pgprot
> >
> > All sounds good to me, although I suppose 32-bit Arm platforms without
> > CONFIG_ARM_DMA_MEM_BUFFERABLE may run into issues if DMA_ATTR_WRITE_COMBINE
> > disappears. Only one way to find out...
>
> Looking at the results of grep, I think only OMAP2+ and Exynos may be
> affected.
>
> However, removing writecombine support from the DMA API is going to
> have a huge impact for framebuffers on earlier ARMs - that's where we
> do expect framebuffers to be mapped "uncached/buffered" for performance
> reasons and not "uncached/unbuffered". It's quite literally the
> difference between console scrolling being usable and totally unusable.
>
> Given what I've said above, switching to using buffered mode for normal
> DMA mappings is data-corrupting risky - as in your filesystem could get
> fried. I don't think we should play fast and loose with people's data
> by randomly changing that "because we'd like to", and I don't see that
> screwing the console is really an option either.
Sorry, I forgot to explain - the reason is dma_alloc_writecombine()
internally uses DMA_ATTR_WRITE_COMBINE, which I'd forgotten about
when grepping - so there's potentially way more users than my greps
above found.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 3/5] x86: Kconfig: Remove CONFIG_NODES_SPAN_OTHER_NODES
From: Hoan Tran OS @ 2019-08-06 16:47 UTC (permalink / raw)
To: Thomas Gleixner, Catalin Marinas, Will Deacon
Cc: Michal Hocko, Benjamin Herrenschmidt, Heiko Carstens,
open list:MEMORY MANAGEMENT, Paul Mackerras, H . Peter Anvin,
sparclinux@vger.kernel.org, Alexander Duyck,
linux-s390@vger.kernel.org, Michael Ellerman, x86@kernel.org,
Mike Rapoport, Christian Borntraeger, Ingo Molnar,
Vlastimil Babka, Open Source Submission, Pavel Tatashin,
Vasily Gorbik, Borislav Petkov,
linux-arm-kernel@lists.infradead.org, Oscar Salvador,
linux-kernel@vger.kernel.org, Andrew Morton,
linuxppc-dev@lists.ozlabs.org, David S . Miller
In-Reply-To: <alpine.DEB.2.21.1907152042110.1767@nanos.tec.linutronix.de>
Hi Thomas,
On 7/15/19 11:43 AM, Thomas Gleixner wrote:
> On Thu, 11 Jul 2019, Hoan Tran OS wrote:
>
>> Remove CONFIG_NODES_SPAN_OTHER_NODES as it's enabled
>> by default with NUMA.
>
> As I told you before this does not mention that the option is now enabled
> even for x86(32bit) configurations which did not enable it before and does
> not longer depend on X86_64_ACPI_NUMA.
Agreed, let me add it into this patch description.
>
> And there is still no rationale why this makes sense.
>
As we know about the memmap_init_zone() function, it is used to
initialize all pages. During initializing, early_pfn_in_nid() function
makes sure the page is in the same node id. Otherwise,
memmap_init_zone() only checks the page validity. It won't work with
node memory spans across the others.
The option CONFIG_NODES_SPAN_OTHER_NODES is only used to enable
early_pfn_in_nid() function.
It occurs during boot-time and won't affect the run-time performance.
And I saw the majority NUMA architectures enable this option by default
with NUMA.
Thanks and Regards
Hoan
> Thanks,
>
> tglx
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] dma-mapping: fix page attributes for dma_mmap_*
From: Russell King - ARM Linux admin @ 2019-08-06 16:45 UTC (permalink / raw)
To: Will Deacon
Cc: Shawn Anastasio, Michael Ellerman, linuxppc-dev, linux-kernel,
iommu, Catalin Marinas, Robin Murphy, Christoph Hellwig,
linux-arm-kernel
In-Reply-To: <20190806160854.htk67msiyadlrl4m@willie-the-truck>
On Tue, Aug 06, 2019 at 05:08:54PM +0100, Will Deacon wrote:
> On Sat, Aug 03, 2019 at 08:48:12AM +0200, Christoph Hellwig wrote:
> > On Fri, Aug 02, 2019 at 11:38:03AM +0100, Will Deacon wrote:
> > >
> > > So this boils down to a terminology mismatch. The Arm architecture doesn't have
> > > anything called "write combine", so in Linux we instead provide what the Arm
> > > architecture calls "Normal non-cacheable" memory for pgprot_writecombine().
> > > Amongst other things, this memory type permits speculation, unaligned accesses
> > > and merging of writes. I found something in the architecture spec about
> > > non-cachable memory, but it's written in Armglish[1].
> > >
> > > pgprot_noncached(), on the other hand, provides what the architecture calls
> > > Strongly Ordered or Device-nGnRnE memory. This is intended for mapping MMIO
> > > (i.e. PCI config space) and therefore forbids speculation, preserves access
> > > size, requires strict alignment and also forces write responses to come from
> > > the endpoint.
> > >
> > > I think the naming mismatch is historical, but on arm64 we wanted to use the
> > > same names as arm32 so that any drivers using these things directly would get
> > > the same behaviour.
> >
> > That all makes sense, but it totally needs a comment. I'll try to draft
> > one based on this. I've also looked at the arm32 code a bit more, and
> > it seems arm always (?) supported Normal non-cacheable attribute, but
> > Linux only optionally uses it for arm v6+ because of fears of drivers
> > missing barriers.
>
> I think it was also to do with aliasing, but I don't recall all of the
> details.
ARMv6+ is where the architecture significantly changed to introduce
the idea of [Normal, Device, Strongly Ordered] where Normal has the
cache attributes.
Before that, we had just "uncached/unbuffered, uncached/buffered,
cached/unbuffered, cached/buffered" modes.
The write buffer (enabled by buffered modes) has no architected
guarantees about how long writes will sit in it, and there is only
the "drain write buffer" instruction to push writes out.
Up to and including ARMv5, we took the easy approach of just using
the "uncached/unbuffered" mode since that is (a) the safest, and (b)
avoids write buffers that alias when there are multiple different
mappings.
We could have used a different approach, making all IO writes contain
a "drain write buffer" instruction, and map DMA memory as "buffered",
but as there were no Linux barriers defined to order memory accesses
to DMA memory (so, for example, ring buffers can be updated in the
correct order) back in those days, using the uncached/unbuffered mode
was the sanest and most reliable solution.
>
> > The other really weird things is that in arm32
> > pgprot_dmacoherent incudes the L_PTE_XN bit, which from my understanding
> > is the no-execture bit, but pgprot_writecombine does not. This seems to
> > not very unintentional. So minus that the whole DMA_ATTR_WRITE_COMBІNE
> > seems to be about flagging old arm specific drivers as having the proper
> > barriers in places and otherwise is a no-op.
>
> I think it only matters for Armv7 CPUs, but yes, we should probably be
> setting L_PTE_XN for both of these memory types.
Conventionally, pgprot_writecombine() has only been used to change
the memory type and not the permissions. Since writecombine memory
is still capable of being executed, I don't see any reason to set XN
for it.
If the user wishes to mmap() using PROT_READ|PROT_EXEC, then is there
really a reason for writecombine to set XN overriding the user?
That said, pgprot_writecombine() is mostly used for framebuffers, which
arguably shouldn't be executable anyway - but who'd want to mmap() the
framebuffer with PROT_EXEC?
>
> > Here is my tentative plan:
> >
> > - respin this patch with a small fix to handle the
> > DMA_ATTR_NON_CONSISTENT (as in ignore it unless actually supported),
> > but keep the name as-is to avoid churn. This should allow 5.3
> > inclusion and backports
> > - remove DMA_ATTR_WRITE_COMBINE support from mips, probably also 5.3
> > material.
> > - move all architectures but arm over to just define
> > pgprot_dmacoherent, including a comment with the above explanation
> > for arm64.
>
> That would be great, thanks.
>
> > - make DMA_ATTR_WRITE_COMBINE a no-op and schedule it for removal,
> > thus removing the last instances of arch_dma_mmap_pgprot
>
> All sounds good to me, although I suppose 32-bit Arm platforms without
> CONFIG_ARM_DMA_MEM_BUFFERABLE may run into issues if DMA_ATTR_WRITE_COMBINE
> disappears. Only one way to find out...
Looking at the results of grep, I think only OMAP2+ and Exynos may be
affected.
However, removing writecombine support from the DMA API is going to
have a huge impact for framebuffers on earlier ARMs - that's where we
do expect framebuffers to be mapped "uncached/buffered" for performance
reasons and not "uncached/unbuffered". It's quite literally the
difference between console scrolling being usable and totally unusable.
Given what I've said above, switching to using buffered mode for normal
DMA mappings is data-corrupting risky - as in your filesystem could get
fried. I don't think we should play fast and loose with people's data
by randomly changing that "because we'd like to", and I don't see that
screwing the console is really an option either.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox