Devicetree
 help / color / mirror / Atom feed
* [PATCH 3/3] dt-bindings: i2c: pxa: Update the documentation for the Armada 3700
From: Romain Perier @ 2016-11-08 15:15 UTC (permalink / raw)
  To: Wolfram Sang, linux-i2c
  Cc: devicetree, Rob Herring, Ian Campbell, Pawel Moll, Mark Rutland,
	Kumar Gala, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
	Gregory Clement, Thomas Petazzoni, Nadav Haklai, Omri Itach,
	Shadi Ammouri, Yahuda Yitschak, Hanna Hawa, Neta Zur Hershkovits,
	Igal Liberman, Marcin Wojtas
In-Reply-To: <20161108151506.1131-1-romain.perier@free-electrons.com>

This commit documents the compatible string to have the compatibility for
the I2C unit found in the Armada 3700.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 Documentation/devicetree/bindings/i2c/i2c-pxa.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-pxa.txt b/Documentation/devicetree/bindings/i2c/i2c-pxa.txt
index 12b78ac..b1b995c 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-pxa.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-pxa.txt
@@ -7,6 +7,7 @@ Required properties :
    compatible processor, e.g. pxa168, pxa910, mmp2, mmp3.
    For the pxa2xx/pxa3xx, an additional node "mrvl,pxa-i2c" is required
    as shown in the example below.
+   For the Armada 3700, the compatible should be "marvell,armada-3700".
 
 Recommended properties :
 
-- 
2.9.3

^ permalink raw reply related

* [PATCH 2/3] arm64: dts: marvell: Add I2C definitions for the Armada 3700
From: Romain Perier @ 2016-11-08 15:15 UTC (permalink / raw)
  To: Wolfram Sang, linux-i2c
  Cc: devicetree, Rob Herring, Ian Campbell, Pawel Moll, Mark Rutland,
	Kumar Gala, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
	Gregory Clement, Thomas Petazzoni, Nadav Haklai, Omri Itach,
	Shadi Ammouri, Yahuda Yitschak, Hanna Hawa, Neta Zur Hershkovits,
	Igal Liberman, Marcin Wojtas
In-Reply-To: <20161108151506.1131-1-romain.perier@free-electrons.com>

The Armada 3700 has two i2c bus interface units, this commit adds the
definitions of the corresponding device nodes. It also enables the node
on the development board for this SoC.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 arch/arm64/boot/dts/marvell/armada-3720-db.dts |  4 ++++
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi   | 18 ++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-3720-db.dts b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
index 1372e9a6..16d84af 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-db.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
@@ -62,6 +62,10 @@
 	};
 };
 
+&i2c0 {
+	status = "okay";
+};
+
 /* CON3 */
 &sata {
 	status = "okay";
diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index c476253..bf2d73d 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -98,6 +98,24 @@
 			/* 32M internal register @ 0xd000_0000 */
 			ranges = <0x0 0x0 0xd0000000 0x2000000>;
 
+			i2c0: i2c@11000 {
+				compatible = "marvell,armada-3700-i2c";
+				reg = <0x11000 0x24>;
+				clocks = <&nb_perih_clk 10>;
+				interrupts = <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
+				mrvl,i2c-fast-mode;
+				status = "disabled";
+			};
+
+			i2c1: i2c@11080 {
+				compatible = "marvell,armada-3700-i2c";
+				reg = <0x11080 0x24>;
+				clocks = <&nb_perih_clk 9>;
+				interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
+				mrvl,i2c-fast-mode;
+				status = "disabled";
+			};
+
 			uart0: serial@12000 {
 				compatible = "marvell,armada-3700-uart";
 				reg = <0x12000 0x400>;
-- 
2.9.3

^ permalink raw reply related

* [PATCH 1/3] i2c: pxa: Add support for the I2C units found in Armada 3700
From: Romain Perier @ 2016-11-08 15:15 UTC (permalink / raw)
  To: Wolfram Sang, linux-i2c-u79uwXL29TY76Z2rM5mHXA
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
	Pawel Moll, Mark Rutland, Kumar Gala, Jason Cooper, Andrew Lunn,
	Sebastian Hesselbarth, Gregory Clement, Thomas Petazzoni,
	Nadav Haklai, Omri Itach, Shadi Ammouri, Yahuda Yitschak,
	Hanna Hawa, Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas
In-Reply-To: <20161108151506.1131-1-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

The Armada 3700 has two I2C controllers that is compliant with the I2C
Bus Specificiation 2.1, supports multi-master and different bus speed:
Standard mode (up to 100 KHz), Fast mode (up to 400 KHz),
High speed mode (up to 3.4 Mhz).

This IP block has a lot of similarity with the PXA, except some register
offsets and bitfield. This commits adds a basic support for this I2C
unit.

Signed-off-by: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/i2c/busses/Kconfig   |  2 +-
 drivers/i2c/busses/i2c-pxa.c | 25 +++++++++++++++++++++++--
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index d252276..2f56a26 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -763,7 +763,7 @@ config I2C_PUV3
 
 config I2C_PXA
 	tristate "Intel PXA2XX I2C adapter"
-	depends on ARCH_PXA || ARCH_MMP || (X86_32 && PCI && OF)
+	depends on ARCH_PXA || ARCH_MMP || ARCH_MVEBU || (X86_32 && PCI && OF)
 	help
 	  If you have devices in the PXA I2C bus, say yes to this option.
 	  This driver can also be built as a module.  If so, the module
diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index e28b825..34ea830 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -55,6 +55,7 @@ enum pxa_i2c_types {
 	REGS_PXA3XX,
 	REGS_CE4100,
 	REGS_PXA910,
+	REGS_A3700,
 };
 
 /*
@@ -91,6 +92,13 @@ static struct pxa_reg_layout pxa_reg_layout[] = {
 		.ilcr = 0x28,
 		.iwcr = 0x30,
 	},
+	[REGS_A3700] = {
+		.ibmr = 0x00,
+		.idbr = 0x04,
+		.icr =	0x08,
+		.isr =	0x0c,
+		.isar = 0x10,
+	},
 };
 
 static const struct platform_device_id i2c_pxa_id_table[] = {
@@ -98,6 +106,7 @@ static const struct platform_device_id i2c_pxa_id_table[] = {
 	{ "pxa3xx-pwri2c",	REGS_PXA3XX },
 	{ "ce4100-i2c",		REGS_CE4100 },
 	{ "pxa910-i2c",		REGS_PXA910 },
+	{ "armada-3700-i2c",	REGS_A3700  },
 	{ },
 };
 MODULE_DEVICE_TABLE(platform, i2c_pxa_id_table);
@@ -122,7 +131,9 @@ MODULE_DEVICE_TABLE(platform, i2c_pxa_id_table);
 #define ICR_SADIE	(1 << 13)	   /* slave address detected int enable */
 #define ICR_UR		(1 << 14)	   /* unit reset */
 #define ICR_FM		(1 << 15)	   /* fast mode */
+#define ICR_BUSMODE_FM	(1 << 16)	   /* shifted fast mode for armada-3700 */
 #define ICR_HS		(1 << 16)	   /* High Speed mode */
+#define ICR_BUSMODE_HS	(1 << 17)	   /* shifted high speed mode for armada-3700 */
 #define ICR_GPIOEN	(1 << 19)	   /* enable GPIO mode for SCL in HS */
 
 #define ISR_RWM		(1 << 0)	   /* read/write mode */
@@ -193,6 +204,8 @@ struct pxa_i2c {
 	unsigned char		master_code;
 	unsigned long		rate;
 	bool			highmode_enter;
+	unsigned long		fm_mask;
+	unsigned long		hs_mask;
 };
 
 #define _IBMR(i2c)	((i2c)->reg_ibmr)
@@ -503,8 +516,8 @@ static void i2c_pxa_reset(struct pxa_i2c *i2c)
 		writel(i2c->slave_addr, _ISAR(i2c));
 
 	/* set control register values */
-	writel(I2C_ICR_INIT | (i2c->fast_mode ? ICR_FM : 0), _ICR(i2c));
-	writel(readl(_ICR(i2c)) | (i2c->high_mode ? ICR_HS : 0), _ICR(i2c));
+	writel(I2C_ICR_INIT | (i2c->fast_mode ? i2c->fm_mask : 0), _ICR(i2c));
+	writel(readl(_ICR(i2c)) | (i2c->high_mode ? i2c->hs_mask : 0), _ICR(i2c));
 
 #ifdef CONFIG_I2C_PXA_SLAVE
 	dev_info(&i2c->adap.dev, "Enabling slave mode\n");
@@ -1137,6 +1150,7 @@ static const struct of_device_id i2c_pxa_dt_ids[] = {
 	{ .compatible = "mrvl,pxa-i2c", .data = (void *)REGS_PXA2XX },
 	{ .compatible = "mrvl,pwri2c", .data = (void *)REGS_PXA3XX },
 	{ .compatible = "mrvl,mmp-twsi", .data = (void *)REGS_PXA910 },
+	{ .compatible = "marvell,armada-3700-i2c", .data = (void *)REGS_A3700 },
 	{}
 };
 MODULE_DEVICE_TABLE(of, i2c_pxa_dt_ids);
@@ -1158,6 +1172,13 @@ static int i2c_pxa_probe_dt(struct platform_device *pdev, struct pxa_i2c *i2c,
 		i2c->use_pio = 1;
 	if (of_get_property(np, "mrvl,i2c-fast-mode", NULL))
 		i2c->fast_mode = 1;
+	if (of_device_is_compatible(np, "marvell,armada-3700-i2c")) {
+		i2c->fm_mask = ICR_BUSMODE_FM;
+		i2c->hs_mask = ICR_BUSMODE_HS;
+	} else {
+		i2c->fm_mask = ICR_FM;
+		i2c->hs_mask = ICR_HS;
+	}
 
 	*i2c_types = (enum pxa_i2c_types)(of_id->data);
 
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 0/3] Add basic support for the I2C units of the Armada 3700
From: Romain Perier @ 2016-11-08 15:15 UTC (permalink / raw)
  To: Wolfram Sang, linux-i2c-u79uwXL29TY76Z2rM5mHXA
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
	Pawel Moll, Mark Rutland, Kumar Gala, Jason Cooper, Andrew Lunn,
	Sebastian Hesselbarth, Gregory Clement, Thomas Petazzoni,
	Nadav Haklai, Omri Itach, Shadi Ammouri, Yahuda Yitschak,
	Hanna Hawa, Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas

This series add basic support for the I2C bus interface units present
in the Armada 3700 to the pxa-i2c driver. It also add the definitions of
the device nodes to the devicetree at the SoC level and for its official
development board: the Armada 3720 DB.

Romain Perier (3):
  i2c: pxa: Add support for the I2C units found in Armada 3700
  arm64: dts: marvell: Add I2C definitions for the Armada 3700
  dt-bindings: i2c: pxa: Update the documentation for the Armada 3700

 Documentation/devicetree/bindings/i2c/i2c-pxa.txt |  1 +
 arch/arm64/boot/dts/marvell/armada-3720-db.dts    |  4 ++++
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi      | 18 ++++++++++++++++
 drivers/i2c/busses/Kconfig                        |  2 +-
 drivers/i2c/busses/i2c-pxa.c                      | 25 +++++++++++++++++++++--
 5 files changed, 47 insertions(+), 3 deletions(-)

-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 0/8] firmware: arm_scpi: add support for legacy SCPI protocol
From: Sudeep Holla @ 2016-11-08 15:11 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Sudeep Holla, Neil Armstrong, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Olof Johansson,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161108145118.GR1041-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org>



On 08/11/16 14:51, Russell King - ARM Linux wrote:
> On Wed, Nov 02, 2016 at 10:52:03PM -0600, Sudeep Holla wrote:
>> This is minor rework of the series[1] from Neil Armstrong's to support
>> legacy SCPI protocol to make DT bindings more generic and move out all
>> the platform specific bindings out of the generic binding document.
>
> Is this what would be in my HBI0282B Juno?
>

No, it's one on the AmLogic Meson GXBB platform. Juno never supported
that except that old firmware use it internally. By that I mean some
version of trusted firmware used legacy SCPI but they are generally
bundled together in fip, so you should not see any issue with upgrade.

> (Note: I only have the original firmware on the board, as the Linaro
> firmware drops are completely broken for it.)
>

I am currently trying to run Linaro 16.10 release, I don't see any issue
except network boot from UEFI which is known and reported.

I will go through your logs in detail and try to replicate your issue.
I assume you have tried replacing the entry contents of the uSD with the
release. I will start with that now.

-- 
Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/2] gpio: xilinx: Add clock support
From: Sören Brinkmann @ 2016-11-08 14:52 UTC (permalink / raw)
  To: Shubhrajyoti Datta; +Cc: linux-gpio, devicetree, michal.simek
