Devicetree
 help / color / mirror / Atom feed
* [PATCH v2 4/6] dt: bindings: add mt7621-pll dt binding documentation
From: Chuanhong Guo @ 2019-07-24  2:23 UTC (permalink / raw)
  To: open list:COMMON CLK FRAMEWORK,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list, open list:MIPS, open list:STAGING SUBSYSTEM
  Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
	Ralf Baechle, Paul Burton, James Hogan, John Crispin,
	Greg Kroah-Hartman, Weijie Gao, NeilBrown, Chuanhong Guo
In-Reply-To: <20190724022310.28010-1-gch981213@gmail.com>

This commit adds device tree binding documentation for MT7621
PLL controller.

Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
---

Change since v1:
drop useless syscon in compatible string

 .../bindings/clock/mediatek,mt7621-pll.txt     | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt7621-pll.txt

diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt7621-pll.txt b/Documentation/devicetree/bindings/clock/mediatek,mt7621-pll.txt
new file mode 100644
index 000000000000..7dcfbd5283e3
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/mediatek,mt7621-pll.txt
@@ -0,0 +1,18 @@
+Binding for Mediatek MT7621 PLL controller
+
+The PLL controller provides the 2 main clocks of the SoC: CPU and BUS.
+
+Required Properties:
+- compatible: has to be "mediatek,mt7621-pll"
+- #clock-cells: has to be one
+
+Optional properties:
+- clock-output-names: should be "cpu", "bus"
+
+Example:
+	pll {
+		compatible = "mediatek,mt7621-pll";
+
+		#clock-cells = <1>;
+		clock-output-names = "cpu", "bus";
+	};
-- 
2.21.0

^ permalink raw reply related

* [PATCH v2 3/6] MIPS: ralink: add clock device providing cpu/bus clock for mt7621
From: Chuanhong Guo @ 2019-07-24  2:23 UTC (permalink / raw)
  To: open list:COMMON CLK FRAMEWORK,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list, open list:MIPS, open list:STAGING SUBSYSTEM
  Cc: Mark Rutland, Weijie Gao, Greg Kroah-Hartman, Stephen Boyd,
	James Hogan, Michael Turquette, Ralf Baechle, Paul Burton,
	Rob Herring, John Crispin, NeilBrown
In-Reply-To: <20190724022310.28010-1-gch981213@gmail.com>

For a long time the mt7621 uses a fixed cpu clock which causes a problem
if the cpu frequency is not 880MHz.
This patch adds cpu/bus clock calculation code and binds clocks to
mt7621-pll node.

Ported from OpenWrt:
c7ca224299 ramips: fix cpu clock of mt7621 and add dt clk devices

Signed-off-by: Weijie Gao <hackpascal@gmail.com>
Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
---

Changes since v1:
1. split patch.
2. calculate clocks using the function called by CLK_OF_DECLARE
   drop direct function call in timer-gic.c of ralink_clk_init
3. drop assignment of mips-hpt-frequency

 arch/mips/include/asm/mach-ralink/mt7621.h | 20 ++++++
 arch/mips/ralink/mt7621.c                  | 77 ++++++++++++++++++++++
 2 files changed, 97 insertions(+)

diff --git a/arch/mips/include/asm/mach-ralink/mt7621.h b/arch/mips/include/asm/mach-ralink/mt7621.h
index 65483a4681ab..51a6e51aef3f 100644
--- a/arch/mips/include/asm/mach-ralink/mt7621.h
+++ b/arch/mips/include/asm/mach-ralink/mt7621.h
@@ -17,6 +17,10 @@
 #define SYSC_REG_CHIP_REV		0x0c
 #define SYSC_REG_SYSTEM_CONFIG0		0x10
 #define SYSC_REG_SYSTEM_CONFIG1		0x14
+#define SYSC_REG_CLKCFG0		0x2c
+#define SYSC_REG_CUR_CLK_STS		0x44
+
+#define MEMC_REG_CPU_PLL		0x648
 
 #define CHIP_REV_PKG_MASK		0x1
 #define CHIP_REV_PKG_SHIFT		16
@@ -24,6 +28,22 @@
 #define CHIP_REV_VER_SHIFT		8
 #define CHIP_REV_ECO_MASK		0xf
 
+#define XTAL_MODE_SEL_MASK		0x7
+#define XTAL_MODE_SEL_SHIFT		6
+
+#define CPU_CLK_SEL_MASK		0x3
+#define CPU_CLK_SEL_SHIFT		30
+
+#define CUR_CPU_FDIV_MASK		0x1f
+#define CUR_CPU_FDIV_SHIFT		8
+#define CUR_CPU_FFRAC_MASK		0x1f
+#define CUR_CPU_FFRAC_SHIFT		0
+
+#define CPU_PLL_PREDIV_MASK		0x3
+#define CPU_PLL_PREDIV_SHIFT		12
+#define CPU_PLL_FBDIV_MASK		0x7f
+#define CPU_PLL_FBDIV_SHIFT		4
+
 #define MT7621_DRAM_BASE                0x0
 #define MT7621_DDR2_SIZE_MIN		32
 #define MT7621_DDR2_SIZE_MAX		256
diff --git a/arch/mips/ralink/mt7621.c b/arch/mips/ralink/mt7621.c
index ba39f3f3a7c7..baf9033a67b4 100644
--- a/arch/mips/ralink/mt7621.c
+++ b/arch/mips/ralink/mt7621.c
@@ -7,12 +7,16 @@
 
 #include <linux/kernel.h>
 #include <linux/init.h>
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
+#include <dt-bindings/clock/mt7621-clk.h>
 
 #include <asm/mipsregs.h>
 #include <asm/smp-ops.h>
 #include <asm/mips-cps.h>
 #include <asm/mach-ralink/ralink_regs.h>
 #include <asm/mach-ralink/mt7621.h>
+#include <asm/time.h>
 
 #include <pinmux.h>
 
@@ -103,11 +107,84 @@ static struct rt2880_pmx_group mt7621_pinmux_data[] = {
 	{ 0 }
 };
 
+static struct clk *clks[MT7621_CLK_MAX];
+static struct clk_onecell_data clk_data = {
+	.clks = clks,
+	.clk_num = ARRAY_SIZE(clks),
+};
+
 phys_addr_t mips_cpc_default_phys_base(void)
 {
 	panic("Cannot detect cpc address");
 }
 
+static struct clk *__init mt7621_add_sys_clkdev(
+	const char *id, unsigned long rate)
+{
+	struct clk *clk;
+	int err;
+
+	clk = clk_register_fixed_rate(NULL, id, NULL, 0, rate);
+	if (IS_ERR(clk))
+		panic("failed to allocate %s clock structure", id);
+
+	err = clk_register_clkdev(clk, id, NULL);
+	if (err)
+		panic("unable to register %s clock device", id);
+
+	return clk;
+}
+
+static void __init mt7621_clocks_init(struct device_node *np)
+{
+	const static u32 prediv_tbl[] = {0, 1, 2, 2};
+	u32 syscfg, xtal_sel, clkcfg, clk_sel, curclk, ffiv, ffrac;
+	u32 pll, prediv, fbdiv;
+	u32 xtal_clk, cpu_clk, bus_clk;
+
+	syscfg = rt_sysc_r32(SYSC_REG_SYSTEM_CONFIG0);
+	xtal_sel = (syscfg >> XTAL_MODE_SEL_SHIFT) & XTAL_MODE_SEL_MASK;
+
+	clkcfg = rt_sysc_r32(SYSC_REG_CLKCFG0);
+	clk_sel = (clkcfg >> CPU_CLK_SEL_SHIFT) & CPU_CLK_SEL_MASK;
+
+	curclk = rt_sysc_r32(SYSC_REG_CUR_CLK_STS);
+	ffiv = (curclk >> CUR_CPU_FDIV_SHIFT) & CUR_CPU_FDIV_MASK;
+	ffrac = (curclk >> CUR_CPU_FFRAC_SHIFT) & CUR_CPU_FFRAC_MASK;
+
+	if (xtal_sel <= 2)
+		xtal_clk = 20 * 1000 * 1000;
+	else if (xtal_sel <= 5)
+		xtal_clk = 40 * 1000 * 1000;
+	else
+		xtal_clk = 25 * 1000 * 1000;
+
+	switch (clk_sel) {
+	case 0:
+		cpu_clk = 500 * 1000 * 1000;
+		break;
+	case 1:
+		pll = rt_memc_r32(MEMC_REG_CPU_PLL);
+		fbdiv = (pll >> CPU_PLL_FBDIV_SHIFT) & CPU_PLL_FBDIV_MASK;
+		prediv = (pll >> CPU_PLL_PREDIV_SHIFT) & CPU_PLL_PREDIV_MASK;
+		cpu_clk = ((fbdiv + 1) * xtal_clk) >> prediv_tbl[prediv];
+		break;
+	default:
+		cpu_clk = xtal_clk;
+	}
+
+	cpu_clk = cpu_clk / ffiv * ffrac;
+	bus_clk = cpu_clk / 4;
+
+	clks[MT7621_CLK_CPU] = mt7621_add_sys_clkdev("cpu", cpu_clk);
+	clks[MT7621_CLK_BUS] = mt7621_add_sys_clkdev("bus", bus_clk);
+
+	pr_info("CPU Clock: %dMHz\n", cpu_clk / 1000000);
+
+	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
+}
+CLK_OF_DECLARE(mt7621_clk, "mediatek,mt7621-pll", mt7621_clocks_init);
+
 void __init ralink_of_remap(void)
 {
 	rt_sysc_membase = plat_of_remap_node("mtk,mt7621-sysc");
-- 
2.21.0

^ permalink raw reply related

* [PATCH v2 2/6] MIPS: ralink: drop ralink_clk_init for mt7621
From: Chuanhong Guo @ 2019-07-24  2:23 UTC (permalink / raw)
  To: open list:COMMON CLK FRAMEWORK,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list, open list:MIPS, open list:STAGING SUBSYSTEM
  Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
	Ralf Baechle, Paul Burton, James Hogan, John Crispin,
	Greg Kroah-Hartman, Weijie Gao, NeilBrown, Chuanhong Guo
In-Reply-To: <20190724022310.28010-1-gch981213@gmail.com>

This function isn't called anywhere. just drop it.

Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
---
Change since v1:
New patch. Split from:
  "MIPS: ralink: fix cpu clock of mt7621 and add dt clk devices"

 arch/mips/ralink/mt7621.c | 43 ---------------------------------------
 1 file changed, 43 deletions(-)

diff --git a/arch/mips/ralink/mt7621.c b/arch/mips/ralink/mt7621.c
index 9415be0d57b8..ba39f3f3a7c7 100644
--- a/arch/mips/ralink/mt7621.c
+++ b/arch/mips/ralink/mt7621.c
@@ -18,11 +18,6 @@
 
 #include "common.h"
 
-#define SYSC_REG_SYSCFG		0x10
-#define SYSC_REG_CPLL_CLKCFG0	0x2c
-#define SYSC_REG_CUR_CLK_STS	0x44
-#define CPU_CLK_SEL		(BIT(30) | BIT(31))
-
 #define MT7621_GPIO_MODE_UART1		1
 #define MT7621_GPIO_MODE_I2C		2
 #define MT7621_GPIO_MODE_UART3_MASK	0x3
@@ -113,44 +108,6 @@ phys_addr_t mips_cpc_default_phys_base(void)
 	panic("Cannot detect cpc address");
 }
 
-void __init ralink_clk_init(void)
-{
-	int cpu_fdiv = 0;
-	int cpu_ffrac = 0;
-	int fbdiv = 0;
-	u32 clk_sts, syscfg;
-	u8 clk_sel = 0, xtal_mode;
-	u32 cpu_clk;
-
-	if ((rt_sysc_r32(SYSC_REG_CPLL_CLKCFG0) & CPU_CLK_SEL) != 0)
-		clk_sel = 1;
-
-	switch (clk_sel) {
-	case 0:
-		clk_sts = rt_sysc_r32(SYSC_REG_CUR_CLK_STS);
-		cpu_fdiv = ((clk_sts >> 8) & 0x1F);
-		cpu_ffrac = (clk_sts & 0x1F);
-		cpu_clk = (500 * cpu_ffrac / cpu_fdiv) * 1000 * 1000;
-		break;
-
-	case 1:
-		fbdiv = ((rt_sysc_r32(0x648) >> 4) & 0x7F) + 1;
-		syscfg = rt_sysc_r32(SYSC_REG_SYSCFG);
-		xtal_mode = (syscfg >> 6) & 0x7;
-		if (xtal_mode >= 6) {
-			/* 25Mhz Xtal */
-			cpu_clk = 25 * fbdiv * 1000 * 1000;
-		} else if (xtal_mode >= 3) {
-			/* 40Mhz Xtal */
-			cpu_clk = 40 * fbdiv * 1000 * 1000;
-		} else {
-			/* 20Mhz Xtal */
-			cpu_clk = 20 * fbdiv * 1000 * 1000;
-		}
-		break;
-	}
-}
-
 void __init ralink_of_remap(void)
 {
 	rt_sysc_membase = plat_of_remap_node("mtk,mt7621-sysc");
-- 
2.21.0

^ permalink raw reply related

* [PATCH v2 1/6] dt-bindings: clock: add dt binding header for mt7621-pll
From: Chuanhong Guo @ 2019-07-24  2:23 UTC (permalink / raw)
  To: open list:COMMON CLK FRAMEWORK,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list, open list:MIPS, open list:STAGING SUBSYSTEM
  Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
	Ralf Baechle, Paul Burton, James Hogan, John Crispin,
	Greg Kroah-Hartman, Weijie Gao, NeilBrown, Chuanhong Guo,
	Rob Herring
In-Reply-To: <20190724022310.28010-1-gch981213@gmail.com>

This patch adds dt binding header for mediatek,mt7621-pll

Signed-off-by: Weijie Gao <hackpascal@gmail.com>
Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Change since v1:
Change commit title prefix.

 include/dt-bindings/clock/mt7621-clk.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
 create mode 100644 include/dt-bindings/clock/mt7621-clk.h

diff --git a/include/dt-bindings/clock/mt7621-clk.h b/include/dt-bindings/clock/mt7621-clk.h
new file mode 100644
index 000000000000..a29e14ee2efe
--- /dev/null
+++ b/include/dt-bindings/clock/mt7621-clk.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2018 Weijie Gao <hackpascal@gmail.com>
+ */
+
+#ifndef __DT_BINDINGS_MT7621_CLK_H
+#define __DT_BINDINGS_MT7621_CLK_H
+
+#define MT7621_CLK_CPU		0
+#define MT7621_CLK_BUS		1
+
+#define MT7621_CLK_MAX		2
+
+#endif /* __DT_BINDINGS_MT7621_CLK_H */
-- 
2.21.0

^ permalink raw reply related

* [PATCH v2 0/6] MIPS: ralink: add CPU clock detection for MT7621
From: Chuanhong Guo @ 2019-07-24  2:23 UTC (permalink / raw)
  To: open list:COMMON CLK FRAMEWORK,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list, open list:MIPS, open list:STAGING SUBSYSTEM
  Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
	Ralf Baechle, Paul Burton, James Hogan, John Crispin,
	Greg Kroah-Hartman, Weijie Gao, NeilBrown, Chuanhong Guo

This patchset ports CPU clock detection for MT7621 from OpenWrt.

Last time I sent this, I forgot to add an binding include which
caused a compile error and the patch doesn't stay in linux-next.

This patchset resent the first two commits and also added binding
documentation for mt7621-pll and used it in mt7621-dts at
drivers/staging.

Changes since v1:
1. changed commit title prefix for dt include
2. split the patch adding clock node (details in that patch body)
3. drop useless syscon in dt documentation
4. drop cpuclock node for gbpc1

Chuanhong Guo (6):
  dt-bindings: clock: add dt binding header for mt7621-pll
  MIPS: ralink: drop ralink_clk_init for mt7621
  MIPS: ralink: add clock device providing cpu/bus clock for mt7621
  dt: bindings: add mt7621-pll dt binding documentation
  staging: mt7621-dts: fix register range of memc node in mt7621.dtsi
  staging: mt7621-dts: add dt nodes for mt7621-pll

 .../bindings/clock/mediatek,mt7621-pll.txt    | 18 ++++
 arch/mips/include/asm/mach-ralink/mt7621.h    | 20 ++++
 arch/mips/ralink/mt7621.c                     | 98 +++++++++++++------
 drivers/staging/mt7621-dts/gbpc1.dts          |  5 -
 drivers/staging/mt7621-dts/mt7621.dtsi        | 17 ++--
 include/dt-bindings/clock/mt7621-clk.h        | 14 +++
 6 files changed, 126 insertions(+), 46 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt7621-pll.txt
 create mode 100644 include/dt-bindings/clock/mt7621-clk.h

-- 
2.21.0

^ permalink raw reply

* RE: [PATCH v5 12/15] ARM: dts: imx6ul: add dma support on ecspi
From: Robin Gong @ 2019-07-24  1:53 UTC (permalink / raw)
  To: Shawn Guo
  Cc: robh+dt@kernel.org, mark.rutland@arm.com, s.hauer@pengutronix.de,
	kernel@pengutronix.de, broonie@kernel.org, festevam@gmail.com,
	vkoul@kernel.org, dan.j.williams@intel.com,
	u.kleine-koenig@pengutronix.de, catalin.marinas@arm.com,
	l.stach@pengutronix.de, will.deacon@arm.com,
	linux-spi@vger.kernel.org, dl-linux-imx,
	linux-arm-kernel@lists.infradead.org, linux-kern
In-Reply-To: <20190724004848.GV15632@dragon>

On 2019-7-24 at 08:49 Shawn Guo <shawnguo@kernel.org> wrote:
> On Tue, Jul 23, 2019 at 09:39:38AM +0000, Robin Gong wrote:
> > On 2019-7-17 at 14:42 Shawn Guo <shawnguo@kernel.org> wrote:
> > > On Mon, Jun 10, 2019 at 04:17:50PM +0800, yibin.gong@nxp.com wrote:
> > > > From: Robin Gong <yibin.gong@nxp.com>
> > > >
> > > > Add dma support on ecspi.
> > > >
> > > > Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> > >
> > > Applied, thanks.
> > Thanks Shawn, but how about other dts patches such as 01/15,02/15?
> 
> I need the authors of the commits being reverted agree on the reverting.
> 
>   Sean Nyekjaer <sean.nyekjaer@prevas.dk>
>   Sascha Hauer <s.hauer@pengutronix.de>
Seems Sean's mail can't be reached.
Hello Sacha, Could you please help test if my patch set could fix your issue even
I revert your patch?
https://patchwork.kernel.org/cover/10984301/
> 
> Shawn

^ permalink raw reply

* Re: [PATCH v5 12/15] ARM: dts: imx6ul: add dma support on ecspi
From: Shawn Guo @ 2019-07-24  0:48 UTC (permalink / raw)
  To: Robin Gong
  Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, catalin.marinas@arm.com,
	s.hauer@pengutronix.de, will.deacon@arm.com, dl-linux-imx,
	linux-spi@vger.kernel.org, vkoul@kernel.org, robh+dt@kernel.org,
	broonie@kernel.org, kernel@pengutronix.de,
	u.kleine-koenig@pengutronix.de, dmaengine@vger.kernel.org,
	dan.j.williams@intel.com, festevam@gmail.com,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <VE1PR04MB663894FA5BC88B130C70AC0789C70@VE1PR04MB6638.eurprd04.prod.outlook.com>

On Tue, Jul 23, 2019 at 09:39:38AM +0000, Robin Gong wrote:
> On 2019-7-17 at 14:42 Shawn Guo <shawnguo@kernel.org> wrote:
> > On Mon, Jun 10, 2019 at 04:17:50PM +0800, yibin.gong@nxp.com wrote:
> > > From: Robin Gong <yibin.gong@nxp.com>
> > >
> > > Add dma support on ecspi.
> > >
> > > Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> > 
> > Applied, thanks.
> Thanks Shawn, but how about other dts patches such as 01/15,02/15?

I need the authors of the commits being reverted agree on the reverting.

  Sean Nyekjaer <sean.nyekjaer@prevas.dk>
  Sascha Hauer <s.hauer@pengutronix.de>

Shawn

^ permalink raw reply

* Re: [PATCH v3 1/6] dt-bindings: opp: Introduce opp-peak-KBps and opp-avg-KBps bindings
From: Saravana Kannan @ 2019-07-24  0:18 UTC (permalink / raw)
  To: Rob Herring
  Cc: Sibi Sankar, Georgi Djakov, Mark Rutland, Viresh Kumar,
	Nishanth Menon, Stephen Boyd, Rafael J. Wysocki, Vincent Guittot,
	Sweeney, Sean, daidavid1, Rajendra Nayak, Bjorn Andersson,
	Evan Green, Android Kernel Team, Linux PM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML
In-Reply-To: <CAL_JsqJeGK2m6hVEqvFxW6dEhi22LXaRKgb5JmnJfmCp+J6XuA@mail.gmail.com>

On Tue, Jul 23, 2019 at 7:27 AM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Jul 22, 2019 at 5:41 PM Saravana Kannan <saravanak@google.com> wrote:
> >
> > On Mon, Jul 22, 2019 at 4:35 PM Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Tue, Jul 16, 2019 at 11:58:08AM -0700, Saravana Kannan wrote:
> > > > On Tue, Jul 16, 2019 at 10:25 AM Sibi Sankar <sibis@codeaurora.org> wrote:
> > > > >
> > > > > Hey Saravana,
> > > > >
> > > > > https://patchwork.kernel.org/patch/10850815/
> > > > > There was already a discussion ^^ on how bandwidth bindings were to be
> > > > > named.
> > > >
> > > > Yes, I'm aware of that series. That series is trying to define a BW
> > > > mapping for an existing frequency OPP table. This patch is NOT about
> > > > adding a mapping to an existing table. This patch is about adding the
> > > > notion of BW OPP tables where BW is the "key" instead of "frequency".
> > > >
> > > > So let's not mixed up these two series.
> > >
> > > Maybe different reasons, but in the end we'd end up with 2 bandwidth
> > > properties. We need to sort out how they'd overlap/coexist.
> >
> > Oh, I totally agree! My point is that the other mapping isn't the
> > right approach because it doesn't handle a whole swath of use cases.
> > The one I'm proposing can act as a super set of the other (as in, can
> > handle that use case too).
> >
> > > The same comment in that series about defining a standard unit suffix
> > > also applies to this one.
> >
> > I thought I read that whole series and I don't remember reading about
> > the unit suffix. But I'll take a closer look. I've chosen to keep the
> > DT units at least as "high of a resolution" as what the APIs accept
> > today. The APIs take KB/s. So I make sure DT can capture KB/s
> > differences. If we all agree that KB/s is "too accurate" then I think
> > we should change everything to MB/s.
>
> Either one is fine with me, but trying to align to what the OS picked
> doesn't work. What does BSD use for example? More important is
> aligning across DT properties so we don't have folks picking whatever
> random unit they like. We generally try to go with the smallest units
> that will have enough (32-bit) range for everyone, so that's probably
> KB/s here.

