Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v7 2/4] usb: dwc3: qcom: Add interconnect support in dwc3 driver
From: Stephen Boyd @ 2020-06-03 17:36 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Doug Anderson, Felipe Balbi,
	Greg Kroah-Hartman, Mark Rutland, Matthias Kaehlcke, Rob Herring,
	Sandeep Maheswaram
  Cc: linux-arm-msm, linux-usb, devicetree, linux-kernel, Manu Gautam,
	Chandana Kishori Chiluveru, Sandeep Maheswaram
In-Reply-To: <1585718145-29537-3-git-send-email-sanm@codeaurora.org>

Quoting Sandeep Maheswaram (2020-03-31 22:15:43)
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index 1dfd024..d33ae86 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -76,8 +85,13 @@ struct dwc3_qcom {
>         enum usb_dr_mode        mode;
>         bool                    is_suspended;
>         bool                    pm_suspended;
> +       struct icc_path         *usb_ddr_icc_path;
> +       struct icc_path         *apps_usb_icc_path;
>  };
>  
> +static int dwc3_qcom_interconnect_enable(struct dwc3_qcom *qcom);
> +static int dwc3_qcom_interconnect_disable(struct dwc3_qcom *qcom);

Please get rid of these. We shouldn't need forward declarations.

> +
>  static inline void dwc3_qcom_setbits(void __iomem *base, u32 offset, u32 val)
>  {
>         u32 reg;
> @@ -285,6 +307,101 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom)
>         return 0;
>  }
>  
> +
> +/**
> + * dwc3_qcom_interconnect_init() - Get interconnect path handles
> + * @qcom:                      Pointer to the concerned usb core.
> + *
> + */
> +static int dwc3_qcom_interconnect_init(struct dwc3_qcom *qcom)
> +{
> +       struct device *dev = qcom->dev;
> +       int ret;
> +
> +       if (!device_is_bound(&qcom->dwc3->dev))
> +               return -EPROBE_DEFER;

How is this supposed to work? I see that this was added in an earlier
revision of this patch series but there isn't any mention of why
device_is_bound() is used here. It would be great if there was a comment
detailing why this is necessary. It sounds like maximum_speed is
important?

Furthermore, dwc3_qcom_interconnect_init() is called by
dwc3_qcom_probe() which is the function that registers the device for
qcom->dwc3->dev. If that device doesn't probe between the time it is
registered by dwc3_qcom_probe() and this function is called then we'll
fail dwc3_qcom_probe() with -EPROBE_DEFER. And that will remove the
qcom->dwc3->dev device from the platform bus because we call
of_platform_depopulate() on the error path of dwc3_qcom_probe().

So isn't this whole thing racy and can potentially lead us to a driver
probe loop where the wrapper (dwc3_qcom) and the core (dwc3) are probing
and we're trying to time it just right so that driver for dwc3 binds
before we setup interconnects? I don't know if dwc3 can communicate to
the wrapper but that would be more of a direct way to do this. Or maybe
the wrapper should try to read the DT property for maximum speed and
fallback to a worst case high bandwidth value if it can't figure it out
itself without help from dwc3 core.

^ permalink raw reply

* Re: [PATCH 2/2] drm/panel: simple: Add support for KOE TX26D202VM0BWA panel
From: Emil Velikov @ 2020-06-03 17:15 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Liu Ying, devicetree, Thierry Reding, NXP Linux Team,
	ML dri-devel
In-Reply-To: <20200602205653.GC56418@ravnborg.org>

On Tue, 2 Jun 2020 at 21:57, Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Emil.
>
> On Tue, Jun 02, 2020 at 01:46:19PM +0100, Emil Velikov wrote:
> > On Tue, 2 Jun 2020 at 08:17, Liu Ying <victor.liu@nxp.com> wrote:
> > >
> > > This patch adds support for Kaohsiung Opto-Electronics Inc.
> > > 10.1" TX26D202VM0BWA WUXGA(1920x1200) TFT LCD panel with LVDS interface.
> > > The panel has dual LVDS channels.
> > >
> > > My panel is manufactured by US Micro Products(USMP).  There is a tag at
> > > the back of the panel, which indicates the panel type is 'TX26D202VM0BWA'
> > > and it's made by KOE in Taiwan.
> > >
> > > The panel spec from USMP can be found at:
> > > https://www.usmicroproducts.com/sites/default/files/datasheets/USMP-T101-192120NDU-A0.pdf
> > >
> > > The below panel spec from KOE is basically the same to the one from USMP.
> > > However, the panel type 'TX26D202VM0BAA' is a little bit different.
> > > It looks that the two types of panel are compatible with each other.
> > > http://www.koe.j-display.com/upload/product/TX26D202VM0BAA.pdf
> > >
> > > Cc: Thierry Reding <thierry.reding@gmail.com>
> > > Cc: Sam Ravnborg <sam@ravnborg.org>
> > > Signed-off-by: Liu Ying <victor.liu@nxp.com>
> > > ---
> > >  drivers/gpu/drm/panel/panel-simple.c | 34 ++++++++++++++++++++++++++++++++++
> > >  1 file changed, 34 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> > > index b6ecd15..7c222ec 100644
> > > --- a/drivers/gpu/drm/panel/panel-simple.c
> > > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > > @@ -2200,6 +2200,37 @@ static const struct panel_desc koe_tx14d24vm1bpa = {
> > >         },
> > >  };
> > >
> > > +static const struct display_timing koe_tx26d202vm0bwa_timing = {
> > > +       .pixelclock = { 151820000, 156720000, 159780000 },
> > > +       .hactive = { 1920, 1920, 1920 },
> > > +       .hfront_porch = { 105, 130, 142 },
> > > +       .hback_porch = { 45, 70, 82 },
> > > +       .hsync_len = { 30, 30, 30 },
> > > +       .vactive = { 1200, 1200, 1200},
> > > +       .vfront_porch = { 3, 5, 10 },
> > > +       .vback_porch = { 2, 5, 10 },
> > > +       .vsync_len = { 5, 5, 5 },
> > > +};
> > > +
> > > +static const struct panel_desc koe_tx26d202vm0bwa = {
> > > +       .timings = &koe_tx26d202vm0bwa_timing,
> > > +       .num_timings = 1,
> > > +       .bpc = 8,
> > > +       .size = {
> > > +               .width = 217,
> > > +               .height = 136,
> > > +       },
> > > +       .delay = {
> > > +               .prepare = 1000,
> > > +               .enable = 1000,
> > > +               .unprepare = 1000,
> > > +               .disable = 1000,
> > Ouch 1s for each delay is huge. Nevertheless it matches the specs so,
> > the series is:
> > Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
> >
> > Sam, Thierry I assume you'll merge the series. Let me know if I should
> > pick it up.
> I am quite busy with non-linux stuff these days so fine if you can pick
> them up. I like that simple panel patches are processed fast.
>
> I expect to have some hours for linux work friday or saturday, but no
> promises...
>
Don't worry - once the DT maintainers ack 1/2, I'll merge the series.

-Emil

^ permalink raw reply

* Re: [PATCH v3 6/6] MAINTAINERS: Add maintainers for MIPS core drivers
From: Serge Semin @ 2020-06-03 17:13 UTC (permalink / raw)
  To: Thomas Gleixner, Daniel Lezcano, Rafael J. Wysocki
  Cc: Serge Semin, Thomas Bogendoerfer, Marc Zyngier,
	Greg Kroah-Hartman, Alexey Malahov, Paul Burton, Rob Herring,
	Arnd Bergmann, Jason Cooper, James Hogan, linux-mips, devicetree,
	linux-kernel
In-Reply-To: <20200603000318.2xgizrgwauw2ywwu@mobilestation>

Daniel, Rafael, Thomas (Gleixner),

Could you take a look at the series this patch belongs to and to this patch in
particular? If you are ok with this, please explicitly ack. It would be great to
merge the leftover series in this merge window.

-Sergey

On Wed, Jun 03, 2020 at 03:03:19AM +0300, Serge Semin wrote:
> On Tue, Jun 02, 2020 at 11:12:31AM +0100, Marc Zyngier wrote:
> > On 2020-06-02 11:09, Serge Semin wrote:
> > > Add Thomas and myself as maintainers of the MIPS CPU and GIC IRQchip,
> > > MIPS
> > > GIC timer and MIPS CPS CPUidle drivers.
> > > 
> > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > 
> > > ---
> > > 
> > > Changelog v3:
> > > - Keep the files list alphabetically ordered.
> > > - Add Thomas as the co-maintainer of the designated drivers.
> > > ---
> > >  MAINTAINERS | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > > 
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 2926327e4976..20532e0287d7 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -11278,6 +11278,17 @@
> > > F:	arch/mips/configs/generic/board-boston.config
> > >  F:	drivers/clk/imgtec/clk-boston.c
> > >  F:	include/dt-bindings/clock/boston-clock.h
> > > 
> > > +MIPS CORE DRIVERS
> > > +M:	Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > > +M:	Serge Semin <fancer.lancer@gmail.com>
> > > +L:	linux-mips@vger.kernel.org
> > > +S:	Supported
> > > +F:	drivers/bus/mips_cdmm.c
> > > +F:	drivers/clocksource/mips-gic-timer.c
> > > +F:	drivers/cpuidle/cpuidle-cps.c
> > > +F:	drivers/irqchip/irq-mips-cpu.c
> > > +F:	drivers/irqchip/irq-mips-gic.c
> > > +
> > >  MIPS GENERIC PLATFORM
> > >  M:	Paul Burton <paulburton@kernel.org>
> > >  L:	linux-mips@vger.kernel.org
> > 
> > Acked-by: Marc Zyngier <maz@kernel.org>
> > 
> > I assume this will go via the MIPS tree.
> 
> Yes, I also think so. Though I suppose first we have to get acks from
> Rafael J. Wysocki (CPU IDLE) or Daniel Lezcano (CPU IDLE,
> CLOCKSOURCE/CLOCKEVENT) or Thomas Gleixner (CLOCKSOURCE, CLOCKEVENT)
> since we are going to maintain the drivers from the subsystems they
> support. Am I right?
> 
> -Sergey
> 
> > 
> > Thanks,
> > 
> >         M.
> > -- 
> > Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH] arm: dts: vexpress: Move mcc node back into motherboard node
From: Andre Przywara @ 2020-06-03 16:22 UTC (permalink / raw)
  To: Rob Herring, Liviu Dudau, Sudeep Holla, Lorenzo Pieralisi
  Cc: devicetree, linux-arm-kernel, Guenter Roeck

Commit 	d9258898ad49 ("arm64: dts: arm: vexpress: Move fixed devices
out of bus node") moved the "mcc" DT node into the root node, because
it does not have any children using "reg" properties, so does violate
some dtc checks about "simple-bus" nodes.
However this broke the vexpress config-bus code, which walks up the
device tree to find the first node with an "arm,vexpress,site" property.
This gave the wrong result (matching the root node instead of the
motherboard node), so broke the clocks and some other devices for
VExpress boards.

Move the whole node back into its original position. This re-introduces
the dtc warning, but is conceptually the right thing to do. The dtc
warning seems to be overzealous here, there are discussions on fixing or
relaxing this check instead.

Fixes: 	d9258898ad49 ("arm64: dts: arm: vexpress: Move fixed devices out of bus node")
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
P.S. The broken commit has not reached mainline yet, but is already in
arm-soc/arm/dt.

 arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 146 ++++++++++++------------
 1 file changed, 73 insertions(+), 73 deletions(-)

diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
index e6308fb76183..a88ee5294d35 100644
--- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
+++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
@@ -100,79 +100,6 @@
 		};
 	};
 
-	mcc {
-		compatible = "arm,vexpress,config-bus";
-		arm,vexpress,config-bridge = <&v2m_sysreg>;
-
-		oscclk0 {
-			/* MCC static memory clock */
-			compatible = "arm,vexpress-osc";
-			arm,vexpress-sysreg,func = <1 0>;
-			freq-range = <25000000 60000000>;
-			#clock-cells = <0>;
-			clock-output-names = "v2m:oscclk0";
-		};
-
-		v2m_oscclk1: oscclk1 {
-			/* CLCD clock */
-			compatible = "arm,vexpress-osc";
-			arm,vexpress-sysreg,func = <1 1>;
-			freq-range = <23750000 65000000>;
-			#clock-cells = <0>;
-			clock-output-names = "v2m:oscclk1";
-		};
-
-		v2m_oscclk2: oscclk2 {
-			/* IO FPGA peripheral clock */
-			compatible = "arm,vexpress-osc";
-			arm,vexpress-sysreg,func = <1 2>;
-			freq-range = <24000000 24000000>;
-			#clock-cells = <0>;
-			clock-output-names = "v2m:oscclk2";
-		};
-
-		volt-vio {
-			/* Logic level voltage */
-			compatible = "arm,vexpress-volt";
-			arm,vexpress-sysreg,func = <2 0>;
-			regulator-name = "VIO";
-			regulator-always-on;
-			label = "VIO";
-		};
-
-		temp-mcc {
-			/* MCC internal operating temperature */
-			compatible = "arm,vexpress-temp";
-			arm,vexpress-sysreg,func = <4 0>;
-			label = "MCC";
-		};
-
-		reset {
-			compatible = "arm,vexpress-reset";
-			arm,vexpress-sysreg,func = <5 0>;
-		};
-
-		muxfpga {
-			compatible = "arm,vexpress-muxfpga";
-			arm,vexpress-sysreg,func = <7 0>;
-		};
-
-		shutdown {
-			compatible = "arm,vexpress-shutdown";
-			arm,vexpress-sysreg,func = <8 0>;
-		};
-
-		reboot {
-			compatible = "arm,vexpress-reboot";
-			arm,vexpress-sysreg,func = <9 0>;
-		};
-
-		dvimode {
-			compatible = "arm,vexpress-dvimode";
-			arm,vexpress-sysreg,func = <11 0>;
-		};
-	};
-
 	bus@8000000 {
 		motherboard-bus {
 			model = "V2M-P1";
@@ -435,6 +362,79 @@
 						};
 					};
 				};
+
+				mcc {
+					compatible = "arm,vexpress,config-bus";
+					arm,vexpress,config-bridge = <&v2m_sysreg>;
+
+					oscclk0 {
+						/* MCC static memory clock */
+						compatible = "arm,vexpress-osc";
+						arm,vexpress-sysreg,func = <1 0>;
+						freq-range = <25000000 60000000>;
+						#clock-cells = <0>;
+						clock-output-names = "v2m:oscclk0";
+					};
+
+					v2m_oscclk1: oscclk1 {
+						/* CLCD clock */
+						compatible = "arm,vexpress-osc";
+						arm,vexpress-sysreg,func = <1 1>;
+						freq-range = <23750000 65000000>;
+						#clock-cells = <0>;
+						clock-output-names = "v2m:oscclk1";
+					};
+
+					v2m_oscclk2: oscclk2 {
+						/* IO FPGA peripheral clock */
+						compatible = "arm,vexpress-osc";
+						arm,vexpress-sysreg,func = <1 2>;
+						freq-range = <24000000 24000000>;
+						#clock-cells = <0>;
+						clock-output-names = "v2m:oscclk2";
+					};
+
+					volt-vio {
+						/* Logic level voltage */
+						compatible = "arm,vexpress-volt";
+						arm,vexpress-sysreg,func = <2 0>;
+						regulator-name = "VIO";
+						regulator-always-on;
+						label = "VIO";
+					};
+
+					temp-mcc {
+						/* MCC internal operating temperature */
+						compatible = "arm,vexpress-temp";
+						arm,vexpress-sysreg,func = <4 0>;
+						label = "MCC";
+					};
+
+					reset {
+						compatible = "arm,vexpress-reset";
+						arm,vexpress-sysreg,func = <5 0>;
+					};
+
+					muxfpga {
+						compatible = "arm,vexpress-muxfpga";
+						arm,vexpress-sysreg,func = <7 0>;
+					};
+
+					shutdown {
+						compatible = "arm,vexpress-shutdown";
+						arm,vexpress-sysreg,func = <8 0>;
+					};
+
+					reboot {
+						compatible = "arm,vexpress-reboot";
+						arm,vexpress-sysreg,func = <9 0>;
+					};
+
+					dvimode {
+						compatible = "arm,vexpress-dvimode";
+						arm,vexpress-sysreg,func = <11 0>;
+					};
+				};
 			};
 		};
 	};
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH] dt-bindings: rtc: Convert imxdi rtc to json-schema
From: Rob Herring @ 2020-06-03 16:15 UTC (permalink / raw)
  To: Anson Huang
  Cc: Alessandro Zummo, Alexandre Belloni, Shawn Guo, Sascha Hauer,
	Sascha Hauer, Fabio Estevam, Roland Stigge,
	open list:REAL TIME CLOCK (RTC) SUBSYSTEM, devicetree,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel@vger.kernel.org, NXP Linux Team
In-Reply-To: <1591184925-13055-1-git-send-email-Anson.Huang@nxp.com>

On Wed, Jun 3, 2020 at 5:59 AM Anson Huang <Anson.Huang@nxp.com> wrote:
>
> Convert the i.MXDI RTC binding to DT schema format using json-schema
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
>  .../devicetree/bindings/rtc/imxdi-rtc.txt          | 20 -----------
>  .../devicetree/bindings/rtc/imxdi-rtc.yaml         | 42 ++++++++++++++++++++++
>  2 files changed, 42 insertions(+), 20 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/rtc/imxdi-rtc.txt
>  create mode 100644 Documentation/devicetree/bindings/rtc/imxdi-rtc.yaml
>
> diff --git a/Documentation/devicetree/bindings/rtc/imxdi-rtc.txt b/Documentation/devicetree/bindings/rtc/imxdi-rtc.txt
> deleted file mode 100644
> index c797bc9..0000000
> --- a/Documentation/devicetree/bindings/rtc/imxdi-rtc.txt
> +++ /dev/null
> @@ -1,20 +0,0 @@
> -* i.MX25 Real Time Clock controller
> -
> -Required properties:
> -- compatible: should be: "fsl,imx25-rtc"
> -- reg: physical base address of the controller and length of memory mapped
> -  region.
> -- clocks: should contain the phandle for the rtc clock
> -- interrupts: rtc alarm interrupt
> -
> -Optional properties:
> -- interrupts: dryice security violation interrupt (second entry)
> -
> -Example:
> -
> -rtc@53ffc000 {
> -       compatible = "fsl,imx25-rtc";
> -       reg = <0x53ffc000 0x4000>;
> -       clocks = <&clks 81>;
> -       interrupts = <25 56>;
> -};
> diff --git a/Documentation/devicetree/bindings/rtc/imxdi-rtc.yaml b/Documentation/devicetree/bindings/rtc/imxdi-rtc.yaml
> new file mode 100644
> index 0000000..6e43926
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/imxdi-rtc.yaml
> @@ -0,0 +1,42 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/rtc/imxdi-rtc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: i.MX25 Real Time Clock controller
> +
> +maintainers:
> +  - Roland Stigge <stigge@antcom.de>
> +
> +properties:
> +  compatible:
> +    const: fsl,imx25-rtc
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    items:
> +      - description: rtc alarm interrupt
> +      - description: dryice security violation interrupt
> +    minItems: 1
> +    maxItems: 2
> +
> +  clocks:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks

Needs:

additionalProperties: false

(or if you have a top level $ref, 'unevaluatedProperties: false')

I fixed these up in what I applied already, but please check all of
yours pending and fix.

Rob

^ permalink raw reply

* [PATCH V3 1/3] clk: vc5: Allow Versaclock driver to support multiple instances
From: Adam Ford @ 2020-06-03 15:43 UTC (permalink / raw)
  To: linux-clk
  Cc: Adam Ford, Michael Turquette, Stephen Boyd, Rob Herring,
	Marek Vasut, devicetree, linux-kernel

Currently, the Versaclock driver is only expecting one instance and
uses hard-coded names for the various clock names.  Unfortunately,
this is a problem when there is more than one instance of the driver,
because the subsequent instantiations of the driver use the identical
name.  Each clock after the fist fails to load, because the clock
subsystem cannot handle two clocks with identical name.

This patch removes the hard-coded name arrays and uses kasprintf to
assign clock names based on names of their respective node and parent
node which gives each clock a unique identifying name.

For a verasaclock node with a name like:
   versaclock5: versaclock_som@6a

The updated clock names would appear like:
    versaclock_som.mux
       versaclock_som.out0_sel_i2cb
       versaclock_som.pfd
          versaclock_som.pll
             versaclock_som.fod3
                versaclock_som.out4
             versaclock_som.fod2
                versaclock_som.out3
             versaclock_som.fod1
                versaclock_som.out2
             versaclock_som.fod0
                versaclock_som.out1

Signed-off-by: Adam Ford <aford173@gmail.com>
---
V3:  Remove 'probed' message
V2:  No change

diff --git a/drivers/clk/clk-versaclock5.c b/drivers/clk/clk-versaclock5.c
index fa96659f8023..41e3a75963b9 100644
--- a/drivers/clk/clk-versaclock5.c
+++ b/drivers/clk/clk-versaclock5.c
@@ -161,30 +161,6 @@ struct vc5_driver_data {
 	struct vc5_hw_data	clk_out[VC5_MAX_CLK_OUT_NUM];
 };
 
-static const char * const vc5_mux_names[] = {
-	"mux"
-};
-
-static const char * const vc5_dbl_names[] = {
-	"dbl"
-};
-
-static const char * const vc5_pfd_names[] = {
-	"pfd"
-};
-
-static const char * const vc5_pll_names[] = {
-	"pll"
-};
-
-static const char * const vc5_fod_names[] = {
-	"fod0", "fod1", "fod2", "fod3",
-};
-
-static const char * const vc5_clk_out_names[] = {
-	"out0_sel_i2cb", "out1", "out2", "out3", "out4",
-};
-
 /*
  * VersaClock5 i2c regmap
  */