In-Reply-To: <1478581990-28276-2-git-send-email-shubhrajyoti.datta@xilinx.com>

On Tue, 2016-11-08 at 10:43:10 +0530, Shubhrajyoti Datta wrote:
> Add basic clock support for xilinx gpio.
> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
>  drivers/gpio/gpio-xilinx.c |   22 ++++++++++++++++++++++
>  1 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c
> index 14b2a62..923cab8 100644
> --- a/drivers/gpio/gpio-xilinx.c
> +++ b/drivers/gpio/gpio-xilinx.c
> @@ -13,6 +13,7 @@
>   */
>  
>  #include <linux/bitops.h>
> +#include <linux/clk.h>
>  #include <linux/init.h>
>  #include <linux/errno.h>
>  #include <linux/module.h>
> @@ -45,6 +46,7 @@
>   * @gpio_state: GPIO state shadow register
>   * @gpio_dir: GPIO direction shadow register
>   * @gpio_lock: Lock used for synchronization
> + * @clk: Clock resource for this controller
>   */
>  struct xgpio_instance {
>  	struct of_mm_gpio_chip mmchip;
> @@ -52,6 +54,7 @@ struct xgpio_instance {
>  	u32 gpio_state[2];
>  	u32 gpio_dir[2];
>  	spinlock_t gpio_lock[2];
> +	struct clk *clk;
>  };
>  
>  static inline int xgpio_index(struct xgpio_instance *chip, int gpio)
> @@ -282,6 +285,7 @@ static int xgpio_remove(struct platform_device *pdev)
>  	struct xgpio_instance *chip = platform_get_drvdata(pdev);
>  
>  	of_mm_gpiochip_remove(&chip->mmchip);
> +	clk_disable_unprepare(chip->clk);
>  
>  	return 0;
>  }
> @@ -307,6 +311,23 @@ static int xgpio_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, chip);
>  
> +	/* Retrieve GPIO clock */
> +	chip->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(chip->clk)) {
> +		if (PTR_ERR(chip->clk) == -ENOENT) {
> +			dev_info(&pdev->dev, "No clocks found for clk\n");
> +			chip->clk = NULL;
> +		} else {
> +			dev_err(&pdev->dev, "axi clock error\n");
> +			return PTR_ERR(chip->clk);
> +		}
> +	}

In the bindings you document 'clocks' as required, but it's handled as
optional here. Things should be consistent and given that the existing
binding doesn't require clocks, we're probably stuck with having the
clock optional.

	Sören

^ permalink raw reply

* Re: [PATCH 0/8] firmware: arm_scpi: add support for legacy SCPI protocol
From: Russell King - ARM Linux @ 2016-11-08 14:51 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: Neil Armstrong, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Olof Johansson,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1478148731-11712-1-git-send-email-sudeep.holla-5wv7dgnIgG8@public.gmane.org>

On Wed, Nov 02, 2016 at 10:52:03PM -0600, Sudeep Holla wrote:
> This is minor rework of the series[1] from Neil Armstrong's to support
> legacy SCPI protocol to make DT bindings more generic and move out all
> the platform specific bindings out of the generic binding document.

Is this what would be in my HBI0282B Juno?

(Note: I only have the original firmware on the board, as the Linaro
firmware drops are completely broken for it.)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 0/5] ARM: da850: new drivers for better LCDC support
From: Sekhar Nori @ 2016-11-08 14:50 UTC (permalink / raw)
  To: Bartosz Golaszewski, Kevin Hilman, Michael Turquette, Rob Herring,
	Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King
  Cc: LKML, arm-soc, linux-drm, linux-devicetree, Jyri Sarha,
	Tomi Valkeinen, David Airlie, Laurent Pinchart, Arnd Bergmann,
	Olof Johansson
In-Reply-To: <1477925138-23457-1-git-send-email-bgolaszewski@baylibre.com>

+ Arnd, Olof

On Monday 31 October 2016 08:15 PM, Bartosz Golaszewski wrote:
> This series adds two new drivers in order to better support the LCDC
> rev1 present on the da850 boards.
> 
> The first patch adds a new memory driver which allows to write to the
> DDR2/mDDR memory controller present on the da8xx SoCs.
> 
> The second patch adds a new bus driver which allows to interact with
> the MSTPRI registers of the SYSCFG0 module

I think patches 1/5 and 2/5 are ready to be merged. If there are no
objections I would like to send a pull request for them to be merged
through ARM-SoC tree for v4.10 kernel.

Thanks,
Sekhar

> 
> As is mentioned in the comments: we don't want to commit to supporting
> stable interfaces (DT bindings or sysfs attributes) so we hardcode the
> settings required by some boards (for now only da850-lcdk) with the
> hope that linux gets an appropriate framework for performance knobs
> in the future.
> 
> Potential extensions of these drivers should be straightforward in the
> future.
> 
> Subsequent patches add DT nodes for the new drivers: disabled nodes
> in da850.dtsi and enabled in da850-lcdk.dts.
> 
> The last patch adds a workaround for current lack of support for drm
> bridges in tilcdc.
> 
> Tested on a da850-lcdk with a display connected over VGA and two
> additional patches for tilcdc (sent to linux-drm): ran simple modetest
> for supported resolutions, used X.org and fluxbox as graphical
> environment, played video with mplayer.
> 
> v1 -> v2:
> - used regular readl()/writel() instead of __raw_** versions
> - used resource_size() instead of calculating the size by hand
> - used ioremap instead of syscon in patch [2/5]
> - added the DT nodes in patches [3/5]-[5/5]
> 
> Bartosz Golaszewski (5):
>   ARM: memory: da8xx-ddrctl: new driver
>   ARM: bus: da8xx-mstpri: new driver
>   ARM: dts: da850: add the mstpri and ddrctl nodes
>   ARM: dts: da850-lcdk: enable mstpri and ddrctl nodes
>   ARM: dts: da850-lcdk: add tilcdc panel node
> 
>  .../devicetree/bindings/bus/ti,da850-mstpri.txt    |  20 ++
>  .../memory-controllers/ti-da8xx-ddrctl.txt         |  20 ++
>  arch/arm/boot/dts/da850-lcdk.dts                   |  71 ++++++
>  arch/arm/boot/dts/da850.dtsi                       |  11 +
>  drivers/bus/Kconfig                                |   9 +
>  drivers/bus/Makefile                               |   2 +
>  drivers/bus/da8xx-mstpri.c                         | 269 +++++++++++++++++++++
>  drivers/memory/Kconfig                             |   8 +
>  drivers/memory/Makefile                            |   1 +
>  drivers/memory/da8xx-ddrctl.c                      | 175 ++++++++++++++
>  10 files changed, 586 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
>  create mode 100644 Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
>  create mode 100644 drivers/bus/da8xx-mstpri.c
>  create mode 100644 drivers/memory/da8xx-ddrctl.c
> 

^ permalink raw reply

* Re: [PATCH 1/2] gpio: xilinx: dt-binding: Add clock node
From: Sören Brinkmann @ 2016-11-08 14:48 UTC (permalink / raw)
  To: Shubhrajyoti Datta
  Cc: linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	michal.simek-gjFFaj9aHVfQT0dZR+AlfA
In-Reply-To: <1478581990-28276-1-git-send-email-shubhrajyoti.datta-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>

On Tue, 2016-11-08 at 10:43:09 +0530, Shubhrajyoti Datta wrote:
> Add the clock node to the dt binding.
> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> ---
>  .../devicetree/bindings/gpio/gpio-xilinx.txt       |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-xilinx.txt b/Documentation/devicetree/bindings/gpio/gpio-xilinx.txt
> index 63bf4be..0821b9d 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio-xilinx.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio-xilinx.txt
> @@ -11,6 +11,7 @@ Required properties:
>  - #gpio-cells : Should be two. The first cell is the pin number and the
>    second cell is used to specify optional parameters (currently unused).
>  - gpio-controller : Marks the device node as a GPIO controller.
> +- clocks: Input clock specifier. Refer to common clock bindings.
>  
>  Optional properties:
>  - interrupts : Interrupt mapping for GPIO IRQ.
> @@ -25,12 +26,14 @@ Optional properties:
>  - xlnx,dout-default-2 : as above but the second channel
>  - xlnx,gpio2-width : as above but for the second channel
>  - xlnx,tri-default-2 : as above but for the second channel
> +- clock-names: Input clock name

Clock names are driver specific and must be documented here.

>  
>  
>  Example:
>  gpio: gpio@40000000 {
>  	#gpio-cells = <2>;
>  	compatible = "xlnx,xps-gpio-1.00.a";
> +	clocks = <&clkc 15>;

Where are the clock-names?

	Sören
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v2] iio: document bindings for mounting matrices
From: Linus Walleij @ 2016-11-08 14:45 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio-u79uwXL29TY76Z2rM5mHXA
  Cc: Linus Walleij, Gregor Boirie, Sebastian Reichel, Samu Onkalo,
	devicetree-u79uwXL29TY76Z2rM5mHXA

The mounting matrix for sensors was introduced in
commit dfc57732ad38 ("iio:core: mounting matrix support")

However the device tree bindings are very terse and since this is
a widely applicable property, we need a proper binding for it
that the other bindings can reference. This will also be useful
for other operating systems and sensor engineering at large.

I think all 3D sensors should support it, the current situation
is probably that the mounting information is confined in magic
userspace components rather than using the mounting matrix, which
is not good for portability and reuse.

Cc: Gregor Boirie <gregor.boirie-ITF29qwbsa/QT0dZR+AlfA@public.gmane.org>
Cc: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Samu Onkalo <samu.onkalo-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
ChangeLog v1->v2:
- Repeat linear algebra and actually get the rotation matrix right.
- Reference the Wikipedia article on the subject.
- Fix up text.
---
 .../devicetree/bindings/iio/mount-matrix.txt       | 108 +++++++++++++++++++++
 1 file changed, 108 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/mount-matrix.txt

