Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v8 03/11] dt-bindings: usb: add binding for USB GPIO based connection detection driver
From: Linus Walleij @ 2019-08-05 10:08 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Rob Herring, Greg Kroah-Hartman, Biju Das, Mark Rutland,
	Matthias Brugger, Adam Thomson, Li Jun, Badhri Jagan Sridharan,
	Heikki Krogerus, Hans de Goede, Andy Shevchenko, Min Guo,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel@vger.kernel.org, linux-usb, Linux ARM, ARM/Mediatek 
In-Reply-To: <1563958245-6321-4-git-send-email-chunfeng.yun@mediatek.com>

On Wed, Jul 24, 2019 at 10:51 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:

> It's used to support dual role switch via GPIO when use Type-B
> receptacle, typically the USB ID pin is connected to an input
> GPIO, and also used to enable/disable device when the USB Vbus
> pin is connected to an input GPIO.
>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> v8 changes:

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply

* RE: [PATCH/RFC 02/12] dt-bindings: display: renesas: lvds: Document renesas,swap-data
From: Fabrizio Castro @ 2019-08-05 10:07 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Kieran Bingham, Jacopo Mondi, David Airlie, Daniel Vetter,
	Rob Herring, Mark Rutland, dri-devel@lists.freedesktop.org,
	linux-renesas-soc@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, Simon Horman, Geert Uytterhoeven,
	Chris Paterson, Biju Das
In-Reply-To: <20190805093543.GB29747@pendragon.ideasonboard.com>

Hi Laurent,

> From: linux-kernel-owner@vger.kernel.org <linux-kernel-owner@vger.kernel.org> On Behalf Of Laurent Pinchart
> Sent: 05 August 2019 10:36
> Subject: Re: [PATCH/RFC 02/12] dt-bindings: display: renesas: lvds: Document renesas,swap-data
> 
> Hi Fabrizio,
> 
> On Mon, Aug 05, 2019 at 08:59:51AM +0000, Fabrizio Castro wrote:
> > > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Sent: 02 August 2019 08:44
> > > Subject: Re: [PATCH/RFC 02/12] dt-bindings: display: renesas: lvds: Document renesas,swap-data
> > >
> > > Hi Fabrizio,
> > >
> > > Thank you for the patch.
> > >
> > > On Fri, Aug 02, 2019 at 08:33:59AM +0100, Fabrizio Castro wrote:
> > > > R-Car D3, R-Car E3, and RZ/G2E support dual-link mode.
> > > > In such a mode, the first LVDS encoder emits even data, and the
> > > > second LVDS encoder emits odd data. This patch documents property
> > > > renesas,swap-data, used to swap even and odd data around.
> > > >
> > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt | 5 +++++
> > > >  1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > > b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > > > index dece79e..8980179 100644
> > > > --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > > > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > > > @@ -52,6 +52,11 @@ Optional properties:
> > > >    mandatory for the first LVDS encoder on R-Car D3, R-Car E3, and RZ/G2E SoCs,
> > > >    and shall point to the second encoder to be used as a companion in dual-link
> > > >    mode. It shall not be set for any other LVDS encoder.
> > > > +- renesas,swap-data : when in dual-link mode, the first LVDS encoder normally
> > > > +  emits even data, and the second LVDS encoder emits odd data. When property
> > > > +  renesas,swap-data is specified, the data emitted by the two encoders will be
> > > > +  swapped around. This property can only be used in conjunction with property
> > > > +  renesas,companion.
> > >
> > > From an LVDS encoder point of view this is more a configuration option
> > > than a description of the hardware. Wouldn't it be better for the LVDS
> > > sink to report which of the odd or even pixels it expects on each of its
> > > endpoints ?
> >
> > Yes, that would be my preference too, and it would be better, I am just not entirely
> > what's the best place for this information though
> >
> > > The LVDS encoder driver could then query that at runtime and
> > > configure itself accordingly. Ideally this should be queried through the
> > > drm_bridge_timings structure (or through a similar mean), not through
> > > DT. An LVDS sink that has a fixed mapping of odd/even pixels to
> > > endpoints wouldn't need the information to be specified in DT at all.
> >
> > Isn't drm_bridge_timings specific for bridges?
> 
> Its name makes it specific to bridges, but the information it contains
> could equally apply to panels. I would thus use it for both, possibly
> after renaming it.

Will give this a try then.

Thanks,
Fab

> 
> --
> Regards,
> 
> Laurent Pinchart

^ permalink raw reply

* Re: [PATCH v8 00/11] add USB GPIO based connection detection driver
From: Linus Walleij @ 2019-08-05 10:06 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Rob Herring, Greg Kroah-Hartman, Biju Das, Mark Rutland,
	Matthias Brugger, Adam Thomson, Li Jun, Badhri Jagan Sridharan,
	Heikki Krogerus, Hans de Goede, Andy Shevchenko, Min Guo,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel@vger.kernel.org, linux-usb, Linux ARM, ARM/Mediatek 
In-Reply-To: <1563958245-6321-1-git-send-email-chunfeng.yun@mediatek.com>

On Wed, Jul 24, 2019 at 10:51 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:

> Because the USB Connector is introduced and the requirement of
> usb-connector.txt binding, the old way using extcon to support
> USB Dual-Role switch is now deprecated, meanwhile there is no
> available common driver when use Type-B connector, typically
> using an input GPIO to detect USB ID pin.

However while this was going on,
drivers/extcon/extcon-fsa9480.c was merged and that detects
not only GPIO on the USB port but multiplexed usecases such
as UART over the USB micro PHY (and no that UART is not
a USB UART, but an actual RX/TX over D-/D+).

That driver also measure a whole slew of funny resistance
values on the ID pin, that is how it does its job.

But for just "hey I'm plugged in" we can surely keep this
ID on GPIO detection in the USB subsystem.

I just get a bit insecure about how we should ideally
handle these "funny-PHY's".

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH 6/8] clk: mvebu: ap806: add AP-DCLK (hclk) to system controller driver
From: Miquel Raynal @ 2019-08-05 10:03 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Michael Turquette, Stephen Boyd
  Cc: devicetree, linux-clk, Thomas Petazzoni, Gregory Clement,
	Antoine Tenart, Maxime Chevallier, Nadav Haklai,
	Grzegorz Jaszczyk, Marcin Wojtas, Stefan Chulski, Yan Markman,
	Omri Itach, Miquel Raynal
In-Reply-To: <20190805100310.29048-1-miquel.raynal@bootlin.com>

From: Omri Itach <omrii@marvell.com>

Add dynamic AP-DCLK clock (hclk) to system controller driver. AP-DCLK
is half the rate of DDR clock, so its derrived from Sample At Reset
configuration. The clock frequency is required for AP806 AXI monitor
profiling feature.

Signed-off-by: Omri Itach <omrii@marvell.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/clk/mvebu/ap806-system-controller.c | 48 ++++++++++++++++++++-
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/mvebu/ap806-system-controller.c b/drivers/clk/mvebu/ap806-system-controller.c
index 2cf874f01394..bc43adff02e0 100644
--- a/drivers/clk/mvebu/ap806-system-controller.c
+++ b/drivers/clk/mvebu/ap806-system-controller.c
@@ -21,7 +21,7 @@
 #define AP806_SAR_REG			0x400
 #define AP806_SAR_CLKFREQ_MODE_MASK	0x1f
 
-#define AP806_CLK_NUM			5
+#define AP806_CLK_NUM			6
 
 static struct clk *ap806_clks[AP806_CLK_NUM];
 