Yeah, that makes sense.

-Saravana

^ permalink raw reply

* [PATCH v7 7/7] of/platform: Don't create device links for default busses
From: Saravana Kannan @ 2019-07-24  0:11 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Greg Kroah-Hartman, Rafael J. Wysocki,
	Frank Rowand
  Cc: Saravana Kannan, devicetree, linux-kernel, David Collins,
	kernel-team
In-Reply-To: <20190724001100.133423-1-saravanak@google.com>

Default busses also have devices created for them. But there's no point
in creating device links for them. It's especially wasteful as it'll
cause the traversal of the entire device tree and also spend a lot of
time checking and figuring out that creating those links isn't allowed.
So check for default busses and skip trying to create device links for
them.

Signed-off-by: Saravana Kannan <saravanak@google.com>
---
 drivers/of/platform.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 41499ddc8d95..676b2f730d1b 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -682,6 +682,8 @@ static int of_link_to_suppliers(struct device *dev)
 		return 0;
 	if (unlikely(!dev->of_node))
 		return 0;
+	if (of_match_node(of_default_bus_match_table, dev->of_node))
+		return 0;
 
 	return __of_link_to_suppliers(dev, dev->of_node);
 }
-- 
2.22.0.709.g102302147b-goog

^ permalink raw reply related

* [PATCH v7 6/7] of/platform: Create device links for all child-supplier depencencies
From: Saravana Kannan @ 2019-07-24  0:10 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Greg Kroah-Hartman, Rafael J. Wysocki,
	Frank Rowand
  Cc: Saravana Kannan, devicetree, linux-kernel, David Collins,
	kernel-team
In-Reply-To: <20190724001100.133423-1-saravanak@google.com>

A parent device can have child devices that it adds when it probes. But
this probing of the parent device can happen way after kernel init is done
-- for example, when the parent device's driver is loaded as a module.

In such cases, if the child devices depend on a supplier in the system, we
need to make sure the supplier gets the sync_state() callback only after
these child devices are added and probed.

To achieve this, when creating device links for a device by looking at its
DT node, don't just look at DT references at the top node level. Look at DT
references in all the descendant nodes too and create device links from the
ancestor device to all these supplier devices.

This way, when the parent device probes and adds child devices, the child
devices can then create their own device links to the suppliers and further
delay the supplier's sync_state() callback to after the child devices are
probed.

Example:
In this illustration, -> denotes DT references and indentation
represents child status.

Device node A
	Device node B -> D
	Device node C -> B, D

Device node D

Assume all these devices have their drivers loaded as modules.

Without this patch, this is the sequence of events:
1. D is added.
2. A is added.
3. Device D probes.
4. Device D gets its sync_state() callback.
5. Device B and C might malfunction because their resources got
   altered/turned off before they can make active requests for them.

With this patch, this is the sequence of events:
1. D is added.
2. A is added and creates device links to D.
3. Device link from A to B is not added because A is a parent of B.
4. Device D probes.
5. Device D does not get it's sync_state() callback because consumer A
   hasn't probed yet.
5. Device A probes.
5. a. Devices B and C are added.
5. b. Device links from B and C to D are added.
5. c. Device A's probe completes.
6. Device D does not get it's sync_state() callback because consumer A
   has probed but consumers B and C haven't probed yet.
7. Device B and C probe.
8. Device D gets it's sync_state() callback because all its consumers
   have probed.
9. None of the devices malfunction.

Signed-off-by: Saravana Kannan <saravanak@google.com>
---
 drivers/of/platform.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 71d6138698ec..41499ddc8d95 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -655,24 +655,35 @@ static bool of_link_property(struct device *dev, struct device_node *con_np,
 	return done ? 0 : -ENODEV;
 }
 
+static int __of_link_to_suppliers(struct device *dev,
+				  struct device_node *con_np)
+{
+	struct device_node *child;
+	struct property *p;
+	bool done = true;
+
+	for_each_property_of_node(con_np, p)
+		if (of_link_property(dev, con_np, p->name))
+			done = false;
+
+	for_each_child_of_node(con_np, child)
+		if (__of_link_to_suppliers(dev, child))
+			done = false;
+
+	return done ? 0 : -ENODEV;
+}
+
 static bool of_devlink;
 core_param(of_devlink, of_devlink, bool, 0);
 
 static int of_link_to_suppliers(struct device *dev)
 {
-	struct property *p;
-	bool done = true;
-
 	if (!of_devlink)
 		return 0;
 	if (unlikely(!dev->of_node))
 		return 0;
 
-	for_each_property_of_node(dev->of_node, p)
-		if (of_link_property(dev, dev->of_node, p->name))
-			done = false;
-
-	return done ? 0 : -ENODEV;
+	return __of_link_to_suppliers(dev, dev->of_node);
 }
 
 #ifndef CONFIG_PPC
-- 
2.22.0.709.g102302147b-goog

^ permalink raw reply related

* [PATCH v7 5/7] of/platform: Pause/resume sync state during init and of_platform_populate()
From: Saravana Kannan @ 2019-07-24  0:10 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Greg Kroah-Hartman, Rafael J. Wysocki,
	Frank Rowand
  Cc: Saravana Kannan, devicetree, linux-kernel, David Collins,
	kernel-team
In-Reply-To: <20190724001100.133423-1-saravanak@google.com>

When all the top level devices are populated from DT during kernel
init, the supplier devices could be added and probed before the
consumer devices are added and linked to the suppliers. To avoid the
sync_state() callback from being called prematurely, pause the
sync_state() callbacks before populating the devices and resume them
at late_initcall_sync().

Similarly, when children devices are populated after kernel init using
of_platform_populate(), there could be supplier-consumer dependencies
between the children devices that are populated. To avoid the same
problem with sync_state() being called prematurely, pause and resume
sync_state() callbacks across of_platform_populate().

Signed-off-by: Saravana Kannan <saravanak@google.com>
---
 drivers/of/platform.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 4344419a26fc..71d6138698ec 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -485,6 +485,7 @@ int of_platform_populate(struct device_node *root,
 	pr_debug("%s()\n", __func__);
 	pr_debug(" starting at: %pOF\n", root);
 
+	device_links_supplier_sync_state_pause();
 	for_each_child_of_node(root, child) {
 		rc = of_platform_bus_create(child, matches, lookup, parent, true);
 		if (rc) {
@@ -492,6 +493,8 @@ int of_platform_populate(struct device_node *root,
 			break;
 		}
 	}
+	device_links_supplier_sync_state_resume();
+
 	of_node_set_flag(root, OF_POPULATED_BUS);
 
 	of_node_put(root);
@@ -688,6 +691,7 @@ static int __init of_platform_default_populate_init(void)
 		return -ENODEV;
 
 	platform_bus_type.add_links = of_link_to_suppliers;
+	device_links_supplier_sync_state_pause();
 	/*
 	 * Handle certain compatibles explicitly, since we don't want to create
 	 * platform_devices for every node in /reserved-memory with a
@@ -708,6 +712,13 @@ static int __init of_platform_default_populate_init(void)
 	return 0;
 }
 arch_initcall_sync(of_platform_default_populate_init);
+
+static int __init of_platform_sync_state_init(void)
+{
+	device_links_supplier_sync_state_resume();
+	return 0;
+}
+late_initcall_sync(of_platform_sync_state_init);
 #endif
 
 int of_platform_device_destroy(struct device *dev, void *data)
-- 
2.22.0.709.g102302147b-goog

^ permalink raw reply related

* [PATCH v7 4/7] driver core: Add sync_state driver/bus callback
From: Saravana Kannan @ 2019-07-24  0:10 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Greg Kroah-Hartman, Rafael J. Wysocki,
	Frank Rowand
  Cc: Saravana Kannan, devicetree, linux-kernel, David Collins,
	kernel-team
In-Reply-To: <20190724001100.133423-1-saravanak@google.com>

This sync_state driver/bus callback is called once all the consumers
of a supplier have probed successfully.

This allows the supplier device's driver/bus to sync the supplier
device's state to the software state with the guarantee that all the
consumers are actively managing the resources provided by the supplier
device.

To maintain backwards compatibility and ease transition from existing
frameworks and resource cleanup schemes, late_initcall_sync is the
earliest when the sync_state callback might be called.

There is no upper bound on the time by which the sync_state callback
has to be called. This is because if a consumer device never probes,
the supplier has to maintain its resources in the state left by the
bootloader. For example, if the bootloader leaves the display
backlight at a fixed voltage and the backlight driver is never probed,
you don't want the backlight to ever be turned off after boot up.

Also, when multiple devices are added after kernel init, some
suppliers could be added before their consumer devices get added. In
these instances, the supplier devices could get their sync_state
callback called right after they probe because the consumers devices
haven't had a chance to create device links to the suppliers.

To handle this correctly, this change also provides APIs to
pause/resume sync state callbacks so that when multiple devices are
added, their sync_state callback evaluation can be postponed to happen
after all of them are added.

Signed-off-by: Saravana Kannan <saravanak@google.com>
---
 drivers/base/core.c    | 65 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/device.h | 23 +++++++++++++++
 2 files changed, 88 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 733d8a9aec76..aec725d9e17e 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -46,6 +46,8 @@ early_param("sysfs.deprecated", sysfs_deprecated_setup);
 /* Device links support. */
 static LIST_HEAD(wait_for_suppliers);
 static DEFINE_MUTEX(wfs_lock);
+static LIST_HEAD(deferred_sync);
+static unsigned int supplier_sync_state_disabled;
 
 #ifdef CONFIG_SRCU
 static DEFINE_MUTEX(device_links_lock);
@@ -634,6 +636,62 @@ int device_links_check_suppliers(struct device *dev)
 	return ret;
 }
 
+static void __device_links_supplier_sync_state(struct device *dev)
+{
+	struct device_link *link;
+
+	if (dev->state_synced)
+		return;
+
+	list_for_each_entry(link, &dev->links.consumers, s_node) {
+		if (link->flags & DL_FLAG_STATELESS)
+			continue;
+		if (link->status != DL_STATE_ACTIVE)
+			return;
+	}
+
+	if (dev->bus->sync_state)
+		dev->bus->sync_state(dev);
+	else if (dev->driver && dev->driver->sync_state)
+		dev->driver->sync_state(dev);
+
+	dev->state_synced = true;
+}
+
+void device_links_supplier_sync_state_pause(void)
+{
+	device_links_write_lock();
+	supplier_sync_state_disabled++;
+	device_links_write_unlock();
+}
+
+void device_links_supplier_sync_state_resume(void)
+{
+	struct device *dev, *tmp;
+
+	device_links_write_lock();
+	if (!supplier_sync_state_disabled) {
+		WARN(true, "Unmatched sync_state pause/resume!");
+		goto out;
+	}
+	supplier_sync_state_disabled--;
+	if (supplier_sync_state_disabled)
+		goto out;
+
+	list_for_each_entry_safe(dev, tmp, &deferred_sync, links.defer_sync) {
+		__device_links_supplier_sync_state(dev);
+		list_del_init(&dev->links.defer_sync);
+	}
+out:
+	device_links_write_unlock();
+}
+
+static void __device_links_supplier_defer_sync(struct device *sup)
+{
+	if (list_empty(&sup->links.defer_sync))
+		list_add_tail(&sup->links.defer_sync, &deferred_sync);
+}
+
 /**
  * device_links_driver_bound - Update device links after probing its driver.
  * @dev: Device to update the links for.
@@ -678,6 +736,11 @@ void device_links_driver_bound(struct device *dev)
 
 		WARN_ON(link->status != DL_STATE_CONSUMER_PROBE);
 		WRITE_ONCE(link->status, DL_STATE_ACTIVE);
+
+		if (supplier_sync_state_disabled)
+			__device_links_supplier_defer_sync(link->supplier);
+		else
+			__device_links_supplier_sync_state(link->supplier);
 	}
 
 	dev->links.status = DL_DEV_DRIVER_BOUND;
@@ -787,6 +850,7 @@ void device_links_driver_cleanup(struct device *dev)
 		WRITE_ONCE(link->status, DL_STATE_DORMANT);
 	}
 
+	list_del_init(&dev->links.defer_sync);
 	__device_links_no_driver(dev);
 
 	device_links_write_unlock();
@@ -1758,6 +1822,7 @@ void device_initialize(struct device *dev)
 	INIT_LIST_HEAD(&dev->links.consumers);
 	INIT_LIST_HEAD(&dev->links.suppliers);
 	INIT_LIST_HEAD(&dev->links.needs_suppliers);
+	INIT_LIST_HEAD(&dev->links.defer_sync);
 	dev->links.status = DL_DEV_NO_DRIVER;
 }
 EXPORT_SYMBOL_GPL(device_initialize);
diff --git a/include/linux/device.h b/include/linux/device.h
index 35aed50033c4..4e74ed9137a0 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -84,6 +84,8 @@ extern void bus_remove_file(struct bus_type *, struct bus_attribute *);
  *		available at the time this function is called.  As in, the
  *		function should NOT stop at the first failed device link if
  *		other unlinked supplier devices are present in the system.
+ *		This is necessary for the sync_state() callback to work
+ *		correctly.
  *
  *		Return 0 if device links have been successfully created to all
  *		the suppliers of this device.  Return an error if some of the
@@ -91,6 +93,13 @@ extern void bus_remove_file(struct bus_type *, struct bus_attribute *);
  *		reattempted in the future.
  * @probe:	Called when a new device or driver add to this bus, and callback
  *		the specific driver's probe to initial the matched device.
+ * @sync_state:	Called to sync device state to software state after all the
+ *		state tracking consumers linked to this device (present at
+ *		the time of late_initcall) have successfully bound to a
+ *		driver. If the device has no consumers, this function will
+ *		be called at late_initcall_sync level. If the device has
+ *		consumers that are never bound to a driver, this function
+ *		will never get called until they do.
  * @remove:	Called when a device removed from this bus.
  * @shutdown:	Called at shut-down time to quiesce the device.
  *
@@ -135,6 +144,7 @@ struct bus_type {
 	int (*uevent)(struct device *dev, struct kobj_uevent_env *env);
 	int (*add_links)(struct device *dev);
 	int (*probe)(struct device *dev);
+	void (*sync_state)(struct device *dev);
 	int (*remove)(struct device *dev);
 	void (*shutdown)(struct device *dev);
 
@@ -280,6 +290,13 @@ enum probe_type {
  * @probe:	Called to query the existence of a specific device,
  *		whether this driver can work with it, and bind the driver
  *		to a specific device.
+ * @sync_state:	Called to sync device state to software state after all the
+ *		state tracking consumers linked to this device (present at
+ *		the time of late_initcall) have successfully bound to a
+ *		driver. If the device has no consumers, this function will
+ *		be called at late_initcall_sync level. If the device has
+ *		consumers that are never bound to a driver, this function
+ *		will never get called until they do.
  * @remove:	Called when the device is removed from the system to
  *		unbind a device from this driver.
  * @shutdown:	Called at shut-down time to quiesce the device.
@@ -318,6 +335,7 @@ struct device_driver {
 
 	int (*edit_links)(struct device *dev);
 	int (*probe) (struct device *dev);
+	void (*sync_state)(struct device *dev);
 	int (*remove) (struct device *dev);
 	void (*shutdown) (struct device *dev);
 	int (*suspend) (struct device *dev, pm_message_t state);
@@ -921,12 +939,14 @@ enum dl_dev_state {
  * @suppliers: List of links to supplier devices.
  * @consumers: List of links to consumer devices.
  * @needs_suppliers: Hook to global list of devices waiting for suppliers.
+ * @defer_sync: Hook to global list of devices that have deferred sync_state.
  * @status: Driver status information.
  */
 struct dev_links_info {
 	struct list_head suppliers;
 	struct list_head consumers;
 	struct list_head needs_suppliers;
+	struct list_head defer_sync;
 	enum dl_dev_state status;
 };
 
@@ -1094,6 +1114,7 @@ struct device {
 	bool			offline:1;
 	bool			of_node_reused:1;
 	bool			has_edit_links:1;
+	bool			state_synced:1;
 #if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \
     defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU) || \
     defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL)