diff --git a/Documentation/devicetree/bindings/iio/mount-matrix.txt b/Documentation/devicetree/bindings/iio/mount-matrix.txt
new file mode 100644
index 000000000000..a3714727f739
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/mount-matrix.txt
@@ -0,0 +1,108 @@
+Mounting matrix
+
+The mounting matrix is a device tree property used to orient any IIO device
+that produce three-dimensional data in relation to the world where it is
+deployed.
+
+The purpose of the mounting matrix is to translate the sensor frame of
+reference into the device frame of reference using a translation matrix as
+defined in linear algebra.
+
+The typical usecase is that where a component has an internal representation
+of the (x,y,z) triplets, such as different registers to read these coordinates,
+and thus implying that the component should be mounted in a certain orientation
+relative to some specific device frame of reference.
+
+For example a device with some kind of screen, where the user is supposed to
+interact with the environment using an accelerometer, gyroscope or magnetometer
+mounted on the same chassis as this screen, will likely take the screen as
+reference to (x,y,z) orientation, with (x,y) corresponding to these axes on the
+screen and (z) being depth, the axis perpendicular to the screen.
+
+For a screen you probably want (x) coordinates to go from negative on the left
+to positive on the right and (z) depth to be negative under the screen and
+positive in front of it, toward the face of the user.
+
+A sensor can be mounted in any angle along the axes relative to the frame of
+reference. This means that the sensor may be flipped upside-down, left-right,
+or tilted at any angle relative to the frame of reference.
+
+Another frame of reference is how the device with its sensor relates to the
+external world, the environment where the device is deployed. Usually the data
+from the sensor is used to figure out how the device is oriented with respect
+to this world. When using the mounting matrix, the sensor and device orientation
+becomes identical and we can focus on the data as it relates to the surrounding
+world.
+
+Device-to-world examples for some three-dimensional sensor types:
+
+- Accelerometers have their world frame of reference toward the center of
+  gravity, usually to the core of the planet. A reading of the (x,y,z) values
+  from the sensor will give a projection of the gravity vector through the
+  device relative to the center of the planet, i.e. relative to its surface at
+  this point. Up and down in the world relative to the device frame of
+  reference can thus be determined. and users would likely expect a value of
+  9.81 m/s^2 upwards along the (z) axis, i.e. out of the screen when the device
+  is held with its screen flat on the planets surface and 0 on the other axes,
+  as the gravity vector is projected 1:1 onto the sensors (z)-axis.
+
+- Magnetometers (compasses) have their world frame of reference relative to the
+  geomagnetic field. The system orientation vis-a-vis the world is defined with
+  respect to the local earth geomagnetic reference frame where (y) is in the
+  ground plane and positive towards magnetic North, (x) is in the ground plane,
+  perpendicular to the North axis and positive towards the East and (z) is
+  perpendicular to the ground plane and positive upwards.
+
+- Gyroscopes detects the movement relative the device itself. The angular
+  velocity is defined as orthogonal to the plane of rotation, so if you put the
+  device on a flat surface and spin it around the z axis (such as rotating a
+  device with a screen lying flat on a table), you should get a negative value
+  along the (z) axis if rotated clockwise, and a positive value if rotated
+  counter-clockwise according to the right-hand rule.
+
+So unless the sensor is ideally mounted, we need a means to indicate the
+relative orientation of any given sensor of this type with respect to the
+frame of reference.
+
+To achieve this, use the device tree property "mount-matrix" for the sensor.
+This supplies a 3x3 rotation matrix in the strict linear algebraic sense,
+to orient the senor axes relative to a desired point of reference. This means
+the resulting values from the sensor, after scaling to proper units, should be
+multiplied by this matrix to give the proper vectors values in three-dimensional
+space, relative to the device or world point of reference.
+
+For more information, consult:
+https://en.wikipedia.org/wiki/Rotation_matrix
+
+The mounting matrix has the layout:
+
+ (x0, y0, z0)
+ (x1, y1, z1)
+ (x2, y2, z3)
+
+And it is represented as an array of strings containing the real values for
+producing the transformation matrix. The real values use a decimal point and
+a minus (-) to indicate a negative value.
+
+Examples:
+
+Identity matrix (nothing happens to the coordinates, which means the device was
+mechanically mounted in an ideal way and we need no transformation):
+
+mount-matrix = "1", "0", "0",
+               "0", "1", "0",
+               "0", "0", "1";
+
+The sensor is mounted 30 degrees (Pi/6 radians) tilted along the X axis, so we
+compensate by performing a -30 degrees rotation around the X axis:
+
+mount-matrix = "1", "0", "0",
+               "0", "0.866", "0.5",
+               "0", "-0.5", "0.866";
+
+The sensor is flipped 180 degrees (Pi radians) around the Z axis, i.e. mounted
+upside-down:
+
+mount-matrix = "0.998", "0.054", "0",
+               "-0.054", "0.998", "0",
+               "0", "0", "1";
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells and pinctrl_parse_index_with_args
From: Tony Lindgren @ 2016-11-08 14:44 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Jon Hunter, Mark Rutland, Rob Herring, Grygorii Strashko,
	Nishanth Menon,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux-OMAP
In-Reply-To: <CACRpkdbdfM8qtMSE1O-VEsnOHGbVuMDhOFtELvqoYuE1JJ=GRg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

* Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> [161108 03:32]:
> On Mon, Nov 7, 2016 at 4:26 PM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
> > * Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> [161104 14:50]:
> 
> >> +struct of_phandle_args;
> >> +
> >>  #ifdef CONFIG_OF
> >>
> >> Let's see if it works!
> >
> > OK so do we know now? It seems there was one more email
> > about it but it may have been without it.
> 
> I haven't seen anything, I think it JustWorks(TM).

OK good to to hear :)

Thanks,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads
From: Thierry Reding @ 2016-11-08 14:42 UTC (permalink / raw)
  To: Laxman Dewangan
  Cc: Linus Walleij, Stephen Warren, Rob Herring, Mark Rutland,
	Jon Hunter, Masahiro Yamada, linux-gpio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-tegra@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <5821D49E.2070308@nvidia.com>

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

On Tue, Nov 08, 2016 at 07:05:26PM +0530, Laxman Dewangan wrote:
> 
> On Tuesday 08 November 2016 06:59 PM, Linus Walleij wrote:
> > On Tue, Nov 8, 2016 at 11:20 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
> > > On Tuesday 08 November 2016 03:45 PM, Linus Walleij wrote:
> > > > If you can *actually* change the volatage, it needs to be modeled
> > > > as a (fixed voltage?) regulator, not as a custom property for the pin
> > > > control attributes. I guess you definiately need the regulator framework
> > > > to accumulate and infer the different consumer requirements anyway
> > > > in that case.
> > > The PMIC voltage output is changed via regulator calls.
> > > Here, we need to have two configruations for given voltage level of
> > > interface:
> > > * One at IO voltage from PMIC via regulator call to change votlage of IO
> > > rail.
> > > * Second, configure the IO pad register to tell the IO voltage level so that
> > > it can configured internally for that level.
> > I understand! (I think.)
> 
> Thanks,
> 
> > 
> > But then the two things (A) changing the regulator voltage and (B) changing
> > the pin setting need to happen at the same time do they
> > not?
> > 
> > Now you're just hardcoding something into these device tree properties
> > and hoping that the regulators will somehow be set up in accordance to
> > what you set up for the pads in the device tree, correct?
> 
> There is two types of configuration in given platform, the IO voltage does
> not get change (fixed in given platform) and in some of cases, get change
> dynamically like SDIO3.0 where the voltage switches to 3.3V and 1.8V.
> 
> Yes, it can be integrated with the regulator handle and then it can call the
> required configurations through notifier and regulator_get_voltage().
> But I think it is too much complex for the static configurations. This
> mandate also to populate the regulator handle and all power tree.