@@ -33,7 +33,7 @@ static struct clk_onecell_data ap806_clk_data = {
 static int ap806_syscon_common_probe(struct platform_device *pdev,
 				     struct device_node *syscon_node)
 {
-	unsigned int freq_mode, cpuclk_freq;
+	unsigned int freq_mode, cpuclk_freq, dclk_freq;
 	const char *name, *fixedclk_name;
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
@@ -93,8 +93,42 @@ static int ap806_syscon_common_probe(struct platform_device *pdev,
 		return -EINVAL;
 	}
 
+	/* Get DCLK frequency (DCLK = DDR_CLK / 2) */
+	switch (freq_mode) {
+	case 0x0:
+	case 0x6:
+		/* DDR_CLK = 1200Mhz */
+		dclk_freq = 600;
+		break;
+	case 0x1:
+	case 0x7:
+	case 0xD:
+		/* DDR_CLK = 1050Mhz */
+		dclk_freq = 525;
+		break;
+	case 0x13:
+	case 0x17:
+		/* DDR_CLK = 650Mhz */
+		dclk_freq = 325;
+		break;
+	case 0x4:
+	case 0x14:
+	case 0x19:
+	case 0x1A:
+	case 0x1B:
+	case 0x1C:
+	case 0x1D:
+		/* DDR_CLK = 800Mhz */
+		dclk_freq = 400;
+		break;
+	default:
+		dclk_freq = 0;
+		dev_err(dev, "invalid Sample at Reset value\n");
+	}
+
 	/* Convert to hertz */
 	cpuclk_freq *= 1000 * 1000;
+	dclk_freq *= 1000 * 1000;
 
 	/* CPU clocks depend on the Sample At Reset configuration */
 	name = ap_cp_unique_name(dev, syscon_node, "pll-cluster-0");
@@ -141,6 +175,14 @@ static int ap806_syscon_common_probe(struct platform_device *pdev,
 		goto fail4;
 	}
 
+	/* AP-DCLK(HCLK) Clock is DDR clock divided by 2 */
+	name = ap_cp_unique_name(dev, syscon_node, "ap-dclk");
+	ap806_clks[5] = clk_register_fixed_rate(dev, name, NULL, 0, dclk_freq);
+	if (IS_ERR(ap806_clks[5])) {
+		ret = PTR_ERR(ap806_clks[5]);
+		goto fail5;
+	}
+
 	ret = of_clk_add_provider(np, of_clk_src_onecell_get, &ap806_clk_data);
 	if (ret)
 		goto fail_clk_add;
@@ -148,6 +190,8 @@ static int ap806_syscon_common_probe(struct platform_device *pdev,
 	return 0;
 
 fail_clk_add:
+	clk_unregister_fixed_factor(ap806_clks[5]);
+fail5:
 	clk_unregister_fixed_factor(ap806_clks[4]);
 fail4:
 	clk_unregister_fixed_factor(ap806_clks[3]);
-- 
2.20.1

^ permalink raw reply related

* [PATCH 1/8] dt-bindings: ap80x: Document AP807 CPU clock compatible
From: Miquel Raynal @ 2019-08-05 10:03 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Michael Turquette, Stephen Boyd
  Cc: devicetree, linux-clk, Thomas Petazzoni, Gregory Clement,
	Antoine Tenart, Maxime Chevallier, Nadav Haklai,
	Grzegorz Jaszczyk, Marcin Wojtas, Stefan Chulski, Yan Markman,
	Miquel Raynal
In-Reply-To: <20190805100310.29048-1-miquel.raynal@bootlin.com>

Add AP807 CPU clock compatible to the bindings.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../bindings/arm/marvell/ap806-system-controller.txt     | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt b/Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt
index 4f21c1024073..59b6b992fbc9 100644
--- a/Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt
+++ b/Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt
@@ -147,11 +147,14 @@ ap_syscon1: system-controller@6f8000 {
 Cluster clocks:
 ---------------
 
-Device Tree Clock bindings for cluster clock of AP806 Marvell. Each
-cluster contain up to 2 CPUs running at the same frequency.
+Device Tree Clock bindings for cluster clock of Marvell
+AP806/AP807. Each cluster contain up to 2 CPUs running at the same
+frequency.
 
 Required properties:
-- compatible: must be  "marvell,ap806-cpu-clock";
+ - compatible: must be one of:
+   * "marvell,ap806-cpu-clock"
+   * "marvell,ap807-cpu-clock"
 - #clock-cells : should be set to 1.
 
 - clocks : shall be the input parent clock(s) phandle for the clock
-- 
2.20.1

^ permalink raw reply related

* [PATCH 0/8] AP807 clocks support
From: Miquel Raynal @ 2019-08-05 10:03 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Michael Turquette, Stephen Boyd
  Cc: devicetree, linux-clk, Thomas Petazzoni, Gregory Clement,
	Antoine Tenart, Maxime Chevallier, Nadav Haklai,
	Grzegorz Jaszczyk, Marcin Wojtas, Stefan Chulski, Yan Markman,
	Miquel Raynal

Hello,

This is the first batch of changes (out of three) to support the brand
new Marvell CN9130 SoCs which are made of one AP807 and one CP115.

This clock series applies on top of Gregory's "AP806 CPU clocks" [1].

[1] https://patchwork.kernel.org/cover/11038577/

Thanks,
Miquèl


Ben Peled (3):
  clk: mvebu: ap80x-cpu: add AP807 CPU clock support
  clk: mvebu: ap806: Prepare the introduction of AP807 clock support
  clk: mvebu: ap80x: add AP807 clock support

Christine Gharzuzi (1):
  clk: mvebu: ap806-cpu: prepare mapping of AP807 CPU clock

Miquel Raynal (3):
  dt-bindings: ap80x: Document AP807 CPU clock compatible
  dt-bindings: ap806: Document AP807 clock compatible
  clk: mvebu: ap806: be more explicit on what SaR is

Omri Itach (1):
  clk: mvebu: ap806: add AP-DCLK (hclk) to system controller driver

 .../arm/marvell/ap806-system-controller.txt   |  17 +-
 drivers/clk/mvebu/ap-cpu-clk.c                | 139 ++++++++++++---
 drivers/clk/mvebu/ap806-system-controller.c   | 162 ++++++++++++++----
 3 files changed, 253 insertions(+), 65 deletions(-)

-- 
2.20.1

^ permalink raw reply

* [PATCH 8/8] clk: mvebu: ap80x: add AP807 clock support
From: Miquel Raynal @ 2019-08-05 10:03 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Michael Turquette, Stephen Boyd
  Cc: devicetree, linux-clk, Thomas Petazzoni, Gregory Clement,
	Antoine Tenart, Maxime Chevallier, Nadav Haklai,
	Grzegorz Jaszczyk, Marcin Wojtas, Stefan Chulski, Yan Markman,
	Ben Peled, Miquel Raynal
In-Reply-To: <20190805100310.29048-1-miquel.raynal@bootlin.com>

From: Ben Peled <bpeled@marvell.com>

Add driver support for AP807 clock.

Signed-off-by: Ben Peled <bpeled@marvell.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/clk/mvebu/ap806-system-controller.c | 28 +++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/clk/mvebu/ap806-system-controller.c b/drivers/clk/mvebu/ap806-system-controller.c
index c64e2cc4a3ba..948bd1e71aea 100644
--- a/drivers/clk/mvebu/ap806-system-controller.c
+++ b/drivers/clk/mvebu/ap806-system-controller.c
@@ -102,6 +102,30 @@ static int ap806_get_sar_clocks(unsigned int freq_mode,
 	return 0;
 }
 
+static int ap807_get_sar_clocks(unsigned int freq_mode,
+				unsigned int *cpuclk_freq,
+				unsigned int *dclk_freq)
+{
+	switch (freq_mode) {
+	case 0x0:
+		*cpuclk_freq = 2000;
+		*dclk_freq = 1200;
+		break;
+	case 0x6:
+		*cpuclk_freq = 2200;
+		*dclk_freq = 1200;
+		break;
+	case 0xD:
+		*cpuclk_freq = 1600;
+		*dclk_freq = 1200;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int ap806_syscon_common_probe(struct platform_device *pdev,
 				     struct device_node *syscon_node)
 {
@@ -130,6 +154,9 @@ static int ap806_syscon_common_probe(struct platform_device *pdev,
 	if (of_device_is_compatible(pdev->dev.of_node,
 				    "marvell,ap806-clock")) {
 		ret = ap806_get_sar_clocks(freq_mode, &cpuclk_freq, &dclk_freq);
+	} else if (of_device_is_compatible(pdev->dev.of_node,
+					   "marvell,ap807-clock")) {
+		ret = ap807_get_sar_clocks(freq_mode, &cpuclk_freq, &dclk_freq);
 	} else {
 		dev_err(dev, "compatible not supported\n");
 		return -EINVAL;
@@ -252,6 +279,7 @@ builtin_platform_driver(ap806_syscon_legacy_driver);
 
 static const struct of_device_id ap806_clock_of_match[] = {
 	{ .compatible = "marvell,ap806-clock", },
+	{ .compatible = "marvell,ap807-clock", },
 	{ }
 };
 
-- 
2.20.1

^ permalink raw reply related

* [PATCH 7/8] clk: mvebu: ap806: Prepare the introduction of AP807 clock support
From: Miquel Raynal @ 2019-08-05 10:03 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Michael Turquette, Stephen Boyd
  Cc: devicetree, linux-clk, Thomas Petazzoni, Gregory Clement,
	Antoine Tenart, Maxime Chevallier, Nadav Haklai,
	Grzegorz Jaszczyk, Marcin Wojtas, Stefan Chulski, Yan Markman,
	Ben Peled, Miquel Raynal
In-Reply-To: <20190805100310.29048-1-miquel.raynal@bootlin.com>

From: Ben Peled <bpeled@marvell.com>

Factor out the code that is only useful to AP806 so it will be easier
to support AP807. No functional changes.

Signed-off-by: Ben Peled <bpeled@marvell.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/clk/mvebu/ap806-system-controller.c | 146 +++++++++++---------
 1 file changed, 80 insertions(+), 66 deletions(-)

diff --git a/drivers/clk/mvebu/ap806-system-controller.c b/drivers/clk/mvebu/ap806-system-controller.c
index bc43adff02e0..c64e2cc4a3ba 100644
--- a/drivers/clk/mvebu/ap806-system-controller.c
+++ b/drivers/clk/mvebu/ap806-system-controller.c
@@ -30,6 +30,78 @@ static struct clk_onecell_data ap806_clk_data = {
 	.clk_num = AP806_CLK_NUM,
 };
 
+static int ap806_get_sar_clocks(unsigned int freq_mode,
+				unsigned int *cpuclk_freq,
+				unsigned int *dclk_freq)
+{
+	switch (freq_mode) {
+	case 0x0:
+		*cpuclk_freq = 2000;
+		*dclk_freq = 600;
+		break;
+	case 0x1:
+		*cpuclk_freq = 2000;
+		*dclk_freq = 525;
+		break;
+	case 0x6:
+		*cpuclk_freq = 1800;
+		*dclk_freq = 600;
+		break;
+	case 0x7:
+		*cpuclk_freq = 1800;
+		*dclk_freq = 525;
+		break;
+	case 0x4:
+		*cpuclk_freq = 1600;
+		*dclk_freq = 400;
+		break;
+	case 0xB:
+		*cpuclk_freq = 1600;
+		*dclk_freq = 450;
+		break;
+	case 0xD:
+		*cpuclk_freq = 1600;
+		*dclk_freq = 525;
+		break;
+	case 0x1a:
+		*cpuclk_freq = 1400;
+		*dclk_freq = 400;
+		break;
+	case 0x14:
+		*cpuclk_freq = 1300;
+		*dclk_freq = 400;
+		break;
+	case 0x17:
+		*cpuclk_freq = 1300;
+		*dclk_freq = 325;
+		break;
+	case 0x19:
+		*cpuclk_freq = 1200;
+		*dclk_freq = 400;
+		break;
+	case 0x13:
+		*cpuclk_freq = 1000;
+		*dclk_freq = 325;
+		break;
+	case 0x1d:
+		*cpuclk_freq = 1000;
+		*dclk_freq = 400;
+		break;
+	case 0x1c:
+		*cpuclk_freq = 800;
+		*dclk_freq = 400;
+		break;
+	case 0x1b:
+		*cpuclk_freq = 600;
+		*dclk_freq = 400;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int ap806_syscon_common_probe(struct platform_device *pdev,
 				     struct device_node *syscon_node)
 {
@@ -54,76 +126,18 @@ static int ap806_syscon_common_probe(struct platform_device *pdev,
 	}
 
 	freq_mode = reg & AP806_SAR_CLKFREQ_MODE_MASK;
-	switch (freq_mode) {
-	case 0x0:
-	case 0x1:
-		cpuclk_freq = 2000;
-		break;
-	case 0x6:
-	case 0x7:
-		cpuclk_freq = 1800;
-		break;
-	case 0x4:
-	case 0xB:
-	case 0xD:
-		cpuclk_freq = 1600;
-		break;
-	case 0x1a:
-		cpuclk_freq = 1400;
-		break;
-	case 0x14:
-	case 0x17:
-		cpuclk_freq = 1300;
-		break;
-	case 0x19:
-		cpuclk_freq = 1200;
-		break;
-	case 0x13:
-	case 0x1d:
-		cpuclk_freq = 1000;
-		break;
-	case 0x1c:
-		cpuclk_freq = 800;
-		break;
-	case 0x1b:
-		cpuclk_freq = 600;
-		break;
-	default:
-		dev_err(dev, "invalid Sample at Reset value\n");
+
+	if (of_device_is_compatible(pdev->dev.of_node,
+				    "marvell,ap806-clock")) {
+		ret = ap806_get_sar_clocks(freq_mode, &cpuclk_freq, &dclk_freq);
+	} else {
+		dev_err(dev, "compatible not supported\n");
 		return -EINVAL;
 	}
 
-	/* Get DCLK frequency (DCLK = DDR_CLK / 2) */
-	switch (freq_mode) {
-	case 0x0:
-	case 0x6:
-		/* DDR_CLK = 1200Mhz */
-		dclk_freq = 600;
-		break;
-	case 0x1:
-	case 0x7:
-	case 0xD:
-		/* DDR_CLK = 1050Mhz */
-		dclk_freq = 525;
-		break;
-	case 0x13:
-	case 0x17:
-		/* DDR_CLK = 650Mhz */
-		dclk_freq = 325;
-		break;
-	case 0x4:
-	case 0x14:
-	case 0x19:
-	case 0x1A:
-	case 0x1B:
-	case 0x1C:
-	case 0x1D:
-		/* DDR_CLK = 800Mhz */
-		dclk_freq = 400;
-		break;
-	default:
-		dclk_freq = 0;
+	if (ret) {
 		dev_err(dev, "invalid Sample at Reset value\n");
+		return ret;
 	}
 
 	/* Convert to hertz */
-- 
2.20.1

^ permalink raw reply related

* [PATCH 5/8] clk: mvebu: ap806: be more explicit on what SaR is
From: Miquel Raynal @ 2019-08-05 10:03 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Michael Turquette, Stephen Boyd
  Cc: devicetree, linux-clk, Thomas Petazzoni, Gregory Clement,
	Antoine Tenart, Maxime Chevallier, Nadav Haklai,
	Grzegorz Jaszczyk, Marcin Wojtas, Stefan Chulski, Yan Markman,
	Miquel Raynal
In-Reply-To: <20190805100310.29048-1-miquel.raynal@bootlin.com>

"SaR" means Sample at Reset. DIP switches can be changed on the board,
their states at reset time is available through a register read.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/clk/mvebu/ap806-system-controller.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/mvebu/ap806-system-controller.c b/drivers/clk/mvebu/ap806-system-controller.c
index 73ba8fd7860f..2cf874f01394 100644
--- a/drivers/clk/mvebu/ap806-system-controller.c
+++ b/drivers/clk/mvebu/ap806-system-controller.c
@@ -89,7 +89,7 @@ static int ap806_syscon_common_probe(struct platform_device *pdev,
 		cpuclk_freq = 600;
 		break;
 	default:
-		dev_err(dev, "invalid SAR value\n");
+		dev_err(dev, "invalid Sample at Reset value\n");
 		return -EINVAL;
 	}
 
-- 
2.20.1

^ permalink raw reply related

* [PATCH 4/8] clk: mvebu: ap80x-cpu: add AP807 CPU clock support
From: Miquel Raynal @ 2019-08-05 10:03 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Michael Turquette, Stephen Boyd
  Cc: devicetree, linux-clk, Thomas Petazzoni, Gregory Clement,
	Antoine Tenart, Maxime Chevallier, Nadav Haklai,
	Grzegorz Jaszczyk, Marcin Wojtas, Stefan Chulski, Yan Markman,
	Ben Peled, Miquel Raynal
In-Reply-To: <20190805100310.29048-1-miquel.raynal@bootlin.com>

From: Ben Peled <bpeled@marvell.com>

Enhance the ap-cpu-clk driver to support both AP806 and AP807 CPU
clocks.

Signed-off-by: Ben Peled <bpeled@marvell.com>
[<miquel.raynal@bootlin.com>: use device data instead of conditions on
the compatible]
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/clk/mvebu/ap-cpu-clk.c | 59 ++++++++++++++++++++++++++++++++--
 1 file changed, 57 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/mvebu/ap-cpu-clk.c b/drivers/clk/mvebu/ap-cpu-clk.c
index 784104f6793b..af5e5acad370 100644
--- a/drivers/clk/mvebu/ap-cpu-clk.c
+++ b/drivers/clk/mvebu/ap-cpu-clk.c
@@ -45,6 +45,7 @@ struct cpu_dfs_regs {
 	unsigned int cluster_offset;
 	unsigned int force_mask;
 	int divider_offset;
+	int divider_ratio;
 	int ratio_offset;
 	int ratio_state_offset;
 	int ratio_state_cluster_offset;
@@ -58,6 +59,7 @@ struct cpu_dfs_regs {
 
 #define AP806_CA72MP2_0_PLL_CR_CLUSTER_OFFSET		0x14
 #define AP806_PLL_CR_0_CPU_CLK_DIV_RATIO_OFFSET		0
+#define AP806_PLL_CR_CPU_CLK_DIV_RATIO			0
 #define AP806_PLL_CR_0_CPU_CLK_DIV_RATIO_MASK \
 			(0x3f << AP806_PLL_CR_0_CPU_CLK_DIV_RATIO_OFFSET)
 #define AP806_PLL_CR_0_CPU_CLK_RELOAD_FORCE_OFFSET	24
@@ -81,11 +83,47 @@ static const struct cpu_dfs_regs ap806_dfs_regs = {
 	.cluster_offset = AP806_CA72MP2_0_PLL_CR_CLUSTER_OFFSET,
 	.force_mask = AP806_PLL_CR_0_CPU_CLK_RELOAD_FORCE_MASK,
 	.divider_offset = AP806_PLL_CR_0_CPU_CLK_DIV_RATIO_OFFSET,
+	.divider_ratio = AP806_PLL_CR_CPU_CLK_DIV_RATIO,
 	.ratio_offset = AP806_PLL_CR_0_CPU_CLK_RELOAD_RATIO_OFFSET,
 	.ratio_state_offset = AP806_CA72MP2_0_PLL_RATIO_STABLE_OFFSET,
 	.ratio_state_cluster_offset = AP806_CA72MP2_0_PLL_RATIO_STABLE_OFFSET,
 };
 
+/* AP807 CPU DFS register mapping */
+#define AP807_DEVICE_GENERAL_CONTROL_10_REG_OFFSET		0x278
+#define AP807_DEVICE_GENERAL_CONTROL_11_REG_OFFSET		0x27c
+#define AP807_DEVICE_GENERAL_STATUS_6_REG_OFFSET		0xc98
+#define AP807_CA72MP2_0_PLL_CR_CLUSTER_OFFSET			0x8
+#define AP807_PLL_CR_0_CPU_CLK_DIV_RATIO_OFFSET			18
+#define AP807_PLL_CR_0_CPU_CLK_DIV_RATIO_MASK \
+		(0x3f << AP807_PLL_CR_0_CPU_CLK_DIV_RATIO_OFFSET)
+#define AP807_PLL_CR_1_CPU_CLK_DIV_RATIO_OFFSET			12
+#define AP807_PLL_CR_1_CPU_CLK_DIV_RATIO_MASK \
+		(0x3f << AP807_PLL_CR_1_CPU_CLK_DIV_RATIO_OFFSET)
+#define AP807_PLL_CR_CPU_CLK_DIV_RATIO				3
+#define AP807_PLL_CR_0_CPU_CLK_RELOAD_FORCE_OFFSET		0
+#define AP807_PLL_CR_0_CPU_CLK_RELOAD_FORCE_MASK \
+		(0x3 << AP807_PLL_CR_0_CPU_CLK_RELOAD_FORCE_OFFSET)
+#define AP807_PLL_CR_0_CPU_CLK_RELOAD_RATIO_OFFSET		6
+#define	AP807_CA72MP2_0_PLL_CLKDIV_RATIO_STABLE_OFFSET		20
+#define AP807_CA72MP2_0_PLL_CLKDIV_RATIO_STABLE_CLUSTER_OFFSET	3
+
+static const struct cpu_dfs_regs ap807_dfs_regs = {
+	.divider_reg = AP807_DEVICE_GENERAL_CONTROL_10_REG_OFFSET,
+	.force_reg = AP807_DEVICE_GENERAL_CONTROL_11_REG_OFFSET,
+	.ratio_reg = AP807_DEVICE_GENERAL_CONTROL_11_REG_OFFSET,
+	.ratio_state_reg = AP807_DEVICE_GENERAL_STATUS_6_REG_OFFSET,
+	.divider_mask = AP807_PLL_CR_0_CPU_CLK_DIV_RATIO_MASK,
+	.cluster_offset = AP807_CA72MP2_0_PLL_CR_CLUSTER_OFFSET,
+	.force_mask = AP807_PLL_CR_0_CPU_CLK_RELOAD_FORCE_MASK,
+	.divider_offset = AP807_PLL_CR_0_CPU_CLK_DIV_RATIO_OFFSET,
+	.divider_ratio = AP807_PLL_CR_CPU_CLK_DIV_RATIO,
+	.ratio_offset = AP807_PLL_CR_0_CPU_CLK_RELOAD_RATIO_OFFSET,
+	.ratio_state_offset = AP807_CA72MP2_0_PLL_CLKDIV_RATIO_STABLE_OFFSET,
+	.ratio_state_cluster_offset =
+		AP807_CA72MP2_0_PLL_CLKDIV_RATIO_STABLE_CLUSTER_OFFSET
+};
+
 /*
  * struct ap806_clk: CPU cluster clock controller instance
  * @cluster: Cluster clock controller index
@@ -133,8 +171,21 @@ static int ap_cpu_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 	cpu_ratio_reg = clk->pll_regs->ratio_reg +
 		(clk->cluster * clk->pll_regs->cluster_offset);
 
-	regmap_update_bits(clk->pll_cr_base, cpu_clkdiv_reg,
-			   clk->pll_regs->divider_mask, divider);
+	regmap_read(clk->pll_cr_base, cpu_clkdiv_reg, &reg);
+	reg &= ~(clk->pll_regs->divider_mask);
+	reg |= (divider << clk->pll_regs->divider_offset);
+
+	/*
+	 * AP807 CPU divider has two channels with ratio 1:3 and divider_ratio
+	 * is 1. Otherwise, in the case of the AP806, divider_ratio is 0.
+	 */
+	if (clk->pll_regs->divider_ratio) {
+		reg &= ~(AP807_PLL_CR_1_CPU_CLK_DIV_RATIO_MASK);
+		reg |= ((divider * clk->pll_regs->divider_ratio) <<
+				AP807_PLL_CR_1_CPU_CLK_DIV_RATIO_OFFSET);
+	}
+	regmap_write(clk->pll_cr_base, cpu_clkdiv_reg, reg);
+
 
 	regmap_update_bits(clk->pll_cr_base, cpu_force_reg,
 			   clk->pll_regs->force_mask,
@@ -287,6 +338,10 @@ static const struct of_device_id ap_cpu_clock_of_match[] = {
 		.compatible = "marvell,ap806-cpu-clock",
 		.data = &ap806_dfs_regs,
 	},
+	{
+		.compatible = "marvell,ap807-cpu-clock",
+		.data = &ap807_dfs_regs,
+	},
 	{ }
 };
 
-- 
2.20.1

^ permalink raw reply related

* [PATCH 3/8] clk: mvebu: ap806-cpu: prepare mapping of AP807 CPU clock
From: Miquel Raynal @ 2019-08-05 10:03 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Michael Turquette, Stephen Boyd
  Cc: devicetree, linux-clk, Thomas Petazzoni, Gregory Clement,
	Antoine Tenart, Maxime Chevallier, Nadav Haklai,
	Grzegorz Jaszczyk, Marcin Wojtas, Stefan Chulski, Yan Markman,
	Christine Gharzuzi, Miquel Raynal
In-Reply-To: <20190805100310.29048-1-miquel.raynal@bootlin.com>

From: Christine Gharzuzi <chrisg@marvell.com>

This patch allows same flow to be executed on chips with different
register mappings like AP806 and, in the future, AP807.

Note: this patch has no functional effect, and only prepares the
driver for additional chips to be supported by retrieving the right
device data depenging on the compatible property.

Signed-off-by: Christine Gharzuzi <chrisg@marvell.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/clk/mvebu/ap-cpu-clk.c | 82 +++++++++++++++++++++++++---------
 1 file changed, 62 insertions(+), 20 deletions(-)

diff --git a/drivers/clk/mvebu/ap-cpu-clk.c b/drivers/clk/mvebu/ap-cpu-clk.c
index e4cecb456884..784104f6793b 100644
--- a/drivers/clk/mvebu/ap-cpu-clk.c
+++ b/drivers/clk/mvebu/ap-cpu-clk.c
@@ -15,6 +15,7 @@
 #include <linux/mfd/syscon.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include "armada_ap_cp_helper.h"
@@ -29,6 +30,26 @@
 
 #define APN806_MAX_DIVIDER		32
 
+/**
+ * struct cpu_dfs_regs: CPU DFS register mapping
+ * @divider_reg: full integer ratio from PLL frequency to CPU clock frequency
+ * @force_reg: request to force new ratio regardless of relation to other clocks
+ * @ratio_reg: central request to switch ratios
+ */
+struct cpu_dfs_regs {
+	unsigned int divider_reg;
+	unsigned int force_reg;
+	unsigned int ratio_reg;
+	unsigned int ratio_state_reg;
+	unsigned int divider_mask;
+	unsigned int cluster_offset;
+	unsigned int force_mask;
+	int divider_offset;
+	int ratio_offset;
+	int ratio_state_offset;
+	int ratio_state_cluster_offset;
+};
+
 /* AP806 CPU DFS register mapping*/
 #define AP806_CA72MP2_0_PLL_CR_0_REG_OFFSET		0x278
 #define AP806_CA72MP2_0_PLL_CR_1_REG_OFFSET		0x280
@@ -43,6 +64,7 @@
 #define AP806_PLL_CR_0_CPU_CLK_RELOAD_FORCE_MASK \
 			(0x1 << AP806_PLL_CR_0_CPU_CLK_RELOAD_FORCE_OFFSET)
 #define AP806_PLL_CR_0_CPU_CLK_RELOAD_RATIO_OFFSET	16
+#define AP806_CA72MP2_0_PLL_RATIO_STABLE_OFFSET	0
 #define AP806_CA72MP2_0_PLL_RATIO_STATE			11
 
 #define STATUS_POLL_PERIOD_US		1
@@ -50,6 +72,20 @@
 
 #define to_ap_cpu_clk(_hw) container_of(_hw, struct ap_cpu_clk, hw)
 
+static const struct cpu_dfs_regs ap806_dfs_regs = {
+	.divider_reg = AP806_CA72MP2_0_PLL_CR_0_REG_OFFSET,
+	.force_reg = AP806_CA72MP2_0_PLL_CR_1_REG_OFFSET,
+	.ratio_reg = AP806_CA72MP2_0_PLL_CR_2_REG_OFFSET,
+	.ratio_state_reg = AP806_CA72MP2_0_PLL_SR_REG_OFFSET,
+	.divider_mask = AP806_PLL_CR_0_CPU_CLK_DIV_RATIO_MASK,
+	.cluster_offset = AP806_CA72MP2_0_PLL_CR_CLUSTER_OFFSET,
+	.force_mask = AP806_PLL_CR_0_CPU_CLK_RELOAD_FORCE_MASK,
+	.divider_offset = AP806_PLL_CR_0_CPU_CLK_DIV_RATIO_OFFSET,
+	.ratio_offset = AP806_PLL_CR_0_CPU_CLK_RELOAD_RATIO_OFFSET,
+	.ratio_state_offset = AP806_CA72MP2_0_PLL_RATIO_STABLE_OFFSET,
+	.ratio_state_cluster_offset = AP806_CA72MP2_0_PLL_RATIO_STABLE_OFFSET,
+};
+
 /*
  * struct ap806_clk: CPU cluster clock controller instance
  * @cluster: Cluster clock controller index
@@ -64,6 +100,7 @@ struct ap_cpu_clk {
 	struct device *dev;
 	struct clk_hw hw;
 	struct regmap *pll_cr_base;
+	const struct cpu_dfs_regs *pll_regs;
 };
 
 static unsigned long ap_cpu_clk_recalc_rate(struct clk_hw *hw,
@@ -73,11 +110,11 @@ static unsigned long ap_cpu_clk_recalc_rate(struct clk_hw *hw,
 	unsigned int cpu_clkdiv_reg;
 	int cpu_clkdiv_ratio;
 
-	cpu_clkdiv_reg = AP806_CA72MP2_0_PLL_CR_0_REG_OFFSET +
-		(clk->cluster * AP806_CA72MP2_0_PLL_CR_CLUSTER_OFFSET);
+	cpu_clkdiv_reg = clk->pll_regs->divider_reg +
+		(clk->cluster * clk->pll_regs->cluster_offset);
 	regmap_read(clk->pll_cr_base, cpu_clkdiv_reg, &cpu_clkdiv_ratio);
-	cpu_clkdiv_ratio &= AP806_PLL_CR_0_CPU_CLK_DIV_RATIO_MASK;
-	cpu_clkdiv_ratio >>= AP806_PLL_CR_0_CPU_CLK_DIV_RATIO_OFFSET;
+	cpu_clkdiv_ratio &= clk->pll_regs->divider_mask;
+	cpu_clkdiv_ratio >>= clk->pll_regs->divider_offset;
 
 	return parent_rate / cpu_clkdiv_ratio;
 }
@@ -89,35 +126,36 @@ static int ap_cpu_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 	int ret, reg, divider = parent_rate / rate;
 	unsigned int cpu_clkdiv_reg, cpu_force_reg, cpu_ratio_reg, stable_bit;
 
-	cpu_clkdiv_reg = AP806_CA72MP2_0_PLL_CR_0_REG_OFFSET +
-		(clk->cluster * AP806_CA72MP2_0_PLL_CR_CLUSTER_OFFSET);
-	cpu_force_reg = AP806_CA72MP2_0_PLL_CR_1_REG_OFFSET +
-		(clk->cluster * AP806_CA72MP2_0_PLL_CR_CLUSTER_OFFSET);
-	cpu_ratio_reg = AP806_CA72MP2_0_PLL_CR_2_REG_OFFSET +
-		(clk->cluster * AP806_CA72MP2_0_PLL_CR_CLUSTER_OFFSET);
+	cpu_clkdiv_reg = clk->pll_regs->divider_reg +
+		(clk->cluster * clk->pll_regs->cluster_offset);
+	cpu_force_reg = clk->pll_regs->force_reg +
+		(clk->cluster * clk->pll_regs->cluster_offset);
+	cpu_ratio_reg = clk->pll_regs->ratio_reg +
+		(clk->cluster * clk->pll_regs->cluster_offset);
 
 	regmap_update_bits(clk->pll_cr_base, cpu_clkdiv_reg,
-			   AP806_PLL_CR_0_CPU_CLK_DIV_RATIO_MASK, divider);
+			   clk->pll_regs->divider_mask, divider);
 
 	regmap_update_bits(clk->pll_cr_base, cpu_force_reg,
-			   AP806_PLL_CR_0_CPU_CLK_RELOAD_FORCE_MASK,
-			   AP806_PLL_CR_0_CPU_CLK_RELOAD_FORCE_MASK);
+			   clk->pll_regs->force_mask,
+			   clk->pll_regs->force_mask);
 
 	regmap_update_bits(clk->pll_cr_base, cpu_ratio_reg,
-			   BIT(AP806_PLL_CR_0_CPU_CLK_RELOAD_RATIO_OFFSET),
-			   BIT(AP806_PLL_CR_0_CPU_CLK_RELOAD_RATIO_OFFSET));
-
-	stable_bit = BIT(clk->cluster * AP806_CA72MP2_0_PLL_RATIO_STATE),
+			   BIT(clk->pll_regs->ratio_offset),
+			   BIT(clk->pll_regs->ratio_offset));
 
+	stable_bit = BIT(clk->pll_regs->ratio_state_offset +
+			 clk->cluster *
+			 clk->pll_regs->ratio_state_cluster_offset),
 	ret = regmap_read_poll_timeout(clk->pll_cr_base,
-				       AP806_CA72MP2_0_PLL_SR_REG_OFFSET, reg,
+				       clk->pll_regs->ratio_state_reg, reg,
 				       reg & stable_bit, STATUS_POLL_PERIOD_US,
 				       STATUS_POLL_TIMEOUT_US);
 	if (ret)
 		return ret;
 
 	regmap_update_bits(clk->pll_cr_base, cpu_ratio_reg,
-			   BIT(AP806_PLL_CR_0_CPU_CLK_RELOAD_RATIO_OFFSET), 0);
+			   BIT(clk->pll_regs->ratio_offset), 0);
 
 	return 0;
 }
@@ -222,6 +260,7 @@ static int ap_cpu_clock_probe(struct platform_device *pdev)
 		ap_cpu_clk[cluster_index].pll_cr_base = regmap;
 		ap_cpu_clk[cluster_index].hw.init = &init;
 		ap_cpu_clk[cluster_index].dev = dev;
+		ap_cpu_clk[cluster_index].pll_regs = of_device_get_match_data(&pdev->dev);
 
 		init.name = ap_cpu_clk[cluster_index].clk_name;
 		init.ops = &ap_cpu_clk_ops;
@@ -244,7 +283,10 @@ static int ap_cpu_clock_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id ap_cpu_clock_of_match[] = {
-	{ .compatible = "marvell,ap806-cpu-clock", },
+	{
+		.compatible = "marvell,ap806-cpu-clock",
+		.data = &ap806_dfs_regs,
+	},
 	{ }
 };
 
-- 
2.20.1

^ permalink raw reply related

* [PATCH 2/8] dt-bindings: ap806: Document AP807 clock compatible
From: Miquel Raynal @ 2019-08-05 10:03 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Michael Turquette, Stephen Boyd
  Cc: devicetree, linux-clk, Thomas Petazzoni, Gregory Clement,
	Antoine Tenart, Maxime Chevallier, Nadav Haklai,
	Grzegorz Jaszczyk, Marcin Wojtas, Stefan Chulski, Yan Markman,
	Miquel Raynal
In-Reply-To: <20190805100310.29048-1-miquel.raynal@bootlin.com>

Add AP807 clock compatible to the bindings.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../bindings/arm/marvell/ap806-system-controller.txt      | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt b/Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt
index 59b6b992fbc9..26410fbb85be 100644
--- a/Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt
+++ b/Documentation/devicetree/bindings/arm/marvell/ap806-system-controller.txt
@@ -18,8 +18,8 @@ Clocks:
 -------
 
 
-The Device Tree node representing the AP806 system controller provides
-a number of clocks:
+The Device Tree node representing the AP806/AP807 system controller
+provides a number of clocks:
 
  - 0: reference clock of CPU cluster 0
  - 1: reference clock of CPU cluster 1
@@ -28,7 +28,9 @@ a number of clocks:
 
 Required properties:
 
- - compatible: must be: "marvell,ap806-clock"
+ - compatible: must be one of:
+   * "marvell,ap806-clock"
+   * "marvell,ap807-clock"
  - #clock-cells: must be set to 1
 
 Pinctrl:
-- 
2.20.1

^ permalink raw reply related

* Re: [RFC,v3 6/9] media: platform: Add Mediatek ISP P1 V4L2 functions
From: Tomasz Figa @ 2019-08-05  9:59 UTC (permalink / raw)
  To: Jungo Lin
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	Sean Cheng (鄭昇弘), Mauro Carvalho Chehab,
	Rynn Wu (吳育恩), srv_heupstream, Rob Herring,
	Ryan Yu (余孟修),
	Frankie Chiu (邱文凱), Hans Verkuil,
	Matthias Brugger, Sj Huang,
	moderated list:ARM/Mediatek SoC support, Laurent Pinchart,
	ddavenport-F7+t8E8rja9g9hUCZPvPmw,
	Frederic Chen (陳俊元)
In-Reply-To: <1564451089.1212.649.camel@mtksdccf07>

Hi Jungo,

On Tue, Jul 30, 2019 at 10:45 AM Jungo Lin <jungo.lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>
> On Mon, 2019-07-29 at 19:04 +0900, Tomasz Figa wrote:
> > On Mon, Jul 29, 2019 at 10:18 AM Jungo Lin <jungo.lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> > > On Fri, 2019-07-26 at 14:49 +0900, Tomasz Figa wrote:
> > > > On Wed, Jul 24, 2019 at 1:31 PM Jungo Lin <jungo.lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> > > > > On Tue, 2019-07-23 at 19:21 +0900, Tomasz Figa wrote:
> > > > > > On Thu, Jul 18, 2019 at 1:39 PM Jungo Lin <jungo.lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> > > > > > > On Wed, 2019-07-10 at 18:54 +0900, Tomasz Figa wrote:
> > > > > > > > On Tue, Jun 11, 2019 at 11:53:41AM +0800, Jungo Lin wrote:
[snip]
> > > > > > > > > +
> > > > > > > > > +   dev_dbg(dev, "%s: node:%d fd:%d idx:%d\n",
> > > > > > > > > +           __func__,
> > > > > > > > > +           node->id,
> > > > > > > > > +           buf->vbb.request_fd,
> > > > > > > > > +           buf->vbb.vb2_buf.index);
> > > > > > > > > +
> > > > > > > > > +   /* For request buffers en-queue, handled in mtk_cam_req_try_queue */
> > > > > > > > > +   if (vb->vb2_queue->uses_requests)
> > > > > > > > > +           return;
> > > > > > > >
> > > > > > > > I'd suggest removing non-request support from this driver. Even if we end up
> > > > > > > > with a need to provide compatibility for non-request mode, then it should be
> > > > > > > > built on top of the requests mode, so that the driver itself doesn't have to
> > > > > > > > deal with two modes.
> > > > > > > >
> > > > > > >
> > > > > > > The purpose of non-request function in this driver is needed by
> > > > > > > our camera middle-ware design. It needs 3A statistics buffers before
> > > > > > > image buffers en-queue. So we need to en-queue 3A statistics with
> > > > > > > non-request mode in this driver. After MW got the 3A statistics data, it
> > > > > > > will en-queue the images, tuning buffer and other meta buffers with
> > > > > > > request mode. Based on this requirement, do you have any suggestion?
> > > > > > > For upstream driver, should we only consider request mode?
> > > > > > >
> > > > > >
> > > > > > Where does that requirement come from? Why the timing of queuing of
> > > > > > the buffers to the driver is important?
> > > > > >
> > > > > > [snip]
> > > > >
> > > > > Basically, this requirement comes from our internal camera
> > > > > middle-ware/3A hal in user space. Since this is not generic requirement,
> > > > > we will follow your original suggestion to keep the request mode only
> > > > > and remove other non-request design in other files. For upstream driver,
> > > > > it should support request mode only.
> > > > >
> > > >
> > > > Note that Chromium OS will use the "upstream driver" and we don't want
> > > > to diverge, so please make the userspace also use only requests. I
> > > > don't see a reason why there would be any need to submit any buffers
> > > > outside of a request.
> > > >
> > > > [snip]
> > >
> > > Ok, I have raised your concern to our colleagues and let him to discuss
> > > with you in another communication channel.
> > >
> >
> > Thanks!
> >
> > Best regards,
> > Tomasz
>
> Our colleague is preparing material to explain the our 3A/MW design. If
> he is ready, he will discuss this with you.

Thanks!

>
> In the original plan, we will deliver P1 v4 patch set tomorrow (31th
> Jul.). But, there are some comments waiting for other experts' input.
> Do you suggest it is better to resolve all comments before v4 patch set
> submitting or continue to discuss these comments on v4?

For the remaining v4l2-compliance issues, we can postpone them and
keep on a TODO list in the next version.

Best regards,
Tomasz

^ permalink raw reply

* Re: [PATCH v5 1/3] PM: wakeup: Add routine to help fetch wakeup source object.
From: Rafael J. Wysocki @ 2019-08-05  9:59 UTC (permalink / raw)
  To: Ran Wang
  Cc: Mark Rutland, Li Biwen, Len Brown, Greg Kroah-Hartman, linux-pm,
	linux-kernel, Li Yang, devicetree, Rob Herring, Pavel Machek,
	linuxppc-dev, linux-arm-kernel
In-Reply-To: <20190724074722.12270-1-ran.wang_1@nxp.com>

On Wednesday, July 24, 2019 9:47:20 AM CEST Ran Wang wrote:
> Some user might want to go through all registered wakeup sources
> and doing things accordingly. For example, SoC PM driver might need to
> do HW programming to prevent powering down specific IP which wakeup
> source depending on. So add this API to help walk through all registered
> wakeup source objects on that list and return them one by one.
> 
> Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
> ---
> Change in v5:
> 	- Update commit message, add decription of walk through all wakeup
> 	source objects.
> 	- Add SCU protection in function wakeup_source_get_next().
> 	- Rename wakeup_source member 'attached_dev' to 'dev' and move it up
> 	(before wakeirq).
> 
> Change in v4:
> 	- None.
> 
> Change in v3:
> 	- Adjust indentation of *attached_dev;.
> 
> Change in v2:
> 	- None.
> 
>  drivers/base/power/wakeup.c | 24 ++++++++++++++++++++++++
>  include/linux/pm_wakeup.h   |  3 +++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> index ee31d4f..2fba891 100644
> --- a/drivers/base/power/wakeup.c
> +++ b/drivers/base/power/wakeup.c
> @@ -14,6 +14,7 @@
>  #include <linux/suspend.h>
>  #include <linux/seq_file.h>
>  #include <linux/debugfs.h>
> +#include <linux/of_device.h>
>  #include <linux/pm_wakeirq.h>
>  #include <trace/events/power.h>
>  
> @@ -226,6 +227,28 @@ void wakeup_source_unregister(struct wakeup_source *ws)
>  	}
>  }
>  EXPORT_SYMBOL_GPL(wakeup_source_unregister);
> +/**
> + * wakeup_source_get_next - Get next wakeup source from the list
> + * @ws: Previous wakeup source object, null means caller want first one.
> + */
> +struct wakeup_source *wakeup_source_get_next(struct wakeup_source *ws)
> +{
> +	struct list_head *ws_head = &wakeup_sources;
> +	struct wakeup_source *next_ws = NULL;
> +	int idx;
> +
> +	idx = srcu_read_lock(&wakeup_srcu);
> +	if (ws)
> +		next_ws = list_next_or_null_rcu(ws_head, &ws->entry,
> +				struct wakeup_source, entry);
> +	else
> +		next_ws = list_entry_rcu(ws_head->next,
> +				struct wakeup_source, entry);
> +	srcu_read_unlock(&wakeup_srcu, idx);
> +

This is incorrect.

The SRCU cannot be unlocked until the caller of this is done
with the object returned by it, or that object can be freed
while it is still being accessed.

Besides, this patch conflicts with some general wakeup sources
changes in the works, so it needs to be deferred and rebased on
top of those changes.

> +	return next_ws;
> +}
> +EXPORT_SYMBOL_GPL(wakeup_source_get_next);
>  
>  /**
>   * device_wakeup_attach - Attach a wakeup source object to a device object.
> @@ -242,6 +265,7 @@ static int device_wakeup_attach(struct device *dev, struct wakeup_source *ws)
>  		return -EEXIST;
>  	}
>  	dev->power.wakeup = ws;
> +	ws->dev = dev;
>  	if (dev->power.wakeirq)
>  		device_wakeup_attach_irq(dev, dev->power.wakeirq);
>  	spin_unlock_irq(&dev->power.lock);
> diff --git a/include/linux/pm_wakeup.h b/include/linux/pm_wakeup.h
> index 9102760..fc23c1a 100644
> --- a/include/linux/pm_wakeup.h
> +++ b/include/linux/pm_wakeup.h
> @@ -23,6 +23,7 @@ struct wake_irq;
>   * @name: Name of the wakeup source
>   * @entry: Wakeup source list entry
>   * @lock: Wakeup source lock
> + * @dev: The device it attached to
>   * @wakeirq: Optional device specific wakeirq
>   * @timer: Wakeup timer list
>   * @timer_expires: Wakeup timer expiration
> @@ -42,6 +43,7 @@ struct wakeup_source {
>  	const char 		*name;
>  	struct list_head	entry;
>  	spinlock_t		lock;
> +	struct device		*dev;
>  	struct wake_irq		*wakeirq;
>  	struct timer_list	timer;
>  	unsigned long		timer_expires;
> @@ -88,6 +90,7 @@ extern void wakeup_source_add(struct wakeup_source *ws);
>  extern void wakeup_source_remove(struct wakeup_source *ws);
>  extern struct wakeup_source *wakeup_source_register(const char *name);
>  extern void wakeup_source_unregister(struct wakeup_source *ws);
> +extern struct wakeup_source *wakeup_source_get_next(struct wakeup_source *ws);
>  extern int device_wakeup_enable(struct device *dev);
>  extern int device_wakeup_disable(struct device *dev);
>  extern void device_set_wakeup_capable(struct device *dev, bool capable);
> 

^ permalink raw reply

* [PATCH V4 3/3] mmc: mmci: sdmmc: add busy_complete callback
From: Ludovic Barre @ 2019-08-05  9:56 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring
  Cc: srinivas.kandagatla, Maxime Coquelin, Alexandre Torgue,
	linux-arm-kernel, linux-kernel, devicetree, linux-mmc,
	linux-stm32, Ludovic Barre
In-Reply-To: <20190805095626.25998-1-ludovic.Barre@st.com>

From: Ludovic Barre <ludovic.barre@st.com>

This patch adds a specific busy_complete callback for sdmmc variant.

sdmmc has 2 status flags:
-busyd0: This is a hardware status flag (inverted value of d0 line).
it does not generate an interrupt.
-busyd0end: This indicates only end of busy following a CMD response.
On busy to Not busy changes, an interrupt is generated (if unmask)
and BUSYD0END status flag is set. Status flag is cleared by writing
corresponding interrupt clear bit in MMCICLEAR.

The legacy busy completion monitors step by step the busy progression
start/in-progress/end. On sdmmc variant, the monitoring of busy steps
is difficult and not adapted (the software can miss a step and locks
the monitoring), the sdmmc has just need to wait the busyd0end bit
without monitoring all the changes.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 drivers/mmc/host/mmci.c             |  3 +++
 drivers/mmc/host/mmci.h             |  1 +
 drivers/mmc/host/mmci_stm32_sdmmc.c | 38 +++++++++++++++++++++++++++++
 3 files changed, 42 insertions(+)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 17948615d4a5..2751415d0fd1 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -263,6 +263,9 @@ static struct variant_data variant_stm32_sdmmc = {
 	.datalength_bits	= 25,
 	.datactrl_blocksz	= 14,
 	.stm32_idmabsize_mask	= GENMASK(12, 5),
+	.busy_timeout		= true,
+	.busy_detect_flag	= MCI_STM32_BUSYD0,
+	.busy_detect_mask	= MCI_STM32_BUSYD0ENDMASK,
 	.init			= sdmmc_variant_init,
 };
 
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 03eb21f1c258..64ae7720477c 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -167,6 +167,7 @@
 #define MCI_ST_CARDBUSY		(1 << 24)
 /* Extended status bits for the STM32 variants */
 #define MCI_STM32_BUSYD0	BIT(20)
+#define MCI_STM32_BUSYD0END	BIT(21)
 
 #define MMCICLEAR		0x038
 #define MCI_CMDCRCFAILCLR	(1 << 0)
diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
index 8e83ae6920ae..bb5499cc9e81 100644
--- a/drivers/mmc/host/mmci_stm32_sdmmc.c
+++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
@@ -282,6 +282,43 @@ static u32 sdmmc_get_dctrl_cfg(struct mmci_host *host)
 	return datactrl;
 }
 
+bool sdmmc_busy_complete(struct mmci_host *host, u32 status, u32 err_msk)
+{
+	void __iomem *base = host->base;
+	u32 busy_d0, busy_d0end, mask;
+
+	mask = readl_relaxed(base + MMCIMASK0);
+	busy_d0end = readl_relaxed(base + MMCISTATUS) & MCI_STM32_BUSYD0END;
+	busy_d0 = readl_relaxed(base + MMCISTATUS) & MCI_STM32_BUSYD0;
+
+	/* complete if there is an error or busy_d0end */
+	if ((status & err_msk) || busy_d0end)
+		goto complete;
+
+	/*
+	 * On response the busy signaling is reflected in the BUSYD0 flag.
+	 * if busy_d0 is in-progress we must activate busyd0end interrupt
+	 * to wait this completion. Else this request has no busy step.
+	 */
+	if (busy_d0) {
+		if (!host->busy_status) {
+			writel_relaxed(mask | host->variant->busy_detect_mask,
+				       base + MMCIMASK0);
+			host->busy_status = status &
+				(MCI_CMDSENT | MCI_CMDRESPEND);
+		}
+		return false;
+	}
+
+complete:
+	writel_relaxed(mask & ~host->variant->busy_detect_mask,
+		       base + MMCIMASK0);
+	writel_relaxed(host->variant->busy_detect_mask, base + MMCICLEAR);
+	host->busy_status = 0;
+
+	return true;
+}
+
 static struct mmci_host_ops sdmmc_variant_ops = {
 	.validate_data = sdmmc_idma_validate_data,
 	.prep_data = sdmmc_idma_prep_data,
@@ -292,6 +329,7 @@ static struct mmci_host_ops sdmmc_variant_ops = {
 	.dma_finalize = sdmmc_idma_finalize,
 	.set_clkreg = mmci_sdmmc_set_clkreg,
 	.set_pwrreg = mmci_sdmmc_set_pwrreg,
+	.busy_complete = sdmmc_busy_complete,
 };
 
 void sdmmc_variant_init(struct mmci_host *host)
-- 
2.17.1

^ permalink raw reply related

* [PATCH V4 2/3] mmc: mmci: add busy_complete callback
From: Ludovic Barre @ 2019-08-05  9:56 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring
  Cc: srinivas.kandagatla, Maxime Coquelin, Alexandre Torgue,
	linux-arm-kernel, linux-kernel, devicetree, linux-mmc,
	linux-stm32, Ludovic Barre
In-Reply-To: <20190805095626.25998-1-ludovic.Barre@st.com>

From: Ludovic Barre <ludovic.barre@st.com>

This patch adds busy_completion callback at mmci_host_ops
to allow to define a specific busy completion by variant.

The legacy code corresponding to busy completion used
by ux500 variants is moved to ux500_busy_complete function.

The busy_detect boolean property is replaced by
busy_complete callback definition.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 drivers/mmc/host/mmci.c | 140 +++++++++++++++++++++-------------------
 drivers/mmc/host/mmci.h |   3 +-
 2 files changed, 75 insertions(+), 68 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index e79c9148af84..17948615d4a5 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -47,6 +47,7 @@
 #define DRIVER_NAME "mmci-pl18x"
 
 static void mmci_variant_init(struct mmci_host *host);
+static void ux500_variant_init(struct mmci_host *host);
 static void ux500v2_variant_init(struct mmci_host *host);
 
 static unsigned int fmax = 515633;
@@ -178,7 +179,6 @@ static struct variant_data variant_ux500 = {
 	.f_max			= 100000000,
 	.signal_direction	= true,
 	.pwrreg_clkgate		= true,
-	.busy_detect		= true,
 	.busy_dpsm_flag		= MCI_DPSM_ST_BUSYMODE,
 	.busy_detect_flag	= MCI_ST_CARDBUSY,
 	.busy_detect_mask	= MCI_ST_BUSYENDMASK,
@@ -187,7 +187,7 @@ static struct variant_data variant_ux500 = {
 	.irq_pio_mask		= MCI_IRQ_PIO_MASK,
 	.start_err		= MCI_STARTBITERR,
 	.opendrain		= MCI_OD,
-	.init			= mmci_variant_init,
+	.init			= ux500_variant_init,
 };
 
 static struct variant_data variant_ux500v2 = {
@@ -211,7 +211,6 @@ static struct variant_data variant_ux500v2 = {
 	.f_max			= 100000000,
 	.signal_direction	= true,
 	.pwrreg_clkgate		= true,
-	.busy_detect		= true,
 	.busy_dpsm_flag		= MCI_DPSM_ST_BUSYMODE,
 	.busy_detect_flag	= MCI_ST_CARDBUSY,
 	.busy_detect_mask	= MCI_ST_BUSYENDMASK,
@@ -613,6 +612,67 @@ static u32 ux500v2_get_dctrl_cfg(struct mmci_host *host)
 	return MCI_DPSM_ENABLE | (host->data->blksz << 16);
 }
 
+static bool ux500_busy_complete(struct mmci_host *host, u32 status, u32 err_msk)
+{
+	void __iomem *base = host->base;
+
+	/*
+	 * Before unmasking for the busy end IRQ, confirm that the
+	 * command was sent successfully. To keep track of having a
+	 * command in-progress, waiting for busy signaling to end,
+	 * store the status in host->busy_status.
+	 *
+	 * Note that, the card may need a couple of clock cycles before
+	 * it starts signaling busy on DAT0, hence re-read the
+	 * MMCISTATUS register here, to allow the busy bit to be set.
+	 * Potentially we may even need to poll the register for a
+	 * while, to allow it to be set, but tests indicates that it
+	 * isn't needed.
+	 */
+	if (!host->busy_status && !(status & err_msk) &&
+	    (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) {
+		writel(readl(base + MMCIMASK0) |
+		       host->variant->busy_detect_mask,
+		       base + MMCIMASK0);
+
+		host->busy_status = status & (MCI_CMDSENT | MCI_CMDRESPEND);
+		return false;
+	}
+
+	/*
+	 * If there is a command in-progress that has been successfully
+	 * sent, then bail out if busy status is set and wait for the
+	 * busy end IRQ.
+	 *
+	 * Note that, the HW triggers an IRQ on both edges while
+	 * monitoring DAT0 for busy completion, but there is only one
+	 * status bit in MMCISTATUS for the busy state. Therefore
+	 * both the start and the end interrupts needs to be cleared,
+	 * one after the other. So, clear the busy start IRQ here.
+	 */
+	if (host->busy_status &&
+	    (status & host->variant->busy_detect_flag)) {
+		writel(host->variant->busy_detect_mask, base + MMCICLEAR);
+		return false;
+	}
+
+	/*
+	 * If there is a command in-progress that has been successfully
+	 * sent and the busy bit isn't set, it means we have received
+	 * the busy end IRQ. Clear and mask the IRQ, then continue to
+	 * process the command.
+	 */
+	if (host->busy_status) {
+		writel(host->variant->busy_detect_mask, base + MMCICLEAR);
+
+		writel(readl(base + MMCIMASK0) &
+		       ~host->variant->busy_detect_mask, base + MMCIMASK0);
+		host->busy_status = 0;
+	}
+
+	return true;
+}
+
 /*
  * All the DMA operation mode stuff goes inside this ifdef.
  * This assumes that you have a generic DMA device interface,
@@ -956,9 +1016,16 @@ void mmci_variant_init(struct mmci_host *host)
 	host->ops = &mmci_variant_ops;
 }
 
+void ux500_variant_init(struct mmci_host *host)
+{
+	host->ops = &mmci_variant_ops;
+	host->ops->busy_complete = ux500_busy_complete;
+}
+
 void ux500v2_variant_init(struct mmci_host *host)
 {
 	host->ops = &mmci_variant_ops;
+	host->ops->busy_complete = ux500_busy_complete;
 	host->ops->get_datactrl_cfg = ux500v2_get_dctrl_cfg;
 }
 
@@ -1242,68 +1309,9 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 		return;
 
 	/* Handle busy detection on DAT0 if the variant supports it. */
-	if (busy_resp && host->variant->busy_detect) {
-
-		/*
-		 * Before unmasking for the busy end IRQ, confirm that the
-		 * command was sent successfully. To keep track of having a
-		 * command in-progress, waiting for busy signaling to end,
-		 * store the status in host->busy_status.
-		 *
-		 * Note that, the card may need a couple of clock cycles before
-		 * it starts signaling busy on DAT0, hence re-read the
-		 * MMCISTATUS register here, to allow the busy bit to be set.
-		 * Potentially we may even need to poll the register for a
-		 * while, to allow it to be set, but tests indicates that it
-		 * isn't needed.
-		 */
-		if (!host->busy_status && !(status & err_msk) &&
-		    (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) {
-
-			writel(readl(base + MMCIMASK0) |
-			       host->variant->busy_detect_mask,
-			       base + MMCIMASK0);
-
-			host->busy_status =
-				status & (MCI_CMDSENT|MCI_CMDRESPEND);
-			return;
-		}
-
-		/*
-		 * If there is a command in-progress that has been successfully
-		 * sent, then bail out if busy status is set and wait for the
-		 * busy end IRQ.
-		 *
-		 * Note that, the HW triggers an IRQ on both edges while
-		 * monitoring DAT0 for busy completion, but there is only one
-		 * status bit in MMCISTATUS for the busy state. Therefore
-		 * both the start and the end interrupts needs to be cleared,
-		 * one after the other. So, clear the busy start IRQ here.
-		 */
-		if (host->busy_status &&
-		    (status & host->variant->busy_detect_flag)) {
-			writel(host->variant->busy_detect_mask,
-			       host->base + MMCICLEAR);
+	if (busy_resp && host->ops->busy_complete)
+		if (!host->ops->busy_complete(host, status, err_msk))
 			return;
-		}
-
-		/*
-		 * If there is a command in-progress that has been successfully
-		 * sent and the busy bit isn't set, it means we have received
-		 * the busy end IRQ. Clear and mask the IRQ, then continue to
-		 * process the command.
-		 */
-		if (host->busy_status) {
-
-			writel(host->variant->busy_detect_mask,
-			       host->base + MMCICLEAR);
-
-			writel(readl(base + MMCIMASK0) &
-			       ~host->variant->busy_detect_mask,
-			       base + MMCIMASK0);
-			host->busy_status = 0;
-		}
-	}
 
 	host->cmd = NULL;
 
@@ -1544,7 +1552,7 @@ static irqreturn_t mmci_irq(int irq, void *dev_id)
 		 * clear the corresponding IRQ.
 		 */
 		status &= readl(host->base + MMCIMASK0);
-		if (host->variant->busy_detect)
+		if (host->ops->busy_complete)
 			writel(status & ~host->variant->busy_detect_mask,
 			       host->base + MMCICLEAR);
 		else
@@ -1971,7 +1979,7 @@ static int mmci_probe(struct amba_device *dev,
 	/*
 	 * Enable busy detection.
 	 */
-	if (variant->busy_detect) {
+	if (host->ops->busy_complete) {
 		u32 max_busy_timeout = 0;
 
 		mmci_ops.card_busy = mmci_card_busy;
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 8f86130af566..03eb21f1c258 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -289,7 +289,6 @@ struct mmci_host;
  * @f_max: maximum clk frequency supported by the controller.
  * @signal_direction: input/out direction of bus signals can be indicated
  * @pwrreg_clkgate: MMCIPOWER register must be used to gate the clock
- * @busy_detect: true if the variant supports busy detection on DAT0.
  * @busy_timeout: true if the variant starts data timer when the DPSM
  *		  enter in Wait_R or Busy state.
  * @busy_dpsm_flag: bitmask enabling busy detection in the DPSM
@@ -337,7 +336,6 @@ struct variant_data {
 	u32			f_max;
 	u8			signal_direction:1;
 	u8			pwrreg_clkgate:1;
-	u8			busy_detect:1;
 	u8			busy_timeout:1;
 	u32			busy_dpsm_flag;
 	u32			busy_detect_flag;
@@ -372,6 +370,7 @@ struct mmci_host_ops {
 	void (*dma_error)(struct mmci_host *host);
 	void (*set_clkreg)(struct mmci_host *host, unsigned int desired);
 	void (*set_pwrreg)(struct mmci_host *host, unsigned int pwr);
+	bool (*busy_complete)(struct mmci_host *host, u32 status, u32 err_msk);
 };
 
 struct mmci_host {
-- 
2.17.1

^ permalink raw reply related

* [PATCH V4 1/3] mmc: mmci: add hardware busy timeout feature
From: Ludovic Barre @ 2019-08-05  9:56 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring
  Cc: srinivas.kandagatla, Maxime Coquelin, Alexandre Torgue,
	linux-arm-kernel, linux-kernel, devicetree, linux-mmc,
	linux-stm32, Ludovic Barre
In-Reply-To: <20190805095626.25998-1-ludovic.Barre@st.com>

From: Ludovic Barre <ludovic.barre@st.com>

In some variants, the data timer starts and decrements
when the DPSM enters in Wait_R or Busy state
(while data transfer or MMC_RSP_BUSY), and generates a
data timeout error if the counter reach 0.

-Define max_busy_timeout (in ms) according to clock.
-Set data timer register if the command has rsp_busy flag.
 If busy_timeout is not defined by framework, the busy
 length after Data Burst is defined as 1 second
 (refer: 4.6.2.2 Write of sd specification part1 v6-0).
-Add MCI_DATATIMEOUT error management in mmci_cmd_irq.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 drivers/mmc/host/mmci.c | 37 ++++++++++++++++++++++++++++++++-----
 drivers/mmc/host/mmci.h |  3 +++
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 4c35e7609c89..e79c9148af84 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1078,6 +1078,7 @@ static void
 mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c)
 {
 	void __iomem *base = host->base;
+	unsigned long long clks = 0;
 
 	dev_dbg(mmc_dev(host->mmc), "op %02x arg %08x flags %08x\n",
 	    cmd->opcode, cmd->arg, cmd->flags);
@@ -1100,6 +1101,19 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c)
 		else
 			c |= host->variant->cmdreg_srsp;
 	}
+
+	if (host->variant->busy_timeout && !cmd->data) {
+		if (cmd->flags & MMC_RSP_BUSY) {
+			if (!cmd->busy_timeout)
+				cmd->busy_timeout = 1000;
+
+			clks = (unsigned long long)cmd->busy_timeout;
+			clks *=	host->cclk;
+			do_div(clks, MSEC_PER_SEC);
+		}
+		writel_relaxed(clks, host->base + MMCIDATATIMER);
+	}
+
 	if (/*interrupt*/0)
 		c |= MCI_CPSM_INTERRUPT;
 
@@ -1206,6 +1220,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 {
 	void __iomem *base = host->base;
 	bool sbc, busy_resp;
+	u32 err_msk;
 
 	if (!cmd)
 		return;
@@ -1218,8 +1233,12 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 	 * handling. Note that we tag on any latent IRQs postponed
 	 * due to waiting for busy status.
 	 */
-	if (!((status|host->busy_status) &
-	      (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT|MCI_CMDSENT|MCI_CMDRESPEND)))
+	err_msk = MCI_CMDCRCFAIL | MCI_CMDTIMEOUT;
+	if (host->variant->busy_timeout && busy_resp)
+		err_msk |= MCI_DATATIMEOUT;
+
+	if (!((status | host->busy_status) &
+	      (err_msk | MCI_CMDSENT | MCI_CMDRESPEND)))
 		return;
 
 	/* Handle busy detection on DAT0 if the variant supports it. */
@@ -1238,8 +1257,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 		 * while, to allow it to be set, but tests indicates that it
 		 * isn't needed.
 		 */
-		if (!host->busy_status &&
-		    !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) &&
+		if (!host->busy_status && !(status & err_msk) &&
 		    (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) {
 
 			writel(readl(base + MMCIMASK0) |
@@ -1293,6 +1311,9 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 		cmd->error = -ETIMEDOUT;
 	} else if (status & MCI_CMDCRCFAIL && cmd->flags & MMC_RSP_CRC) {
 		cmd->error = -EILSEQ;
+	} else if (host->variant->busy_timeout && busy_resp &&
+		   status & MCI_DATATIMEOUT) {
+		cmd->error = -ETIMEDOUT;
 	} else {
 		cmd->resp[0] = readl(base + MMCIRESPONSE0);
 		cmd->resp[1] = readl(base + MMCIRESPONSE1);
@@ -1951,6 +1972,8 @@ static int mmci_probe(struct amba_device *dev,
 	 * Enable busy detection.
 	 */
 	if (variant->busy_detect) {
+		u32 max_busy_timeout = 0;
+
 		mmci_ops.card_busy = mmci_card_busy;
 		/*
 		 * Not all variants have a flag to enable busy detection
@@ -1960,7 +1983,11 @@ static int mmci_probe(struct amba_device *dev,
 			mmci_write_datactrlreg(host,
 					       host->variant->busy_dpsm_flag);
 		mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY;
-		mmc->max_busy_timeout = 0;
+
+		if (variant->busy_timeout)
+			max_busy_timeout = ~0UL / (mmc->f_max / MSEC_PER_SEC);
+
+		mmc->max_busy_timeout = max_busy_timeout;
 	}
 
 	/* Prepare a CMD12 - needed to clear the DPSM on some variants. */
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 4f071bd34e59..8f86130af566 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -290,6 +290,8 @@ struct mmci_host;
  * @signal_direction: input/out direction of bus signals can be indicated
  * @pwrreg_clkgate: MMCIPOWER register must be used to gate the clock
  * @busy_detect: true if the variant supports busy detection on DAT0.
+ * @busy_timeout: true if the variant starts data timer when the DPSM
+ *		  enter in Wait_R or Busy state.
  * @busy_dpsm_flag: bitmask enabling busy detection in the DPSM
  * @busy_detect_flag: bitmask identifying the bit in the MMCISTATUS register
  *		      indicating that the card is busy
@@ -336,6 +338,7 @@ struct variant_data {
 	u8			signal_direction:1;
 	u8			pwrreg_clkgate:1;
 	u8			busy_detect:1;
+	u8			busy_timeout:1;
 	u32			busy_dpsm_flag;
 	u32			busy_detect_flag;
 	u32			busy_detect_mask;
-- 
2.17.1

^ permalink raw reply related

* [PATCH V4 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant
From: Ludovic Barre @ 2019-08-05  9:56 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring
  Cc: devicetree, Alexandre Torgue, linux-mmc, linux-kernel,
	srinivas.kandagatla, Ludovic Barre, Maxime Coquelin, linux-stm32,
	linux-arm-kernel

From: Ludovic Barre <ludovic.barre@st.com>

This patch series adds busy detect for stm32 sdmmc variant.
Some adaptations are required:
-On sdmmc the data timer is started on data transfert
and busy state, so we must add hardware busy timeout support.
-Add busy_complete callback at mmci_host_ops to allow to define
a specific busy completion by variant.
-Add sdmmc busy_complete calback.

V4:
-Re-work with busy_complete callback
-In series, move "mmc: mmci: add hardware busy timeout feature" in
first to simplify busy_complete prototype with err_msk parameter.

V3:
-rebase on latest mmc next
-replace re-read by status parameter. 

V2:
-mmci_cmd_irq cleanup in separate patch.
-simplify the busy_detect_flag exclude
-replace sdmmc specific comment in
"mmc: mmci: avoid fake busy polling in mmci_irq"
to focus on common behavior

Ludovic Barre (3):
  mmc: mmci: add hardware busy timeout feature
  mmc: mmci: add busy_complete callback
  mmc: mmci: sdmmc: add busy_complete callback

 drivers/mmc/host/mmci.c             | 178 +++++++++++++++++-----------
 drivers/mmc/host/mmci.h             |   7 +-
 drivers/mmc/host/mmci_stm32_sdmmc.c |  38 ++++++
 3 files changed, 151 insertions(+), 72 deletions(-)

-- 
2.17.1

^ permalink raw reply

* Re: [RFC PATCH 01/11] devfreq: exynos-bus: Extract exynos_bus_profile_init()
From: Krzysztof Kozlowski @ 2019-08-05  9:56 UTC (permalink / raw)
  To: Artur Świgoń
  Cc: devicetree, linux-arm-kernel, linux-samsung-soc@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-pm, dri-devel, Chanwoo Choi,
	myungjoo.ham, Inki Dae, Seung Woo Kim, georgi.djakov,
	Marek Szyprowski, Bartłomiej Żołnierkiewicz
In-Reply-To: <bda10bcc66aae96355e74c4739229d72bcc95b0d.camel@partner.samsung.com>

On Wed, 31 Jul 2019 at 15:00, Artur Świgoń <a.swigon@partner.samsung.com> wrote:
>
> Hi,
>
> On Wed, 2019-07-24 at 21:07 +0200, Krzysztof Kozlowski wrote:
> > On Tue, Jul 23, 2019 at 02:20:06PM +0200, Artur Świgoń wrote:
> > > This patch adds a new static function, exynos_bus_profile_init(), extracted
> > > from exynos_bus_probe().
> > >
> > > Signed-off-by: Artur Świgoń <a.swigon@partner.samsung.com>
> > > ---
> > >  drivers/devfreq/exynos-bus.c | 106 ++++++++++++++++++++---------------
> > >  1 file changed, 60 insertions(+), 46 deletions(-)
> > >
> > > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> > > index d9f377912c10..d8f1efaf2d49 100644
> > > --- a/drivers/devfreq/exynos-bus.c
> > > +++ b/drivers/devfreq/exynos-bus.c
> > > @@ -372,12 +372,69 @@ static int exynos_bus_parse_of(struct device_node *np,
> > >     return ret;
> > >  }
> > >
> > > +static int exynos_bus_profile_init(struct exynos_bus *bus,
> > > +                              struct devfreq_dev_profile *profile)
> > > +{
> > > +   struct device *dev = bus->dev;
> > > +   struct devfreq_simple_ondemand_data *ondemand_data;
> > > +   int ret;
> > > +
> > > +   /* Initialize the struct profile and governor data for parent device */
> > > +   profile->polling_ms = 50;
> > > +   profile->target = exynos_bus_target;
> > > +   profile->get_dev_status = exynos_bus_get_dev_status;
> > > +   profile->exit = exynos_bus_exit;
> > > +
> > > +   ondemand_data = devm_kzalloc(dev, sizeof(*ondemand_data), GFP_KERNEL);
> > > +   if (!ondemand_data) {
> > > +           ret = -ENOMEM;
> > > +           goto err;
> >
> > Just return proper error code. Less lines, obvious code since you do not
> > have any cleanup in error path.
>
> I was advised to avoid modifying code being moved (in one patch). I do make
> changes in these places in patch 04/11, i.e. change the original label 'err' to
> 'out'. What's your opinion on making the proposed changes to patches 01 and 02
> (s/goto err/return ret/) in patch 04 instead?

Yes, you're right. I also prefer not to touch moved code.

Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH v2 6/6] ARM: dts: mmp2: add OLPC XO 1.75 machine
From: Lubomir Rintel @ 2019-08-05  9:56 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Olof Johansson, Rob Herring, Mark Rutland, linux-arm-kernel,
	devicetree, linux-kernel
In-Reply-To: <20190803085824.GB8224@amd>

----- On Aug 3, 2019, at 10:58 AM, Pavel Machek pavel@ucw.cz wrote:

> On Fri 2019-08-02 12:33:26, Lubomir Rintel wrote:
>> This is a fairly complete description of an OLPC XO 1.75 laptop.
>> What's missing for now is the GPU, LCD controller, DCON, the panel and
>> audio.
> 
> Ok, but I need GPU/LCD/panel... that's my only output. Is video
> expected to work in 5.2? Does the firmware pass right device tree,
> including the GPU/LCD/DCON?

The firmware (and the dts) uses a simple-framebuffer. You won't get
any nifty features, but you'll get a framebuffer.

Hopefully we'll get a proper DRM video soon. The status is roughly
as follows:

LCDC: potentially supported by the Armada DRM driver. Patches sent to
Russell King some months ago, not there yet. I'd prefer not to nag him.

DCON: Russell dislikes the idea of a DRM bridge, DRM maintainers prefer
if the driver was not a DRM encoder driver. This seems to require
quite some work to fix [1].

[1] https://www.spinics.net/lists/dri-devel/msg201927.html

GPU: supported by the etnaviv driver, good enough for 2D acceleration
to work with xorg-x11-video-armada driver. 3D (weston and mutter alike)
is broken. To add this to the device tree, the clock and power needs
to be figured out.

> Is there config somewhere I could use?
> 
> Thanks a lot,
>								Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures)
> http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* Re: [PATCH v1] gpio: mpc8xxx: Add new platforms GPIO DT node description
From: Alexander Stein @ 2019-08-05  9:47 UTC (permalink / raw)
  To: Hui Song
  Cc: linux-devel, linux-gpio, linux-kernel, linux-arm-kernel,
	devicetree
In-Reply-To: <20190805091432.9656-2-hui.song_1@nxp.com>

On Monday, August 5, 2019, 11:14:32 AM CEST Hui Song wrote:
> From: Song Hui <hui.song_1@nxp.com>
> 
> Update the NXP GPIO node dt-binding file for QorIQ and
> Layerscape platforms, and add one more example with
> ls1028a GPIO node.
> 
> Signed-off-by: Song Hui <hui.song_1@nxp.com>
> ---
>  Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt b/Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt
> index 69d4616..fbe6d75 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt
> @@ -28,7 +28,7 @@ gpio0: gpio@1100 {
>  Example of gpio-controller node for a ls2080a SoC:

   ^^^^^^^                               ^^^^^^^
This is an example for ls2080a...

>  gpio0: gpio@2300000 {
> -	compatible = "fsl,ls2080a-gpio", "fsl,qoriq-gpio";
> +	compatible = "fsl,ls1028a-gpio","fsl,ls2080a-gpio", "fsl,qoriq-gpio";

so I doubt there should be a ls1028a compatible here though.

>  	reg = <0x0 0x2300000 0x0 0x10000>;
>  	interrupts = <0 36 0x4>; /* Level high type */
>  	gpio-controller;

Best regards,
Alexander

^ permalink raw reply

* RE: [PATCH/RFC 12/12] arm64: dts: renesas: Add EK874 board with idk-2121wr display support
From: Fabrizio Castro @ 2019-08-05  9:44 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Laurent Pinchart, Kieran Bingham, Jacopo Mondi, Rob Herring,
	Mark Rutland, Simon Horman, Magnus Damm, Linux-Renesas,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Geert Uytterhoeven, Chris Paterson, Biju Das,
	ebiharaml@si-linux.co.jp
In-Reply-To: <CAMuHMdVRg_N3S1OPVyy_Wj13XUHnTdWPY3iaOTuogah5oZCUWw@mail.gmail.com>

Hi Geert,

Thank you for your feedback!

> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 02 August 2019 10:11
> Subject: Re: [PATCH/RFC 12/12] arm64: dts: renesas: Add EK874 board with idk-2121wr display support
> 
> Hi Fabrizio,
> 
> On Fri, Aug 2, 2019 at 9:35 AM Fabrizio Castro
> <fabrizio.castro@bp.renesas.com> wrote:
> > The EK874 is advertised as compatible with panel IDK-2121WR from
> > Advantech, however the panel isn't sold alongside the board.
> > A new dts, adding everything that's required to get the panel to
> > to work with the EK874, is the most convenient way to support the
> > EK874 when it's connected to the IDK-2121WR.
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts
> > @@ -0,0 +1,112 @@
> 
> [...]
> 
> > +       panel-lvds {
> > +               compatible = "advantech,idk-2121wr", "panel-lvds";
> > +
> > +               width-mm = <476>;
> > +               height-mm = <268>;
> > +
> > +               data-mapping = "vesa-24";
> > +
> > +               panel-timing {
> > +                       clock-frequency = <148500000>;
> > +                       hactive = <1920>;
> > +                       vactive = <1080>;
> > +                       hsync-len = <44>;
> > +                       hfront-porch = <88>;
> > +                       hback-porch = <148>;
> > +                       vfront-porch = <4>;
> > +                       vback-porch = <36>;
> > +                       vsync-len = <5>;
> > +               };
> > +
> > +               ports {
> > +                       #address-cells = <1>;
> > +                       #size-cells = <0>;
> > +
> > +                       port@0 {
> > +                               reg = <0>;
> > +                               lvds0_panel_in: endpoint {
> > +                                       remote-endpoint = <&lvds0_out>;
> > +                               };
> > +                       };
> > +
> > +                       port@1 {
> > +                               reg = <1>;
> > +                               lvds1_panel_in: endpoint {
> > +                                       remote-endpoint = <&lvds1_out>;
> > +                               };
> > +                       };
> > +               };
> > +       };
> > +};
> 
> [...]
> 
> > +&lvds0 {
> > +       renesas,swap-data;
> > +
> > +       ports {
> > +               port@1 {
> > +                       lvds0_out: endpoint {
> > +                               remote-endpoint = <&lvds0_panel_in>;
> > +                       };
> > +               };
> > +       };
> > +};
> > +
> > +&lvds1 {
> > +       status = "okay";
> > +
> > +       clocks = <&cpg CPG_MOD 727>, <&x13_clk>, <&extal_clk>;
> > +       clock-names = "fck", "dclkin.0", "extal";
> > +
> > +       ports {
> > +               port@1 {
> > +                       lvds1_out: endpoint {
> > +                               remote-endpoint = <&lvds1_panel_in>;
> > +                       };
> > +               };
> > +       };
> > +};
> 
> Shouldn't the actual panel definition, and the lvds remote-endpoint setup,
> be extracted into a separate .dtsi, to be included here?
> 
> Cfr. arch/arm/boot/dts/r8a77xx-aa104xd12-panel.dtsi and
> arch/arm/boot/dts/r8a77xx-aa121td01-panel.dtsi.

It looks like those displays are commonly used with Marzen, Lager, and Koelsch
boards, I am not aware of any plans for reusing this particular panel.
Perhaps we should still make this more generic and create a .dtsi?

Thanks,
Fab

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

^ permalink raw reply

* Re: [PATCH v2 3/6] arm64: dts: amlogic: g12: add temperature sensor
From: guillaume La Roque @ 2019-08-05  9:41 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: daniel.lezcano, khilman, devicetree, linux-amlogic, linux-kernel,
	linux-arm-kernel, linux-pm
In-Reply-To: <CAFBinCBYPiLgmTNk+7Db3EPSPePwbnAshCbomYPXWdse8i0oJw@mail.gmail.com>

hi Martin,


thanks for comments i will fix in v3.


guillaume

On 8/3/19 7:52 PM, Martin Blumenstingl wrote:
> Hi Guillaume,
>
> On Wed, Jul 31, 2019 at 5:36 PM Guillaume La Roque
> <glaroque@baylibre.com> wrote:
>> Add cpu and ddr temperature sensors for G12 Socs
>>
>> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
> with the nit-pick below addressed:
> Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>
>> ---
>>  .../boot/dts/amlogic/meson-g12-common.dtsi    | 22 +++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
>> index 06e186ca41e3..7f862a3490fb 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
>> @@ -1353,6 +1353,28 @@
>>                                 };
>>                         };
>>
>> +                       cpu_temp: temperature-sensor@34800 {
>> +                               compatible = "amlogic,g12-cpu-thermal",
>> +                                            "amlogic,g12-thermal";
>> +                               reg = <0x0 0x34800 0x0 0x50>;
>> +                               interrupts = <GIC_SPI 35 IRQ_TYPE_EDGE_RISING>;
>> +                               clocks = <&clkc CLKID_TS>;
>> +                               status = "okay";
> I believe nodes are enabled automatically if they don't have a status property
>
>> +                               #thermal-sensor-cells = <0>;
>> +                               amlogic,ao-secure = <&sec_AO>;
>> +                       };
>> +
>> +                       ddr_temp: temperature-sensor@34c00 {
>> +                               compatible = "amlogic,g12-ddr-thermal",
>> +                                            "amlogic,g12-thermal";
>> +                               reg = <0x0 0x34c00 0x0 0x50>;
>> +                               interrupts = <GIC_SPI 36 IRQ_TYPE_EDGE_RISING>;
>> +                               clocks = <&clkc CLKID_TS>;
>> +                               status = "okay";
> same here
>
>
> Martin

^ permalink raw reply

* RE: [PATCH/RFC 12/12] arm64: dts: renesas: Add EK874 board with idk-2121wr display support
From: Fabrizio Castro @ 2019-08-05  9:37 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Kieran Bingham, Jacopo Mondi, Rob Herring, Mark Rutland,
	Simon Horman, Magnus Damm, linux-renesas-soc@vger.kernel.org,
	devicetree@vger.kernel.org, Geert Uytterhoeven, Chris Paterson,
	Biju Das, ebiharaml@si-linux.co.jp
In-Reply-To: <20190802083424.GM5008@pendragon.ideasonboard.com>

Hi Laurent,

> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: 02 August 2019 09:34
> Subject: Re: [PATCH/RFC 12/12] arm64: dts: renesas: Add EK874 board with idk-2121wr display support
> 
> Hi Fabrizio,
> 
> Thank you for the patch.
> 
> On Fri, Aug 02, 2019 at 08:34:09AM +0100, Fabrizio Castro wrote:
> > The EK874 is advertised as compatible with panel IDK-2121WR from
> > Advantech, however the panel isn't sold alongside the board.
> > A new dts, adding everything that's required to get the panel to
> > to work with the EK874, is the most convenient way to support the
> > EK874 when it's connected to the IDK-2121WR.
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > ---
> >  arch/arm64/boot/dts/renesas/Makefile               |   3 +-
> >  .../boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts | 112 +++++++++++++++++++++
> >  2 files changed, 114 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts
> >
> > diff --git a/arch/arm64/boot/dts/renesas/Makefile b/arch/arm64/boot/dts/renesas/Makefile
> > index 42b74c2..ce48478 100644
> > --- a/arch/arm64/boot/dts/renesas/Makefile
> > +++ b/arch/arm64/boot/dts/renesas/Makefile
> > @@ -1,7 +1,8 @@
> >  # SPDX-License-Identifier: GPL-2.0
> >  dtb-$(CONFIG_ARCH_R8A774A1) += r8a774a1-hihope-rzg2m.dtb
> >  dtb-$(CONFIG_ARCH_R8A774A1) += r8a774a1-hihope-rzg2m-ex.dtb
> > -dtb-$(CONFIG_ARCH_R8A774C0) += r8a774c0-cat874.dtb r8a774c0-ek874.dtb
> > +dtb-$(CONFIG_ARCH_R8A774C0) += r8a774c0-cat874.dtb r8a774c0-ek874.dtb \
> > +			       r8a774c0-ek874-idk-2121wr.dtb
> >  dtb-$(CONFIG_ARCH_R8A7795) += r8a7795-salvator-x.dtb r8a7795-h3ulcb.dtb
> >  dtb-$(CONFIG_ARCH_R8A7795) += r8a7795-h3ulcb-kf.dtb
> >  dtb-$(CONFIG_ARCH_R8A7795) += r8a7795-salvator-xs.dtb
> > diff --git a/arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts b/arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-
> 2121wr.dts
> > new file mode 100644
> > index 0000000..d989998
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts
> > @@ -0,0 +1,112 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Device Tree Source for the Silicon Linux RZ/G2E evaluation kit (EK874),
> > + * connected to an Advantech IDK-2121WR 21.5" LVDS panel
> > + *
> > + * Copyright (C) 2019 Renesas Electronics Corp.
> > + */
> > +
> > +#include "r8a774c0-ek874.dts"
> > +
> > +/ {
> > +	backlight: backlight {
> > +		compatible = "pwm-backlight";
> > +		pwms = <&pwm5 0 50000>;
> > +
> > +		brightness-levels = <0 4 8 16 32 64 128 255>;
> > +		default-brightness-level = <6>;
> > +
> > +		power-supply = <&reg_12p0v>;
> > +		enable-gpios = <&gpio6 12 GPIO_ACTIVE_HIGH>;
> > +	};
> > +
> > +	panel-lvds {
> > +		compatible = "advantech,idk-2121wr", "panel-lvds";
> > +
> > +		width-mm = <476>;
> > +		height-mm = <268>;
> > +
> > +		data-mapping = "vesa-24";
> > +
> > +		panel-timing {
> > +			clock-frequency = <148500000>;
> > +			hactive = <1920>;
> > +			vactive = <1080>;
> > +			hsync-len = <44>;
> > +			hfront-porch = <88>;
> > +			hback-porch = <148>;
> > +			vfront-porch = <4>;
> > +			vback-porch = <36>;
> > +			vsync-len = <5>;
> > +		};
> > +
> > +		ports {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +
> > +			port@0 {
> > +				reg = <0>;
> > +				lvds0_panel_in: endpoint {
> > +					remote-endpoint = <&lvds0_out>;
> > +				};
> > +			};
> > +
> > +			port@1 {
> > +				reg = <1>;
> > +				lvds1_panel_in: endpoint {
> > +					remote-endpoint = <&lvds1_out>;
> > +				};
> > +			};
> > +		};
> > +	};
> > +};
> > +
> > +&gpio0 {
> > +	lvds-connector-en-gpio{
> > +		gpio-hog;
> > +		gpios = <17 GPIO_ACTIVE_HIGH>;
> > +		output-low;
> > +		line-name = "lvds-connector-en-gpio";
> > +	};
> 
> Any chance to specify this as the panel's enable signal in the panel DT
> node ?

I am not too sure, as this is not exactly an enable signal. When GP0_17 is low
then LVDS[01] are connected to the LVDS connector, when GP0_17 is high
then LVDS[01] are connected to the LT8918L.
Perhaps we should leave this fixed to low to avoid confusion?

Thanks,
Fab

> 
> > +};
> > +
> > +&lvds0 {
> > +	renesas,swap-data;
> 
> Let's discuss this property in reply to the DT bindings patch.
> 
> > +
> > +	ports {
> > +		port@1 {
> > +			lvds0_out: endpoint {
> > +				remote-endpoint = <&lvds0_panel_in>;
> > +			};
> > +		};
> > +	};
> > +};
> > +
> > +&lvds1 {
> > +	status = "okay";
> > +
> > +	clocks = <&cpg CPG_MOD 727>, <&x13_clk>, <&extal_clk>;
> > +	clock-names = "fck", "dclkin.0", "extal";
> > +
> > +	ports {
> > +		port@1 {
> > +			lvds1_out: endpoint {
> > +				remote-endpoint = <&lvds1_panel_in>;
> > +			};
> > +		};
> > +	};
> > +};
> > +
> > +&pfc {
> > +	pwm5_pins: pwm5 {
> > +		groups = "pwm5_a";
> > +		function = "pwm5";
> > +	};
> > +};
> > +
> > +&pwm5 {
> > +	pinctrl-0 = <&pwm5_pins>;
> > +	pinctrl-names = "default";
> > +
> > +	status = "okay";
> > +};
> 
> I haven't reviewed pinouts in detail, but the patch otherwise looks sane
> to me. Another candidate for DT overlays though ;-)
> 
> --
> Regards,
> 
> Laurent Pinchart

^ permalink raw reply

* Re: [PATCH/RFC 02/12] dt-bindings: display: renesas: lvds: Document renesas,swap-data
From: Laurent Pinchart @ 2019-08-05  9:35 UTC (permalink / raw)
  To: Fabrizio Castro
  Cc: Mark Rutland, devicetree@vger.kernel.org, Chris Paterson,
	Geert Uytterhoeven, Simon Horman, David Airlie, Kieran Bingham,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	Biju Das, linux-renesas-soc@vger.kernel.org, Rob Herring,
	Jacopo Mondi
In-Reply-To: <TY1PR01MB17706A4FF4C26CD4BDA1A5DAC0DA0@TY1PR01MB1770.jpnprd01.prod.outlook.com>

Hi Fabrizio,

On Mon, Aug 05, 2019 at 08:59:51AM +0000, Fabrizio Castro wrote:
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Sent: 02 August 2019 08:44
> > Subject: Re: [PATCH/RFC 02/12] dt-bindings: display: renesas: lvds: Document renesas,swap-data
> > 
> > Hi Fabrizio,
> > 
> > Thank you for the patch.
> > 
> > On Fri, Aug 02, 2019 at 08:33:59AM +0100, Fabrizio Castro wrote:
> > > R-Car D3, R-Car E3, and RZ/G2E support dual-link mode.
> > > In such a mode, the first LVDS encoder emits even data, and the
> > > second LVDS encoder emits odd data. This patch documents property
> > > renesas,swap-data, used to swap even and odd data around.
> > >
> > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > ---
> > >  Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > > index dece79e..8980179 100644
> > > --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > > @@ -52,6 +52,11 @@ Optional properties:
> > >    mandatory for the first LVDS encoder on R-Car D3, R-Car E3, and RZ/G2E SoCs,
> > >    and shall point to the second encoder to be used as a companion in dual-link
> > >    mode. It shall not be set for any other LVDS encoder.
> > > +- renesas,swap-data : when in dual-link mode, the first LVDS encoder normally
> > > +  emits even data, and the second LVDS encoder emits odd data. When property
> > > +  renesas,swap-data is specified, the data emitted by the two encoders will be
> > > +  swapped around. This property can only be used in conjunction with property
> > > +  renesas,companion.
> > 
> > From an LVDS encoder point of view this is more a configuration option
> > than a description of the hardware. Wouldn't it be better for the LVDS
> > sink to report which of the odd or even pixels it expects on each of its
> > endpoints ?
> 
> Yes, that would be my preference too, and it would be better, I am just not entirely
> what's the best place for this information though
> 
> > The LVDS encoder driver could then query that at runtime and
> > configure itself accordingly. Ideally this should be queried through the
> > drm_bridge_timings structure (or through a similar mean), not through
> > DT. An LVDS sink that has a fixed mapping of odd/even pixels to
> > endpoints wouldn't need the information to be specified in DT at all.
> 
> Isn't drm_bridge_timings specific for bridges?

Its name makes it specific to bridges, but the information it contains
could equally apply to panels. I would thus use it for both, possibly
after renaming it.

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ 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