@@ -1437,6 +1458,8 @@ struct device_link *device_link_add(struct device *consumer,
 void device_link_del(struct device_link *link);
 void device_link_remove(void *consumer, struct device *supplier);
 void device_link_remove_from_wfs(struct device *consumer);
+void device_links_supplier_sync_state_pause(void);
+void device_links_supplier_sync_state_resume(void);
 
 #ifndef dev_fmt
 #define dev_fmt(fmt) fmt
-- 
2.22.0.709.g102302147b-goog

^ permalink raw reply related

* [PATCH v7 3/7] of/platform: Add functional dependency link from DT bindings
From: Saravana Kannan @ 2019-07-24  0:10 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Greg Kroah-Hartman, Rafael J. Wysocki,
	Frank Rowand, Jonathan Corbet
  Cc: Saravana Kannan, devicetree, linux-kernel, David Collins,
	kernel-team, linux-doc
In-Reply-To: <20190724001100.133423-1-saravanak@google.com>

Add device-links after the devices are created (but before they are
probed) by looking at common DT bindings like clocks and
interconnects.

Automatically adding device-links for functional dependencies at the
framework level provides the following benefits:

- Optimizes device probe order and avoids the useless work of
  attempting probes of devices that will not probe successfully
  (because their suppliers aren't present or haven't probed yet).

  For example, in a commonly available mobile SoC, registering just
  one consumer device's driver at an initcall level earlier than the
  supplier device's driver causes 11 failed probe attempts before the
  consumer device probes successfully. This was with a kernel with all
  the drivers statically compiled in. This problem gets a lot worse if
  all the drivers are loaded as modules without direct symbol
  dependencies.

- Supplier devices like clock providers, interconnect providers, etc
  need to keep the resources they provide active and at a particular
  state(s) during boot up even if their current set of consumers don't
  request the resource to be active. This is because the rest of the
  consumers might not have probed yet and turning off the resource
  before all the consumers have probed could lead to a hang or
  undesired user experience.

  Some frameworks (Eg: regulator) handle this today by turning off
  "unused" resources at late_initcall_sync and hoping all the devices
  have probed by then. This is not a valid assumption for systems with
  loadable modules. Other frameworks (Eg: clock) just don't handle
  this due to the lack of a clear signal for when they can turn off
  resources. This leads to downstream hacks to handle cases like this
  that can easily be solved in the upstream kernel.

  By linking devices before they are probed, we give suppliers a clear
  count of the number of dependent consumers. Once all of the
  consumers are active, the suppliers can turn off the unused
  resources without making assumptions about the number of consumers.

By default we just add device-links to track "driver presence" (probe
succeeded) of the supplier device. If any other functionality provided
by device-links are needed, it is left to the consumer/supplier
devices to change the link when they probe.

Signed-off-by: Saravana Kannan <saravanak@google.com>
---
 .../admin-guide/kernel-parameters.txt         |   5 +
 drivers/of/platform.c                         | 165 ++++++++++++++++++
 2 files changed, 170 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 46b826fcb5ad..12937349d79d 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3170,6 +3170,11 @@
 			This can be set from sysctl after boot.
 			See Documentation/admin-guide/sysctl/vm.rst for details.
 
+	of_devlink	[KNL] Make device links from common DT bindings. Useful
+			for optimizing probe order and making sure resources
+			aren't turned off before the consumer devices have
+			probed.
+
 	ohci1394_dma=early	[HW] enable debugging via the ohci1394 driver.
 			See Documentation/debugging-via-ohci1394.txt for more
 			info.
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 7801e25e6895..4344419a26fc 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -508,6 +508,170 @@ int of_platform_default_populate(struct device_node *root,
 }
 EXPORT_SYMBOL_GPL(of_platform_default_populate);
 
+bool of_link_is_valid(struct device_node *con, struct device_node *sup)
+{
+	of_node_get(sup);
+	/*
+	 * Don't allow linking a device node as a consumer of one of its
+	 * descendant nodes. By definition, a child node can't be a functional
+	 * dependency for the parent node.
+	 */
+	while (sup) {
+		if (sup == con) {
+			of_node_put(sup);
+			return false;
+		}
+		sup = of_get_next_parent(sup);
+	}
+	return true;
+}
+
+static int of_link_to_phandle(struct device *dev, struct device_node *sup_np)
+{
+	struct platform_device *sup_dev;
+	u32 dl_flags = DL_FLAG_AUTOPROBE_CONSUMER;
+	int ret = 0;
+
+	/*
+	 * Since we are trying to create device links, we need to find
+	 * the actual device node that owns this supplier phandle.
+	 * Often times it's the same node, but sometimes it can be one
+	 * of the parents. So walk up the parent till you find a
+	 * device.
+	 */
+	while (sup_np && !of_find_property(sup_np, "compatible", NULL))
+		sup_np = of_get_next_parent(sup_np);
+	if (!sup_np)
+		return 0;
+
+	if (!of_link_is_valid(dev->of_node, sup_np)) {
+		of_node_put(sup_np);
+		return 0;
+	}
+	sup_dev = of_find_device_by_node(sup_np);
+	of_node_put(sup_np);
+	if (!sup_dev)
+		return -ENODEV;
+	if (!device_link_add(dev, &sup_dev->dev, dl_flags))
+		ret = -ENODEV;
+	put_device(&sup_dev->dev);
+	return ret;
+}
+
+static struct device_node *parse_prop_cells(struct device_node *np,
+					    const char *prop, int index,
+					    const char *binding,
+					    const char *cell)
+{
+	struct of_phandle_args sup_args;
+
+	/* Don't need to check property name for every index. */
+	if (!index && strcmp(prop, binding))
+		return NULL;
+
+	if (of_parse_phandle_with_args(np, binding, cell, index, &sup_args))
+		return NULL;
+
+	return sup_args.np;
+}
+
+static struct device_node *parse_clocks(struct device_node *np,
+					const char *prop, int index)
+{
+	return parse_prop_cells(np, prop, index, "clocks", "#clock-cells");
+}
+
+static struct device_node *parse_interconnects(struct device_node *np,
+					       const char *prop, int index)
+{
+	return parse_prop_cells(np, prop, index, "interconnects",
+				"#interconnect-cells");
+}
+
+static int strcmp_suffix(const char *str, const char *suffix)
+{
+	unsigned int len, suffix_len;
+
+	len = strlen(str);
+	suffix_len = strlen(suffix);
+	if (len <= suffix_len)
+		return -1;
+	return strcmp(str + len - suffix_len, suffix);
+}
+
+static struct device_node *parse_regulators(struct device_node *np,
+					    const char *prop, int index)
+{
+	if (index || strcmp_suffix(prop, "-supply"))
+		return NULL;
+
+	return of_parse_phandle(np, prop, 0);
+}
+
+/**
+ * struct supplier_bindings - Information for parsing supplier DT binding
+ *
+ * @parse_prop:		If the function cannot parse the property, return NULL.
+ *			Otherwise, return the phandle listed in the property
+ *			that corresponds to the index.
+ */
+struct supplier_bindings {
+	struct device_node *(*parse_prop)(struct device_node *np,
+					  const char *name, int index);
+};
+
+static const struct supplier_bindings bindings[] = {
+	{ .parse_prop = parse_clocks, },
+	{ .parse_prop = parse_interconnects, },
+	{ .parse_prop = parse_regulators, },
+	{ },
+};
+
+static bool of_link_property(struct device *dev, struct device_node *con_np,
+			     const char *prop)
+{
+	struct device_node *phandle;
+	struct supplier_bindings *s = bindings;
+	unsigned int i = 0;
+	bool done = true, matched = false;
+
+	while (!matched && s->parse_prop) {
+		while ((phandle = s->parse_prop(con_np, prop, i))) {
+			matched = true;
+			i++;
+			if (of_link_to_phandle(dev, phandle))
+				/*
+				 * Don't stop at the first failure. See
+				 * Documentation for bus_type.add_links for
+				 * more details.
+				 */
+				done = false;
+		}
+		s++;
+	}
+	return done ? 0 : -ENODEV;
+}
+
+static bool of_devlink;
+core_param(of_devlink, of_devlink, bool, 0);
+
+static int of_link_to_suppliers(struct device *dev)
+{
+	struct property *p;
+	bool done = true;
+
+	if (!of_devlink)
+		return 0;
+	if (unlikely(!dev->of_node))
+		return 0;
+
+	for_each_property_of_node(dev->of_node, p)
+		if (of_link_property(dev, dev->of_node, p->name))
+			done = false;
+
+	return done ? 0 : -ENODEV;
+}
+
 #ifndef CONFIG_PPC
 static const struct of_device_id reserved_mem_matches[] = {
 	{ .compatible = "qcom,rmtfs-mem" },
@@ -523,6 +687,7 @@ static int __init of_platform_default_populate_init(void)
 	if (!of_have_populated_dt())
 		return -ENODEV;
 
+	platform_bus_type.add_links = of_link_to_suppliers;
 	/*
 	 * Handle certain compatibles explicitly, since we don't want to create
 	 * platform_devices for every node in /reserved-memory with a
-- 
2.22.0.709.g102302147b-goog

^ permalink raw reply related

* [PATCH v7 2/7] driver core: Add edit_links() callback for drivers
From: Saravana Kannan @ 2019-07-24  0:10 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Greg Kroah-Hartman, Rafael J. Wysocki,
	Frank Rowand
  Cc: Saravana Kannan, devicetree, linux-kernel, David Collins,
	kernel-team
In-Reply-To: <20190724001100.133423-1-saravanak@google.com>

The driver core/bus adding supplier-consumer dependencies by default
enables functional dependencies to be tracked correctly even when the
consumer devices haven't had their drivers registered or loaded (if they
are modules).

However, when the bus incorrectly adds dependencies that it shouldn't
have added, the devices might never probe.

For example, if device-C is a consumer of device-S and they have
phandles to each other in DT, the following could happen:

1.  Device-S get added first.
2.  The bus add_links() callback will (incorrectly) try to link it as
    a consumer of device-C.
3.  Since device-C isn't present, device-S will be put in
    "waiting-for-supplier" list.
4.  Device-C gets added next.
5.  All devices in "waiting-for-supplier" list are retried for linking.
6.  Device-S gets linked as consumer to Device-C.
7.  The bus add_links() callback will (correctly) try to link it as
    a consumer of device-S.
8.  This isn't allowed because it would create a cyclic device links.

Neither devices will get probed since the supplier is marked as
dependent on the consumer. And the consumer will never probe because the
consumer can't get resources from the supplier.

Without this patch, things stay in this broken state. However, with this
patch, the execution will continue like this:

9.  Device-C's driver is loaded.
10. Device-C's driver removes Device-S as a consumer of Device-C.
11. Device-C's driver adds Device-C as a consumer of Device-S.
12. Device-S probes.
14. Device-C probes.

Signed-off-by: Saravana Kannan <saravanak@google.com>
---
 drivers/base/core.c    | 24 ++++++++++++++++++++++--
 drivers/base/dd.c      | 29 +++++++++++++++++++++++++++++
 include/linux/device.h | 18 ++++++++++++++++++
 3 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 1b4eb221968f..733d8a9aec76 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -422,6 +422,19 @@ static void device_link_wait_for_supplier(struct device *consumer)
 	mutex_unlock(&wfs_lock);
 }
 
+/**
+ * device_link_remove_from_wfs - Unmark device as waiting for supplier
+ * @consumer: Consumer device
+ *
+ * Unmark the consumer device as waiting for suppliers to become available.
+ */
+void device_link_remove_from_wfs(struct device *consumer)
+{
+	mutex_lock(&wfs_lock);
+	list_del_init(&consumer->links.needs_suppliers);
+	mutex_unlock(&wfs_lock);
+}
+
 /**
  * device_link_check_waiting_consumers - Try to unmark waiting consumers
  *
@@ -439,12 +452,19 @@ static void device_link_wait_for_supplier(struct device *consumer)
 static void device_link_check_waiting_consumers(void)
 {
 	struct device *dev, *tmp;
+	int ret;
 
 	mutex_lock(&wfs_lock);
 	list_for_each_entry_safe(dev, tmp, &wait_for_suppliers,
-				 links.needs_suppliers)
-		if (!dev->bus->add_links(dev))
+				 links.needs_suppliers) {
+		ret = 0;
+		if (dev->has_edit_links)
+			ret = driver_edit_links(dev);
+		else if (dev->bus->add_links)
+			ret = dev->bus->add_links(dev);
+		if (!ret)
 			list_del_init(&dev->links.needs_suppliers);
+	}
 	mutex_unlock(&wfs_lock);
 }
 
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 994a90747420..5e7041ede0d7 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -698,6 +698,12 @@ int driver_probe_device(struct device_driver *drv, struct device *dev)
 	pr_debug("bus: '%s': %s: matched device %s with driver %s\n",
 		 drv->bus->name, __func__, dev_name(dev), drv->name);
 
+	if (drv->edit_links) {
+		if (drv->edit_links(dev))
+			dev->has_edit_links = true;
+		else
+			device_link_remove_from_wfs(dev);
+	}
 	pm_runtime_get_suppliers(dev);
 	if (dev->parent)
 		pm_runtime_get_sync(dev->parent);
@@ -786,6 +792,29 @@ struct device_attach_data {
 	bool have_async;
 };
 
+static int __driver_edit_links(struct device_driver *drv, void *data)
+{
+	struct device *dev = data;
+
+	if (!drv->edit_links)
+		return 0;
+
+	if (driver_match_device(drv, dev) <= 0)
+		return 0;
+
+	return drv->edit_links(dev);
+}
+
+int driver_edit_links(struct device *dev)
+{
+	int ret;
+
+	device_lock(dev);
+	ret = bus_for_each_drv(dev->bus, NULL, dev, __driver_edit_links);
+	device_unlock(dev);
+	return ret;
+}
+
 static int __device_attach_driver(struct device_driver *drv, void *_data)
 {
 	struct device_attach_data *data = _data;
diff --git a/include/linux/device.h b/include/linux/device.h
index 5d70babb7462..35aed50033c4 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -263,6 +263,20 @@ enum probe_type {
  * @probe_type:	Type of the probe (synchronous or asynchronous) to use.
  * @of_match_table: The open firmware table.
  * @acpi_match_table: The ACPI match table.
+ * @edit_links:	Called to allow a matched driver to edit the device links the
+ *		bus might have added incorrectly. This will be useful to handle
+ *		cases where the bus incorrectly adds functional dependencies
+ *		that aren't true or tries to create cyclic dependencies. But
+ *		doesn't correctly handle functional dependencies that are
+ *		missed by the bus as the supplier's sync_state might get to
+ *		execute before the driver for a missing consumer is loaded and
+ *		gets to edit the device links for the consumer.
+ *
+ *		This function might be called multiple times after a new device
+ *		is added.  The function is expected to create all the device
+ *		links for the new device and return 0 if it was completed
+ *		successfully or return an error if it needs to be reattempted
+ *		in the future.
  * @probe:	Called to query the existence of a specific device,
  *		whether this driver can work with it, and bind the driver
  *		to a specific device.
@@ -302,6 +316,7 @@ struct device_driver {
 	const struct of_device_id	*of_match_table;
 	const struct acpi_device_id	*acpi_match_table;
 
+	int (*edit_links)(struct device *dev);
 	int (*probe) (struct device *dev);
 	int (*remove) (struct device *dev);
 	void (*shutdown) (struct device *dev);
@@ -1078,6 +1093,7 @@ struct device {
 	bool			offline_disabled:1;
 	bool			offline:1;
 	bool			of_node_reused:1;
+	bool			has_edit_links:1;
 #if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \
     defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU) || \
     defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL)
@@ -1329,6 +1345,7 @@ extern int  __must_check device_attach(struct device *dev);
 extern int __must_check driver_attach(struct device_driver *drv);
 extern void device_initial_probe(struct device *dev);
 extern int __must_check device_reprobe(struct device *dev);
+extern int driver_edit_links(struct device *dev);
 
 extern bool device_is_bound(struct device *dev);
 
@@ -1419,6 +1436,7 @@ struct device_link *device_link_add(struct device *consumer,
 				    struct device *supplier, u32 flags);
 void device_link_del(struct device_link *link);
 void device_link_remove(void *consumer, struct device *supplier);
+void device_link_remove_from_wfs(struct device *consumer);
 
 #ifndef dev_fmt
 #define dev_fmt(fmt) fmt
-- 
2.22.0.709.g102302147b-goog

^ permalink raw reply related

* [PATCH v7 1/7] driver core: Add support for linking devices during device addition
From: Saravana Kannan @ 2019-07-24  0:10 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Greg Kroah-Hartman, Rafael J. Wysocki,
	Frank Rowand
  Cc: Saravana Kannan, devicetree, linux-kernel, David Collins,
	kernel-team
In-Reply-To: <20190724001100.133423-1-saravanak@google.com>

When devices are added, the bus might want to create device links to track
functional dependencies between supplier and consumer devices. This
tracking of supplier-consumer relationship allows optimizing device probe
order and tracking whether all consumers of a supplier are active. The
add_links bus callback is added to support this.

However, when consumer devices are added, they might not have a supplier
device to link to despite needing mandatory resources/functionality from
one or more suppliers. A waiting_for_suppliers list is created to track
such consumers and retry linking them when new devices get added.

Signed-off-by: Saravana Kannan <saravanak@google.com>
---
 drivers/base/core.c    | 83 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/device.h | 14 +++++++
 2 files changed, 97 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index da84a73f2ba6..1b4eb221968f 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -44,6 +44,8 @@ early_param("sysfs.deprecated", sysfs_deprecated_setup);
 #endif
 
 /* Device links support. */
+static LIST_HEAD(wait_for_suppliers);
+static DEFINE_MUTEX(wfs_lock);
 
 #ifdef CONFIG_SRCU
 static DEFINE_MUTEX(device_links_lock);
@@ -401,6 +403,51 @@ struct device_link *device_link_add(struct device *consumer,
 }
 EXPORT_SYMBOL_GPL(device_link_add);
 
+/**
+ * device_link_wait_for_supplier - Mark device as waiting for supplier
+ * @consumer: Consumer device
+ *
+ * Marks the consumer device as waiting for suppliers to become available. The
+ * consumer device will never be probed until it's unmarked as waiting for
+ * suppliers. The caller is responsible for adding the link to the supplier
+ * once the supplier device is present.
+ *
+ * This function is NOT meant to be called from the probe function of the
+ * consumer but rather from code that creates/adds the consumer device.
+ */
+static void device_link_wait_for_supplier(struct device *consumer)
+{
+	mutex_lock(&wfs_lock);
+	list_add_tail(&consumer->links.needs_suppliers, &wait_for_suppliers);
+	mutex_unlock(&wfs_lock);
+}
+
+/**
+ * device_link_check_waiting_consumers - Try to unmark waiting consumers
+ *
+ * Loops through all consumers waiting on suppliers and tries to add all their
+ * supplier links. If that succeeds, the consumer device is unmarked as waiting
+ * for suppliers. Otherwise, they are left marked as waiting on suppliers,
+ *
+ * The add_links bus callback is expected to return 0 if it has found and added
+ * all the supplier links for the consumer device. It should return an error if
+ * it isn't able to do so.
+ *
+ * The caller of device_link_wait_for_supplier() is expected to call this once
+ * it's aware of potential suppliers becoming available.
+ */
+static void device_link_check_waiting_consumers(void)
+{
+	struct device *dev, *tmp;
+
+	mutex_lock(&wfs_lock);
+	list_for_each_entry_safe(dev, tmp, &wait_for_suppliers,
+				 links.needs_suppliers)
+		if (!dev->bus->add_links(dev))
+			list_del_init(&dev->links.needs_suppliers);
+	mutex_unlock(&wfs_lock);
+}
+
 static void device_link_free(struct device_link *link)
 {
 	while (refcount_dec_not_one(&link->rpm_active))
@@ -535,6 +582,19 @@ int device_links_check_suppliers(struct device *dev)
 	struct device_link *link;
 	int ret = 0;
 
+	/*
+	 * If a device is waiting for one or more suppliers (in
+	 * wait_for_suppliers list), it is not ready to probe yet. So just
+	 * return -EPROBE_DEFER without having to check the links with existing
+	 * suppliers.
+	 */
+	mutex_lock(&wfs_lock);
+	if (!list_empty(&dev->links.needs_suppliers)) {
+		mutex_unlock(&wfs_lock);
+		return -EPROBE_DEFER;
+	}
+	mutex_unlock(&wfs_lock);
+
 	device_links_write_lock();
 
 	list_for_each_entry(link, &dev->links.suppliers, c_node) {
@@ -812,6 +872,10 @@ static void device_links_purge(struct device *dev)
 {
 	struct device_link *link, *ln;
 
+	mutex_lock(&wfs_lock);
+	list_del(&dev->links.needs_suppliers);
+	mutex_unlock(&wfs_lock);
+
 	/*
 	 * Delete all of the remaining links from this device to any other
 	 * devices (either consumers or suppliers).
@@ -1673,6 +1737,7 @@ void device_initialize(struct device *dev)
 #endif
 	INIT_LIST_HEAD(&dev->links.consumers);
 	INIT_LIST_HEAD(&dev->links.suppliers);
+	INIT_LIST_HEAD(&dev->links.needs_suppliers);
 	dev->links.status = DL_DEV_NO_DRIVER;
 }
 EXPORT_SYMBOL_GPL(device_initialize);
@@ -2108,6 +2173,24 @@ int device_add(struct device *dev)
 					     BUS_NOTIFY_ADD_DEVICE, dev);
 
 	kobject_uevent(&dev->kobj, KOBJ_ADD);
+
+	/*
+	 * Check if any of the other devices (consumers) have been waiting for
+	 * this device (supplier) to be added so that they can create a device
+	 * link to it.
+	 *
+	 * This needs to happen after device_pm_add() because device_link_add()
+	 * requires the supplier be registered before it's called.
+	 *
+	 * But this also needs to happe before bus_probe_device() to make sure
+	 * waiting consumers can link to it before the driver is bound to the
+	 * device and the driver sync_state callback is called for this device.
+	 */
+	device_link_check_waiting_consumers();
+
+	if (dev->bus && dev->bus->add_links && dev->bus->add_links(dev))
+		device_link_wait_for_supplier(dev);
+
 	bus_probe_device(dev);
 	if (parent)
 		klist_add_tail(&dev->p->knode_parent,
diff --git a/include/linux/device.h b/include/linux/device.h
index c330b75c6c57..5d70babb7462 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -78,6 +78,17 @@ extern void bus_remove_file(struct bus_type *, struct bus_attribute *);
  *		-EPROBE_DEFER it will queue the device for deferred probing.
  * @uevent:	Called when a device is added, removed, or a few other things
  *		that generate uevents to add the environment variables.
+ * @add_links:	Called, perhaps multiple times per device, after a device is
+ *		added to this bus.  The function is expected to create device
+ *		links to all the suppliers of the input device that are
+ *		available at the time this function is called.  As in, the
+ *		function should NOT stop at the first failed device link if
+ *		other unlinked supplier devices are present in the system.
+ *
+ *		Return 0 if device links have been successfully created to all
+ *		the suppliers of this device.  Return an error if some of the
+ *		suppliers are not yet available and this function needs to be
+ *		reattempted in the future.
  * @probe:	Called when a new device or driver add to this bus, and callback
  *		the specific driver's probe to initial the matched device.
  * @remove:	Called when a device removed from this bus.
@@ -122,6 +133,7 @@ struct bus_type {
 
 	int (*match)(struct device *dev, struct device_driver *drv);
 	int (*uevent)(struct device *dev, struct kobj_uevent_env *env);
+	int (*add_links)(struct device *dev);
 	int (*probe)(struct device *dev);
 	int (*remove)(struct device *dev);
 	void (*shutdown)(struct device *dev);
@@ -893,11 +905,13 @@ enum dl_dev_state {
  * struct dev_links_info - Device data related to device links.
  * @suppliers: List of links to supplier devices.
  * @consumers: List of links to consumer devices.
+ * @needs_suppliers: Hook to global list of devices waiting for suppliers.
  * @status: Driver status information.
  */
 struct dev_links_info {
 	struct list_head suppliers;
 	struct list_head consumers;
+	struct list_head needs_suppliers;
 	enum dl_dev_state status;
 };
 
-- 
2.22.0.709.g102302147b-goog

^ permalink raw reply related

* [PATCH v7 0/7] Solve postboot supplier cleanup and optimize probe ordering
From: Saravana Kannan @ 2019-07-24  0:10 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Greg Kroah-Hartman, Rafael J. Wysocki,
	Frank Rowand
  Cc: Saravana Kannan, devicetree, linux-kernel, David Collins,
	kernel-team

Add device-links to track functional dependencies between devices
after they are created (but before they are probed) by looking at
their common DT bindings like clocks, interconnects, etc.

Having functional dependencies automatically added before the devices
are probed, provides the following benefits:

- Optimizes device probe order and avoids the useless work of
  attempting probes of devices that will not probe successfully
  (because their suppliers aren't present or haven't probed yet).

  For example, in a commonly available mobile SoC, registering just
  one consumer device's driver at an initcall level earlier than the
  supplier device's driver causes 11 failed probe attempts before the
  consumer device probes successfully. This was with a kernel with all
  the drivers statically compiled in. This problem gets a lot worse if
  all the drivers are loaded as modules without direct symbol
  dependencies.

- Supplier devices like clock providers, interconnect providers, etc
  need to keep the resources they provide active and at a particular
  state(s) during boot up even if their current set of consumers don't
  request the resource to be active. This is because the rest of the
  consumers might not have probed yet and turning off the resource
  before all the consumers have probed could lead to a hang or
  undesired user experience.

  Some frameworks (Eg: regulator) handle this today by turning off
  "unused" resources at late_initcall_sync and hoping all the devices
  have probed by then. This is not a valid assumption for systems with
  loadable modules. Other frameworks (Eg: clock) just don't handle
  this due to the lack of a clear signal for when they can turn off
  resources. This leads to downstream hacks to handle cases like this
  that can easily be solved in the upstream kernel.

  By linking devices before they are probed, we give suppliers a clear
  count of the number of dependent consumers. Once all of the
  consumers are active, the suppliers can turn off the unused
  resources without making assumptions about the number of consumers.

By default we just add device-links to track "driver presence" (probe
succeeded) of the supplier device. If any other functionality provided
by device-links are needed, it is left to the consumer/supplier
devices to change the link when they probe.

v1 -> v2:
- Drop patch to speed up of_find_device_by_node()
- Drop depends-on property and use existing bindings

v2 -> v3:
- Refactor the code to have driver core initiate the linking of devs
- Have driver core link consumers to supplier before it's probed
- Add support for drivers to edit the device links before probing

v3 -> v4:
- Tested edit_links() on system with cyclic dependency. Works.
- Added some checks to make sure device link isn't attempted from
  parent device node to child device node.
- Added way to pause/resume sync_state callbacks across
  of_platform_populate().
- Recursively parse DT node to create device links from parent to
  suppliers of parent and all child nodes.

v4 -> v5:
- Fixed copy-pasta bugs with linked list handling
- Walk up the phandle reference till I find an actual device (needed
  for regulators to work)
- Added support for linking devices from regulator DT bindings
- Tested the whole series again to make sure cyclic dependencies are
  broken with edit_links() and regulator links are created properly.

v5 -> v6:
- Split, squashed and reordered some of the patches.
- Refactored the device linking code to follow the same code pattern for
  any property.

v6 -> v7:
- No functional changes.
- Renamed i to index
- Added comment to clarify not having to check property name for every
  index
- Added "matched" variable to clarify code. No functional change.
- Added comments to include/linux/device.h for add_links()

I've also not updated this patch series to handle the new patch [1] from
Rafael. Will do that once this patch series is close to being Acked.

[1] - https://lore.kernel.org/lkml/3121545.4lOhFoIcdQ@kreacher/

-Saravana


Saravana Kannan (7):
  driver core: Add support for linking devices during device addition
  driver core: Add edit_links() callback for drivers
  of/platform: Add functional dependency link from DT bindings
  driver core: Add sync_state driver/bus callback
  of/platform: Pause/resume sync state during init and
    of_platform_populate()
  of/platform: Create device links for all child-supplier depencencies
  of/platform: Don't create device links for default busses

 .../admin-guide/kernel-parameters.txt         |   5 +
 drivers/base/core.c                           | 168 ++++++++++++++++
 drivers/base/dd.c                             |  29 +++
 drivers/of/platform.c                         | 189 ++++++++++++++++++
 include/linux/device.h                        |  55 +++++
 5 files changed, 446 insertions(+)

-- 
2.22.0.709.g102302147b-goog

^ permalink raw reply

* Re: [PATCH v6 3/7] of/platform: Add functional dependency link from DT bindings
From: Saravana Kannan @ 2019-07-23 23:57 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, Greg Kroah-Hartman, Rafael J. Wysocki, Frank Rowand,
	Jonathan Corbet,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel@vger.kernel.org, David Collins, Android Kernel Team,
	Linux Doc Mailing List
In-Reply-To: <CAL_JsqJC-xvj5OJeFRPPNaYJj=-RqmXFJepZ5Q2+z36-7qyPgQ@mail.gmail.com>

On Tue, Jul 23, 2019 at 3:18 PM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Tue, Jul 23, 2019 at 2:49 PM Saravana Kannan <saravanak@google.com> wrote:
> >
> > On Tue, Jul 23, 2019 at 11:06 AM Rob Herring <robh+dt@kernel.org> wrote:
> > >
> > > On Sat, Jul 20, 2019 at 12:17 AM Saravana Kannan <saravanak@google.com> wrote:
> > > >
> > > > Add device-links after the devices are created (but before they are
> > > > probed) by looking at common DT bindings like clocks and
> > > > interconnects.
> > >
> > > The structure now looks a lot better to me. A few minor things below.
> >
> > Thanks.
> >
> > > >
> > > > Automatically adding device-links for functional dependencies at the
> > > > framework level provides the following benefits:
> > > >
> > > > - Optimizes device probe order and avoids the useless work of
> > > >   attempting probes of devices that will not probe successfully
> > > >   (because their suppliers aren't present or haven't probed yet).
> > > >
> > > >   For example, in a commonly available mobile SoC, registering just
> > > >   one consumer device's driver at an initcall level earlier than the
> > > >   supplier device's driver causes 11 failed probe attempts before the
> > > >   consumer device probes successfully. This was with a kernel with all
> > > >   the drivers statically compiled in. This problem gets a lot worse if
> > > >   all the drivers are loaded as modules without direct symbol
> > > >   dependencies.
> > > >
> > > > - Supplier devices like clock providers, interconnect providers, etc
> > > >   need to keep the resources they provide active and at a particular
> > > >   state(s) during boot up even if their current set of consumers don't
> > > >   request the resource to be active. This is because the rest of the
> > > >   consumers might not have probed yet and turning off the resource
> > > >   before all the consumers have probed could lead to a hang or
> > > >   undesired user experience.
> > > >
> > > >   Some frameworks (Eg: regulator) handle this today by turning off
> > > >   "unused" resources at late_initcall_sync and hoping all the devices
> > > >   have probed by then. This is not a valid assumption for systems with
> > > >   loadable modules. Other frameworks (Eg: clock) just don't handle
> > > >   this due to the lack of a clear signal for when they can turn off
> > > >   resources. This leads to downstream hacks to handle cases like this
> > > >   that can easily be solved in the upstream kernel.
> > > >
> > > >   By linking devices before they are probed, we give suppliers a clear
> > > >   count of the number of dependent consumers. Once all of the
> > > >   consumers are active, the suppliers can turn off the unused
> > > >   resources without making assumptions about the number of consumers.
> > > >
> > > > By default we just add device-links to track "driver presence" (probe
> > > > succeeded) of the supplier device. If any other functionality provided
> > > > by device-links are needed, it is left to the consumer/supplier
> > > > devices to change the link when they probe.
> > > >
> > > > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > > > ---
> > > >  .../admin-guide/kernel-parameters.txt         |   5 +
> > > >  drivers/of/platform.c                         | 158 ++++++++++++++++++
> > > >  2 files changed, 163 insertions(+)
> > > >
> > > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > > > index 138f6664b2e2..109b4310844f 100644
> > > > --- a/Documentation/admin-guide/kernel-parameters.txt
> > > > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > > > @@ -3141,6 +3141,11 @@
> > > >                         This can be set from sysctl after boot.
> > > >                         See Documentation/sysctl/vm.txt for details.
> > > >
> > > > +       of_devlink      [KNL] Make device links from common DT bindings. Useful
> > > > +                       for optimizing probe order and making sure resources
> > > > +                       aren't turned off before the consumer devices have
> > > > +                       probed.
> > > > +
> > > >         ohci1394_dma=early      [HW] enable debugging via the ohci1394 driver.
> > > >                         See Documentation/debugging-via-ohci1394.txt for more
> > > >                         info.
> > > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > > > index 04ad312fd85b..88a2086e26fa 100644
> > > > --- a/drivers/of/platform.c
> > > > +++ b/drivers/of/platform.c
> > > > @@ -509,6 +509,163 @@ int of_platform_default_populate(struct device_node *root,
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(of_platform_default_populate);
> > > >
> > > > +bool of_link_is_valid(struct device_node *con, struct device_node *sup)
> > > > +{
> > > > +       of_node_get(sup);
> > > > +       /*
> > > > +        * Don't allow linking a device node as a consumer of one of its
> > > > +        * descendant nodes. By definition, a child node can't be a functional
> > > > +        * dependency for the parent node.
> > > > +        */
> > > > +       while (sup) {
> > > > +               if (sup == con) {
> > > > +                       of_node_put(sup);
> > > > +                       return false;
> > > > +               }
> > > > +               sup = of_get_next_parent(sup);
> > > > +       }
> > > > +       return true;
> > > > +}
> > > > +
> > > > +static int of_link_to_phandle(struct device *dev, struct device_node *sup_np)
> > > > +{
> > > > +       struct platform_device *sup_dev;
> > > > +       u32 dl_flags = DL_FLAG_AUTOPROBE_CONSUMER;
> > > > +       int ret = 0;
> > > > +
> > > > +       /*
> > > > +        * Since we are trying to create device links, we need to find
> > > > +        * the actual device node that owns this supplier phandle.
> > > > +        * Often times it's the same node, but sometimes it can be one
> > > > +        * of the parents. So walk up the parent till you find a
> > > > +        * device.
> > > > +        */
> > > > +       while (sup_np && !of_find_property(sup_np, "compatible", NULL))
> > > > +               sup_np = of_get_next_parent(sup_np);
> > > > +       if (!sup_np)
> > > > +               return 0;
> > > > +
> > > > +       if (!of_link_is_valid(dev->of_node, sup_np)) {
> > > > +               of_node_put(sup_np);
> > > > +               return 0;
> > > > +       }
> > > > +       sup_dev = of_find_device_by_node(sup_np);
> > > > +       of_node_put(sup_np);
> > > > +       if (!sup_dev)
> > > > +               return -ENODEV;
> > > > +       if (!device_link_add(dev, &sup_dev->dev, dl_flags))
> > > > +               ret = -ENODEV;
> > > > +       put_device(&sup_dev->dev);
> > > > +       return ret;
> > > > +}
> > > > +
> > > > +static struct device_node *parse_prop_cells(struct device_node *np,
> > > > +                                           const char *prop, int i,
> > >
> > > I like 'i' for for loops, but less so for function params. Perhaps
> > > 'index' instead like of_parse_phandle_with_args.
> >
> > Sounds good.
> >
> > >
> > > > +                                           const char *binding,
> > > > +                                           const char *cell)
> > > > +{
> > > > +       struct of_phandle_args sup_args;
> > > > +
> > > > +       if (!i && strcmp(prop, binding))
> > >
> > > Why the '!i' test?
> >
> > To avoid a string comparison for every index. It's kinda wasteful once
> > the first index passes.
>
> That's not very obvious and pretty fragile though this is a static
> function. Perhaps we should split to match() and parse() functions.

Yeah, I did think about doing this. That's why I made it a struct for
supplier_bindings instead of just an array of function pointers. But
having a parse function just for a strcmp() was creating a lot of code
noise. So went ahead and did it this way. We can keep it this way and
if we later see the need for a separate parse function, it should be
easy to do so (because it's already a struct for each binding).

> At
> least put a comment here as to what we're doing.

Done.

> >
> > > > +               return NULL;
> > > > +
> > > > +       if (of_parse_phandle_with_args(np, binding, cell, i, &sup_args))
> > > > +               return NULL;
> > > > +
> > > > +       return sup_args.np;
> > > > +}
> > > > +
> > > > +static struct device_node *parse_clocks(struct device_node *np,
> > > > +                                       const char *prop, int i)
> > > > +{
> > > > +       return parse_prop_cells(np, prop, i, "clocks", "#clock-cells");
> > > > +}
> > > > +
> > > > +static struct device_node *parse_interconnects(struct device_node *np,
> > > > +                                              const char *prop, int i)
> > > > +{
> > > > +       return parse_prop_cells(np, prop, i, "interconnects",
> > > > +                               "#interconnect-cells");
> > > > +}
> > > > +
> > > > +static int strcmp_suffix(const char *str, const char *suffix)
> > > > +{
> > > > +       unsigned int len, suffix_len;
> > > > +
> > > > +       len = strlen(str);
> > > > +       suffix_len = strlen(suffix);
> > > > +       if (len <= suffix_len)
> > > > +               return -1;
> > > > +       return strcmp(str + len - suffix_len, suffix);
> > > > +}
> > > > +
> > > > +static struct device_node *parse_regulators(struct device_node *np,
> > > > +                                           const char *prop, int i)
> > > > +{
> > > > +       if (i || strcmp_suffix(prop, "-supply"))
> > > > +               return NULL;
> > > > +
> > > > +       return of_parse_phandle(np, prop, 0);
> > > > +}
> > > > +
> > > > +/**
> > > > + * struct supplier_bindings - Information for parsing supplier DT binding
> > > > + *
> > > > + * @parse_prop:                If the function cannot parse the property, return NULL.
> > > > + *                     Otherwise, return the phandle listed in the property
> > > > + *                     that corresponds to index i.
> > > > + */
> > > > +struct supplier_bindings {
> > > > +       struct device_node *(*parse_prop)(struct device_node *np,
> > > > +                                         const char *name, int i);
> > > > +};
> > > > +
> > > > +struct supplier_bindings bindings[] = {
> > >
> > > static const
> >
> > Will do.
> >
> > >
> > > > +       { .parse_prop = parse_clocks, },
> > > > +       { .parse_prop = parse_interconnects, },
> > > > +       { .parse_prop = parse_regulators, },
> > > > +       { },
> > > > +};
> > > > +
> > > > +static bool of_link_property(struct device *dev, struct device_node *con_np,
> > > > +                            const char *prop)
> > > > +{
> > > > +       struct device_node *phandle;
> > > > +       struct supplier_bindings *s = bindings;
> > > > +       unsigned int i = 0;
> > > > +       bool done = true;
> > > > +
> > > > +       while (!i && s->parse_prop) {
> > >
> > > Using 'i' is a little odd. Perhaps a 'matched' bool would be easier to read.
> >
> > That's how I wrote it first (locally) and then redid it this way
> > because the bool felt very superfluous. I don't think this is that
> > hard to understand.
>
> Alright...

I like the name "matched" over "found" that I had used locally. So, I
actually went ahead and did this.

-Saravana

> > > > +               while ((phandle = s->parse_prop(con_np, prop, i))) {
> > > > +                       i++;
> > > > +                       if (of_link_to_phandle(dev, phandle))
> > > > +                               done = false;
> > >
> > > Just return here. No point in continuing as 'done' is never set back to true.
> >
> > Actually, there is a point for this. Say Device-C depends on suppliers
> > Device-S1 and Device-S2 and they are listed in DT in that order.
> >
> > Say, S1 gets populated after late_initcall_sync but S2 is probes way
> > before that. If I don't continue past a "failed linking" to S1 and
> > also link up to S2, then S2 will get a sync_state() callback before C
> > is probed. So I have to go through all possible suppliers and as many
> > as possible.
> >
> > Let me add a comment about this somewhere in the code (probably the
> > header that defines the add_links() ops).
>
> Okay, makes sense.
>
> Rob

^ permalink raw reply

* Re: [PATCH V6 16/21] soc/tegra: pmc: Add pmc wake support for tegra210
From: Sowjanya Komatineni @ 2019-07-23 23:39 UTC (permalink / raw)
  To: Dmitry Osipenko, thierry.reding, jonathanh, tglx, jason,
	marc.zyngier, linus.walleij, stefan, mark.rutland
  Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
	josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
	robh+dt, devicetree
In-Reply-To: <a545cc66-45cd-504a-4390-8274b8b79540@gmail.com>


On 7/23/19 7:27 AM, Dmitry Osipenko wrote:
> 23.07.2019 6:43, Dmitry Osipenko пишет:
>> 23.07.2019 6:31, Sowjanya Komatineni пишет:
>>> On 7/22/19 8:25 PM, Dmitry Osipenko wrote:
>>>> 23.07.2019 6:09, Sowjanya Komatineni пишет:
>>>>> On 7/22/19 8:03 PM, Dmitry Osipenko wrote:
>>>>>> 23.07.2019 4:52, Sowjanya Komatineni пишет:
>>>>>>> On 7/22/19 6:41 PM, Dmitry Osipenko wrote:
>>>>>>>> 23.07.2019 4:08, Dmitry Osipenko пишет:
>>>>>>>>> 23.07.2019 3:58, Dmitry Osipenko пишет:
>>>>>>>>>> 21.07.2019 22:40, Sowjanya Komatineni пишет:
>>>>>>>>>>> This patch implements PMC wakeup sequence for Tegra210 and defines
>>>>>>>>>>> common used RTC alarm wake event.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>>>>>>> ---
>>>>>>>>>>>     drivers/soc/tegra/pmc.c | 111
>>>>>>>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>>>>>>     1 file changed, 111 insertions(+)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>>>>>>>>>>> index 91c84d0e66ae..c556f38874e1 100644
>>>>>>>>>>> --- a/drivers/soc/tegra/pmc.c
>>>>>>>>>>> +++ b/drivers/soc/tegra/pmc.c
>>>>>>>>>>> @@ -57,6 +57,12 @@
>>>>>>>>>>>     #define  PMC_CNTRL_SYSCLK_OE        BIT(11) /* system clock
>>>>>>>>>>> enable */
>>>>>>>>>>>     #define  PMC_CNTRL_SYSCLK_POLARITY    BIT(10) /* sys clk
>>>>>>>>>>> polarity */
>>>>>>>>>>>     #define  PMC_CNTRL_MAIN_RST        BIT(4)
>>>>>>>>>>> +#define  PMC_CNTRL_LATCH_WAKEUPS    BIT(5)
>>>>>>>>> Please follow the TRM's bits naming.
>>>>>>>>>
>>>>>>>>> PMC_CNTRL_LATCHWAKE_EN
>>>>>>>>>
>>>>>>>>>>> +#define PMC_WAKE_MASK            0x0c
>>>>>>>>>>> +#define PMC_WAKE_LEVEL            0x10
>>>>>>>>>>> +#define PMC_WAKE_STATUS            0x14
>>>>>>>>>>> +#define PMC_SW_WAKE_STATUS        0x18
>>>>>>>>>>>       #define DPD_SAMPLE            0x020
>>>>>>>>>>>     #define  DPD_SAMPLE_ENABLE        BIT(0)
>>>>>>>>>>> @@ -87,6 +93,11 @@
>>>>>>>>>>>       #define PMC_SCRATCH41            0x140
>>>>>>>>>>>     +#define PMC_WAKE2_MASK            0x160
>>>>>>>>>>> +#define PMC_WAKE2_LEVEL            0x164
>>>>>>>>>>> +#define PMC_WAKE2_STATUS        0x168
>>>>>>>>>>> +#define PMC_SW_WAKE2_STATUS        0x16c
>>>>>>>>>>> +
>>>>>>>>>>>     #define PMC_SENSOR_CTRL            0x1b0
>>>>>>>>>>>     #define  PMC_SENSOR_CTRL_SCRATCH_WRITE    BIT(2)
>>>>>>>>>>>     #define  PMC_SENSOR_CTRL_ENABLE_RST    BIT(1)
>>>>>>>>>>> @@ -1922,6 +1933,55 @@ static const struct irq_domain_ops
>>>>>>>>>>> tegra_pmc_irq_domain_ops = {
>>>>>>>>>>>         .alloc = tegra_pmc_irq_alloc,
>>>>>>>>>>>     };
>>>>>>>>>>>     +static int tegra210_pmc_irq_set_wake(struct irq_data *data,
>>>>>>>>>>> unsigned int on)
>>>>>>>>>>> +{
>>>>>>>>>>> +    struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>>>>>>>>>>> +    unsigned int offset, bit;
>>>>>>>>>>> +    u32 value;
>>>>>>>>>>> +
>>>>>>>>>>> +    if (data->hwirq == ULONG_MAX)
>>>>>>>>>>> +        return 0;
>>>>>>>>>>> +
>>>>>>>>>>> +    offset = data->hwirq / 32;
>>>>>>>>>>> +    bit = data->hwirq % 32;
>>>>>>>>>>> +
>>>>>>>>>>> +    /*
>>>>>>>>>>> +     * Latch wakeups to SW_WAKE_STATUS register to capture events
>>>>>>>>>>> +     * that would not make it into wakeup event register during
>>>>>>>>>>> LP0 exit.
>>>>>>>>>>> +     */
>>>>>>>>>>> +    value = tegra_pmc_readl(pmc, PMC_CNTRL);
>>>>>>>>>>> +    value |= PMC_CNTRL_LATCH_WAKEUPS;
>>>>>>>>>>> +    tegra_pmc_writel(pmc, value, PMC_CNTRL);
>>>>>>>>>>> +    udelay(120);
>>>>>>>>>> Why it takes so much time to latch the values? Shouldn't some
>>>>>>>>>> status-bit
>>>>>>>>>> be polled for the completion of latching?
>>>>>>>>>>
>>>>>>>>>> Is this register-write really getting buffered in the PMC?
>>>>>>>>>>
>>>>>>>>>>> +    value &= ~PMC_CNTRL_LATCH_WAKEUPS;
>>>>>>>>>>> +    tegra_pmc_writel(pmc, value, PMC_CNTRL);
>>>>>>>>>>> +    udelay(120);
>>>>>>>>>> 120 usecs to remove latching, really?
>>>>>>>>>>
>>>>>>>>>>> +    tegra_pmc_writel(pmc, 0, PMC_SW_WAKE_STATUS);
>>>>>>>>>>> +    tegra_pmc_writel(pmc, 0, PMC_SW_WAKE2_STATUS);
>>>>>>>>>>> +
>>>>>>>>>>> +    tegra_pmc_writel(pmc, 0, PMC_WAKE_STATUS);
>>>>>>>>>>> +    tegra_pmc_writel(pmc, 0, PMC_WAKE2_STATUS);
>>>>>>>>>>> +
>>>>>>>>>>> +    /* enable PMC wake */
>>>>>>>>>>> +    if (data->hwirq >= 32)
>>>>>>>>>>> +        offset = PMC_WAKE2_MASK;
>>>>>>>>>>> +    else
>>>>>>>>>>> +        offset = PMC_WAKE_MASK;
>>>>>>>>>>> +
>>>>>>>>>>> +    value = tegra_pmc_readl(pmc, offset);
>>>>>>>>>>> +
>>>>>>>>>>> +    if (on)
>>>>>>>>>>> +        value |= 1 << bit;
>>>>>>>>>>> +    else
>>>>>>>>>>> +        value &= ~(1 << bit);
>>>>>>>>>>> +
>>>>>>>>>>> +    tegra_pmc_writel(pmc, value, offset);
>>>>>>>>>> Why the latching is done *before* writing into the WAKE registers?
>>>>>>>>>> What
>>>>>>>>>> it is latching then?
>>>>>>>>> I'm looking at the TRM doc and it says that latching should be done
>>>>>>>>> *after* writing to the WAKE_MASK / LEVEL registers.
>>>>>>>>>
>>>>>>>>> Secondly it says that it's enough to do:
>>>>>>>>>
>>>>>>>>> value = tegra_pmc_readl(pmc, PMC_CNTRL);
>>>>>>>>> value |= PMC_CNTRL_LATCH_WAKEUPS;
>>>>>>>>> tegra_pmc_writel(pmc, value, PMC_CNTRL);
>>>>>>>>>
>>>>>>>>> in order to latch. There is no need for the delay and to remove the
>>>>>>>>> "LATCHWAKE_EN" bit, it should be a oneshot action.
>>>>>>>> Although, no. TRM says "stops latching on transition from 1
>>>>>>>> to 0 (sequence - set to 1,set to 0)", so it's not a oneshot action.
>>>>>>>>
>>>>>>>> Have you tested this code at all? I'm wondering how it happens to
>>>>>>>> work
>>>>>>>> without a proper latching.
>>>>>>> Yes, ofcourse its tested and this sequence to do transition is
>>>>>>> recommendation from Tegra designer.
>>>>>>> Will check if TRM doesn't have update properly or will re-confirm
>>>>>>> internally on delay time...
>>>>>>>
>>>>>>> On any of the wake event PMC wakeup happens and WAKE_STATUS register
>>>>>>> will have bits set for all events that triggered wake.
>>>>>>> After wakeup PMC doesn't update SW_WAKE_STATUS register as per PMC
>>>>>>> design.
>>>>>>> SW latch register added in design helps to provide a way to capture
>>>>>>> those events that happen right during wakeup time and didnt make it to
>>>>>>> SW_WAKE_STATUS register.
>>>>>>> So before next suspend entry, latching all prior wake events into SW
>>>>>>> WAKE_STATUS and then clearing them.
>>>>>> I'm now wondering whether the latching cold be turned ON permanently
>>>>>> during of the PMC's probe, for simplicity.
>>>>> latching should be done on suspend-resume cycle as wake events gets
>>>>> generates on every suspend-resume cycle.
>>>> You're saying that PMC "doesn't update SW_WAKE_STATUS" after wake-up,
>>>> then I don't quite understand what's the point of disabling the latching
>>>> at all.
>>> When latch wake enable is set, events are latched and during 1 to 0
>>> transition latching is disabled.
>>>
>>> This is to avoid sw_wake_status and wake_status showing diff events.
>> Okay.
>>
>>> Currently driver is not relying on SW_WAKE_STATUS but its good to latch
>>> and clear so even at some point for some reason when SW_WAKE_STATUS is
>>> used, this wlil not cause mismatch with wake_status.
>> Then the latching need to be enabled on suspend and disabled early on
>> resume to get a proper WAKE status.
> Actually, it will be better to simply not implement the latching until
> it will become really needed. In general you shouldn't add into the
> patchset anything that is unused.

OK, will remove latch_wake for now.

Will send next version once I get all the review feedback ..

^ permalink raw reply

* [PATCH] dt-bindings: clk: allwinner, sun4i-a10-ccu: Correct path in $id
From: Rob Herring @ 2019-07-23 23:08 UTC (permalink / raw)
  To: devicetree
  Cc: Stephen Boyd, Michael Turquette, Maxime Ripard, Chen-Yu Tsai,
	linux-clk, linux-arm-kernel

The path in the schema '$id' value is wrong. Fix it.

Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@kernel.org>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: linux-clk@vger.kernel.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
I can take this via the DT tree.

Rob

 .../devicetree/bindings/clock/allwinner,sun4i-a10-ccu.yaml      | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/clock/allwinner,sun4i-a10-ccu.yaml b/Documentation/devicetree/bindings/clock/allwinner,sun4i-a10-ccu.yaml
index c935405458fe..fa4d143a73de 100644
--- a/Documentation/devicetree/bindings/clock/allwinner,sun4i-a10-ccu.yaml
+++ b/Documentation/devicetree/bindings/clock/allwinner,sun4i-a10-ccu.yaml
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 %YAML 1.2
 ---
-$id: http://devicetree.org/schemas/phy/allwinner,sun4i-a10-ccu.yaml#
+$id: http://devicetree.org/schemas/clock/allwinner,sun4i-a10-ccu.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
 title: Allwinner Clock Control Unit Device Tree Bindings
-- 
2.20.1

^ permalink raw reply related

* [PATCH] ARM: dts: rockchip: Limit WiFi TX power on rk3288-veyron-jerry
From: Matthias Kaehlcke @ 2019-07-23 22:52 UTC (permalink / raw)
  To: Heiko Stuebner, Rob Herring, Mark Rutland
  Cc: linux-arm-kernel, linux-rockchip, devicetree, linux-kernel,
	Douglas Anderson, Matthias Kaehlcke

The downstream Chrome OS 3.14 kernel for jerry limits WiFi TX power
through calibration data in the device tree [1]. Add a DT node for
the WiFi chip and use the downstream calibration data.

Not all calibration data entries have the length specified in the
binding (Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt),
however this is the data used by the downstream ('official') kernel
and the binding mentions that "the length can vary between hw
versions".

[1] https://crrev.com/c/271237

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 arch/arm/boot/dts/rk3288-veyron-jerry.dts | 147 ++++++++++++++++++++++
 1 file changed, 147 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288-veyron-jerry.dts b/arch/arm/boot/dts/rk3288-veyron-jerry.dts
index b1613af83d5d..2d0d5a4603ba 100644
--- a/arch/arm/boot/dts/rk3288-veyron-jerry.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-jerry.dts
@@ -82,6 +82,153 @@
 	};
 };
 
+&sdio0 {
+	mwifiex: wifi@1 {
+		compatible = "marvell,sd8897";
+		reg = <1>;
+		status = "okay";
+
+		marvell,caldata-txpwrlimit-2g = /bits/ 8 <
+0x01 0x00 0x06 0x00 0x08 0x02 0x89 0x01
+0x24 0x00 0x67 0x09 0x14 0x01 0x00 0x0f 0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0c
+0x05 0x0c 0x06 0x0c 0x07 0x09 0x08 0x09 0x09 0x09 0x0a 0x0c 0x0b 0x0c 0x0c 0x0c
+0x0d 0x09 0x0e 0x09 0x0f 0x09 0x89 0x01 0x24 0x00 0x67 0x09 0x14 0x02 0x00 0x0f
+0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0c 0x05 0x0c 0x06 0x0c 0x07 0x09 0x08 0x09
+0x09 0x09 0x0a 0x0c 0x0b 0x0c 0x0c 0x0c 0x0d 0x09 0x0e 0x09 0x0f 0x09 0x89 0x01
+0x24 0x00 0x67 0x09 0x14 0x03 0x00 0x0f 0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0c
+0x05 0x0c 0x06 0x0c 0x07 0x09 0x08 0x09 0x09 0x09 0x0a 0x0c 0x0b 0x0c 0x0c 0x0c
+0x0d 0x09 0x0e 0x09 0x0f 0x09 0x89 0x01 0x24 0x00 0x67 0x09 0x14 0x04 0x00 0x0f
+0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0c 0x05 0x0c 0x06 0x0c 0x07 0x09 0x08 0x09
+0x09 0x09 0x0a 0x0c 0x0b 0x0c 0x0c 0x0c 0x0d 0x09 0x0e 0x09 0x0f 0x09 0x89 0x01
+0x24 0x00 0x67 0x09 0x14 0x05 0x00 0x0f 0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0c
+0x05 0x0c 0x06 0x0c 0x07 0x09 0x08 0x09 0x09 0x09 0x0a 0x0c 0x0b 0x0c 0x0c 0x0c
+0x0d 0x09 0x0e 0x09 0x0f 0x09 0x89 0x01 0x24 0x00 0x67 0x09 0x14 0x06 0x00 0x0f
+0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0c 0x05 0x0c 0x06 0x0c 0x07 0x09 0x08 0x09
+0x09 0x09 0x0a 0x0c 0x0b 0x0c 0x0c 0x0c 0x0d 0x09 0x0e 0x09 0x0f 0x09 0x89 0x01
+0x24 0x00 0x67 0x09 0x14 0x07 0x00 0x0f 0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0c
+0x05 0x0c 0x06 0x0c 0x07 0x09 0x08 0x09 0x09 0x09 0x0a 0x0c 0x0b 0x0c 0x0c 0x0c
+0x0d 0x09 0x0e 0x09 0x0f 0x09 0x89 0x01 0x24 0x00 0x67 0x09 0x14 0x08 0x00 0x0f
+0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0c 0x05 0x0c 0x06 0x0c 0x07 0x09 0x08 0x09
+0x09 0x09 0x0a 0x0c 0x0b 0x0c 0x0c 0x0c 0x0d 0x09 0x0e 0x09 0x0f 0x09 0x89 0x01
+0x24 0x00 0x67 0x09 0x14 0x09 0x00 0x0f 0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0c
+0x05 0x0c 0x06 0x0c 0x07 0x09 0x08 0x09 0x09 0x09 0x0a 0x0c 0x0b 0x0c 0x0c 0x0c
+0x0d 0x09 0x0e 0x09 0x0f 0x09 0x89 0x01 0x24 0x00 0x67 0x09 0x14 0x0a 0x00 0x0f
+0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0c 0x05 0x0c 0x06 0x0c 0x07 0x09 0x08 0x09
+0x09 0x09 0x0a 0x0c 0x0b 0x0c 0x0c 0x0c 0x0d 0x09 0x0e 0x09 0x0f 0x09 0x89 0x01
+0x24 0x00 0x67 0x09 0x14 0x0b 0x00 0x0f 0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0c
+0x05 0x0c 0x06 0x0c 0x07 0x09 0x08 0x09 0x09 0x09 0x0a 0x0c 0x0b 0x0c 0x0c 0x0c
+0x0d 0x09 0x0e 0x09 0x0f 0x09 0x89 0x01 0x24 0x00 0x67 0x09 0x14 0x0c 0x00 0x0f
+0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0c 0x05 0x0c 0x06 0x0c 0x07 0x09 0x08 0x09
+0x09 0x09 0x0a 0x0c 0x0b 0x0c 0x0c 0x0c 0x0d 0x09 0x0e 0x09 0x0f 0x09 0x89 0x01
+0x24 0x00 0x67 0x09 0x14 0x0d 0x00 0x0f 0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0c
+0x05 0x0c 0x06 0x0c 0x07 0x09 0x08 0x09 0x09 0x09 0x0a 0x0c 0x0b 0x0c 0x0c 0x0c
+0x0d 0x09 0x0e 0x09 0x0f 0x09>;
+
+		marvell,caldata-txpwrlimit-5g-sub0 = /bits/ 8 <
+0x01 0x00 0x06 0x00 0xf0 0x01 0x89 0x01
+0x3a 0x00 0x88 0x13 0x14 0x24 0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0a 0x05 0x0a
+0x06 0x0a 0x07 0x09 0x08 0x09 0x09 0x09 0x0a 0x0a 0x0b 0x0a 0x0c 0x0a 0x0d 0x09
+0x0e 0x09 0x0f 0x09 0x10 0x05 0x11 0x05 0x12 0x05 0x13 0x05 0x14 0x05 0x15 0x05
+0x16 0x05 0x17 0x05 0x18 0x05 0x19 0x05 0x1a 0x05 0x1b 0x05 0x89 0x01 0x3a 0x00
+0x88 0x13 0x14 0x28 0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0a 0x05 0x0a 0x06 0x0a
+0x07 0x09 0x08 0x09 0x09 0x09 0x0a 0x0a 0x0b 0x0a 0x0c 0x0a 0x0d 0x09 0x0e 0x09
+0x0f 0x09 0x10 0x05 0x11 0x05 0x12 0x05 0x13 0x05 0x14 0x05 0x15 0x05 0x16 0x05
+0x17 0x05 0x18 0x05 0x19 0x05 0x1a 0x05 0x1b 0x05 0x89 0x01 0x3a 0x00 0x88 0x13
+0x14 0x2c 0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0a 0x05 0x0a 0x06 0x0a 0x07 0x09
+0x08 0x09 0x09 0x09 0x0a 0x0a 0x0b 0x0a 0x0c 0x0a 0x0d 0x09 0x0e 0x09 0x0f 0x09
+0x10 0x05 0x11 0x05 0x12 0x05 0x13 0x05 0x14 0x05 0x15 0x05 0x16 0x05 0x17 0x05
+0x18 0x05 0x19 0x05 0x1a 0x05 0x1b 0x05 0x89 0x01 0x3a 0x00 0x88 0x13 0x14 0x30
+0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0a 0x05 0x0a 0x06 0x0a 0x07 0x09 0x08 0x09
+0x09 0x09 0x0a 0x0a 0x0b 0x0a 0x0c 0x0a 0x0d 0x09 0x0e 0x09 0x0f 0x09 0x10 0x05
+0x11 0x05 0x12 0x05 0x13 0x05 0x14 0x05 0x15 0x05 0x16 0x05 0x17 0x05 0x18 0x05
+0x19 0x05 0x1a 0x05 0x1b 0x05 0x89 0x01 0x3a 0x00 0x88 0x13 0x14 0x34 0x01 0x0c
+0x02 0x0c 0x03 0x0c 0x04 0x0a 0x05 0x0a 0x06 0x0a 0x07 0x09 0x08 0x09 0x09 0x09
+0x0a 0x0a 0x0b 0x0a 0x0c 0x0a 0x0d 0x09 0x0e 0x09 0x0f 0x09 0x10 0x05 0x11 0x05
+0x12 0x05 0x13 0x05 0x14 0x05 0x15 0x05 0x16 0x05 0x17 0x05 0x18 0x05 0x19 0x05
+0x1a 0x05 0x1b 0x05 0x89 0x01 0x3a 0x00 0x88 0x13 0x14 0x38 0x01 0x0c 0x02 0x0c
+0x03 0x0c 0x04 0x0a 0x05 0x0a 0x06 0x0a 0x07 0x09 0x08 0x09 0x09 0x09 0x0a 0x0a
+0x0b 0x0a 0x0c 0x0a 0x0d 0x09 0x0e 0x09 0x0f 0x09 0x10 0x05 0x11 0x05 0x12 0x05
+0x13 0x05 0x14 0x05 0x15 0x05 0x16 0x05 0x17 0x05 0x18 0x05 0x19 0x05 0x1a 0x05
+0x1b 0x05 0x89 0x01 0x3a 0x00 0x88 0x13 0x14 0x3c 0x01 0x0c 0x02 0x0c 0x03 0x0c
+0x04 0x0a 0x05 0x0a 0x06 0x0a 0x07 0x09 0x08 0x09 0x09 0x09 0x0a 0x0a 0x0b 0x0a
+0x0c 0x0a 0x0d 0x09 0x0e 0x09 0x0f 0x09 0x10 0x05 0x11 0x05 0x12 0x05 0x13 0x05
+0x14 0x05 0x15 0x05 0x16 0x05 0x17 0x05 0x18 0x05 0x19 0x05 0x1a 0x05 0x1b 0x05
+0x89 0x01 0x3a 0x00 0x88 0x13 0x14 0x40 0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0a
+0x05 0x0a 0x06 0x0a 0x07 0x09 0x08 0x09 0x09 0x09 0x0a 0x0a 0x0b 0x0a 0x0c 0x0a
+0x0d 0x09 0x0e 0x09 0x0f 0x09 0x10 0x05 0x11 0x05 0x12 0x05 0x13 0x05 0x14 0x05
+0x15 0x05 0x16 0x05 0x17 0x05 0x18 0x05 0x19 0x05 0x1a 0x05 0x1b 0x05>;
+
+		marvell,caldata-txpwrlimit-5g-sub1 = /bits/ 8 <
+0x01 0x00 0x06 0x00 0xaa 0x02 0x89 0x01
+0x3a 0x00 0x88 0x13 0x14 0x64 0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0a 0x05 0x0a
+0x06 0x0a 0x07 0x09 0x08 0x09 0x09 0x09 0x0a 0x0a 0x0b 0x0a 0x0c 0x0a 0x0d 0x09
+0x0e 0x09 0x0f 0x09 0x10 0x05 0x11 0x05 0x12 0x05 0x13 0x05 0x14 0x05 0x15 0x05
+0x16 0x05 0x17 0x05 0x18 0x05 0x19 0x05 0x1a 0x05 0x1b 0x05 0x89 0x01 0x3a 0x00
+0x88 0x13 0x14 0x68 0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0a 0x05 0x0a 0x06 0x0a
+0x07 0x09 0x08 0x09 0x09 0x09 0x0a 0x0a 0x0b 0x0a 0x0c 0x0a 0x0d 0x09 0x0e 0x09
+0x0f 0x09 0x10 0x05 0x11 0x05 0x12 0x05 0x13 0x05 0x14 0x05 0x15 0x05 0x16 0x05
+0x17 0x05 0x18 0x05 0x19 0x05 0x1a 0x05 0x1b 0x05 0x89 0x01 0x3a 0x00 0x88 0x13
+0x14 0x6c 0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0a 0x05 0x0a 0x06 0x0a 0x07 0x09
+0x08 0x09 0x09 0x09 0x0a 0x0a 0x0b 0x0a 0x0c 0x0a 0x0d 0x09 0x0e 0x09 0x0f 0x09
+0x10 0x05 0x11 0x05 0x12 0x05 0x13 0x05 0x14 0x05 0x15 0x05 0x16 0x05 0x17 0x05
+0x18 0x05 0x19 0x05 0x1a 0x05 0x1b 0x05 0x89 0x01 0x3a 0x00 0x88 0x13 0x14 0x70
+0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0a 0x05 0x0a 0x06 0x0a 0x07 0x09 0x08 0x09
+0x09 0x09 0x0a 0x0a 0x0b 0x0a 0x0c 0x0a 0x0d 0x09 0x0e 0x09 0x0f 0x09 0x10 0x05
+0x11 0x05 0x12 0x05 0x13 0x05 0x14 0x05 0x15 0x05 0x16 0x05 0x17 0x05 0x18 0x05
+0x19 0x05 0x1a 0x05 0x1b 0x05 0x89 0x01 0x3a 0x00 0x88 0x13 0x14 0x74 0x01 0x0c
+0x02 0x0c 0x03 0x0c 0x04 0x0a 0x05 0x0a 0x06 0x0a 0x07 0x09 0x08 0x09 0x09 0x09
+0x0a 0x0a 0x0b 0x0a 0x0c 0x0a 0x0d 0x09 0x0e 0x09 0x0f 0x09 0x10 0x05 0x11 0x05
+0x12 0x05 0x13 0x05 0x14 0x05 0x15 0x05 0x16 0x05 0x17 0x05 0x18 0x05 0x19 0x05
+0x1a 0x05 0x1b 0x05 0x89 0x01 0x3a 0x00 0x88 0x13 0x14 0x78 0x01 0x0c 0x02 0x0c
+0x03 0x0c 0x04 0x0a 0x05 0x0a 0x06 0x0a 0x07 0x09 0x08 0x09 0x09 0x09 0x0a 0x0a
+0x0b 0x0a 0x0c 0x0a 0x0d 0x09 0x0e 0x09 0x0f 0x09 0x10 0x05 0x11 0x05 0x12 0x05
+0x13 0x05 0x14 0x05 0x15 0x05 0x16 0x05 0x17 0x05 0x18 0x05 0x19 0x05 0x1a 0x05
+0x1b 0x05 0x89 0x01 0x3a 0x00 0x88 0x13 0x14 0x7c 0x01 0x0c 0x02 0x0c 0x03 0x0c
+0x04 0x0a 0x05 0x0a 0x06 0x0a 0x07 0x09 0x08 0x09 0x09 0x09 0x0a 0x0a 0x0b 0x0a
+0x0c 0x0a 0x0d 0x09 0x0e 0x09 0x0f 0x09 0x10 0x05 0x11 0x05 0x12 0x05 0x13 0x05
+0x14 0x05 0x15 0x05 0x16 0x05 0x17 0x05 0x18 0x05 0x19 0x05 0x1a 0x05 0x1b 0x05
+0x89 0x01 0x3a 0x00 0x88 0x13 0x14 0x80 0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0a
+0x05 0x0a 0x06 0x0a 0x07 0x09 0x08 0x09 0x09 0x09 0x0a 0x0a 0x0b 0x0a 0x0c 0x0a
+0x0d 0x09 0x0e 0x09 0x0f 0x09 0x10 0x05 0x11 0x05 0x12 0x05 0x13 0x05 0x14 0x05
+0x15 0x05 0x16 0x05 0x17 0x05 0x18 0x05 0x19 0x05 0x1a 0x05 0x1b 0x05 0x89 0x01
+0x3a 0x00 0x88 0x13 0x14 0x84 0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0a 0x05 0x0a
+0x06 0x0a 0x07 0x09 0x08 0x09 0x09 0x09 0x0a 0x0a 0x0b 0x0a 0x0c 0x0a 0x0d 0x09
+0x0e 0x09 0x0f 0x09 0x10 0x05 0x11 0x05 0x12 0x05 0x13 0x05 0x14 0x05 0x15 0x05
+0x16 0x05 0x17 0x05 0x18 0x05 0x19 0x05 0x1a 0x05 0x1b 0x05 0x89 0x01 0x3a 0x00
+0x88 0x13 0x14 0x88 0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0a 0x05 0x0a 0x06 0x0a
+0x07 0x08 0x08 0x08 0x09 0x08 0x0a 0x0a 0x0b 0x0a 0x0c 0x0a 0x0d 0x08 0x0e 0x08
+0x0f 0x08 0x10 0x04 0x11 0x04 0x12 0x05 0x13 0x05 0x14 0x05 0x15 0x05 0x16 0x04
+0x17 0x04 0x18 0x05 0x19 0x05 0x1a 0x05 0x1b 0x05 0x89 0x01 0x3a 0x00 0x88 0x13
+0x14 0x8c 0x01 0x0c 0x02 0x0c 0x03 0x0c 0x04 0x0a 0x05 0x0a 0x06 0x0a 0x07 0x08
+0x08 0x08 0x09 0x08 0x0a 0x0a 0x0b 0x0a 0x0c 0x0a 0x0d 0x08 0x0e 0x08 0x0f 0x08
+0x10 0x04 0x11 0x04 0x12 0x05 0x13 0x05 0x14 0x05 0x15 0x05 0x16 0x04 0x17 0x04
+0x18 0x05 0x19 0x05 0x1a 0x05 0x1b 0x05>;
+
+		marvell,caldata-txpwrlimit-5g-sub2 = /bits/ 8 <
+0x01 0x00 0x06 0x00 0x36 0x01 0x89 0x01
+0x3a 0x00 0x88 0x13 0x14 0x95 0x01 0x0b 0x02 0x0b 0x03 0x0b 0x04 0x0a 0x05 0x0a
+0x06 0x0a 0x07 0x08 0x08 0x08 0x09 0x08 0x0a 0x0a 0x0b 0x0a 0x0c 0x0a 0x0d 0x08
+0x0e 0x08 0x0f 0x08 0x10 0x04 0x11 0x04 0x12 0x05 0x13 0x05 0x14 0x05 0x15 0x05
+0x16 0x04 0x17 0x04 0x18 0x05 0x19 0x05 0x1a 0x05 0x1b 0x05 0x89 0x01 0x3a 0x00
+0x88 0x13 0x14 0x99 0x01 0x0b 0x02 0x0b 0x03 0x0b 0x04 0x0a 0x05 0x0a 0x06 0x0a
+0x07 0x08 0x08 0x08 0x09 0x08 0x0a 0x0a 0x0b 0x0a 0x0c 0x0a 0x0d 0x08 0x0e 0x08
+0x0f 0x08 0x10 0x04 0x11 0x04 0x12 0x05 0x13 0x05 0x14 0x05 0x15 0x05 0x16 0x04
+0x17 0x04 0x18 0x05 0x19 0x05 0x1a 0x05 0x1b 0x05 0x89 0x01 0x3a 0x00 0x88 0x13
+0x14 0x9d 0x01 0x0b 0x02 0x0b 0x03 0x0b 0x04 0x0a 0x05 0x0a 0x06 0x0a 0x07 0x08
+0x08 0x08 0x09 0x08 0x0a 0x0a 0x0b 0x0a 0x0c 0x0a 0x0d 0x08 0x0e 0x08 0x0f 0x08
+0x10 0x04 0x11 0x04 0x12 0x05 0x13 0x05 0x14 0x05 0x15 0x05 0x16 0x04 0x17 0x04
+0x18 0x05 0x19 0x05 0x1a 0x05 0x1b 0x05 0x89 0x01 0x3a 0x00 0x88 0x13 0x14 0xa1
+0x01 0x0b 0x02 0x0b 0x03 0x0b 0x04 0x0a 0x05 0x0a 0x06 0x0a 0x07 0x08 0x08 0x08
+0x09 0x08 0x0a 0x0a 0x0b 0x0a 0x0c 0x0a 0x0d 0x08 0x0e 0x08 0x0f 0x08 0x10 0x04
+0x11 0x04 0x12 0x05 0x13 0x05 0x14 0x05 0x15 0x05 0x16 0x04 0x17 0x04 0x18 0x05
+0x19 0x05 0x1a 0x05 0x1b 0x05 0x89 0x01 0x3a 0x00 0x88 0x13 0x14 0xa5 0x01 0x0b
+0x02 0x0b 0x03 0x0b 0x04 0x0a 0x05 0x0a 0x06 0x0a 0x07 0x08 0x08 0x08 0x09 0x08
+0x0a 0x0a 0x0b 0x0a 0x0c 0x0a 0x0d 0x08 0x0e 0x08 0x0f 0x08 0x10 0x04 0x11 0x04
+0x12 0x05 0x13 0x05 0x14 0x05 0x15 0x05 0x16 0x04 0x17 0x04 0x18 0x05 0x19 0x05
+0x1a 0x05 0x1b 0x05>;
+	};
+};
+
 &sdmmc {
 	disable-wp;
 	pinctrl-names = "default";
-- 
2.22.0.709.g102302147b-goog

^ permalink raw reply related

* Re: [PATCH 2/2] ARM: dts: aspeed: tiogapass: Add Riser card
From: Joel Stanley @ 2019-07-23 22:33 UTC (permalink / raw)
  To: Vijay Khemka
  Cc: Jean Delvare, Guenter Roeck, Rob Herring, Mark Rutland,
	Andrew Jeffery, linux-hwmon@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-aspeed@lists.ozlabs.org, openbmc @ lists . ozlabs . org,
	Sai Dasari
In-Reply-To: <C9C6AC86-B353-4CDA-8B63-50587F48DF44@fb.com>

On Tue, 23 Jul 2019 at 17:22, Vijay Khemka <vijaykhemka@fb.com> wrote:
>
> Team,
> This patch also needs review. I separated first patch with v2 and that was acked. Please review this as well.

Do you have a coworker who has access to the schematics and can review
the device tree? They would make a great reviewer.

If no reviewer steps forward I will merge it next time I'm merging patches.

Cheers,

Joel

>
> Regards
> -Vijay
>
> On 7/22/19, 12:41 PM, "Vijay Khemka" <vijaykhemka@fb.com> wrote:
>
>     Added i2c mux for riser card and multiple ava card and its sensor
>     components for Facebook Tiogapass platform
>
>     Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
>     ---
>      .../dts/aspeed-bmc-facebook-tiogapass.dts     | 230 ++++++++++++++++++
>      1 file changed, 230 insertions(+)
>
>     diff --git a/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts b/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts
>     index b7783833a58c..8d0bcb3cd419 100644
>     --- a/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts
>     +++ b/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts
>     @@ -12,6 +12,27 @@
>         aliases {
>                 serial0 = &uart1;
>                 serial4 = &uart5;
>     +
>     +           /*
>     +            * Hardcode the bus number of i2c switches' channels to
>     +            * avoid breaking the legacy applications.
>     +            */
>     +           i2c16 = &imux16;
>     +           i2c17 = &imux17;
>     +           i2c18 = &imux18;
>     +           i2c19 = &imux19;
>     +           i2c20 = &imux20;
>     +           i2c21 = &imux21;
>     +           i2c22 = &imux22;
>     +           i2c23 = &imux23;
>     +           i2c24 = &imux24;
>     +           i2c25 = &imux25;
>     +           i2c26 = &imux26;
>     +           i2c27 = &imux27;
>     +           i2c28 = &imux28;
>     +           i2c29 = &imux29;
>     +           i2c30 = &imux30;
>     +           i2c31 = &imux31;
>         };
>         chosen {
>                 stdout-path = &uart5;
>     @@ -124,6 +145,215 @@
>      &i2c1 {
>         status = "okay";
>         //X24 Riser
>     +   i2c-switch@71 {
>     +           compatible = "nxp,pca9544";
>     +           #address-cells = <1>;
>     +           #size-cells = <0>;
>     +           reg = <0x71>;
>     +
>     +           imux16: i2c@0 {
>     +                   #address-cells = <1>;
>     +                   #size-cells = <0>;
>     +                   reg = <0>;
>     +
>     +                   ina219@45 {
>     +                           compatible = "ti,ina219";
>     +                           reg = <0x45>;
>     +                   };
>     +
>     +                   tmp75@48 {
>     +                           compatible = "ti,tmp75";
>     +                           reg = <0x48>;
>     +                   };
>     +
>     +                   tmp421@49 {
>     +                           compatible = "ti,tmp75";
>     +                           reg = <0x49>;
>     +                   };
>     +
>     +                   eeprom@50 {
>     +                           compatible = "atmel,24c64";
>     +                           reg = <0x50>;
>     +                           pagesize = <32>;
>     +                   };
>     +
>     +                   i2c-switch@73 {
>     +                           compatible = "nxp,pca9546";
>     +                           #address-cells = <1>;
>     +                           #size-cells = <0>;
>     +                           reg = <0x73>;
>     +
>     +                           imux20: i2c@0 {
>     +                                   #address-cells = <1>;
>     +                                   #size-cells = <0>;
>     +                                   reg = <0>;
>     +                           };
>     +
>     +                           imux21: i2c@1 {
>     +                                   #address-cells = <1>;
>     +                                   #size-cells = <0>;
>     +                                   reg = <1>;
>     +                           };
>     +
>     +                           imux22: i2c@2 {
>     +                                   #address-cells = <1>;
>     +                                   #size-cells = <0>;
>     +                                   reg = <2>;
>     +                           };
>     +
>     +                           imux23: i2c@3 {
>     +                                   #address-cells = <1>;
>     +                                   #size-cells = <0>;
>     +                                   reg = <3>;
>     +                           };
>     +
>     +                   };
>     +
>     +           };
>     +
>     +           imux17: i2c@1 {
>     +                   #address-cells = <1>;
>     +                   #size-cells = <0>;
>     +                   reg = <1>;
>     +
>     +                   ina219@45 {
>     +                           compatible = "ti,ina219";
>     +                           reg = <0x45>;
>     +                   };
>     +
>     +                   tmp421@48 {
>     +                           compatible = "ti,tmp75";
>     +                           reg = <0x48>;
>     +                   };
>     +
>     +                   tmp421@49 {
>     +                           compatible = "ti,tmp75";
>     +                           reg = <0x49>;
>     +                   };
>     +
>     +                   eeprom@50 {
>     +                           compatible = "atmel,24c64";
>     +                           reg = <0x50>;
>     +                           pagesize = <32>;
>     +                   };
>     +
>     +                   i2c-switch@73 {
>     +                           compatible = "nxp,pca9546";
>     +                           #address-cells = <1>;
>     +                           #size-cells = <0>;
>     +                           reg = <0x73>;
>     +
>     +                           imux24: i2c@0 {
>     +                                   #address-cells = <1>;
>     +                                   #size-cells = <0>;
>     +                                   reg = <0>;
>     +                           };
>     +
>     +                           imux25: i2c@1 {
>     +                                   #address-cells = <1>;
>     +                                   #size-cells = <0>;
>     +                                   reg = <1>;
>     +                           };
>     +
>     +                           imux26: i2c@2 {
>     +                                   #address-cells = <1>;
>     +                                   #size-cells = <0>;
>     +                                   reg = <2>;
>     +                           };
>     +
>     +                           imux27: i2c@3 {
>     +                                   #address-cells = <1>;
>     +                                   #size-cells = <0>;
>     +                                   reg = <3>;
>     +                           };
>     +
>     +                   };
>     +
>     +           };
>     +
>     +           imux18: i2c@2 {
>     +                   #address-cells = <1>;
>     +                   #size-cells = <0>;
>     +                   reg = <2>;
>     +
>     +                   ina219@45 {
>     +                           compatible = "ti,ina219";
>     +                           reg = <0x45>;
>     +                   };
>     +
>     +                   tmp421@48 {
>     +                           compatible = "ti,tmp75";
>     +                           reg = <0x48>;
>     +                   };
>     +
>     +                   tmp421@49 {
>     +                           compatible = "ti,tmp75";
>     +                           reg = <0x49>;
>     +                   };
>     +
>     +                   eeprom@50 {
>     +                           compatible = "atmel,24c64";
>     +                           reg = <0x50>;
>     +                           pagesize = <32>;
>     +                   };
>     +
>     +                   i2c-switch@73 {
>     +                           compatible = "nxp,pca9546";
>     +                           #address-cells = <1>;
>     +                           #size-cells = <0>;
>     +                           reg = <0x73>;
>     +
>     +                           imux28: i2c@0 {
>     +                                   #address-cells = <1>;
>     +                                   #size-cells = <0>;
>     +                                   reg = <0>;
>     +                           };
>     +
>     +                           imux29: i2c@1 {
>     +                                   #address-cells = <1>;
>     +                                   #size-cells = <0>;
>     +                                   reg = <1>;
>     +                           };
>     +
>     +                           imux30: i2c@2 {
>     +                                   #address-cells = <1>;
>     +                                   #size-cells = <0>;
>     +                                   reg = <2>;
>     +                           };
>     +
>     +                           imux31: i2c@3 {
>     +                                   #address-cells = <1>;
>     +                                   #size-cells = <0>;
>     +                                   reg = <3>;
>     +                           };
>     +
>     +                   };
>     +
>     +           };
>     +
>     +           imux19: i2c@3 {
>     +                   #address-cells = <1>;
>     +                   #size-cells = <0>;
>     +                   reg = <3>;
>     +
>     +                   i2c-switch@40 {
>     +                           compatible = "ti,ina219";
>     +                           reg = <0x40>;
>     +                   };
>     +
>     +                   i2c-switch@41 {
>     +                           compatible = "ti,ina219";
>     +                           reg = <0x41>;
>     +                   };
>     +
>     +                   i2c-switch@45 {
>     +                           compatible = "ti,ina219";
>     +                           reg = <0x45>;
>     +                   };
>     +
>     +           };
>     +
>     +   };
>      };
>
>      &i2c2 {
>     --
>     2.17.1
>
>
>

^ permalink raw reply

* Re: [PATCH v2 5/5] dt-bindings: dsp: fsl: Add DSP core binding support
From: Rob Herring @ 2019-07-23 22:32 UTC (permalink / raw)
  To: Daniel Baluta
  Cc: Marco Felsch, Shawn Guo, Mark Rutland, Dong Aisheng, Peng Fan,
	Anson Huang, devicetree, linux-kernel@vger.kernel.org,
	NXP Linux Team, S.j. Wang, paul.olaru, Sascha Hauer,
	Leonard Crestez, Fabio Estevam,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	sound-open-firmware
In-Reply-To: <20190723084104.12639-6-daniel.baluta@nxp.com>

On Tue, Jul 23, 2019 at 2:41 AM Daniel Baluta <daniel.baluta@nxp.com> wrote:
>
> This describes the DSP device tree node.
>
> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> ---
>  .../devicetree/bindings/dsp/fsl,dsp.yaml      | 87 +++++++++++++++++++
>  1 file changed, 87 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dsp/fsl,dsp.yaml
>
> diff --git a/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml
> new file mode 100644
> index 000000000000..d112486eda0e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml
> @@ -0,0 +1,87 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/freescale/fsl,dsp.yaml#

This needs updating to match the path.

> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP i.MX8 DSP core
> +
> +maintainers:
> +  - Daniel Baluta <daniel.baluta@nxp.com>
> +
> +description: |
> +  Some boards from i.MX8 family contain a DSP core used for
> +  advanced pre- and post- audio processing.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - fsl,imx8qxp-dsp
> +
> +  reg:
> +    description: Should contain register location and length
> +
> +  clocks:
> +    items:
> +      - description: ipg clock
> +      - description: ocram clock
> +      - description: core clock
> +
> +  clock-names:
> +    items:
> +      - const: ipg
> +      - const: ocram
> +      - const: core
> +
> +  power-domains:
> +    description:
> +      List of phandle and PM domain specifier as documented in
> +      Documentation/devicetree/bindings/power/power_domain.txt
> +    maxItems: 4

Need a blank line here.

With those 2 fixes:

Reviewed-by: Rob Herring <robh@kernel.org>

> +  mboxes:
> +    description:
> +      List of <&phandle type channel> - 2 channels for TXDB, 2 channels for RXDB
> +      (see mailbox/fsl,mu.txt)
> +    maxItems: 4
> +
> +  mbox-names:
> +    items:
> +      - const: txdb0
> +      - const: txdb1
> +      - const: rxdb0
> +      - const: rxdb1
> +
> +  memory-region:
> +    description:
> +       phandle to a node describing reserved memory (System RAM memory)
> +       used by DSP (see bindings/reserved-memory/reserved-memory.txt)
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - power-domains
> +  - mboxes
> +  - mbox-names
> +  - memory-region
> +
> +examples:
> +  - |
> +    #include <dt-bindings/firmware/imx/rsrc.h>
> +    #include <dt-bindings/clock/imx8-clock.h>
> +    dsp@596e8000 {
> +        compatbile = "fsl,imx8qxp-dsp";
> +        reg = <0x596e8000 0x88000>;
> +        clocks = <&adma_lpcg IMX_ADMA_LPCG_DSP_IPG_CLK>,
> +                 <&adma_lpcg IMX_ADMA_LPCG_OCRAM_IPG_CLK>,
> +                 <&adma_lpcg IMX_ADMA_LPCG_DSP_CORE_CLK>;
> +        clock-names = "ipg", "ocram", "core";
> +        power-domains = <&pd IMX_SC_R_MU_13A>,
> +                        <&pd IMX_SC_R_MU_13B>,
> +                        <&pd IMX_SC_R_DSP>,
> +                        <&pd IMX_SC_R_DSP_RAM>;
> +        mbox-names = "txdb0", "txdb1", "rxdb0", "rxdb1";
> +        mboxes = <&lsio_mu13 2 0>, <&lsio_mu13 2 1>, <&lsio_mu13 3 0>, <&lsio_mu13 3 1>;
> +    };
> --
> 2.17.1
>

^ permalink raw reply

* Re: [PATCH net-next 1/3] enetc: Add mdio bus driver for the PCIe MDIO endpoint
From: Andrew Lunn @ 2019-07-23 22:24 UTC (permalink / raw)
  To: Claudiu Manoil
  Cc: David S . Miller, devicetree, netdev, alexandru.marginean,
	linux-kernel, Li Yang, Rob Herring, linux-arm-kernel
In-Reply-To: <1563894955-545-2-git-send-email-claudiu.manoil@nxp.com>

> +	bus = mdiobus_alloc_size(sizeof(u32 *));
> +	if (!bus)
> +		return -ENOMEM;
> +

> +	bus->priv = pci_iomap_range(pdev, 0, ENETC_MDIO_REG_OFFSET, 0);

This got me confused for a while. You allocate space for a u32
pointer. bus->priv will point to this space. However, you are not
using this space, you {ab}use the pointer to directly hold the return
from pci_iomap_range(). This works, but sparse is probably unhappy,
and you are wasting the space the u32 pointer takes.

    Andrew

^ permalink raw reply

* [PATCH v3 2/7] drivers: Introduce device lookup variants by of_node
From: Suzuki K Poulose @ 2019-07-23 22:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, rafael, linux-arm-kernel, Suzuki K Poulose,
	Maarten Lankhorst, Maxime Ripard, dri-devel, David Airlie,
	Daniel Vetter, devicetree, Florian Fainelli, Frank Rowand,
	Heiko Stuebner, Liam Girdwood, linux-i2c, linux-rockchip,
	linux-spi, Mathieu Poirier, Rob Herring, Srinivas Kandagatla,
	Takashi Iwai, Wolfram
In-Reply-To: <20190723221838.12024-1-suzuki.poulose@arm.com>

Introduce wrappers for {bus/driver/class}_find_device() to
locate devices by its of_node.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: dri-devel@lists.freedesktop.org
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: devicetree@vger.kernel.org
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: linux-i2c@vger.kernel.org
Cc: linux-rockchip@lists.infradead.org
Cc: linux-spi@vger.kernel.org
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Alan Tull <atull@kernel.org>
Cc: Moritz Fischer <mdf@kernel.org>
Cc: linux-fpga@vger.kernel.org
Cc: Peter Rosin <peda@axentia.se>
Cc: Mark Brown <broonie@kernel.org>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Heiner Kallweit <hkallweit1@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Thor Thayer <thor.thayer@linux.intel.com>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Peter Rosin <peda@axentia.se>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 - Dropped the reviewed-by tags from Thor, Mark, Andrew and Peter as the
   patches are mereged, though there are no functional changes.
---
 drivers/amba/tegra-ahb.c              | 11 +-------
 drivers/fpga/fpga-bridge.c            |  8 +-----
 drivers/fpga/fpga-mgr.c               |  8 +-----
 drivers/gpu/drm/drm_mipi_dsi.c        |  7 +----
 drivers/i2c/i2c-core-of.c             |  7 +----
 drivers/mfd/altera-sysmgr.c           | 14 ++--------
 drivers/mux/core.c                    |  7 +----
 drivers/net/phy/mdio_bus.c            |  9 +------
 drivers/nvmem/core.c                  |  7 +----
 drivers/of/of_mdio.c                  |  8 +-----
 drivers/of/platform.c                 |  7 +----
 drivers/regulator/of_regulator.c      |  7 +----
 drivers/spi/spi.c                     | 20 +++------------
 include/linux/device.h                | 37 +++++++++++++++++++++++++++
 sound/soc/rockchip/rk3399_gru_sound.c |  9 ++-----
 15 files changed, 56 insertions(+), 110 deletions(-)

diff --git a/drivers/amba/tegra-ahb.c b/drivers/amba/tegra-ahb.c
index aa64eece77a6..57d3b2e2d007 100644
--- a/drivers/amba/tegra-ahb.c
+++ b/drivers/amba/tegra-ahb.c
@@ -134,22 +134,13 @@ static inline void gizmo_writel(struct tegra_ahb *ahb, u32 value, u32 offset)
 }
 
 #ifdef CONFIG_TEGRA_IOMMU_SMMU
-static int tegra_ahb_match_by_smmu(struct device *dev, const void *data)
-{
-	struct tegra_ahb *ahb = dev_get_drvdata(dev);
-	const struct device_node *dn = data;
-
-	return (ahb->dev->of_node == dn) ? 1 : 0;
-}
-
 int tegra_ahb_enable_smmu(struct device_node *dn)
 {
 	struct device *dev;
 	u32 val;
 	struct tegra_ahb *ahb;
 
-	dev = driver_find_device(&tegra_ahb_driver.driver, NULL, dn,
-				 tegra_ahb_match_by_smmu);
+	dev = driver_find_device_by_of_node(&tegra_ahb_driver.driver, dn);
 	if (!dev)
 		return -EPROBE_DEFER;
 	ahb = dev_get_drvdata(dev);
diff --git a/drivers/fpga/fpga-bridge.c b/drivers/fpga/fpga-bridge.c
index 80bd8f1b2aa6..4bab9028940a 100644
--- a/drivers/fpga/fpga-bridge.c
+++ b/drivers/fpga/fpga-bridge.c
@@ -19,11 +19,6 @@ static struct class *fpga_bridge_class;
 /* Lock for adding/removing bridges to linked lists*/
 static spinlock_t bridge_list_lock;
 
-static int fpga_bridge_of_node_match(struct device *dev, const void *data)
-{
-	return dev->of_node == data;
-}
-
 /**
  * fpga_bridge_enable - Enable transactions on the bridge
  *
@@ -104,8 +99,7 @@ struct fpga_bridge *of_fpga_bridge_get(struct device_node *np,
 {
 	struct device *dev;
 
-	dev = class_find_device(fpga_bridge_class, NULL, np,
-				fpga_bridge_of_node_match);
+	dev = class_find_device_by_of_node(fpga_bridge_class, np);
 	if (!dev)
 		return ERR_PTR(-ENODEV);
 
diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index c3866816456a..e05104f5e40c 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -482,11 +482,6 @@ struct fpga_manager *fpga_mgr_get(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(fpga_mgr_get);
 
-static int fpga_mgr_of_node_match(struct device *dev, const void *data)
-{
-	return dev->of_node == data;
-}
-
 /**
  * of_fpga_mgr_get - Given a device node, get a reference to a fpga mgr.
  *
@@ -498,8 +493,7 @@ struct fpga_manager *of_fpga_mgr_get(struct device_node *node)
 {
 	struct device *dev;
 
-	dev = class_find_device(fpga_mgr_class, NULL, node,
-				fpga_mgr_of_node_match);
+	dev = class_find_device_by_of_node(fpga_mgr_class, node);
 	if (!dev)
 		return ERR_PTR(-ENODEV);
 
diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index ad19df0686c9..bd2498bbd74a 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -93,11 +93,6 @@ static struct bus_type mipi_dsi_bus_type = {
 	.pm = &mipi_dsi_device_pm_ops,
 };
 
-static int of_device_match(struct device *dev, const void *data)
-{
-	return dev->of_node == data;
-}
-
 /**
  * of_find_mipi_dsi_device_by_node() - find the MIPI DSI device matching a
  *    device tree node
@@ -110,7 +105,7 @@ struct mipi_dsi_device *of_find_mipi_dsi_device_by_node(struct device_node *np)
 {
 	struct device *dev;
 
-	dev = bus_find_device(&mipi_dsi_bus_type, NULL, np, of_device_match);
+	dev = bus_find_device_by_of_node(&mipi_dsi_bus_type, np);
 
 	return dev ? to_mipi_dsi_device(dev) : NULL;
 }
diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c
index d1c48dec7118..6f632d543fcc 100644
--- a/drivers/i2c/i2c-core-of.c
+++ b/drivers/i2c/i2c-core-of.c
@@ -113,11 +113,6 @@ void of_i2c_register_devices(struct i2c_adapter *adap)
 	of_node_put(bus);
 }
 
-static int of_dev_node_match(struct device *dev, const void *data)
-{
-	return dev->of_node == data;
-}
-
 static int of_dev_or_parent_node_match(struct device *dev, const void *data)
 {
 	if (dev->of_node == data)
@@ -135,7 +130,7 @@ struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
 	struct device *dev;
 	struct i2c_client *client;
 
-	dev = bus_find_device(&i2c_bus_type, NULL, node, of_dev_node_match);
+	dev = bus_find_device_by_of_node(&i2c_bus_type, node);
 	if (!dev)
 		return NULL;
 
diff --git a/drivers/mfd/altera-sysmgr.c b/drivers/mfd/altera-sysmgr.c
index 2ee14d8a6d31..d2a13a547a3c 100644
--- a/drivers/mfd/altera-sysmgr.c
+++ b/drivers/mfd/altera-sysmgr.c
@@ -87,16 +87,6 @@ static struct regmap_config altr_sysmgr_regmap_cfg = {
 	.use_single_write = true,
 };
 
-/**
- * sysmgr_match_phandle
- * Matching function used by driver_find_device().
- * Return: True if match is found, otherwise false.
- */
-static int sysmgr_match_phandle(struct device *dev, const void *data)
-{
-	return dev->of_node == (const struct device_node *)data;
-}
-
 /**
  * altr_sysmgr_regmap_lookup_by_phandle
  * Find the sysmgr previous configured in probe() and return regmap property.
@@ -117,8 +107,8 @@ struct regmap *altr_sysmgr_regmap_lookup_by_phandle(struct device_node *np,
 	if (!sysmgr_np)
 		return ERR_PTR(-ENODEV);
 
-	dev = driver_find_device(&altr_sysmgr_driver.driver, NULL,
-				 (void *)sysmgr_np, sysmgr_match_phandle);
+	dev = driver_find_device_by_of_node(&altr_sysmgr_driver.driver,
+					    (void *)sysmgr_np);
 	of_node_put(sysmgr_np);
 	if (!dev)
 		return ERR_PTR(-EPROBE_DEFER);
diff --git a/drivers/mux/core.c b/drivers/mux/core.c
index d1271c1ee23c..1fb22388e7e0 100644
--- a/drivers/mux/core.c
+++ b/drivers/mux/core.c
@@ -405,17 +405,12 @@ int mux_control_deselect(struct mux_control *mux)
 }
 EXPORT_SYMBOL_GPL(mux_control_deselect);
 
-static int of_dev_node_match(struct device *dev, const void *data)
-{
-	return dev->of_node == data;
-}
-
 /* Note this function returns a reference to the mux_chip dev. */
 static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
 {
 	struct device *dev;
 
-	dev = class_find_device(&mux_class, NULL, np, of_dev_node_match);
+	dev = class_find_device_by_of_node(&mux_class, np);
 
 	return dev ? to_mux_chip(dev) : NULL;
 }
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index bd04fe762056..ce940871331e 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -262,11 +262,6 @@ static struct class mdio_bus_class = {
 };
 
 #if IS_ENABLED(CONFIG_OF_MDIO)
-/* Helper function for of_mdio_find_bus */
-static int of_mdio_bus_match(struct device *dev, const void *mdio_bus_np)
-{
-	return dev->of_node == mdio_bus_np;
-}
 /**
  * of_mdio_find_bus - Given an mii_bus node, find the mii_bus.
  * @mdio_bus_np: Pointer to the mii_bus.
@@ -287,9 +282,7 @@ struct mii_bus *of_mdio_find_bus(struct device_node *mdio_bus_np)
 	if (!mdio_bus_np)
 		return NULL;
 
-	d = class_find_device(&mdio_bus_class, NULL,  mdio_bus_np,
-			      of_mdio_bus_match);
-
+	d = class_find_device_by_of_node(&mdio_bus_class, mdio_bus_np);
 	return d ? to_mii_bus(d) : NULL;
 }
 EXPORT_SYMBOL(of_mdio_find_bus);
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index ac5d945be88a..057d1ff87d5d 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -76,11 +76,6 @@ static struct bus_type nvmem_bus_type = {
 	.name		= "nvmem",
 };
 
-static int of_nvmem_match(struct device *dev, const void *nvmem_np)
-{
-	return dev->of_node == nvmem_np;
-}
-
 static struct nvmem_device *of_nvmem_find(struct device_node *nvmem_np)
 {
 	struct device *d;
@@ -88,7 +83,7 @@ static struct nvmem_device *of_nvmem_find(struct device_node *nvmem_np)
 	if (!nvmem_np)
 		return NULL;
 
-	d = bus_find_device(&nvmem_bus_type, NULL, nvmem_np, of_nvmem_match);
+	d = bus_find_device_by_of_node(&nvmem_bus_type, nvmem_np);
 
 	if (!d)
 		return NULL;
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 44f53496cab1..000b95787df1 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -280,12 +280,6 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
 }
 EXPORT_SYMBOL(of_mdiobus_register);
 
-/* Helper function for of_phy_find_device */
-static int of_phy_match(struct device *dev, const void *phy_np)
-{
-	return dev->of_node == phy_np;
-}
-
 /**
  * of_phy_find_device - Give a PHY node, find the phy_device
  * @phy_np: Pointer to the phy's device tree node
@@ -301,7 +295,7 @@ struct phy_device *of_phy_find_device(struct device_node *phy_np)
 	if (!phy_np)
 		return NULL;
 
-	d = bus_find_device(&mdio_bus_type, NULL, phy_np, of_phy_match);
+	d = bus_find_device_by_of_node(&mdio_bus_type, phy_np);
 	if (d) {
 		mdiodev = to_mdio_device(d);
 		if (mdiodev->flags & MDIO_DEVICE_FLAG_PHY)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 7801e25e6895..b47a2292fe8e 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -37,11 +37,6 @@ static const struct of_device_id of_skipped_node_table[] = {
 	{} /* Empty terminated list */
 };
 
-static int of_dev_node_match(struct device *dev, const void *data)
-{
-	return dev->of_node == data;
-}
-
 /**
  * of_find_device_by_node - Find the platform_device associated with a node
  * @np: Pointer to device tree node
@@ -55,7 +50,7 @@ struct platform_device *of_find_device_by_node(struct device_node *np)
 {
 	struct device *dev;
 
-	dev = bus_find_device(&platform_bus_type, NULL, np, of_dev_node_match);
+	dev = bus_find_device_by_of_node(&platform_bus_type, np);
 	return dev ? to_platform_device(dev) : NULL;
 }
 EXPORT_SYMBOL(of_find_device_by_node);
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 397918ebba55..20dcc9c03adc 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -460,16 +460,11 @@ struct regulator_init_data *regulator_of_get_init_data(struct device *dev,
 	return NULL;
 }
 
-static int of_node_match(struct device *dev, const void *data)
-{
-	return dev->of_node == data;
-}
-
 struct regulator_dev *of_find_regulator_by_node(struct device_node *np)
 {
 	struct device *dev;
 
-	dev = class_find_device(&regulator_class, NULL, np, of_node_match);
+	dev = class_find_device_by_of_node(&regulator_class, np);
 
 	return dev ? dev_to_rdev(dev) : NULL;
 }
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 75ac046cae52..a591da87981a 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -3652,37 +3652,25 @@ EXPORT_SYMBOL_GPL(spi_write_then_read);
 /*-------------------------------------------------------------------------*/
 
 #if IS_ENABLED(CONFIG_OF)
-static int __spi_of_device_match(struct device *dev, const void *data)
-{
-	return dev->of_node == data;
-}
-
 /* must call put_device() when done with returned spi_device device */
 struct spi_device *of_find_spi_device_by_node(struct device_node *node)
 {
-	struct device *dev = bus_find_device(&spi_bus_type, NULL, node,
-						__spi_of_device_match);
+	struct device *dev = bus_find_device_by_of_node(&spi_bus_type, node);
+
 	return dev ? to_spi_device(dev) : NULL;
 }
 EXPORT_SYMBOL_GPL(of_find_spi_device_by_node);
 #endif /* IS_ENABLED(CONFIG_OF) */
 
 #if IS_ENABLED(CONFIG_OF_DYNAMIC)
-static int __spi_of_controller_match(struct device *dev, const void *data)
-{
-	return dev->of_node == data;
-}
-
 /* the spi controllers are not using spi_bus, so we find it with another way */
 static struct spi_controller *of_find_spi_controller_by_node(struct device_node *node)
 {
 	struct device *dev;
 
-	dev = class_find_device(&spi_master_class, NULL, node,
-				__spi_of_controller_match);
+	dev = class_find_device_by_of_node(&spi_master_class, node);
 	if (!dev && IS_ENABLED(CONFIG_SPI_SLAVE))
-		dev = class_find_device(&spi_slave_class, NULL, node,
-					__spi_of_controller_match);
+		dev = class_find_device_by_of_node(&spi_slave_class, node);
 	if (!dev)
 		return NULL;
 
diff --git a/include/linux/device.h b/include/linux/device.h
index 3ba376b8b456..29d8d7ad41e6 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -186,6 +186,18 @@ static inline struct device *bus_find_device_by_name(struct bus_type *bus,
 	return bus_find_device(bus, start, name, device_match_name);
 }
 
+/**
+ * bus_find_device_by_of_node : device iterator for locating a particular device
+ * matching the of_node.
+ * @bus: bus type
+ * @np: of_node of the device to match.
+ */
+static inline struct device *
+bus_find_device_by_of_node(struct bus_type *bus, const struct device_node *np)
+{
+	return bus_find_device(bus, NULL, np, device_match_of_node);
+}
+
 struct device *subsys_find_device_by_id(struct bus_type *bus, unsigned int id,
 					struct device *hint);
 int bus_for_each_drv(struct bus_type *bus, struct device_driver *start,
@@ -366,6 +378,19 @@ static inline struct device *driver_find_device_by_name(struct device_driver *dr
 	return driver_find_device(drv, NULL, name, device_match_name);
 }
 
+/**
+ * driver_find_device_by_of_node- device iterator for locating a particular device
+ * by of_node pointer.
+ * @driver: the driver we're iterating
+ * @np: of_node pointer to match.
+ */
+static inline struct device *
+driver_find_device_by_of_node(struct device_driver *drv,
+			      const struct device_node *np)
+{
+	return driver_find_device(drv, NULL, np, device_match_of_node);
+}
+
 void driver_deferred_probe_add(struct device *dev);
 int driver_deferred_probe_check_state(struct device *dev);
 int driver_deferred_probe_check_state_continue(struct device *dev);
@@ -507,6 +532,18 @@ static inline struct device *class_find_device_by_name(struct class *class,
 	return class_find_device(class, NULL, name, device_match_name);
 }
 
+/**
+ * class_find_device_by_of_node : device iterator for locating a particular device
+ * matching the of_node.
+ * @class: class type
+ * @np: of_node of the device to match.
+ */
+static inline struct device *
+class_find_device_by_of_node(struct class *class, const struct device_node *np)
+{
+	return class_find_device(class, NULL, np, device_match_of_node);
+}
+
 struct class_attribute {
 	struct attribute attr;
 	ssize_t (*show)(struct class *class, struct class_attribute *attr,
diff --git a/sound/soc/rockchip/rk3399_gru_sound.c b/sound/soc/rockchip/rk3399_gru_sound.c
index c16b0ffe8cfc..d951100bf770 100644
--- a/sound/soc/rockchip/rk3399_gru_sound.c
+++ b/sound/soc/rockchip/rk3399_gru_sound.c
@@ -422,11 +422,6 @@ static const struct dailink_match_data dailink_match[] = {
 	},
 };
 
-static int of_dev_node_match(struct device *dev, const void *data)
-{
-	return dev->of_node == data;
-}
-
 static int rockchip_sound_codec_node_match(struct device_node *np_codec)
 {
 	struct device *dev;
@@ -438,8 +433,8 @@ static int rockchip_sound_codec_node_match(struct device_node *np_codec)
 			continue;
 
 		if (dailink_match[i].bus_type) {
-			dev = bus_find_device(dailink_match[i].bus_type, NULL,
-					      np_codec, of_dev_node_match);
+			dev = bus_find_device_by_of_node(dailink_match[i].bus_type,
+							 np_codec);
 			if (!dev)
 				continue;
 			put_device(dev);
-- 
2.21.0

^ permalink raw reply related

* [PATCH v3 0/7] drivers: Add generic device lookup helpers
From: Suzuki K Poulose @ 2019-07-23 22:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, rafael, linux-arm-kernel, Suzuki K Poulose, Alan Tull,
	Alessandro Zummo, Alexander Aring, Alexander Shishkin,
	Alexandre Belloni, Andrew Lunn, Arnd Bergmann, Bjorn Helgaas,
	Daniel Vetter, Dan Murphy, David Airlie, David S. Miller,
	devicetree, Doug Ledford, dri-devel, Elie Morisse, Eric Anholt,
	Florian Fainelli, Frank Rowand, Harald Freudenberger,
	Heikki Krogerus, Heiko Carstens, Heiko Stübner,
	Heiner Kallweit, Inki Dae, Jacek Anaszewski, James E.J. Bottomley,
	Jason Gunthorpe, Jiri Slaby, Joe Perches, Joerg Roedel, Lee Jones,
	Len Brown, Liam Girdwood, linux-acpi, linux-fpga, linux-i2c,
	linux-leds, linux-rockchip, linux-rtc, linux-spi, linux-usb,
	linux-wpan, Maarten Lankhorst, Mark Brown, Martin K. Petersen,
	Mathieu Poirier, Maxime Coquelin, Maxime Ripard, Mika Westerberg,
	Moritz Fischer, Nehal Shah, Oliver Neukum, Pavel Machek,
	Peter Oberparleiter, Peter Rosin, Rafael J. Wysocki, Rob Herring,
	Robin Murphy, Sandy Huang, Sebastian Andrzej Siewior,
	Seung-Woo Kim, Shyam Sundar S K, Srinivas Kandagatla,
	Stefan Schmidt, Takashi Iwai, Thor Thayer, Tomas Winkler,
	Ulf Hansson, Will Deacon, Wolfram Sang

We have device iterators to find a particular device matching a criteria
for a given bus/class/driver. i.e, {bus,class,driver}_find_device()
APIs. The matching criteria is a function pointer for the APIs. Often the
lookup is based on a generic property of a device (e.g, name, fwnode, of node
pointer or device type) rather than a driver specific information. However, each
driver writes up its own "match" function, spilling the similar match functions
all over the driver subsystems.

This series adds device match helpers by generic device properties of a device.
Using these generic match functions, introduce wrappers to the existing
 *_find_device() helpers and converts the existing users to use the new wrappers.
 i.e,
	{bus/class/driver}_find_device_by_name()
	{bus/class/driver}_find_device_by_fwnode()
	{bus/class/driver}_find_device_by_devt()
	{bus/class/driver}_find_device_by_acpi_dev()

Also adds a wrapper for finding a platform device by driver, to avoid
spilling the platform_bus internals in the individual drivers. Applies
on 5.3-rc1.

   [0] git://linux-arm.org/linux-skp.git driver-cleanup/v3
RFC[1] https://marc.info/?i=1559577023-558-1-git-send-email-suzuki.poulose@arm.com
V1 [2] https://marc.info/?i=1559747630-28065-1-git-send-email-suzuki.poulose@arm.com
V2 [3] https://marc.info/?i=1560534863-15115-1-git-send-email-suzuki.poulose@arm.com

Changes since v2:
 - Merge the device match helper introduction patch with the variants
   of the helpers, for better review.
 - Use platform_find_device_by_driver for mcde drm driver.

Cc: Alan Tull <atull@kernel.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexander Aring <alex.aring@gmail.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Dan Murphy <dmurphy@ti.com>
Cc: David Airlie <airlied@linux.ie>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: devicetree@vger.kernel.org
Cc: Doug Ledford <dledford@redhat.com>
Cc: dri-devel@lists.freedesktop.org
Cc: Elie Morisse <syniurge@gmail.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Harald Freudenberger <freude@linux.ibm.com>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: "Heiko Stübner" <heiko@sntech.de>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Heiner Kallweit <hkallweit1@gmail.com>
Cc: Inki Dae <inki.dae@samsung.com>
Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Joe Perches <joe@perches.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Len Brown <lenb@kernel.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: linux-acpi@vger.kernel.org
Cc: linux-fpga@vger.kernel.org
Cc: linux-i2c@vger.kernel.org
Cc: linux-leds@vger.kernel.org
Cc: linux-rockchip@lists.infradead.org
Cc: linux-rtc@vger.kernel.org
Cc: linux-spi@vger.kernel.org
Cc: linux-usb@vger.kernel.org
Cc: linux-wpan@vger.kernel.org
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Moritz Fischer <mdf@kernel.org>
Cc: Nehal Shah <nehal-bakulchandra.shah@amd.com>
Cc: Oliver Neukum <oneukum@suse.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
Cc: Peter Rosin <peda@axentia.se>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Sandy Huang <hjc@rock-chips.com>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
Cc: Shyam Sundar S K <shyam-sundar.s-k@amd.com>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: Stefan Schmidt <stefan@datenfreihafen.org>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: Thor Thayer <thor.thayer@linux.intel.com>
Cc: Tomas Winkler <tomas.winkler@intel.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Wolfram Sang <wsa@the-dreams.de>

Suzuki K Poulose (7):
  drivers: Introduce device lookup variants by name
  drivers: Introduce device lookup variants by of_node
  drivers: Introduce device lookup variants by fwnode
  drivers: Introduce device lookup variants by device type
  drivers: Introduce device lookup variants by ACPI_COMPANION device
  drivers: Add generic helper to match any device
  platform: Add platform_find_device_by_driver() helper

 drivers/amba/tegra-ahb.c                      |  11 +-
 drivers/base/bus.c                            |  24 --
 drivers/base/core.c                           |  39 ++-
 drivers/base/devcon.c                         |   8 +-
 drivers/base/platform.c                       |  14 ++
 drivers/fpga/fpga-bridge.c                    |   8 +-
 drivers/fpga/fpga-mgr.c                       |   8 +-
 drivers/gpu/drm/drm_mipi_dsi.c                |   7 +-
 drivers/gpu/drm/exynos/exynos_drm_drv.c       |   9 +-
 drivers/gpu/drm/mcde/mcde_drv.c               |   3 +-
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c   |   3 +-
 drivers/gpu/drm/vc4/vc4_drv.c                 |   3 +-
 .../hwtracing/coresight/coresight-platform.c  |  11 +-
 drivers/hwtracing/coresight/coresight-priv.h  |   2 -
 drivers/hwtracing/coresight/coresight.c       |   4 +-
 drivers/hwtracing/intel_th/core.c             |  10 +-
 drivers/hwtracing/stm/core.c                  |   9 +-
 drivers/i2c/busses/i2c-amd-mp2-pci.c          |   8 +-
 drivers/i2c/i2c-core-acpi.c                   |  11 +-
 drivers/i2c/i2c-core-of.c                     |   7 +-
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c    |   8 +-
 drivers/iommu/arm-smmu-v3.c                   |   9 +-
 drivers/iommu/arm-smmu.c                      |   9 +-
 drivers/leds/led-class.c                      |   9 +-
 drivers/mfd/altera-sysmgr.c                   |  14 +-
 drivers/misc/mei/main.c                       |   9 +-
 drivers/mux/core.c                            |   7 +-
 .../ethernet/hisilicon/hns/hns_dsaf_misc.c    |   8 +-
 drivers/net/phy/mdio_bus.c                    |   9 +-
 drivers/nvmem/core.c                          |   7 +-
 drivers/of/of_mdio.c                          |   8 +-
 drivers/of/platform.c                         |   7 +-
 drivers/pci/probe.c                           |   7 +-
 drivers/regulator/of_regulator.c              |   7 +-
 drivers/rtc/interface.c                       |  11 +-
 drivers/s390/cio/ccwgroup.c                   |  18 +-
 drivers/s390/cio/device.c                     |  15 +-
 drivers/s390/crypto/zcrypt_api.c              |  22 +-
 drivers/scsi/scsi_proc.c                      |   9 +-
 drivers/spi/spi.c                             |  28 +--
 drivers/tty/tty_io.c                          |   8 +-
 drivers/usb/core/devio.c                      |   8 +-
 drivers/usb/roles/class.c                     |  16 +-
 drivers/usb/typec/class.c                     |  16 +-
 include/linux/device.h                        | 237 +++++++++++++++++-
 include/linux/platform_device.h               |   3 +
 net/ieee802154/core.c                         |   7 +-
 sound/soc/rockchip/rk3399_gru_sound.c         |   9 +-
 48 files changed, 341 insertions(+), 383 deletions(-)

-- 
2.21.0

^ permalink raw reply


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