It looks as if regulator notifiers should be able to support whatever
use-cases we may have (I suspect that we really only need pre- and post-
voltage-change notifications.

> The simple way for static configuration (case where voltage does not get
> change), just take the power tree IO voltage from DT and configure the IO
> pad control register.

For static configurations where we have a regulator along with a pinmux
setting for the I/O voltage we could potentially run into issues where
both settings don't match. How would we handle that?

> For dynamic case, there is some sequence need to be followed based on
> voltage direction change (towards lower or towards higher) for the voltage
> change and the IO pad voltage configuration and it is simple to do it from
> client driver.

Are patches available for this? It'd be useful to know how this will end
up being used in order to come up with the best solution.

In general I think it's good to send a complete series of patches, even
if it's long and spans multiple subsystems. As it is it's difficult for
anyone else (myself included) to understand where this is headed, which
makes it more complicated than necessary to get at the final solution.

> > To me it seems like the pins/pads should all have an <&phandle> to
> > the regulator controlling its voltage output, in the device tree.
> > 
> > In the Linux kernel, the driver has to regulator_[bulk_]get() this for
> > each pin, check the voltage with regulator_get_voltage() and set up
> > this according to the supplied voltage.
> > 
> > The driver then ideally should subscribe to regulator voltage notifier
> > events to change the setting if the voltage changes. I guess. But
> > atleast the first step seems inevitable: get the voltage from a regulator.
> > 
> > Else there is no dependency between the regulator and its consumer.
> > 
> > So what your pins need is a regulator phandle, not a magic value to
> > be poked into a register, hoping things will match up.
> > 
> > I understand that this is a simple quick-and-dirty solution but it is
> > not the right solution.
> 
> 
> Yaah, the static power tree configuration is much simple in this approach
> without having regulator drivers and support.
> 
> Integrating with regulator driver can be done here also.
> 
> I like to have both approach, through pinmux DT and also from regulator. So
> based on the platform, if regulator supported then populate required
> properties in DT for regulator else go on standard pinmux DT way (for
> non-regulator cases).

Again, it would be best to see this in actual use. Right now it's not
clear that we'll even have both cases. Implementing both approaches will
mean potentially unused code.

On another note: in my experience you seldom need both cases, since the
dynamic configuration is the hard one, and the static configuration will
usually be a special case of the dynamic configuration.

Thierry

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

^ permalink raw reply

* Re: [PATCH v5 1/7] drm: sunxi: Add a basic DRM driver for Allwinner DE2
From: Jean-Francois Moine @ 2016-11-08 14:37 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Dave Airlie, Liam Girdwood, Mark Brown, Rob Herring,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20161107223741.gjxj4tqwuxud2iqc@lukather>

On Mon, 7 Nov 2016 23:37:41 +0100
Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:

> Hi,
> 
> On Fri, Oct 28, 2016 at 07:34:20PM +0200, Jean-Francois Moine wrote:
> > On Fri, 28 Oct 2016 00:03:16 +0200
> > Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
	[snip]
> > > > > We've been calling them bus and mod.
> > > > 
> > > > I can understand "bus" (which is better than "apb"), but why "mod"?
> > > 
> > > Allwinner has been calling the clocks that are supposed to generate
> > > the external signals (depending on where you were looking) module or
> > > mod clocks (which is also why we have mod in the clock
> > > compatibles). The module 1 clocks being used for the audio and the
> > > module 0 for the rest (SPI, MMC, NAND, display, etc.)
> > 
> > I did not find any 'module' in the H3 documentation.
> > So, is it really a good name?
> 
> It's true that they use it less nowadays, but they still do,
> ie. https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/drivers/clk/sunxi/clk-sun8iw7.c#L513

There is a 'mod' suffix, but it is used for the bus gates only, not for
the main clocks.

> And we have to remain consistent anyway.

I don't see any consistency in the H3 DT:
- the bus gates are named "ahb" and apb"
- the (main) clocks are named "mmc", "usb0_phy" and "ir"
There is no "bus" nor "mod".

> > > > > > +
> > > > > > +- resets: phandle to the reset of the device
> > > > > > +
> > > > > > +- ports: phandle's to the LCD ports
> > > > > 
> > > > > Please use the OF graph.
> > > > 
> > > > These ports are references to the graph of nodes. See
> > > > 	http://www.kernelhub.org/?msg=911825&p=2
> > > 
> > > In an OF-graph, your phandle to the LCD controller would be replaced
> > > by an output endpoint.
> > 
> > This is the DE controller. There is no endpoint link at this level.
> 
> The display engine definitely has an endpoint: the TCON.

Not at all. The video chain is simply
	CRTC (TCON) -> connector (HDMI/LCD/DAC/..)
The DE is an ancillary device which handles the planes.

> > The Device Engine just handles the planes of the LCDs, but, indeed,
> > the LCDs must know about the DE and the DE must know about the LCDs.
> > There are 2 ways to realize this knowledge in the DT:
> > 1) either the DE has one or two phandle's to the LCDs,
> > 2) or the LCDs have a phandle to the DE.
> > 
> > I chose the 1st way, the DE ports pointing to the endpoint of the LCDs
> > which is part of the video link (OF-graph LCD <-> connector).
> > It would be possible to have phandles to the LCDs themselves, but this
> > asks for more code.
> > 
> > The second way is also possible, but it also complexifies a bit the
> > exchanges DE <-> LCD.
> 
> I'm still not sure how it would complexify anything, and why you can't
> use the display graph to model the relation between the display engine
> and the TCON (and why you want to use a generic property that refers
> to the of-graph while it really isn't).

Complexification:
1- my solution:
  At startup time, the DE device is the DRM device. It has to know the
  devices entering in the video chains.
  The CRTCs (LCD/TCON) are found by
	ports[i] -> parent
  The connectors are found by
	ports[i] -> endpoint -> remote_endpoint -> parent
2- with ports pointing to the LCDs:
  The CRTCs (LCD/TCON) are simply
	ports[i]
  The connectors are found by
	loop on all ports of ports[i]
		ports[i][j] -> endpoint -> remote_endpoint -> parent
3- with a phandle to the DE in the LCDs:
  The DE cannot be the DRM device because there is no information about
  the video devices in the DT. Then, the DRM devices are the LCDs.
  These LCDs must give their indices to the DE. So, the DE must implement
  some callback function to accept a LCD definition, and there must be
  a list of DEs in the driver to make the association DE <-> LCD[i]
  Some more problem may be raised if a user wants to have the same frame
  buffer on the 2 LCDs of a DE.

Anyway, my solution is already used in the IMX Soc.
See 'display-subsystem' in arch/arm/boot/dts/imx6q.dtsi for an example.

> > > > > > +void de2_disable_vblank(struct drm_device *drm, unsigned crtc)
> > > > > > +{
> > > > > > +	struct priv *priv = drm->dev_private;
> > > > > > +	struct lcd *lcd = priv->lcds[crtc];
> > > > > > +
> > > > > > +	tcon_write(lcd->mmio, gint0,
> > > > > > +			 tcon_read(lcd->mmio, gint0) &
> > > > > > +					~TCON_GINT0_TCON1_Vb_Int_En);
> > > > > > +}
> > > > > > +
> > > > > > +/* panel functions */
> > > > > 
> > > > > Panel functions? In the CRTC driver?
> > > > 
> > > > Yes, dumb panel.
> > > 
> > > What do you mean by that? Using a Parallel/RGB interface?
> > 
> > Sorry, I though this was a well-known name. The 'dump panel' was used
> > in the documentation of my previous ARM machine as the video frame sent
> > to the HDMI controller. 'video_frame' is OK for you?
> 
> If it's the frame sent to the encoder, then it would be the CRTC by
> DRM's nomenclature.

The CRTC is a software entity. The frame buffer is a hardware entity.

> > > > > > +static const struct {
> > > > > > +	char chan;
> > > > > > +	char layer;
> > > > > > +	char pipe;
> > > > > > +} plane2layer[DE2_N_PLANES] = {
> > > > > > +	[DE2_PRIMARY_PLANE] =	{0, 0, 0},
> > > > > > +	[DE2_CURSOR_PLANE] =	{1, 0, 1},
> > > > > > +	[DE2_VI_PLANE] =	{0, 1, 0},
> > > > > > +};
	[snip]
> > > > > 
> > > > > Comments?
> > > > 
> > > > This
> > > > 	primary plane is channel 0 (VI), layer 0, pipe 0
> > > > 	cursor plane is channel 1 (UI), layer 0, pipe 1
> > > > 	overlay plane is channel 0 (VI), layer 1, pipe 0
> > > > or the full explanation:
> > > >     Constraints:
> > > > 	The VI channels can do RGB or YUV, while UI channels can do RGB
> > > > 	only.
> > > > 	The LCD 0 has 1 VI channel and 4 UI channels, while
> > > > 	LCD 1 has only 1 VI channel and 1 UI channel.
> > > > 	The cursor must go to a channel bigger than the primary channel,
> > > > 	otherwise it is not transparent.
> > > >     First try:
> > > > 	Letting the primary plane (usually RGB) in the 2nd channel (UI),
> > > > 	as this is done in the legacy driver, asks for the cursor to go
> > > > 	to the next channel (UI), but this one does not exist in LCD1.
> > > >     Retained layout:
> > > > 	So, we must use only 2 channels for the same behaviour on LCD0
> > > > 	(H3) and LCD1 (A83T)
> > > > 	The retained combination is:
> > > > 		- primary plane in the first channel (VI),
> > > > 		- cursor plane inthe 2nd channel (UI), and
> > > > 		- overlay plane in the 1st channel (VI).
> > > > 
> > > > 	Note that there could be 3 overlay planes (a channel has 4
> > > > 	layers), but I am not sure that the A83T or the H3 could
> > > > 	support 3 simultaneous video streams...
> > > 
> > > Do you know if the pipe works in the old display engine?
> > > 
> > > Especially about the two-steps composition that wouldn't allow you to
> > > have alpha on all the planes?
> > > 
> > > If it is similar, I think hardcoding the pipe number is pretty bad,
> > > because that would restrict the combination of planes and formats,
> > > while some other might have worked.
> > 
> > From what I understood about the DE2, the pipes just define the priority
> > of the overlay channels (one pipe for one channel).
> > With the cursor constraint, there must be at least 2 channels in
> > order (primary, cursor). Then, with these 2 channels/pipes, there can be
> > 6 so-called overlay planes (3 RGB/YUV and 3 RGB only).
> > Enabling the pipes 2 and 3 (LCD 0 only) would offer 8 more planes, but
> > RGB only. Then, it might be useful to have dynamic pipes.
> 
> That's very valuable (and definitely should go into a comment),
> thanks!
> 
> I still believe that's it should be into a (simple at first)
> atomic_check. That would be easier to extend and quite easy to
> document and get simply by looking at the code.

Sorry for I don't understand what you mean.

> > > > > > +static int __init de2_drm_init(void)
> > > > > > +{
> > > > > > +	int ret;
> > > > > > +
> > > > > > +/* uncomment to activate the drm traces at startup time */
> > > > > > +/*	drm_debug = DRM_UT_CORE | DRM_UT_DRIVER | DRM_UT_KMS |
> > > > > > +			DRM_UT_PRIME | DRM_UT_ATOMIC; */
> > > > > 
> > > > > That's useless.
> > > > 
> > > > Right, but it seems that some people don't know how to debug a DRM
> > > > driver. This is only a reminder.
> > > > 
> > > > > > +	DRM_DEBUG_DRIVER("\n");
> > > > > > +
> > > > > > +	ret = platform_driver_register(&de2_lcd_platform_driver);
> > > > > > +	if (ret < 0)
> > > > > > +		return ret;
> > > > > > +
> > > > > > +	ret = platform_driver_register(&de2_drm_platform_driver);
> > > > > > +	if (ret < 0)
> > > > > > +		platform_driver_unregister(&de2_lcd_platform_driver);
> > > > > > +
> > > > > > +	return ret;
> > > > > > +}
> > > > > 
> > > > > And that really shouldn't be done that way.
> > > > 
> > > > May you explain?
> > > 
> > > This goes against the whole idea of the device and driver
> > > model. Drivers should only register themselves, device should be
> > > created by buses (or by using some external components if the bus
> > > can't: DT, ACPI, etc.). If there's a match, you get probed.
> > > 
> > > A driver that creates its own device just to probe itself violates
> > > that.
> > 
> > In this function (module init), there is no driver yet.
> > The module contains 2 drivers: the DE (planes) and the LCD (CRTC),
> > and there is no macro to handle such modules.
> 
> Ah, yes, my bad. I thought you were registering a device and a
> driver. Still this is a very unusual pattern. Why do you need to split
> the two? Can't you just merge them?

The DE and the LCDs are different devices on different drivers.
A DE must be only one device because it has to handle concurent
accesses from its 2 LCDs. Then 2 drivers.

But only one module. Why? Because there cannot be double direction
calls from one module to an other one, and, in our case, for example,
- the DRM (DE) device must call vblank functions which are handled in
  the CRTC (TCON) device, and
- the CRTC device must call DE initialization functions at startup time.

> > > > > > +int de2_plane_init(struct drm_device *drm, struct lcd *lcd)
> > > > > > +{
> > > > > > +	int ret, possible_crtcs = 1 << lcd->crtc_idx;
> > > > > > +
> > > > > > +	ret = de2_one_plane_init(drm, &lcd->planes[DE2_PRIMARY_PLANE],
> > > > > > +				DRM_PLANE_TYPE_PRIMARY, possible_crtcs,
> > > > > > +				ui_formats, ARRAY_SIZE(ui_formats));
> > > > > > +	if (ret >= 0)
> > > > > > +		ret = de2_one_plane_init(drm, &lcd->planes[DE2_CURSOR_PLANE],
> > > > > > +				DRM_PLANE_TYPE_CURSOR, possible_crtcs,
> > > > > > +				ui_formats, ARRAY_SIZE(ui_formats));
> > > > > 
> > > > > Nothing looks really special about that cursor plane. Any reasion not
> > > > > to make it an overlay?
> > > > 
> > > > As explained above (channel/layer/pipe plane definitions), the cursor
> > > > cannot go in a channel lower or equal to the one of the primary plane.
> > > > Then, it must be known and, so, have an explicit plane.
> > > 
> > > If you were to make it a plane, you could use atomic_check to check
> > > this and make sure this doesn't happen. And you would gain a generic
> > > plane that can be used for other purposes if needed.
> > 
> > The function drm_crtc_init_with_planes() offers a cursor plane for free.
> > On the other side, having 6 overlay planes is more than the SoCs can
> > support.
> 
> It's not really for free, it costs you a generic plane that could
> definitely be used for something else and cannot anymore because
> they've been hardcoded to a cursor.
> 
> And having a camera, the VPU or even an application directly output
> directly into one of these planes seems a much better use of a generic
> plane than a cursor.

Looking at the harder case (A83T), there may be 8 planes on 2 channels.
Using a primary plane and the cursor,
	8 planes - primary plane - cursor plane = 6 planes
6 planes are available.
If I count correctly, in your example:
	one camera + one VPU + one application = 3 planes
3 planes are used.
So, 3 planes are still available.

On the other side, removing the cursor would just let one more plane.
Do we really need this one? In other words, I'd be pleased to know how
you run 7 applications doing video overlay.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH v5 6/8] Documentation: bindings: add compatible specific to legacy SCPI protocol
From: Sudeep Holla @ 2016-11-08 14:32 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, Neil Armstrong, linux-kernel, Sudeep Holla,
	Olof Johansson, linux-amlogic, linux-arm-kernel
In-Reply-To: <1478148731-11712-7-git-send-email-sudeep.holla@arm.com>

Hi Rob,

On 03/11/16 04:52, Sudeep Holla wrote:
> This patch adds specific compatible to support legacy SCPI protocol.
>

Sorry for messing it up before, I think this version is much better.

Only this patch introduces new compatible, while 5,7,8/8 are just
reorganization to move the platform specific stuff out of the generic
SCPI bindings. It would be good if you can have a quick look and
provide ack if you are happy with these patches.

-- 
Regards,
Sudeep

> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  Documentation/devicetree/bindings/arm/arm,scpi.txt | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt b/Documentation/devicetree/bindings/arm/arm,scpi.txt
> index d1882c4540d0..ebd03fc93135 100644
> --- a/Documentation/devicetree/bindings/arm/arm,scpi.txt
> +++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
> @@ -7,7 +7,9 @@ by Linux to initiate various system control and power operations.
>
>  Required properties:
>
> -- compatible : should be "arm,scpi"
> +- compatible : should be
> +	* "arm,scpi" : For implementations complying to SCPI v1.0 or above
> +	* "arm,legacy-scpi" : For implementations complying pre SCPI v1.0
>  - mboxes: List of phandle and mailbox channel specifiers
>  	  All the channels reserved by remote SCP firmware for use by
>  	  SCPI message protocol should be specified in any order
>

^ permalink raw reply

* Re: [PATCH v2 3/4] Device bindings documentation updated ACPI-enabled platforms not currently supported
From: Luis Oliveira @ 2016-11-08 14:18 UTC (permalink / raw)
  To: Andy Shevchenko, Luis Oliveira, Wolfram Sang, Ramiro Oliveira
  Cc: Mark Rutland, jarkko.nikula, mika.westerberg, linux-i2c,
	linux-kernel, robh+dt, devicetree, CARLOS.PALMINHA
In-Reply-To: <1477047272.6423.6.camel@linux.intel.com>

Hi,


As you suggested I will split the drivers. I am thinking of doing 5 patches:

- factor out master() parts

- separate Master part to i2c-designware-master.c (changes in i2c-designware-core.c)

- enable Slave part to i2c-designware-slave (changes in i2c-designware-core.c)

- glue drivers and device bindings

- cleaning


Regards,

Luis

On 21-Oct-16 11:54, Andy Shevchenko wrote:
> On Fri, 2016-10-21 at 10:56 +0100, Luis Oliveira wrote:
>> Since practically 90% of the code is shared between master and slave,
>> I was
>> thinking if it will be acceptable to use the same driver for both but
>> differentiate the master/slave mode by the compatible strings.
> It might be possible to split like other drivers do:
>
> 1. Core part (i2c-designware-core.c)
> 2. Master part (i2c-designware-master.c)
> 3. Slave part (i2c-designware-slave.c)
> 4. Glue drivers (like: i2c-designware-platdrv.c)
>
>> Thanks,
>> Luis
>>
>> On 10/18/2016 16:17, Wolfram Sang wrote:
>>>> This is needed because the configuration is different and the i2c-
>>>> designware
>>>> cannot be master/slave without a reset. To resolve that I added
>>>> this property
>>>> to bind it as a slave when needed.
>>> Aww, pity that the HW can't do that. Do you have details why?
>>>
>>> If that is really a HW limitation, then I'd suggest having a
>>> seperate
>>> driver for slave-only mode so we can differentiate by compatible
>>> strings.
>>>
>>

^ permalink raw reply

* [PATCH 4/4 v2] arm64: dts: h3ulcb: rename SDHI0 pins
From: Vladimir Barinov @ 2016-11-08 14:16 UTC (permalink / raw)
  To: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland
  Cc: devicetree, linux-renesas-soc, Vladimir Barinov
In-Reply-To: <1478196242-939-1-git-send-email-vladimir.barinov@cogentembedded.com>

This changes SDHI0 pin names for H3ULCB board

Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
---
Changes in version 2:
- Initially added

diff --git a/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts b/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts
index 8d0ac07..6ffb051 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts
@@ -163,13 +163,13 @@
 		function = "avb";
 	};
 
-	sdhi0_pins_3v3: sd0_3v3 {
+	sdhi0_pins: sd0 {
 		groups = "sdhi0_data4", "sdhi0_ctrl";
 		function = "sdhi0";
 		power-source = <3300>;
 	};
 
-	sdhi0_pins_1v8: sd0_1v8 {
+	sdhi0_pins_uhs: sd0 {
 		groups = "sdhi0_data4", "sdhi0_ctrl";
 		function = "sdhi0";
 		power-source = <1800>;
@@ -291,8 +291,8 @@
 };
 
 &sdhi0 {
-	pinctrl-0 = <&sdhi0_pins_3v3>;
-	pinctrl-1 = <&sdhi0_pins_1v8>;
+	pinctrl-0 = <&sdhi0_pins>;
+	pinctrl-1 = <&sdhi0_pins_uhs>;
 	pinctrl-names = "default", "state_uhs";
 
 	vmmc-supply = <&vcc_sdhi0>;

^ permalink raw reply related

* [PATCH 3/4 v2] arm64: dts: h3ulcb: enable SDHI2
From: Vladimir Barinov @ 2016-11-08 14:16 UTC (permalink / raw)
  To: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA, Vladimir Barinov
In-Reply-To: <1478196242-939-1-git-send-email-vladimir.barinov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>

This supports SDHI2 for H3ULCB onboard eMMC

Signed-off-by: Vladimir Barinov <vladimir.barinov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
Reviewed-off-by: Simon Horman <horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
---
Changes in version 2:
- renamed sdhi2_pins_3v3 to sdhi2_pins
- renamed sd2_3v3 to sd2
- renamed sdhi2_pins_1v8 to sdhi2_pins_uhs
- renamed sd2_1v8 to sd2_uhs

 arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts | 43 ++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts b/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts
index a244edb..e46687e 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts
@@ -55,6 +55,24 @@
 		clock-frequency = <24576000>;
 	};
 
+	reg_1p8v: regulator0 {
+		compatible = "regulator-fixed";
+		regulator-name = "fixed-1.8V";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
+	reg_3p3v: regulator1 {
+		compatible = "regulator-fixed";
+		regulator-name = "fixed-3.3V";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
 	vcc_sdhi0: regulator-vcc-sdhi0 {
 		compatible = "regulator-fixed";
 
@@ -113,6 +131,18 @@
 		function = "sdhi0";
 		power-source = <1800>;
 	};
+
+	sdhi2_pins: sd2 {
+		groups = "sdhi2_data8", "sdhi2_ctrl";
+		function = "sdhi2";
+		power-source = <3300>;
+	};
+
+	sdhi2_pins_uhs: sd2_uhs {
+		groups = "sdhi2_data8", "sdhi2_ctrl";
+		function = "sdhi2";
+		power-source = <1800>;
+	};
 
 	sound_pins: sound {
 		groups = "ssi01239_ctrl", "ssi0_data", "ssi1_data_a";
@@ -128,6 +158,19 @@
 	status = "okay";
 };
 
+&sdhi2 {
+	/* used for on-board 8bit eMMC */
+	pinctrl-0 = <&sdhi2_pins>;
+	pinctrl-1 = <&sdhi2_pins_uhs>;
+	pinctrl-names = "default", "state_uhs";
+
+	vmmc-supply = <&reg_3p3v>;
+	vqmmc-supply = <&reg_1p8v>;
+	bus-width = <8>;
+	non-removable;
+	status = "okay";
+};
+
 &ssi1 {
 	shared-pin;
 };
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 9/9 v2] arm64: dts: m3ulcb: enable SDHI2
From: Vladimir Barinov @ 2016-11-08 14:14 UTC (permalink / raw)
  To: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA, Vladimir Barinov
In-Reply-To: <1478196375-1131-1-git-send-email-vladimir.barinov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>

This supports SDHI2 for M3ULCB onboard eMMC

Signed-off-by: Vladimir Barinov <vladimir.barinov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
Reviewed-off-by: Simon Horman <horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
---
Changes in version 2:
- renamed sdhi2_pins_3v3 to sdhi2_pins
- renamed sd2_3v3 to sd2
- renamed sdhi2_pins_1v8 to sdhi2_pins_uhs
- renamed sd2_1v8 to sd2_uhs

 arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts | 43 ++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts b/arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts
index a244edb..e46687e 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts
@@ -55,6 +55,24 @@
 		};
 	};
 
+	reg_1p8v: regulator0 {
+		compatible = "regulator-fixed";
+		regulator-name = "fixed-1.8V";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
+	reg_3p3v: regulator1 {
+		compatible = "regulator-fixed";
+		regulator-name = "fixed-3.3V";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
 	vcc_sdhi0: regulator-vcc-sdhi0 {
 		compatible = "regulator-fixed";
 
@@ -113,6 +131,18 @@
 		function = "sdhi0";
 		power-source = <1800>;
 	};
+
+	sdhi2_pins: sd2 {
+		groups = "sdhi2_data8", "sdhi2_ctrl";
+		function = "sdhi2";
+		power-source = <3300>;
+	};
+
+	sdhi2_pins_uhs: sd2_uhs {
+		groups = "sdhi2_data8", "sdhi2_ctrl";
+		function = "sdhi2";
+		power-source = <1800>;
+	};
 };
 
 &sdhi0 {
@@ -128,6 +158,19 @@
 	status = "okay";
 };
 
+&sdhi2 {
+	/* used for on-board 8bit eMMC */
+	pinctrl-0 = <&sdhi2_pins>;
+	pinctrl-1 = <&sdhi2_pins_uhs>;
+	pinctrl-names = "default", "state_uhs";
+
+	vmmc-supply = <&reg_3p3v>;
+	vqmmc-supply = <&reg_1p8v>;
+	bus-width = <8>;
+	non-removable;
+	status = "okay";
+};
+
 &scif2 {
 	pinctrl-0 = <&scif2_pins>;
 	pinctrl-names = "default";
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 6/9 v2] arm64: dts: m3ulcb: enable SDHI0
From: Vladimir Barinov @ 2016-11-08 14:14 UTC (permalink / raw)
  To: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland
  Cc: devicetree, linux-renesas-soc, Vladimir Barinov
In-Reply-To: <1478196375-1131-1-git-send-email-vladimir.barinov@cogentembedded.com>

This supports SDHI0 on M3ULCB board SD card slot

Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
Reviewed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
Changes in version 2:
- renamed sdhi0_pins_3v3 to sdhi0_pins
- renamed sd0_3v3 to sd0
- renamed sdhi0_pins_1v8 to sdhi0_pins_uhs
- renamed sd0_1v8 to sd0_uhs

 arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts | 48 ++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts b/arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts
index 3329f78..5be0cf6 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts
@@ -44,6 +44,30 @@
 			gpios = <&gpio6 11 GPIO_ACTIVE_LOW>;
 		};
 	};
+
+	vcc_sdhi0: regulator-vcc-sdhi0 {
+		compatible = "regulator-fixed";
+
+		regulator-name = "SDHI0 Vcc";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpio = <&gpio5 2 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+	};
+
+	vccq_sdhi0: regulator-vccq-sdhi0 {
+		compatible = "regulator-gpio";
+
+		regulator-name = "SDHI0 VccQ";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpios = <&gpio5 1 GPIO_ACTIVE_HIGH>;
+		gpios-states = <1>;
+		states = <3300000 1
+			  1800000 0>;
+	};
 };
 
 &extal_clk {
@@ -68,8 +92,33 @@
 		groups = "scif_clk_a";
 		function = "scif_clk";
 	};
