Devicetree
 help / color / mirror / Atom feed
* [PATCH v2 2/2] remoteproc/k3-dsp: Add support for C71x DSPs
From: Suman Anna @ 2020-05-21 15:16 UTC (permalink / raw)
  To: Bjorn Andersson, Rob Herring, Mathieu Poirier
  Cc: Lokesh Vutla, linux-remoteproc, devicetree, linux-arm-kernel,
	linux-kernel, Suman Anna
In-Reply-To: <20200521151636.28260-1-s-anna@ti.com>

The Texas Instrument's K3 J721E SoCs have a newer next-generation
C71x DSP Subsystem in the MAIN voltage domain in addition to the
previous generation C66x DSP subsystems. The C71x DSP subsystem is
based on the TMS320C71x DSP CorePac module. The C71x CPU is a true
64-bit machine including 64-bit memory addressing and single-cycle
64-bit base arithmetic operations and supports vector signal processing
providing a significant lift in DSP processing power over C66x DSPs.
J721E SoCs use a C711 (a one-core 512-bit vector width CPU core) DSP
that is cache coherent with the A72 Arm cores.

Each subsystem has one or more Fixed/Floating-Point DSP CPUs, with 32 KB
of L1P Cache, 48 KB of L1D SRAM that can be configured and partitioned as
either RAM and/or Cache, and 512 KB of L2 SRAM configurable as either RAM
and/or Cache. The CorePac also includes a Matrix Multiplication Accelerator
(MMA), a Stream Engine (SE) and a C71x Memory Management Unit (CMMU), an
Interrupt Controller (INTC) and a Powerdown Management Unit (PMU) modules.

Update the existing K3 DSP remoteproc driver to add support for this C71x
DSP subsystem. The firmware loading support is provided by using the newly
added 64-bit ELF loader support, and is limited to images using only
external DDR memory at the moment. The L1D and L2 SRAMs are used as scratch
memory when using as RAMs, and cannot be used for loadable segments. The
CMMU is also not supported to begin with, and the driver is designed to
treat the MMU as if it is in bypass mode.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
v2: 
 - k3_dsp_rproc_prepare/unprepare plugged in dynamically based on local reset,
   C71x doesn't use local resets
 - Dropped the sanity_check ops override, not needed on latest codebase
v1: https://patchwork.kernel.org/patch/11458595/

 drivers/remoteproc/ti_k3_dsp_remoteproc.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c b/drivers/remoteproc/ti_k3_dsp_remoteproc.c
index 610fbbf85ee6..2dbed316b6ac 100644
--- a/drivers/remoteproc/ti_k3_dsp_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c
@@ -406,8 +406,6 @@ static void *k3_dsp_rproc_da_to_va(struct rproc *rproc, u64 da, size_t len)
 }
 
 static const struct rproc_ops k3_dsp_rproc_ops = {
-	.prepare	= k3_dsp_rproc_prepare,
-	.unprepare	= k3_dsp_rproc_unprepare,
 	.start		= k3_dsp_rproc_start,
 	.stop		= k3_dsp_rproc_stop,
 	.kick		= k3_dsp_rproc_kick,
@@ -617,6 +615,10 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev)
 
 	rproc->has_iommu = false;
 	rproc->recovery_disabled = true;
+	if (data->uses_lreset) {
+		rproc->ops->prepare = k3_dsp_rproc_prepare;
+		rproc->ops->unprepare = k3_dsp_rproc_unprepare;
+	}
 	kproc = rproc->priv;
 	kproc->rproc = rproc;
 	kproc->dev = dev;
@@ -744,6 +746,12 @@ static const struct k3_dsp_mem_data c66_mems[] = {
 	{ .name = "l1dram", .dev_addr = 0xf00000 },
 };
 
+/* C71x cores only have a L1P Cache, there are no L1P SRAMs */
+static const struct k3_dsp_mem_data c71_mems[] = {
+	{ .name = "l2sram", .dev_addr = 0x800000 },
+	{ .name = "l1dram", .dev_addr = 0xe00000 },
+};
+
 static const struct k3_dsp_dev_data c66_data = {
 	.mems = c66_mems,
 	.num_mems = ARRAY_SIZE(c66_mems),
@@ -751,8 +759,16 @@ static const struct k3_dsp_dev_data c66_data = {
 	.uses_lreset = true,
 };
 
+static const struct k3_dsp_dev_data c71_data = {
+	.mems = c71_mems,
+	.num_mems = ARRAY_SIZE(c71_mems),
+	.boot_align_addr = SZ_2M,
+	.uses_lreset = false,
+};
+
 static const struct of_device_id k3_dsp_of_match[] = {
 	{ .compatible = "ti,j721e-c66-dsp", .data = &c66_data, },
+	{ .compatible = "ti,j721e-c71-dsp", .data = &c71_data, },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, k3_dsp_of_match);
-- 
2.26.0


^ permalink raw reply related

* [PATCH v2 1/2] dt-bindings: remoteproc: k3-dsp: Update bindings for C71x DSPs
From: Suman Anna @ 2020-05-21 15:16 UTC (permalink / raw)
  To: Bjorn Andersson, Rob Herring, Mathieu Poirier
  Cc: Lokesh Vutla, linux-remoteproc, devicetree, linux-arm-kernel,
	linux-kernel, Suman Anna, Rob Herring
In-Reply-To: <20200521151636.28260-1-s-anna@ti.com>

Some Texas Instruments K3 family of SoCs have one of more newer
generation TMS320C71x CorePac processor subsystem in addition to
the existing TMS320C66x CorePac processor subsystems. Update the
device tree bindings document for the C71x DSP devices.

The example is also updated to show the single C71 DSP present
on J721E SoCs.

Signed-off-by: Suman Anna <s-anna@ti.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
v2: 
 - Rebased patch, no changes to binding properties
 - Example additions indented one level to right as part of rebase and
   changes done in updated C66x bindings patch
 - Added Rob's Reviewed-by
v1: https://patchwork.kernel.org/patch/11458601/

 .../bindings/remoteproc/ti,k3-dsp-rproc.yaml  | 76 +++++++++++++++++--
 1 file changed, 68 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
index cdf649655838..47642015c884 100644
--- a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
@@ -27,9 +27,12 @@ description: |
 
 properties:
   compatible:
-    const: ti,j721e-c66-dsp
+    enum:
+      - ti,j721e-c66-dsp
+      - ti,j721e-c71-dsp
     description:
       Use "ti,j721e-c66-dsp" for C66x DSPs on K3 J721E SoCs
+      Use "ti,j721e-c71-dsp" for C71x DSPs on K3 J721E SoCs
 
   reg:
     description: |
@@ -37,18 +40,11 @@ properties:
       Each entry should have the memory region's start address
       and the size of the region, the representation matching
       the parent node's '#address-cells' and '#size-cells' values.
-    minItems: 3
-    maxItems: 3
 
   reg-names:
     description: |
       Should contain strings with the names of the specific internal
       memory regions, and should be defined in this order
-    maxItems: 3
-    items:
-      - const: l2sram
-      - const: l1pram
-      - const: l1dram
 
   ti,sci:
     $ref: /schemas/types.yaml#/definitions/phandle
@@ -121,6 +117,41 @@ properties:
       should be defined as per the generic bindings in,
       Documentation/devicetree/bindings/sram/sram.yaml
 
+if:
+  properties:
+    compatible:
+      enum:
+        - ti,j721e-c66-dsp
+then:
+  properties:
+    reg:
+      minItems: 3
+      maxItems: 3
+    reg-names:
+      minItems: 3
+      maxItems: 3
+      items:
+        - const: l2sram
+        - const: l1pram
+        - const: l1dram
+else:
+  if:
+    properties:
+      compatible:
+        enum:
+          - ti,j721e-c71-dsp
+  then:
+    properties:
+      reg:
+        minItems: 2
+        maxItems: 2
+      reg-names:
+        minItems: 2
+        maxItems: 2
+        items:
+          - const: l2sram
+          - const: l1dram
+
 required:
  - compatible
  - reg
@@ -160,6 +191,18 @@ examples:
                 reg = <0x00 0xa6100000 0x00 0xf00000>;
                 no-map;
             };
+
+            c71_0_dma_memory_region: c71-dma-memory@a8000000 {
+                compatible = "shared-dma-pool";
+                reg = <0x00 0xa8000000 0x00 0x100000>;
+                no-map;
+            };
+
+            c71_0_memory_region: c71-memory@a8100000 {
+                compatible = "shared-dma-pool";
+                reg = <0x00 0xa8100000 0x00 0xf00000>;
+                no-map;
+            };
         };
 
         cbass_main: bus@100000 {
@@ -167,6 +210,7 @@ examples:
             #address-cells = <2>;
             #size-cells = <2>;
             ranges = <0x00 0x00100000 0x00 0x00100000 0x00 0x00020000>, /* ctrl mmr */
+                     <0x00 0x64800000 0x00 0x64800000 0x00 0x00800000>, /* C71_0 */
                      <0x4d 0x80800000 0x4d 0x80800000 0x00 0x00800000>, /* C66_0 */
                      <0x4d 0x81800000 0x4d 0x81800000 0x00 0x00800000>; /* C66_1 */
 
@@ -186,5 +230,21 @@ examples:
                                 <&c66_0_memory_region>;
                 mboxes = <&mailbox0_cluster3 &mbox_c66_0>;
             };
+
+            /* J721E C71_0 DSP node */
+            c71_0: dsp@64800000 {
+                compatible = "ti,j721e-c71-dsp";
+                reg = <0x00 0x64800000 0x00 0x00080000>,
+                      <0x00 0x64e00000 0x00 0x0000c000>;
+                reg-names = "l2sram", "l1dram";
+                ti,sci = <&dmsc>;
+                ti,sci-dev-id = <15>;
+                ti,sci-proc-ids = <0x30 0xFF>;
+                resets = <&k3_reset 15 1>;
+                firmware-name = "j7-c71_0-fw";
+                memory-region = <&c71_0_dma_memory_region>,
+                                <&c71_0_memory_region>;
+                mboxes = <&mailbox0_cluster4 &mbox_c71_0>;
+            };
         };
     };
-- 
2.26.0


^ permalink raw reply related

* Re: [PATCH 1/2] dt-bindings: spi: Add Baikal-T1 System Boot SPI Controller binding
From: Serge Semin @ 2020-05-21 15:11 UTC (permalink / raw)
  To: Rob Herring
  Cc: Serge Semin, Mark Brown, Ramil Zaripov, Alexey Malahov,
	Thomas Bogendoerfer, Paul Burton, Ralf Baechle, John Garry,
	Chuanhong Guo, Tomer Maimon, Lee Jones, Miquel Raynal,
	Arnd Bergmann, open list:MIPS, linux-spi, devicetree,
	linux-kernel@vger.kernel.org
In-Reply-To: <CAL_JsqLLMh1LAvVXccyjLc4SqTAaPQ5LC7Nb6Q5ib8_3a0q6Ow@mail.gmail.com>