@@ -692,8 +668,7 @@ static int vc5_map_index_to_output(const enum vc5_model model,
 
 static const struct of_device_id clk_vc5_of_match[];
 
-static int vc5_probe(struct i2c_client *client,
-		     const struct i2c_device_id *id)
+static int vc5_probe(struct i2c_client *client, const struct i2c_device_id *id)
 {
 	struct vc5_driver_data *vc5;
 	struct clk_init_data init;
@@ -742,7 +717,7 @@ static int vc5_probe(struct i2c_client *client,
 	if (!IS_ERR(vc5->pin_clkin)) {
 		vc5->clk_mux_ins |= VC5_MUX_IN_CLKIN;
 		parent_names[init.num_parents++] =
-			__clk_get_name(vc5->pin_clkin);
+		    __clk_get_name(vc5->pin_clkin);
 	}
 
 	if (!init.num_parents) {
@@ -750,12 +725,13 @@ static int vc5_probe(struct i2c_client *client,
 		return -EINVAL;
 	}
 
-	init.name = vc5_mux_names[0];
+	init.name = kasprintf(GFP_KERNEL, "%pOFn.mux", client->dev.of_node);
 	init.ops = &vc5_mux_ops;
 	init.flags = 0;
 	init.parent_names = parent_names;
 	vc5->clk_mux.init = &init;
 	ret = devm_clk_hw_register(&client->dev, &vc5->clk_mux);
+	kfree(init.name);	/* clock framework made a copy of the name */
 	if (ret) {
 		dev_err(&client->dev, "unable to register %s\n", init.name);
 		goto err_clk;
@@ -764,13 +740,16 @@ static int vc5_probe(struct i2c_client *client,
 	if (vc5->chip_info->flags & VC5_HAS_PFD_FREQ_DBL) {
 		/* Register frequency doubler */
 		memset(&init, 0, sizeof(init));
-		init.name = vc5_dbl_names[0];
+		init.name = kasprintf(GFP_KERNEL, "%pOFn.dbl",
+				      client->dev.of_node);
 		init.ops = &vc5_dbl_ops;
 		init.flags = CLK_SET_RATE_PARENT;
-		init.parent_names = vc5_mux_names;
+		init.parent_names = parent_names;
+		parent_names[0] = clk_hw_get_name(&vc5->clk_mux);
 		init.num_parents = 1;
 		vc5->clk_mul.init = &init;
 		ret = devm_clk_hw_register(&client->dev, &vc5->clk_mul);
+		kfree(init.name); /* clock framework made a copy of the name */
 		if (ret) {
 			dev_err(&client->dev, "unable to register %s\n",
 				init.name);
@@ -780,16 +759,18 @@ static int vc5_probe(struct i2c_client *client,
 
 	/* Register PFD */
 	memset(&init, 0, sizeof(init));
-	init.name = vc5_pfd_names[0];
+	init.name = kasprintf(GFP_KERNEL, "%pOFn.pfd", client->dev.of_node);
 	init.ops = &vc5_pfd_ops;
 	init.flags = CLK_SET_RATE_PARENT;
+	init.parent_names = parent_names;
 	if (vc5->chip_info->flags & VC5_HAS_PFD_FREQ_DBL)
-		init.parent_names = vc5_dbl_names;
+		parent_names[0] = clk_hw_get_name(&vc5->clk_mul);
 	else
-		init.parent_names = vc5_mux_names;
+		parent_names[0] = clk_hw_get_name(&vc5->clk_mux);
 	init.num_parents = 1;
 	vc5->clk_pfd.init = &init;
 	ret = devm_clk_hw_register(&client->dev, &vc5->clk_pfd);
+	kfree(init.name);	/* clock framework made a copy of the name */
 	if (ret) {
 		dev_err(&client->dev, "unable to register %s\n", init.name);
 		goto err_clk;
@@ -797,15 +778,17 @@ static int vc5_probe(struct i2c_client *client,
 
 	/* Register PLL */
 	memset(&init, 0, sizeof(init));
-	init.name = vc5_pll_names[0];
+	init.name = kasprintf(GFP_KERNEL, "%pOFn.pll", client->dev.of_node);
 	init.ops = &vc5_pll_ops;
 	init.flags = CLK_SET_RATE_PARENT;
-	init.parent_names = vc5_pfd_names;
+	init.parent_names = parent_names;
+	parent_names[0] = clk_hw_get_name(&vc5->clk_pfd);
 	init.num_parents = 1;
 	vc5->clk_pll.num = 0;
 	vc5->clk_pll.vc5 = vc5;
 	vc5->clk_pll.hw.init = &init;
 	ret = devm_clk_hw_register(&client->dev, &vc5->clk_pll.hw);
+	kfree(init.name); /* clock framework made a copy of the name */
 	if (ret) {
 		dev_err(&client->dev, "unable to register %s\n", init.name);
 		goto err_clk;
@@ -815,15 +798,18 @@ static int vc5_probe(struct i2c_client *client,
 	for (n = 0; n < vc5->chip_info->clk_fod_cnt; n++) {
 		idx = vc5_map_index_to_output(vc5->chip_info->model, n);
 		memset(&init, 0, sizeof(init));
-		init.name = vc5_fod_names[idx];
+		init.name = kasprintf(GFP_KERNEL, "%pOFn.fod%d",
+				      client->dev.of_node, idx);
 		init.ops = &vc5_fod_ops;
 		init.flags = CLK_SET_RATE_PARENT;
-		init.parent_names = vc5_pll_names;
+		init.parent_names = parent_names;
+		parent_names[0] = clk_hw_get_name(&vc5->clk_pll.hw);
 		init.num_parents = 1;
 		vc5->clk_fod[n].num = idx;
 		vc5->clk_fod[n].vc5 = vc5;
 		vc5->clk_fod[n].hw.init = &init;
 		ret = devm_clk_hw_register(&client->dev, &vc5->clk_fod[n].hw);
+		kfree(init.name); /* clock framework made a copy of the name */
 		if (ret) {
 			dev_err(&client->dev, "unable to register %s\n",
 				init.name);
@@ -833,32 +819,36 @@ static int vc5_probe(struct i2c_client *client,
 
 	/* Register MUX-connected OUT0_I2C_SELB output */
 	memset(&init, 0, sizeof(init));
-	init.name = vc5_clk_out_names[0];
+	init.name = kasprintf(GFP_KERNEL, "%pOFn.out0_sel_i2cb",
+			      client->dev.of_node);
 	init.ops = &vc5_clk_out_ops;
 	init.flags = CLK_SET_RATE_PARENT;
-	init.parent_names = vc5_mux_names;
+	init.parent_names = parent_names;
+	parent_names[0] = clk_hw_get_name(&vc5->clk_mux);
 	init.num_parents = 1;
 	vc5->clk_out[0].num = idx;
 	vc5->clk_out[0].vc5 = vc5;
 	vc5->clk_out[0].hw.init = &init;
 	ret = devm_clk_hw_register(&client->dev, &vc5->clk_out[0].hw);
+	kfree(init.name);	/* clock framework made a copy of the name */
 	if (ret) {
-		dev_err(&client->dev, "unable to register %s\n",
-			init.name);
+		dev_err(&client->dev, "unable to register %s\n", init.name);
 		goto err_clk;
 	}
 
 	/* Register FOD-connected OUTx outputs */
 	for (n = 1; n < vc5->chip_info->clk_out_cnt; n++) {
 		idx = vc5_map_index_to_output(vc5->chip_info->model, n - 1);
-		parent_names[0] = vc5_fod_names[idx];
+		parent_names[0] = clk_hw_get_name(&vc5->clk_fod[idx].hw);
 		if (n == 1)
-			parent_names[1] = vc5_mux_names[0];
+			parent_names[1] = clk_hw_get_name(&vc5->clk_mux);
 		else
-			parent_names[1] = vc5_clk_out_names[n - 1];
+			parent_names[1] =
+			    clk_hw_get_name(&vc5->clk_out[n - 1].hw);
 
 		memset(&init, 0, sizeof(init));
-		init.name = vc5_clk_out_names[idx + 1];
+		init.name = kasprintf(GFP_KERNEL, "%pOFn.out%d",
+				      client->dev.of_node, idx + 1);
 		init.ops = &vc5_clk_out_ops;
 		init.flags = CLK_SET_RATE_PARENT;
 		init.parent_names = parent_names;
@@ -866,8 +856,8 @@ static int vc5_probe(struct i2c_client *client,
 		vc5->clk_out[n].num = idx;
 		vc5->clk_out[n].vc5 = vc5;
 		vc5->clk_out[n].hw.init = &init;
-		ret = devm_clk_hw_register(&client->dev,
-					   &vc5->clk_out[n].hw);
+		ret = devm_clk_hw_register(&client->dev, &vc5->clk_out[n].hw);
+		kfree(init.name); /* clock framework made a copy of the name */
 		if (ret) {
 			dev_err(&client->dev, "unable to register %s\n",
 				init.name);
-- 
2.17.1


^ permalink raw reply related

* [PATCH V3 3/3] clk: vc5: Enable addition output configurations of the Versaclock
From: Adam Ford @ 2020-06-03 15:43 UTC (permalink / raw)
  To: linux-clk
  Cc: Adam Ford, Michael Turquette, Stephen Boyd, Rob Herring,
	Marek Vasut, devicetree, linux-kernel
In-Reply-To: <20200603154329.31579-1-aford173@gmail.com>

The existing driver is expecting the Versaclock to be pre-programmed,
and only sets the output frequency.  Unfortunately, not all devices
are pre-programmed, and the Versaclock chip has more options beyond
just the frequency.

This patch enables the following additional features:

   - Programmable voltage: 1.8V, 2.5V, or 3.3V​
   - Slew Percentage of normal: 85%, 90%, or 100%
   - Output Type: LVPECL, CMOS, HCSL, or LVDS

Signed-off-by: Adam Ford <aford173@gmail.com>
---
V3:  Make source code change following the binding changes since there
     is a dependency and move changes to binding header file to the
     patch with bindings changes.

diff --git a/drivers/clk/clk-versaclock5.c b/drivers/clk/clk-versaclock5.c
index 41e3a75963b9..9a5fb3834b9a 100644
--- a/drivers/clk/clk-versaclock5.c
+++ b/drivers/clk/clk-versaclock5.c
@@ -24,6 +24,8 @@
 #include <linux/regmap.h>
 #include <linux/slab.h>
 
+#include <dt-bindings/clk/versaclock.h>
+
 /* VersaClock5 registers */
 #define VC5_OTP_CONTROL				0x00
 
@@ -89,6 +91,28 @@
 
 /* Clock control register for clock 1,2 */
 #define VC5_CLK_OUTPUT_CFG(idx, n)	(0x60 + ((idx) * 0x2) + (n))
+#define VC5_CLK_OUTPUT_CFG0_CFG_SHIFT	5
+#define VC5_CLK_OUTPUT_CFG0_CFG_MASK GENMASK(7, VC5_CLK_OUTPUT_CFG0_CFG_SHIFT)
+
+#define VC5_CLK_OUTPUT_CFG0_CFG_LVPECL	(VC5_LVPECL)
+#define VC5_CLK_OUTPUT_CFG0_CFG_CMOS		(VC5_CMOS)
+#define VC5_CLK_OUTPUT_CFG0_CFG_HCSL33	(VC5_HCSL33)
+#define VC5_CLK_OUTPUT_CFG0_CFG_LVDS		(VC5_LVDS)
+#define VC5_CLK_OUTPUT_CFG0_CFG_CMOS2		(VC5_CMOS2)
+#define VC5_CLK_OUTPUT_CFG0_CFG_CMOSD		(VC5_CMOSD)
+#define VC5_CLK_OUTPUT_CFG0_CFG_HCSL25	(VC5_HCSL25)
+
+#define VC5_CLK_OUTPUT_CFG0_PWR_SHIFT	3
+#define VC5_CLK_OUTPUT_CFG0_PWR_MASK GENMASK(4, VC5_CLK_OUTPUT_CFG0_PWR_SHIFT)
+#define VC5_CLK_OUTPUT_CFG0_PWR_18	(0<<VC5_CLK_OUTPUT_CFG0_PWR_SHIFT)
+#define VC5_CLK_OUTPUT_CFG0_PWR_25	(2<<VC5_CLK_OUTPUT_CFG0_PWR_SHIFT)
+#define VC5_CLK_OUTPUT_CFG0_PWR_33	(3<<VC5_CLK_OUTPUT_CFG0_PWR_SHIFT)
+#define VC5_CLK_OUTPUT_CFG0_SLEW_SHIFT	0
+#define VC5_CLK_OUTPUT_CFG0_SLEW_MASK GENMASK(1, VC5_CLK_OUTPUT_CFG0_SLEW_SHIFT)
+#define VC5_CLK_OUTPUT_CFG0_SLEW_80	(0<<VC5_CLK_OUTPUT_CFG0_SLEW_SHIFT)
+#define VC5_CLK_OUTPUT_CFG0_SLEW_85	(1<<VC5_CLK_OUTPUT_CFG0_SLEW_SHIFT)
+#define VC5_CLK_OUTPUT_CFG0_SLEW_90	(2<<VC5_CLK_OUTPUT_CFG0_SLEW_SHIFT)
+#define VC5_CLK_OUTPUT_CFG0_SLEW_100	(3<<VC5_CLK_OUTPUT_CFG0_SLEW_SHIFT)
 #define VC5_CLK_OUTPUT_CFG1_EN_CLKBUF	BIT(0)
 
 #define VC5_CLK_OE_SHDN				0x68
@@ -143,6 +167,8 @@ struct vc5_hw_data {
 	u32			div_int;
 	u32			div_frc;
 	unsigned int		num;
+	unsigned int		clk_output_cfg0;
+	unsigned int		clk_output_cfg0_mask;
 };
 
 struct vc5_driver_data {
@@ -567,6 +593,17 @@ static int vc5_clk_out_prepare(struct clk_hw *hw)
 	regmap_update_bits(vc5->regmap, VC5_CLK_OUTPUT_CFG(hwdata->num, 1),
 			   VC5_CLK_OUTPUT_CFG1_EN_CLKBUF,
 			   VC5_CLK_OUTPUT_CFG1_EN_CLKBUF);
+	if (hwdata->clk_output_cfg0_mask) {
+		dev_dbg(&vc5->client->dev, "Update output %d mask 0x%0X val 0x%0X\n",
+			hwdata->num, hwdata->clk_output_cfg0_mask,
+			hwdata->clk_output_cfg0);
+
+		regmap_update_bits(vc5->regmap,
+			VC5_CLK_OUTPUT_CFG(hwdata->num, 0),
+			hwdata->clk_output_cfg0_mask,
+			hwdata->clk_output_cfg0);
+	}
+
 	return 0;
 }
 
@@ -666,6 +703,120 @@ static int vc5_map_index_to_output(const enum vc5_model model,
 	}
 }
 
+static int vc5_update_mode(struct device_node *np_output,
+			   struct vc5_hw_data *clk_out)
+{
+	u32 value;
+
+	if (!of_property_read_u32(np_output, "idt,mode", &value)) {
+		clk_out->clk_output_cfg0_mask |= VC5_CLK_OUTPUT_CFG0_CFG_MASK;
+		switch (value) {
+		case VC5_CLK_OUTPUT_CFG0_CFG_LVPECL:
+		case VC5_CLK_OUTPUT_CFG0_CFG_CMOS:
+		case VC5_CLK_OUTPUT_CFG0_CFG_HCSL33:
+		case VC5_CLK_OUTPUT_CFG0_CFG_LVDS:
+		case VC5_CLK_OUTPUT_CFG0_CFG_CMOS2:
+		case VC5_CLK_OUTPUT_CFG0_CFG_CMOSD:
+		case VC5_CLK_OUTPUT_CFG0_CFG_HCSL25:
+			clk_out->clk_output_cfg0 |=
+			    value << VC5_CLK_OUTPUT_CFG0_CFG_SHIFT;
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+	return 0;
+}
+
+static int vc5_update_power(struct device_node *np_output,
+			    struct vc5_hw_data *clk_out)
+{
+	u32 value;
+
+	if (!of_property_read_u32(np_output,
+				  "idt,voltage-microvolts", &value)) {
+		clk_out->clk_output_cfg0_mask |= VC5_CLK_OUTPUT_CFG0_PWR_MASK;
+		switch (value) {
+		case 1800000:
+			clk_out->clk_output_cfg0 |= VC5_CLK_OUTPUT_CFG0_PWR_18;
+			break;
+		case 2500000:
+			clk_out->clk_output_cfg0 |= VC5_CLK_OUTPUT_CFG0_PWR_25;
+			break;
+		case 3300000:
+			clk_out->clk_output_cfg0 |= VC5_CLK_OUTPUT_CFG0_PWR_33;
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+	return 0;
+}
+
+static int vc5_update_slew(struct device_node *np_output,
+			   struct vc5_hw_data *clk_out)
+{
+	u32 value;
+
+	if (!of_property_read_u32(np_output, "idt,slew-percent", &value)) {
+		clk_out->clk_output_cfg0_mask |= VC5_CLK_OUTPUT_CFG0_SLEW_MASK;
+		switch (value) {
+		case 80:
+			clk_out->clk_output_cfg0 |= VC5_CLK_OUTPUT_CFG0_SLEW_80;
+			break;
+		case 85:
+			clk_out->clk_output_cfg0 |= VC5_CLK_OUTPUT_CFG0_SLEW_85;
+			break;
+		case 90:
+			clk_out->clk_output_cfg0 |= VC5_CLK_OUTPUT_CFG0_SLEW_90;
+			break;
+		case 100:
+			clk_out->clk_output_cfg0 |=
+			    VC5_CLK_OUTPUT_CFG0_SLEW_100;
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+	return 0;
+}
+
+static int vc5_get_output_config(struct i2c_client *client,
+				 struct vc5_hw_data *clk_out)
+{
+	struct device_node *np_output;
+	char *child_name;
+	int ret = 0;
+
+	child_name = kasprintf(GFP_KERNEL, "OUT%d", clk_out->num + 1);
+	np_output = of_get_child_by_name(client->dev.of_node, child_name);
+	kfree(child_name);
+	if (!np_output)
+		goto output_done;
+
+	ret = vc5_update_mode(np_output, clk_out);
+	if (ret)
+		goto output_error;
+
+	ret = vc5_update_power(np_output, clk_out);
+	if (ret)
+		goto output_error;
+
+	ret = vc5_update_slew(np_output, clk_out);
+
+output_error:
+	if (ret) {
+		dev_err(&client->dev,
+			"Invalid clock output configuration OUT%d\n",
+			clk_out->num + 1);
+	}
+
+	of_node_put(np_output);
+
+output_done:
+	return ret;
+}
+
 static const struct of_device_id clk_vc5_of_match[];
 
 static int vc5_probe(struct i2c_client *client, const struct i2c_device_id *id)
@@ -863,6 +1014,11 @@ static int vc5_probe(struct i2c_client *client, const struct i2c_device_id *id)
 				init.name);
 			goto err_clk;
 		}
+
+		/* Fetch Clock Output configuration from DT (if specified) */
+		ret = vc5_get_output_config(client, &vc5->clk_out[n]);
+		if (ret)
+			goto err_clk;
 	}
 
 	ret = of_clk_add_hw_provider(client->dev.of_node, vc5_of_clk_get, vc5);
-- 
2.17.1


^ permalink raw reply related

* [PATCH V3 2/3] dt: Add additional option bindings for IDT VersaClock
From: Adam Ford @ 2020-06-03 15:43 UTC (permalink / raw)
  To: linux-clk
  Cc: Adam Ford, Michael Turquette, Stephen Boyd, Rob Herring,
	Marek Vasut, devicetree, linux-kernel
In-Reply-To: <20200603154329.31579-1-aford173@gmail.com>

The VersaClock driver now supports some additional bindings to support
child nodes which can configure optional settings like mode, voltage
and slew.

This patch updates the binding document to describe what is available
in the driver.

Signed-off-by: Adam Ford <aford173@gmail.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
V3:  Re-order binding change and driver change so biding comes first.
     Move all binding related changes to this patch.

diff --git a/Documentation/devicetree/bindings/clock/idt,versaclock5.txt b/Documentation/devicetree/bindings/clock/idt,versaclock5.txt
index bcff681a4bd0..6165b6ddb1a9 100644
--- a/Documentation/devicetree/bindings/clock/idt,versaclock5.txt
+++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.txt
@@ -31,6 +31,29 @@ Required properties:
 		- 5p49v5933 and
 		- 5p49v5935: (optional) property not present or "clkin".
 
+For all output ports, a corresponding, optional child node named OUT1,
+OUT2, etc. can represent a each output, and the node can be used to
+specify the following:
+
+- itd,mode: can be one of the following:
+                 - VC5_LVPECL
+                 - VC5_CMOS
+                 - VC5_HCSL33
+                 - VC5_LVDS
+                 - VC5_CMOS2
+                 - VC5_CMOSD
+                 - VC5_HCSL25
+
+- idt,voltage-microvolts:  can be one of the following
+                 - 1800000
+                 - 2500000
+                 - 3300000
+-  idt,slew-percent: Percent of normal, can be one of
+                 - 80
+                 - 85
+                 - 90
+                 - 100
+
 ==Mapping between clock specifier and physical pins==
 
 When referencing the provided clock in the DT using phandle and
@@ -81,6 +104,16 @@ i2c-master-node {
 		/* Connect XIN input to 25MHz reference */
 		clocks = <&ref25m>;
 		clock-names = "xin";
+
+		OUT1 {
+			itd,mode = <VC5_CMOS>;
+			idt,voltage-microvolts = <1800000>;
+			idt,slew-percent = <80>;
+		};
+		OUT2 {
+			...
+		};
+		...
 	};
 };
 
diff --git a/include/dt-bindings/clk/versaclock.h b/include/dt-bindings/clk/versaclock.h
new file mode 100644
index 000000000000..c6a6a0946564
--- /dev/null
+++ b/include/dt-bindings/clk/versaclock.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/* This file defines field values used by the versaclock 6 family
+ * for defining output type
+ */
+
+#define VC5_LVPECL	0
+#define VC5_CMOS	1
+#define VC5_HCSL33	2
+#define VC5_LVDS	3
+#define VC5_CMOS2	4
+#define VC5_CMOSD	5
+#define VC5_HCSL25	6
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH v8 0/5] support reserving crashkernel above 4G on arm64 kdump
From: John Donnelly @ 2020-06-03 15:30 UTC (permalink / raw)
  To: chenzhou
  Cc: Prabhakar Kushwaha, Devicetree List, Arnd Bergmann, Baoquan He,
	Linux Doc Mailing List, Catalin Marinas, Bhupesh Sharma,
	RuiRui Yang, kexec mailing list, Linux Kernel Mailing List,
	Rob Herring, Simon Horman, James Morse, guohanjun,
	Thomas Gleixner, Prabhakar Kushwaha, Will Deacon, Ingo Molnar,
	linux-arm-kernel, nsaenzjulienne
In-Reply-To: <8463464e-5461-f328-621c-bacc6a3b88dd@huawei.com>



> On Jun 3, 2020, at 8:20 AM, chenzhou <chenzhou10@huawei.com> wrote:
> 
> Hi,
> 
> 
> On 2020/6/3 19:47, Prabhakar Kushwaha wrote:
>> Hi Chen,
>> 
>> On Tue, Jun 2, 2020 at 8:12 PM John Donnelly <john.p.donnelly@oracle.com> wrote:
>>> 
>>> 
>>>> On Jun 2, 2020, at 12:38 AM, Prabhakar Kushwaha <prabhakar.pkin@gmail.com> wrote:
>>>> 
>>>> On Tue, Jun 2, 2020 at 3:29 AM John Donnelly <john.p.donnelly@oracle.com> wrote:
>>>>> Hi .  See below !
>>>>> 
>>>>>> On Jun 1, 2020, at 4:02 PM, Bhupesh Sharma <bhsharma@redhat.com> wrote:
>>>>>> 
>>>>>> Hi John,
>>>>>> 
>>>>>> On Tue, Jun 2, 2020 at 1:01 AM John Donnelly <John.P.donnelly@oracle.com> wrote:
>>>>>>> Hi,
>>>>>>> 
>>>>>>> 
>>>>>>> On 6/1/20 7:02 AM, Prabhakar Kushwaha wrote:
>>>>>>>> Hi Chen,
>>>>>>>> 
>>>>>>>> On Thu, May 21, 2020 at 3:05 PM Chen Zhou <chenzhou10@huawei.com> wrote:
>>>>>>>>> This patch series enable reserving crashkernel above 4G in arm64.
>>>>>>>>> 
>>>>>>>>> There are following issues in arm64 kdump:
>>>>>>>>> 1. We use crashkernel=X to reserve crashkernel below 4G, which will fail
>>>>>>>>> when there is no enough low memory.
>>>>>>>>> 2. Currently, crashkernel=Y@X can be used to reserve crashkernel above 4G,
>>>>>>>>> in this case, if swiotlb or DMA buffers are required, crash dump kernel
>>>>>>>>> will boot failure because there is no low memory available for allocation.
>>>>>>>>> 
>>>>>>>> We are getting "warn_alloc" [1] warning during boot of kdump kernel
>>>>>>>> with bootargs as [2] of primary kernel.
>>>>>>>> This error observed on ThunderX2  ARM64 platform.
>>>>>>>> 
>>>>>>>> It is observed with latest upstream tag (v5.7-rc3) with this patch set
>>>>>>>> and https://urldefense.com/v3/__https://lists.infradead.org/pipermail/kexec/2020-May/025128.html__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbiIAAlzu$
>>>>>>>> Also **without** this patch-set
>>>>>>>> "https://urldefense.com/v3/__https://www.spinics.net/lists/arm-kernel/msg806882.html__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbjC6ujMA$"
>>>>>>>> 
>>>>>>>> This issue comes whenever crashkernel memory is reserved after 0xc000_0000.
>>>>>>>> More details discussed earlier in
>>>>>>>> https://urldefense.com/v3/__https://www.spinics.net/lists/arm-kernel/msg806882.html__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbjC6ujMA$  without any
>>>>>>>> solution
>>>>>>>> 
>>>>>>>> This patch-set is expected to solve similar kind of issue.
>>>>>>>> i.e. low memory is only targeted for DMA, swiotlb; So above mentioned
>>>>>>>> observation should be considered/fixed. .
>>>>>>>> 
>>>>>>>> --pk
>>>>>>>> 
>>>>>>>> [1]
>>>>>>>> [   30.366695] DMI: Cavium Inc. Saber/Saber, BIOS
>>>>>>>> TX2-FW-Release-3.1-build_01-2803-g74253a541a mm/dd/yyyy
>>>>>>>> [   30.367696] NET: Registered protocol family 16
>>>>>>>> [   30.369973] swapper/0: page allocation failure: order:6,
>>>>>>>> mode:0x1(GFP_DMA), nodemask=(null),cpuset=/,mems_allowed=0
>>>>>>>> [   30.369980] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.7.0-rc3+ #121
>>>>>>>> [   30.369981] Hardware name: Cavium Inc. Saber/Saber, BIOS
>>>>>>>> TX2-FW-Release-3.1-build_01-2803-g74253a541a mm/dd/yyyy
>>>>>>>> [   30.369984] Call trace:
>>>>>>>> [   30.369989]  dump_backtrace+0x0/0x1f8
>>>>>>>> [   30.369991]  show_stack+0x20/0x30
>>>>>>>> [   30.369997]  dump_stack+0xc0/0x10c
>>>>>>>> [   30.370001]  warn_alloc+0x10c/0x178
>>>>>>>> [   30.370004]  __alloc_pages_slowpath.constprop.111+0xb10/0xb50
>>>>>>>> [   30.370006]  __alloc_pages_nodemask+0x2b4/0x300
>>>>>>>> [   30.370008]  alloc_page_interleave+0x24/0x98
>>>>>>>> [   30.370011]  alloc_pages_current+0xe4/0x108
>>>>>>>> [   30.370017]  dma_atomic_pool_init+0x44/0x1a4
>>>>>>>> [   30.370020]  do_one_initcall+0x54/0x228
>>>>>>>> [   30.370027]  kernel_init_freeable+0x228/0x2cc
>>>>>>>> [   30.370031]  kernel_init+0x1c/0x110
>>>>>>>> [   30.370034]  ret_from_fork+0x10/0x18
>>>>>>>> [   30.370036] Mem-Info:
>>>>>>>> [   30.370064] active_anon:0 inactive_anon:0 isolated_anon:0
>>>>>>>> [   30.370064]  active_file:0 inactive_file:0 isolated_file:0
>>>>>>>> [   30.370064]  unevictable:0 dirty:0 writeback:0 unstable:0
>>>>>>>> [   30.370064]  slab_reclaimable:34 slab_unreclaimable:4438
>>>>>>>> [   30.370064]  mapped:0 shmem:0 pagetables:14 bounce:0
>>>>>>>> [   30.370064]  free:1537719 free_pcp:219 free_cma:0
>>>>>>>> [   30.370070] Node 0 active_anon:0kB inactive_anon:0kB
>>>>>>>> active_file:0kB inactive_file:0kB unevictable:0kB isolated(anon):0kB
>>>>>>>> isolated(file):0kB mapped:0kB dirty:0kB writeback:0kB shmem:0kB
>>>>>>>> shmem_thp: 0kB shmem_pmdmapped: 0kB anon_thp: 0kB writeback_tmp:0kB
>>>>>>>> unstable:0kB all_unreclaimable? no
>>>>>>>> [   30.370073] Node 1 active_anon:0kB inactive_anon:0kB
>>>>>>>> active_file:0kB inactive_file:0kB unevictable:0kB isolated(anon):0kB
>>>>>>>> isolated(file):0kB mapped:0kB dirty:0kB writeback:0kB shmem:0kB
>>>>>>>> shmem_thp: 0kB shmem_pmdmapped: 0kB anon_thp: 0kB writeback_tmp:0kB
>>>>>>>> unstable:0kB all_unreclaimable? no
>>>>>>>> [   30.370079] Node 0 DMA free:0kB min:0kB low:0kB high:0kB
>>>>>>>> reserved_highatomic:0KB active_anon:0kB inactive_anon:0kB
>>>>>>>> active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB
>>>>>>>> present:128kB managed:0kB mlocked:0kB kernel_stack:0kB pagetables:0kB
>>>>>>>> bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB
>>>>>>>> [   30.370084] lowmem_reserve[]: 0 250 6063 6063
>>>>>>>> [   30.370090] Node 0 DMA32 free:256000kB min:408kB low:664kB
>>>>>>>> high:920kB reserved_highatomic:0KB active_anon:0kB inactive_anon:0kB
>>>>>>>> active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB
>>>>>>>> present:269700kB managed:256000kB mlocked:0kB kernel_stack:0kB
>>>>>>>> pagetables:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB
>>>>>>>> [   30.370094] lowmem_reserve[]: 0 0 5813 5813
>>>>>>>> [   30.370100] Node 0 Normal free:5894876kB min:9552kB low:15504kB
>>>>>>>> high:21456kB reserved_highatomic:0KB active_anon:0kB inactive_anon:0kB
>>>>>>>> active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB
>>>>>>>> present:8388608kB managed:5953112kB mlocked:0kB kernel_stack:21672kB
>>>>>>>> pagetables:56kB bounce:0kB free_pcp:876kB local_pcp:176kB free_cma:0kB
>>>>>>>> [   30.370104] lowmem_reserve[]: 0 0 0 0
>>>>>>>> [   30.370107] Node 0 DMA: 0*4kB 0*8kB 0*16kB 0*32kB 0*64kB 0*128kB
>>>>>>>> 0*256kB 0*512kB 0*1024kB 0*2048kB 0*4096kB = 0kB
>>>>>>>> [   30.370113] Node 0 DMA32: 0*4kB 0*8kB 0*16kB 0*32kB 0*64kB 0*128kB
>>>>>>>> 0*256kB 0*512kB 0*1024kB 1*2048kB (M) 62*4096kB (M) = 256000kB
>>>>>>>> [   30.370119] Node 0 Normal: 2*4kB (M) 3*8kB (ME) 2*16kB (UE) 3*32kB
>>>>>>>> (UM) 1*64kB (U) 2*128kB (M) 2*256kB (ME) 3*512kB (ME) 3*1024kB (ME)
>>>>>>>> 3*2048kB (UME) 1436*4096kB (M) = 5893600kB
>>>>>>>> [   30.370129] Node 0 hugepages_total=0 hugepages_free=0
>>>>>>>> hugepages_surp=0 hugepages_size=1048576kB
>>>>>>>> [   30.370130] 0 total pagecache pages
>>>>>>>> [   30.370132] 0 pages in swap cache
>>>>>>>> [   30.370134] Swap cache stats: add 0, delete 0, find 0/0
>>>>>>>> [   30.370135] Free swap  = 0kB
>>>>>>>> [   30.370136] Total swap = 0kB
>>>>>>>> [   30.370137] 2164609 pages RAM
>>>>>>>> [   30.370139] 0 pages HighMem/MovableOnly
>>>>>>>> [   30.370140] 612331 pages reserved
>>>>>>>> [   30.370141] 0 pages hwpoisoned
>>>>>>>> [   30.370143] DMA: failed to allocate 256 KiB pool for atomic
>>>>>>>> coherent allocation
>>>>>>> 
>>>>>>> During my testing I saw the same error and Chen's  solution corrected it .
>>>>>> Which combination you are using on your side? I am using Prabhakar's
>>>>>> suggested environment and can reproduce the issue
>>>>>> with or without Chen's crashkernel support above 4G patchset.
>>>>>> 
>>>>>> I am also using a ThunderX2 platform with latest makedumpfile code and
>>>>>> kexec-tools (with the suggested patch
>>>>>> <https://urldefense.com/v3/__https://lists.infradead.org/pipermail/kexec/2020-May/025128.html__;!!GqivPVa7Brio!J6lUig58-Gw6TKZnEEYzEeSU36T-1SqlB1kImU00xtX_lss5Tx-JbUmLE9TJC3foXBLg$ >).
>>>>>> 
>>>>>> Thanks,
>>>>>> Bhupesh
>>>>> 
>>>>> I did this activity 5 months ago and I have moved on to other activities. My DMA failures were related to PCI devices that could not be enumerated because  low-DMA space was not  available when crashkernel was moved above 4G; I don’t recall the exact platform.
>>>>> 
>>>>> 
>>>>> 
>>>>> For this failure ,
>>>>> 
>>>>>>>> DMA: failed to allocate 256 KiB pool for atomic
>>>>>>>> coherent allocation
>>>>> 
>>>>> Is due to :
>>>>> 
>>>>> 
>>>>> 3618082c
>>>>> ("arm64 use both ZONE_DMA and ZONE_DMA32")
>>>>> 
>>>>> With the introduction of ZONE_DMA to support the Raspberry DMA
>>>>> region below 1G, the crashkernel is placed in the upper 4G
>>>>> ZONE_DMA_32 region. Since the crashkernel does not have access
>>>>> to the ZONE_DMA region, it prints out call trace during bootup.
>>>>> 
>>>>> It is due to having this CONFIG item  ON  :
>>>>> 
>>>>> 
>>>>> CONFIG_ZONE_DMA=y
>>>>> 
>>>>> Turning off ZONE_DMA fixes a issue and Raspberry PI 4 will
>>>>> use the device tree to specify memory below 1G.
>>>>> 
>>>>> 
>>>> Disabling ZONE_DMA is temporary solution.  We may need proper solution
>>> 
>>> Perhaps the Raspberry platform configuration dependencies need separated  from “server class” Arm  equipment ?  Or auto-configured on boot ?  Consult an expert ;-)
>>> 
>>> 
>>> 
>>>>> I would like to see Chen’s feature added , perhaps as EXPERIMENTAL,  so we can get some configuration testing done on it.   It corrects having a DMA zone in low memory while crash-kernel is above 4GB.  This has been going on for a year now.
>>>> I will also like this patch to be added in Linux as early as possible.
>>>> 
>>>> Issue mentioned by me happens with or without this patch.
>>>> 
>>>> This patch-set can consider fixing because it uses low memory for DMA
>>>> & swiotlb only.
>>>> We can consider restricting crashkernel within the required range like below
>>>> 
>>>> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
>>>> index 7f9e5a6dc48c..bd67b90d35bd 100644
>>>> --- a/kernel/crash_core.c
>>>> +++ b/kernel/crash_core.c
>>>> @@ -354,7 +354,7 @@ int __init reserve_crashkernel_low(void)
>>>>                       return 0;
>>>>       }
>>>> 
>>>> -       low_base = memblock_find_in_range(0, 1ULL << 32, low_size, CRASH_ALIGN);
>>>> +       low_base = memblock_find_in_range(0,0xc0000000, low_size, CRASH_ALIGN);
>>>>       if (!low_base) {
>>>>               pr_err("Cannot reserve %ldMB crashkernel low memory,
>>>> please try smaller size.\n",
>>>>                      (unsigned long)(low_size >> 20));
>>>> 
>>>> 
>>>    I suspect  0xc0000000  would need to be a CONFIG item  and not hard-coded.
>>> 
>> if you consider this as valid change,  can you please incorporate as
>> part of your patch-set.
> 
> After commit 1a8e1cef7 ("arm64: use both ZONE_DMA and ZONE_DMA32"),the 0-4G memory is splited
> to DMA [mem 0x0000000000000000-0x000000003fffffff] and DMA32 [mem 0x0000000040000000-0x00000000ffffffff] on arm64.
> 
> From the above discussion, on your platform, the low crashkernel fall in DMA32 region, but your environment needs to access DMA
> region, so there is the call trace.
> 
> I have a question, why do you choose 0xc0000000 here?
> 
> Besides, this is common code, we also need to consider about x86.
> 

 + nsaenzjulienne@suse.de 

  Exactly .  This is why it needs to be a CONFIG option for  Raspberry ..,  or device tree option. 


  We could revert 1a8e1cef7 since it broke  Arm kdump too.


> 
> Thanks,
> Chen Zhou
> 


 
 

^ permalink raw reply

* Re: [PATCH V1 1/2] mmc: sdhci-msm: Add interconnect bandwidth scaling support
From: Matthias Kaehlcke @ 2020-06-03 15:30 UTC (permalink / raw)
  To: Pradeep P V K
  Cc: bjorn.andersson, adrian.hunter, robh+dt, ulf.hansson, vbadigan,
	sboyd, georgi.djakov, linux-mmc, linux-kernel, linux-arm-msm,
	devicetree, linux-mmc-owner, rnayak, sibis, matthias
In-Reply-To: <1591175376-2374-2-git-send-email-ppvk@codeaurora.org>

Hi Pradeep,

On Wed, Jun 03, 2020 at 02:39:35PM +0530, Pradeep P V K wrote:
> Interconnect bandwidth scaling support is now added as a
> part of OPP [1]. So, make sure interconnect driver is ready
> before handling interconnect scaling.
> 
> This change is based on
> [1] [Patch v8] Introduce OPP bandwidth bindings
> (https://lkml.org/lkml/2020/5/12/493)
> 
> [2] [Patch v3] mmc: sdhci-msm: Fix error handling
> for dev_pm_opp_of_add_table()
> (https://lkml.org/lkml/2020/5/5/491)
> 
> Signed-off-by: Pradeep P V K <ppvk@codeaurora.org>
> ---
>  drivers/mmc/host/sdhci-msm.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index b277dd7..bf95484 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -14,6 +14,7 @@
>  #include <linux/slab.h>
>  #include <linux/iopoll.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/interconnect.h>
>  
>  #include "sdhci-pltfm.h"
>  #include "cqhci.h"
> @@ -1999,6 +2000,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  	struct sdhci_pltfm_host *pltfm_host;
>  	struct sdhci_msm_host *msm_host;
>  	struct clk *clk;
> +	struct icc_path *sdhc_path;

nit: The 'sdhc_' prefix doesn't provide any useful information, change it
to 'icc_path', which makes evident what it is?

>  	int ret;
>  	u16 host_version, core_minor;
>  	u32 core_version, config;
> @@ -2070,6 +2072,20 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  	}
>  	msm_host->bulk_clks[0].clk = clk;
>  
> +	/* Make sure that ICC driver is ready for interconnect bandwdith
> +	 * scaling before registering the device for OPP.
> +	 */
> +	sdhc_path = of_icc_get(&pdev->dev, NULL);
> +	ret = PTR_ERR_OR_ZERO(sdhc_path);
> +	if (ret) {

nit: IMO it would be clearer to do this instead of using PTR_ERR_OR_ZERO()
(as for the OPP table below):

	if (IS_ERR(sdhc_path)) {
		ret = PTR_ERR(sdhc_path);

> +		if (ret == -EPROBE_DEFER)
> +			dev_info(&pdev->dev, "defer icc path: %d\n", ret);

This log seems to add little more than noise, or are there particular reasons
why it is useful in this driver? Most drivers just return silently in case of
deferred probing.

> +		else
> +			dev_err(&pdev->dev, "failed to get icc path:%d\n", ret);
> +		goto bus_clk_disable;
> +	}
> +	icc_put(sdhc_path);
> +
>  	msm_host->opp_table = dev_pm_opp_set_clkname(&pdev->dev, "core");
>  	if (IS_ERR(msm_host->opp_table)) {
>  		ret = PTR_ERR(msm_host->opp_table);
> -- 
> 1.9.1
> 

^ permalink raw reply

* [PATCH v5 1/5] firmware: xilinx: Add ZynqMP firmware ioctl enums for RPU configuration.
From: Ben Levinsky @ 2020-06-03 14:49 UTC (permalink / raw)
  To: ohad, bjorn.andersson, michals, jollys, rajanv, robh+dt,
	mark.rutland
  Cc: linux-remoteproc, linux-arm-kernell, devicetree, linux-kernel
In-Reply-To: <1591195783-10290-1-git-send-email-ben.levinsky@xilinx.com>

Add ZynqMP firmware ioctl enums for RPU configuration.

Signed-off-by: Ben Levinsky <ben.levinsky@xilinx.com>
---
v3:
- add xilinx-related platform mgmt fn's instead of wrapping around
  function pointer in xilinx eemi ops struct
v4:
- add default values for enums
---
 include/linux/firmware/xlnx-zynqmp.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index 5968df8..bb347df 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -104,6 +104,10 @@ enum pm_ret_status {
 };
 
 enum pm_ioctl_id {
+	IOCTL_GET_RPU_OPER_MODE = 0,
+	IOCTL_SET_RPU_OPER_MODE = 1,
+	IOCTL_RPU_BOOT_ADDR_CONFIG = 2,
+	IOCTL_TCM_COMB_CONFIG = 3,
 	IOCTL_SD_DLL_RESET = 6,
 	IOCTL_SET_SD_TAPDELAY,
 	IOCTL_SET_PLL_FRAC_MODE,
@@ -129,6 +133,21 @@ enum pm_query_id {
 	PM_QID_CLOCK_GET_MAX_DIVISOR,
 };
 
+enum rpu_oper_mode {
+	PM_RPU_MODE_LOCKSTEP = 0,
+	PM_RPU_MODE_SPLIT = 1,
+};
+
+enum rpu_boot_mem {
+	PM_RPU_BOOTMEM_LOVEC = 0,
+	PM_RPU_BOOTMEM_HIVEC = 1,
+};
+
+enum rpu_tcm_comb {
+	PM_RPU_TCM_SPLIT = 0,
+	PM_RPU_TCM_COMB = 1,
+};
+
 enum zynqmp_pm_reset_action {
 	PM_RESET_ACTION_RELEASE,
 	PM_RESET_ACTION_ASSERT,
-- 
2.7.4


^ permalink raw reply related

* [PATCH v5 4/5] dt-bindings: remoteproc: Add documentation for ZynqMP R5 rproc bindings
From: Ben Levinsky @ 2020-06-03 14:49 UTC (permalink / raw)
  To: ohad, bjorn.andersson, michals, jollys, rajanv, robh+dt,
	mark.rutland
  Cc: linux-remoteproc, linux-arm-kernell, devicetree, linux-kernel,
	Jason Wu, Wendy Liang, Michal Simek
In-Reply-To: <1591195783-10290-1-git-send-email-ben.levinsky@xilinx.com>

Add binding for ZynqMP R5 OpenAMP.

Represent the RPU domain resources in one device node. Each RPU
processor is a subnode of the top RPU domain node.

Signed-off-by: Ben Levinsky <ben.levinsky@xilinx.com>
Signed-off-by: Jason Wu <j.wu@xilinx.com>
Signed-off-by: Wendy Liang <jliang@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
v3:
- update zynqmp_r5 yaml parsing to not raise warnings for extra
  information in children of R5 node. The warning "node has a unit
  name, but no reg or ranges property" will still be raised though 
  as this particular node is needed to describe the
  '#address-cells' and '#size-cells' information.
v4::
- remove warning '/example-0/rpu@ff9a0000/r5@0: 
  node has a unit name, but no reg or ranges property'
  by adding reg to r5 node.
v5:
- update device tree sample and yaml parsing to not raise any warnings
- description for memory-region in yaml parsing
- compatible string in yaml parsing for TCM

---
 .../remoteproc/xilinx,zynqmp-r5-remoteproc.yaml    | 132 +++++++++++++++++++++
 1 file changed, 132 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml

diff --git a/Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml b/Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml
new file mode 100644
index 0000000..23fbdce
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml
@@ -0,0 +1,132 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Xilinx R5 remote processor controller bindings
+
+description:
+  This document defines the binding for the remoteproc component that loads and
+  boots firmwares on the Xilinx Zynqmp and Versal family chipset.
+
+  Note that the Linux has global addressing view of the R5-related memory (TCM)
+  so the absolute address ranges are provided in TCM reg's.
+maintainers:
+  - Ed Mooring <ed.mooring@xilinx.com>
+  - Ben Levinsky <ben.levinsky@xilinx.com>
+
+properties:
+  compatible:
+    const: "xlnx,zynqmp-r5-remoteproc-1.0"
+
+  core_conf:
+    description:
+      R5 core configuration (valid string - split or lock-step)
+    maxItems: 1
+
+  interrupts:
+    description:
+      Interrupt mapping for remoteproc IPI. It is required if the
+      user uses the remoteproc driver with the RPMsg kernel driver.
+    maxItems: 6
+
+  memory-region:
+    description:
+      collection of memory carveouts used for elf-loading and inter-processor
+      communication.
+    maxItems: 4
+    minItems: 4
+  pnode-id:
+    maxItems: 1
+  mboxes:
+    maxItems: 2
+  mbox-names:
+    maxItems: 2
+if:
+  properties:
+    compatible:
+      enum:
+        - "xlnx,zynqmp-r5-tcm-1.0"
+
+
+examples:
+  - |
+     reserved-memory {
+          #address-cells = <1>;
+          #size-cells = <1>;
+          ranges;
+          rpu0vdev0vring0: rpu0vdev0vring0@3ed40000 {
+               no-map;
+               reg = <0x3ed40000 0x4000>;
+          };
+          rpu0vdev0vring1: rpu0vdev0vring1@3ed44000 {
+               no-map;
+               reg = <0x3ed44000 0x4000>;
+          };
+          rpu0vdev0buffer: rpu0vdev0buffer@3ed48000 {
+               no-map;
+               reg = <0x3ed48000 0x100000>;
+          };
+          rproc_0_reserved: rproc@3ed000000 {
+               no-map;
+               reg = <0x3ed00000 0x40000>;
+          };
+     };
+     rpu {
+          compatible = "xlnx,zynqmp-r5-remoteproc-1.0";
+          #address-cells = <1>;
+          #size-cells = <1>;
+          ranges;
+          core_conf = "lock-step";
+          r5_0 {
+               ranges;
+               #address-cells = <1>;
+               #size-cells = <1>;
+               memory-region = <&rproc_0_reserved>, <&rpu0vdev0buffer>, <&rpu0vdev0vring0>, <&rpu0vdev0vring1>;
+               pnode-id = <0x7>;
+               mboxes = <&ipi_mailbox_rpu0 0>, <&ipi_mailbox_rpu0 1>;
+               mbox-names = "tx", "rx";
+               tcm_0_a: tcm_0@0 {
+                    #address-cells = <1>;
+                    #size-cells = <1>;
+                    reg = <0xFFE00000 0x10000>;
+                    pnode-id = <0xf>;
+                    compatible = "zynqmp-r5-tcm-1.0";
+               };
+               tcm_0_b: tcm_0@1 {
+                    #address-cells = <2>;
+                    #size-cells = <2>;
+                    reg = <0xFFE20000 0x10000>;
+                    pnode-id = <0x10>;
+                    compatible = "zynqmp-r5-tcm-1.0";
+               };
+          };
+     };
+
+
+     zynqmp_ipi1 {
+          compatible = "xlnx,zynqmp-ipi-mailbox";
+          interrupt-parent = <&gic>;
+          interrupts = <0 29 4>;
+          xlnx,ipi-id = <7>;
+          #address-cells = <1>;
+          #size-cells = <1>;
+          ranges;
+
+          /* APU<->RPU0 IPI mailbox controller */
+          ipi_mailbox_rpu0: mailbox@ff90000 {
+               reg = <0xff990600 0x20>,
+                     <0xff990620 0x20>,
+                     <0xff9900c0 0x20>,
+                     <0xff9900e0 0x20>;
+               reg-names = "local_request_region",
+                        "local_response_region",
+                        "remote_request_region",
+                        "remote_response_region";
+               #mbox-cells = <1>;
+               xlnx,ipi-id = <1>;
+          };
+     };
+
+...
-- 
2.7.4


^ permalink raw reply related

* [PATCH v5 0/5] remoteproc: Add zynqmp_r5 driver
From: Ben Levinsky @ 2020-06-03 14:49 UTC (permalink / raw)
  To: ohad, bjorn.andersson, michals, jollys, rajanv, robh+dt,
	mark.rutland
  Cc: linux-remoteproc, linux-arm-kernell, devicetree, linux-kernel

Provide basic driver to control Arm R5  co-processor found on
Xilinx ZynqMP UltraScale+ and Versal MPSoC's.

Currently it is able to start, stop and load elf on to the
processor.

The driver was tested on Xilinx ZynqMP and Versal.

v2:
- remove domain struct as per review from Mathieu
v3:
- add xilinx-related platform mgmt fn's instead of wrapping around
  function pointer in xilinx eemi ops struct
- update zynqmp_r5 yaml parsing to not raise warnings for extra
  information in children of R5 node. The warning "node has a unit
  name, but no reg or ranges property" will still be raised though 
  as this particular node is needed to describe the
  '#address-cells' and '#size-cells' information.
v4:
- add default values for enums
- fix formatting as per checkpatch.pl --strict. Note that 1 warning and 1 check
  are still raised as each is due to fixing the warning results in that
particular line going over 80 characters.
- remove warning '/example-0/rpu@ff9a0000/r5@0: 
  node has a unit name, but no reg or ranges property'
  by adding reg to r5 node.
v5:
- update device tree sample and yaml parsing to not raise any warnings
- description for memory-region in yaml parsing
- compatible string in yaml parsing for TCM
- parse_fw change from use of rproc_of_resm_mem_entry_init to rproc_mem_entry_init and use of alloc/release
- var's of type zynqmp_r5_pdata all have same local variable name
- use dev_dbg instead of dev_info


Ben Levinsky (5):
  firmware: xilinx: Add ZynqMP firmware ioctl enums for RPU
    configuration.
  firmware: xilinx: Add shutdown/wakeup APIs
  firmware: xilinx: Add RPU configuration APIs
  dt-bindings: remoteproc: Add documentation for ZynqMP R5 rproc
    bindings
  remoteproc: Add initial zynqmp R5 remoteproc driver

 .../remoteproc/xilinx,zynqmp-r5-remoteproc.yaml    | 126 +++
 drivers/firmware/xilinx/zynqmp.c                   | 134 +++
 drivers/remoteproc/Kconfig                         |  10 +
 drivers/remoteproc/Makefile                        |   1 +
 drivers/remoteproc/zynqmp_r5_remoteproc.c          | 902 +++++++++++++++++++++
 include/linux/firmware/xlnx-zynqmp.h               |  75 ++
 6 files changed, 1248 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml
 create mode 100644 drivers/remoteproc/zynqmp_r5_remoteproc.c

-- 
2.7.4


^ permalink raw reply

* [PATCH v5 3/5] firmware: xilinx: Add RPU configuration APIs
From: Ben Levinsky @ 2020-06-03 14:49 UTC (permalink / raw)
  To: ohad, bjorn.andersson, michals, jollys, rajanv, robh+dt,
	mark.rutland
  Cc: linux-remoteproc, linux-arm-kernell, devicetree, linux-kernel
In-Reply-To: <1591195783-10290-1-git-send-email-ben.levinsky@xilinx.com>

This patch adds APIs to provide access and a configuration interface
to the current power state of a sub-system on Zynqmp sub-system.

Signed-off-by: Ben Levinsky <ben.levinsky@xilinx.com>
---
v3:
- add xilinx-related platform mgmt fn's instead of wrapping around
  function pointer in xilinx eemi ops struct
v4:
- add default values for enums

---
 drivers/firmware/xilinx/zynqmp.c     | 99 ++++++++++++++++++++++++++++++++++++
 include/linux/firmware/xlnx-zynqmp.h | 34 +++++++++++++
 2 files changed, 133 insertions(+)

diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index 16a8d69..20c1f58 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -845,6 +845,61 @@ int zynqmp_pm_release_node(const u32 node)
 EXPORT_SYMBOL_GPL(zynqmp_pm_release_node);
 
 /**
+ * zynqmp_pm_get_rpu_mode() - Get RPU mode
+ * @node_id:	Node ID of the device
+ * @arg1:	Argument 1 to requested IOCTL call
+ * @arg2:	Argument 2 to requested IOCTL call
+ * @out:	Returned output value
+ *
+ * Return: RPU mode
+ */
+int zynqmp_pm_get_rpu_mode(u32 node_id,
+			   u32 arg1, u32 arg2, u32 *out)
+{
+	return zynqmp_pm_invoke_fn(PM_IOCTL, node_id,
+				   IOCTL_GET_RPU_OPER_MODE, 0, 0, out);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_get_rpu_mode);
+
+/**
+ * zynqmp_pm_set_rpu_mode() - Set RPU mode
+ * @node_id:	Node ID of the device
+ * @ioctl_id:	ID of the requested IOCTL
+ * @arg2:	Argument 2 to requested IOCTL call
+ * @out:	Returned output value
+ *
+ * This function is used to set RPU mode.
+ *
+ * Return: Returns status, either success or error+reason
+ */
+int zynqmp_pm_set_rpu_mode(u32 node_id,
+			   u32 arg1, u32 arg2, u32 *out)
+{
+	return zynqmp_pm_invoke_fn(PM_IOCTL, node_id,
+				   IOCTL_SET_RPU_OPER_MODE, 0, 0, out);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_set_rpu_mode);
+
+/**
+ * zynqmp_pm_tcm_comb_config - configure TCM
+ * @node_id:	Node ID of the device
+ * @arg1:	Argument 1 to requested IOCTL call
+ * @arg2:	Argument 2 to requested IOCTL call
+ * @out:	Returned output value
+ *
+ * This function is used to set RPU mode.
+ *
+ * Return: Returns status, either success or error+reason
+ */
+int zynqmp_pm_set_tcm_config(u32 node_id,
+			     u32 arg1, u32 arg2, u32 *out)
+{
+	return zynqmp_pm_invoke_fn(PM_IOCTL, node_id,
+				   IOCTL_TCM_COMB_CONFIG, 0, 0, out);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_set_tcm_config);
+
+/**
  * zynqmp_pm_force_powerdown - PM call to request for another PU or subsystem to
  *             be powered down forcefully
  * @target:    Node ID of the targeted PU or subsystem
@@ -880,6 +935,50 @@ int zynqmp_pm_request_wakeup(const u32 node,
 EXPORT_SYMBOL_GPL(zynqmp_pm_request_wakeup);
 
 /**
+ * zynqmp_pm_get_node_status - PM call to request a node's current power state
+ * @node:      ID of the component or sub-system in question
+ * @status:        Current operating state of the requested node
+ * @requirements:  Current requirements asserted on the node,
+ *         used for slave nodes only.
+ * @usage:     Usage information, used for slave nodes only:
+ *         PM_USAGE_NO_MASTER  - No master is currently using
+ *                       the node
+ *         PM_USAGE_CURRENT_MASTER - Only requesting master is
+ *                       currently using the node
+ *         PM_USAGE_OTHER_MASTER   - Only other masters are
+ *                       currently using the node
+ *         PM_USAGE_BOTH_MASTERS   - Both the current and at least
+ *                       one other master is currently
+ *                       using the node
+ *
+ * Return: status, either success or error+reason
+ */
+int zynqmp_pm_get_node_status(const u32 node,
+			      u32 *status, u32 *requirements,
+			      u32 *usage)
+
+{
+	u32 ret_payload[PAYLOAD_ARG_CNT];
+	int ret;
+
+	if (!status)
+		return -EINVAL;
+
+	ret = zynqmp_pm_invoke_fn(PM_GET_NODE_STATUS, node, 0, 0, 0,
+				  ret_payload);
+	if (ret_payload[0] == XST_PM_SUCCESS) {
+		*status = ret_payload[1];
+		if (requirements)
+			*requirements = ret_payload[2];
+		if (usage)
+			*usage = ret_payload[3];
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_get_node_status);
+
+/**
  * zynqmp_pm_set_requirement() - PM call to set requirement for PM slaves
  * @node:		Node ID of the slave
  * @capabilities:	Requested capabilities of the slave
diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index 4d83a7d..0caca9e 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -64,6 +64,7 @@
 
 enum pm_api_id {
 	PM_GET_API_VERSION = 1,
+	PM_GET_NODE_STATUS = 3,
 	PM_FORCE_POWERDOWN = 8,
 	PM_REQUEST_WAKEUP = 10,
 	PM_SYSTEM_SHUTDOWN = 12,
@@ -384,6 +385,14 @@ int zynqmp_pm_request_wakeup(const u32 node,
 			     const bool set_addr,
 			     const u64 address,
 			     const enum zynqmp_pm_request_ack ack);
+int zynqmp_pm_get_node_status(const u32 node, u32 *status,
+			      u32 *requirements, u32 *usage);
+int zynqmp_pm_get_rpu_mode(u32 node_id,
+			   u32 arg1, u32 arg2, u32 *out);
+int zynqmp_pm_set_rpu_mode(u32 node_id,
+			   u32 arg1, u32 arg2, u32 *out);
+int zynqmp_pm_set_tcm_config(u32 node_id,
+			     u32 arg1, u32 arg2, u32 *out);
 #else
 static inline struct zynqmp_eemi_ops *zynqmp_pm_get_eemi_ops(void)
 {
@@ -548,6 +557,31 @@ static inline int zynqmp_pm_request_wakeup(const u32 node,
 {
 	return -ENODEV;
 }
+
+static inline int zynqmp_pm_get_node_status(const u32 node,
+					    u32 *status, u32 *requirements,
+					    u32 *usage)
+{
+	return -ENODEV;
+}
+
+static inline int zynqmp_pm_get_rpu_mode(u32 node_id,
+					 u32 arg1, u32 arg2, u32 *out)
+{
+	return -ENODEV;
+}
+
+static inline int zynqmp_pm_set_rpu_mode(u32 node_id,
+					 u32 arg1, u32 arg2, u32 *out)
+{
+	return -ENODEV;
+}
+
+static inline int zynqmp_pm_set_tcm_config(u32 node_id,
+					   u32 arg1, u32 arg2, u32 *out)
+{
+	return -ENODEV;
+}
 #endif
 
 #endif /* __FIRMWARE_ZYNQMP_H__ */
-- 
2.7.4


^ permalink raw reply related

* [PATCH v5 5/5] remoteproc: Add initial zynqmp R5 remoteproc driver
From: Ben Levinsky @ 2020-06-03 14:49 UTC (permalink / raw)
  To: ohad, bjorn.andersson, michals, jollys, rajanv, robh+dt,
	mark.rutland
  Cc: linux-remoteproc, linux-arm-kernell, devicetree, linux-kernel,
	Wendy Liang, Michal Simek, Ed Mooring, Jason Wu
In-Reply-To: <1591195783-10290-1-git-send-email-ben.levinsky@xilinx.com>

R5 is included in Xilinx Zynq UltraScale MPSoC so by adding this
remotproc driver, we can boot the R5 sub-system in different
configurations.

Acked-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
Acked-by: Ben Levinsky <ben.levinsky@xilinx.com>
Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
Signed-off-by: Ben Levinsky <ben.levinsky@xilinx.com>
Signed-off-by: Wendy Liang <wendy.liang@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Ed Mooring <ed.mooring@xilinx.com>
Signed-off-by: Jason Wu <j.wu@xilinx.com>
Tested-by: Ben Levinsky <ben.levinsky@xilinx.com>
---
v2:
- remove domain struct as per review from Mathieu
v3:
- add xilinx-related platform mgmt fn's instead of wrapping around
  function pointer in xilinx eemi ops struct
v4:
- add default values for enums
- fix formatting as per checkpatch.pl --strict. Note that 1 warning and 1 check
  are still raised as each is due to fixing the warning results in that
particular line going over 80 characters.
v5:
- parse_fw change from use of rproc_of_resm_mem_entry_init to rproc_mem_entry_init and use of alloc/release
- var's of type zynqmp_r5_pdata all have same local variable name
- use dev_dbg instead of dev_info

---
 drivers/remoteproc/Kconfig                |  10 +
 drivers/remoteproc/Makefile               |   1 +
 drivers/remoteproc/zynqmp_r5_remoteproc.c | 898 ++++++++++++++++++++++++++++++
 3 files changed, 909 insertions(+)
 create mode 100644 drivers/remoteproc/zynqmp_r5_remoteproc.c

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index fbaed07..735bd7f 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -240,6 +240,16 @@ config STM32_RPROC
 
 	  This can be either built-in or a loadable module.
 
+config ZYNQMP_R5_REMOTEPROC
+	tristate "ZynqMP_R5 remoteproc support"
+	depends on ARM64 && PM && ARCH_ZYNQMP
+	select RPMSG_VIRTIO
+	select MAILBOX
+	select ZYNQMP_IPI_MBOX
+	help
+	  Say y here to support ZynqMP R5 remote processors via the remote
+	  processor framework.
+
 endif # REMOTEPROC
 
 endmenu
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 0effd38..806ac3f 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -27,5 +27,6 @@ obj-$(CONFIG_QCOM_WCNSS_PIL)		+= qcom_wcnss_pil.o
 qcom_wcnss_pil-y			+= qcom_wcnss.o
 qcom_wcnss_pil-y			+= qcom_wcnss_iris.o
 obj-$(CONFIG_ST_REMOTEPROC)		+= st_remoteproc.o
+obj-$(CONFIG_ZYNQMP_R5_REMOTEPROC)	+= zynqmp_r5_remoteproc.o
 obj-$(CONFIG_ST_SLIM_REMOTEPROC)	+= st_slim_rproc.o
 obj-$(CONFIG_STM32_RPROC)		+= stm32_rproc.o
diff --git a/drivers/remoteproc/zynqmp_r5_remoteproc.c b/drivers/remoteproc/zynqmp_r5_remoteproc.c
new file mode 100644
index 0000000..0e4f3ad
--- /dev/null
+++ b/drivers/remoteproc/zynqmp_r5_remoteproc.c
@@ -0,0 +1,898 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Zynq R5 Remote Processor driver
+ *
+ * Copyright (C) 2019, 2020 Xilinx Inc. Ben Levinsky <ben.levinsky@xilinx.com>
+ * Copyright (C) 2015 - 2018 Xilinx Inc.
+ * Copyright (C) 2015 Jason Wu <j.wu@xilinx.com>
+ *
+ * Based on origin OMAP and Zynq Remote Processor driver
+ *
+ * Copyright (C) 2012 Michal Simek <monstr@monstr.eu>
+ * Copyright (C) 2012 PetaLogix
+ * Copyright (C) 2011 Texas Instruments, Inc.
+ * Copyright (C) 2011 Google, Inc.
+ */
+
+#include <linux/atomic.h>
+#include <linux/cpu.h>
+#include <linux/dma-mapping.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/firmware/xlnx-zynqmp.h>
+#include <linux/genalloc.h>
+#include <linux/idr.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/mailbox_client.h>
+#include <linux/mailbox/zynqmp-ipi-message.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/pfn.h>
+#include <linux/platform_device.h>
+#include <linux/remoteproc.h>
+#include <linux/skbuff.h>
+#include <linux/slab.h>
+#include <linux/sysfs.h>
+
+#include "remoteproc_internal.h"
+
+#define MAX_RPROCS	2 /* Support up to 2 RPU */
+#define MAX_MEM_PNODES	4 /* Max power nodes for one RPU memory instance */
+
+#define DEFAULT_FIRMWARE_NAME	"rproc-rpu-fw"
+
+/* PM proc states */
+#define PM_PROC_STATE_ACTIVE 1U
+
+/* IPI buffer MAX length */
+#define IPI_BUF_LEN_MAX	32U
+/* RX mailbox client buffer max length */
+#define RX_MBOX_CLIENT_BUF_MAX	(IPI_BUF_LEN_MAX + \
+				 sizeof(struct zynqmp_ipi_message))
+
+static bool autoboot __read_mostly;
+
+/**
+ * struct zynqmp_r5_mem - zynqmp rpu memory data
+ * @pnode_id: TCM power domain ids
+ * @res: memory resource
+ * @node: list node
+ */
+struct zynqmp_r5_mem {
+	u32 pnode_id[MAX_MEM_PNODES];
+	struct resource res;
+	struct list_head node;
+};
+
+/**
+ * struct zynqmp_r5_pdata - zynqmp rpu remote processor private data
+ * @dev: device of RPU instance
+ * @rproc: rproc handle
+ * @pnode_id: RPU CPU power domain id
+ * @mems: memory resources
+ * @is_r5_mode_set: indicate if r5 operation mode is set
+ * @tx_mc: tx mailbox client
+ * @rx_mc: rx mailbox client
+ * @tx_chan: tx mailbox channel
+ * @rx_chan: rx mailbox channel
+ * @mbox_work: mbox_work for the RPU remoteproc
+ * @tx_mc_skbs: socket buffers for tx mailbox client
+ * @rx_mc_buf: rx mailbox client buffer to save the rx message
+ */
+struct zynqmp_r5_pdata {
+	struct device dev;
+	struct rproc *rproc;
+	u32 pnode_id;
+	struct list_head mems;
+	bool is_r5_mode_set;
+	struct mbox_client tx_mc;
+	struct mbox_client rx_mc;
+	struct mbox_chan *tx_chan;
+	struct mbox_chan *rx_chan;
+	struct work_struct mbox_work;
+	struct sk_buff_head tx_mc_skbs;
+	unsigned char rx_mc_buf[RX_MBOX_CLIENT_BUF_MAX];
+};
+
+/**
+ * table of RPUs
+ */
+struct zynqmp_r5_pdata rpus[MAX_RPROCS];
+/**
+ *  RPU core configuration
+ */
+enum rpu_oper_mode rpu_mode;
+
+/*
+ * r5_set_mode - set RPU operation mode
+ * @pdata: Remote processor private data
+ *
+ * set RPU oepration mode
+ *
+ * Return: 0 for success, negative value for failure
+ */
+static int r5_set_mode(struct zynqmp_r5_pdata *pdata)
+{
+	u32 val[PAYLOAD_ARG_CNT] = {0}, expect, tcm_mode;
+	struct device *dev = &pdata->dev;
+	int ret;
+
+	expect = (u32)rpu_mode;
+	ret = zynqmp_pm_get_rpu_mode(pdata->pnode_id, 0, 0, val);
+	if (ret < 0) {
+		dev_err(dev, "failed to get RPU oper mode.\n");
+		return ret;
+	}
+	if (val[0] == expect) {
+		dev_dbg(dev, "RPU mode matches: %x\n", val[0]);
+	} else {
+		ret = zynqmp_pm_set_rpu_mode(pdata->pnode_id,
+					     expect, 0, val);
+		if (ret < 0) {
+			dev_err(dev,
+				"failed to set RPU oper mode.\n");
+			return ret;
+		}
+	}
+
+	tcm_mode = (expect == (u32)PM_RPU_MODE_LOCKSTEP) ?
+		    PM_RPU_TCM_COMB : PM_RPU_TCM_SPLIT;
+	ret = zynqmp_pm_set_tcm_config(pdata->pnode_id, tcm_mode, 0, val);
+	if (ret < 0) {
+		dev_err(dev, "failed to config TCM to %x.\n",
+			expect);
+		return ret;
+	}
+	pdata->is_r5_mode_set = true;
+	return 0;
+}
+
+/**
+ * r5_is_running - check if r5 is running
+ * @pdata: Remote processor private data
+ *
+ * check if R5 is running
+ *
+ * Return: true if r5 is running, false otherwise
+ */
+static bool r5_is_running(struct zynqmp_r5_pdata *pdata)
+{
+	u32 status, requirements, usage;
+	struct device *dev = &pdata->dev;
+
+	if (zynqmp_pm_get_node_status(pdata->pnode_id,
+				      &status, &requirements, &usage)) {
+		dev_err(dev, "Failed to get RPU node %d status.\n",
+			pdata->pnode_id);
+		return false;
+	} else if (status != PM_PROC_STATE_ACTIVE) {
+		dev_dbg(dev, "RPU is not running.\n");
+		return false;
+	}
+
+	dev_dbg(dev, "RPU is running.\n");
+	return true;
+}
+
+/*
+ * ZynqMP R5 remoteproc memory release function
+ */
+static int zynqmp_r5_mem_release(struct rproc *rproc,
+				 struct rproc_mem_entry *mem)
+{
+	struct zynqmp_r5_mem *priv;
+	int i, ret;
+	struct device *dev = &rproc->dev;
+
+	priv = mem->priv;
+	if (!priv)
+		return 0;
+	for (i = 0; i < MAX_MEM_PNODES; i++) {
+		if (priv->pnode_id[i]) {
+			dev_dbg(dev, "%s, pnode %d\n",
+				__func__, priv->pnode_id[i]);
+			ret = zynqmp_pm_release_node(priv->pnode_id[i]);
+			if (ret < 0) {
+				dev_err(dev,
+					"failed to release power node: %u\n",
+					priv->pnode_id[i]);
+				return ret;
+			}
+		} else {
+			break;
+		}
+	}
+	return 0;
+}
+
+/*
+ * ZynqMP R5 remoteproc operations
+ */
+static int zynqmp_r5_rproc_start(struct rproc *rproc)
+{
+	struct device *dev = rproc->dev.parent;
+	struct zynqmp_r5_pdata *pdata = rproc->priv;
+	enum rpu_boot_mem bootmem;
+	int ret;
+	/* Set up R5 if not already setup */
+	ret = pdata->is_r5_mode_set ? 0 : r5_set_mode(pdata);
+	if (ret) {
+		dev_err(dev, "failed to set R5 operation mode.\n");
+		return ret;
+	}
+	if ((rproc->bootaddr & 0xF0000000) == 0xF0000000)
+		bootmem = PM_RPU_BOOTMEM_HIVEC;
+	else
+		bootmem = PM_RPU_BOOTMEM_LOVEC;
+	dev_dbg(dev, "RPU boot from %s.",
+		bootmem == PM_RPU_BOOTMEM_HIVEC ? "OCM" : "TCM");
+	ret = zynqmp_pm_request_wakeup(pdata->pnode_id, 1,
+				       bootmem, ZYNQMP_PM_REQUEST_ACK_NO);
+	if (ret < 0) {
+		dev_err(dev, "failed to boot R5.\n");
+		return ret;
+	}
+	return 0;
+}
+
+static int zynqmp_r5_rproc_stop(struct rproc *rproc)
+{
+	struct zynqmp_r5_pdata *pdata = rproc->priv;
+	int ret;
+
+	ret = zynqmp_pm_force_powerdown(pdata->pnode_id,
+					ZYNQMP_PM_REQUEST_ACK_BLOCKING);
+	if (ret < 0) {
+		dev_err(&pdata->dev, "failed to shutdown R5.\n");
+		return ret;
+	}
+	pdata->is_r5_mode_set = false;
+	return 0;
+}
+
+static int zynqmp_r5_rproc_mem_alloc(struct rproc *rproc,
+				      struct rproc_mem_entry *mem)
+{
+	struct device *dev = rproc->dev.parent;
+	void *va;
+
+	dev_dbg(rproc->dev.parent, "map memory: %pa\n", &mem->dma);
+	va = ioremap_wc(mem->dma, mem->len);
+	if (IS_ERR_OR_NULL(va)) {
+		dev_err(dev, "Unable to map memory region: %pa+%x\n",
+			&mem->dma, mem->len);
+		return -ENOMEM;
+	}
+
+	/* Update memory entry va */
+	mem->va = va;
+
+	return 0;
+}
+static int zynqmp_r5_rproc_mem_release(struct rproc *rproc,
+				       struct rproc_mem_entry *mem)
+{
+	dev_dbg(rproc->dev.parent, "unmap memory: %pa\n", &mem->dma);
+	iounmap(mem->va);
+
+	return 0;
+}
+
+static int zynqmp_r5_parse_fw(struct rproc *rproc, const struct firmware *fw)
+{
+	int num_mems, i, ret;
+	struct zynqmp_r5_pdata *pdata = rproc->priv;
+	struct device *dev = &pdata->dev;
+	struct device_node *np = dev->of_node;
+	struct rproc_mem_entry *mem;
+	struct device_node *child;
+	struct resource rsc;
+
+	num_mems = of_count_phandle_with_args(np, "memory-region", NULL);
+	if (num_mems <= 0)
+		return 0;
+	for (i = 0; i < num_mems; i++) {
+		struct device_node *node;
+		struct zynqmp_r5_mem *zynqmp_mem;
+		struct reserved_mem *rmem;
+		char rproc_name[20];
+
+		node = of_parse_phandle(np, "memory-region", i);
+		rmem = of_reserved_mem_lookup(node);
+		if (!rmem) {
+			dev_err(dev, "unable to acquire memory-region\n");
+			return -EINVAL;
+		}
+
+		if (strstr(node->name, "vdev0buffer")) {
+			/* Register DMA region */
+			mem = rproc_mem_entry_init(dev, NULL,
+						   (dma_addr_t)rmem->base,
+						   rmem->size, rmem->base,
+						   NULL, NULL,
+						   "vdev0buffer");
+			if (!mem) {
+				dev_err(dev, "unable to initialize memory-region %s\n",
+					node->name);
+				return -ENOMEM;
+			}
+			dev_dbg(dev, "parsed %s at  %llx\r\n", mem->name,
+				mem->dma);
+			rproc_add_carveout(rproc, mem);
+			continue;
+		} else if (strstr(node->name, "vdev") &&
+				    strstr(node->name, "vring")) {
+			int id, vring_id;
+			char name[16];
+
+			id = node->name[8] - '0';
+			vring_id = node->name[14] - '0';
+			snprintf(name, sizeof(name), "vdev%dvring%d", id,
+				 vring_id);
+			/* Register vring */
+			mem = rproc_mem_entry_init(dev, NULL,
+						   (dma_addr_t)rmem->base,
+						   rmem->size, rmem->base,
+						   zynqmp_r5_rproc_mem_alloc,
+						   zynqmp_r5_rproc_mem_release,
+						   name);
+			dev_dbg(dev, "parsed %s at %llx\r\n", mem->name,
+				mem->dma);
+			rproc_add_carveout(rproc, mem);
+			continue;
+		} else {
+			mem = rproc_mem_entry_init(dev, NULL,
+						   (dma_addr_t)rmem->base,
+						   rmem->size, rmem->base,
+						   zynqmp_r5_rproc_mem_alloc,
+						   zynqmp_r5_rproc_mem_release,
+						   node->name);
+
+			if (!mem) {
+				dev_err(dev,
+					"unable to init memory-region %s\n",
+					node->name);
+				return -ENOMEM;
+			}
+			mem->of_resm_idx = i;
+
+			rproc_add_carveout(rproc, mem);
+		}
+	}
+
+	/* map TCM */
+	for_each_available_child_of_node(np, child) {
+		struct property *prop;
+		const __be32 *cur;
+		u32 pnode_id;
+		void *va;
+		dma_addr_t dma;
+		resource_size_t size;
+
+		ret = of_address_to_resource(child, 0, &rsc);
+
+		i = 0;
+		of_property_for_each_u32(child, "pnode-id", prop, cur,
+					 pnode_id) {
+			ret = zynqmp_pm_request_node(pnode_id,
+				ZYNQMP_PM_CAPABILITY_ACCESS, 0,
+				ZYNQMP_PM_REQUEST_ACK_BLOCKING);
+			if (ret < 0) {
+				dev_err(dev, "failed to request power node: %u\n",
+					pnode_id);
+				return ret;
+			}
+		}
+		size = resource_size(&rsc);
+		va = devm_ioremap_wc(dev, rsc.start, size);
+		if (!va)
+			return -ENOMEM;
+
+		/* zero out tcm base address */
+		if (rsc.start & 0xffe00000) {
+			rsc.start &= 0x000fffff;
+			/* handle tcm banks 1 a and b
+			 * (0xffe9000 and oxffeb0000)
+			 */
+				if (rsc.start & 0x80000)
+					rsc.start -= 0x90000;
+		}
+
+		dma = (dma_addr_t)rsc.start;
+		mem = rproc_mem_entry_init(dev, va, dma, (int)size, rsc.start,
+					   NULL, zynqmp_r5_mem_release,
+					   rsc.name);
+		if (!mem)
+			return -ENOMEM;
+
+		rproc_add_carveout(rproc, mem);
+	}
+
+	ret = rproc_elf_load_rsc_table(rproc, fw);
+	if (ret == -EINVAL)
+		ret = 0;
+	return ret;
+}
+
+/* kick a firmware */
+static void zynqmp_r5_rproc_kick(struct rproc *rproc, int vqid)
+{
+	struct device *dev = rproc->dev.parent;
+	struct zynqmp_r5_pdata *pdata = rproc->priv;
+
+	dev_dbg(dev, "KICK Firmware to start send messages vqid %d\n", vqid);
+
+	if (vqid < 0) {
+		/* If vqid is negative, does not pass the vqid to
+		 * mailbox. As vqid is supposed to be 0 or possive.
+		 * It also gives a way to just kick instead but
+		 * not use the IPI buffer. It is better to provide
+		 * a proper way to pass the short message, which will
+		 * need to sync to upstream first, for now,
+		 * use negative vqid to assume no message will be
+		 * passed with IPI buffer, but just raise interrupt.
+		 * This will be faster as it doesn't need to copy the
+		 * message to the IPI buffer.
+		 *
+		 * It will ignore the return, as failure is due to
+		 * there already kicks in the mailbox queue.
+		 */
+		(void)mbox_send_message(pdata->tx_chan, NULL);
+	} else {
+		struct sk_buff *skb;
+		unsigned int skb_len;
+		struct zynqmp_ipi_message *mb_msg;
+		int ret;
+
+		skb_len = (unsigned int)(sizeof(vqid) + sizeof(mb_msg));
+		skb = alloc_skb(skb_len, GFP_ATOMIC);
+		if (!skb) {
+			dev_err(dev,
+				"Failed to allocate skb to kick remote.\n");
+			return;
+		}
+		mb_msg = (struct zynqmp_ipi_message *)skb_put(skb, skb_len);
+		mb_msg->len = sizeof(vqid);
+		memcpy(mb_msg->data, &vqid, sizeof(vqid));
+		skb_queue_tail(&pdata->tx_mc_skbs, skb);
+		ret = mbox_send_message(pdata->tx_chan, mb_msg);
+		if (ret < 0) {
+			dev_warn(dev, "Failed to kick remote.\n");
+			skb_dequeue_tail(&pdata->tx_mc_skbs);
+			kfree_skb(skb);
+		}
+	}
+}
+
+static struct rproc_ops zynqmp_r5_rproc_ops = {
+	.start		= zynqmp_r5_rproc_start,
+	.stop		= zynqmp_r5_rproc_stop,
+	.load		= rproc_elf_load_segments,
+	.parse_fw	= zynqmp_r5_parse_fw,
+	.find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
+	.sanity_check	= rproc_elf_sanity_check,
+	.get_boot_addr	= rproc_elf_get_boot_addr,
+	.kick		= zynqmp_r5_rproc_kick,
+};
+
+/* zynqmp_r5_mem_probe() - probes RPU TCM memory device
+ * @pdata: pointer to the RPU remoteproc private data
+ * @node: pointer to the memory node
+ *
+ * Function to retrieve resources for RPU TCM memory device.
+ */
+static int zynqmp_r5_mem_probe(struct zynqmp_r5_pdata *pdata,
+			       struct device_node *node)
+{
+	struct device *dev;
+	struct zynqmp_r5_mem *mem;
+	int ret;
+	struct property *prop;
+	const __be32 *cur;
+	u32 val;
+	int i = 0;
+
+	dev = &pdata->dev;
+	mem = devm_kzalloc(dev, sizeof(*mem), GFP_KERNEL);
+	if (!mem)
+		return -ENOMEM;
+	ret = of_address_to_resource(node, 0, &mem->res);
+	if (ret < 0) {
+		dev_err(dev, "failed to get resource of memory %s",
+			of_node_full_name(node));
+		return -EINVAL;
+	}
+
+	/* Get the power domain id */
+	if (of_find_property(node, "pnode-id", NULL)) {
+		of_property_for_each_u32(node, "pnode-id", prop, cur, val)
+			mem->pnode_id[i++] = val;
+	}
+	list_add_tail(&mem->node, &pdata->mems);
+	return 0;
+}
+
+/**
+ * zynqmp_r5_release() - ZynqMP R5 device release function
+ * @dev: pointer to the device struct of ZynqMP R5
+ *
+ * Function to release ZynqMP R5 device.
+ */
+static void zynqmp_r5_release(struct device *dev)
+{
+	struct zynqmp_r5_pdata *pdata;
+	struct rproc *rproc;
+	struct sk_buff *skb;
+
+	pdata = dev_get_drvdata(dev);
+	rproc = pdata->rproc;
+	if (rproc) {
+		rproc_del(rproc);
+		rproc_free(rproc);
+	}
+	if (pdata->tx_chan)
+		mbox_free_channel(pdata->tx_chan);
+	if (pdata->rx_chan)
+		mbox_free_channel(pdata->rx_chan);
+	/* Discard all SKBs */
+	while (!skb_queue_empty(&pdata->tx_mc_skbs)) {
+		skb = skb_dequeue(&pdata->tx_mc_skbs);
+		kfree_skb(skb);
+	}
+
+	put_device(dev->parent);
+}
+
+/**
+ * event_notified_idr_cb() - event notified idr callback
+ * @id: idr id
+ * @ptr: pointer to idr private data
+ * @data: data passed to idr_for_each callback
+ *
+ * Pass notification to remoteproc virtio
+ *
+ * Return: 0. having return is to satisfy the idr_for_each() function
+ *          pointer input argument requirement.
+ **/
+static int event_notified_idr_cb(int id, void *ptr, void *data)
+{
+	struct rproc *rproc = data;
+
+	(void)rproc_vq_interrupt(rproc, id);
+	return 0;
+}
+
+/**
+ * handle_event_notified() - remoteproc notification work funciton
+ * @work: pointer to the work structure
+ *
+ * It checks each registered remoteproc notify IDs.
+ */
+static void handle_event_notified(struct work_struct *work)
+{
+	struct rproc *rproc;
+	struct zynqmp_r5_pdata *pdata;
+
+	pdata = container_of(work, struct zynqmp_r5_pdata, mbox_work);
+
+	(void)mbox_send_message(pdata->rx_chan, NULL);
+	rproc = pdata->rproc;
+	/*
+	 * We only use IPI for interrupt. The firmware side may or may
+	 * not write the notifyid when it trigger IPI.
+	 * And thus, we scan through all the registered notifyids.
+	 */
+	idr_for_each(&rproc->notifyids, event_notified_idr_cb, rproc);
+}
+
+/**
+ * zynqmp_r5_mb_rx_cb() - Receive channel mailbox callback
+ * @cl: mailbox client
+ * @mssg: message pointer
+ *
+ * It will schedule the R5 notification work.
+ */
+static void zynqmp_r5_mb_rx_cb(struct mbox_client *cl, void *mssg)
+{
+	struct zynqmp_r5_pdata *pdata;
+
+	pdata = container_of(cl, struct zynqmp_r5_pdata, rx_mc);
+	if (mssg) {
+		struct zynqmp_ipi_message *ipi_msg, *buf_msg;
+		size_t len;
+
+		ipi_msg = (struct zynqmp_ipi_message *)mssg;
+		buf_msg = (struct zynqmp_ipi_message *)pdata->rx_mc_buf;
+		len = (ipi_msg->len >= IPI_BUF_LEN_MAX) ?
+		      IPI_BUF_LEN_MAX : ipi_msg->len;
+		buf_msg->len = len;
+		memcpy(buf_msg->data, ipi_msg->data, len);
+	}
+	schedule_work(&pdata->mbox_work);
+}
+
+/**
+ * zynqmp_r5_mb_tx_done() - Request has been sent to the remote
+ * @cl: mailbox client
+ * @mssg: pointer to the message which has been sent
+ * @r: status of last TX - OK or error
+ *
+ * It will be called by the mailbox framework when the last TX has done.
+ */
+static void zynqmp_r5_mb_tx_done(struct mbox_client *cl, void *mssg, int r)
+{
+	struct zynqmp_r5_pdata *pdata;
+	struct sk_buff *skb;
+
+	if (!mssg)
+		return;
+	pdata = container_of(cl, struct zynqmp_r5_pdata, tx_mc);
+	skb = skb_dequeue(&pdata->tx_mc_skbs);
+	kfree_skb(skb);
+}
+
+/**
+ * zynqmp_r5_setup_mbox() - Setup mailboxes
+ *
+ * @pdata: pointer to the ZynqMP R5 processor platform data
+ * @node: pointer of the device node
+ *
+ * Function to setup mailboxes to talk to RPU.
+ *
+ * Return: 0 for success, negative value for failure.
+ */
+static int zynqmp_r5_setup_mbox(struct zynqmp_r5_pdata *pdata,
+				struct device_node *node)
+{
+	struct device *dev = &pdata->dev;
+	struct mbox_client *mclient;
+
+	/* Setup TX mailbox channel client */
+	mclient = &pdata->tx_mc;
+	mclient->dev = dev;
+	mclient->rx_callback = NULL;
+	mclient->tx_block = false;
+	mclient->knows_txdone = false;
+	mclient->tx_done = zynqmp_r5_mb_tx_done;
+
+	/* Setup TX mailbox channel client */
+	mclient = &pdata->rx_mc;
+	mclient->dev = dev;
+	mclient->rx_callback = zynqmp_r5_mb_rx_cb;
+	mclient->tx_block = false;
+	mclient->knows_txdone = false;
+
+	INIT_WORK(&pdata->mbox_work, handle_event_notified);
+
+	/* Request TX and RX channels */
+	pdata->tx_chan = mbox_request_channel_byname(&pdata->tx_mc, "tx");
+	if (IS_ERR(pdata->tx_chan)) {
+		dev_err(dev, "failed to request mbox tx channel.\n");
+		pdata->tx_chan = NULL;
+		return -EINVAL;
+	}
+	pdata->rx_chan = mbox_request_channel_byname(&pdata->rx_mc, "rx");
+	if (IS_ERR(pdata->rx_chan)) {
+		dev_err(dev, "failed to request mbox rx channel.\n");
+		pdata->rx_chan = NULL;
+		return -EINVAL;
+	}
+	skb_queue_head_init(&pdata->tx_mc_skbs);
+	return 0;
+}
+
+/**
+ * zynqmp_r5_probe() - Probes ZynqMP R5 processor device node
+ * @pdata: pointer to the ZynqMP R5 processor platform data
+ * @pdev: parent RPU domain platform device
+ * @node: pointer of the device node
+ *
+ * Function to retrieve the information of the ZynqMP R5 device node.
+ *
+ * Return: 0 for success, negative value for failure.
+ */
+static int zynqmp_r5_probe(struct zynqmp_r5_pdata *pdata,
+			   struct platform_device *pdev,
+			   struct device_node *node)
+{
+	struct device *dev = &pdata->dev;
+	struct rproc *rproc;
+	struct device_node *nc;
+	int ret;
+
+	/* Create device for ZynqMP R5 device */
+	dev->parent = &pdev->dev;
+	dev->release = zynqmp_r5_release;
+	dev->of_node = node;
+	dev_set_name(dev, "%s", of_node_full_name(node));
+	dev_set_drvdata(dev, pdata);
+	ret = device_register(dev);
+	if (ret) {
+		dev_err(dev, "failed to register device.\n");
+		return ret;
+	}
+	get_device(&pdev->dev);
+
+	/* Allocate remoteproc instance */
+	rproc = rproc_alloc(dev, dev_name(dev), &zynqmp_r5_rproc_ops, NULL, 0);
+	if (!rproc) {
+		dev_err(dev, "rproc allocation failed.\n");
+		ret = -ENOMEM;
+		goto error;
+	}
+	rproc->auto_boot = autoboot;
+	pdata->rproc = rproc;
+	rproc->priv = pdata;
+
+	/*
+	 * The device has not been spawned from a device tree, so
+	 * arch_setup_dma_ops has not been called, thus leaving
+	 * the device with dummy DMA ops.
+	 * Fix this by inheriting the parent's DMA ops and mask.
+	 */
+	rproc->dev.dma_mask = pdev->dev.dma_mask;
+	set_dma_ops(&rproc->dev, get_dma_ops(&pdev->dev));
+
+	/* Probe R5 memory devices */
+	INIT_LIST_HEAD(&pdata->mems);
+	for_each_available_child_of_node(node, nc) {
+		ret = zynqmp_r5_mem_probe(pdata, nc);
+		if (ret) {
+			dev_err(dev, "failed to probe memory %s.\n",
+				of_node_full_name(nc));
+			goto error;
+		}
+	}
+
+	/* Set up DMA mask */
+	ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
+	if (ret) {
+		dev_warn(dev, "dma_set_coherent_mask failed: %d\n", ret);
+		/* If DMA is not configured yet, try to configure it. */
+		ret = of_dma_configure(dev, node, true);
+		if (ret) {
+			dev_err(dev, "failed to configure DMA.\n");
+			goto error;
+		}
+	}
+
+	/* Get R5 power domain node */
+	ret = of_property_read_u32(node, "pnode-id", &pdata->pnode_id);
+	if (ret) {
+		dev_err(dev, "failed to get power node id.\n");
+		goto error;
+	}
+
+	/* Check if R5 is running */
+	if (r5_is_running(pdata)) {
+		atomic_inc(&rproc->power);
+		rproc->state = RPROC_RUNNING;
+	}
+
+	if (!of_get_property(dev->of_node, "mboxes", NULL)) {
+		dev_dbg(dev, "no mailboxes.\n");
+		goto error;
+	} else {
+		ret = zynqmp_r5_setup_mbox(pdata, node);
+		if (ret < 0)
+			goto error;
+	}
+
+	/* Add R5 remoteproc */
+	ret = rproc_add(rproc);
+	if (ret) {
+		dev_err(dev, "rproc registration failed\n");
+		goto error;
+	}
+	return 0;
+error:
+	if (pdata->rproc)
+		rproc_free(pdata->rproc);
+	pdata->rproc = NULL;
+	device_unregister(dev);
+	put_device(&pdev->dev);
+	return ret;
+}
+
+static int zynqmp_r5_remoteproc_probe(struct platform_device *pdev)
+{
+	const unsigned char *prop;
+	int ret, i;
+	struct device *dev = &pdev->dev;
+	struct device_node *nc;
+	struct zynqmp_r5_pdata *pdata;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+	platform_set_drvdata(pdev, pdata);
+
+	prop = of_get_property(dev->of_node, "core_conf", NULL);
+	if (!prop) {
+		dev_err(&pdev->dev, "core_conf is not used.\n");
+		return -EINVAL;
+	}
+
+	dev_dbg(dev, "RPU core_conf: %s\n", prop);
+	if (!strcmp(prop, "split")) {
+		rpu_mode = PM_RPU_MODE_SPLIT;
+	} else if (!strcmp(prop, "lockstep")) {
+		rpu_mode = PM_RPU_MODE_LOCKSTEP;
+	} else {
+		dev_err(dev,
+			"Invalid core_conf mode provided - %s , %d\n",
+			prop, (int)rpu_mode);
+		return -EINVAL;
+	}
+
+	i = 0;
+	for_each_available_child_of_node(dev->of_node, nc) {
+		ret = zynqmp_r5_probe(&rpus[i], pdev, nc);
+		if (ret) {
+			dev_err(dev, "failed to probe rpu %s.\n",
+				of_node_full_name(nc));
+			return ret;
+		}
+		i++;
+	}
+
+	return 0;
+}
+
+static int zynqmp_r5_remoteproc_remove(struct platform_device *pdev)
+{
+	int i;
+
+	for (i = 0; i < MAX_RPROCS; i++) {
+		struct zynqmp_r5_pdata *pdata = &rpus[i];
+		struct rproc *rproc;
+
+		rproc = pdata->rproc;
+		if (rproc) {
+			rproc_del(rproc);
+			rproc_free(rproc);
+			pdata->rproc = NULL;
+		}
+		if (pdata->tx_chan) {
+			mbox_free_channel(pdata->tx_chan);
+			pdata->tx_chan = NULL;
+		}
+		if (pdata->rx_chan) {
+			mbox_free_channel(pdata->rx_chan);
+			pdata->rx_chan = NULL;
+		}
+
+		device_unregister(&pdata->dev);
+	}
+
+	return 0;
+}
+
+/* Match table for OF platform binding */
+static const struct of_device_id zynqmp_r5_remoteproc_match[] = {
+	{ .compatible = "xlnx,zynqmp-r5-remoteproc-1.0", },
+	{ /* end of list */ },
+};
+MODULE_DEVICE_TABLE(of, zynqmp_r5_remoteproc_match);
+
+static struct platform_driver zynqmp_r5_remoteproc_driver = {
+	.probe = zynqmp_r5_remoteproc_probe,
+	.remove = zynqmp_r5_remoteproc_remove,
+	.driver = {
+		.name = "zynqmp_r5_remoteproc",
+		.of_match_table = zynqmp_r5_remoteproc_match,
+	},
+};
+module_platform_driver(zynqmp_r5_remoteproc_driver);
+
+module_param_named(autoboot,  autoboot, bool, 0444);
+MODULE_PARM_DESC(autoboot,
+		 "enable | disable autoboot. (default: true)");
+
+MODULE_AUTHOR("Ben Levinsky <ben.levinsky@xilinx.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("ZynqMP R5 remote processor control driver");
-- 
2.7.4


^ permalink raw reply related

* [PATCH v5 2/5] firmware: xilinx: Add shutdown/wakeup APIs
From: Ben Levinsky @ 2020-06-03 14:49 UTC (permalink / raw)
  To: ohad, bjorn.andersson, michals, jollys, rajanv, robh+dt,
	mark.rutland
  Cc: linux-remoteproc, linux-arm-kernell, devicetree, linux-kernel
In-Reply-To: <1591195783-10290-1-git-send-email-ben.levinsky@xilinx.com>

Add shutdown/wakeup a resource eemi operations to shutdown
or bringup a resource.

Signed-off-by: Ben Levinsky <ben.levinsky@xilinx.com>
---
v3:
- add xilinx-related platform mgmt fn's instead of wrapping around
  function pointer in xilinx eemi ops struct
- fix formatting
v4:
- add default values for enums

---
 drivers/firmware/xilinx/zynqmp.c     | 35 +++++++++++++++++++++++++++++++++++
 include/linux/firmware/xlnx-zynqmp.h | 22 ++++++++++++++++++++++
 2 files changed, 57 insertions(+)

diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index bfaf29a..16a8d69 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -845,6 +845,41 @@ int zynqmp_pm_release_node(const u32 node)
 EXPORT_SYMBOL_GPL(zynqmp_pm_release_node);
 
 /**
+ * zynqmp_pm_force_powerdown - PM call to request for another PU or subsystem to
+ *             be powered down forcefully
+ * @target:    Node ID of the targeted PU or subsystem
+ * @ack:   Flag to specify whether acknowledge is requested
+ *
+ * Return: status, either success or error+reason
+ */
+int zynqmp_pm_force_powerdown(const u32 target,
+			      const enum zynqmp_pm_request_ack ack)
+{
+	return zynqmp_pm_invoke_fn(PM_FORCE_POWERDOWN, target, ack, 0, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_force_powerdown);
+
+/**
+ * zynqmp_pm_request_wakeup - PM call to wake up selected master or subsystem
+ * @node:  Node ID of the master or subsystem
+ * @set_addr:  Specifies whether the address argument is relevant
+ * @address:   Address from which to resume when woken up
+ * @ack:   Flag to specify whether acknowledge requested
+ *
+ * Return: status, either success or error+reason
+ */
+int zynqmp_pm_request_wakeup(const u32 node,
+			     const bool set_addr,
+			     const u64 address,
+			     const enum zynqmp_pm_request_ack ack)
+{
+	/* set_addr flag is encoded into 1st bit of address */
+	return zynqmp_pm_invoke_fn(PM_REQUEST_WAKEUP, node, address | set_addr,
+				   address >> 32, ack, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_request_wakeup);
+
+/**
  * zynqmp_pm_set_requirement() - PM call to set requirement for PM slaves
  * @node:		Node ID of the slave
  * @capabilities:	Requested capabilities of the slave
diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index bb347df..4d83a7d 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -64,6 +64,8 @@
 
 enum pm_api_id {
 	PM_GET_API_VERSION = 1,
+	PM_FORCE_POWERDOWN = 8,
+	PM_REQUEST_WAKEUP = 10,
 	PM_SYSTEM_SHUTDOWN = 12,
 	PM_REQUEST_NODE = 13,
 	PM_RELEASE_NODE,
@@ -376,6 +378,12 @@ int zynqmp_pm_write_pggs(u32 index, u32 value);
 int zynqmp_pm_read_pggs(u32 index, u32 *value);
 int zynqmp_pm_system_shutdown(const u32 type, const u32 subtype);
 int zynqmp_pm_set_boot_health_status(u32 value);
+int zynqmp_pm_force_powerdown(const u32 target,
+			      const enum zynqmp_pm_request_ack ack);
+int zynqmp_pm_request_wakeup(const u32 node,
+			     const bool set_addr,
+			     const u64 address,
+			     const enum zynqmp_pm_request_ack ack);
 #else
 static inline struct zynqmp_eemi_ops *zynqmp_pm_get_eemi_ops(void)
 {
@@ -526,6 +534,20 @@ static inline int zynqmp_pm_set_boot_health_status(u32 value)
 {
 	return -ENODEV;
 }
+
+static inline int zynqmp_pm_force_powerdown(const u32 target,
+				    const enum zynqmp_pm_request_ack ack)
+{
+	return -ENODEV;
+}
+
+static inline int zynqmp_pm_request_wakeup(const u32 node,
+					   const bool set_addr,
+					   const u64 address,
+				   const enum zynqmp_pm_request_ack ack)
+{
+	return -ENODEV;
+}
 #endif
 
 #endif /* __FIRMWARE_ZYNQMP_H__ */
-- 
2.7.4


^ permalink raw reply related

* Re: --[[SPOOF or PHISHING]]--Re: [PATCH v2 09/12] iio: imu: inv_icm42600: add buffer support in iio devices
From: Jean-Baptiste Maneyrol @ 2020-06-03 14:47 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: robh+dt@kernel.org, robh@kernel.org, mchehab+huawei@kernel.org,
	davem@davemloft.net, gregkh@linuxfoundation.org,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <MN2PR12MB4422CB235D2E2F0C37E86C37C48B0@MN2PR12MB4422.namprd12.prod.outlook.com>

Hi Jonathan,

something I forgot, about the __packed attribute for the sensor data structure struct inv_icm42600_fifo_sensor_data located inside inv_icm42600_buffer.h.

I added it because this structure is used for decoding the FIFO data frame which can be unaligned. It is also used for storing data in other modules, but __packed attribute should not change anything here.

Thanks,
JB



From: linux-iio-owner@vger.kernel.org <linux-iio-owner@vger.kernel.org> on behalf of Jean-Baptiste Maneyrol <JManeyrol@invensense.com>
Sent: Tuesday, June 2, 2020 14:57
To: Jonathan Cameron <jic23@kernel.org>
Cc: robh+dt@kernel.org <robh+dt@kernel.org>; robh@kernel.org <robh@kernel.org>; mchehab+huawei@kernel.org <mchehab+huawei@kernel.org>; davem@davemloft.net <davem@davemloft.net>; gregkh@linuxfoundation.org <gregkh@linuxfoundation.org>; linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; devicetree@vger.kernel.org <devicetree@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>
Subject: --[[SPOOF or PHISHING]]--Re: [PATCH v2 09/12] iio: imu: inv_icm42600: add buffer support in iio devices 
 
 CAUTION: This email originated from outside of the organization. Please make sure the sender is who they say they are and do not click links or open attachments unless you recognize the sender and know the content is safe.

Hi Jonathan,

I agree that this multiplexed watermark computation value is anything except simple and clear to understand.
I will add more documentation about it. And it also triggered a kbuild robot issue, because it is using 64 bits modulo without using math64 macros.

For buffer preenable/postenable/..., the sequence I am using currently is:
- preenable -> turn chip on (pm_runtime_get)
- update_scan_mode -> set FIFO en bits configuration (which sensor data is going into the fifo)
- hwfifo_watermark -> compute and set watermark value
- postenable -> turn FIFO on (and multiplexed with a FIFO on counter since used by accel & gyro)
- predisable -> turn FIFO off (multiplexed with counter)
- postdisable -> turn chip off (pm_runtime_put)

This setting is working well. Good to note that if there is an error when enabling the buffer, postdisable will always get called after preenable. So it ensures pm_runtime reference counter to be always OK.

Another way would be to only store configuration in internal state with update_scan_mode and hwfifo_watermark, and do everything in postenable/predisable. This is a possibility, but makes things a little more complex.

For hwfifo flush, this is an interesting feature when there is a need to have data immediately. Or when there is a need to do a clean change of configuration. In Android systems, Android framework is mainly using FIFO flush to change the sensor configuration (ODR, watermark) in a clean way. For our case with the FIFO interleaved this is a not an issue. If there are samples from the 2 sensors, it means the 2 buffers are enabled. And if data is coming to the iio buffer sooner than expected, that should not be a problem. The limitation I see when the 2 sensors are runnings, is that we will return less data than should have been possible. I limit FIFO reading to the provided n bytes, so we could read less than n samples of 1 sensor.

Something I have in mind, that would be really interesting to be able to set/change watermark value when the buffer is enabled. Otherwise, we are always loosing events by turning sensor off when we want to change the value. Is there any limitation to work this way, or should it be possible to implement this feature in the future ?

Thanks,
JB


From: Jonathan Cameron <jic23@kernel.org>
Sent: Sunday, May 31, 2020 14:56
To: Jean-Baptiste Maneyrol <JManeyrol@invensense.com>
Cc: robh+dt@kernel.org <robh+dt@kernel.org>; robh@kernel.org <robh@kernel.org>; mchehab+huawei@kernel.org <mchehab+huawei@kernel.org>; davem@davemloft.net <davem@davemloft.net>; gregkh@linuxfoundation.org <gregkh@linuxfoundation.org>; linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; devicetree@vger.kernel.org <devicetree@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 09/12] iio: imu: inv_icm42600: add buffer support in iio devices 
 
 CAUTION: This email originated from outside of the organization. Please make sure the sender is who they say they are and do not click links or open attachments unless you recognize the sender and know the content is safe.

On Wed, 27 May 2020 20:57:08 +0200
Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> wrote:

> Add all FIFO parsing and reading functions. Add accel and gyro
> kfifo buffer and FIFO data parsing. Use device interrupt for
> reading data FIFO and launching accel and gyro parsing.
> 
> Support hwfifo watermark by multiplexing gyro and accel settings.
> Support hwfifo flush.

Both of these are complex given the interactions of the two sensors
types and to be honest I couldn't figure out exactly what the intent was.
Needs more docs!

Thanks,

Jonathan

> 
> Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
> ---
>  drivers/iio/imu/inv_icm42600/Kconfig          |   1 +
>  drivers/iio/imu/inv_icm42600/Makefile         |   1 +
>  drivers/iio/imu/inv_icm42600/inv_icm42600.h   |   8 +
>  .../iio/imu/inv_icm42600/inv_icm42600_accel.c | 160 ++++-
>  .../imu/inv_icm42600/inv_icm42600_buffer.c    | 555 ++++++++++++++++++
>  .../imu/inv_icm42600/inv_icm42600_buffer.h    |  98 ++++
>  .../iio/imu/inv_icm42600/inv_icm42600_core.c  |  30 +
>  .../iio/imu/inv_icm42600/inv_icm42600_gyro.c  | 160 ++++-
>  8 files changed, 1011 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
>  create mode 100644 drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.h
> 
> diff --git a/drivers/iio/imu/inv_icm42600/Kconfig b/drivers/iio/imu/inv_icm42600/Kconfig
> index 22390a72f0a3..50cbcfcb6cf1 100644
> --- a/drivers/iio/imu/inv_icm42600/Kconfig
> +++ b/drivers/iio/imu/inv_icm42600/Kconfig
> @@ -2,6 +2,7 @@
>  
>  config INV_ICM42600
>        tristate
> +     select IIO_BUFFER
>  
>  config INV_ICM42600_I2C
>        tristate "InvenSense ICM-426xx I2C driver"
> diff --git a/drivers/iio/imu/inv_icm42600/Makefile b/drivers/iio/imu/inv_icm42600/Makefile
> index 48965824f00c..0f49f6df3647 100644
> --- a/drivers/iio/imu/inv_icm42600/Makefile
> +++ b/drivers/iio/imu/inv_icm42600/Makefile
> @@ -5,6 +5,7 @@ inv-icm42600-y += inv_icm42600_core.o
>  inv-icm42600-y += inv_icm42600_gyro.o
>  inv-icm42600-y += inv_icm42600_accel.o
>  inv-icm42600-y += inv_icm42600_temp.o
> +inv-icm42600-y += inv_icm42600_buffer.o
>  
>  obj-$(CONFIG_INV_ICM42600_I2C) += inv-icm42600-i2c.o
>  inv-icm42600-i2c-y += inv_icm42600_i2c.o
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600.h b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
> index 43749f56426c..4d5811562a61 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600.h
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
> @@ -14,6 +14,8 @@
>  #include <linux/pm.h>
>  #include <linux/iio/iio.h>
>  
> +#include "inv_icm42600_buffer.h"
> +
>  enum inv_icm42600_chip {
>        INV_CHIP_ICM42600,
>        INV_CHIP_ICM42602,
> @@ -123,6 +125,7 @@ struct inv_icm42600_suspended {
>   *  @indio_gyro:     gyroscope IIO device.
>   *  @indio_accel:    accelerometer IIO device.
>   *  @buffer:         data transfer buffer aligned for DMA.
> + *  @fifo:           FIFO management structure.
>   */
>  struct inv_icm42600_state {
>        struct mutex lock;
> @@ -137,6 +140,7 @@ struct inv_icm42600_state {
>        struct iio_dev *indio_gyro;
>        struct iio_dev *indio_accel;
>        uint8_t buffer[2] ____cacheline_aligned;
> +     struct inv_icm42600_fifo fifo;
>  };
>  
>  /* Virtual register addresses: @bank on MSB (4 upper bits), @address on LSB */
> @@ -377,6 +381,10 @@ int inv_icm42600_core_probe(struct regmap *regmap, int chip, int irq,
>  
>  int inv_icm42600_gyro_init(struct inv_icm42600_state *st);
>  
> +int inv_icm42600_gyro_parse_fifo(struct iio_dev *indio_dev);
> +
>  int inv_icm42600_accel_init(struct inv_icm42600_state *st);
>  
> +int inv_icm42600_accel_parse_fifo(struct iio_dev *indio_dev);
> +
>  #endif
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
> index 6a615d7ffb24..c73ce204efc6 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
> @@ -11,9 +11,12 @@
>  #include <linux/delay.h>
>  #include <linux/math64.h>
>  #include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/kfifo_buf.h>
>  
>  #include "inv_icm42600.h"
>  #include "inv_icm42600_temp.h"
> +#include "inv_icm42600_buffer.h"
>  
>  #define INV_ICM42600_ACCEL_CHAN(_modifier, _index, _ext_info)                \
>        {                                                               \
> @@ -64,6 +67,76 @@ static const struct iio_chan_spec inv_icm42600_accel_channels[] = {
>        INV_ICM42600_TEMP_CHAN(INV_ICM42600_ACCEL_SCAN_TEMP),
>  };
>  
> +/* IIO buffer data: 8 bytes */
> +struct inv_icm42600_accel_buffer {
> +     struct inv_icm42600_fifo_sensor_data accel;
> +     int8_t temp;
> +     uint8_t padding;
> +};
> +
> +#define INV_ICM42600_SCAN_MASK_ACCEL_3AXIS                           \
> +     (BIT(INV_ICM42600_ACCEL_SCAN_X) |                               \
> +     BIT(INV_ICM42600_ACCEL_SCAN_Y) |                                \
> +     BIT(INV_ICM42600_ACCEL_SCAN_Z))
> +
> +#define INV_ICM42600_SCAN_MASK_TEMP  BIT(INV_ICM42600_ACCEL_SCAN_TEMP)
> +
> +static const unsigned long inv_icm42600_accel_scan_masks[] = {
> +     /* 3-axis accel + temperature */
> +     INV_ICM42600_SCAN_MASK_ACCEL_3AXIS | INV_ICM42600_SCAN_MASK_TEMP,
> +     0,
> +};
> +
> +/* enable accelerometer sensor and FIFO write */
> +static int inv_icm42600_accel_update_scan_mode(struct iio_dev *indio_dev,
> +                                            const unsigned long *scan_mask)
> +{
> +     struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> +     struct inv_icm42600_sensor_conf conf = INV_ICM42600_SENSOR_CONF_INIT;
> +     unsigned int fifo_en = 0;
> +     unsigned int sleep_temp = 0;
> +     unsigned int sleep_accel = 0;
> +     unsigned int sleep;
> +     int ret;
> +
> +     mutex_lock(&st->lock);
> +
> +     if (*scan_mask & INV_ICM42600_SCAN_MASK_TEMP) {
> +             /* enable temp sensor */
> +             ret = inv_icm42600_set_temp_conf(st, true, &sleep_temp);
> +             if (ret)
> +                     goto out_unlock;
> +             fifo_en |= INV_ICM42600_SENSOR_TEMP;
> +     }
> +
> +     if (*scan_mask & INV_ICM42600_SCAN_MASK_ACCEL_3AXIS) {
> +             /* enable accel sensor */
> +             conf.mode = INV_ICM42600_SENSOR_MODE_LOW_NOISE;
> +             ret = inv_icm42600_set_accel_conf(st, &conf, &sleep_accel);
> +             if (ret)
> +                     goto out_unlock;
> +             fifo_en |= INV_ICM42600_SENSOR_ACCEL;
> +     }
> +
> +     /* update data FIFO write */
> +     ret = inv_icm42600_buffer_set_fifo_en(st, fifo_en | st->fifo.en);
> +     if (ret)
> +             goto out_unlock;
> +
> +     ret = inv_icm42600_buffer_update_watermark(st);
> +
> +out_unlock:
> +     mutex_unlock(&st->lock);
> +     /* sleep maximum required time */
> +     if (sleep_accel > sleep_temp)
> +             sleep = sleep_accel;
> +     else
> +             sleep = sleep_temp;
> +     if (sleep)
> +             msleep(sleep);
> +     return ret;
> +}
> +
>  static int inv_icm42600_accel_read_sensor(struct inv_icm42600_state *st,
>                                          struct iio_chan_spec const *chan,
>                                          int16_t *val)
> @@ -248,7 +321,12 @@ static int inv_icm42600_accel_write_odr(struct inv_icm42600_state *st,
>        mutex_lock(&st->lock);
>  
>        ret = inv_icm42600_set_accel_conf(st, &conf, NULL);
> +     if (ret)
> +             goto out_unlock;
> +     inv_icm42600_buffer_update_fifo_period(st);
> +     inv_icm42600_buffer_update_watermark(st);
>  
> +out_unlock:
>        mutex_unlock(&st->lock);
>        pm_runtime_mark_last_busy(dev);
>        pm_runtime_put_autosuspend(dev);
> @@ -563,12 +641,51 @@ static int inv_icm42600_accel_write_raw_get_fmt(struct iio_dev *indio_dev,
>        }
>  }
>  
> +static int inv_icm42600_accel_hwfifo_set_watermark(struct iio_dev *indio_dev,
> +                                                unsigned int val)
> +{
> +     struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> +     int ret;
> +
> +     mutex_lock(&st->lock);
> +
> +     st->fifo.watermark.accel = val;
> +     ret = inv_icm42600_buffer_update_watermark(st);
> +
> +     mutex_unlock(&st->lock);
> +
> +     return ret;
> +}
> +
> +static int inv_icm42600_accel_hwfifo_flush(struct iio_dev *indio_dev,
> +                                        unsigned int count)
> +{
> +     struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> +     int ret;
> +
> +     if (count == 0)
> +             return 0;
> +
> +     mutex_lock(&st->lock);
> +
> +     ret = inv_icm42600_buffer_hwfifo_flush(st, count);
> +     if (!ret)
> +             ret = st->fifo.nb.accel;
> +
> +     mutex_unlock(&st->lock);
> +
> +     return ret;
> +}
> +
>  static const struct iio_info inv_icm42600_accel_info = {
>        .read_raw = inv_icm42600_accel_read_raw,
>        .read_avail = inv_icm42600_accel_read_avail,
>        .write_raw = inv_icm42600_accel_write_raw,
>        .write_raw_get_fmt = inv_icm42600_accel_write_raw_get_fmt,
>        .debugfs_reg_access = inv_icm42600_debugfs_reg,
> +     .update_scan_mode = inv_icm42600_accel_update_scan_mode,
> +     .hwfifo_set_watermark = inv_icm42600_accel_hwfifo_set_watermark,
> +     .hwfifo_flush_to_buffer = inv_icm42600_accel_hwfifo_flush,
>  };
>  
>  int inv_icm42600_accel_init(struct inv_icm42600_state *st)
> @@ -576,6 +693,7 @@ int inv_icm42600_accel_init(struct inv_icm42600_state *st)
>        struct device *dev = regmap_get_device(st->map);
>        const char *name;
>        struct iio_dev *indio_dev;
> +     struct iio_buffer *buffer;
>  
>        name = devm_kasprintf(dev, GFP_KERNEL, "%s-accel", st->name);
>        if (!name)
> @@ -585,14 +703,54 @@ int inv_icm42600_accel_init(struct inv_icm42600_state *st)
>        if (!indio_dev)
>                return -ENOMEM;
>  
> +     buffer = devm_iio_kfifo_allocate(dev);
> +     if (!buffer)
> +             return -ENOMEM;
> +
>        iio_device_set_drvdata(indio_dev, st);
>        indio_dev->dev.parent = dev;
>        indio_dev->name = name;
>        indio_dev->info = &inv_icm42600_accel_info;
> -     indio_dev->modes = INDIO_DIRECT_MODE;
> +     indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
>        indio_dev->channels = inv_icm42600_accel_channels;
>        indio_dev->num_channels = ARRAY_SIZE(inv_icm42600_accel_channels);
> +     indio_dev->available_scan_masks = inv_icm42600_accel_scan_masks;
> +     indio_dev->setup_ops = &inv_icm42600_buffer_ops;
> +
> +     iio_device_attach_buffer(indio_dev, buffer);
>  
>        st->indio_accel = indio_dev;
>        return devm_iio_device_register(dev, st->indio_accel);
>  }
> +
> +int inv_icm42600_accel_parse_fifo(struct iio_dev *indio_dev)
> +{
> +     struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> +     ssize_t i, size;
> +     const void *accel, *gyro, *timestamp;
> +     const int8_t *temp;
> +     unsigned int odr;
> +     struct inv_icm42600_accel_buffer buffer = {
> +             .padding = 0,
> +     };
> +
> +     /* parse all fifo packets */
> +     for (i = 0; i < st->fifo.count; i += size) {
> +             size = inv_icm42600_fifo_decode_packet(&st->fifo.data[i],
> +                             &accel, &gyro, &temp, &timestamp, &odr);
> +             /* quit if error or FIFO is empty */
> +             if (size <= 0)
> +                     return size;
> +
> +             /* skip packet if no accel data or data is invalid */
> +             if (accel == NULL || !inv_icm42600_fifo_is_data_valid(accel))
> +                     continue;
> +
> +             /* fill and push data buffer */
> +             memcpy(&buffer.accel, accel, sizeof(buffer.accel));
> +             buffer.temp = temp ? *temp : 0;
> +             iio_push_to_buffers(indio_dev, &buffer);
> +     }
> +
> +     return 0;
> +}
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
> new file mode 100644
> index 000000000000..c91075f62231
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
> @@ -0,0 +1,555 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2020 Invensense, Inc.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +#include <linux/mutex.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/delay.h>
> +#include <linux/math64.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +
> +#include "inv_icm42600.h"
> +#include "inv_icm42600_buffer.h"
> +
> +/* FIFO header: 1 byte */
> +#define INV_ICM42600_FIFO_HEADER_MSG         BIT(7)
> +#define INV_ICM42600_FIFO_HEADER_ACCEL               BIT(6)
> +#define INV_ICM42600_FIFO_HEADER_GYRO                BIT(5)
> +#define INV_ICM42600_FIFO_HEADER_TMST_FSYNC  GENMASK(3, 2)
> +#define INV_ICM42600_FIFO_HEADER_ODR_ACCEL   BIT(1)
> +#define INV_ICM42600_FIFO_HEADER_ODR_GYRO    BIT(0)
> +
> +struct inv_icm42600_fifo_1sensor_packet {
> +     uint8_t header;
> +     struct inv_icm42600_fifo_sensor_data data;
> +     int8_t temp;
> +} __packed;
> +#define INV_ICM42600_FIFO_1SENSOR_PACKET_SIZE                8
> +
> +struct inv_icm42600_fifo_2sensors_packet {
> +     uint8_t header;
> +     struct inv_icm42600_fifo_sensor_data accel;
> +     struct inv_icm42600_fifo_sensor_data gyro;
> +     int8_t temp;
> +     __be16 timestamp;
> +} __packed;
> +#define INV_ICM42600_FIFO_2SENSORS_PACKET_SIZE               16
> +
> +ssize_t inv_icm42600_fifo_decode_packet(const void *packet, const void **accel,
> +                                     const void **gyro, const int8_t **temp,
> +                                     const void **timestamp, unsigned int *odr)
> +{
> +     const struct inv_icm42600_fifo_1sensor_packet *pack1 = packet;
> +     const struct inv_icm42600_fifo_2sensors_packet *pack2 = packet;
> +     uint8_t header = *((const uint8_t *)packet);
> +
> +     /* FIFO empty */
> +     if (header & INV_ICM42600_FIFO_HEADER_MSG) {
> +             *accel = NULL;
> +             *gyro = NULL;
> +             *temp = NULL;
> +             *timestamp = NULL;
> +             *odr = 0;
> +             return 0;
> +     }
> +
> +     /* handle odr flags */
> +     *odr = 0;
> +     if (header & INV_ICM42600_FIFO_HEADER_ODR_GYRO)
> +             *odr |= INV_ICM42600_SENSOR_GYRO;
> +     if (header & INV_ICM42600_FIFO_HEADER_ODR_ACCEL)
> +             *odr |= INV_ICM42600_SENSOR_ACCEL;
> +
> +     /* accel + gyro */
> +     if ((header & INV_ICM42600_FIFO_HEADER_ACCEL) &&
> +         (header & INV_ICM42600_FIFO_HEADER_GYRO)) {
> +             *accel = &pack2->accel;
> +             *gyro = &pack2->gyro;
> +             *temp = &pack2->temp;
> +             *timestamp = &pack2->timestamp;
> +             return INV_ICM42600_FIFO_2SENSORS_PACKET_SIZE;
> +     }
> +
> +     /* accel only */
> +     if (header & INV_ICM42600_FIFO_HEADER_ACCEL) {
> +             *accel = &pack1->data;
> +             *gyro = NULL;
> +             *temp = &pack1->temp;
> +             *timestamp = NULL;
> +             return INV_ICM42600_FIFO_1SENSOR_PACKET_SIZE;
> +     }
> +
> +     /* gyro only */
> +     if (header & INV_ICM42600_FIFO_HEADER_GYRO) {
> +             *accel = NULL;
> +             *gyro = &pack1->data;
> +             *temp = &pack1->temp;
> +             *timestamp = NULL;
> +             return INV_ICM42600_FIFO_1SENSOR_PACKET_SIZE;
> +     }
> +
> +     /* invalid packet if here */
> +     return -EINVAL;
> +}
> +
> +void inv_icm42600_buffer_update_fifo_period(struct inv_icm42600_state *st)
> +{
> +     uint32_t period_gyro, period_accel, period;
> +
> +     if (st->fifo.en & INV_ICM42600_SENSOR_GYRO)
> +             period_gyro = inv_icm42600_odr_to_period(st->conf.gyro.odr);
> +     else
> +             period_gyro = U32_MAX;
> +
> +     if (st->fifo.en & INV_ICM42600_SENSOR_ACCEL)
> +             period_accel = inv_icm42600_odr_to_period(st->conf.accel.odr);
> +     else
> +             period_accel = U32_MAX;
> +
> +     if (period_gyro <= period_accel)
> +             period = period_gyro;
> +     else
> +             period = period_accel;
> +
> +     st->fifo.period = period;
> +}
> +
> +int inv_icm42600_buffer_set_fifo_en(struct inv_icm42600_state *st,
> +                                 unsigned int fifo_en)
> +{
> +     unsigned int mask, val;
> +     int ret;
> +
> +     /* update only FIFO EN bits */
> +     mask = INV_ICM42600_FIFO_CONFIG1_TMST_FSYNC_EN |
> +             INV_ICM42600_FIFO_CONFIG1_TEMP_EN |
> +             INV_ICM42600_FIFO_CONFIG1_GYRO_EN |
> +             INV_ICM42600_FIFO_CONFIG1_ACCEL_EN;
> +
> +     val = 0;
> +     if (fifo_en & INV_ICM42600_SENSOR_GYRO)
> +             val |= INV_ICM42600_FIFO_CONFIG1_GYRO_EN;
> +     if (fifo_en & INV_ICM42600_SENSOR_ACCEL)
> +             val |= INV_ICM42600_FIFO_CONFIG1_ACCEL_EN;
> +     if (fifo_en & INV_ICM42600_SENSOR_TEMP)
> +             val |= INV_ICM42600_FIFO_CONFIG1_TEMP_EN;
> +
> +     ret = regmap_update_bits(st->map, INV_ICM42600_REG_FIFO_CONFIG1,
> +                              mask, val);
> +     if (ret)
> +             return ret;
> +
> +     st->fifo.en = fifo_en;
> +     inv_icm42600_buffer_update_fifo_period(st);
> +
> +     return 0;
> +}
> +
> +static size_t inv_icm42600_get_packet_size(unsigned int fifo_en)
> +{
> +     size_t packet_size;
> +
> +     if ((fifo_en & INV_ICM42600_SENSOR_GYRO) &&
> +         (fifo_en & INV_ICM42600_SENSOR_ACCEL))
> +             packet_size = INV_ICM42600_FIFO_2SENSORS_PACKET_SIZE;
> +     else
> +             packet_size = INV_ICM42600_FIFO_1SENSOR_PACKET_SIZE;
> +
> +     return packet_size;
> +}
> +
> +static unsigned int inv_icm42600_wm_truncate(unsigned int watermark,
> +                                          size_t packet_size)
> +{
> +     size_t wm_size;
> +     unsigned int wm;
> +
> +     wm_size = watermark * packet_size;
> +     if (wm_size > INV_ICM42600_FIFO_WATERMARK_MAX)
> +             wm_size = INV_ICM42600_FIFO_WATERMARK_MAX;
> +
> +     wm = wm_size / packet_size;
> +
> +     return wm;
> +}
> +

I think some overview docs on how this is working would be good.
Set out the aims for the watermark selected and how it is achieved.

> +int inv_icm42600_buffer_update_watermark(struct inv_icm42600_state *st)
> +{
> +     size_t packet_size, wm_size;
> +     unsigned int wm_gyro, wm_accel, watermark;
> +     uint32_t period_gyro, period_accel, period;
> +     int64_t latency_gyro, latency_accel, latency;
> +     bool restore;
> +     __le16 raw_wm;
> +     int ret;
> +
> +     packet_size = inv_icm42600_get_packet_size(st->fifo.en);
> +
> +     /* get minimal latency, depending on sensor watermark and odr */
> +     wm_gyro = inv_icm42600_wm_truncate(st->fifo.watermark.gyro,
> +                                        packet_size);
> +     wm_accel = inv_icm42600_wm_truncate(st->fifo.watermark.accel,
> +                                         packet_size);
> +     period_gyro = inv_icm42600_odr_to_period(st->conf.gyro.odr);
> +     period_accel = inv_icm42600_odr_to_period(st->conf.accel.odr);
> +     latency_gyro = (int64_t)period_gyro * (int64_t)wm_gyro;
> +     latency_accel = (int64_t)period_accel * (int64_t)wm_accel;
> +     if (latency_gyro == 0) {
> +             latency = latency_accel;
> +             watermark = wm_accel;
> +     } else if (latency_accel == 0) {
> +             latency = latency_gyro;
> +             watermark = wm_gyro;
> +     } else {
> +             /* compute the smallest latency that is a multiple of both */
> +             if (latency_gyro <= latency_accel) {
> +                     latency = latency_gyro;
> +                     latency -= latency_accel % latency_gyro;
> +             } else {
> +                     latency = latency_accel;
> +                     latency -= latency_gyro % latency_accel;
> +             }
> +             /* use the shortest period */
> +             if (period_gyro <= period_accel)
> +                     period = period_gyro;
> +             else
> +                     period = period_accel;
> +             /* all this works because periods are multiple of each others */
> +             watermark = div_s64(latency, period);
> +             if (watermark < 1)
> +                     watermark = 1;
> +     }
> +     wm_size = watermark * packet_size;
> +
> +     /* changing FIFO watermark requires to turn off watermark interrupt */
> +     ret = regmap_update_bits_check(st->map, INV_ICM42600_REG_INT_SOURCE0,
> +                                    INV_ICM42600_INT_SOURCE0_FIFO_THS_INT1_EN,
> +                                    0, &restore);
> +     if (ret)
> +             return ret;
> +
> +     raw_wm = INV_ICM42600_FIFO_WATERMARK_VAL(wm_size);
> +     memcpy(st->buffer, &raw_wm, sizeof(raw_wm));
> +     ret = regmap_bulk_write(st->map, INV_ICM42600_REG_FIFO_WATERMARK,
> +                             st->buffer, sizeof(raw_wm));
> +     if (ret)
> +             return ret;
> +
> +     /* restore watermark interrupt */
> +     if (restore) {
> +             ret = regmap_update_bits(st->map, INV_ICM42600_REG_INT_SOURCE0,
> +                                      INV_ICM42600_INT_SOURCE0_FIFO_THS_INT1_EN,
> +                                      INV_ICM42600_INT_SOURCE0_FIFO_THS_INT1_EN);
> +             if (ret)
> +                     return ret;
> +     }
> +
> +     return 0;
> +}
> +
> +static int inv_icm42600_buffer_preenable(struct iio_dev *indio_dev)
> +{
> +     struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> +     struct device *dev = regmap_get_device(st->map);
> +
> +     pm_runtime_get_sync(dev);
> +
> +     return 0;
> +}
> +
> +/*
> + * update_scan_mode callback is turning sensors on and setting data FIFO enable
> + * bits.
> + */
> +static int inv_icm42600_buffer_postenable(struct iio_dev *indio_dev)
> +{
> +     struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> +     int ret;
> +
> +     mutex_lock(&st->lock);
> +
> +     /* exit if FIFO is already on */
> +     if (st->fifo.on) {
> +             ret = 0;
> +             goto out_on;
> +     }
> +
> +     /* set FIFO threshold interrupt */
> +     ret = regmap_update_bits(st->map, INV_ICM42600_REG_INT_SOURCE0,
> +                              INV_ICM42600_INT_SOURCE0_FIFO_THS_INT1_EN,
> +                              INV_ICM42600_INT_SOURCE0_FIFO_THS_INT1_EN);
> +     if (ret)
> +             goto out_unlock;
> +
> +     /* flush FIFO data */
> +     ret = regmap_write(st->map, INV_ICM42600_REG_SIGNAL_PATH_RESET,
> +                        INV_ICM42600_SIGNAL_PATH_RESET_FIFO_FLUSH);
> +     if (ret)
> +             goto out_unlock;
> +
> +     /* set FIFO in streaming mode */
> +     ret = regmap_write(st->map, INV_ICM42600_REG_FIFO_CONFIG,
> +                        INV_ICM42600_FIFO_CONFIG_STREAM);
> +     if (ret)
> +             goto out_unlock;
> +
> +     /* workaround: first read of FIFO count after reset is always 0 */
> +     ret = regmap_bulk_read(st->map, INV_ICM42600_REG_FIFO_COUNT, st->buffer, 2);
> +     if (ret)
> +             goto out_unlock;
> +
> +out_on:
> +     /* increase FIFO on counter */
> +     st->fifo.on++;
> +out_unlock:
> +     mutex_unlock(&st->lock);
> +     return ret;
> +}
> +
> +static int inv_icm42600_buffer_predisable(struct iio_dev *indio_dev)
> +{
> +     struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> +     int ret;
> +
> +     mutex_lock(&st->lock);
> +
> +     /* exit if there are several sensors using the FIFO */
> +     if (st->fifo.on > 1) {
> +             ret = 0;
> +             goto out_off;
> +     }
> +
> +     /* set FIFO in bypass mode */
> +     ret = regmap_write(st->map, INV_ICM42600_REG_FIFO_CONFIG,
> +                        INV_ICM42600_FIFO_CONFIG_BYPASS);
> +     if (ret)
> +             goto out_unlock;
> +
> +     /* flush FIFO data */
> +     ret = regmap_write(st->map, INV_ICM42600_REG_SIGNAL_PATH_RESET,
> +                        INV_ICM42600_SIGNAL_PATH_RESET_FIFO_FLUSH);
> +     if (ret)
> +             goto out_unlock;
> +
> +     /* disable FIFO threshold interrupt */
> +     ret = regmap_update_bits(st->map, INV_ICM42600_REG_INT_SOURCE0,
> +                              INV_ICM42600_INT_SOURCE0_FIFO_THS_INT1_EN, 0);
> +     if (ret)
> +             goto out_unlock;
> +
> +out_off:
> +     /* decrease FIFO on counter */
> +     st->fifo.on--;
> +out_unlock:
> +     mutex_unlock(&st->lock);
> +     return ret;
> +}
> +
> +static int inv_icm42600_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> +     struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> +     struct device *dev = regmap_get_device(st->map);
> +     unsigned int sensor;
> +     unsigned int *watermark;
> +     struct inv_icm42600_sensor_conf conf = INV_ICM42600_SENSOR_CONF_INIT;
> +     unsigned int sleep_temp = 0;
> +     unsigned int sleep_sensor = 0;
> +     unsigned int sleep;
> +     int ret;
> +
> +     if (indio_dev == st->indio_gyro) {
> +             sensor = INV_ICM42600_SENSOR_GYRO;
> +             watermark = &st->fifo.watermark.gyro;
> +     } else if (indio_dev == st->indio_accel) {
> +             sensor = INV_ICM42600_SENSOR_ACCEL;
> +             watermark = &st->fifo.watermark.accel;
> +     } else {
> +             return -EINVAL;
> +     }
> +
> +     mutex_lock(&st->lock);
> +
> +     ret = inv_icm42600_buffer_set_fifo_en(st, st->fifo.en & ~sensor);
> +     if (ret)
> +             goto out_unlock;
> +
> +     *watermark = 0;
> +     ret = inv_icm42600_buffer_update_watermark(st);
> +     if (ret)
> +             goto out_unlock;
> +
> +     conf.mode = INV_ICM42600_SENSOR_MODE_OFF;
> +     if (sensor == INV_ICM42600_SENSOR_GYRO)
> +             ret = inv_icm42600_set_gyro_conf(st, &conf, &sleep_sensor);
> +     else
> +             ret = inv_icm42600_set_accel_conf(st, &conf, &sleep_sensor);
> +     if (ret)
> +             goto out_unlock;
> +
> +     /* if FIFO is off, turn temperature off */
> +     if (!st->fifo.on)
> +             ret = inv_icm42600_set_temp_conf(st, false, &sleep_temp);
> +
> +out_unlock:
> +     mutex_unlock(&st->lock);
> +
> +     /* sleep maximum required time */
> +     if (sleep_sensor > sleep_temp)
> +             sleep = sleep_sensor;
> +     else
> +             sleep = sleep_temp;
> +     if (sleep)
> +             msleep(sleep);
> +
> +     pm_runtime_mark_last_busy(dev);
> +     pm_runtime_put_autosuspend(dev);
> +
> +     return ret;
> +}
> +
> +const struct iio_buffer_setup_ops inv_icm42600_buffer_ops = {
> +     .preenable = inv_icm42600_buffer_preenable,
> +     .postenable = inv_icm42600_buffer_postenable,

We've been slowly eroding the difference between preenable and posteenable.
Would be good to understand why you need to define both?

> +     .predisable = inv_icm42600_buffer_predisable,
> +     .postdisable = inv_icm42600_buffer_postdisable,
> +};
> +
> +int inv_icm42600_buffer_fifo_read(struct inv_icm42600_state *st,
> +                               unsigned int max)
> +{
> +     size_t max_count;
> +     __be16 *raw_fifo_count;
> +     ssize_t i, size;
> +     const void *accel, *gyro, *timestamp;
> +     const int8_t *temp;
> +     unsigned int odr;
> +     int ret;
> +
> +     /* reset all samples counters */
> +     st->fifo.count = 0;
> +     st->fifo.nb.gyro = 0;
> +     st->fifo.nb.accel = 0;
> +     st->fifo.nb.total = 0;
> +
> +     /* compute maximum FIFO read size */
> +     if (max == 0)
> +             max_count = sizeof(st->fifo.data);
> +     else
> +             max_count = max * inv_icm42600_get_packet_size(st->fifo.en);
> +
> +     /* read FIFO count value */
> +     raw_fifo_count = (__be16 *)st->buffer;
> +     ret = regmap_bulk_read(st->map, INV_ICM42600_REG_FIFO_COUNT,
> +                            raw_fifo_count, sizeof(*raw_fifo_count));
> +     if (ret)
> +             return ret;
> +     st->fifo.count = be16_to_cpup(raw_fifo_count);
> +
> +     /* check and clamp FIFO count value */
> +     if (st->fifo.count == 0)
> +             return 0;
> +     if (st->fifo.count > max_count)
> +             st->fifo.count = max_count;
> +
> +     /* read all FIFO data in internal buffer */
> +     ret = regmap_noinc_read(st->map, INV_ICM42600_REG_FIFO_DATA,
> +                             st->fifo.data, st->fifo.count);
> +     if (ret)
> +             return ret;
> +
> +     /* compute number of samples for each sensor */
> +     for (i = 0; i < st->fifo.count; i += size) {
> +             size = inv_icm42600_fifo_decode_packet(&st->fifo.data[i],
> +                             &accel, &gyro, &temp, &timestamp, &odr);
> +             if (size <= 0)
> +                     break;
> +             if (gyro != NULL && inv_icm42600_fifo_is_data_valid(gyro))
> +                     st->fifo.nb.gyro++;
> +             if (accel != NULL && inv_icm42600_fifo_is_data_valid(accel))
> +                     st->fifo.nb.accel++;
> +             st->fifo.nb.total++;
> +     }
> +
> +     return 0;
> +}
> +
> +int inv_icm42600_buffer_fifo_parse(struct inv_icm42600_state *st)
> +{
> +     int ret;
> +
> +     if (st->fifo.nb.total == 0)
> +             return 0;
> +
> +     if (st->fifo.nb.gyro > 0) {
> +             ret = inv_icm42600_gyro_parse_fifo(st->indio_gyro);
> +             if (ret)
> +                     return ret;
> +     }
> +
> +     if (st->fifo.nb.accel > 0) {
> +             ret = inv_icm42600_accel_parse_fifo(st->indio_accel);
> +             if (ret)
> +                     return ret;
> +     }
> +
> +     return 0;
> +}
> +
> +int inv_icm42600_buffer_hwfifo_flush(struct inv_icm42600_state *st,
> +                                  unsigned int count)
> +{
> +     int ret;
> +
> +     ret = inv_icm42600_buffer_fifo_read(st, count);
> +     if (ret)
> +             return ret;
Definitely searching my memory for how this works in the core, so
I may have it wrong.

This is a bit unusual (I think).  The intent of the flush
is to read up to 'n' bytes because someone just did a read on the buffer
or select, and there was data in the hwfifo capable of satisfying the read
even though we haven't yet reached the watermark.

Given both sensor types are coming from one buffer, do we have a potential
issue here or under serving even though data is available?

The case I worry may be served late is when an poll / select
is waiting for sufficient data.

So what should we be doing?  We want to guarantee to provide data
for each sensor type if it's in the hwfifo. As such we could keep reading
until we have enough, but that could cause some issues if the two data rates
are very different (overflow on the other kfifo)

Maybe what you have here is the best we can do. 

I'm assuming the watermark level has a similar problem.  One value represents
the sum of the two types of data.

> +
> +     if (st->fifo.nb.total == 0)
> +             return 0;
> +
> +     if (st->fifo.nb.gyro > 0) {
> +             ret = inv_icm42600_gyro_parse_fifo(st->indio_gyro);
> +             if (ret)
> +                     return ret;
> +     }
> +
> +     if (st->fifo.nb.accel > 0) {
> +             ret = inv_icm42600_accel_parse_fifo(st->indio_accel);
> +             if (ret)
> +                     return ret;
> +     }
> +
> +     return 0;
> +}
> +
> +int inv_icm42600_buffer_init(struct inv_icm42600_state *st)
> +{
> +     unsigned int val;
> +     int ret;
> +
> +     /*
> +      * Default FIFO configuration (bits 7 to 5)
> +      * - use invalid value
> +      * - FIFO count in bytes
> +      * - FIFO count in big endian
> +      */
> +     val = INV_ICM42600_INTF_CONFIG0_FIFO_COUNT_ENDIAN;
> +     ret = regmap_update_bits(st->map, INV_ICM42600_REG_INTF_CONFIG0,
> +                              GENMASK(7, 5), val);
> +     if (ret)
> +             return ret;
> +
> +     /*
> +      * Enable FIFO partial read and continuous watermark interrupt.
> +      * Disable all FIFO EN bits.
> +      */
> +     val = INV_ICM42600_FIFO_CONFIG1_RESUME_PARTIAL_RD |
> +           INV_ICM42600_FIFO_CONFIG1_WM_GT_TH;
> +     return regmap_update_bits(st->map, INV_ICM42600_REG_FIFO_CONFIG1,
> +                               GENMASK(6, 5) | GENMASK(3, 0), val);
> +}
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.h b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.h
> new file mode 100644
> index 000000000000..de2a3949dcc7
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.h
> @@ -0,0 +1,98 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020 Invensense, Inc.
> + */
> +
> +#ifndef INV_ICM42600_BUFFER_H_
> +#define INV_ICM42600_BUFFER_H_
> +
> +#include <linux/kernel.h>
> +#include <linux/bits.h>
> +
> +struct inv_icm42600_state;
> +
> +#define INV_ICM42600_SENSOR_GYRO     BIT(0)
> +#define INV_ICM42600_SENSOR_ACCEL    BIT(1)
> +#define INV_ICM42600_SENSOR_TEMP     BIT(2)
> +
> +/**
> + * struct inv_icm42600_fifo - FIFO state variables
> + * @on:              reference counter for FIFO on.
> + * @en:              bits field of INV_ICM42600_SENSOR_* for FIFO EN bits.
> + * @period:  FIFO internal period.
> + * @watermark:       watermark configuration values for accel and gyro.
> + * @count:   number of bytes in the FIFO data buffer.
> + * @nb:              gyro, accel and total samples in the FIFO data buffer.
> + * @data:    FIFO data buffer aligned for DMA (2kB + 32 bytes of read cache).
> + */
> +struct inv_icm42600_fifo {
> +     unsigned int on;
> +     unsigned int en;
> +     uint32_t period;
> +     struct {
> +             unsigned int gyro;
> +             unsigned int accel;
> +     } watermark;
> +     size_t count;
> +     struct {
> +             size_t gyro;
> +             size_t accel;
> +             size_t total;
> +     } nb;
> +     uint8_t data[2080] ____cacheline_aligned;
> +};
> +
> +/* FIFO data packet */
> +struct inv_icm42600_fifo_sensor_data {
> +     __be16 x;
> +     __be16 y;
> +     __be16 z;
> +} __packed;

Why packed?  Should be anyway I think.

> +#define INV_ICM42600_FIFO_DATA_INVALID               -32768
> +
> +static inline int16_t inv_icm42600_fifo_get_sensor_data(__be16 d)
> +{
> +     return be16_to_cpu(d);
> +}
> +
> +static inline bool
> +inv_icm42600_fifo_is_data_valid(const struct inv_icm42600_fifo_sensor_data *s)
> +{
> +     int16_t x, y, z;
> +
> +     x = inv_icm42600_fifo_get_sensor_data(s->x);
> +     y = inv_icm42600_fifo_get_sensor_data(s->y);
> +     z = inv_icm42600_fifo_get_sensor_data(s->z);
> +
> +     if (x == INV_ICM42600_FIFO_DATA_INVALID &&
> +         y == INV_ICM42600_FIFO_DATA_INVALID &&
> +         z == INV_ICM42600_FIFO_DATA_INVALID)
> +             return false;
> +
> +     return true;
> +}
> +
> +ssize_t inv_icm42600_fifo_decode_packet(const void *packet, const void **accel,
> +                                     const void **gyro, const int8_t **temp,
> +                                     const void **timestamp, unsigned int *odr);
> +
> +extern const struct iio_buffer_setup_ops inv_icm42600_buffer_ops;
> +
> +int inv_icm42600_buffer_init(struct inv_icm42600_state *st);
> +
> +void inv_icm42600_buffer_update_fifo_period(struct inv_icm42600_state *st);
> +
> +int inv_icm42600_buffer_set_fifo_en(struct inv_icm42600_state *st,
> +                                 unsigned int fifo_en);
> +
> +int inv_icm42600_buffer_update_watermark(struct inv_icm42600_state *st);
> +
> +int inv_icm42600_buffer_fifo_read(struct inv_icm42600_state *st,
> +                               unsigned int max);
> +
> +int inv_icm42600_buffer_fifo_parse(struct inv_icm42600_state *st);
> +
> +int inv_icm42600_buffer_hwfifo_flush(struct inv_icm42600_state *st,
> +                                  unsigned int count);
> +
> +#endif
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> index 246c1eb52231..6f1c1eb83953 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> @@ -18,6 +18,7 @@
>  #include <linux/iio/iio.h>
>  
>  #include "inv_icm42600.h"
> +#include "inv_icm42600_buffer.h"
>  
>  static const struct regmap_range_cfg inv_icm42600_regmap_ranges[] = {
>        {
> @@ -429,6 +430,18 @@ static irqreturn_t inv_icm42600_irq_handler(int irq, void *_data)
>        if (status & INV_ICM42600_INT_STATUS_FIFO_FULL)
>                dev_warn(dev, "FIFO full data lost!\n");
>  
> +     /* FIFO threshold reached */
> +     if (status & INV_ICM42600_INT_STATUS_FIFO_THS) {
> +             ret = inv_icm42600_buffer_fifo_read(st, 0);
> +             if (ret) {
> +                     dev_err(dev, "FIFO read error %d\n", ret);
> +                     goto out_unlock;
> +             }
> +             ret = inv_icm42600_buffer_fifo_parse(st);
> +             if (ret)
> +                     dev_err(dev, "FIFO parsing error %d\n", ret);
> +     }
> +
>  out_unlock:
>        mutex_unlock(&st->lock);
>        return IRQ_HANDLED;
> @@ -600,6 +613,10 @@ int inv_icm42600_core_probe(struct regmap *regmap, int chip, int irq,
>        if (ret)
>                return ret;
>  
> +     ret = inv_icm42600_buffer_init(st);
> +     if (ret)
> +             return ret;
> +
>        ret = inv_icm42600_gyro_init(st);
>        if (ret)
>                return ret;
> @@ -645,6 +662,14 @@ static int __maybe_unused inv_icm42600_suspend(struct device *dev)
>                goto out_unlock;
>        }
>  
> +     /* disable FIFO data streaming */
> +     if (st->fifo.on) {
> +             ret = regmap_write(st->map, INV_ICM42600_REG_FIFO_CONFIG,
> +                                INV_ICM42600_FIFO_CONFIG_BYPASS);
> +             if (ret)
> +                     goto out_unlock;
> +     }
> +
>        ret = inv_icm42600_set_pwr_mgmt0(st, INV_ICM42600_SENSOR_MODE_OFF,
>                                         INV_ICM42600_SENSOR_MODE_OFF, false,
>                                         NULL);
> @@ -684,6 +709,11 @@ static int __maybe_unused inv_icm42600_resume(struct device *dev)
>        if (ret)
>                goto out_unlock;
>  
> +     /* restore FIFO data streaming */
> +     if (st->fifo.on)
> +             ret = regmap_write(st->map, INV_ICM42600_REG_FIFO_CONFIG,
> +                                INV_ICM42600_FIFO_CONFIG_STREAM);
> +
>  out_unlock:
>        mutex_unlock(&st->lock);
>        return ret;
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
> index 38654e0d217b..b05c33876b8d 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
> @@ -11,9 +11,12 @@
>  #include <linux/delay.h>
>  #include <linux/math64.h>
>  #include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/kfifo_buf.h>
>  
>  #include "inv_icm42600.h"
>  #include "inv_icm42600_temp.h"
> +#include "inv_icm42600_buffer.h"
>  
>  #define INV_ICM42600_GYRO_CHAN(_modifier, _index, _ext_info)         \
>        {                                                               \
> @@ -64,6 +67,76 @@ static const struct iio_chan_spec inv_icm42600_gyro_channels[] = {
>        INV_ICM42600_TEMP_CHAN(INV_ICM42600_GYRO_SCAN_TEMP),
>  };
>  
> +/* IIO buffer data: 8 bytes */
> +struct inv_icm42600_gyro_buffer {
> +     struct inv_icm42600_fifo_sensor_data gyro;
> +     int8_t temp;
> +     uint8_t padding;
> +};
> +
> +#define INV_ICM42600_SCAN_MASK_GYRO_3AXIS                            \
> +     (BIT(INV_ICM42600_GYRO_SCAN_X) |                                \
> +     BIT(INV_ICM42600_GYRO_SCAN_Y) |                                 \
> +     BIT(INV_ICM42600_GYRO_SCAN_Z))
> +
> +#define INV_ICM42600_SCAN_MASK_TEMP  BIT(INV_ICM42600_GYRO_SCAN_TEMP)
> +
> +static const unsigned long inv_icm42600_gyro_scan_masks[] = {
> +     /* 3-axis gyro + temperature */
> +     INV_ICM42600_SCAN_MASK_GYRO_3AXIS | INV_ICM42600_SCAN_MASK_TEMP,
> +     0,
> +};
> +
> +/* enable gyroscope sensor and FIFO write */
> +static int inv_icm42600_gyro_update_scan_mode(struct iio_dev *indio_dev,
> +                                           const unsigned long *scan_mask)
> +{
> +     struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> +     struct inv_icm42600_sensor_conf conf = INV_ICM42600_SENSOR_CONF_INIT;
> +     unsigned int fifo_en = 0;
> +     unsigned int sleep_gyro = 0;
> +     unsigned int sleep_temp = 0;
> +     unsigned int sleep;
> +     int ret;
> +
> +     mutex_lock(&st->lock);
> +
> +     if (*scan_mask & INV_ICM42600_SCAN_MASK_TEMP) {
> +             /* enable temp sensor */
> +             ret = inv_icm42600_set_temp_conf(st, true, &sleep_temp);
> +             if (ret)
> +                     goto out_unlock;
> +             fifo_en |= INV_ICM42600_SENSOR_TEMP;
> +     }
> +
> +     if (*scan_mask & INV_ICM42600_SCAN_MASK_GYRO_3AXIS) {
> +             /* enable gyro sensor */
> +             conf.mode = INV_ICM42600_SENSOR_MODE_LOW_NOISE;
> +             ret = inv_icm42600_set_gyro_conf(st, &conf, &sleep_gyro);
> +             if (ret)
> +                     goto out_unlock;
> +             fifo_en |= INV_ICM42600_SENSOR_GYRO;
> +     }
> +
> +     /* update data FIFO write */
> +     ret = inv_icm42600_buffer_set_fifo_en(st, fifo_en | st->fifo.en);
> +     if (ret)
> +             goto out_unlock;
> +
> +     ret = inv_icm42600_buffer_update_watermark(st);
> +
> +out_unlock:
> +     mutex_unlock(&st->lock);
> +     /* sleep maximum required time */
> +     if (sleep_gyro > sleep_temp)
> +             sleep = sleep_gyro;
> +     else
> +             sleep = sleep_temp;
> +     if (sleep)
> +             msleep(sleep);
> +     return ret;
> +}
> +
>  static int inv_icm42600_gyro_read_sensor(struct inv_icm42600_state *st,
>                                         struct iio_chan_spec const *chan,
>                                         int16_t *val)
> @@ -260,7 +333,12 @@ static int inv_icm42600_gyro_write_odr(struct inv_icm42600_state *st,
>        mutex_lock(&st->lock);
>  
>        ret = inv_icm42600_set_gyro_conf(st, &conf, NULL);
> +     if (ret)
> +             goto out_unlock;
> +     inv_icm42600_buffer_update_fifo_period(st);
> +     inv_icm42600_buffer_update_watermark(st);
>  
> +out_unlock:
>        mutex_unlock(&st->lock);
>        pm_runtime_mark_last_busy(dev);
>        pm_runtime_put_autosuspend(dev);
> @@ -574,12 +652,51 @@ static int inv_icm42600_gyro_write_raw_get_fmt(struct iio_dev *indio_dev,
>        }
>  }
>  
> +static int inv_icm42600_gyro_hwfifo_set_watermark(struct iio_dev *indio_dev,
> +                                               unsigned int val)
> +{
> +     struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> +     int ret;
> +
> +     mutex_lock(&st->lock);
> +
> +     st->fifo.watermark.gyro = val;
> +     ret = inv_icm42600_buffer_update_watermark(st);
> +
> +     mutex_unlock(&st->lock);
> +
> +     return ret;
> +}
> +
> +static int inv_icm42600_gyro_hwfifo_flush(struct iio_dev *indio_dev,
> +                                       unsigned int count)
> +{

Nothing to do with this patch, but I realised reading this that we have
some 'unusual' use of the word flush here.  It's a straight forward
read function so not sure why we called it flush.

> +     struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> +     int ret;
> +
> +     if (count == 0)
> +             return 0;
> +
> +     mutex_lock(&st->lock);
> +
> +     ret = inv_icm42600_buffer_hwfifo_flush(st, count);
> +     if (!ret)
> +             ret = st->fifo.nb.gyro;
> +
> +     mutex_unlock(&st->lock);
> +
> +     return ret;
> +}
> +
>  static const struct iio_info inv_icm42600_gyro_info = {
>        .read_raw = inv_icm42600_gyro_read_raw,
>        .read_avail = inv_icm42600_gyro_read_avail,
>        .write_raw = inv_icm42600_gyro_write_raw,
>        .write_raw_get_fmt = inv_icm42600_gyro_write_raw_get_fmt,
>        .debugfs_reg_access = inv_icm42600_debugfs_reg,
> +     .update_scan_mode = inv_icm42600_gyro_update_scan_mode,
> +     .hwfifo_set_watermark = inv_icm42600_gyro_hwfifo_set_watermark,
> +     .hwfifo_flush_to_buffer = inv_icm42600_gyro_hwfifo_flush,
>  };
>  
>  int inv_icm42600_gyro_init(struct inv_icm42600_state *st)
> @@ -587,6 +704,7 @@ int inv_icm42600_gyro_init(struct inv_icm42600_state *st)
>        struct device *dev = regmap_get_device(st->map);
>        const char *name;
>        struct iio_dev *indio_dev;
> +     struct iio_buffer *buffer;
>  
>        name = devm_kasprintf(dev, GFP_KERNEL, "%s-gyro", st->name);
>        if (!name)
> @@ -596,14 +714,54 @@ int inv_icm42600_gyro_init(struct inv_icm42600_state *st)
>        if (!indio_dev)
>                return -ENOMEM;
>  
> +     buffer = devm_iio_kfifo_allocate(dev);
> +     if (!buffer)
> +             return -ENOMEM;
> +
>        iio_device_set_drvdata(indio_dev, st);
>        indio_dev->dev.parent = dev;
>        indio_dev->name = name;
>        indio_dev->info = &inv_icm42600_gyro_info;
> -     indio_dev->modes = INDIO_DIRECT_MODE;
> +     indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
>        indio_dev->channels = inv_icm42600_gyro_channels;
>        indio_dev->num_channels = ARRAY_SIZE(inv_icm42600_gyro_channels);
> +     indio_dev->available_scan_masks = inv_icm42600_gyro_scan_masks;
> +     indio_dev->setup_ops = &inv_icm42600_buffer_ops;
> +
> +     iio_device_attach_buffer(indio_dev, buffer);
>  
>        st->indio_gyro = indio_dev;
>        return devm_iio_device_register(dev, st->indio_gyro);
>  }
> +
> +int inv_icm42600_gyro_parse_fifo(struct iio_dev *indio_dev)
> +{
> +     struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> +     ssize_t i, size;
> +     const void *accel, *gyro, *timestamp;
> +     const int8_t *temp;
> +     unsigned int odr;
> +     struct inv_icm42600_gyro_buffer buffer = {
> +             .padding = 0,

Might be worth a comment here or where the structure is defined
on why we make padding explicit.

> +     };
> +
> +     /* parse all fifo packets */
> +     for (i = 0; i < st->fifo.count; i += size) {
> +             size = inv_icm42600_fifo_decode_packet(&st->fifo.data[i],
> +                             &accel, &gyro, &temp, &timestamp, &odr);
> +             /* quit if error or FIFO is empty */
> +             if (size <= 0)
> +                     return size;
> +
> +             /* skip packet if no gyro data or data is invalid */
> +             if (gyro == NULL || !inv_icm42600_fifo_is_data_valid(gyro))
> +                     continue;
> +
> +             /* fill and push data buffer */
> +             memcpy(&buffer.gyro, gyro, sizeof(buffer.gyro));
> +             buffer.temp = temp ? *temp : 0;
> +             iio_push_to_buffers(indio_dev, &buffer);
> +     }
> +
> +     return 0;
> +}

^ permalink raw reply

* Re: [PATCH] i2c: add 'single-master' property to generic bindings
From: Rob Herring @ 2020-06-03 13:58 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: Linux I2C, devicetree, Laine Jaakko EXT
In-Reply-To: <20200529230252.GA4798@kunai>

On Fri, May 29, 2020 at 5:03 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> Hi Rob,
>
> thanks for the review!
>
> > Could you just have different timeouts for clearing stalled bus. You
> > know quickly if 'single-master' is set, but have to wait longer if not?
>
> Timeouts are a difficult topic with I2C; there is no timeout defined.
> However, if you want to start communictaing and don't have a 'bus idle'
> condition, then the new property makes a difference. With
> "single-master", we know the bus is stalled. With "multi-master" it
> could be another master communicating.
>
> > Note that we need to add a bunch of these properties to dt-schema
> > i2c-controller.yaml. I hadn't done that because I want to dual license
> > in the process, but lots of folks have touched i2c.txt IIRC.
>
> What is your motivation for dual licensing?

Non-GPL OS's use DT.

Rob

^ permalink raw reply

* Re: [PATCH 10/10] dt-bindings: mtd: fsl-upm-nand: Deprecate chip-delay and fsl,upm-wait-flags
From: Boris Brezillon @ 2020-06-03 13:52 UTC (permalink / raw)
  To: Anton Vorontsov, Miquel Raynal, linux-mtd
  Cc: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
	linuxppc-dev, Richard Weinberger, Vignesh Raghavendra,
	Tudor Ambarus, Rob Herring, devicetree
In-Reply-To: <20200603134922.1352340-11-boris.brezillon@collabora.com>

And I forgot to Cc the DT maintainer/ML on this one :-/

On Wed,  3 Jun 2020 15:49:22 +0200
Boris Brezillon <boris.brezillon@collabora.com> wrote:

> Those properties are no longer parsed by the driver which is being passed
> those information by the core now. Let's deprecate them.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
>  Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt b/Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt
> index fce4894f5a98..25f07c1f9e44 100644
> --- a/Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt
> @@ -7,14 +7,16 @@ Required properties:
>  - fsl,upm-cmd-offset : UPM pattern offset for the command latch.
>  
>  Optional properties:
> -- fsl,upm-wait-flags : add chip-dependent short delays after running the
> -	UPM pattern (0x1), after writing a data byte (0x2) or after
> -	writing out a buffer (0x4).
>  - fsl,upm-addr-line-cs-offsets : address offsets for multi-chip support.
>  	The corresponding address lines are used to select the chip.
>  - gpios : may specify optional GPIOs connected to the Ready-Not-Busy pins
>  	(R/B#). For multi-chip devices, "n" GPIO definitions are required
>  	according to the number of chips.
> +
> +Deprecated properties:
> +- fsl,upm-wait-flags : add chip-dependent short delays after running the
> +	UPM pattern (0x1), after writing a data byte (0x2) or after
> +	writing out a buffer (0x4).
>  - chip-delay : chip dependent delay for transferring data from array to
>  	read registers (tR). Required if property "gpios" is not used
>  	(R/B# pins not connected).
> @@ -52,8 +54,6 @@ upm@3,0 {
>  	fsl,upm-cmd-offset = <0x08>;
>  	/* Multi-chip NAND device */
>  	fsl,upm-addr-line-cs-offsets = <0x0 0x200>;
> -	fsl,upm-wait-flags = <0x5>;
> -	chip-delay = <25>; // in micro-seconds
>  
>  	nand@0 {
>  		#address-cells = <1>;


^ permalink raw reply

* Re: [PATCH v3 04/20] arm64: dts: arm: vexpress: Move fixed devices out of bus node
From: Rob Herring @ 2020-06-03 13:49 UTC (permalink / raw)
  To: André Przywara
  Cc: Guenter Roeck, Sudeep Holla, Liviu Dudau, Lorenzo Pieralisi,
	Mark Rutland, devicetree,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <1d111f40-1702-7ea0-825f-ab08d77353e9@arm.com>

On Wed, Jun 3, 2020 at 5:21 AM André Przywara <andre.przywara@arm.com> wrote:
>
> On 02/06/2020 00:12, Rob Herring wrote:
>
> Hi,
>
> > On Mon, Jun 1, 2020 at 4:15 AM André Przywara <andre.przywara@arm.com> wrote:
> >>
> >> On 28/05/2020 14:30, André Przywara wrote:
> >>
> >> Hi,
> >>
> >>> On 28/05/2020 03:48, Guenter Roeck wrote:
> >>>
> >>> Hi Guenter,
> >>>
> >>>> On Wed, May 13, 2020 at 11:30:00AM +0100, Andre Przywara wrote:
> >>>>> The devicetree compiler complains when DT nodes without a reg property
> >>>>> live inside a (simple) bus node:
> >>>>> Warning (simple_bus_reg): Node /bus@8000000/motherboard-bus/refclk32khz
> >>>>>                           missing or empty reg/ranges property
> >>>>>
> >>>>> Move the fixed clocks, the fixed regulator, the leds and the config bus
> >>>>> subtree to the root node, since they do not depend on any busses.
> >>>>>
> >>>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> >>>>
> >>>> This patch results in tracebacks when booting the vexpress-a15 machine
> >>>> with vexpress-v2p-ca15-tc1 devicetree file in qemu. Reverting it as well
> >>>> as the subsequent patches affecting the same file (to avoid revert
> >>>> conflicts) fixes the problem.
> >>>
> >>> Many thanks for the heads up! I was able to reproduce it here. On the
> >>> first glance it looks like the UART is probed before the clocks now,
> >>> because the traversal of the changed DT leads to a different probe
> >>> order. I will look into how to fix this.
> >>
> >> Turned out to be a bit more complicated:
> >> The arm,vexpress,config-bus driver walks up the device tree to find a
> >> arm,vexpress,site property [1]. With this patch the first parent node
> >> with that property it finds is now the root node, with the wrong site ID
> >> (0xf instead of 0x0). So it queries the wrong clocks (those IDs are
> >> actually reserved there), and QEMU reports back "0", consequently [2].
> >> Finding a clock frequency in the range of [0, 0] won't get very far.
> >>
> >> Possible solutions are:
> >> 1) Just keep the mcc and its children at where it is in mainline right
> >> now, so *partly* reverting this patch. This has the problem of still
> >> producing a dtc warning, so kind of defeats the purpose of this patch.
> >>
> >> 2) Add a "arm,vexpress,site = <0>;" line to the "mcc" node itself.
> >> Works, but looks somewhat dodgy, as the mcc node should really be a
> >> child of the motherboard node, and we should not hack around this.
> >>
> >> 3) Dig deeper and fix the DT in a way that makes dtc happy. Might
> >> involve (dummy?) ranges or reg properties. My gut feeling is that
> >> arm,vexpress-sysreg,func should really have been "reg" in the first
> >> place, but that's too late to change now, anyway.
> >>
> >> I will post 2) as a fix if 3) turns out to be not feasible.
> >
> > I would just do 1).
> >
> > To some extent, the warnings are for avoiding poor design on new
> > bindings. We need a way to distinguish between existing boards and new
> > ones. Maybe dts needs to learn some warning disable annotations or we
> > need per target warning settings (DTC_FLAGS_foo.dtb ?). Or maybe this
> > check is just too strict.
>
> So I was always wondering about this check, actually. A simple-bus
> describes a bus which is mapped into the CPU address space (in contrast
> to say an I2C bus, for instance). So children of this bus node typically
> have a reg property.
>
> Now also those simple-bus nodes seem to be used to logically group
> hardware in a DT (see this "motherboard" node here). *If* we go with
> this, we should also allow other subnodes, for instance fixed-clocks:
> after all there is probably an actual fixed crystal oscillator on the
> motherboard, so it would also belong in there.

Yes, that's probably right. We'd want things grouped if this was
something applied as an overlay.

> I see that (ab)using simple-bus for *just* grouping nodes is probably
> not a good design, but I don't see why *every* child must be mapped into
> the address space.
>
> Maybe dtc's simple-bus check should indeed be relaxed, to just require
> *at least one* child with a reg or ranges property, but also allow other
> nodes?

It's made you think about the proper location, so it's accomplished
its goal. Maybe this is one that's not without false positives. It
would be good to distinguish between what's for sure a warning and
what's maybe a warning as just blindly fixing the warning is not the
answer.

Rob

^ permalink raw reply

* [PATCH v6 2/2] devicetree: bindings: phy: Document ipq806x dwc3 qcom phy
From: Ansuel Smith @ 2020-06-03 13:22 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Ansuel Smith, Andy Gross, Kishon Vijay Abraham I, Rob Herring,
	Mark Rutland, linux-arm-msm, linux-kernel, devicetree
In-Reply-To: <20200603132237.6036-1-ansuelsmth@gmail.com>

Document dwc3 qcom phy hs and ss phy bindings needed to correctly
inizialize and use usb on ipq806x SoC.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
v6:
* Add maximum value
v5:
* Fix dt_binding_check error
v4:
* Add qcom to specific bindings
v3:
* Use explicit reg instead of regmap

 .../bindings/phy/qcom,ipq806x-usb-phy-hs.yaml | 58 ++++++++++++++
 .../bindings/phy/qcom,ipq806x-usb-phy-ss.yaml | 76 +++++++++++++++++++
 2 files changed, 134 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom,ipq806x-usb-phy-hs.yaml
 create mode 100644 Documentation/devicetree/bindings/phy/qcom,ipq806x-usb-phy-ss.yaml

diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq806x-usb-phy-hs.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq806x-usb-phy-hs.yaml
new file mode 100644
index 000000000000..c019de7478e3
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,ipq806x-usb-phy-hs.yaml
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/qcom,ipq806x-usb-phy-hs.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm ipq806x usb DWC3 HS PHY CONTROLLER
+
+maintainers:
+  - Ansuel Smith <ansuelsmth@gmail.com>
+
+description:
+  DWC3 PHY nodes are defined to describe on-chip Synopsis Physical layer
+  controllers used in ipq806x. Each DWC3 PHY controller should have its
+  own node.
+
+properties:
+  compatible:
+    const: qcom,ipq806x-usb-phy-hs
+
+  "#phy-cells":
+    const: 0
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    minItems: 1
+    maxItems: 2
+
+  clock-names:
+    minItems: 1
+    maxItems: 2
+    description: |
+      - "ref" Is required
+      - "xo"	Optional external reference clock
+    items:
+      - const: ref
+      - const: xo
+
+required:
+  - compatible
+  - "#phy-cells"
+  - reg
+  - clocks
+  - clock-names
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,gcc-ipq806x.h>
+
+    hs_phy_0: phy@110f8800 {
+      compatible = "qcom,ipq806x-usb-phy-hs";
+      reg = <0x110f8800 0x30>;
+      clocks = <&gcc USB30_0_UTMI_CLK>;
+      clock-names = "ref";
+      #phy-cells = <0>;
+    };
diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq806x-usb-phy-ss.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq806x-usb-phy-ss.yaml
new file mode 100644
index 000000000000..3696a8d7a5c7
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,ipq806x-usb-phy-ss.yaml
@@ -0,0 +1,76 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/qcom,ipq806x-usb-phy-ss.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm ipq806x usb DWC3 SS PHY CONTROLLER
+
+maintainers:
+  - Ansuel Smith <ansuelsmth@gmail.com>
+
+description:
+  DWC3 PHY nodes are defined to describe on-chip Synopsis Physical layer
+  controllers used in ipq806x. Each DWC3 PHY controller should have its
+  own node.
+
+properties:
+  compatible:
+    const: qcom,ipq806x-usb-phy-ss
+
+  "#phy-cells":
+    const: 0
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    minItems: 1
+    maxItems: 2
+
+  clock-names:
+    minItems: 1
+    maxItems: 2
+    items:
+      - const: ref
+      - const: xo
+
+  qcom,rx-eq:
+    description: Override value for rx_eq.
+    default: 4
+    maximum: 7
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+
+  qcom,tx-deamp-3_5db:
+    description: Override value for transmit preemphasis.
+    default: 23
+    maximum: 63
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+
+  qcom,mpll:
+    description: Override value for mpll.
+    default: 0
+    maximum: 7
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+
+required:
+  - compatible
+  - "#phy-cells"
+  - reg
+  - clocks
+  - clock-names
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,gcc-ipq806x.h>
+
+    ss_phy_0: phy@110f8830 {
+      compatible = "qcom,ipq806x-usb-phy-ss";
+      reg = <0x110f8830 0x30>;
+      clocks = <&gcc USB30_0_MASTER_CLK>;
+      clock-names = "ref";
+      #phy-cells = <0>;
+    };
-- 
2.25.1


^ permalink raw reply related

* [PATCH v6 1/2] phy: qualcomm: add qcom ipq806x dwc usb phy driver
From: Ansuel Smith @ 2020-06-03 13:22 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Ansuel Smith, Andy Gross, Andy Gross, Kishon Vijay Abraham I,
	Rob Herring, Mark Rutland, linux-arm-msm, linux-kernel,
	devicetree

This has lost in the original push for the dwc3 qcom driver.
This is needed for ipq806x SoC as without this the usb ports
doesn't work at all.

Signed-off-by: Andy Gross <agross@codeaurora.org>
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
v6:
* Use GENMASK instead of hex value
v4:
* Add qcom to specific bindings
v3:
* Use reg instead of regmap phandle
v2:
* Renamed config from PHY_QCOM_DWC3 to PHY_QCOM_IPQ806X_USB
* Rename inline function to generic name to reduce length
* Fix check reported by checkpatch --strict
* Rename compatible to qcom,ipq806x-usb-phy-(hs/ss)

 drivers/phy/qualcomm/Kconfig                |  12 +
 drivers/phy/qualcomm/Makefile               |   1 +
 drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c | 593 ++++++++++++++++++++
 3 files changed, 606 insertions(+)
 create mode 100644 drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c

diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
index e46824da29f6..9d41c3d12800 100644
--- a/drivers/phy/qualcomm/Kconfig
+++ b/drivers/phy/qualcomm/Kconfig
@@ -91,3 +91,15 @@ config PHY_QCOM_USB_HSIC
 	select GENERIC_PHY
 	help
 	  Support for the USB HSIC ULPI compliant PHY on QCOM chipsets.
+
+config PHY_QCOM_IPQ806X_USB
+	tristate "Qualcomm IPQ806x DWC3 USB PHY driver"
+	depends on ARCH_QCOM
+	depends on HAS_IOMEM
+	depends on OF
+	select GENERIC_PHY
+	help
+	  This option enables support for the Synopsis PHYs present inside the
+	  Qualcomm USB3.0 DWC3 controller on ipq806x SoC. This driver supports
+	  both HS and SS PHY controllers.
+
diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
index 283251d6a5d9..8629299c1495 100644
--- a/drivers/phy/qualcomm/Makefile
+++ b/drivers/phy/qualcomm/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_PHY_QCOM_UFS_14NM)		+= phy-qcom-ufs-qmp-14nm.o
 obj-$(CONFIG_PHY_QCOM_UFS_20NM)		+= phy-qcom-ufs-qmp-20nm.o
 obj-$(CONFIG_PHY_QCOM_USB_HS) 		+= phy-qcom-usb-hs.o
 obj-$(CONFIG_PHY_QCOM_USB_HSIC) 	+= phy-qcom-usb-hsic.o
+obj-$(CONFIG_PHY_QCOM_IPQ806X_USB)		+= phy-qcom-ipq806x-usb.o
diff --git a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
new file mode 100644
index 000000000000..f37cd8760118
--- /dev/null
+++ b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
@@ -0,0 +1,593 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2014-2015, Code Aurora Forum. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/delay.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+
+/* USB QSCRATCH Hardware registers */
+#define QSCRATCH_GENERAL_CFG		(0x08)
+#define HSUSB_PHY_CTRL_REG		(0x10)
+
+/* PHY_CTRL_REG */
+#define HSUSB_CTRL_DMSEHV_CLAMP		BIT(24)
+#define HSUSB_CTRL_USB2_SUSPEND		BIT(23)
+#define HSUSB_CTRL_UTMI_CLK_EN		BIT(21)
+#define HSUSB_CTRL_UTMI_OTG_VBUS_VALID	BIT(20)
+#define HSUSB_CTRL_USE_CLKCORE		BIT(18)
+#define HSUSB_CTRL_DPSEHV_CLAMP		BIT(17)
+#define HSUSB_CTRL_COMMONONN		BIT(11)
+#define HSUSB_CTRL_ID_HV_CLAMP		BIT(9)
+#define HSUSB_CTRL_OTGSESSVLD_CLAMP	BIT(8)
+#define HSUSB_CTRL_CLAMP_EN		BIT(7)
+#define HSUSB_CTRL_RETENABLEN		BIT(1)
+#define HSUSB_CTRL_POR			BIT(0)
+
+/* QSCRATCH_GENERAL_CFG */
+#define HSUSB_GCFG_XHCI_REV		BIT(2)
+
+/* USB QSCRATCH Hardware registers */
+#define SSUSB_PHY_CTRL_REG		(0x00)
+#define SSUSB_PHY_PARAM_CTRL_1		(0x04)
+#define SSUSB_PHY_PARAM_CTRL_2		(0x08)
+#define CR_PROTOCOL_DATA_IN_REG		(0x0c)
+#define CR_PROTOCOL_DATA_OUT_REG	(0x10)
+#define CR_PROTOCOL_CAP_ADDR_REG	(0x14)
+#define CR_PROTOCOL_CAP_DATA_REG	(0x18)
+#define CR_PROTOCOL_READ_REG		(0x1c)
+#define CR_PROTOCOL_WRITE_REG		(0x20)
+
+/* PHY_CTRL_REG */
+#define SSUSB_CTRL_REF_USE_PAD		BIT(28)
+#define SSUSB_CTRL_TEST_POWERDOWN	BIT(27)
+#define SSUSB_CTRL_LANE0_PWR_PRESENT	BIT(24)
+#define SSUSB_CTRL_SS_PHY_EN		BIT(8)
+#define SSUSB_CTRL_SS_PHY_RESET		BIT(7)
+
+/* SSPHY control registers - Does this need 0x30? */
+#define SSPHY_CTRL_RX_OVRD_IN_HI(lane)	(0x1006 + 0x100 * (lane))
+#define SSPHY_CTRL_TX_OVRD_DRV_LO(lane)	(0x1002 + 0x100 * (lane))
+
+/* SSPHY SoC version specific values */
+#define SSPHY_RX_EQ_VALUE		4 /* Override value for rx_eq */
+/* Override value for transmit preemphasis */
+#define SSPHY_TX_DEEMPH_3_5DB		23
+/* Override value for mpll */
+#define SSPHY_MPLL_VALUE		0
+
+/* QSCRATCH PHY_PARAM_CTRL1 fields */
+#define PHY_PARAM_CTRL1_TX_FULL_SWING_MASK	GENMASK(26, 19)
+#define PHY_PARAM_CTRL1_TX_DEEMPH_6DB_MASK	GENMASK(19, 13)
+#define PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB_MASK	GENMASK(13, 7)
+#define PHY_PARAM_CTRL1_LOS_BIAS_MASK		GENMASK(7, 2)
+
+#define PHY_PARAM_CTRL1_MASK				\
+		(PHY_PARAM_CTRL1_TX_FULL_SWING_MASK |	\
+		 PHY_PARAM_CTRL1_TX_DEEMPH_6DB_MASK |	\
+		 PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB_MASK |	\
+		 PHY_PARAM_CTRL1_LOS_BIAS_MASK)
+
+#define PHY_PARAM_CTRL1_TX_FULL_SWING(x)	\
+		(((x) << 20) & PHY_PARAM_CTRL1_TX_FULL_SWING_MASK)
+#define PHY_PARAM_CTRL1_TX_DEEMPH_6DB(x)	\
+		(((x) << 14) & PHY_PARAM_CTRL1_TX_DEEMPH_6DB_MASK)
+#define PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB(x)	\
+		(((x) <<  8) & PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB_MASK)
+#define PHY_PARAM_CTRL1_LOS_BIAS(x)	\
+		(((x) <<  3) & PHY_PARAM_CTRL1_LOS_BIAS_MASK)
+
+/* RX OVRD IN HI bits */
+#define RX_OVRD_IN_HI_RX_RESET_OVRD		BIT(13)
+#define RX_OVRD_IN_HI_RX_RX_RESET		BIT(12)
+#define RX_OVRD_IN_HI_RX_EQ_OVRD		BIT(11)
+#define RX_OVRD_IN_HI_RX_EQ_MASK		GENMASK(10, 7)
+#define RX_OVRD_IN_HI_RX_EQ(x)			((x) << 8)
+#define RX_OVRD_IN_HI_RX_EQ_EN_OVRD		BIT(7)
+#define RX_OVRD_IN_HI_RX_EQ_EN			BIT(6)
+#define RX_OVRD_IN_HI_RX_LOS_FILTER_OVRD	BIT(5)
+#define RX_OVRD_IN_HI_RX_LOS_FILTER_MASK	GENMASK(4, 2)
+#define RX_OVRD_IN_HI_RX_RATE_OVRD		BIT(2)
+#define RX_OVRD_IN_HI_RX_RATE_MASK		GENMASK(2, 0)
+
+/* TX OVRD DRV LO register bits */
+#define TX_OVRD_DRV_LO_AMPLITUDE_MASK		GENMASK(6, 0)
+#define TX_OVRD_DRV_LO_PREEMPH_MASK		GENMASK(13, 6)
+#define TX_OVRD_DRV_LO_PREEMPH(x)		((x) << 7)
+#define TX_OVRD_DRV_LO_EN			BIT(14)
+
+/* MPLL bits */
+#define SSPHY_MPLL_MASK				GENMASK(8, 5)
+#define SSPHY_MPLL(x)				((x) << 5)
+
+/* SS CAP register bits */
+#define SS_CR_CAP_ADDR_REG			BIT(0)
+#define SS_CR_CAP_DATA_REG			BIT(0)
+#define SS_CR_READ_REG				BIT(0)
+#define SS_CR_WRITE_REG				BIT(0)
+
+struct usb_phy {
+	void __iomem		*base;
+	struct device		*dev;
+	struct clk		*xo_clk;
+	struct clk		*ref_clk;
+	u32			rx_eq;
+	u32			tx_deamp_3_5db;
+	u32			mpll;
+};
+
+struct phy_drvdata {
+	struct phy_ops	ops;
+	u32		clk_rate;
+};
+
+/**
+ * Write register and read back masked value to confirm it is written
+ *
+ * @base - QCOM DWC3 PHY base virtual address.
+ * @offset - register offset.
+ * @mask - register bitmask specifying what should be updated
+ * @val - value to write.
+ */
+static inline void usb_phy_write_readback(struct usb_phy *phy_dwc3,
+					  u32 offset,
+					  const u32 mask, u32 val)
+{
+	u32 write_val, tmp = readl(phy_dwc3->base + offset);
+
+	tmp &= ~mask;		/* retain other bits */
+	write_val = tmp | val;
+
+	writel(write_val, phy_dwc3->base + offset);
+
+	/* Read back to see if val was written */
+	tmp = readl(phy_dwc3->base + offset);
+	tmp &= mask;		/* clear other bits */
+
+	if (tmp != val)
+		dev_err(phy_dwc3->dev, "write: %x to QSCRATCH: %x FAILED\n",
+			val, offset);
+}
+
+static int wait_for_latch(void __iomem *addr)
+{
+	u32 retry = 10;
+
+	while (true) {
+		if (!readl(addr))
+			break;
+
+		if (--retry == 0)
+			return -ETIMEDOUT;
+
+		usleep_range(10, 20);
+	}
+
+	return 0;
+}
+
+/**
+ * Write SSPHY register
+ *
+ * @base - QCOM DWC3 PHY base virtual address.
+ * @addr - SSPHY address to write.
+ * @val - value to write.
+ */
+static int usb_ss_write_phycreg(struct usb_phy *phy_dwc3,
+				u32 addr, u32 val)
+{
+	int ret;
+
+	writel(addr, phy_dwc3->base + CR_PROTOCOL_DATA_IN_REG);
+	writel(SS_CR_CAP_ADDR_REG,
+	       phy_dwc3->base + CR_PROTOCOL_CAP_ADDR_REG);
+
+	ret = wait_for_latch(phy_dwc3->base + CR_PROTOCOL_CAP_ADDR_REG);
+	if (ret)
+		goto err_wait;
+
+	writel(val, phy_dwc3->base + CR_PROTOCOL_DATA_IN_REG);
+	writel(SS_CR_CAP_DATA_REG,
+	       phy_dwc3->base + CR_PROTOCOL_CAP_DATA_REG);
+
+	ret = wait_for_latch(phy_dwc3->base + CR_PROTOCOL_CAP_DATA_REG);
+	if (ret)
+		goto err_wait;
+
+	writel(SS_CR_WRITE_REG, phy_dwc3->base + CR_PROTOCOL_WRITE_REG);
+
+	ret = wait_for_latch(phy_dwc3->base + CR_PROTOCOL_WRITE_REG);
+
+err_wait:
+	if (ret)
+		dev_err(phy_dwc3->dev, "timeout waiting for latch\n");
+	return ret;
+}
+
+/**
+ * Read SSPHY register.
+ *
+ * @base - QCOM DWC3 PHY base virtual address.
+ * @addr - SSPHY address to read.
+ */
+static int usb_ss_read_phycreg(struct usb_phy *phy_dwc3,
+			       u32 addr, u32 *val)
+{
+	int ret;
+
+	writel(addr, phy_dwc3->base + CR_PROTOCOL_DATA_IN_REG);
+	writel(SS_CR_CAP_ADDR_REG,
+	       phy_dwc3->base + CR_PROTOCOL_CAP_ADDR_REG);
+
+	ret = wait_for_latch(phy_dwc3->base + CR_PROTOCOL_CAP_ADDR_REG);
+	if (ret)
+		goto err_wait;
+
+	/*
+	 * Due to hardware bug, first read of SSPHY register might be
+	 * incorrect. Hence as workaround, SW should perform SSPHY register
+	 * read twice, but use only second read and ignore first read.
+	 */
+	writel(SS_CR_READ_REG, phy_dwc3->base + CR_PROTOCOL_READ_REG);
+
+	ret = wait_for_latch(phy_dwc3->base + CR_PROTOCOL_READ_REG);
+	if (ret)
+		goto err_wait;
+
+	/* throwaway read */
+	readl(phy_dwc3->base + CR_PROTOCOL_DATA_OUT_REG);
+
+	writel(SS_CR_READ_REG, phy_dwc3->base + CR_PROTOCOL_READ_REG);
+
+	ret = wait_for_latch(phy_dwc3->base + CR_PROTOCOL_READ_REG);
+	if (ret)
+		goto err_wait;
+
+	*val = readl(phy_dwc3->base + CR_PROTOCOL_DATA_OUT_REG);
+
+err_wait:
+	return ret;
+}
+
+static int qcom_ipq806x_usb_hs_phy_init(struct phy *phy)
+{
+	struct usb_phy *phy_dwc3 = phy_get_drvdata(phy);
+	int ret;
+	u32 val;
+
+	ret = clk_prepare_enable(phy_dwc3->xo_clk);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(phy_dwc3->ref_clk);
+	if (ret) {
+		clk_disable_unprepare(phy_dwc3->xo_clk);
+		return ret;
+	}
+
+	/*
+	 * HSPHY Initialization: Enable UTMI clock, select 19.2MHz fsel
+	 * enable clamping, and disable RETENTION (power-on default is ENABLED)
+	 */
+	val = HSUSB_CTRL_DPSEHV_CLAMP | HSUSB_CTRL_DMSEHV_CLAMP |
+		HSUSB_CTRL_RETENABLEN  | HSUSB_CTRL_COMMONONN |
+		HSUSB_CTRL_OTGSESSVLD_CLAMP | HSUSB_CTRL_ID_HV_CLAMP |
+		HSUSB_CTRL_DPSEHV_CLAMP | HSUSB_CTRL_UTMI_OTG_VBUS_VALID |
+		HSUSB_CTRL_UTMI_CLK_EN | HSUSB_CTRL_CLAMP_EN | 0x70;
+
+	/* use core clock if external reference is not present */
+	if (!phy_dwc3->xo_clk)
+		val |= HSUSB_CTRL_USE_CLKCORE;
+
+	writel(val, phy_dwc3->base + HSUSB_PHY_CTRL_REG);
+	usleep_range(2000, 2200);
+
+	/* Disable (bypass) VBUS and ID filters */
+	writel(HSUSB_GCFG_XHCI_REV, phy_dwc3->base + QSCRATCH_GENERAL_CFG);
+
+	return 0;
+}
+
+static int qcom_ipq806x_usb_hs_phy_exit(struct phy *phy)
+{
+	struct usb_phy *phy_dwc3 = phy_get_drvdata(phy);
+
+	clk_disable_unprepare(phy_dwc3->ref_clk);
+	clk_disable_unprepare(phy_dwc3->xo_clk);
+
+	return 0;
+}
+
+static int qcom_ipq806x_usb_ss_phy_init(struct phy *phy)
+{
+	struct usb_phy *phy_dwc3 = phy_get_drvdata(phy);
+	int ret;
+	u32 data = 0;
+
+	ret = clk_prepare_enable(phy_dwc3->xo_clk);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(phy_dwc3->ref_clk);
+	if (ret) {
+		clk_disable_unprepare(phy_dwc3->xo_clk);
+		return ret;
+	}
+
+	/* reset phy */
+	data = readl(phy_dwc3->base + SSUSB_PHY_CTRL_REG);
+	writel(data | SSUSB_CTRL_SS_PHY_RESET,
+	       phy_dwc3->base + SSUSB_PHY_CTRL_REG);
+	usleep_range(2000, 2200);
+	writel(data, phy_dwc3->base + SSUSB_PHY_CTRL_REG);
+
+	/* clear REF_PAD if we don't have XO clk */
+	if (!phy_dwc3->xo_clk)
+		data &= ~SSUSB_CTRL_REF_USE_PAD;
+	else
+		data |= SSUSB_CTRL_REF_USE_PAD;
+
+	writel(data, phy_dwc3->base + SSUSB_PHY_CTRL_REG);
+
+	/* wait for ref clk to become stable, this can take up to 30ms */
+	msleep(30);
+
+	data |= SSUSB_CTRL_SS_PHY_EN | SSUSB_CTRL_LANE0_PWR_PRESENT;
+	writel(data, phy_dwc3->base + SSUSB_PHY_CTRL_REG);
+
+	/*
+	 * WORKAROUND: There is SSPHY suspend bug due to which USB enumerates
+	 * in HS mode instead of SS mode. Workaround it by asserting
+	 * LANE0.TX_ALT_BLOCK.EN_ALT_BUS to enable TX to use alt bus mode
+	 */
+	ret = usb_ss_read_phycreg(phy_dwc3, 0x102D, &data);
+	if (ret)
+		goto err_phy_trans;
+
+	data |= (1 << 7);
+	ret = usb_ss_write_phycreg(phy_dwc3, 0x102D, data);
+	if (ret)
+		goto err_phy_trans;
+
+	ret = usb_ss_read_phycreg(phy_dwc3, 0x1010, &data);
+	if (ret)
+		goto err_phy_trans;
+
+	data &= ~0xff0;
+	data |= 0x20;
+	ret = usb_ss_write_phycreg(phy_dwc3, 0x1010, data);
+	if (ret)
+		goto err_phy_trans;
+
+	/*
+	 * Fix RX Equalization setting as follows
+	 * LANE0.RX_OVRD_IN_HI. RX_EQ_EN set to 0
+	 * LANE0.RX_OVRD_IN_HI.RX_EQ_EN_OVRD set to 1
+	 * LANE0.RX_OVRD_IN_HI.RX_EQ set based on SoC version
+	 * LANE0.RX_OVRD_IN_HI.RX_EQ_OVRD set to 1
+	 */
+	ret = usb_ss_read_phycreg(phy_dwc3,
+				  SSPHY_CTRL_RX_OVRD_IN_HI(0), &data);
+	if (ret)
+		goto err_phy_trans;
+
+	data &= ~RX_OVRD_IN_HI_RX_EQ_EN;
+	data |= RX_OVRD_IN_HI_RX_EQ_EN_OVRD;
+	data &= ~RX_OVRD_IN_HI_RX_EQ_MASK;
+	data |= RX_OVRD_IN_HI_RX_EQ(phy_dwc3->rx_eq);
+	data |= RX_OVRD_IN_HI_RX_EQ_OVRD;
+	ret = usb_ss_write_phycreg(phy_dwc3,
+				   SSPHY_CTRL_RX_OVRD_IN_HI(0), data);
+	if (ret)
+		goto err_phy_trans;
+
+	/*
+	 * Set EQ and TX launch amplitudes as follows
+	 * LANE0.TX_OVRD_DRV_LO.PREEMPH set based on SoC version
+	 * LANE0.TX_OVRD_DRV_LO.AMPLITUDE set to 110
+	 * LANE0.TX_OVRD_DRV_LO.EN set to 1.
+	 */
+	ret = usb_ss_read_phycreg(phy_dwc3,
+				  SSPHY_CTRL_TX_OVRD_DRV_LO(0), &data);
+	if (ret)
+		goto err_phy_trans;
+
+	data &= ~TX_OVRD_DRV_LO_PREEMPH_MASK;
+	data |= TX_OVRD_DRV_LO_PREEMPH(phy_dwc3->tx_deamp_3_5db);
+	data &= ~TX_OVRD_DRV_LO_AMPLITUDE_MASK;
+	data |= 0x6E;
+	data |= TX_OVRD_DRV_LO_EN;
+	ret = usb_ss_write_phycreg(phy_dwc3,
+				   SSPHY_CTRL_TX_OVRD_DRV_LO(0), data);
+	if (ret)
+		goto err_phy_trans;
+
+	data = 0;
+	data &= ~SSPHY_MPLL_MASK;
+	data |= SSPHY_MPLL(phy_dwc3->mpll);
+	usb_ss_write_phycreg(phy_dwc3, 0x30, data);
+
+	/*
+	 * Set the QSCRATCH PHY_PARAM_CTRL1 parameters as follows
+	 * TX_FULL_SWING [26:20] amplitude to 110
+	 * TX_DEEMPH_6DB [19:14] to 32
+	 * TX_DEEMPH_3_5DB [13:8] set based on SoC version
+	 * LOS_BIAS [7:3] to 9
+	 */
+	data = readl(phy_dwc3->base + SSUSB_PHY_PARAM_CTRL_1);
+
+	data &= ~PHY_PARAM_CTRL1_MASK;
+
+	data |= PHY_PARAM_CTRL1_TX_FULL_SWING(0x6e) |
+		PHY_PARAM_CTRL1_TX_DEEMPH_6DB(0x20) |
+		PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB(phy_dwc3->tx_deamp_3_5db) |
+		PHY_PARAM_CTRL1_LOS_BIAS(0x9);
+
+	usb_phy_write_readback(phy_dwc3, SSUSB_PHY_PARAM_CTRL_1,
+			       PHY_PARAM_CTRL1_MASK, data);
+
+err_phy_trans:
+	return ret;
+}
+
+static int qcom_ipq806x_usb_ss_phy_exit(struct phy *phy)
+{
+	struct usb_phy *phy_dwc3 = phy_get_drvdata(phy);
+
+	/* Sequence to put SSPHY in low power state:
+	 * 1. Clear REF_PHY_EN in PHY_CTRL_REG
+	 * 2. Clear REF_USE_PAD in PHY_CTRL_REG
+	 * 3. Set TEST_POWERED_DOWN in PHY_CTRL_REG to enable PHY retention
+	 */
+	usb_phy_write_readback(phy_dwc3, SSUSB_PHY_CTRL_REG,
+			       SSUSB_CTRL_SS_PHY_EN, 0x0);
+	usb_phy_write_readback(phy_dwc3, SSUSB_PHY_CTRL_REG,
+			       SSUSB_CTRL_REF_USE_PAD, 0x0);
+	usb_phy_write_readback(phy_dwc3, SSUSB_PHY_CTRL_REG,
+			       SSUSB_CTRL_TEST_POWERDOWN, 0x0);
+
+	clk_disable_unprepare(phy_dwc3->ref_clk);
+	clk_disable_unprepare(phy_dwc3->xo_clk);
+
+	return 0;
+}
+
+static const struct phy_drvdata qcom_ipq806x_usb_hs_drvdata = {
+	.ops = {
+		.init		= qcom_ipq806x_usb_hs_phy_init,
+		.exit		= qcom_ipq806x_usb_hs_phy_exit,
+		.owner		= THIS_MODULE,
+	},
+	.clk_rate = 60000000,
+};
+
+static const struct phy_drvdata qcom_ipq806x_usb_ss_drvdata = {
+	.ops = {
+		.init		= qcom_ipq806x_usb_ss_phy_init,
+		.exit		= qcom_ipq806x_usb_ss_phy_exit,
+		.owner		= THIS_MODULE,
+	},
+	.clk_rate = 125000000,
+};
+
+static const struct of_device_id qcom_ipq806x_usb_phy_table[] = {
+	{ .compatible = "qcom,ipq806x-usb-phy-hs",
+	  .data = &qcom_ipq806x_usb_hs_drvdata },
+	{ .compatible = "qcom,ipq806x-usb-phy-ss",
+	  .data = &qcom_ipq806x_usb_ss_drvdata },
+	{ /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, qcom_ipq806x_usb_phy_table);
+
+static int qcom_ipq806x_usb_phy_probe(struct platform_device *pdev)
+{
+	struct usb_phy	*phy_dwc3;
+	struct phy_provider		*phy_provider;
+	struct phy			*generic_phy;
+	const struct of_device_id *match;
+	const struct phy_drvdata *data;
+	struct resource			*res;
+	resource_size_t			size;
+	struct device_node *np;
+
+	phy_dwc3 = devm_kzalloc(&pdev->dev, sizeof(*phy_dwc3), GFP_KERNEL);
+	if (!phy_dwc3)
+		return -ENOMEM;
+
+	match = of_match_node(qcom_ipq806x_usb_phy_table, pdev->dev.of_node);
+	data = match->data;
+
+	phy_dwc3->dev = &pdev->dev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -EINVAL;
+	size = resource_size(res);
+	phy_dwc3->base = devm_ioremap(phy_dwc3->dev, res->start, size);
+
+	if (IS_ERR(phy_dwc3->base)) {
+		dev_err(phy_dwc3->dev, "failed to map reg\n");
+		return PTR_ERR(phy_dwc3->base);
+	}
+
+	phy_dwc3->ref_clk = devm_clk_get(phy_dwc3->dev, "ref");
+	if (IS_ERR(phy_dwc3->ref_clk)) {
+		dev_dbg(phy_dwc3->dev, "cannot get reference clock\n");
+		return PTR_ERR(phy_dwc3->ref_clk);
+	}
+
+	clk_set_rate(phy_dwc3->ref_clk, data->clk_rate);
+
+	phy_dwc3->xo_clk = devm_clk_get(phy_dwc3->dev, "xo");
+	if (IS_ERR(phy_dwc3->xo_clk)) {
+		dev_dbg(phy_dwc3->dev, "cannot get TCXO clock\n");
+		phy_dwc3->xo_clk = NULL;
+	}
+
+	/* Parse device node to probe HSIO settings */
+	np = of_node_get(pdev->dev.of_node);
+	if (!of_compat_cmp(match->compatible, "qcom,ipq806x-usb-phy-ss",
+			   strlen(match->compatible))) {
+		if (of_property_read_u32(np, "qcom,rx-eq", &phy_dwc3->rx_eq) ||
+		    of_property_read_u32(np, "qcom,tx-deamp_3_5db",
+					 &phy_dwc3->tx_deamp_3_5db) ||
+		    of_property_read_u32(np, "qcom,mpll", &phy_dwc3->mpll)) {
+			dev_err(phy_dwc3->dev, "cannot get HSIO settings from device node, using default values\n");
+
+			/* Default HSIO settings */
+			phy_dwc3->rx_eq = SSPHY_RX_EQ_VALUE;
+			phy_dwc3->tx_deamp_3_5db = SSPHY_TX_DEEMPH_3_5DB;
+			phy_dwc3->mpll = SSPHY_MPLL_VALUE;
+		}
+	}
+
+	generic_phy = devm_phy_create(phy_dwc3->dev, pdev->dev.of_node,
+				      &data->ops);
+
+	if (IS_ERR(generic_phy))
+		return PTR_ERR(generic_phy);
+
+	phy_set_drvdata(generic_phy, phy_dwc3);
+	platform_set_drvdata(pdev, phy_dwc3);
+
+	phy_provider = devm_of_phy_provider_register(phy_dwc3->dev,
+						     of_phy_simple_xlate);
+
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	return 0;
+}
+
+static struct platform_driver qcom_ipq806x_usb_phy_driver = {
+	.probe		= qcom_ipq806x_usb_phy_probe,
+	.driver		= {
+		.name	= "qcom-ipq806x-usb-phy",
+		.owner	= THIS_MODULE,
+		.of_match_table = qcom_ipq806x_usb_phy_table,
+	},
+};
+
+module_platform_driver(qcom_ipq806x_usb_phy_driver);
+
+MODULE_ALIAS("platform:phy-qcom-ipq806x-usb");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Andy Gross <agross@codeaurora.org>");
+MODULE_AUTHOR("Ivan T. Ivanov <iivanov@mm-sol.com>");
+MODULE_DESCRIPTION("DesignWare USB3 QCOM PHY driver");
-- 
2.25.1


^ permalink raw reply related

* Re: [PATCH v8 0/5] support reserving crashkernel above 4G on arm64 kdump
From: chenzhou @ 2020-06-03 13:20 UTC (permalink / raw)
  To: Prabhakar Kushwaha, John Donnelly
  Cc: Bhupesh Sharma, Simon Horman, Devicetree List, Baoquan He,
	Will Deacon, Linux Doc Mailing List, Catalin Marinas,
	kexec mailing list, Linux Kernel Mailing List, Rob Herring,
	Ingo Molnar, Arnd Bergmann, guohanjun, James Morse,
	Thomas Gleixner, Prabhakar Kushwaha, RuiRui Yang,
	linux-arm-kernel
In-Reply-To: <CAJ2QiJJhUCnobrMHui5=6zLzgy3KsoPxrqiH_oYT8Jhb5MkmbA@mail.gmail.com>

Hi,


On 2020/6/3 19:47, Prabhakar Kushwaha wrote:
> Hi Chen,
>
> On Tue, Jun 2, 2020 at 8:12 PM John Donnelly <john.p.donnelly@oracle.com> wrote:
>>
>>
>>> On Jun 2, 2020, at 12:38 AM, Prabhakar Kushwaha <prabhakar.pkin@gmail.com> wrote:
>>>
>>> On Tue, Jun 2, 2020 at 3:29 AM John Donnelly <john.p.donnelly@oracle.com> wrote:
>>>> Hi .  See below !
>>>>
>>>>> On Jun 1, 2020, at 4:02 PM, Bhupesh Sharma <bhsharma@redhat.com> wrote:
>>>>>
>>>>> Hi John,
>>>>>
>>>>> On Tue, Jun 2, 2020 at 1:01 AM John Donnelly <John.P.donnelly@oracle.com> wrote:
>>>>>> Hi,
>>>>>>
>>>>>>
>>>>>> On 6/1/20 7:02 AM, Prabhakar Kushwaha wrote:
>>>>>>> Hi Chen,
>>>>>>>
>>>>>>> On Thu, May 21, 2020 at 3:05 PM Chen Zhou <chenzhou10@huawei.com> wrote:
>>>>>>>> This patch series enable reserving crashkernel above 4G in arm64.
>>>>>>>>
>>>>>>>> There are following issues in arm64 kdump:
>>>>>>>> 1. We use crashkernel=X to reserve crashkernel below 4G, which will fail
>>>>>>>> when there is no enough low memory.
>>>>>>>> 2. Currently, crashkernel=Y@X can be used to reserve crashkernel above 4G,
>>>>>>>> in this case, if swiotlb or DMA buffers are required, crash dump kernel
>>>>>>>> will boot failure because there is no low memory available for allocation.
>>>>>>>>
>>>>>>> We are getting "warn_alloc" [1] warning during boot of kdump kernel
>>>>>>> with bootargs as [2] of primary kernel.
>>>>>>> This error observed on ThunderX2  ARM64 platform.
>>>>>>>
>>>>>>> It is observed with latest upstream tag (v5.7-rc3) with this patch set
>>>>>>> and https://urldefense.com/v3/__https://lists.infradead.org/pipermail/kexec/2020-May/025128.html__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbiIAAlzu$
>>>>>>> Also **without** this patch-set
>>>>>>> "https://urldefense.com/v3/__https://www.spinics.net/lists/arm-kernel/msg806882.html__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbjC6ujMA$"
>>>>>>>
>>>>>>> This issue comes whenever crashkernel memory is reserved after 0xc000_0000.
>>>>>>> More details discussed earlier in
>>>>>>> https://urldefense.com/v3/__https://www.spinics.net/lists/arm-kernel/msg806882.html__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbjC6ujMA$  without any
>>>>>>> solution
>>>>>>>
>>>>>>> This patch-set is expected to solve similar kind of issue.
>>>>>>> i.e. low memory is only targeted for DMA, swiotlb; So above mentioned
>>>>>>> observation should be considered/fixed. .
>>>>>>>
>>>>>>> --pk
>>>>>>>
>>>>>>> [1]
>>>>>>> [   30.366695] DMI: Cavium Inc. Saber/Saber, BIOS
>>>>>>> TX2-FW-Release-3.1-build_01-2803-g74253a541a mm/dd/yyyy
>>>>>>> [   30.367696] NET: Registered protocol family 16
>>>>>>> [   30.369973] swapper/0: page allocation failure: order:6,
>>>>>>> mode:0x1(GFP_DMA), nodemask=(null),cpuset=/,mems_allowed=0
>>>>>>> [   30.369980] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.7.0-rc3+ #121
>>>>>>> [   30.369981] Hardware name: Cavium Inc. Saber/Saber, BIOS
>>>>>>> TX2-FW-Release-3.1-build_01-2803-g74253a541a mm/dd/yyyy
>>>>>>> [   30.369984] Call trace:
>>>>>>> [   30.369989]  dump_backtrace+0x0/0x1f8
>>>>>>> [   30.369991]  show_stack+0x20/0x30
>>>>>>> [   30.369997]  dump_stack+0xc0/0x10c
>>>>>>> [   30.370001]  warn_alloc+0x10c/0x178
>>>>>>> [   30.370004]  __alloc_pages_slowpath.constprop.111+0xb10/0xb50
>>>>>>> [   30.370006]  __alloc_pages_nodemask+0x2b4/0x300
>>>>>>> [   30.370008]  alloc_page_interleave+0x24/0x98
>>>>>>> [   30.370011]  alloc_pages_current+0xe4/0x108
>>>>>>> [   30.370017]  dma_atomic_pool_init+0x44/0x1a4
>>>>>>> [   30.370020]  do_one_initcall+0x54/0x228
>>>>>>> [   30.370027]  kernel_init_freeable+0x228/0x2cc
>>>>>>> [   30.370031]  kernel_init+0x1c/0x110
>>>>>>> [   30.370034]  ret_from_fork+0x10/0x18
>>>>>>> [   30.370036] Mem-Info:
>>>>>>> [   30.370064] active_anon:0 inactive_anon:0 isolated_anon:0
>>>>>>> [   30.370064]  active_file:0 inactive_file:0 isolated_file:0
>>>>>>> [   30.370064]  unevictable:0 dirty:0 writeback:0 unstable:0
>>>>>>> [   30.370064]  slab_reclaimable:34 slab_unreclaimable:4438
>>>>>>> [   30.370064]  mapped:0 shmem:0 pagetables:14 bounce:0
>>>>>>> [   30.370064]  free:1537719 free_pcp:219 free_cma:0
>>>>>>> [   30.370070] Node 0 active_anon:0kB inactive_anon:0kB
>>>>>>> active_file:0kB inactive_file:0kB unevictable:0kB isolated(anon):0kB
>>>>>>> isolated(file):0kB mapped:0kB dirty:0kB writeback:0kB shmem:0kB
>>>>>>> shmem_thp: 0kB shmem_pmdmapped: 0kB anon_thp: 0kB writeback_tmp:0kB
>>>>>>> unstable:0kB all_unreclaimable? no
>>>>>>> [   30.370073] Node 1 active_anon:0kB inactive_anon:0kB
>>>>>>> active_file:0kB inactive_file:0kB unevictable:0kB isolated(anon):0kB
>>>>>>> isolated(file):0kB mapped:0kB dirty:0kB writeback:0kB shmem:0kB
>>>>>>> shmem_thp: 0kB shmem_pmdmapped: 0kB anon_thp: 0kB writeback_tmp:0kB
>>>>>>> unstable:0kB all_unreclaimable? no
>>>>>>> [   30.370079] Node 0 DMA free:0kB min:0kB low:0kB high:0kB
>>>>>>> reserved_highatomic:0KB active_anon:0kB inactive_anon:0kB
>>>>>>> active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB
>>>>>>> present:128kB managed:0kB mlocked:0kB kernel_stack:0kB pagetables:0kB
>>>>>>> bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB
>>>>>>> [   30.370084] lowmem_reserve[]: 0 250 6063 6063
>>>>>>> [   30.370090] Node 0 DMA32 free:256000kB min:408kB low:664kB
>>>>>>> high:920kB reserved_highatomic:0KB active_anon:0kB inactive_anon:0kB
>>>>>>> active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB
>>>>>>> present:269700kB managed:256000kB mlocked:0kB kernel_stack:0kB
>>>>>>> pagetables:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB
>>>>>>> [   30.370094] lowmem_reserve[]: 0 0 5813 5813
>>>>>>> [   30.370100] Node 0 Normal free:5894876kB min:9552kB low:15504kB
>>>>>>> high:21456kB reserved_highatomic:0KB active_anon:0kB inactive_anon:0kB
>>>>>>> active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB
>>>>>>> present:8388608kB managed:5953112kB mlocked:0kB kernel_stack:21672kB
>>>>>>> pagetables:56kB bounce:0kB free_pcp:876kB local_pcp:176kB free_cma:0kB
>>>>>>> [   30.370104] lowmem_reserve[]: 0 0 0 0
>>>>>>> [   30.370107] Node 0 DMA: 0*4kB 0*8kB 0*16kB 0*32kB 0*64kB 0*128kB
>>>>>>> 0*256kB 0*512kB 0*1024kB 0*2048kB 0*4096kB = 0kB
>>>>>>> [   30.370113] Node 0 DMA32: 0*4kB 0*8kB 0*16kB 0*32kB 0*64kB 0*128kB
>>>>>>> 0*256kB 0*512kB 0*1024kB 1*2048kB (M) 62*4096kB (M) = 256000kB
>>>>>>> [   30.370119] Node 0 Normal: 2*4kB (M) 3*8kB (ME) 2*16kB (UE) 3*32kB
>>>>>>> (UM) 1*64kB (U) 2*128kB (M) 2*256kB (ME) 3*512kB (ME) 3*1024kB (ME)
>>>>>>> 3*2048kB (UME) 1436*4096kB (M) = 5893600kB
>>>>>>> [   30.370129] Node 0 hugepages_total=0 hugepages_free=0
>>>>>>> hugepages_surp=0 hugepages_size=1048576kB
>>>>>>> [   30.370130] 0 total pagecache pages
>>>>>>> [   30.370132] 0 pages in swap cache
>>>>>>> [   30.370134] Swap cache stats: add 0, delete 0, find 0/0
>>>>>>> [   30.370135] Free swap  = 0kB
>>>>>>> [   30.370136] Total swap = 0kB
>>>>>>> [   30.370137] 2164609 pages RAM
>>>>>>> [   30.370139] 0 pages HighMem/MovableOnly
>>>>>>> [   30.370140] 612331 pages reserved
>>>>>>> [   30.370141] 0 pages hwpoisoned
>>>>>>> [   30.370143] DMA: failed to allocate 256 KiB pool for atomic
>>>>>>> coherent allocation
>>>>>>
>>>>>> During my testing I saw the same error and Chen's  solution corrected it .
>>>>> Which combination you are using on your side? I am using Prabhakar's
>>>>> suggested environment and can reproduce the issue
>>>>> with or without Chen's crashkernel support above 4G patchset.
>>>>>
>>>>> I am also using a ThunderX2 platform with latest makedumpfile code and
>>>>> kexec-tools (with the suggested patch
>>>>> <https://urldefense.com/v3/__https://lists.infradead.org/pipermail/kexec/2020-May/025128.html__;!!GqivPVa7Brio!J6lUig58-Gw6TKZnEEYzEeSU36T-1SqlB1kImU00xtX_lss5Tx-JbUmLE9TJC3foXBLg$ >).
>>>>>
>>>>> Thanks,
>>>>> Bhupesh
>>>>
>>>> I did this activity 5 months ago and I have moved on to other activities. My DMA failures were related to PCI devices that could not be enumerated because  low-DMA space was not  available when crashkernel was moved above 4G; I don’t recall the exact platform.
>>>>
>>>>
>>>>
>>>> For this failure ,
>>>>
>>>>>>> DMA: failed to allocate 256 KiB pool for atomic
>>>>>>> coherent allocation
>>>>
>>>> Is due to :
>>>>
>>>>
>>>> 3618082c
>>>> ("arm64 use both ZONE_DMA and ZONE_DMA32")
>>>>
>>>> With the introduction of ZONE_DMA to support the Raspberry DMA
>>>> region below 1G, the crashkernel is placed in the upper 4G
>>>> ZONE_DMA_32 region. Since the crashkernel does not have access
>>>> to the ZONE_DMA region, it prints out call trace during bootup.
>>>>
>>>> It is due to having this CONFIG item  ON  :
>>>>
>>>>
>>>> CONFIG_ZONE_DMA=y
>>>>
>>>> Turning off ZONE_DMA fixes a issue and Raspberry PI 4 will
>>>> use the device tree to specify memory below 1G.
>>>>
>>>>
>>> Disabling ZONE_DMA is temporary solution.  We may need proper solution
>>
>> Perhaps the Raspberry platform configuration dependencies need separated  from “server class” Arm  equipment ?  Or auto-configured on boot ?  Consult an expert ;-)
>>
>>
>>
>>>> I would like to see Chen’s feature added , perhaps as EXPERIMENTAL,  so we can get some configuration testing done on it.   It corrects having a DMA zone in low memory while crash-kernel is above 4GB.  This has been going on for a year now.
>>> I will also like this patch to be added in Linux as early as possible.
>>>
>>> Issue mentioned by me happens with or without this patch.
>>>
>>> This patch-set can consider fixing because it uses low memory for DMA
>>> & swiotlb only.
>>> We can consider restricting crashkernel within the required range like below
>>>
>>> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
>>> index 7f9e5a6dc48c..bd67b90d35bd 100644
>>> --- a/kernel/crash_core.c
>>> +++ b/kernel/crash_core.c
>>> @@ -354,7 +354,7 @@ int __init reserve_crashkernel_low(void)
>>>                        return 0;
>>>        }
>>>
>>> -       low_base = memblock_find_in_range(0, 1ULL << 32, low_size, CRASH_ALIGN);
>>> +       low_base = memblock_find_in_range(0,0xc0000000, low_size, CRASH_ALIGN);
>>>        if (!low_base) {
>>>                pr_err("Cannot reserve %ldMB crashkernel low memory,
>>> please try smaller size.\n",
>>>                       (unsigned long)(low_size >> 20));
>>>
>>>
>>     I suspect  0xc0000000  would need to be a CONFIG item  and not hard-coded.
>>
> if you consider this as valid change,  can you please incorporate as
> part of your patch-set.

After commit 1a8e1cef7 ("arm64: use both ZONE_DMA and ZONE_DMA32"),the 0-4G memory is splited
to DMA [mem 0x0000000000000000-0x000000003fffffff] and DMA32 [mem 0x0000000040000000-0x00000000ffffffff] on arm64.

From the above discussion, on your platform, the low crashkernel fall in DMA32 region, but your environment needs to access DMA
region, so there is the call trace.

I have a question, why do you choose 0xc0000000 here?

Besides, this is common code, we also need to consider about x86.

Thanks,
Chen Zhou

>
> --pk.
>
> .
>



^ permalink raw reply

* [RESEND v7 4/6] mfd: stm32: enable regmap fast_io for stm32-lptimer
From: Benjamin Gaignard @ 2020-06-03 12:54 UTC (permalink / raw)
  To: fabrice.gasnier, lee.jones, robh+dt, mcoquelin.stm32,
	alexandre.torgue, linux, daniel.lezcano, tglx
  Cc: devicetree, linux-stm32, linux-arm-kernel, linux-kernel,
	Benjamin Gaignard
In-Reply-To: <20200603125439.23275-1-benjamin.gaignard@st.com>

Because stm32-lptimer need to write in registers in interrupt context
enable regmap fast_io to use a spin_lock to protect registers access
rather than a mutex.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/stm32-lptimer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mfd/stm32-lptimer.c b/drivers/mfd/stm32-lptimer.c
index a00f99f36559..746e51a17cc8 100644
--- a/drivers/mfd/stm32-lptimer.c
+++ b/drivers/mfd/stm32-lptimer.c
@@ -17,6 +17,7 @@ static const struct regmap_config stm32_lptimer_regmap_cfg = {
 	.val_bits = 32,
 	.reg_stride = sizeof(u32),
 	.max_register = STM32_LPTIM_MAX_REGISTER,
+	.fast_io = true,
 };
 
 static int stm32_lptimer_detect_encoder(struct stm32_lptimer *ddata)
-- 
2.15.0


^ permalink raw reply related

* [RESEND v7 6/6] ARM: mach-stm32: select low power timer for STM32MP157
From: Benjamin Gaignard @ 2020-06-03 12:54 UTC (permalink / raw)
  To: fabrice.gasnier, lee.jones, robh+dt, mcoquelin.stm32,
	alexandre.torgue, linux, daniel.lezcano, tglx
  Cc: devicetree, linux-stm32, linux-arm-kernel, linux-kernel,
	Benjamin Gaignard
In-Reply-To: <20200603125439.23275-1-benjamin.gaignard@st.com>

Make MACH_STM32MP157 select CLKSRC_STM32_LP to get a broadcast timer.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
 arch/arm/mach-stm32/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-stm32/Kconfig b/arch/arm/mach-stm32/Kconfig
index 57699bd8f107..d78f55b7b1d0 100644
--- a/arch/arm/mach-stm32/Kconfig
+++ b/arch/arm/mach-stm32/Kconfig
@@ -46,6 +46,7 @@ if ARCH_MULTI_V7
 config MACH_STM32MP157
 	bool "STMicroelectronics STM32MP157"
 	select ARM_ERRATA_814220
+	select CLKSRC_STM32_LP
 	default y
 
 endif # ARMv7-A
-- 
2.15.0


^ permalink raw reply related


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