+
+	sdhi0_pins: sd0 {
+		groups = "sdhi0_data4", "sdhi0_ctrl";
+		function = "sdhi0";
+		power-source = <3300>;
+	};
+
+	sdhi0_pins_uhs: sd0_uhs {
+		groups = "sdhi0_data4", "sdhi0_ctrl";
+		function = "sdhi0";
+		power-source = <1800>;
+	};
 };
 
+&sdhi0 {
+	pinctrl-0 = <&sdhi0_pins>;
+	pinctrl-1 = <&sdhi0_pins_uhs>;
+	pinctrl-names = "default", "state_uhs";
+
+	vmmc-supply = <&vcc_sdhi0>;
+	vqmmc-supply = <&vccq_sdhi0>;
+	cd-gpios = <&gpio3 12 GPIO_ACTIVE_LOW>;
+	bus-width = <4>;
+	sd-uhs-sdr50;
+	status = "okay";
+};
+
 &scif2 {
 	pinctrl-0 = <&scif2_pins>;
 	pinctrl-names = "default";
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads
From: Laxman Dewangan @ 2016-11-08 13:35 UTC (permalink / raw)
  To: Linus Walleij
  Cc: thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	Stephen Warren, Rob Herring, Mark Rutland, Jon Hunter,
	Masahiro Yamada,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CACRpkdYO9ZCTGVdnyhVZ4DNLsHq+d-1_xuws2be8yab+MsyyVw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>


On Tuesday 08 November 2016 06:59 PM, Linus Walleij wrote:
> On Tue, Nov 8, 2016 at 11:20 AM, Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>> On Tuesday 08 November 2016 03:45 PM, Linus Walleij wrote:
>>> If you can *actually* change the volatage, it needs to be modeled
>>> as a (fixed voltage?) regulator, not as a custom property for the pin
>>> control attributes. I guess you definiately need the regulator framework
>>> to accumulate and infer the different consumer requirements anyway
>>> in that case.
>> The PMIC voltage output is changed via regulator calls.
>> Here, we need to have two configruations for given voltage level of
>> interface:
>> * One at IO voltage from PMIC via regulator call to change votlage of IO
>> rail.
>> * Second, configure the IO pad register to tell the IO voltage level so that
>> it can configured internally for that level.
> I understand! (I think.)

Thanks,

>
> But then the two things (A) changing the regulator voltage and (B) changing
> the pin setting need to happen at the same time do they
> not?
>
> Now you're just hardcoding something into these device tree properties
> and hoping that the regulators will somehow be set up in accordance to
> what you set up for the pads in the device tree, correct?

There is two types of configuration in given platform, the IO voltage 
does not get change (fixed in given platform) and in some of cases, get 
change dynamically like SDIO3.0 where the voltage switches to 3.3V and 1.8V.

Yes, it can be integrated with the regulator handle and then it can call 
the required configurations through notifier and regulator_get_voltage().
But I think it is too much complex for the static configurations. This 
mandate also to populate the regulator handle and all power tree.

The simple way for static configuration (case where voltage does not get 
change), just take the power tree IO voltage from DT and configure the 
IO pad control register.

For dynamic case, there is some sequence need to be followed based on 
voltage direction change (towards lower or towards higher) for the 
voltage change and the IO pad voltage configuration and it is simple to 
do it from client driver.



>
> To me it seems like the pins/pads should all have an <&phandle> to
> the regulator controlling its voltage output, in the device tree.
>
> In the Linux kernel, the driver has to regulator_[bulk_]get() this for
> each pin, check the voltage with regulator_get_voltage() and set up
> this according to the supplied voltage.
>
> The driver then ideally should subscribe to regulator voltage notifier
> events to change the setting if the voltage changes. I guess. But
> atleast the first step seems inevitable: get the voltage from a regulator.
>
> Else there is no dependency between the regulator and its consumer.
>
> So what your pins need is a regulator phandle, not a magic value to
> be poked into a register, hoping things will match up.
>
> I understand that this is a simple quick-and-dirty solution but it is
> not the right solution.


Yaah, the static power tree configuration is much simple in this 
approach without having regulator drivers and support.

Integrating with regulator driver can be done here also.

I like to have both approach, through pinmux DT and also from regulator. 
So based on the platform, if regulator supported then populate required 
properties in DT for regulator else go on standard pinmux DT way (for 
non-regulator cases).

Need your opinion?

^ permalink raw reply

* Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads
From: Linus Walleij @ 2016-11-08 13:29 UTC (permalink / raw)
  To: Laxman Dewangan
  Cc: thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	Stephen Warren, Rob Herring, Mark Rutland, Jon Hunter,
	Masahiro Yamada,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <5821A6D3.7010000-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

On Tue, Nov 8, 2016 at 11:20 AM, Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> On Tuesday 08 November 2016 03:45 PM, Linus Walleij wrote:

>> If you can *actually* change the volatage, it needs to be modeled
>> as a (fixed voltage?) regulator, not as a custom property for the pin
>> control attributes. I guess you definiately need the regulator framework
>> to accumulate and infer the different consumer requirements anyway
>> in that case.
>
> The PMIC voltage output is changed via regulator calls.
> Here, we need to have two configruations for given voltage level of
> interface:
> * One at IO voltage from PMIC via regulator call to change votlage of IO
> rail.
> * Second, configure the IO pad register to tell the IO voltage level so that
> it can configured internally for that level.

I understand! (I think.)

But then the two things (A) changing the regulator voltage and (B) changing
the pin setting need to happen at the same time do they
not?

Now you're just hardcoding something into these device tree properties
and hoping that the regulators will somehow be set up in accordance to
what you set up for the pads in the device tree, correct?

To me it seems like the pins/pads should all have an <&phandle> to
the regulator controlling its voltage output, in the device tree.

In the Linux kernel, the driver has to regulator_[bulk_]get() this for
each pin, check the voltage with regulator_get_voltage() and set up
this according to the supplied voltage.

The driver then ideally should subscribe to regulator voltage notifier
events to change the setting if the voltage changes. I guess. But
atleast the first step seems inevitable: get the voltage from a regulator.

Else there is no dependency between the regulator and its consumer.

So what your pins need is a regulator phandle, not a magic value to
be poked into a register, hoping things will match up.

I understand that this is a simple quick-and-dirty solution but it is
not the right solution.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 4/6] ARM: dts: add basic support for Rockchip RK1108 SOC
From: Heiko Stübner @ 2016-11-08 13:20 UTC (permalink / raw)
  To: Andy Yan
  Cc: elaine.zhang-TNX95d0MmH7DzftRWevZcw,
	mturquette-rdvid1DuHRBWk0Htik3J/w,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <0516ad0b-bfbe-ec80-fdb6-e118dab3e758-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