On Thu, May 21, 2020 at 08:57:10AM -0600, Rob Herring wrote:
> On Mon, May 18, 2020 at 3:27 PM Serge Semin
> <Sergey.Semin@baikalelectronics.ru> wrote:
> >
> > On Mon, May 18, 2020 at 09:26:59AM -0600, Rob Herring wrote:
> > > On Fri, May 08, 2020 at 12:36:20PM +0300, Serge Semin wrote:
> > > > Baikal-T1 Boot SPI is a part of the SoC System Controller and is
> > > > responsible for the system bootup from an external SPI flash. It's a DW
> > > > APB SSI-based SPI-controller with no interrupts, no DMA, with just one
> > > > native chip-select available and a single reference clock. Since Baikal-T1
> > > > SoC is normally booted up from an external SPI flash this SPI controller
> > > > in most of the cases is supposed to be connected to a single SPI-nor
> > > > flash. Additionally in order to provide a transparent from CPU point of
> > > > view initial code execution procedure the system designers created an IP
> > > > block which physically maps the SPI flash found at CS0 to a memory region.
> > > >
> > > > Co-developed-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> > > > Signed-off-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > > > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > > > Cc: Paul Burton <paulburton@kernel.org>
> > > > Cc: Ralf Baechle <ralf@linux-mips.org>
> > > > Cc: John Garry <john.garry@huawei.com>
> > > > Cc: Chuanhong Guo <gch981213@gmail.com>
> > > > Cc: Tomer Maimon <tmaimon77@gmail.com>
> > > > Cc: Lee Jones <lee.jones@linaro.org>
> > > > Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > Cc: Arnd Bergmann <arnd@arndb.de>
> > > > Cc: linux-mips@vger.kernel.org
> > > > Cc: linux-spi@vger.kernel.org
> > > > ---
> > > >  .../bindings/spi/baikal,bt1-sys-ssi.yaml      | 100 ++++++++++++++++++
> > > >  1 file changed, 100 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml b/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> > > > new file mode 100644
> > > > index 000000000000..d9d3257d78f4
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> > > > @@ -0,0 +1,100 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +# Copyright (C) 2020 BAIKAL ELECTRONICS, JSC
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/spi/baikal,bt1-sys-ssi.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Baikal-T1 System Boot SSI Controller
> > > > +
> > > > +description: |
> > > > +  Baikal-T1 System Controller includes a Boot SPI Controller, which is
> > > > +  responsible for loading chip bootup code from an external SPI flash. In order
> > > > +  to do this transparently from CPU point of view there is a dedicated IP block
> > > > +  mapping the 16MB flash to a dedicated MMIO range. The controller is based on
> > > > +  the DW APB SSI IP-core but equipped with very limited resources: no IRQ,
> > > > +  no DMA, a single native CS being necessarily connected to a 16MB SPI flash
> > > > +  (otherwise the system won't bootup from the flash), internal Tx/Rx FIFO of
> > > > +  just 8 bytes depth. Access to DW APB SSI controller registers is mutually
> > > > +  exclusive from normal MMIO interface and from physically mapped SPI Flash
> > > > +  memory. So either one or another way of using the controller functionality
> > > > +  can be enabled at a time.
> > > > +
> > > > +maintainers:
> > > > +  - Serge Semin <fancer.lancer@gmail.com>
> > > > +
> > > > +allOf:
> > > > +  - $ref: spi-controller.yaml#
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    const: baikal,bt1-sys-ssi
> > > > +
> > > > +  reg:
> > > > +    items:
> > > > +      - description: Baikal-T1 Boot Controller configuration registers
> > > > +      - description: Physically mapped SPI flash ROM found at CS0
> > > > +
> > > > +  reg-names:
> > > > +    items:
> > > > +      - const: config
> > > > +      - const: map
> > > > +
> > > > +  clocks:
> > > > +    description: SPI Controller reference clock source
> > >
> > > Can drop this.
> >
> > Ok.
> >
> > >
> > > > +    maxItems: 1
> > > > +
> > > > +  clock-names:
> > > > +    items:
> > > > +      - const: ssi_clk
> > > > +
> > > > +  num-cs:
> > > > +    const: 1
> > > > +
> > > > +patternProperties:
> > > > +  "^.*@[0-9a-f]+":
> > > > +    type: object
> > > > +    properties:
> > > > +      reg:
> > > > +        minimum: 0
> > > > +        maximum: 0
> > > > +
> > > > +      spi-rx-bus-width:
> > > > +        const: 1
> > > > +
> > > > +      spi-tx-bus-width:
> > > > +        const: 1
> > >
> > > What's the point of these 2 properties if they aren't required?
> >
> > Yes, they are optional, but this is a constraint on the bus-width parameters.
> > DW APB SSI provides a single laned Tx and Rx.
> 
> Are you just trying to keep someone from saying 'spi-tx-bus-width: 2'
> for example?

Right.

> 
> You could also say 'spi-tx-bus-width: false' here to disallow the
> property. I guess the above is fine.

Ok. If it's fine I'll leave them as is then. Right?

What about the next question you've asked:

> >
> > > +
> > > +unevaluatedProperties: false
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - reg-names
> >
> > > +  - "#address-cells"
> > > +  - "#size-cells"
> >
> > These 2 are required by spi-controller.yaml, so you can drop here.
> 
> Yes, "#address-cells" is required, but "#size-cells" isn't. Is this supposed to
> be like that?

As far as I can see in spi-controller.yaml, "#address-cells" is required, but
"#size-cells" isn't. Is this intentional?

-Sergey

> 
> Rob

^ permalink raw reply

* Re: [RFC v1 2/3] drivers: nvmem: Add driver for QTI qfprom-efuse support
From: Doug Anderson @ 2020-05-21 15:10 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: Ravi Kumar Bokka (Temp), Rob Herring, LKML,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Rajendra Nayak, Sai Prakash Ranjan, dhavalp, mturney, sparate,
	c_rbokka, mkurumel
In-Reply-To: <99f07eaa-d072-f391-098e-e6f7a50a1960@linaro.org>

Hi,

On Thu, May 21, 2020 at 8:01 AM Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
>
> On 20/05/2020 23:48, Doug Anderson wrote:
> >> Is this only applicable for corrected address space?
> > I guess I was proposing a two dts-node / two drive approach here.
> >
> > dts node #1:just covers the memory range for accessing the FEC-corrected data
> > driver #1: read-only and reads the FEC-corrected data
> >
> > dts node #2: covers the memory range that's_not_  the FEC-corrected
> > memory range.
> > driver #2: read-write.  reading reads uncorrected data
> >
> > Does that seem sane?
>
> I see your point but it does not make sense to have two node for same thing.

OK, so that sounds as if we want to go with the proposal where we
"deprecate the old driver and/or bindings and say that there really
should just be one node and one driver".

Would this be acceptable to you?

1. Officially mark the old bindings as deprecated.

2. Leave the old driver there to support the old deprecated bindings,
at least until everyone can be transferred over.  There seem to be
quite a few existing users of "qcom,qfprom" and we're supposed to make
an attempt at keeping the old device trees working, at least for a
little while.  Once everyone is transferred over we could decide to
delete the old driver.

3. We will have a totally new driver here.

4. A given device tree will _not_ be allowed to have both
"qcom,qfprom" specified and "qcom,SOC-qfprom" specified.  ...and by
"qcom,SOC-qfprom" I mean that SOC should be replaced by the SoC name,
so "qcom,sc7180-qfprom" or "qcom,sdm845-qfprom".  So once you switch
to the new node it replaces the old node.


> Isn't the raw address space reads used to for blowing and checking the
> fuses if they are blown correctly or not and software usage of these
> fuses should only be done from correct address space?
>
> the read interface to user should be always from corrected address space
> and write interface should be to raw address space.

Great.  That sounds right to me.  Presumably the driver could add some
sort of "debugfs" access to read the raw address space if needed.

-Doug

^ permalink raw reply

* Re: [PATCH v3 5/7] arm64: dts: mt8183: add usb and phy nodes
From: Matthias Brugger @ 2020-05-21 15:06 UTC (permalink / raw)
  To: Chunfeng Yun, Greg Kroah-Hartman, Rob Herring
  Cc: Mark Rutland, Mathias Nyman, linux-usb, devicetree,
	linux-arm-kernel, linux-mediatek, linux-kernel
In-Reply-To: <1567150854-30033-6-git-send-email-chunfeng.yun@mediatek.com>



On 30/08/2019 09:40, Chunfeng Yun wrote:
> Add USB related nodes for MT8183, set it as host mode by default.
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> v2~v3: no changes
> ---
>  arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 22 +++++++++
>  arch/arm64/boot/dts/mediatek/mt8183.dtsi    | 55 +++++++++++++++++++++
>  2 files changed, 77 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> index d8e555cbb5d3..142ff52f0f42 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> @@ -6,7 +6,9 @@
>   */
>  
>  /dts-v1/;
> +#include <dt-bindings/gpio/gpio.h>
>  #include "mt8183.dtsi"
> +#include "mt6358.dtsi"

mt6358.dtsi is accepted upstream now.
While the rest of the series implements wake up function, I understand that this
patch is independent and enables USB without wake up.

If so, let me know and I can take it now.

Regards,
Matthias

>  
>  / {
>  	model = "MediaTek MT8183 evaluation board";
> @@ -24,6 +26,16 @@
>  	chosen {
>  		stdout-path = "serial0:921600n8";
>  	};
> +
> +	usb_vbus: regulator@0 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "p0_vbus";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		gpio = <&pio 42 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +		regulator-always-on;
> +	};
>  };
>  
>  &auxadc {
> @@ -135,6 +147,16 @@
>  
>  };
>  
> +&ssusb {
> +	vusb33-supply = <&mt6358_vusb_reg>;
> +	dr_mode = "host";
> +	status = "okay";
> +};
> +
> +&usb_host {
> +	status = "okay";
> +};
> +
>  &uart0 {
>  	status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index c2749c4631bc..28da334237c6 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -8,6 +8,7 @@
>  #include <dt-bindings/clock/mt8183-clk.h>
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  #include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/phy/phy.h>
>  #include "mt8183-pinfunc.h"
>  
>  / {
> @@ -372,6 +373,35 @@
>  			status = "disabled";
>  		};
>  
> +		ssusb: usb@11201000 {
> +			compatible = "mediatek,mt8183-mtu3", "mediatek,mtu3";
> +			reg = <0 0x11201000 0 0x2e00>,
> +			      <0 0x11203e00 0 0x0100>;
> +			reg-names = "mac", "ippc";
> +			interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_LOW>;
> +			phys = <&u2port0 PHY_TYPE_USB2>,
> +			       <&u3port0 PHY_TYPE_USB3>;
> +			clocks = <&infracfg CLK_INFRA_UNIPRO_SCK>,
> +				 <&infracfg CLK_INFRA_USB>;
> +			clock-names = "sys_ck", "ref_ck";
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges;
> +			status = "disabled";
> +
> +			usb_host: xhci@11200000 {
> +				compatible = "mediatek,mt8183-xhci",
> +					     "mediatek,mtk-xhci";
> +				reg = <0 0x11200000 0 0x1000>;
> +				reg-names = "mac";
> +				interrupts = <GIC_SPI 73 IRQ_TYPE_LEVEL_LOW>;
> +				clocks = <&infracfg CLK_INFRA_UNIPRO_SCK>,
> +					 <&infracfg CLK_INFRA_USB>;
> +				clock-names = "sys_ck", "ref_ck";
> +				status = "disabled";
> +			};
> +		};
> +
>  		audiosys: syscon@11220000 {
>  			compatible = "mediatek,mt8183-audiosys", "syscon";
>  			reg = <0 0x11220000 0 0x1000>;
> @@ -384,6 +414,31 @@
>  			reg = <0 0x11f10000 0 0x1000>;
>  		};
>  
> +		u3phy: usb-phy@11f40000 {
> +			compatible = "mediatek,mt8183-tphy",
> +				     "mediatek,generic-tphy-v2";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges = <0 0 0x11f40000 0x1000>;
> +			status = "okay";
> +
> +			u2port0: usb-phy@0 {
> +				reg = <0x0 0x700>;
> +				clocks = <&clk26m>;
> +				clock-names = "ref";
> +				#phy-cells = <1>;
> +				status = "okay";
> +			};
> +
> +			u3port0: usb-phy@0700 {
> +				reg = <0x0700 0x900>;
> +				clocks = <&clk26m>;
> +				clock-names = "ref";
> +				#phy-cells = <1>;
> +				status = "okay";
> +			};
> +		};
> +
>  		mfgcfg: syscon@13000000 {
>  			compatible = "mediatek,mt8183-mfgcfg", "syscon";
>  			reg = <0 0x13000000 0 0x1000>;
> 

^ permalink raw reply

* Re: [PATCH v4 4/7] clocksource: dw_apb_timer: Set clockevent any-possible-CPU mask
From: Serge Semin @ 2020-05-21 15:04 UTC (permalink / raw)
  To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano
  Cc: Serge Semin, Alexey Malahov, Paul Burton, Ralf Baechle,
	Alessandro Zummo, Alexandre Belloni, Arnd Bergmann, Rob Herring,
	linux-mips, linux-rtc, devicetree, Allison Randal, afzal mohammed,
	Kate Stewart, Greg Kroah-Hartman, Enrico Weigelt, Alexios Zavras,
	linux-kernel
In-Reply-To: <20200521005321.12129-5-Sergey.Semin@baikalelectronics.ru>

Daniel,

On Thu, May 21, 2020 at 03:53:17AM +0300, Serge Semin wrote:
> Currently the DW APB Timer driver binds all clockevent timers to
> CPU #0. This isn't good for multiple reasons. First of all seeing
> the device is placed on APB bus (which makes it accessible from any
> CPU core), accessible over MMIO and having the DYNIRQ flag set we
> can be sure that manually binding the timer to any CPU just isn't
> correct. By doing so we just set an extra limitation on device usage.
> This also doesn't reflect the device actual capability, since by
> setting the IRQ affinity we can make it virtually local to any CPU.
> Secondly imagine if you had a real CPU-local timer with the same
> rating and the same CPU-affinity. In this case if DW APB timer was
> registered first, then due to the clockevent framework tick-timer
> selection procedure we'll end up with the real CPU-local timer being
> left unselected for clock-events tracking. But on most of the platforms
> (MIPS/ARM/etc) such timers are normally embedded into the CPU core and
> are accessible with much better performance then devices placed on APB.
> For instance in MIPS architectures there is r4k-timer, which is
> CPU-local, assigned with the same rating, and normally its
> clockevent device is registered after the platform-specific one.
> 
> So in order to fix all of these issues lets set the DW APB clockevent
> timer cpumask to be 'cpu_possible_mask'. By doing so the clockevent
> framework would prefer to select the real CPU-local timer instead
> of DW APB one. Otherwise if there is no other than DW APB device for
> clockevents tracking then it will be selected.

Hm, seems to me it has been a mistake to remove the CPU affiliation for the
timers since x86 has got a specific use-case with assigning the timers to all
detected CPUs. Though for our needs we don't need this feature. I'll make the
affiliation optional then. Like this:

> dw_apb_clockevent_init(int cpu, const char *name, unsigned rating,
>                      void __iomem *base, int irq, unsigned long freq)
> {
> ...
>       dw_ced->ced.cpumask = cpu < 0 ? cpu_possible_mask : cpumask_of(cpu);
> ...
> }

DW APB Timer OF-based devices won't make the timer affiliated to the CPU #0, but
to any possible CPU by passing -1 to the dw_apb_clockevent_init():
--- drivers/clocksource/dw_apb_timer_of.c
+++ drivers/clocksource/dw_apb_timer_of.c
-       ced = dw_apb_clockevent_init(0, event_timer->name, 300, iobase, irq,
                                     rate);
+       ced = dw_apb_clockevent_init(-1, event_timer->name, 300, iobase, irq,
                                     rate);

What do you think?

-Sergey

> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: linux-mips@vger.kernel.org
> Cc: linux-rtc@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> ---
>  drivers/clocksource/dw_apb_timer.c    | 18 +++++++-----------
>  drivers/clocksource/dw_apb_timer_of.c |  3 +--
>  include/linux/dw_apb_timer.h          |  2 +-
>  3 files changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/clocksource/dw_apb_timer.c b/drivers/clocksource/dw_apb_timer.c
> index b207a77b0831..8ebb43916423 100644
> --- a/drivers/clocksource/dw_apb_timer.c
> +++ b/drivers/clocksource/dw_apb_timer.c
> @@ -106,6 +106,7 @@ static irqreturn_t dw_apb_clockevent_irq(int irq, void *data)
>  		dw_ced->eoi(&dw_ced->timer);
>  
>  	evt->event_handler(evt);
> +
>  	return IRQ_HANDLED;
>  }
>  
> @@ -123,8 +124,7 @@ static int apbt_shutdown(struct clock_event_device *evt)
>  	struct dw_apb_clock_event_device *dw_ced = ced_to_dw_apb_ced(evt);
>  	u32 ctrl;
>  
> -	pr_debug("%s CPU %d state=shutdown\n", __func__,
> -		 cpumask_first(evt->cpumask));
> +	pr_debug("%s state=shutdown\n", __func__);
>  
>  	ctrl = apbt_readl(&dw_ced->timer, APBTMR_N_CONTROL);
>  	ctrl &= ~APBTMR_CONTROL_ENABLE;
> @@ -137,8 +137,7 @@ static int apbt_set_oneshot(struct clock_event_device *evt)
>  	struct dw_apb_clock_event_device *dw_ced = ced_to_dw_apb_ced(evt);
>  	u32 ctrl;
>  
> -	pr_debug("%s CPU %d state=oneshot\n", __func__,
> -		 cpumask_first(evt->cpumask));
> +	pr_debug("%s state=oneshot\n", __func__);
>  
>  	ctrl = apbt_readl(&dw_ced->timer, APBTMR_N_CONTROL);
>  	/*
> @@ -170,8 +169,7 @@ static int apbt_set_periodic(struct clock_event_device *evt)
>  	unsigned long period = DIV_ROUND_UP(dw_ced->timer.freq, HZ);
>  	u32 ctrl;
>  
> -	pr_debug("%s CPU %d state=periodic\n", __func__,
> -		 cpumask_first(evt->cpumask));
> +	pr_debug("%s state=periodic\n", __func__);
>  
>  	ctrl = apbt_readl(&dw_ced->timer, APBTMR_N_CONTROL);
>  	ctrl |= APBTMR_CONTROL_MODE_PERIODIC;
> @@ -194,8 +192,7 @@ static int apbt_resume(struct clock_event_device *evt)
>  {
>  	struct dw_apb_clock_event_device *dw_ced = ced_to_dw_apb_ced(evt);
>  
> -	pr_debug("%s CPU %d state=resume\n", __func__,
> -		 cpumask_first(evt->cpumask));
> +	pr_debug("%s state=resume\n", __func__);
>  
>  	apbt_enable_int(&dw_ced->timer);
>  	return 0;
> @@ -222,7 +219,6 @@ static int apbt_next_event(unsigned long delta,
>  /**
>   * dw_apb_clockevent_init() - use an APB timer as a clock_event_device
>   *
> - * @cpu:	The CPU the events will be targeted at.
>   * @name:	The name used for the timer and the IRQ for it.
>   * @rating:	The rating to give the timer.
>   * @base:	I/O base for the timer registers.
> @@ -237,7 +233,7 @@ static int apbt_next_event(unsigned long delta,
>   * releasing the IRQ.
>   */
>  struct dw_apb_clock_event_device *
> -dw_apb_clockevent_init(int cpu, const char *name, unsigned rating,
> +dw_apb_clockevent_init(const char *name, unsigned int rating,
>  		       void __iomem *base, int irq, unsigned long freq)
>  {
>  	struct dw_apb_clock_event_device *dw_ced =
> @@ -257,7 +253,7 @@ dw_apb_clockevent_init(int cpu, const char *name, unsigned rating,
>  	dw_ced->ced.max_delta_ticks = 0x7fffffff;
>  	dw_ced->ced.min_delta_ns = clockevent_delta2ns(5000, &dw_ced->ced);
>  	dw_ced->ced.min_delta_ticks = 5000;
> -	dw_ced->ced.cpumask = cpumask_of(cpu);
> +	dw_ced->ced.cpumask = cpu_possible_mask;
>  	dw_ced->ced.features = CLOCK_EVT_FEAT_PERIODIC |
>  				CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_DYNIRQ;
>  	dw_ced->ced.set_state_shutdown = apbt_shutdown;
> diff --git a/drivers/clocksource/dw_apb_timer_of.c b/drivers/clocksource/dw_apb_timer_of.c
> index 8c28b127759f..0a2505b323d7 100644
> --- a/drivers/clocksource/dw_apb_timer_of.c
> +++ b/drivers/clocksource/dw_apb_timer_of.c
> @@ -73,8 +73,7 @@ static void __init add_clockevent(struct device_node *event_timer)
>  
>  	timer_get_base_and_rate(event_timer, &iobase, &rate);
>  
> -	ced = dw_apb_clockevent_init(0, event_timer->name, 300, iobase, irq,
> -				     rate);
> +	ced = dw_apb_clockevent_init(event_timer->name, 300, iobase, irq, rate);
>  	if (!ced)
>  		panic("Unable to initialise clockevent device");
>  
> diff --git a/include/linux/dw_apb_timer.h b/include/linux/dw_apb_timer.h
> index 82ebf9223948..689022bc8d17 100644
> --- a/include/linux/dw_apb_timer.h
> +++ b/include/linux/dw_apb_timer.h
> @@ -39,7 +39,7 @@ void dw_apb_clockevent_resume(struct dw_apb_clock_event_device *dw_ced);
>  void dw_apb_clockevent_stop(struct dw_apb_clock_event_device *dw_ced);
>  
>  struct dw_apb_clock_event_device *
> -dw_apb_clockevent_init(int cpu, const char *name, unsigned rating,
> +dw_apb_clockevent_init(const char *name, unsigned int rating,
>  		       void __iomem *base, int irq, unsigned long freq);
>  struct dw_apb_clocksource *
>  dw_apb_clocksource_init(unsigned rating, const char *name, void __iomem *base,
> -- 
> 2.25.1
> 

^ permalink raw reply

* Re: [PATCH 12/12] bus: fsl-mc: Add ACPI support for fsl-mc
From: Laurentiu Tudor @ 2020-05-21 15:03 UTC (permalink / raw)
  To: Lorenzo Pieralisi, linux-arm-kernel
  Cc: Diana Craciun, Makarand Pawagi, iommu, linux-acpi, devicetree,
	linux-pci, Rob Herring, Rafael J. Wysocki, Joerg Roedel,
	Hanjun Guo, Bjorn Helgaas, Sudeep Holla, Robin Murphy,
	Catalin Marinas, Will Deacon, Marc Zyngier
In-Reply-To: <20200521130008.8266-13-lorenzo.pieralisi@arm.com>

Hi Lorenzo,

On 5/21/2020 4:00 PM, Lorenzo Pieralisi wrote:
> From: Diana Craciun <diana.craciun@oss.nxp.com>
> 
> Add ACPI support in the fsl-mc driver. Driver parses MC DSDT table to
> extract memory and other resources.
> 
> Interrupt (GIC ITS) information is extracted from the MADT table
> by drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c.
> 
> IORT table is parsed to configure DMA.
> 
> Signed-off-by: Makarand Pawagi <makarand.pawagi@nxp.com>
> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> ---

The author of this patch should be Makarand. I think I accidentaly broke
it when we exchanged the patches. Very sorry about it.

---
Best Regards, Laurentiu


>  drivers/bus/fsl-mc/fsl-mc-bus.c             | 73 +++++++++++++++-----
>  drivers/bus/fsl-mc/fsl-mc-msi.c             | 37 +++++-----
>  drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c | 75 ++++++++++++++++++++-
>  3 files changed, 150 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
> index 824ff77bbe86..324d49d6df89 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> @@ -18,6 +18,8 @@
>  #include <linux/bitops.h>
>  #include <linux/msi.h>
>  #include <linux/dma-mapping.h>
> +#include <linux/acpi.h>
> +#include <linux/iommu.h>
>  
>  #include "fsl-mc-private.h"
>  
> @@ -38,6 +40,7 @@ struct fsl_mc {
>  	struct fsl_mc_device *root_mc_bus_dev;
>  	u8 num_translation_ranges;
>  	struct fsl_mc_addr_translation_range *translation_ranges;
> +	void *fsl_mc_regs;
>  };
>  
>  /**
> @@ -56,6 +59,10 @@ struct fsl_mc_addr_translation_range {
>  	phys_addr_t start_phys_addr;
>  };
>  
> +#define FSL_MC_FAPR	0x28
> +#define MC_FAPR_PL	BIT(18)
> +#define MC_FAPR_BMT	BIT(17)
> +
>  /**
>   * fsl_mc_bus_match - device to driver matching callback
>   * @dev: the fsl-mc device to match against
> @@ -124,7 +131,10 @@ static int fsl_mc_dma_configure(struct device *dev)
>  	while (dev_is_fsl_mc(dma_dev))
>  		dma_dev = dma_dev->parent;
>  
> -	return of_dma_configure_id(dev, dma_dev->of_node, 0, &input_id);
> +	if (dev_of_node(dma_dev))
> +		return of_dma_configure_id(dev, dma_dev->of_node, 0, &input_id);
> +
> +	return acpi_dma_configure_id(dev, DEV_DMA_COHERENT, &input_id);
>  }
>  
>  static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
> @@ -865,8 +875,11 @@ static int fsl_mc_bus_probe(struct platform_device *pdev)
>  	struct fsl_mc_io *mc_io = NULL;
>  	int container_id;
>  	phys_addr_t mc_portal_phys_addr;
> -	u32 mc_portal_size;
> -	struct resource res;
> +	u32 mc_portal_size, mc_stream_id;
> +	struct resource *plat_res;
> +
> +	if (!iommu_present(&fsl_mc_bus_type))
> +		return -EPROBE_DEFER;
>  
>  	mc = devm_kzalloc(&pdev->dev, sizeof(*mc), GFP_KERNEL);
>  	if (!mc)
> @@ -874,19 +887,33 @@ static int fsl_mc_bus_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, mc);
>  
> +	plat_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	mc->fsl_mc_regs = devm_ioremap_resource(&pdev->dev, plat_res);
> +	if (IS_ERR(mc->fsl_mc_regs))
> +		return PTR_ERR(mc->fsl_mc_regs);
> +
> +	if (IS_ENABLED(CONFIG_ACPI) && !dev_of_node(&pdev->dev)) {
> +		mc_stream_id = readl(mc->fsl_mc_regs + FSL_MC_FAPR);
> +		/*
> +		 * HW ORs the PL and BMT bit, places the result in bit 15 of
> +		 * the StreamID and ORs in the ICID. Calculate it accordingly.
> +		 */
> +		mc_stream_id = (mc_stream_id & 0xffff) |
> +				((mc_stream_id & (MC_FAPR_PL | MC_FAPR_BMT)) ?
> +					0x4000 : 0);
> +		error = acpi_dma_configure_id(&pdev->dev, DEV_DMA_COHERENT,
> +					      &mc_stream_id);
> +		if (error)
> +			dev_warn(&pdev->dev, "failed to configure dma: %d.\n",
> +				 error);
> +	}
> +
>  	/*
>  	 * Get physical address of MC portal for the root DPRC:
>  	 */
> -	error = of_address_to_resource(pdev->dev.of_node, 0, &res);
> -	if (error < 0) {
> -		dev_err(&pdev->dev,
> -			"of_address_to_resource() failed for %pOF\n",
> -			pdev->dev.of_node);
> -		return error;
> -	}
> -
> -	mc_portal_phys_addr = res.start;
> -	mc_portal_size = resource_size(&res);
> +	plat_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	mc_portal_phys_addr = plat_res->start;
> +	mc_portal_size = resource_size(plat_res);
>  	error = fsl_create_mc_io(&pdev->dev, mc_portal_phys_addr,
>  				 mc_portal_size, NULL,
>  				 FSL_MC_IO_ATOMIC_CONTEXT_PORTAL, &mc_io);
> @@ -903,11 +930,13 @@ static int fsl_mc_bus_probe(struct platform_device *pdev)
>  	dev_info(&pdev->dev, "MC firmware version: %u.%u.%u\n",
>  		 mc_version.major, mc_version.minor, mc_version.revision);
>  
> -	error = get_mc_addr_translation_ranges(&pdev->dev,
> -					       &mc->translation_ranges,
> -					       &mc->num_translation_ranges);
> -	if (error < 0)
> -		goto error_cleanup_mc_io;
> +	if (dev_of_node(&pdev->dev)) {
> +		error = get_mc_addr_translation_ranges(&pdev->dev,
> +						&mc->translation_ranges,
> +						&mc->num_translation_ranges);
> +		if (error < 0)
> +			goto error_cleanup_mc_io;
> +	}
>  
>  	error = dprc_get_container_id(mc_io, 0, &container_id);
>  	if (error < 0) {
> @@ -934,6 +963,7 @@ static int fsl_mc_bus_probe(struct platform_device *pdev)
>  		goto error_cleanup_mc_io;
>  
>  	mc->root_mc_bus_dev = mc_bus_dev;
> +	mc_bus_dev->dev.fwnode = pdev->dev.fwnode;
>  	return 0;
>  
>  error_cleanup_mc_io:
> @@ -967,11 +997,18 @@ static const struct of_device_id fsl_mc_bus_match_table[] = {
>  
>  MODULE_DEVICE_TABLE(of, fsl_mc_bus_match_table);
>  
> +static const struct acpi_device_id fsl_mc_bus_acpi_match_table[] = {
> +	{"NXP0008", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(acpi, fsl_mc_bus_acpi_match_table);
> +
>  static struct platform_driver fsl_mc_bus_driver = {
>  	.driver = {
>  		   .name = "fsl_mc_bus",
>  		   .pm = NULL,
>  		   .of_match_table = fsl_mc_bus_match_table,
> +		   .acpi_match_table = fsl_mc_bus_acpi_match_table,
>  		   },
>  	.probe = fsl_mc_bus_probe,
>  	.remove = fsl_mc_bus_remove,
> diff --git a/drivers/bus/fsl-mc/fsl-mc-msi.c b/drivers/bus/fsl-mc/fsl-mc-msi.c
> index e7bbff445a83..8edadf05cbb7 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-msi.c
> +++ b/drivers/bus/fsl-mc/fsl-mc-msi.c
> @@ -13,6 +13,7 @@
>  #include <linux/irq.h>
>  #include <linux/irqdomain.h>
>  #include <linux/msi.h>
> +#include <linux/acpi_iort.h>
>  
>  #include "fsl-mc-private.h"
>  
> @@ -179,25 +180,31 @@ struct irq_domain *fsl_mc_msi_create_irq_domain(struct fwnode_handle *fwnode,
>  
>  struct irq_domain *fsl_mc_find_msi_domain(struct device *dev)
>  {
> -	struct irq_domain *msi_domain = NULL;
> +	struct device *root_dprc_dev;
> +	struct device *bus_dev;
> +	struct irq_domain *msi_domain;
>  	struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
>  
> -	msi_domain = of_msi_map_get_device_domain(dev, mc_dev->icid,
> +	fsl_mc_get_root_dprc(dev, &root_dprc_dev);
> +	bus_dev = root_dprc_dev->parent;
> +
> +	if (bus_dev->of_node) {
> +		msi_domain = of_msi_map_get_device_domain(dev,
> +						  mc_dev->icid,
>  						  DOMAIN_BUS_FSL_MC_MSI);
>  
> -	/*
> -	 * if the msi-map property is missing assume that all the
> -	 * child containers inherit the domain from the parent
> -	 */
> -	if (!msi_domain) {
> -		struct device *root_dprc_dev;
> -		struct device *bus_dev;
> -
> -		fsl_mc_get_root_dprc(dev, &root_dprc_dev);
> -		bus_dev = root_dprc_dev->parent;
> -		msi_domain = of_msi_get_domain(bus_dev,
> -					       bus_dev->of_node,
> -					       DOMAIN_BUS_FSL_MC_MSI);
> +		/*
> +		 * if the msi-map property is missing assume that all the
> +		 * child containers inherit the domain from the parent
> +		 */
> +		if (!msi_domain)
> +
> +			msi_domain = of_msi_get_domain(bus_dev,
> +						bus_dev->of_node,
> +						DOMAIN_BUS_FSL_MC_MSI);
> +	} else {
> +		msi_domain = iort_get_device_domain(dev, mc_dev->icid,
> +						    DOMAIN_BUS_FSL_MC_MSI);
>  	}
>  
>  	return msi_domain;
> diff --git a/drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c b/drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c
> index a5c8d577e424..b8b948fb6b2d 100644
> --- a/drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c
> +++ b/drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c
> @@ -7,6 +7,8 @@
>   *
>   */
>  
> +#include <linux/acpi.h>
> +#include <linux/acpi_iort.h>
>  #include <linux/of_device.h>
>  #include <linux/of_address.h>
>  #include <linux/irq.h>
> @@ -30,7 +32,8 @@ static u32 fsl_mc_msi_domain_get_msi_id(struct irq_domain *domain,
>  	u32 out_id;
>  
>  	of_node = irq_domain_get_of_node(domain);
> -	out_id = of_msi_map_id(&mc_dev->dev, of_node, mc_dev->icid);
> +	out_id = of_node ? of_msi_map_id(&mc_dev->dev, of_node, mc_dev->icid) :
> +			iort_msi_map_id(&mc_dev->dev, mc_dev->icid);
>  
>  	return out_id;
>  }
> @@ -79,7 +82,67 @@ static const struct of_device_id its_device_id[] = {
>  	{},
>  };
>  
> -static int __init its_fsl_mc_msi_init(void)
> +static int __init its_fsl_mc_msi_init_one(struct fwnode_handle *handle,
> +					  const char *name)
> +{
> +	struct irq_domain *parent;
> +	struct irq_domain *mc_msi_domain;
> +
> +	parent = irq_find_matching_fwnode(handle, DOMAIN_BUS_NEXUS);
> +	if (!parent || !msi_get_domain_info(parent)) {
> +		pr_err("%s: Unable to locate ITS domain\n", name);
> +		return -ENXIO;
> +	}
> +
> +	mc_msi_domain = fsl_mc_msi_create_irq_domain(handle,
> +						&its_fsl_mc_msi_domain_info,
> +						parent);
> +	if (!mc_msi_domain)
> +		pr_err("ACPIF: unable to create fsl-mc domain\n");
> +
> +	pr_info("fsl-mc MSI: domain created\n");
> +
> +	return 0;
> +}
> +
> +static int __init
> +its_fsl_mc_msi_parse_madt(union acpi_subtable_headers *header,
> +			  const unsigned long end)
> +{
> +	struct acpi_madt_generic_translator *its_entry;
> +	struct fwnode_handle *dom_handle;
> +	const char *node_name;
> +	int err = -ENXIO;
> +
> +	its_entry = (struct acpi_madt_generic_translator *)header;
> +	node_name = kasprintf(GFP_KERNEL, "ITS@0x%lx",
> +			      (long)its_entry->base_address);
> +
> +	dom_handle = iort_find_domain_token(its_entry->translation_id);
> +	if (!dom_handle) {
> +		pr_err("%s: Unable to locate ITS domain handle\n", node_name);
> +		goto out;
> +	}
> +
> +	err = its_fsl_mc_msi_init_one(dom_handle, node_name);
> +	if (!err)
> +		pr_info("fsl-mc MSI: %s domain created\n", node_name);
> +
> +out:
> +	kfree(node_name);
> +	return err;
> +}
> +
> +
> +static int __init its_fsl_mc_acpi_msi_init(void)
> +{
> +	acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
> +			      its_fsl_mc_msi_parse_madt, 0);
> +
> +	return 0;
> +}
> +
> +static int __init its_fsl_mc_of_msi_init(void)
>  {
>  	struct device_node *np;
>  	struct irq_domain *parent;
> @@ -113,4 +176,12 @@ static int __init its_fsl_mc_msi_init(void)
>  	return 0;
>  }
>  
> +static int __init its_fsl_mc_msi_init(void)
> +{
> +	its_fsl_mc_of_msi_init();
> +	its_fsl_mc_acpi_msi_init();
> +
> +	return 0;
> +}
> +
>  early_initcall(its_fsl_mc_msi_init);
> 

^ permalink raw reply

* Re: [RFC v1 2/3] drivers: nvmem: Add driver for QTI qfprom-efuse support
From: Srinivas Kandagatla @ 2020-05-21 15:00 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Ravi Kumar Bokka (Temp), Rob Herring, LKML,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Rajendra Nayak, Sai Prakash Ranjan, dhavalp, mturney, sparate,
	c_rbokka, mkurumel
In-Reply-To: <CAD=FV=UbZPQ74COXJbOikq9Wcx1UvtuMuMA+nqkx44uySoqggg@mail.gmail.com>



On 20/05/2020 23:48, Doug Anderson wrote:
>> Is this only applicable for corrected address space?
> I guess I was proposing a two dts-node / two drive approach here.
> 
> dts node #1:just covers the memory range for accessing the FEC-corrected data
> driver #1: read-only and reads the FEC-corrected data
> 
> dts node #2: covers the memory range that's_not_  the FEC-corrected
> memory range.
> driver #2: read-write.  reading reads uncorrected data
> 
> Does that seem sane?

I see your point but it does not make sense to have two node for same thing.

Isn't the raw address space reads used to for blowing and checking the 
fuses if they are blown correctly or not and software usage of these 
fuses should only be done from correct address space?

the read interface to user should be always from corrected address space
and write interface should be to raw address space.


--srini

> 
> 

^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: spi: Add Baikal-T1 System Boot SPI Controller binding
From: Rob Herring @ 2020-05-21 14:57 UTC (permalink / raw)
  To: Serge Semin
  Cc: Serge Semin, Mark Brown, Ramil Zaripov, Alexey Malahov,
	Thomas Bogendoerfer, Paul Burton, Ralf Baechle, John Garry,
	Chuanhong Guo, Tomer Maimon, Lee Jones, Miquel Raynal,
	Arnd Bergmann, open list:MIPS, linux-spi, devicetree,
	linux-kernel@vger.kernel.org
In-Reply-To: <20200518212703.vju456kd3telctux@mobilestation>

On Mon, May 18, 2020 at 3:27 PM Serge Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
>
> On Mon, May 18, 2020 at 09:26:59AM -0600, Rob Herring wrote:
> > On Fri, May 08, 2020 at 12:36:20PM +0300, Serge Semin wrote:
> > > Baikal-T1 Boot SPI is a part of the SoC System Controller and is
> > > responsible for the system bootup from an external SPI flash. It's a DW
> > > APB SSI-based SPI-controller with no interrupts, no DMA, with just one
> > > native chip-select available and a single reference clock. Since Baikal-T1
> > > SoC is normally booted up from an external SPI flash this SPI controller
> > > in most of the cases is supposed to be connected to a single SPI-nor
> > > flash. Additionally in order to provide a transparent from CPU point of
> > > view initial code execution procedure the system designers created an IP
> > > block which physically maps the SPI flash found at CS0 to a memory region.
> > >
> > > Co-developed-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> > > Signed-off-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > > Cc: Paul Burton <paulburton@kernel.org>
> > > Cc: Ralf Baechle <ralf@linux-mips.org>
> > > Cc: John Garry <john.garry@huawei.com>
> > > Cc: Chuanhong Guo <gch981213@gmail.com>
> > > Cc: Tomer Maimon <tmaimon77@gmail.com>
> > > Cc: Lee Jones <lee.jones@linaro.org>
> > > Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> > > Cc: Arnd Bergmann <arnd@arndb.de>
> > > Cc: linux-mips@vger.kernel.org
> > > Cc: linux-spi@vger.kernel.org
> > > ---
> > >  .../bindings/spi/baikal,bt1-sys-ssi.yaml      | 100 ++++++++++++++++++
> > >  1 file changed, 100 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml b/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> > > new file mode 100644
> > > index 000000000000..d9d3257d78f4
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/spi/baikal,bt1-sys-ssi.yaml
> > > @@ -0,0 +1,100 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +# Copyright (C) 2020 BAIKAL ELECTRONICS, JSC
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/spi/baikal,bt1-sys-ssi.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Baikal-T1 System Boot SSI Controller
> > > +
> > > +description: |
> > > +  Baikal-T1 System Controller includes a Boot SPI Controller, which is
> > > +  responsible for loading chip bootup code from an external SPI flash. In order
> > > +  to do this transparently from CPU point of view there is a dedicated IP block
> > > +  mapping the 16MB flash to a dedicated MMIO range. The controller is based on
> > > +  the DW APB SSI IP-core but equipped with very limited resources: no IRQ,
> > > +  no DMA, a single native CS being necessarily connected to a 16MB SPI flash
> > > +  (otherwise the system won't bootup from the flash), internal Tx/Rx FIFO of
> > > +  just 8 bytes depth. Access to DW APB SSI controller registers is mutually
> > > +  exclusive from normal MMIO interface and from physically mapped SPI Flash
> > > +  memory. So either one or another way of using the controller functionality
> > > +  can be enabled at a time.
> > > +
> > > +maintainers:
> > > +  - Serge Semin <fancer.lancer@gmail.com>
> > > +
> > > +allOf:
> > > +  - $ref: spi-controller.yaml#
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: baikal,bt1-sys-ssi
> > > +
> > > +  reg:
> > > +    items:
> > > +      - description: Baikal-T1 Boot Controller configuration registers
> > > +      - description: Physically mapped SPI flash ROM found at CS0
> > > +
> > > +  reg-names:
> > > +    items:
> > > +      - const: config
> > > +      - const: map
> > > +
> > > +  clocks:
> > > +    description: SPI Controller reference clock source
> >
> > Can drop this.
>
> Ok.
>
> >
> > > +    maxItems: 1
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: ssi_clk
> > > +
> > > +  num-cs:
> > > +    const: 1
> > > +
> > > +patternProperties:
> > > +  "^.*@[0-9a-f]+":
> > > +    type: object
> > > +    properties:
> > > +      reg:
> > > +        minimum: 0
> > > +        maximum: 0
> > > +
> > > +      spi-rx-bus-width:
> > > +        const: 1
> > > +
> > > +      spi-tx-bus-width:
> > > +        const: 1
> >
> > What's the point of these 2 properties if they aren't required?
>
> Yes, they are optional, but this is a constraint on the bus-width parameters.
> DW APB SSI provides a single laned Tx and Rx.

Are you just trying to keep someone from saying 'spi-tx-bus-width: 2'
for example?

You could also say 'spi-tx-bus-width: false' here to disallow the
property. I guess the above is fine.

Rob

^ permalink raw reply

* Re: [PATCH v3 01/16] spi: dw: Add Tx/Rx finish wait methods to the MID DMA
From: Feng Tang @ 2020-05-21 14:55 UTC (permalink / raw)
  To: Serge Semin
  Cc: Serge Semin, Mark Brown, Grant Likely, Vinod Koul, Alan Cox,
	Linus Walleij, Georgy Vlasov, Ramil Zaripov, Alexey Malahov,
	Thomas Bogendoerfer, Paul Burton, Ralf Baechle, Arnd Bergmann,
	Andy Shevchenko, Rob Herring, linux-mips, devicetree,
	Jarkko Nikula, Thomas Gleixner, Wan Ahmad Zainie, Linus Walleij,
	Clement Leger, linux-spi, linux-kernel
In-Reply-To: <20200521114736.b2azyfvym372vkdl@mobilestation>

Hi Serge,

On Thu, May 21, 2020 at 02:47:36PM +0300, Serge Semin wrote:
> Hello Feng,
> 
> On Thu, May 21, 2020 at 11:09:24AM +0800, Feng Tang wrote:
> > Hi Serge,
> > 
> > On Thu, May 21, 2020 at 04:21:51AM +0300, Serge Semin wrote:
> 
> [nip]
> 
> > >  /*
> > >   * dws->dma_chan_busy is set before the dma transfer starts, callback for rx
> > >   * channel will clear a corresponding bit.
> > > @@ -200,6 +267,8 @@ static void dw_spi_dma_rx_done(void *arg)
> > >  {
> > >  	struct dw_spi *dws = arg;
> > >  
> > > +	dw_spi_dma_wait_rx_done(dws);
> > 
> > I can understand the problem about TX, but I don't see how RX
> > will get hurt, can you elaborate more? thanks
> > 
> > - Feng
> 
> Your question is correct. You are right with your hypothesis. Ideally upon the
> dw_spi_dma_rx_done() execution Rx FIFO must be already empty. That's why the
> commit log signifies the error being mostly related with Tx FIFO. But
> practically there are many reasons why Rx FIFO might be left with data:
> DMA engine failures, incorrect DMA configuration (if DW SPI or DW DMA driver
> messed something up), controller hanging up, and so on. It's better to catch
> an error at this stage while propagating it up to the SPI device drivers.
> Especially seeing the wait-check implementation doesn't gives us much of the
> execution overhead in normal conditions. So by calling dw_spi_dma_wait_rx_done()
> we make sure that all the data has been fetched and we may freely get the
> buffers back to the client driver.

I see your point about checking RX. But I still don't think checking
RX FIFO level is the right way to detect error. Some data left in
RX FIFO doesn't always mean a error, say for some case if there is
20 words in RX FIFO, and the driver starts a DMA request for 16
words, then after a sucessful DMA transaction, there are 4 words
left without any error.

Thanks,
Feng

> 
> -Sergey

^ permalink raw reply

* Re: [PATCH v12 2/3] i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver
From: Andy Shevchenko @ 2020-05-21 14:53 UTC (permalink / raw)
  To: Tali Perry
  Cc: Wolfram Sang, Ofer Yehielli, Brendan Higgins, avifishman70,
	Tomer Maimon, kfting, Patrick Venture, Nancy Yuen, Benjamin Fair,
	Rob Herring, linux-arm-kernel, linux-i2c, OpenBMC Maillist,
	devicetree, Linux Kernel Mailing List
In-Reply-To: <CAHb3i=vcVLWHjdiJoNZQrwJCqzszpOL7e9SAjqObsZCRH4ifwg@mail.gmail.com>

On Thu, May 21, 2020 at 05:45:03PM +0300, Tali Perry wrote:
> On Thu, May 21, 2020 at 5:31 PM Wolfram Sang <wsa@the-dreams.de> wrote:
> > On Thu, May 21, 2020 at 05:23:40PM +0300, Andy Shevchenko wrote:
> > > On Thu, May 21, 2020 at 02:09:09PM +0300, Tali Perry wrote:
> > > > Add Nuvoton NPCM BMC I2C controller driver.
> > >
> > > Thanks. My comments below.
> > > After addressing them, FWIW,
> > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >
> > Thanks, Andy, for all the review!
> >
> 
> Highly appreciate your time and patience for a newbie :)
> 
> > From a glimpse, this looks good to go. I will have a close look later
> > today.
> >
> > > > +#ifdef CONFIG_DEBUG_FS
> > >
> > > Again, why is this here?
> > >
> > > Have you checked debugfs.h for !CONFIG_DEBUG_FS case?
> 
> I compiled both options. I removed the ifdef in most places, except in the
> struct itself. Users that don't use the debugfs don't need this in the struct.
> 
> >
> > I wondered also about DEBUG_FS entries. I can see their value when
> > developing the driver. But since this is done now, do they really help a
> > user to debug a difficult case? I am not sure, and then I wonder if we
> > should have that code in upstream. I am open for discussion, though.
> 
> The user wanted to have health monitor implemented on top of the driver.
> The user has 16 channels connected the multiple devices. All are operated
> using various daemons in the system. Sometimes the slave devices are power down.
> Therefor the user wanted to track the health status of the devices.

Ah, then there are these options I have in mind (Wolfram, FYI as well!):
1) push with debugfs as a temporary solution and convert to devlink health protocol [1];
2) drop it and develop devlink_health solution;
3) push debugfs and wait if I²C will gain devlink health support

[1]: https://www.kernel.org/doc/html/latest/networking/devlink/devlink-health.html

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH v12 2/3] i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver
From: Tali Perry @ 2020-05-21 14:45 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Andy Shevchenko, Ofer Yehielli, Brendan Higgins, avifishman70,
	Tomer Maimon, kfting, Patrick Venture, Nancy Yuen, Benjamin Fair,
	Rob Herring, linux-arm-kernel, linux-i2c, OpenBMC Maillist,
	devicetree, Linux Kernel Mailing List
In-Reply-To: <20200521143100.GA16812@ninjato>

On Thu, May 21, 2020 at 5:31 PM Wolfram Sang <wsa@the-dreams.de> wrote:
>
> Hi Tali, Andy!
>
> On Thu, May 21, 2020 at 05:23:40PM +0300, Andy Shevchenko wrote:
> > On Thu, May 21, 2020 at 02:09:09PM +0300, Tali Perry wrote:
> > > Add Nuvoton NPCM BMC I2C controller driver.
> >
> > Thanks. My comments below.
> > After addressing them, FWIW,
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> Thanks, Andy, for all the review!
>

Highly appreciate your time and patience for a newbie :)

> From a glimpse, this looks good to go. I will have a close look later
> today.
>
> > > +#ifdef CONFIG_DEBUG_FS
> >
> > Again, why is this here?
> >
> > Have you checked debugfs.h for !CONFIG_DEBUG_FS case?

I compiled both options. I removed the ifdef in most places, except in the
struct itself. Users that don't use the debugfs don't need this in the struct.

>
> I wondered also about DEBUG_FS entries. I can see their value when
> developing the driver. But since this is done now, do they really help a
> user to debug a difficult case? I am not sure, and then I wonder if we
> should have that code in upstream. I am open for discussion, though.

The user wanted to have health monitor implemented on top of the driver.
The user has 16 channels connected the multiple devices. All are operated
using various daemons in the system. Sometimes the slave devices are power down.
Therefor the user wanted to track the health status of the devices.

>
> > > +MODULE_VERSION("0.1.3");
> >
> > Module version is defined by kernel commit hash. But it's up to you and
> > subsystem maintainer to decide.
>
> Please drop it. I also think commit id's (or even kernel versions) are a
> more precise description.

will remove.

>
> Regards,
>
>    Wolfram
>

BR,
Tali

^ permalink raw reply

* Re: [PATCH v2] arm: dts: am33xx-bone-common: add gpio-line-names
From: Robert Nelson @ 2020-05-21 14:41 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: Drew Fustini, Linus Walleij, Benoît Cousson, Tony Lindgren,
	Rob Herring, Linux-OMAP, devicetree, linux kernel, Jason Kridner
In-Reply-To: <71dbf4e6-e65b-f001-319c-0b354f675568@ti.com>

> Not sure if it should be in am335x-bone-common.dtsi.
>
> For example:
> am335x-boneblack.dts
>   #include "am335x-bone-common.dtsi"
>   #include "am335x-boneblack-common.dtsi" <-- hdmi defined only here

Ah crap, yeah that's a good point.. So if we stick it in...
am335x-boneblack-common.dtsi

Then the Black-Wireless now has Ethernet...

am335x-boneblack-wireless.dts
#include "am335x-bone-common.dtsi"
#include "am335x-boneblack-common.dtsi"

It's going to be ugly, copy and paste mess, but i guess we might as
well stick it in the device " am335x-boneblack.dts"?

Regards,

-- 
Robert Nelson
https://rcn-ee.com/

^ permalink raw reply

* Re: [PATCH 04/16] arm64: dts: arm: Fix node address fields
From: Liviu Dudau @ 2020-05-21 14:40 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Andre Przywara, Rob Herring, Sudeep Holla, Lorenzo Pieralisi,
	Mark Rutland, devicetree, linux-arm-kernel
In-Reply-To: <347cdcba-a1cf-d308-1cc2-6c2194f40d19@arm.com>

On Tue, May 05, 2020 at 06:18:19PM +0100, Robin Murphy wrote:
> On 2020-05-05 5:52 pm, Andre Przywara wrote:
> > The Arm Ltd. boards were using an outdated address convention in the DT
> > node names, by separating the high from the low 32-bits of an address by
> > a comma.
> 
> I thought that historically that was deliberate, since the actual thing
> being encoded is <chip select>,<address>, rather than just cosmetically
> splitting a 64-bit address value?
> 
> Or maybe I'm thinking too far back and things have already changed in the
> meantime :/

Robin is right, if you look in the "ARM Motherboard Express µATX Technical
Reference Manual", in the RS1 memory map (aka "Cortex-A Series memory map")
the Ethernet for example is at CS2 offset 0x02000000. CS2 area is between
0x18000000 and 0x1c000000. So actual physical address for LAN9118 is
0x1a000000.

You might want to drop this patch or convert to physical addresses.

Best regards,
Liviu

> 
> Robin.
> 
> > Remove the comma from the node name suffix to be DT spec compliant.
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >   arch/arm/boot/dts/vexpress-v2m-rs1.dtsi              | 10 +++++-----
> >   arch/arm64/boot/dts/arm/foundation-v8.dtsi           |  4 ++--
> >   arch/arm64/boot/dts/arm/juno-motherboard.dtsi        |  6 +++---
> >   arch/arm64/boot/dts/arm/rtsm_ve-motherboard-rs2.dtsi |  2 +-
> >   arch/arm64/boot/dts/arm/rtsm_ve-motherboard.dtsi     |  6 +++---
> >   5 files changed, 14 insertions(+), 14 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
> > index 5c183483ec3b..8010cdcdb37a 100644
> > --- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
> > +++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
> > @@ -31,7 +31,7 @@
> >   			#interrupt-cells = <1>;
> >   			ranges;
> > -			nor_flash: flash@0,00000000 {
> > +			nor_flash: flash@0 {
> >   				compatible = "arm,vexpress-flash", "cfi-flash";
> >   				reg = <0 0x00000000 0x04000000>,
> >   				      <4 0x00000000 0x04000000>;
> > @@ -41,13 +41,13 @@
> >   				};
> >   			};
> > -			psram@1,00000000 {
> > +			psram@100000000 {
> >   				compatible = "arm,vexpress-psram", "mtd-ram";
> >   				reg = <1 0x00000000 0x02000000>;
> >   				bank-width = <4>;
> >   			};
> > -			ethernet@2,02000000 {
> > +			ethernet@202000000 {
> >   				compatible = "smsc,lan9118", "smsc,lan9115";
> >   				reg = <2 0x02000000 0x10000>;
> >   				interrupts = <15>;
> > @@ -59,14 +59,14 @@
> >   				vddvario-supply = <&v2m_fixed_3v3>;
> >   			};
> > -			usb@2,03000000 {
> > +			usb@203000000 {
> >   				compatible = "nxp,usb-isp1761";
> >   				reg = <2 0x03000000 0x20000>;
> >   				interrupts = <16>;
> >   				port1-otg;
> >   			};
> > -			iofpga@3,00000000 {
> > +			iofpga@300000000 {
> >   				compatible = "simple-bus";
> >   				#address-cells = <1>;
> >   				#size-cells = <1>;
> > diff --git a/arch/arm64/boot/dts/arm/foundation-v8.dtsi b/arch/arm64/boot/dts/arm/foundation-v8.dtsi
> > index 12f039fa3dad..e26b492795c5 100644
> > --- a/arch/arm64/boot/dts/arm/foundation-v8.dtsi
> > +++ b/arch/arm64/boot/dts/arm/foundation-v8.dtsi
> > @@ -151,7 +151,7 @@
> >   				<0 0 41 &gic 0 0 GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>,
> >   				<0 0 42 &gic 0 0 GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH>;
> > -		ethernet@2,02000000 {
> > +		ethernet@202000000 {
> >   			compatible = "smsc,lan91c111";
> >   			reg = <2 0x02000000 0x10000>;
> >   			interrupts = <15>;
> > @@ -178,7 +178,7 @@
> >   			clock-output-names = "v2m:refclk32khz";
> >   		};
> > -		iofpga@3,00000000 {
> > +		iofpga@300000000 {
> >   			compatible = "simple-bus";
> >   			#address-cells = <1>;
> >   			#size-cells = <1>;
> > diff --git a/arch/arm64/boot/dts/arm/juno-motherboard.dtsi b/arch/arm64/boot/dts/arm/juno-motherboard.dtsi
> > index e3983ded3c3c..d5cefddde08c 100644
> > --- a/arch/arm64/boot/dts/arm/juno-motherboard.dtsi
> > +++ b/arch/arm64/boot/dts/arm/juno-motherboard.dtsi
> > @@ -103,7 +103,7 @@
> >   				};
> >   			};
> > -			flash@0,00000000 {
> > +			flash@0 {
> >   				/* 2 * 32MiB NOR Flash memory mounted on CS0 */
> >   				compatible = "arm,vexpress-flash", "cfi-flash";
> >   				reg = <0 0x00000000 0x04000000>;
> > @@ -120,7 +120,7 @@
> >   				};
> >   			};
> > -			ethernet@2,00000000 {
> > +			ethernet@200000000 {
> >   				compatible = "smsc,lan9118", "smsc,lan9115";
> >   				reg = <2 0x00000000 0x10000>;
> >   				interrupts = <3>;
> > @@ -133,7 +133,7 @@
> >   				vddvario-supply = <&mb_fixed_3v3>;
> >   			};
> > -			iofpga@3,00000000 {
> > +			iofpga@300000000 {
> >   				compatible = "simple-bus";
> >   				#address-cells = <1>;
> >   				#size-cells = <1>;
> > diff --git a/arch/arm64/boot/dts/arm/rtsm_ve-motherboard-rs2.dtsi b/arch/arm64/boot/dts/arm/rtsm_ve-motherboard-rs2.dtsi
> > index 60703b5763c6..350cbf17e8b4 100644
> > --- a/arch/arm64/boot/dts/arm/rtsm_ve-motherboard-rs2.dtsi
> > +++ b/arch/arm64/boot/dts/arm/rtsm_ve-motherboard-rs2.dtsi
> > @@ -9,7 +9,7 @@
> >   		motherboard {
> >   			arm,v2m-memory-map = "rs2";
> > -			iofpga@3,00000000 {
> > +			iofpga@300000000 {
> >   				virtio-p9@140000 {
> >   					compatible = "virtio,mmio";
> >   					reg = <0x140000 0x200>;
> > diff --git a/arch/arm64/boot/dts/arm/rtsm_ve-motherboard.dtsi b/arch/arm64/boot/dts/arm/rtsm_ve-motherboard.dtsi
> > index e333c8d2d0e4..d1bfa62ca073 100644
> > --- a/arch/arm64/boot/dts/arm/rtsm_ve-motherboard.dtsi
> > +++ b/arch/arm64/boot/dts/arm/rtsm_ve-motherboard.dtsi
> > @@ -17,14 +17,14 @@
> >   			#interrupt-cells = <1>;
> >   			ranges;
> > -			flash@0,00000000 {
> > +			flash@0 {
> >   				compatible = "arm,vexpress-flash", "cfi-flash";
> >   				reg = <0 0x00000000 0x04000000>,
> >   				      <4 0x00000000 0x04000000>;
> >   				bank-width = <4>;
> >   			};
> > -			ethernet@2,02000000 {
> > +			ethernet@202000000 {
> >   				compatible = "smsc,lan91c111";
> >   				reg = <2 0x02000000 0x10000>;
> >   				interrupts = <15>;
> > @@ -51,7 +51,7 @@
> >   				clock-output-names = "v2m:refclk32khz";
> >   			};
> > -			iofpga@3,00000000 {
> > +			iofpga@300000000 {
> >   				compatible = "simple-bus";
> >   				#address-cells = <1>;
> >   				#size-cells = <1>;
> > 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

^ permalink raw reply

* Re: [PATCH 2/2] dt-bindings: nvmem: mediatek: add support for MediaTek mt8183 SoC
From: Matthias Brugger @ 2020-05-21 14:40 UTC (permalink / raw)
  To: michael.mei, orz811017, Srinivas Kandagatla, Rob Herring
  Cc: linux-mediatek, srv_heupstream, open list:OPEN FIRMWARE AND...,
	linux-kernel@vger.kernel.org
In-Reply-To: <20190416081922.21711-3-michael.mei@mediatek.com>

[adding the corresponding maintainer]

On 16/04/2019 10:19, michael.mei@mediatek.com wrote:
> From: Michael Mei <michael.mei@mediatek.com>
> 
> This updates dt-binding documentation for MediaTek mt8183
> For the both SoCs supported all rely on the fallback binding
> of the generic case with "mediatek,efuse".
> 
> Signed-off-by: Michael Mei <michael.mei@mediatek.com>

Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>

> ---
> This patch is based on v5.1-rc1 and these patches:
> 
> https://patchwork.kernel.org/patch/10856987/
> https://patchwork.kernel.org/patch/10839021/
> https://patchwork.kernel.org/patch/10879015/
> https://patchwork.kernel.org/patch/10878999/
> https://patchwork.kernel.org/patch/10858941/
> https://patchwork.kernel.org/patch/10846685/
> https://patchwork.kernel.org/patch/10893519
> ---
>  Documentation/devicetree/bindings/nvmem/mtk-efuse.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt b/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt
> index 0668c45a156d..b4d448bb60ce 100644
> --- a/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt
> +++ b/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt
> @@ -7,6 +7,7 @@ Required properties:
>  	      "mediatek,mt7622-efuse", "mediatek,efuse": for MT7622
>  	      "mediatek,mt7623-efuse", "mediatek,efuse": for MT7623
>  	      "mediatek,mt8173-efuse" or "mediatek,efuse": for MT8173
> +	      "mediatek,mt8183-efuse" or "mediatek,efuse": for MT8183
>  - reg: Should contain registers location and length
>  
>  = Data cells =
> 

^ permalink raw reply

* Re: [PATCH v7 13/24] iommu/arm-smmu-v3: Enable broadcast TLB maintenance
From: Marc Zyngier @ 2020-05-21 14:38 UTC (permalink / raw)
  To: Will Deacon
  Cc: Jean-Philippe Brucker, iommu, devicetree, linux-arm-kernel,
	linux-pci, linux-mm, joro, catalin.marinas, robin.murphy,
	kevin.tian, baolu.lu, Jonathan.Cameron, jacob.jun.pan,
	christian.koenig, felix.kuehling, zhangfei.gao, jgg, xuzaibo,
	fenghua.yu, hch
In-Reply-To: <20200521141730.GJ6608@willie-the-truck>

On 2020-05-21 15:17, Will Deacon wrote:
> [+Marc]
> 
> On Tue, May 19, 2020 at 07:54:51PM +0200, Jean-Philippe Brucker wrote:
>> The SMMUv3 can handle invalidation targeted at TLB entries with shared
>> ASIDs. If the implementation supports broadcast TLB maintenance, 
>> enable it
>> and keep track of it in a feature bit. The SMMU will then be affected 
>> by
>> inner-shareable TLB invalidations from other agents.
>> 
>> A major side-effect of this change is that stage-2 translation 
>> contexts
>> are now affected by all invalidations by VMID. VMIDs are all shared 
>> and
>> the only ways to prevent over-invalidation, since the stage-2 page 
>> tables
>> are not shared between CPU and SMMU, are to either disable BTM or 
>> allocate
>> different VMIDs. This patch does not address the problem.
> 
> This sounds like a potential performance issue, particularly as we 
> expose
> stage-2 contexts via VFIO directly. Maybe we could reserve some portion 
> of
> VMID space for the SMMU? Marc, what do you reckon?

Certainly doable when we have 16bits VMIDs. With smaller VMID spaces 
(like on
v8.0), this is a bit more difficult (we do have pretty large v8.0 
systems
around). How many VMID bits are we talking about?

         M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* Re: [PATCH v12 3/3] i2c: npcm7xx: Add support for slave mode for Nuvoton
From: Andy Shevchenko @ 2020-05-21 14:36 UTC (permalink / raw)
  To: Tali Perry
  Cc: ofery, brendanhiggins, avifishman70, tmaimon77, kfting, venture,
	yuenn, benjaminfair, robh+dt, wsa, linux-arm-kernel, linux-i2c,
	openbmc, devicetree, linux-kernel
In-Reply-To: <20200521110910.45518-4-tali.perry1@gmail.com>

On Thu, May 21, 2020 at 02:09:10PM +0300, Tali Perry wrote:
> Add support for slave mode for Nuvoton
> NPCM BMC I2C controller driver.

...

> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> +/*
> + * npcm_i2caddr array:
> + * The module supports having multiple own slave addresses.
> + * Since the addr regs are sprinkled all over the address space,
> + * use this array to get the address or each register.
> + */
> +#define I2C_NUM_OWN_ADDR 10
> +const int  npcm_i2caddr[I2C_NUM_OWN_ADDR] = {NPCM_I2CADDR1, NPCM_I2CADDR2,

Extra spaces.
On top. please start assignment from the new line.

> +					     NPCM_I2CADDR3, NPCM_I2CADDR4,
> +					     NPCM_I2CADDR5, NPCM_I2CADDR6,
> +					     NPCM_I2CADDR7, NPCM_I2CADDR8,

> +					     NPCM_I2CADDR9, NPCM_I2CADDR10};

Split }; to new line and leave comma with the last member.

> +#endif

...

> +static int  npcm_i2c_slave_enable(struct npcm_i2c *bus, enum i2c_addr addr_type,
> +				  u8 addr, bool enable)

Extra spaces. Check entire patch for that and fix accordingly.

> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> +	if (bus->slave)

> +		npcm_i2c_slave_enable(bus, I2C_SLAVE_ADDR1, bus->slave->addr,
> +				      true);

I would leave this on one line.

> +#endif

...

> +static void npcm_i2c_write_fifo_slave(struct npcm_i2c *bus, u16 max_bytes)
> +{
> +	u8 size_free_fifo;

+ blank line.

> +	/*
> +	 * Fill the FIFO, while the FIFO is not full and there are more bytes
> +	 * to write
> +	 */
> +	npcm_i2c_clear_fifo_int(bus);
> +	npcm_i2c_clear_tx_fifo(bus);
> +	iowrite8(0, bus->reg + NPCM_I2CTXF_CTL);

> +	size_free_fifo = I2C_HW_FIFO_SIZE - npcm_i2c_fifo_usage(bus);

Dup, move into loop.

> +	while (max_bytes-- && size_free_fifo) {
> +		if (bus->slv_wr_size > 0) {
> +			bus->slv_wr_ind = bus->slv_wr_ind % I2C_HW_FIFO_SIZE;
> +			npcm_i2c_wr_byte(bus, bus->slv_wr_buf[bus->slv_wr_ind]);
> +			bus->slv_wr_ind++;
> +			bus->slv_wr_ind = bus->slv_wr_ind % I2C_HW_FIFO_SIZE;
> +			bus->slv_wr_size--;
> +			size_free_fifo = I2C_HW_FIFO_SIZE -
> +					 npcm_i2c_fifo_usage(bus);
> +		} else {
> +			break;
> +		}
> +	}

	while (...) {
		if (...)
			break;
		...
	}

> +}

...

> +static int npcm_i2c_slave_get_wr_buf(struct npcm_i2c *bus)
> +{
> +	int i;

> +	u8 value = 0;

Redundant assignment.

> +	int ind;
> +	int ret = bus->slv_wr_ind;
> +
> +	/* fill a cyclic buffer */
> +	for (i = 0; i < I2C_HW_FIFO_SIZE; i++) {
> +		if (bus->slv_wr_size >= I2C_HW_FIFO_SIZE)
> +			break;
> +		i2c_slave_event(bus->slave, I2C_SLAVE_READ_REQUESTED, &value);
> +		ind = (bus->slv_wr_ind + bus->slv_wr_size) % I2C_HW_FIFO_SIZE;
> +		bus->slv_wr_buf[ind] = value;
> +		bus->slv_wr_size++;
> +		i2c_slave_event(bus->slave, I2C_SLAVE_READ_PROCESSED, &value);
> +	}
> +	return I2C_HW_FIFO_SIZE - ret;
> +}

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH v2] arm: dts: am33xx-bone-common: add gpio-line-names
From: Grygorii Strashko @ 2020-05-21 14:34 UTC (permalink / raw)
  To: Drew Fustini, Linus Walleij, Benoît Cousson, Tony Lindgren,
	Rob Herring, Linux-OMAP, devicetree, linux-kernel, Jason Kridner,
	Robert Nelson
In-Reply-To: <20200520214757.GA362547@x1>



On 21/05/2020 00:47, Drew Fustini wrote:
> Add gpio-line-names properties to the GPIO controller nodes.
> 
> BeagleBone boards have P8 and P9 headers [0] which expose many of the
> AM3358 ZCZ SoC balls to stacking expansion boards called "capes", or to
> other external connections like jumper wires connected to a breadboard.
> BeagleBone users will often refer to the "Cape Exanpsion Headers" pin
> diagram [1] as it is in the "Bone101" getting started tutorial. [2]
> 
> Most of the P8 and P9 header pins can muxed to a GPIO line.  The
> gpio-line-names describe which P8 or P9 pin that line goes to and the
> default mux for that P8 or P9 pin if it is not GPIO.
> 
> For example, gpiochip 1 line 0 is connected to P8 header pin 25 (P8_25)
> however the default device tree has the corresponding BGA ball (ZCZ U7)
> muxed to mmc1_dat0 as it is used for the on-board eMMC chip.  For that
> GPIO line to be used, one would need to modify the device tree to
> disable the eMMC and change the pin mux for that ball to GPIO mode.
> 
> Some of the AM3358 ZCZ balls corresponding to GPIO lines are not routed
> to a P8 or P9 header, but are instead wired to some peripheral device
> like on-board eMMC, HDMI framer IC, or status LEDs.  Those names are in
> brackets to denote those GPIO lines can not be used.
> 
> Some GPIO lines are named "[NC]" as the corresponding balls are not
> routed to anything on the PCB.
> 
> The goal for these names is to make it easier for a user viewing the
> output of gpioinfo to determine which P8 or P9 pin is connected to a
> GPIO line.  The output of gpioinfo on a BeagleBone Black would be:
> 
> debian@beaglebone:~$ gpioinfo
> gpiochip0 - 32 lines:
> 	line   0: "[ethernet]"       unused   input  active-high
> 	line   1: "[ethernet]"       unused   input  active-high
> 	line   2: "P9_22 [spi0_sclk]" unused input active-high
> 	line   3: "P9_21 [spi0_d0]" unused input active-high
> 	line   4: "P9_18 [spi0_d1]" unused input active-high
> 	line   5: "P9_17 [spi0_cs0]" unused input active-high
> 	line   6:  "[sd card]"         "cd"   input   active-low [used]
> 	line   7: "P9_42A [ecappwm0]" unused input active-high
> 	line   8: "P8_35 [hdmi]" unused input active-high
> 	line   9: "P8_33 [hdmi]" unused input active-high
> 	line  10: "P8_31 [hdmi]" unused input active-high
> 	line  11: "P8_32 [hdmi]" unused input active-high

[...]

> 
> [0] https://git.io/JfgOd
> [1] https://beagleboard.org/capes
> [1] https://beagleboard.org/Support/bone101
> [2] https://beagleboard.org/static/images/cape-headers.png
> 
> Reviewed-by: Jason Kridner <jason@beagleboard.org>
> Reviewed-by: Robert Nelson <robertcnelson@gmail.com>
> Signed-off-by: Drew Fustini <drew@beagleboard.org>
> ---
>   arch/arm/boot/dts/am335x-bone-common.dtsi | 144 ++++++++++++++++++++++

Not sure if it should be in am335x-bone-common.dtsi.

For example:
am335x-boneblack.dts
  #include "am335x-bone-common.dtsi"
  #include "am335x-boneblack-common.dtsi" <-- hdmi defined only here

am335x-bonegreen.dts
  #include "am335x-bone-common.dtsi"
  #include "am335x-bonegreen-common.dtsi"

>   1 file changed, 144 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/am335x-bone-common.dtsi b/arch/arm/boot/dts/am335x-bone-common.dtsi
> index 6c9187bc0f17..d86e67b0e852 100644
> --- a/arch/arm/boot/dts/am335x-bone-common.dtsi
> +++ b/arch/arm/boot/dts/am335x-bone-common.dtsi
> @@ -397,3 +397,147 @@ &rtc {
>   	clocks = <&clk_32768_ck>, <&clk_24mhz_clkctrl AM3_CLK_24MHZ_CLKDIV32K_CLKCTRL 0>;
>   	clock-names = "ext-clk", "int-clk";
>   };
> +
> +&gpio0 {
> +	gpio-line-names =
> +		"[ethernet]",
> +		"[ethernet]",
> +		"P9_22 [spi0_sclk]",
> +		"P9_21 [spi0_d0]",
> +		"P9_18 [spi0_d1]",
> +		"P9_17 [spi0_cs0]",
> +		"[sd card]",
> +		"P9_42A [ecappwm0]",
> +		"P8_35 [hdmi]",
> +		"P8_33 [hdmi]",
> +		"P8_31 [hdmi]",
> +		"P8_32 [hdmi]",

[...]

-- 
Best regards,
grygorii

^ permalink raw reply

* Re: [PATCH v12 2/3] i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver
From: Wolfram Sang @ 2020-05-21 14:31 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Tali Perry, ofery, brendanhiggins, avifishman70, tmaimon77,
	kfting, venture, yuenn, benjaminfair, robh+dt, linux-arm-kernel,
	linux-i2c, openbmc, devicetree, linux-kernel
In-Reply-To: <20200521142340.GM1634618@smile.fi.intel.com>

[-- Attachment #1: Type: text/plain, Size: 1136 bytes --]

Hi Tali, Andy!

On Thu, May 21, 2020 at 05:23:40PM +0300, Andy Shevchenko wrote:
> On Thu, May 21, 2020 at 02:09:09PM +0300, Tali Perry wrote:
> > Add Nuvoton NPCM BMC I2C controller driver.
> 
> Thanks. My comments below.
> After addressing them, FWIW,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Thanks, Andy, for all the review!

From a glimpse, this looks good to go. I will have a close look later
today.

> > +#ifdef CONFIG_DEBUG_FS
> 
> Again, why is this here?
> 
> Have you checked debugfs.h for !CONFIG_DEBUG_FS case?

I wondered also about DEBUG_FS entries. I can see their value when
developing the driver. But since this is done now, do they really help a
user to debug a difficult case? I am not sure, and then I wonder if we
should have that code in upstream. I am open for discussion, though.

> > +MODULE_VERSION("0.1.3");
> 
> Module version is defined by kernel commit hash. But it's up to you and
> subsystem maintainer to decide.

Please drop it. I also think commit id's (or even kernel versions) are a
more precise description.

Regards,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v12 2/3] i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver
From: Andy Shevchenko @ 2020-05-21 14:23 UTC (permalink / raw)
  To: Tali Perry
  Cc: ofery, brendanhiggins, avifishman70, tmaimon77, kfting, venture,
	yuenn, benjaminfair, robh+dt, wsa, linux-arm-kernel, linux-i2c,
	openbmc, devicetree, linux-kernel
In-Reply-To: <20200521110910.45518-3-tali.perry1@gmail.com>

On Thu, May 21, 2020 at 02:09:09PM +0300, Tali Perry wrote:
> Add Nuvoton NPCM BMC I2C controller driver.

Thanks. My comments below.
After addressing them, FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

...

> +	/* Frequency larger than 1 MHZ is not supported */

1 MHZ -> 1MHz

...

> +#ifdef CONFIG_DEBUG_FS

Again, why is this here?

Have you checked debugfs.h for !CONFIG_DEBUG_FS case?

> +/* i2c debugfs directory: used to keep health monitor of i2c devices */
> +static struct dentry *npcm_i2c_debugfs_dir;
> +
> +static void i2c_init_debugfs(struct platform_device *pdev, struct npcm_i2c *bus)
> +{
> +	struct dentry *d;
> +
> +	if (!npcm_i2c_debugfs_dir)
> +		return;
> +
> +	d = debugfs_create_dir(dev_name(&pdev->dev), npcm_i2c_debugfs_dir);
> +	if (IS_ERR_OR_NULL(d))
> +		return;
> +
> +	debugfs_create_u64("ber_cnt", 0444, d, &bus->ber_cnt);
> +	debugfs_create_u64("nack_cnt", 0444, d, &bus->nack_cnt);
> +	debugfs_create_u64("rec_succ_cnt", 0444, d, &bus->rec_succ_cnt);
> +	debugfs_create_u64("rec_fail_cnt", 0444, d, &bus->rec_fail_cnt);
> +	debugfs_create_u64("timeout_cnt", 0444, d, &bus->timeout_cnt);
> +
> +	bus->debugfs = d;
> +}

> +#else
> +static void i2c_init_debugfs(struct platform_device *pdev, struct npcm_i2c *bus)
> +{
> +}

This is completely redundant.

> +#endif

...

> +#ifdef CONFIG_DEBUG_FS

Ditto.

> +static int __init npcm_i2c_init(void)
> +{
> +	struct dentry *dir;
> +
> +	dir = debugfs_create_dir("i2c", NULL);
> +	if (IS_ERR_OR_NULL(dir))
> +		return 0;
> +
> +	npcm_i2c_debugfs_dir = dir;
> +	return 0;
> +}
> +
> +static void __exit npcm_i2c_exit(void)
> +{
> +	debugfs_remove_recursive(npcm_i2c_debugfs_dir);
> +}
> +
> +module_init(npcm_i2c_init);
> +module_exit(npcm_i2c_exit);
> +#endif

...

> +MODULE_VERSION("0.1.3");

Module version is defined by kernel commit hash. But it's up to you and
subsystem maintainer to decide.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH v4 7/7] clocksource: mips-gic-timer: Mark GIC timer as unstable if ref clock changes
From: Serge Semin @ 2020-05-21 14:22 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Serge Semin, Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano,
	Alexey Malahov, Paul Burton, Ralf Baechle, Alessandro Zummo,
	Alexandre Belloni, Arnd Bergmann, Rob Herring,
	open list:BROADCOM NVRAM DRIVER, linux-rtc,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Paul Cercueil, Krzysztof Kozlowski, Geert Uytterhoeven,
	Randy Dunlap, Claudiu Beznea, Maarten ter Huurne,
	Vincenzo Frascino, Linux Kernel Mailing List
In-Reply-To: <CAMuHMdW5TqfDTZZCscXCK-Fkd7Gq1Ciyu1_sDzzR0B+_W-2hfg@mail.gmail.com>

On Thu, May 21, 2020 at 11:09:50AM +0200, Geert Uytterhoeven wrote:
> Hi Serge,
> 
> On Thu, May 21, 2020 at 2:54 AM Serge Semin
> <Sergey.Semin@baikalelectronics.ru> wrote:
> > Currently clocksource framework doesn't support the clocks with variable
> > frequency. Since MIPS GIC timer ticks rate might be unstable on some
> > platforms, we must make sure that it justifies the clocksource
> > requirements. MIPS GIC timer is incremented with the CPU cluster reference
> > clocks rate. So in case if CPU frequency changes, the MIPS GIC tick rate
> > changes synchronously. Due to this the clocksource subsystem can't rely on
> > the timer to measure system clocks anymore. This commit marks the MIPS GIC
> > based clocksource as unstable if reference clock (normally it's a CPU
> > reference clocks) rate changes. The clocksource will execute a watchdog
> > thread, which lowers the MIPS GIC timer rating to zero and fallbacks to a
> > new stable one.
> >
> > Note we don't need to set the CLOCK_SOURCE_MUST_VERIFY flag to the MIPS
> > GIC clocksource since normally the timer is stable. The only reason why
> > it gets unstable is due to the ref clock rate change, which event we
> > detect here in the driver by means of the clocks event notifier.
> >
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> Thanks for your patch!
> 
> > --- a/drivers/clocksource/mips-gic-timer.c
> > +++ b/drivers/clocksource/mips-gic-timer.c
> > @@ -24,6 +24,9 @@
> >  static DEFINE_PER_CPU(struct clock_event_device, gic_clockevent_device);
> >  static int gic_timer_irq;
> >  static unsigned int gic_frequency;
> > +static bool __read_mostly gic_clock_unstable;
> > +
> > +static void git_clocksource_unstable(char *reason);
> 
> gic_clocksource_unstable? (everywhere)

This is the most used word lately. So my hands write git everywhere by itself.)
Thanks for noticing this.

-Sergey

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v7 13/24] iommu/arm-smmu-v3: Enable broadcast TLB maintenance
From: Will Deacon @ 2020-05-21 14:17 UTC (permalink / raw)
  To: Jean-Philippe Brucker
  Cc: iommu, devicetree, linux-arm-kernel, linux-pci, linux-mm, joro,
	catalin.marinas, robin.murphy, kevin.tian, baolu.lu,
	Jonathan.Cameron, jacob.jun.pan, christian.koenig, felix.kuehling,
	zhangfei.gao, jgg, xuzaibo, fenghua.yu, hch, maz
In-Reply-To: <20200519175502.2504091-14-jean-philippe@linaro.org>

[+Marc]

On Tue, May 19, 2020 at 07:54:51PM +0200, Jean-Philippe Brucker wrote:
> The SMMUv3 can handle invalidation targeted at TLB entries with shared
> ASIDs. If the implementation supports broadcast TLB maintenance, enable it
> and keep track of it in a feature bit. The SMMU will then be affected by
> inner-shareable TLB invalidations from other agents.
> 
> A major side-effect of this change is that stage-2 translation contexts
> are now affected by all invalidations by VMID. VMIDs are all shared and
> the only ways to prevent over-invalidation, since the stage-2 page tables
> are not shared between CPU and SMMU, are to either disable BTM or allocate
> different VMIDs. This patch does not address the problem.

This sounds like a potential performance issue, particularly as we expose
stage-2 contexts via VFIO directly. Maybe we could reserve some portion of
VMID space for the SMMU? Marc, what do you reckon?

Will

^ permalink raw reply

* Re: [PATCH V1 RESEND 1/3] perf/imx_ddr: Add system PMU identifier for userspace
From: John Garry @ 2020-05-21 14:16 UTC (permalink / raw)
  To: Mark Rutland, Will Deacon
  Cc: Rob Herring, Joakim Zhang, shawnguo, linux-imx, linux-arm-kernel,
	devicetree, linux-kernel
In-Reply-To: <20200521132641.GB47848@C02TD0UTHF1T.local>

On 21/05/2020 14:26, Mark Rutland wrote:
> On Wed, May 20, 2020 at 08:33:04AM +0100, Will Deacon wrote:
>> On Tue, May 19, 2020 at 12:51:25PM -0600, Rob Herring wrote:
>>> On Tue, May 12, 2020 at 03:31:13PM +0800, Joakim Zhang wrote:
>>>> +static ssize_t ddr_perf_identifier_show(struct device *dev,
>>>> +					struct device_attribute *attr,
>>>> +					char *page)
>>>> +{
>>>> +	struct ddr_pmu *pmu = dev_get_drvdata(dev);
>>>> +
>>>> +	return sprintf(page, "%s\n", pmu->devtype_data->identifier);
>>>
>>> Why do we need yet another way to identify the SoC from userspace?
>>
>> I also really dislike this. What's the preferred way to identify the SoC
>> from userspace? It's needed so that the perf userspace tool can describe
>> perf events that are supported for the PMU, as this isn't probe-able
>> directly from the hardware. We have the same issue with the SMMUv3 PMCG [1],
>> and so we need to solve the problem for both DT and ACPI.
> 
> Worth noting that while in this case it happens to identify the SoC,
> in general you can have distinct instances of system IP in a single
> system, so I do think that we need *something* instance-specific, even
> if that's combined with SoC info.
> 

Hi Mark,

> Where IP gets reused across SoCs, it makes sense for that to not depend
> on top-level SoC info.

This would be quite an uncommon case. Generally most instances of a 
given PMU in a SoC would be identical implementations.

And anyway, we should be able to solve that problem in perf tool, as 
long as the PMU device name is fixed. Like what we have for the SMMUv3 
PMU, where the device name contains the device bus address, i.e don't 
use idr for perf drivers device naming....

Thanks,
John

^ permalink raw reply

* Re: [PATCH v7 00/24] iommu: Shared Virtual Addressing for SMMUv3
From: Will Deacon @ 2020-05-21 14:17 UTC (permalink / raw)
  To: Jean-Philippe Brucker
  Cc: devicetree, kevin.tian, jacob.jun.pan, jgg, linux-pci, joro,
	Jonathan.Cameron, fenghua.yu, hch, linux-mm, iommu, zhangfei.gao,
	catalin.marinas, felix.kuehling, xuzaibo, robin.murphy,
	christian.koenig, linux-arm-kernel, baolu.lu
In-Reply-To: <20200521103513.GE5360@willie-the-truck>

On Thu, May 21, 2020 at 11:35:14AM +0100, Will Deacon wrote:
> On Tue, May 19, 2020 at 07:54:38PM +0200, Jean-Philippe Brucker wrote:
> > Shared Virtual Addressing (SVA) allows to share process page tables with
> > devices using the IOMMU, PASIDs and I/O page faults. Add SVA support to
> > the Arm SMMUv3 driver.
> > 
> > Since v6 [1]:
> > * Rename ioasid_free() to ioasid_put() in patch 02, requiring changes to
> >   the Intel drivers.
> > * Use mmu_notifier_register() in patch 16 to avoid copying the ops and
> >   simplify the invalidate() notifier in patch 17.
> > * As a result, replace context spinlock with a mutex. Simplified locking in
> >   patch 11 (That patch still looks awful, but I think the series is more
> >   readable overall). And I've finally been able to remove the GFP_ATOMIC
> >   allocations.
> > * Use a single patch (04) for io-pgfault.c, since the code was simplified
> >   in v6. Fixed partial list in patch 04.
> 
> There's an awful lot here and it stretches across quite a few subsystems,
> with different git trees. What's the plan for merging it?
> 
> I'm happy to take some of the arm64 and smmu changes for 5.8, then perhaps
> we can review what's left and target 5.9? It would also be helpful to split
> that up into separate series where there aren't strong dependencies, I
> think.

Hmm, so the way the series is structured makes it quite difficult to apply
much of this at all :(

I've taken patch 5 into the arm64 tree and patch 8 into the smmu tree. I'll
leave a couple of Acks on some of the simpler patches, but I think this
really needs splitting up a bit to make it more manageable.

I also notice a bunch of TODOs that get introduced and then removed. Given
that the series needs to be bisectable, these shouldn't be needed and can
just be removed.

Thanks,

Will

^ permalink raw reply

* Re: [PATCH v7 14/24] iommu/arm-smmu-v3: Add SVA feature checking
From: Will Deacon @ 2020-05-21 14:17 UTC (permalink / raw)
  To: Jean-Philippe Brucker
  Cc: iommu, devicetree, linux-arm-kernel, linux-pci, linux-mm, joro,
	catalin.marinas, robin.murphy, kevin.tian, baolu.lu,
	Jonathan.Cameron, jacob.jun.pan, christian.koenig, felix.kuehling,
	zhangfei.gao, jgg, xuzaibo, fenghua.yu, hch, Suzuki K Poulose
In-Reply-To: <20200519175502.2504091-15-jean-philippe@linaro.org>

On Tue, May 19, 2020 at 07:54:52PM +0200, Jean-Philippe Brucker wrote:
> Aggregate all sanity-checks for sharing CPU page tables with the SMMU
> under a single ARM_SMMU_FEAT_SVA bit. For PCIe SVA, users also need to
> check FEAT_ATS and FEAT_PRI. For platform SVA, they will most likely have
> to check FEAT_STALLS.
> 
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>  drivers/iommu/arm-smmu-v3.c | 72 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 9332253e3608..a9f6f1d7014e 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -660,6 +660,7 @@ struct arm_smmu_device {
>  #define ARM_SMMU_FEAT_RANGE_INV		(1 << 15)
>  #define ARM_SMMU_FEAT_E2H		(1 << 16)
>  #define ARM_SMMU_FEAT_BTM		(1 << 17)
> +#define ARM_SMMU_FEAT_SVA		(1 << 18)
>  	u32				features;
>  
>  #define ARM_SMMU_OPT_SKIP_PREFETCH	(1 << 0)
> @@ -3935,6 +3936,74 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass)
>  	return 0;
>  }
>  
> +static bool arm_smmu_supports_sva(struct arm_smmu_device *smmu)
> +{
> +	unsigned long reg, fld;
> +	unsigned long oas;
> +	unsigned long asid_bits;
> +
> +	u32 feat_mask = ARM_SMMU_FEAT_BTM | ARM_SMMU_FEAT_COHERENCY;

Aha -- here's the coherency check I missed!

> +
> +	if ((smmu->features & feat_mask) != feat_mask)
> +		return false;
> +
> +	if (!(smmu->pgsize_bitmap & PAGE_SIZE))
> +		return false;
> +
> +	/*
> +	 * Get the smallest PA size of all CPUs (sanitized by cpufeature). We're
> +	 * not even pretending to support AArch32 here.
> +	 */
> +	reg = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1);
> +	fld = cpuid_feature_extract_unsigned_field(reg, ID_AA64MMFR0_PARANGE_SHIFT);
> +	switch (fld) {
> +	case 0x0:
> +		oas = 32;
> +		break;
> +	case 0x1:
> +		oas = 36;
> +		break;
> +	case 0x2:
> +		oas = 40;
> +		break;
> +	case 0x3:
> +		oas = 42;
> +		break;
> +	case 0x4:
> +		oas = 44;
> +		break;
> +	case 0x5:
> +		oas = 48;
> +		break;
> +	case 0x6:

We can use ID_AA64MMFR0_PARANGE_xx constants instead of the hardcoded hex
numbers here.

With that:

Acked-by: Will Deacon <will@kernel.org>

Will

^ permalink raw reply


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