Am Dienstag, 8. November 2016, 20:31:55 schrieb Andy Yan:
> Hi Heiko:
> 
> On 2016年11月04日 16:00, Heiko Stuebner wrote:
> > Am Donnerstag, 3. November 2016, 20:40:48 CET schrieb Andy Yan:

> >> +	gic: interrupt-controller@32010000 {
> >> +		compatible = "arm,cortex-a15-gic";
> > 
> > compatible = "arm,gic-400"; ?
> > 
> >> +		interrupt-controller;
> >> +		#interrupt-cells = <3>;
> >> +		#address-cells = <0>;
> >> +
> >> +		reg = <0x32011000 0x1000>,
> >> +		      <0x32012000 0x1000>;
> > 
> > please provide all 4 register areas and also the interrupt (
> 
>      I only found 2 register areas in our rockchip linux 3.10 source
> code. And haven't found the interrupt. From the arm,gic bindings, the
> interrupt property is optional. So am not sure if we
> really need it here.

Devicetree is a hardware description, so it's not a factor if we "need" it but 
only if it is present in the hardware. And we really want this information to 
be complete, as these additional areas are necessary if someone wants to use 
the virtualization extensions the cortext-A7 does contain.

The gic is a very standard component and the gic400 used here should definitly 
have those two additional areas as well as the interrupt.

I think the memory areas are pretty standard and should be for the rk1108:
reg = <0x32011000 0x1000>,
      <0x32012000 0x1000>,
      <0x32014000 0x2000>,
      <0x32016000 0x2000>;

The TRM talks about 128 SPI and 3 PPI interrupts but the irq-list does not 
contain them, so this seems to be an error in the TRM, as the gic interrupt 
should be one of those PPI interrupts.


Heiko

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] ARM: tegra: nyan: Mark all USB ports as host
From: Paul Kocialkowski @ 2016-11-08 13:02 UTC (permalink / raw)
  To: Jon Hunter, Thierry Reding
  Cc: Peter De Schrijver, devicetree, Stephen Warren, linux-kernel,
	linux-tegra, Alexandre Courbot, linux-arm-kernel
In-Reply-To: <3fbd3fb9-1dc5-a46d-355b-f7c94b3c43ef@nvidia.com>

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

Le mardi 08 novembre 2016 à 11:09 +0000, Jon Hunter a écrit :
> On 08/11/16 11:07, Thierry Reding wrote:
> > 
> > * PGP Signed by an unknown key
> > 
> > On Tue, Nov 08, 2016 at 09:47:42AM +0000, Jon Hunter wrote:
> > > 
> > > 
> > > On 08/11/16 08:54, Peter De Schrijver wrote:
> > > > 
> > > > On Mon, Nov 07, 2016 at 02:09:31PM +0000, Jon Hunter wrote:
> > > > > 
> > > > > 
> > > > > On 07/11/16 13:28, Thierry Reding wrote:
> > > > > > 
> > > > > > > 
> > > > > > > Old Signed by an unknown key
> > > > > > 
> > > > > > On Sun, Sep 18, 2016 at 12:28:52PM +0200, Paul Kocialkowski wrote:
> > > > > > > 
> > > > > > > Nyan boards only have host USB ports (2 external, 1 internal),
> > > > > > > there is
> > > > > > > no OTG-enabled connector.
> > > > > > > 
> > > > > > > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > > > > > > ---
> > > > > > >  arch/arm/boot/dts/tegra124-nyan.dtsi | 2 +-
> > > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > 
> > > > > > Where is this information coming from? I don't have one of the Nyans
> > > > > > myself, but one of the Tegra132 devices I have, which I think was
> > > > > > derived from one of the Nyans uses one of the external host ports as
> > > > > > forced recovery port, for which it would need OTG.
> > > > > > 
> > > > > > I suspect that the way to get U-Boot onto the Nyans is via tegrarcm?
> > > > > > In that case I think one of the ports must be OTG.
> > > > > 
> > > > > It is true that the port on the back on the nyan-big can be used with
> > > > > recovery mode. I was thinking that this is not a true OTG port as it
> > > > > is
> > > > > just a 4-pin type A socket and does not have an ID pin. Thinking some
> > > > > more about this the USB spec does include a "Host Negotiation Protocol
> > > > > (HNP)" that allows a host and device to swap roles and so keeping it
> > > > > as
> > > > > OTG seems valid afterall.
> > > > 
> > > > I don't think the bootrom implements that though. I expect recovery mode
> > > > to just program the controller in device mode, without performing any
> > > > negotiation.
> > > 
> > > I am not talking about the bootrom and I would not expect the bootrom to
> > > do that. However, the kernel could.
> > 
> > Either way, configuring the controller in device mode is enough to make
> > the host detect it, otherwise tegrarcm wouldn't work.
> > 
> > From the point of view of the binding I think "otg" is the most accurate
> > option because we know that the controller can operate in both modes. If
> > it currently doesn't or how exactly switching modes is done is outside
> > the scope of this property.
> > 
> > Is everyone okay with just dropping this patch?
> 
> Fine with me.

Same here.

-- 
Paul Kocialkowski, developer of low-level free software for embedded devices

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* Re: [PATCH v6 14/14] sdhci: sdhci-msm: update dll configuration
From: Adrian Hunter @ 2016-11-08 12:57 UTC (permalink / raw)
  To: Ritesh Harjani, ulf.hansson-QSEj5FYQhm4dnm+yROfE0A,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	shawn.lin-TNX95d0MmH7DzftRWevZcw, sboyd-sgV2jX0FEOL9JmXXK+q4OQ
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	david.brown-QSEj5FYQhm4dnm+yROfE0A,
	andy.gross-QSEj5FYQhm4dnm+yROfE0A,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	georgi.djakov-QSEj5FYQhm4dnm+yROfE0A,
	alex.lemberg-XdAiOPVOjttBDgjK7y7TUQ,
	mateusz.nowak-ral2JQCrhuEAvxtiuMwx3w,
	Yuliy.Izrailov-XdAiOPVOjttBDgjK7y7TUQ,
	asutoshd-sgV2jX0FEOL9JmXXK+q4OQ, kdorfman-sgV2jX0FEOL9JmXXK+q4OQ,
	david.griego-QSEj5FYQhm4dnm+yROfE0A,
	stummala-sgV2jX0FEOL9JmXXK+q4OQ, venkatg-sgV2jX0FEOL9JmXXK+q4OQ,
	rnayak-sgV2jX0FEOL9JmXXK+q4OQ,
	pramod.gurav-QSEj5FYQhm4dnm+yROfE0A, Krishna Konda
In-Reply-To: <1478517877-23733-15-git-send-email-riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

On 07/11/16 13:24, Ritesh Harjani wrote:
> The newer msm sdhci's cores use a different DLL hardware for HS400.
> Update the configuration and calibration of the newer DLL block.
> 
> The HS400 DLL block used previously is CDC LP 533 and requires
> programming multiple registers and waiting for configuration to
> complete and then enable it. It has about 18 register writes and
> two register reads.
> 
> The newer HS400 DLL block is SDC4 DLL and requires two register
> writes for configuration and one register read to confirm that it
> is initialized. There is an additional register write to enable
> the power save mode for SDC4 DLL block.
> 
> Signed-off-by: Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Signed-off-by: Krishna Konda <kkonda-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

Acked-by: Adrian Hunter <adrian.hunter-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

> ---
>  drivers/mmc/host/sdhci-msm.c | 132 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 130 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index ae19d1a..84812e6 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -51,6 +51,7 @@
>  #define INT_MASK		0xf
>  #define MAX_PHASES		16
>  #define CORE_DLL_LOCK		BIT(7)
> +#define CORE_DDR_DLL_LOCK	BIT(11)
>  #define CORE_DLL_EN		BIT(16)
>  #define CORE_CDR_EN		BIT(17)
>  #define CORE_CK_OUT_EN		BIT(18)
> @@ -62,6 +63,7 @@
>  #define CORE_DLL_STATUS		0x108
>  
>  #define CORE_DLL_CONFIG_2	0x1b4
> +#define CORE_DDR_CAL_EN		BIT(0)
>  #define CORE_FLL_CYCLE_CNT	BIT(18)
>  #define CORE_DLL_CLOCK_DISABLE	BIT(21)
>  
> @@ -100,6 +102,11 @@
>  #define CORE_DDR_200_CFG		0x184
>  #define CORE_CDC_T4_DLY_SEL		BIT(0)
>  #define CORE_START_CDC_TRAFFIC		BIT(6)
> +#define CORE_VENDOR_SPEC3	0x1b0
> +#define CORE_PWRSAVE_DLL	BIT(3)
> +
> +#define CORE_DDR_CONFIG		0x1b8
> +#define DDR_CONFIG_POR_VAL	0x80040853
>  
>  #define CORE_VENDOR_SPEC_CAPABILITIES0	0x11c
>  
> @@ -126,6 +133,7 @@ struct sdhci_msm_host {
>  	bool tuning_done;
>  	bool calibration_done;
>  	u8 saved_tuning_phase;
> +	bool use_cdclp533;
>  };
>  
>  /* Platform specific tuning */
> @@ -583,6 +591,93 @@ static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
>  	return ret;
>  }
>  
> +static int sdhci_msm_cm_dll_sdc4_calibration(struct sdhci_host *host)
> +{
> +	u32 dll_status, config;
> +	int ret;
> +
> +	pr_debug("%s: %s: Enter\n", mmc_hostname(host->mmc), __func__);
> +
> +	/*
> +	 * Currently the CORE_DDR_CONFIG register defaults to desired
> +	 * configuration on reset. Currently reprogramming the power on
> +	 * reset (POR) value in case it might have been modified by
> +	 * bootloaders. In the future, if this changes, then the desired
> +	 * values will need to be programmed appropriately.
> +	 */
> +	writel_relaxed(DDR_CONFIG_POR_VAL, host->ioaddr + CORE_DDR_CONFIG);
> +
> +	/* Write 1 to DDR_CAL_EN field in CORE_DLL_CONFIG_2 */
> +	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG_2);
> +	config |= CORE_DDR_CAL_EN;
> +	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG_2);
> +
> +	/* Poll on DDR_DLL_LOCK bit in CORE_DLL_STATUS to be set */
> +	ret = readl_relaxed_poll_timeout(host->ioaddr + CORE_DLL_STATUS,
> +					 dll_status,
> +					 (dll_status & CORE_DDR_DLL_LOCK),
> +					 10, 1000);
> +
> +	if (ret == -ETIMEDOUT) {
> +		pr_err("%s: %s: CM_DLL_SDC4 calibration was not completed\n",
> +		       mmc_hostname(host->mmc), __func__);
> +		goto out;
> +	}
> +
> +	/* set CORE_PWRSAVE_DLL bit in CORE_VENDOR_SPEC3 */
> +	config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC3);
> +	config |= CORE_PWRSAVE_DLL;
> +	writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC3);
> +
> +	/*
> +	 * Drain writebuffer to ensure above DLL calibration
> +	 * and PWRSAVE DLL is enabled.
> +	 */
> +	wmb();
> +out:
> +	pr_debug("%s: %s: Exit, ret %d\n", mmc_hostname(host->mmc),
> +		 __func__, ret);
> +	return ret;
> +}
> +
> +static int sdhci_msm_hs400_dll_calibration(struct sdhci_host *host)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +	int ret;
> +	u32 config;
> +
> +	pr_debug("%s: %s: Enter\n", mmc_hostname(host->mmc), __func__);
> +
> +	/*
> +	 * Retuning in HS400 (DDR mode) will fail, just reset the
> +	 * tuning block and restore the saved tuning phase.
> +	 */
> +	ret = msm_init_cm_dll(host);
> +	if (ret)
> +		goto out;
> +
> +	/* Set the selected phase in delay line hw block */
> +	ret = msm_config_cm_dll_phase(host, msm_host->saved_tuning_phase);
> +	if (ret)
> +		goto out;
> +
> +	/* Write 1 to CMD_DAT_TRACK_SEL field in DLL_CONFIG */
> +	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> +	config |= CORE_CMD_DAT_TRACK_SEL;
> +	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +	if (msm_host->use_cdclp533)
> +		/* Calibrate CDCLP533 DLL HW */
> +		ret = sdhci_msm_cdclp533_calibration(host);
> +	else
> +		/* Calibrate CM_DLL_SDC4 HW */
> +		ret = sdhci_msm_cm_dll_sdc4_calibration(host);
> +out:
> +	pr_debug("%s: %s: Exit, ret %d\n", mmc_hostname(host->mmc),
> +		 __func__, ret);
> +	return ret;
> +}
> +
>  static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
>  {
>  	int tuning_seq_cnt = 3;
> @@ -731,7 +826,7 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>  	if (host->clock > CORE_FREQ_100MHZ &&
>  	    msm_host->tuning_done && !msm_host->calibration_done &&
>  	    (mmc->ios.timing == MMC_TIMING_MMC_HS400))
> -		if (!sdhci_msm_cdclp533_calibration(host))
> +		if (!sdhci_msm_hs400_dll_calibration(host))
>  			msm_host->calibration_done = true;
>  	spin_lock_irq(&host->lock);
>  }
> @@ -842,7 +937,7 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>  	struct mmc_ios curr_ios = host->mmc->ios;
> -	u32 config;
> +	u32 config, dll_lock;
>  	int rc;
>  
>  	if (!clock) {
> @@ -899,7 +994,33 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>  			config |= CORE_HC_SELECT_IN_EN;
>  			writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
>  		}
> +		if (!msm_host->clk_rate && !msm_host->use_cdclp533) {
> +			/*
> +			 * Poll on DLL_LOCK or DDR_DLL_LOCK bits in
> +			 * CORE_DLL_STATUS to be set.  This should get set
> +			 * within 15 us at 200 MHz.
> +			 */
> +			rc = readl_relaxed_poll_timeout(host->ioaddr +
> +							CORE_DLL_STATUS,
> +							dll_lock,
> +							(dll_lock &
> +							(CORE_DLL_LOCK |
> +							CORE_DDR_DLL_LOCK)), 10,
> +							1000);
> +			if (rc == -ETIMEDOUT)
> +				pr_err("%s: Unable to get DLL_LOCK/DDR_DLL_LOCK, dll_status: 0x%08x\n",
> +				       mmc_hostname(host->mmc), dll_lock);
> +		}
>  	} else {
> +		if (!msm_host->use_cdclp533) {
> +			/* set CORE_PWRSAVE_DLL bit in CORE_VENDOR_SPEC3 */
> +			config = readl_relaxed(host->ioaddr +
> +					CORE_VENDOR_SPEC3);
> +			config &= ~CORE_PWRSAVE_DLL;
> +			writel_relaxed(config, host->ioaddr +
> +					CORE_VENDOR_SPEC3);
> +		}
> +
>  		/* Select the default clock (free running MCLK) */
>  		config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
>  		config &= ~CORE_HC_MCLK_SEL_MASK;
> @@ -1086,6 +1207,13 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  		msm_host->use_14lpp_dll_reset = true;
>  
>  	/*
> +	 * SDCC 5 controller with major version 1, minor version 0x34 and later
> +	 * with HS 400 mode support will use CM DLL instead of CDC LP 533 DLL.
> +	 */
> +	if ((core_major == 1) && (core_minor < 0x34))
> +		msm_host->use_cdclp533 = true;
> +
> +	/*
>  	 * Support for some capabilities is not advertised by newer
>  	 * controller versions and must be explicitly enabled.
>  	 */
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v6 13/14] mmc: sdhci-msm: Add calibration tuning for CDCLP533 circuit
From: Adrian Hunter @ 2016-11-08 12:50 UTC (permalink / raw)
  To: Ritesh Harjani, ulf.hansson, linux-mmc, shawn.lin, sboyd
  Cc: devicetree, linux-clk, david.brown, andy.gross, linux-arm-msm,
	georgi.djakov, alex.lemberg, mateusz.nowak, Yuliy.Izrailov,
	asutoshd, kdorfman, david.griego, stummala, venkatg, rnayak,
	pramod.gurav
In-Reply-To: <1478517877-23733-14-git-send-email-riteshh@codeaurora.org>

On 07/11/16 13:24, Ritesh Harjani wrote:
> In HS400 mode a new RCLK is introduced on the interface for read data
> transfers. The eMMC5.0 device transmits the read data to the host with
> respect to rising and falling edges of RCLK. In order to ensure correct
> operation of read data transfers in HS400 mode, the incoming RX data
> needs to be sampled by delayed version of RCLK.
> 
> The CDCLP533 delay circuit shifts the RCLK by T/4. It needs to be
> initialized, configured and enabled once during HS400 mode switch and
> when operational voltage/clock is changed.
> 
> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-msm.c | 163 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 163 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 6431bb8..ae19d1a 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -19,6 +19,7 @@
>  #include <linux/delay.h>
>  #include <linux/mmc/mmc.h>
>  #include <linux/slab.h>
> +#include <linux/iopoll.h>
>  
>  #include "sdhci-pltfm.h"
>  
> @@ -57,6 +58,7 @@
>  #define CORE_DLL_PDN		BIT(29)
>  #define CORE_DLL_RST		BIT(30)
>  #define CORE_DLL_CONFIG		0x100
> +#define CORE_CMD_DAT_TRACK_SEL	BIT(0)
>  #define CORE_DLL_STATUS		0x108
>  
>  #define CORE_DLL_CONFIG_2	0x1b4
> @@ -72,6 +74,33 @@
>  #define CORE_HC_SELECT_IN_HS400	(6 << 19)
>  #define CORE_HC_SELECT_IN_MASK	(7 << 19)
>  
> +#define CORE_CSR_CDC_CTLR_CFG0		0x130
> +#define CORE_SW_TRIG_FULL_CALIB		BIT(16)
> +#define CORE_HW_AUTOCAL_ENA		BIT(17)
> +
> +#define CORE_CSR_CDC_CTLR_CFG1		0x134
> +#define CORE_CSR_CDC_CAL_TIMER_CFG0	0x138
> +#define CORE_TIMER_ENA			BIT(16)
> +
> +#define CORE_CSR_CDC_CAL_TIMER_CFG1	0x13C
> +#define CORE_CSR_CDC_REFCOUNT_CFG	0x140
> +#define CORE_CSR_CDC_COARSE_CAL_CFG	0x144
> +#define CORE_CDC_OFFSET_CFG		0x14C
> +#define CORE_CSR_CDC_DELAY_CFG		0x150
> +#define CORE_CDC_SLAVE_DDA_CFG		0x160
> +#define CORE_CSR_CDC_STATUS0		0x164
> +#define CORE_CALIBRATION_DONE		BIT(0)
> +
> +#define CORE_CDC_ERROR_CODE_MASK	0x7000000
> +
> +#define CORE_CSR_CDC_GEN_CFG		0x178
> +#define CORE_CDC_SWITCH_BYPASS_OFF	BIT(0)
> +#define CORE_CDC_SWITCH_RC_EN		BIT(1)
> +
> +#define CORE_DDR_200_CFG		0x184
> +#define CORE_CDC_T4_DLY_SEL		BIT(0)
> +#define CORE_START_CDC_TRAFFIC		BIT(6)
> +
>  #define CORE_VENDOR_SPEC_CAPABILITIES0	0x11c
>  
>  #define INVALID_TUNING_PHASE	-1
> @@ -429,6 +458,131 @@ static int msm_init_cm_dll(struct sdhci_host *host)
>  	return 0;
>  }
>  
> +static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +	u32 config, calib_done;
> +	int ret;
> +
> +	pr_debug("%s: %s: Enter\n", mmc_hostname(host->mmc), __func__);
> +
> +	/*
> +	 * Retuning in HS400 (DDR mode) will fail, just reset the
> +	 * tuning block and restore the saved tuning phase.
> +	 */
> +	ret = msm_init_cm_dll(host);
> +	if (ret)
> +		goto out;
> +
> +	/* Set the selected phase in delay line hw block */
> +	ret = msm_config_cm_dll_phase(host, msm_host->saved_tuning_phase);
> +	if (ret)
> +		goto out;
> +
> +	/* Write 1 to CMD_DAT_TRACK_SEL field in DLL_CONFIG */
> +	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> +	config |= CORE_CMD_DAT_TRACK_SEL;
> +	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +
> +	/* Write 0 to CDC_T4_DLY_SEL field in VENDOR_SPEC_DDR200_CFG */
> +	config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
> +	config &= ~CORE_CDC_T4_DLY_SEL;
> +	writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
> +
> +	/* Write 0 to CDC_SWITCH_BYPASS_OFF field in CORE_CSR_CDC_GEN_CFG */
> +	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_GEN_CFG);
> +	config &= ~CORE_CDC_SWITCH_BYPASS_OFF;
> +	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_GEN_CFG);
> +
> +	/* Write 1 to CDC_SWITCH_RC_EN field in CORE_CSR_CDC_GEN_CFG */
> +	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_GEN_CFG);
> +	config |= CORE_CDC_SWITCH_RC_EN;
> +	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_GEN_CFG);
> +
> +	/* Write 0 to START_CDC_TRAFFIC field in CORE_DDR200_CFG */
> +	config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
> +	config &= ~CORE_START_CDC_TRAFFIC;
> +	writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
> +
> +	/*
> +	 * Perform CDC Register Initialization Sequence
> +	 *
> +	 * CORE_CSR_CDC_CTLR_CFG0	0x11800EC
> +	 * CORE_CSR_CDC_CTLR_CFG1	0x3011111
> +	 * CORE_CSR_CDC_CAL_TIMER_CFG0	0x1201000
> +	 * CORE_CSR_CDC_CAL_TIMER_CFG1	0x4
> +	 * CORE_CSR_CDC_REFCOUNT_CFG	0xCB732020
> +	 * CORE_CSR_CDC_COARSE_CAL_CFG	0xB19
> +	 * CORE_CSR_CDC_DELAY_CFG	0x3AC
> +	 * CORE_CDC_OFFSET_CFG		0x0
> +	 * CORE_CDC_SLAVE_DDA_CFG	0x16334
> +	 */
> +
> +	writel_relaxed(0x11800EC, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> +	writel_relaxed(0x3011111, host->ioaddr + CORE_CSR_CDC_CTLR_CFG1);
> +	writel_relaxed(0x1201000, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
> +	writel_relaxed(0x4, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG1);
> +	writel_relaxed(0xCB732020, host->ioaddr + CORE_CSR_CDC_REFCOUNT_CFG);
> +	writel_relaxed(0xB19, host->ioaddr + CORE_CSR_CDC_COARSE_CAL_CFG);
> +	writel_relaxed(0x3AC, host->ioaddr + CORE_CSR_CDC_DELAY_CFG);
> +	writel_relaxed(0x0, host->ioaddr + CORE_CDC_OFFSET_CFG);
> +	writel_relaxed(0x16334, host->ioaddr + CORE_CDC_SLAVE_DDA_CFG);
> +
> +	/* CDC HW Calibration */
> +
> +	/* Write 1 to SW_TRIG_FULL_CALIB field in CORE_CSR_CDC_CTLR_CFG0 */
> +	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> +	config |= CORE_SW_TRIG_FULL_CALIB;
> +	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> +
> +	/* Write 0 to SW_TRIG_FULL_CALIB field in CORE_CSR_CDC_CTLR_CFG0 */
> +	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> +	config &= ~CORE_SW_TRIG_FULL_CALIB;
> +	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> +
> +	/* Write 1 to HW_AUTOCAL_ENA field in CORE_CSR_CDC_CTLR_CFG0 */
> +	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> +	config |= CORE_HW_AUTOCAL_ENA;
> +	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> +
> +	/* Write 1 to TIMER_ENA field in CORE_CSR_CDC_CAL_TIMER_CFG0 */
> +	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
> +	config |= CORE_TIMER_ENA;
> +	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
> +
> +	/* Poll on CALIBRATION_DONE field in CORE_CSR_CDC_STATUS0 to be 1 */
> +	ret = readl_relaxed_poll_timeout(host->ioaddr + CORE_CSR_CDC_STATUS0,
> +					 calib_done,
> +					 (calib_done & CORE_CALIBRATION_DONE),
> +					 1, 50);
> +
> +	if (ret == -ETIMEDOUT) {
> +		pr_err("%s: %s: CDC calibration was not completed\n",
> +		       mmc_hostname(host->mmc), __func__);
> +		goto out;
> +	}
> +
> +	/* Verify CDC_ERROR_CODE field in CORE_CSR_CDC_STATUS0 is 0 */
> +	ret = readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
> +			& CORE_CDC_ERROR_CODE_MASK;
> +	if (ret) {
> +		pr_err("%s: %s: CDC error code %d\n",
> +		       mmc_hostname(host->mmc), __func__, ret);
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	/* Write 1 to START_CDC_TRAFFIC field in CORE_DDR200_CFG */
> +	config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
> +	config |= CORE_START_CDC_TRAFFIC;
> +	writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
> +out:
> +	pr_debug("%s: %s: Exit, ret %d\n", mmc_hostname(host->mmc),
> +		 __func__, ret);
> +	return ret;
> +}
> +
>  static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
>  {
>  	int tuning_seq_cnt = 3;
> @@ -571,6 +725,15 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>  	dev_dbg(mmc_dev(mmc), "%s: clock=%u uhs=%u ctrl_2=0x%x\n",
>  		mmc_hostname(host->mmc), host->clock, uhs, ctrl_2);
>  	sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
> +
> +	spin_unlock_irq(&host->lock);
> +	/* CDCLP533 HW calibration is only required for HS400 mode*/
> +	if (host->clock > CORE_FREQ_100MHZ &&
> +	    msm_host->tuning_done && !msm_host->calibration_done &&
> +	    (mmc->ios.timing == MMC_TIMING_MMC_HS400))
> +		if (!sdhci_msm_cdclp533_calibration(host))
> +			msm_host->calibration_done = true;
> +	spin_lock_irq(&host->lock);
>  }
>  
>  static void sdhci_msm_voltage_switch(struct sdhci_host *host)
> 


^ 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