Devicetree
 help / color / mirror / Atom feed
* [PATCH v3] PCI: mediatek: Add system pm support for MT2712
From: honghui.zhang @ 2018-06-01  3:04 UTC (permalink / raw)
  To: lorenzo.pieralisi, marc.zyngier, bhelgaas, matthias.bgg,
	linux-arm-kernel, linux-mediatek, linux-pci, linux-kernel,
	devicetree, yingjoe.chen, eddie.huang, ryder.lee
  Cc: honghui.zhang, hongkun.cao, youlin.pei, yong.wu, yt.shen,
	sean.wang, xinping.qian

From: Honghui Zhang <honghui.zhang@mediatek.com>

The MTCMOS of PCIe Host for MT2712 will be off when system suspend, and all
the internal control register will be reset after system resume. The PCIe
link should be re-established and the related control register values
should be re-set after system resume.

Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
CC: Ryder Lee <ryder.lee@mediatek.com>
---
 drivers/pci/host/pcie-mediatek.c | 60 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/drivers/pci/host/pcie-mediatek.c b/drivers/pci/host/pcie-mediatek.c
index dabf1086..5363cc7 100644
--- a/drivers/pci/host/pcie-mediatek.c
+++ b/drivers/pci/host/pcie-mediatek.c
@@ -132,12 +132,14 @@ struct mtk_pcie_port;
 /**
  * struct mtk_pcie_soc - differentiate between host generations
  * @need_fix_class_id: whether this host's class ID needed to be fixed or not
+ * @pm_support: whether the host's MTCMOS will be off when suspend
  * @ops: pointer to configuration access functions
  * @startup: pointer to controller setting functions
  * @setup_irq: pointer to initialize IRQ functions
  */
 struct mtk_pcie_soc {
 	bool need_fix_class_id;
+	bool pm_support;
 	struct pci_ops *ops;
 	int (*startup)(struct mtk_pcie_port *port);
 	int (*setup_irq)(struct mtk_pcie_port *port, struct device_node *node);
@@ -1179,12 +1181,69 @@ static int mtk_pcie_probe(struct platform_device *pdev)
 	return err;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int mtk_pcie_suspend_noirq(struct device *dev)
+{
+	struct mtk_pcie *pcie = dev_get_drvdata(dev);
+	const struct mtk_pcie_soc *soc = pcie->soc;
+	struct mtk_pcie_port *port;
+
+	if (!soc->pm_support)
+		return 0;
+
+	list_for_each_entry(port, &pcie->ports, list) {
+		clk_disable_unprepare(port->ahb_ck);
+		clk_disable_unprepare(port->sys_ck);
+		phy_power_off(port->phy);
+	}
+
+	return 0;
+}
+
+static int mtk_pcie_resume_noirq(struct device *dev)
+{
+	struct mtk_pcie *pcie = dev_get_drvdata(dev);
+	const struct mtk_pcie_soc *soc = pcie->soc;
+	struct mtk_pcie_port *port;
+	int ret;
+
+	if (!soc->pm_support)
+		return 0;
+
+	list_for_each_entry(port, &pcie->ports, list) {
+		phy_power_on(port->phy);
+		clk_prepare_enable(port->sys_ck);
+		clk_prepare_enable(port->ahb_ck);
+
+		ret = soc->startup(port);
+		if (ret) {
+			dev_err(dev, "Port%d link down\n", port->slot);
+			phy_power_off(port->phy);
+			clk_disable_unprepare(port->sys_ck);
+			clk_disable_unprepare(port->ahb_ck);
+			return ret;
+		}
+
+		if (IS_ENABLED(CONFIG_PCI_MSI))
+			mtk_pcie_enable_msi(port);
+	}
+
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops mtk_pcie_pm_ops = {
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(mtk_pcie_suspend_noirq,
+				      mtk_pcie_resume_noirq)
+};
+
 static const struct mtk_pcie_soc mtk_pcie_soc_v1 = {
 	.ops = &mtk_pcie_ops,
 	.startup = mtk_pcie_startup_port,
 };
 
 static const struct mtk_pcie_soc mtk_pcie_soc_mt2712 = {
+	.pm_support = true,
 	.ops = &mtk_pcie_ops_v2,
 	.startup = mtk_pcie_startup_port_v2,
 	.setup_irq = mtk_pcie_setup_irq,
@@ -1211,6 +1270,7 @@ static struct platform_driver mtk_pcie_driver = {
 		.name = "mtk-pcie",
 		.of_match_table = mtk_pcie_ids,
 		.suppress_bind_attrs = true,
+		.pm = &mtk_pcie_pm_ops,
 	},
 };
 builtin_platform_driver(mtk_pcie_driver);
-- 
2.6.4

^ permalink raw reply related

* Re: [PATCH 6/6] arm64: dts: socionext: Add missing cooling device properties for CPUs
From: Masahiro Yamada @ 2018-06-01  3:25 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: arm-soc, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	Vincent Guittot, ionela.voinescu, Daniel Lezcano, chris.redpath,
	DTML, linux-arm-kernel, Linux Kernel Mailing List
In-Reply-To: <744f4c0a9a6f0d3acfc36e49ef62f17f53831b3b.1527225682.git.viresh.kumar@linaro.org>

2018-05-25 14:40 GMT+09:00 Viresh Kumar <viresh.kumar@linaro.org>:
> The cooling device properties, like "#cooling-cells" and
> "dynamic-power-coefficient", should either be present for all the CPUs
> of a cluster or none. If these are present only for a subset of CPUs of
> a cluster then things will start falling apart as soon as the CPUs are
> brought online in a different order. For example, this will happen
> because the operating system looks for such properties in the CPU node
> it is trying to bring up, so that it can register a cooling device.
>
> Add such missing properties.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>


Applied to linux-uniphier.

I had already sent a PR for v4.18-rc1 before I received this patch.
Please wait for v4.19-rc1.

Thanks.

> ---
>  arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi b/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi
> index 3a5ed789c056..10ffb5019013 100644
> --- a/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi
> +++ b/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi
> @@ -58,6 +58,7 @@
>                         clocks = <&sys_clk 32>;
>                         enable-method = "psci";
>                         operating-points-v2 = <&cluster0_opp>;
> +                       #cooling-cells = <2>;
>                 };
>
>                 cpu2: cpu@100 {
> @@ -77,6 +78,7 @@
>                         clocks = <&sys_clk 33>;
>                         enable-method = "psci";
>                         operating-points-v2 = <&cluster1_opp>;
> +                       #cooling-cells = <2>;
>                 };
>         };
>
> --
> 2.15.0.194.g9af6a3dea062
>



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply

* Re: [PATCH 05/15] arm: dts: uniphier: Add missing cooling device properties for CPUs
From: Masahiro Yamada @ 2018-06-01  3:25 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: arm-soc, Rob Herring, Mark Rutland, Vincent Guittot,
	ionela.voinescu, Daniel Lezcano, chris.redpath, DTML,
	linux-arm-kernel, Linux Kernel Mailing List
In-Reply-To: <51ac4a5f0466aeed3f223663d9c34d6345b2a1f2.1527244201.git.viresh.kumar@linaro.org>

2018-05-25 19:31 GMT+09:00 Viresh Kumar <viresh.kumar@linaro.org>:
> The cooling device properties, like "#cooling-cells" and
> "dynamic-power-coefficient", should either be present for all the CPUs
> of a cluster or none. If these are present only for a subset of CPUs of
> a cluster then things will start falling apart as soon as the CPUs are
> brought online in a different order. For example, this will happen
> because the operating system looks for such properties in the CPU node
> it is trying to bring up, so that it can register a cooling device.
>
> Add such missing properties.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>


Applied to linux-uniphier.

I had already sent a PR for v4.18-rc1 before I received this patch.
Please wait for v4.19-rc1.

Thanks.


> ---
>  arch/arm/boot/dts/uniphier-pxs2.dtsi | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/arch/arm/boot/dts/uniphier-pxs2.dtsi b/arch/arm/boot/dts/uniphier-pxs2.dtsi
> index debcbd15c24b..40ed15465095 100644
> --- a/arch/arm/boot/dts/uniphier-pxs2.dtsi
> +++ b/arch/arm/boot/dts/uniphier-pxs2.dtsi
> @@ -36,6 +36,7 @@
>                         enable-method = "psci";
>                         next-level-cache = <&l2>;
>                         operating-points-v2 = <&cpu_opp>;
> +                       #cooling-cells = <2>;
>                 };
>
>                 cpu2: cpu@2 {
> @@ -46,6 +47,7 @@
>                         enable-method = "psci";
>                         next-level-cache = <&l2>;
>                         operating-points-v2 = <&cpu_opp>;
> +                       #cooling-cells = <2>;
>                 };
>
>                 cpu3: cpu@3 {
> @@ -56,6 +58,7 @@
>                         enable-method = "psci";
>                         next-level-cache = <&l2>;
>                         operating-points-v2 = <&cpu_opp>;
> +                       #cooling-cells = <2>;
>                 };
>         };
>
> --
> 2.15.0.194.g9af6a3dea062
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply

* Re: [PATCH v2 0/2] slimbus: Add QCOM SLIMBus NGD driver
From: Vinod @ 2018-06-01  5:14 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: gregkh, robh+dt, kramasub, sdharia, girishm, linux-kernel,
	mark.rutland, bgoswami, devicetree, broonie, linux-arm-msm,
	alsa-devel
In-Reply-To: <20180525111547.18556-1-srinivas.kandagatla@linaro.org>

On 25-05-18, 12:15, Srinivas Kandagatla wrote:
> This patchset adds support to basic version of Qualcomm NGD SLIMBus
> controller driver found SoCs from B family.
> 
> This controller is light-weight SLIMBus controller driver responsible for
> communicating with slave HW directly over the bus using messaging
> interface, and communicating with master component residing on ADSP
> for bandwidth and data-channel management.
> 
> Tested this patchset on DB820c with WCD9335 codec.
> I have pushed my working branch to [1] incase someone want to try.
> 
> This patch has dependency on https://lkml.org/lkml/2018/5/17/251

Looks good to me know, FWIW:

Reviewed-by: Vinod Koul <vkoul@kernel.org>

-- 
~Vinod

^ permalink raw reply

* Re: [PATCH v4 2/6] mfd: bd71837: Devicetree bindings for ROHM BD71837 PMIC
From: Matti Vaittinen @ 2018-06-01  6:25 UTC (permalink / raw)
  To: Rob Herring
  Cc: Matti Vaittinen, Matti Vaittinen, Michael Turquette, Stephen Boyd,
	Mark Rutland, Lee Jones, Liam Girdwood, Mark Brown, linux-clk,
	devicetree, linux-kernel@vger.kernel.org, mikko.mutanen,
	heikki.haikola
In-Reply-To: <CAL_Jsq+33G7zMEDOUXHQijxqqbvqykxZENLe1-52ECkXwo2o4g@mail.gmail.com>

On Thu, May 31, 2018 at 09:07:24AM -0500, Rob Herring wrote:
> On Thu, May 31, 2018 at 5:23 AM, Matti Vaittinen
> <mazziesaccount@gmail.com> wrote:
> > On Thu, May 31, 2018 at 10:17:17AM +0300, Matti Vaittinen wrote:
> >> On Wed, May 30, 2018 at 10:01:29PM -0500, Rob Herring wrote:
> >> > On Wed, May 30, 2018 at 11:42:03AM +0300, Matti Vaittinen wrote:
> >> > > Document devicetree bindings for ROHM BD71837 PMIC MFD.
> >> > > + - interrupts            : The interrupt line the device is connected to.
> >> > > + - interrupt-controller  : Marks the device node as an interrupt controller.
> >> >
> >> > What sub blocks have interrupts?
> >>
> >> The PMIC can generate interrupts from events which cause it to reset.
> >> Eg, irq from watchdog line change, power button pushes, reset request
> >> via register interface etc. I don't know any generic handling for these
> >> interrupts. In "normal" use-case this PMIC is powering the processor
> >> where driver is running and I do not see reasonable handling because
> >> power-reset is going to follow the irq.
> >>
> >
> > Oh, but when reading this I understand that the interrupt-controller
> > property should at least be optional.
> 
> I don't think it should. The h/w either has an interrupt controller or
> it doesn't.

I hope this explains why I did this interrupt controller - please tell
me if this is legitimate use-case and what you think of following:

+Optional properties:
+ - interrupt-controller	: Marks the device node as an interrupt controller.
+			  BD71837MWV can report different power state change
+			  events to other devices. Different events can be seen
+			  as separate BD71837 domain interrupts.
+ - #interrupt-cells	: The number of cells to describe an IRQ should be 1.
+			    The first cell is the IRQ number.
+			    masks from ../interrupt-controller/interrupts.txt.

> My concern is you added it but nothing uses it which tells
> me your binding is incomplete. I'd rather see complete bindings even
> if you don't have drivers.

So this makes me wonder if my use-case for interrupt controller is
valid. I thought making this PMIC as interrupt controller is a nice way
of hiding the irq register and i2c access from other potential drivers
using these interrupts. But as I don't know what could be the potential
user for these irqs, I don't know how to complete binding. This is why I
also thought of making this optional, so that the potential for using
the interrupts would be there but it was not required when interrupts
are not needed.

> For example, as-is, there's not really any
> need for the clocks child node. You can just make the parent a clock
> provider.

This sounds correct. I just lack of knowledge on how to handle clocks
in "standard way" using the clock framework and this was a result of
my first attempt. (Funny, I have written clk / synchronization drivers
for work in the past but still I have no idea on how to do this in
"standard way").

> But we need a complete picture of the h/w to make that
> determination.

My attempt is to create generic driver for this PMIC. I would rather not
limit it's use to any particular board/soc. The example binding is based
on my test environment where I simply connected this PMIC break out
board to beagle bone black. (I do also have a test board where i.MX8 and
peripherials are powered by this PMIC but I rather not limit this driver
to such single setup. Besides the linux running on that board is not
'standard')

The PMIC itself just has this 32.768 KHz clock output. Clock can be
enabled/disabled via register interface. I think this is intended to be
used for RTC but I thought this driver does not need to care about that.
I thought it is just a good idea to provide control via clk subsystem
and to not make assumptions on use-cases in this driver.

Best Regards
    Matti Vaittinen

^ permalink raw reply

* [PATCH 0/3] arm64: allwinner: a64: Add initial support for Pinebook
From: Vasily Khoruzhick @ 2018-06-01  6:28 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai, Rob Herring, Mark Rutland,
	Catalin Marinas, Will Deacon, linux-arm-kernel, devicetree
  Cc: Vasily Khoruzhick

This series adds dts for Pinebook with few prerequisites - PWM and R_I2C
devices nodes.

Andre Przywara (1):
  dts: sunxi: A64: Add PWM controllers

Icenowy Zheng (2):
  arm64: allwinner: a64: add R_I2C controller
  arm64: dts: allwinner: add support for Pinebook

 arch/arm64/boot/dts/allwinner/Makefile        |   1 +
 .../dts/allwinner/sun50i-a64-pinebook.dts     | 285 ++++++++++++++++++
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  45 +++
 3 files changed, 331 insertions(+)
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts

-- 
2.17.1

^ permalink raw reply

* [PATCH 1/3] arm64: allwinner: a64: add R_I2C controller
From: Vasily Khoruzhick @ 2018-06-01  6:28 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai, Rob Herring, Mark Rutland,
	Catalin Marinas, Will Deacon, linux-arm-kernel, devicetree
  Cc: Icenowy Zheng
In-Reply-To: <20180601062901.8052-1-anarsoul@gmail.com>

From: Icenowy Zheng <icenowy@aosc.io>

Allwinner A64 has a I2C controller, which is in the R_ MMIO zone and has
two groups of pinmuxes on PL bank, so it's called R_I2C.

Add support for this I2C controller and the pinmux which doesn't conflict
with RSB.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 1b2ef28c42bd..b5e903ccf0ec 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -46,6 +46,7 @@
 #include <dt-bindings/clock/sun8i-r-ccu.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/reset/sun50i-a64-ccu.h>
+#include <dt-bindings/reset/sun8i-r-ccu.h>
 
 / {
 	interrupt-parent = <&gic>;
@@ -655,6 +656,17 @@
 			#reset-cells = <1>;
 		};
 
+		r_i2c: i2c@1f02400 {
+			compatible = "allwinner,sun6i-a31-i2c";
+			reg = <0x01f02400 0x400>;
+			interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&r_ccu CLK_APB0_I2C>;
+			resets = <&r_ccu RST_APB0_I2C>;
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
 		r_pio: pinctrl@1f02c00 {
 			compatible = "allwinner,sun50i-a64-r-pinctrl";
 			reg = <0x01f02c00 0x400>;
@@ -670,6 +682,11 @@
 				pins = "PL0", "PL1";
 				function = "s_rsb";
 			};
+
+			r_i2c_pins_a: i2c-a {
+				pins = "PL8", "PL9";
+				function = "s_i2c";
+			};
 		};
 
 		r_rsb: rsb@1f03400 {
-- 
2.17.1

^ permalink raw reply related

* [PATCH 2/3] dts: sunxi: A64: Add PWM controllers
From: Vasily Khoruzhick @ 2018-06-01  6:29 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai, Rob Herring, Mark Rutland,
	Catalin Marinas, Will Deacon, linux-arm-kernel, devicetree
  Cc: Andre Przywara
In-Reply-To: <20180601062901.8052-1-anarsoul@gmail.com>

From: Andre Przywara <andre.przywara@arm.com>

The Allwinner A64 SoC features two PWM controllers, which are fully
compatible to the one used in the A13 and H3 chips.

Add the nodes for the devices (one for the "normal" PWM, the other for
the one in the CPUS domain) and the pins their outputs are connected to.

On the A64 the "normal" PWM is muxed together with one of the MDIO pins
used to communicate with the Ethernet PHY, so it won't be usable on many
boards. But the Pinebook laptop uses this pin for controlling the LCD
backlight.

On Pine64 the CPUS PWM pin however is routed to the "RPi2" header,
at the same location as the PWM pin on the RaspberryPi.

[vasily: fixed comment message as requested by Stefan Bruens]

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Tested-by: Vasily Khoruzhick <anarsoul@gmail.com> on Pinebook (only the "normal" PWM)
Tested-by: Harald Geyer <harald@ccbib.org> on Teres-I (only the "normal" PWM)
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index b5e903ccf0ec..e94bfa8477f6 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -365,6 +365,11 @@
 				bias-pull-up;
 			};
 
+			pwm_pin: pwm_pin {
+				pins = "PD22";
+				function = "pwm";
+			};
+
 			rmii_pins: rmii_pins {
 				pins = "PD10", "PD11", "PD13", "PD14", "PD17",
 				       "PD18", "PD19", "PD20", "PD22", "PD23";
@@ -630,6 +635,15 @@
 			#interrupt-cells = <3>;
 		};
 
+		pwm: pwm@1c21400 {
+			compatible = "allwinner,sun50i-a64-pwm",
+				     "allwinner,sun5i-a13-pwm";
+			reg = <0x01c21400 0x400>;
+			clocks = <&osc24M>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
 		rtc: rtc@1f00000 {
 			compatible = "allwinner,sun6i-a31-rtc";
 			reg = <0x01f00000 0x54>;
@@ -667,6 +681,15 @@
 			#size-cells = <0>;
 		};
 
+		r_pwm: pwm@1f03800 {
+			compatible = "allwinner,sun50i-a64-pwm",
+				     "allwinner,sun5i-a13-pwm";
+			reg = <0x01f03800 0x400>;
+			clocks = <&osc24M>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
 		r_pio: pinctrl@1f02c00 {
 			compatible = "allwinner,sun50i-a64-r-pinctrl";
 			reg = <0x01f02c00 0x400>;
@@ -687,6 +710,11 @@
 				pins = "PL8", "PL9";
 				function = "s_i2c";
 			};
+
+			r_pwm_pin: pwm {
+				pins = "PL10";
+				function = "s_pwm";
+			};
 		};
 
 		r_rsb: rsb@1f03400 {
-- 
2.17.1

^ permalink raw reply related

* [PATCH 3/3] arm64: dts: allwinner: add support for Pinebook
From: Vasily Khoruzhick @ 2018-06-01  6:29 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai, Rob Herring, Mark Rutland,
	Catalin Marinas, Will Deacon, linux-arm-kernel, devicetree
  Cc: Vasily Khoruzhick, Icenowy Zheng
In-Reply-To: <20180601062901.8052-1-anarsoul@gmail.com>

From: Icenowy Zheng <icenowy@aosc.xyz>

Pinebook is a A64-based laptop produced by Pine64, with the following
peripherals:

USB:
- Two external USB ports (one is directly connected to A64's OTG
controller, the other is under a internal hub connected to the host-only
controller.)
- USB HID keyboard and touchpad connected to the internal hub.
- USB UVC camera connected to the internal hub.

Power-related:
- A DC IN jack connected to AXP803's DCIN pin.
- A Li-Polymer battery connected to AXP803's battery pins.

Storage:
- An eMMC by Foresee on the main board (in the product revision of the
main board it's designed to be switchable).
- An external MicroSD card slot.

Display:
- An eDP LCD panel (1366x768) connected via an ANX6345 RGB-eDP bridge.
- A mini HDMI port.

Misc:
- A Hall sensor designed to detect the status of lid, connected to GPIO PL12.
- A headphone jack connected to the SoC's internal codec.
- A debug UART port muxed with headphone jack.

This commit adds basical support for it.

[vasily: squashed several commits into one, added simplefb node, added usbphy
	 to ehci0 and ohci0 nodes]

Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 arch/arm64/boot/dts/allwinner/Makefile        |   1 +
 .../dts/allwinner/sun50i-a64-pinebook.dts     | 285 ++++++++++++++++++
 2 files changed, 286 insertions(+)
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts

diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile
index 8bebe7da5ed9..a8c6d0c6f2c5 100644
--- a/arch/arm64/boot/dts/allwinner/Makefile
+++ b/arch/arm64/boot/dts/allwinner/Makefile
@@ -4,6 +4,7 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-nanopi-a64.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-olinuxino.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-orangepi-win.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-pine64-plus.dtb sun50i-a64-pine64.dtb
+dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-pinebook.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-sopine-baseboard.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-teres-i.dtb
 dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-pc2.dtb
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
new file mode 100644
index 000000000000..d952db217702
--- /dev/null
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
@@ -0,0 +1,285 @@
+/*
+ * Copyright (C) 2017 Icenowy Zheng <icenowy@aosc.xyz>
+ * Copyright (C) 2018 Vasily Khoruzhick <anarsoul@gmail.com>
+ *
+ * SPDX-License-Identifier: (GPL-2.0 OR MIT)
+ */
+
+/dts-v1/;
+
+#include "sun50i-a64.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/pwm/pwm.h>
+
+/ {
+	model = "Pinebook";
+	compatible = "pine64,pinebook", "allwinner,sun50i-a64";
+
+	aliases {
+		serial0 = &uart0;
+		ethernet0 = &rtl8723cs;
+	};
+
+	backlight: backlight {
+		compatible = "pwm-backlight";
+		pwms = <&pwm 0 50000 0>;
+		brightness-levels = <0 10 20 30 40 50 60 70 80 90 100>;
+		default-brightness-level = <2>;
+		enable-gpios = <&pio 3 23 GPIO_ACTIVE_HIGH>; /* PD23 */
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+
+		framebuffer-lcd {
+			panel-supply = <&reg_dc1sw>;
+			dvdd25-supply = <&reg_dldo2>;
+			dvdd12-supply = <&reg_fldo1>;
+		};
+	};
+
+	gpio_keys {
+		compatible = "gpio-keys";
+
+		lid_switch {
+			label = "Lid Switch";
+			gpios = <&r_pio 0 12 GPIO_ACTIVE_LOW>; /* PL12 */
+			linux,input-type = <EV_SW>;
+			linux,code = <SW_LID>;
+			linux,can-disable;
+		};
+	};
+
+	reg_vcc3v3: vcc3v3 {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc3v3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+	};
+
+	wifi_pwrseq: wifi_pwrseq {
+		compatible = "mmc-pwrseq-simple";
+		reset-gpios = <&r_pio 0 2 GPIO_ACTIVE_LOW>; /* PL2 */
+	};
+};
+
+&ehci0 {
+	phys = <&usbphy 0>;
+	phy-names = "usb";
+	status = "okay";
+};
+
+&ehci1 {
+	status = "okay";
+};
+
+&mmc0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&mmc0_pins>;
+	vmmc-supply = <&reg_dcdc1>;
+	cd-gpios = <&pio 5 6 GPIO_ACTIVE_HIGH>;
+	cd-inverted;
+	disable-wp;
+	bus-width = <4>;
+	status = "okay";
+};
+
+&mmc1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&mmc1_pins>;
+	vmmc-supply = <&reg_dldo4>;
+	vqmmc-supply = <&reg_eldo1>;
+	mmc-pwrseq = <&wifi_pwrseq>;
+	bus-width = <4>;
+	non-removable;
+	status = "okay";
+
+	rtl8723cs: wifi@1 {
+		reg = <1>;
+	};
+};
+
+&mmc2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&mmc2_pins>;
+	vmmc-supply = <&reg_dcdc1>;
+	vqmmc-supply = <&reg_eldo1>;
+	bus-width = <8>;
+	non-removable;
+	cap-mmc-hw-reset;
+	mmc-hs200-1_8v;
+	status = "okay";
+};
+
+&ohci0 {
+	phys = <&usbphy 0>;
+	phy-names = "usb";
+	status = "okay";
+};
+
+&ohci1 {
+	status = "okay";
+};
+
+&pwm {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pwm_pin>;
+	status = "okay";
+};
+
+&r_rsb {
+	status = "okay";
+
+	axp803: pmic@3a3 {
+		compatible = "x-powers,axp803";
+		reg = <0x3a3>;
+		interrupt-parent = <&r_intc>;
+		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+	};
+};
+
+/* The ANX6345 eDP-bridge is on r_i2c. There is no linux (mainline)
+ * driver for this chip at the moment, the bootloader initializes it.
+ * However it can be accessed with the i2c-dev driver from user space.
+ */
+&r_i2c {
+	clock-frequency = <100000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&r_i2c_pins_a>;
+	status = "okay";
+};
+
+#include "axp803.dtsi"
+
+&reg_aldo1 {
+	regulator-min-microvolt = <2800000>;
+	regulator-max-microvolt = <2800000>;
+	regulator-name = "vcc-csi";
+};
+
+&reg_aldo2 {
+	regulator-always-on;
+	regulator-min-microvolt = <1800000>;
+	regulator-max-microvolt = <3300000>;
+	regulator-name = "vcc-pl";
+};
+
+&reg_aldo3 {
+	regulator-always-on;
+	regulator-min-microvolt = <2700000>;
+	regulator-max-microvolt = <3300000>;
+	regulator-name = "vcc-pll-avcc";
+};
+
+&reg_dc1sw {
+	regulator-name = "vcc-lcd";
+};
+
+&reg_dcdc1 {
+	regulator-always-on;
+	regulator-min-microvolt = <3300000>;
+	regulator-max-microvolt = <3300000>;
+	regulator-name = "vcc-3v3";
+};
+
+&reg_dcdc2 {
+	regulator-always-on;
+	regulator-min-microvolt = <1000000>;
+	regulator-max-microvolt = <1300000>;
+	regulator-name = "vdd-cpux";
+};
+
+/* DCDC3 is polyphased with DCDC2 */
+
+&reg_dcdc5 {
+	regulator-always-on;
+	regulator-min-microvolt = <1200000>;
+	regulator-max-microvolt = <1200000>;
+	regulator-name = "vcc-dram";
+};
+
+&reg_dcdc6 {
+	regulator-always-on;
+	regulator-min-microvolt = <1100000>;
+	regulator-max-microvolt = <1100000>;
+	regulator-name = "vdd-sys";
+};
+
+&reg_dldo1 {
+	regulator-min-microvolt = <3300000>;
+	regulator-max-microvolt = <3300000>;
+	regulator-name = "vcc-hdmi";
+};
+
+&reg_dldo2 {
+	regulator-min-microvolt = <2500000>;
+	regulator-max-microvolt = <2500000>;
+	regulator-name = "vcc-edp";
+};
+
+&reg_dldo3 {
+	regulator-min-microvolt = <3300000>;
+	regulator-max-microvolt = <3300000>;
+	regulator-name = "avdd-csi";
+};
+
+&reg_dldo4 {
+	regulator-min-microvolt = <3300000>;
+	regulator-max-microvolt = <3300000>;
+	regulator-name = "vcc-wifi";
+};
+
+&reg_eldo1 {
+	regulator-always-on;
+	regulator-min-microvolt = <1800000>;
+	regulator-max-microvolt = <1800000>;
+	regulator-name = "cpvdd";
+};
+
+&reg_eldo3 {
+	regulator-min-microvolt = <1800000>;
+	regulator-max-microvolt = <1800000>;
+	regulator-name = "vdd-1v8-csi";
+};
+
+&reg_fldo1 {
+	regulator-min-microvolt = <1200000>;
+	regulator-max-microvolt = <1200000>;
+	regulator-name = "vcc-1v2-hsic";
+};
+
+&reg_fldo2 {
+	regulator-always-on;
+	regulator-min-microvolt = <1100000>;
+	regulator-max-microvolt = <1100000>;
+	regulator-name = "vdd-cpus";
+};
+
+&reg_ldo_io0 {
+	regulator-min-microvolt = <3300000>;
+	regulator-max-microvolt = <3300000>;
+	regulator-name = "vcc-usb";
+	status = "okay";
+};
+
+&reg_rtc_ldo {
+	regulator-name = "vcc-rtc";
+};
+
+&uart0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart0_pins_a>;
+	status = "okay";
+};
+
+&usb_otg {
+	dr_mode = "host";
+};
+
+&usbphy {
+	usb0_vbus-supply = <&reg_ldo_io0>;
+	usb1_vbus-supply = <&reg_ldo_io0>;
+	status = "okay";
+};
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH] ARM:drm ivip Intel FPGA Video and Image Processing Suite
From: kbuild test robot @ 2018-06-01  6:42 UTC (permalink / raw)
  Cc: devicetree, yves.vandervennet, hean.loong.ong, chin.liang.see,
	linux-kernel, dri-devel, Dinh Nguyen, Rob Herring,
	Laurent Pinchart, Daniel Vetter, linux-arm-kernel, kbuild-all
In-Reply-To: <1527745851-3339-4-git-send-email-hean.loong.ong@intel.com>

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

Hi Ong,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm/drm-next]
[also build test WARNING on v4.17-rc7 next-20180531]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Hean-Loong-Ong/ARM-drm-ivip-Intel-FPGA-Video-and-Image-Processing-Suite/20180601-132429
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sparc64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/cdev.h:8:0,
                    from include/drm/drmP.h:36,
                    from drivers/gpu//drm/ivip/intel_vip_core.c:24:
   drivers/gpu//drm/ivip/intel_vip_core.c: In function 'intelvipfb_enable':
>> drivers/gpu//drm/ivip/intel_vip_core.c:58:33: warning: format '%x' expects argument of type 'unsigned int', but argument 3 has type 'dma_addr_t {aka long long unsigned int}' [-Wformat=]
     dev_info(pipe->plane.dev->dev, "Address 0x%x\n", addr);
                                    ^
   include/linux/device.h:1382:51: note: in definition of macro 'dev_info'
    #define dev_info(dev, fmt, arg...) _dev_info(dev, fmt, ##arg)
                                                      ^~~

vim +58 drivers/gpu//drm/ivip/intel_vip_core.c

  > 24	#include <drm/drmP.h>
    25	#include <drm/drm_atomic.h>
    26	#include <drm/drm_atomic_helper.h>
    27	#include <drm/drm_crtc_helper.h>
    28	#include <drm/drm_fb_helper.h>
    29	#include <drm/drm_fb_cma_helper.h>
    30	#include <drm/drm_gem_cma_helper.h>
    31	#include <drm/drm_plane_helper.h>
    32	#include <drm/drm_simple_kms_helper.h>
    33	#include <drm/drm_gem_framebuffer_helper.h>
    34	
    35	#include <linux/init.h>
    36	#include <linux/kernel.h>
    37	#include <linux/module.h>
    38	
    39	#include "intel_vip_drv.h"
    40	
    41	static void intelvipfb_enable(struct drm_simple_display_pipe *pipe,
    42		       struct drm_crtc_state *crtc_state)
    43	{
    44		/*
    45		 * The frameinfo variable has to correspond to the size of the VIP Suite
    46		 * Frame Reader register 7 which will determine the maximum size used
    47		 * in this frameinfo
    48		 */
    49	
    50		u32 frameinfo;
    51		struct intelvipfb_priv *priv = pipe->plane.dev->dev_private;
    52		void __iomem *base = priv->base;
    53		struct drm_plane_state *state = pipe->plane.state;
    54		dma_addr_t addr;
    55	
    56		addr = drm_fb_cma_get_gem_addr(state->fb, state, 0);
    57	
  > 58		dev_info(pipe->plane.dev->dev, "Address 0x%x\n", addr);
    59	
    60		frameinfo =
    61			readl(base + INTELVIPFB_FRAME_READER) & 0x00ffffff;
    62		writel(frameinfo, base + INTELVIPFB_FRAME_INFO);
    63		writel(addr, base + INTELVIPFB_FRAME_START);
    64		/* Finally set the control register to 1 to start streaming */
    65		writel(1, base + INTELVIPFB_CONTROL);
    66	}
    67	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 53269 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply

* Re: [PATCH v2 3/6] clk: ti: dra7: Add clkctrl clock data for the mcan clocks
From: Faiz Abbas @ 2018-06-01  6:48 UTC (permalink / raw)
  To: Tero Kristo, Rob Herring
  Cc: devicetree, paul, tony, linux-kernel, bcousson, linux-omap,
	linux-clk, linux-arm-kernel
In-Reply-To: <678ef467-a235-91db-8bae-f249ad98eac8@ti.com>

Hi,

On Thursday 31 May 2018 06:59 PM, Tero Kristo wrote:
> On 31/05/18 13:14, Faiz Abbas wrote:
>> Hi,
>>
>> On Thursday 31 May 2018 09:33 AM, Rob Herring wrote:
>>> On Wed, May 30, 2018 at 07:41:30PM +0530, Faiz Abbas wrote:
>>>> Add clkctrl data for the m_can clocks and register it within the
>> ...
>>>>   diff --git a/include/dt-bindings/clock/dra7.h
>>>> b/include/dt-bindings/clock/dra7.h
>>>> index 5e1061b15aed..d7549c57cac3 100644
>>>> --- a/include/dt-bindings/clock/dra7.h
>>>> +++ b/include/dt-bindings/clock/dra7.h
>>>> @@ -168,5 +168,6 @@
>>>>   #define DRA7_COUNTER_32K_CLKCTRL    DRA7_CLKCTRL_INDEX(0x50)
>>>>   #define DRA7_UART10_CLKCTRL    DRA7_CLKCTRL_INDEX(0x80)
>>>>   #define DRA7_DCAN1_CLKCTRL    DRA7_CLKCTRL_INDEX(0x88)
>>>> +#define DRA7_ADC_CLKCTRL    DRA7_CLKCTRL_INDEX(0xa0)
>>>
>>> ADC and mcan are the same thing?
>>>
>>
>> The register to control MCAN clocks is called ADC_CLKCTRL, Yes.
> 
> Is there any reason for this or is that just a documentation bug?
> 

Looks like they meant to have an ADC in dra74 or dra72 but decided
against it and then many years later used the same registers for MCAN
instead. You can see ADC_CLKCTRL exists in dra72 TRM but is explicitly
disabled.

http://www.ti.com/lit/ug/spruic2b/spruic2b.pdf pg:1524


Thanks,
Faiz

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v1 0/4] add mailbox support for i.MX7D
From: Oleksij Rempel @ 2018-06-01  6:58 UTC (permalink / raw)
  To: Shawn Guo, Fabio Estevam, Rob Herring, Mark Rutland
  Cc: Oleksij Rempel, linux-clk, linux-arm-kernel, kernel, devicetree

This patches are providing support for mailbox (Messaging Unit)
for i.MX7D.
Functionality was tested on PHYTEC phyBOARD-Zeta i.MX7D with
Linux running on all cores: ARM Cortex-A7 and ARM Cortex-M4.

Both parts of i.MX messaging Unit are visible for all CPUs available
on i.MX7D. Communication worked independent of MU side in combination
with CPU. For example MU-A used on ARM Cortex-A7 and MU-B used on ARM Cortex-M4
or other ways around.

The question to NXP developers: are there are limitations or
recommendations about MU vs CPU combination? The i.MX7D documentation
talks about "Processor A" and "Processor B". It is not quite clear what
processor it actually is (A7 or M4).

Oleksij Rempel (4):
  clk: imx7d: add IMX7D_MU_ROOT_CLK
  dt-bindings: mailbox: provide imx-mailbox documentation
  ARM: dts: imx7s: add i.MX7 messaging unit support
  mailbox: Add support for i.MX7D messaging unit

 .../bindings/mailbox/imx-mailbox.txt          |  35 +++
 arch/arm/boot/dts/imx7s.dtsi                  |  18 ++
 drivers/clk/imx/clk-imx7d.c                   |   1 +
 drivers/mailbox/Kconfig                       |   6 +
 drivers/mailbox/Makefile                      |   2 +
 drivers/mailbox/imx-mailbox.c                 | 289 ++++++++++++++++++
 6 files changed, 351 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/imx-mailbox.txt
 create mode 100644 drivers/mailbox/imx-mailbox.c

-- 
2.17.1

^ permalink raw reply

* [PATCH v1 1/4] clk: imx7d: add IMX7D_MU_ROOT_CLK
From: Oleksij Rempel @ 2018-06-01  6:58 UTC (permalink / raw)
  To: Shawn Guo, Fabio Estevam, Rob Herring, Mark Rutland
  Cc: Oleksij Rempel, linux-clk, linux-arm-kernel, kernel, devicetree
In-Reply-To: <20180601065821.28234-1-o.rempel@pengutronix.de>

This clock is needed for iMX mailbox driver

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/clk/imx/clk-imx7d.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/imx/clk-imx7d.c b/drivers/clk/imx/clk-imx7d.c
index 975a20d3cc94..1c2541dc40e7 100644
--- a/drivers/clk/imx/clk-imx7d.c
+++ b/drivers/clk/imx/clk-imx7d.c
@@ -793,6 +793,7 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node)
 	clks[IMX7D_DRAM_PHYM_ROOT_CLK] = imx_clk_gate4("dram_phym_root_clk", "dram_phym_cg", base + 0x4130, 0);
 	clks[IMX7D_DRAM_PHYM_ALT_ROOT_CLK] = imx_clk_gate4("dram_phym_alt_root_clk", "dram_phym_alt_post_div", base + 0x4130, 0);
 	clks[IMX7D_DRAM_ALT_ROOT_CLK] = imx_clk_gate4("dram_alt_root_clk", "dram_alt_post_div", base + 0x4130, 0);
+	clks[IMX7D_MU_ROOT_CLK] = imx_clk_gate2("mu_root_clk", "ipg_root_clk", base + 0x4270, 0);
 	clks[IMX7D_OCOTP_CLK] = imx_clk_gate4("ocotp_clk", "ipg_root_clk", base + 0x4230, 0);
 	clks[IMX7D_SNVS_CLK] = imx_clk_gate4("snvs_clk", "ipg_root_clk", base + 0x4250, 0);
 	clks[IMX7D_CAAM_CLK] = imx_clk_gate4("caam_clk", "ipg_root_clk", base + 0x4240, 0);
-- 
2.17.1

^ permalink raw reply related

* [PATCH v1 2/4] dt-bindings: mailbox: provide imx-mailbox documentation
From: Oleksij Rempel @ 2018-06-01  6:58 UTC (permalink / raw)
  To: Shawn Guo, Fabio Estevam, Rob Herring, Mark Rutland
  Cc: Oleksij Rempel, linux-clk, linux-arm-kernel, kernel, devicetree
In-Reply-To: <20180601065821.28234-1-o.rempel@pengutronix.de>

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 .../bindings/mailbox/imx-mailbox.txt          | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/imx-mailbox.txt

diff --git a/Documentation/devicetree/bindings/mailbox/imx-mailbox.txt b/Documentation/devicetree/bindings/mailbox/imx-mailbox.txt
new file mode 100644
index 000000000000..a45604b33039
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/imx-mailbox.txt
@@ -0,0 +1,35 @@
+i.MX Messaging Unit
+===================
+
+The i.MX Messaging Unit (MU) contains two register sets: "A" and "B". In most cases
+they are accessible from all Processor Units. On one hand, at least for mailbox functionality,
+it makes no difference which application or processor is using which set of the MU. On
+other hand, the register sets for each of the MU parts are not identical.
+
+Required properties:
+- compatible :	Shell be one of:
+                    "fsl,imx7s-mu-a" and "fsl,imx7s-mu-b" for i.MX7S or i.MX7D
+- reg : 	physical base address of the mailbox and length of
+		memory mapped region.
+- #mbox-cells:	Common mailbox binding property to identify the number
+		of cells required for the mailbox specifier. Should be 1.
+- interrupts :	interrupt number. The interrupt specifier format
+		depends on the interrupt controller parent.
+- clocks     :  phandle to the input clock.
+
+Example:
+	mu0a: mu@30aa0000 {
+		compatible = "fsl,imx7s-mu-a";
+		reg = <0x30aa0000 0x28>;
+		interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&clks IMX7D_MU_ROOT_CLK>;
+		#mbox-cells = <1>;
+	};
+
+	mu0b: mu@30ab0000 {
+		compatible = "fsl,imx7s-mu-b";
+		reg = <0x30ab0000 0x28>;
+		interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&clks IMX7D_MU_ROOT_CLK>;
+		#mbox-cells = <1>;
+	};
-- 
2.17.1

^ permalink raw reply related

* [PATCH v1 3/4] ARM: dts: imx7s: add i.MX7 messaging unit support
From: Oleksij Rempel @ 2018-06-01  6:58 UTC (permalink / raw)
  To: Shawn Guo, Fabio Estevam, Rob Herring, Mark Rutland
  Cc: Oleksij Rempel, linux-clk, linux-arm-kernel, kernel, devicetree
In-Reply-To: <20180601065821.28234-1-o.rempel@pengutronix.de>

Define the Messaging Unit (MU) for i.MX7 in the processor's dtsi.
The respective driver is added in the next commit.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 arch/arm/boot/dts/imx7s.dtsi | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index d9437e773b37..299bed72f69a 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -1008,6 +1008,24 @@
 				status = "disabled";
 			};
 
+			mu0a: mu@30aa0000 {
+				compatible = "fsl,imx7s-mu-a";
+				reg = <0x30aa0000 0x10000>;
+				interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clks IMX7D_MU_ROOT_CLK>;
+				#mbox-cells = <1>;
+				status = "disabled";
+			};
+
+			mu0b: mu@30ab0000 {
+				compatible = "fsl,imx7s-mu-b";
+				reg = <0x30ab0000 0x10000>;
+				interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clks IMX7D_MU_ROOT_CLK>;
+				#mbox-cells = <1>;
+				status = "disabled";
+			};
+
 			usbotg1: usb@30b10000 {
 				compatible = "fsl,imx7d-usb", "fsl,imx27-usb";
 				reg = <0x30b10000 0x200>;
-- 
2.17.1

^ permalink raw reply related

* [PATCH v1 4/4] mailbox: Add support for i.MX7D messaging unit
From: Oleksij Rempel @ 2018-06-01  6:58 UTC (permalink / raw)
  To: Shawn Guo, Fabio Estevam, Rob Herring, Mark Rutland
  Cc: Oleksij Rempel, linux-clk, linux-arm-kernel, kernel, devicetree
In-Reply-To: <20180601065821.28234-1-o.rempel@pengutronix.de>

The Mailbox controller is able to send messages (up to 4 32 bit words)
between the endpoints.

This driver was tested using the mailbox-test driver sending messages
between the Cortex-A7 and the Cortex-M4.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/mailbox/Kconfig       |   6 +
 drivers/mailbox/Makefile      |   2 +
 drivers/mailbox/imx-mailbox.c | 289 ++++++++++++++++++++++++++++++++++
 3 files changed, 297 insertions(+)
 create mode 100644 drivers/mailbox/imx-mailbox.c

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index a2bb27446dce..e1d2738a2e4c 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -15,6 +15,12 @@ config ARM_MHU
 	  The controller has 3 mailbox channels, the last of which can be
 	  used in Secure mode only.
 
+config IMX_MBOX
+	tristate "iMX Mailbox"
+	depends on SOC_IMX7D || COMPILE_TEST
+	help
+	  Mailbox implementation for iMX7D Messaging Unit (MU).
+
 config PLATFORM_MHU
 	tristate "Platform MHU Mailbox"
 	depends on OF
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index cc23c3a43fcd..ba2fe1b6dd62 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -7,6 +7,8 @@ obj-$(CONFIG_MAILBOX_TEST)	+= mailbox-test.o
 
 obj-$(CONFIG_ARM_MHU)	+= arm_mhu.o
 
+obj-$(CONFIG_IMX_MBOX)	+= imx-mailbox.o
+
 obj-$(CONFIG_PLATFORM_MHU)	+= platform_mhu.o
 
 obj-$(CONFIG_PL320_MBOX)	+= pl320-ipc.o
diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
new file mode 100644
index 000000000000..2bc9f11393b1
--- /dev/null
+++ b/drivers/mailbox/imx-mailbox.c
@@ -0,0 +1,289 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 Pengutronix, Oleksij Rempel <o.rempel@pengutronix.de>
+ */
+
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mailbox_controller.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+
+/* Transmit Register */
+#define IMX_MU_xTRn(x)		(0x00 + 4 * (x))
+/* Receive Register */
+#define IMX_MU_xRRn(x)		(0x10 + 4 * (x))
+/* Status Register */
+#define IMX_MU_xSR		0x20
+#define IMX_MU_xSR_TEn(x)	BIT(20 + (x))
+#define IMX_MU_xSR_RFn(x)	BIT(24 + (x))
+#define IMX_MU_xSR_BRDIP	BIT(9)
+
+/* Control Register */
+#define IMX_MU_xCR		0x24
+/* Transmit Interrupt Enable */
+#define IMX_MU_xCR_TIEn(x)	BIT(20 + (x))
+/* Receive Interrupt Enable */
+#define IMX_MU_xCR_RIEn(x)	BIT(24 + (x))
+
+#define IMX_MU_MAX_CHANS	4u
+
+struct imx_mu_priv;
+
+struct imx_mu_cfg {
+	unsigned int		chans;
+	void (*init_hw)(struct imx_mu_priv *priv);
+};
+
+struct imx_mu_con_priv {
+	int			irq;
+	unsigned int		bidx;
+	unsigned int		idx;
+};
+
+struct imx_mu_priv {
+	struct device		*dev;
+	const struct imx_mu_cfg	*dcfg;
+	void __iomem		*base;
+
+	struct mbox_controller	mbox;
+	struct mbox_chan	mbox_chans[IMX_MU_MAX_CHANS];
+
+	struct imx_mu_con_priv  con_priv[IMX_MU_MAX_CHANS];
+	struct clk		*clk;
+};
+
+static struct imx_mu_priv *to_imx_mu_priv(struct mbox_controller *mbox)
+{
+	return container_of(mbox, struct imx_mu_priv, mbox);
+}
+
+static void imx_mu_write(struct imx_mu_priv *priv, u32 val, u32 offs)
+{
+	iowrite32(val, priv->base + offs);
+}
+
+static u32 imx_mu_read(struct imx_mu_priv *priv, u32 offs)
+{
+	return ioread32(priv->base + offs);
+}
+
+static u32 imx_mu_rmw(struct imx_mu_priv *priv, u32 offs, u32 set, u32 clr)
+{
+	u32 val;
+
+	val = imx_mu_read(priv, offs);
+	val &= ~clr;
+	val |= set;
+	imx_mu_write(priv, val, offs);
+
+	return val;
+}
+
+static irqreturn_t imx_mu_isr(int irq, void *p)
+{
+	struct mbox_chan *chan = p;
+	struct imx_mu_con_priv *cp = chan->con_priv;
+	struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
+
+	u32 val, dat;
+
+	val = imx_mu_read(priv, IMX_MU_xSR);
+	val &= IMX_MU_xSR_TEn(cp->bidx) | IMX_MU_xSR_RFn(cp->bidx);
+	if (!val)
+		return IRQ_NONE;
+
+	if (val & IMX_MU_xSR_TEn(cp->bidx)) {
+		imx_mu_rmw(priv, IMX_MU_xCR, 0, IMX_MU_xCR_TIEn(cp->bidx));
+		mbox_chan_txdone(chan, 0);
+	}
+
+	if (val & IMX_MU_xSR_RFn(cp->bidx)) {
+		dat = imx_mu_read(priv, IMX_MU_xRRn(cp->idx));
+		mbox_chan_received_data(chan, (void *)&dat);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static bool imx_mu_last_tx_done(struct mbox_chan *chan)
+{
+	struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
+	struct imx_mu_con_priv *cp = chan->con_priv;
+	u32 val;
+
+	val = imx_mu_read(priv, IMX_MU_xSR);
+	/* test if transmit register is empty */
+	return (!(val & IMX_MU_xSR_TEn(cp->bidx)));
+}
+
+static int imx_mu_send_data(struct mbox_chan *chan, void *data)
+{
+	struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
+	struct imx_mu_con_priv *cp = chan->con_priv;
+	u32 *arg = data;
+
+	if (imx_mu_last_tx_done(chan))
+		return -EBUSY;
+
+	imx_mu_write(priv, *arg, IMX_MU_xTRn(cp->idx));
+	imx_mu_rmw(priv, IMX_MU_xCR, IMX_MU_xSR_TEn(cp->bidx), 0);
+
+	return 0;
+}
+
+static int imx_mu_startup(struct mbox_chan *chan)
+{
+	struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
+	struct imx_mu_con_priv *cp = chan->con_priv;
+	int ret;
+
+	ret = request_irq(cp->irq, imx_mu_isr,
+			  IRQF_SHARED, "imx_mu_chan", chan);
+	if (ret) {
+		dev_err(chan->mbox->dev,
+			"Unable to acquire IRQ %d\n", cp->irq);
+		return ret;
+	}
+
+	imx_mu_rmw(priv, IMX_MU_xCR, IMX_MU_xCR_RIEn(cp->bidx), 0);
+
+	return 0;
+}
+
+static void imx_mu_shutdown(struct mbox_chan *chan)
+{
+	struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
+	struct imx_mu_con_priv *cp = chan->con_priv;
+
+	imx_mu_rmw(priv, IMX_MU_xCR, 0,
+		   IMX_MU_xCR_TIEn(cp->bidx) | IMX_MU_xCR_RIEn(cp->bidx));
+
+	free_irq(cp->irq, chan);
+}
+
+static const struct mbox_chan_ops imx_mu_ops = {
+	.send_data = imx_mu_send_data,
+	.startup = imx_mu_startup,
+	.shutdown = imx_mu_shutdown,
+	.last_tx_done = imx_mu_last_tx_done,
+};
+
+static int imx_mu_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct resource *iomem;
+	struct imx_mu_priv *priv;
+	const struct imx_mu_cfg *dcfg;
+	unsigned int i, chans;
+	int irq, ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	dcfg = of_device_get_match_data(dev);
+	if (!dcfg)
+		return -EINVAL;
+
+	priv->dcfg = dcfg;
+	priv->dev = dev;
+
+	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->base = devm_ioremap_resource(&pdev->dev, iomem);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq <= 0)
+		return irq < 0 ? irq : -EINVAL;
+
+	priv->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(priv->clk)) {
+		if (PTR_ERR(priv->clk) == -ENOENT) {
+			priv->clk = NULL;
+		} else {
+			dev_err(dev, "Failed to get clock\n");
+			return PTR_ERR(priv->clk);
+		}
+	}
+
+	ret = clk_prepare_enable(priv->clk);
+	if (ret) {
+		dev_err(dev, "Failed to enable clock\n");
+		return ret;
+	}
+
+	chans = min(dcfg->chans, IMX_MU_MAX_CHANS);
+	/* Initialize channel identifiers */
+	for (i = 0; i < chans; i++) {
+		struct imx_mu_con_priv *cp = &priv->con_priv[i];
+
+		cp->bidx = 3 - i;
+		cp->idx = i;
+		cp->irq = irq;
+		priv->mbox_chans[i].con_priv = cp;
+	}
+
+	priv->mbox.dev = dev;
+	priv->mbox.ops = &imx_mu_ops;
+	priv->mbox.chans = priv->mbox_chans;
+	priv->mbox.num_chans = chans;
+	priv->mbox.txdone_irq = true;
+
+	platform_set_drvdata(pdev, priv);
+
+	if (priv->dcfg->init_hw)
+		priv->dcfg->init_hw(priv);
+
+	return mbox_controller_register(&priv->mbox);
+}
+
+static int imx_mu_remove(struct platform_device *pdev)
+{
+	struct imx_mu_priv *priv = platform_get_drvdata(pdev);
+
+	mbox_controller_unregister(&priv->mbox);
+	clk_disable_unprepare(priv->clk);
+
+	return 0;
+}
+
+
+static void imx_mu_init_imx7d_a(struct imx_mu_priv *priv)
+{
+	/* Set default config */
+	imx_mu_write(priv, 0, IMX_MU_xCR);
+}
+
+static const struct imx_mu_cfg imx_mu_cfg_imx7d_a = {
+	.chans = IMX_MU_MAX_CHANS,
+	.init_hw = imx_mu_init_imx7d_a,
+};
+
+static const struct imx_mu_cfg imx_mu_cfg_imx7d_b = {
+	.chans = IMX_MU_MAX_CHANS,
+};
+
+static const struct of_device_id imx_mu_dt_ids[] = {
+	{ .compatible = "fsl,imx7s-mu-a", .data = &imx_mu_cfg_imx7d_a },
+	{ .compatible = "fsl,imx7s-mu-b", .data = &imx_mu_cfg_imx7d_b },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, imx_mu_dt_ids);
+
+static struct platform_driver imx_mu_driver = {
+	.probe		= imx_mu_probe,
+	.remove		= imx_mu_remove,
+	.driver = {
+		.name	= "imx_mu",
+		.of_match_table = imx_mu_dt_ids,
+	},
+};
+module_platform_driver(imx_mu_driver);
+
+MODULE_AUTHOR("Oleksij Rempel <o.rempel@pengutronix.de>");
+MODULE_DESCRIPTION("Message Unit driver for i.MX7");
+MODULE_LICENSE("GPL v2");
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH v3 1/6] mtd: rawnand: add Reed-Solomon error correction algorithm
From: Boris Brezillon @ 2018-06-01  7:26 UTC (permalink / raw)
  To: Stefan Agner
  Cc: dwmw2, computersforpeace, marek.vasut, robh+dt, mark.rutland,
	thierry.reding, benjamin.lindqvist, pgaikwad, dev, mirza.krak,
	richard, pdeschrijver, linux-kernel, krzk, jonathanh, devicetree,
	linux-mtd, marcel, miquel.raynal, linux-tegra, digetx
In-Reply-To: <20180531221637.6017-2-stefan@agner.ch>

On Fri,  1 Jun 2018 00:16:32 +0200
Stefan Agner <stefan@agner.ch> wrote:

> Add Reed-Solomon (RS) to the enumeration of ECC algorithms.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>

Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>

> ---
>  drivers/mtd/nand/raw/nand_base.c | 1 +
>  include/linux/mtd/rawnand.h      | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index f28c3a555861..9eb5678dd6d0 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -5744,6 +5744,7 @@ static int of_get_nand_ecc_mode(struct device_node *np)
>  static const char * const nand_ecc_algos[] = {
>  	[NAND_ECC_HAMMING]	= "hamming",
>  	[NAND_ECC_BCH]		= "bch",
> +	[NAND_ECC_RS]		= "rs",
>  };
>  
>  static int of_get_nand_ecc_algo(struct device_node *np)
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 5dad59b31244..6a82da8c44ce 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -114,6 +114,7 @@ enum nand_ecc_algo {
>  	NAND_ECC_UNKNOWN,
>  	NAND_ECC_HAMMING,
>  	NAND_ECC_BCH,
> +	NAND_ECC_RS,
>  };
>  
>  /*

^ permalink raw reply

* Re: [PATCH v3 2/6] mtd: rawnand: add an option to specify NAND chip as a boot device
From: Boris Brezillon @ 2018-06-01  7:26 UTC (permalink / raw)
  To: Stefan Agner
  Cc: dwmw2, computersforpeace, marek.vasut, robh+dt, mark.rutland,
	thierry.reding, benjamin.lindqvist, pgaikwad, dev, mirza.krak,
	richard, pdeschrijver, linux-kernel, krzk, jonathanh, devicetree,
	linux-mtd, marcel, miquel.raynal, linux-tegra, digetx
In-Reply-To: <20180531221637.6017-3-stefan@agner.ch>

On Fri,  1 Jun 2018 00:16:33 +0200
Stefan Agner <stefan@agner.ch> wrote:

> Allow to define a NAND chip as a boot device. This can be helpful
> for the selection of the ECC algorithm and strength in case the boot
> ROM supports only a subset of controller provided options.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>

Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>

> ---
>  Documentation/devicetree/bindings/mtd/nand.txt | 4 ++++
>  drivers/mtd/nand/raw/nand_base.c               | 3 +++
>  include/linux/mtd/rawnand.h                    | 6 ++++++
>  3 files changed, 13 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
> index 8bb11d809429..8daf81b9748c 100644
> --- a/Documentation/devicetree/bindings/mtd/nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
> @@ -43,6 +43,10 @@ Optional NAND chip properties:
>  		     This is particularly useful when only the in-band area is
>  		     used by the upper layers, and you want to make your NAND
>  		     as reliable as possible.
> +- nand-is-boot-medium: Whether the NAND chip is a boot medium. Drivers might use
> +		       this information to select ECC algorithms supported by
> +		       the boot ROM or similar restrictions.
> +
>  - nand-rb: shall contain the native Ready/Busy ids.
>  
>  The ECC strength and ECC step size properties define the correction capability
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 9eb5678dd6d0..c8fb7c9855e2 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -5826,6 +5826,9 @@ static int nand_dt_init(struct nand_chip *chip)
>  	if (of_get_nand_bus_width(dn) == 16)
>  		chip->options |= NAND_BUSWIDTH_16;
>  
> +	if (of_property_read_bool(dn, "nand-is-boot-medium"))
> +		chip->options |= NAND_IS_BOOT_MEDIUM;
> +
>  	if (of_get_nand_on_flash_bbt(dn))
>  		chip->bbt_options |= NAND_BBT_USE_FLASH;
>  
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 6a82da8c44ce..8e54fcf2fa94 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -212,6 +212,12 @@ enum nand_ecc_algo {
>   */
>  #define NAND_WAIT_TCCS		0x00200000
>  
> +/*
> + * Whether the NAND chip is a boot medium. Drivers might use this information
> + * to select ECC algorithms supported by the boot ROM or similar restrictions.
> + */
> +#define NAND_IS_BOOT_MEDIUM	0x00400000
> +
>  /* Options set by nand scan */
>  /* Nand scan has allocated controller struct */
>  #define NAND_CONTROLLER_ALLOC	0x80000000

^ permalink raw reply

* Re: [PATCH v3 3/6] mtd: rawnand: tegra: add devicetree binding
From: Boris Brezillon @ 2018-06-01  7:30 UTC (permalink / raw)
  To: Stefan Agner
  Cc: dwmw2, computersforpeace, marek.vasut, robh+dt, mark.rutland,
	thierry.reding, benjamin.lindqvist, pgaikwad, dev, mirza.krak,
	richard, pdeschrijver, linux-kernel, krzk, jonathanh, devicetree,
	linux-mtd, marcel, miquel.raynal, linux-tegra, digetx
In-Reply-To: <20180531221637.6017-4-stefan@agner.ch>

On Fri,  1 Jun 2018 00:16:34 +0200
Stefan Agner <stefan@agner.ch> wrote:

> This adds the devicetree binding for the Tegra 2 NAND flash
> controller.
> 
> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  .../bindings/mtd/nvidia-tegra20-nand.txt      | 64 +++++++++++++++++++
>  1 file changed, 64 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> new file mode 100644
> index 000000000000..5cd984ef046b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> @@ -0,0 +1,64 @@
> +NVIDIA Tegra NAND Flash controller
> +
> +Required properties:
> +- compatible: Must be one of:
> +  - "nvidia,tegra20-nand"

As discussed previously, I prefer "nvidia,tegra20-nand-controller" or
"nvidia,tegra20-nfc".

> +- reg: MMIO address range
> +- interrupts: interrupt output of the NFC controller
> +- clocks: Must contain an entry for each entry in clock-names.
> +  See ../clocks/clock-bindings.txt for details.
> +- clock-names: Must include the following entries:
> +  - nand
> +- resets: Must contain an entry for each entry in reset-names.
> +  See ../reset/reset.txt for details.
> +- reset-names: Must include the following entries:
> +  - nand
> +
> +Optional children nodes:
> +Individual NAND chips are children of the NAND controller node. Currently
> +only one NAND chip supported.
> +
> +Required children node properties:
> +- reg: An integer ranging from 1 to 6 representing the CS line to use.
> +
> +Optional children node properties:
> +- nand-ecc-mode: String, operation mode of the NAND ecc mode. Currently only
> +		 "hw" is supported.
> +- nand-ecc-algo: string, algorithm of NAND ECC.
> +		 Supported values with "hw" ECC mode are: "rs", "bch".
> +- nand-bus-width : See nand.txt
> +- nand-on-flash-bbt: See nand.txt
> +- nand-ecc-strength: integer representing the number of bits to correct
> +		     per ECC step (always 512). Supported strength using HW ECC
> +		     modes are:
> +		     - RS: 4, 6, 8
> +		     - BCH: 4, 8, 14, 16
> +- nand-ecc-maximize: See nand.txt
> +- nand-is-boot-medium: Makes sure only ECC strengths supported by the boot ROM
> +		       are choosen.
> +- wp-gpios: GPIO specifier for the write protect pin.
> +
> +Optional child node of NAND chip nodes:
> +Partitions: see partition.txt
> +
> +  Example:
> +	nand@70008000 {

	nand-controller@70008000 {

> +		compatible = "nvidia,tegra20-nand";

		compatible = "nvidia,tegra20-nand-controller";

or

		compatible = "nvidia,tegra20-nfc";

> +		reg = <0x70008000 0x100>;
> +		interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&tegra_car TEGRA20_CLK_NDFLASH>;
> +		clock-names = "nand";
> +		resets = <&tegra_car 13>;
> +		reset-names = "nand";
> +
> +		nand-chip@0 {

		nand@0 {

> +			reg = <0>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			nand-bus-width = <8>;
> +			nand-on-flash-bbt;
> +			nand-ecc-algo = "bch";
> +			nand-ecc-strength = <8>;
> +			wp-gpios = <&gpio TEGRA_GPIO(S, 0) GPIO_ACTIVE_LOW>;
> +		};
> +	};

With this addressed,

Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>

^ permalink raw reply

* Re: [PATCH v4 5/6] clk: bd71837: Add driver for BD71837 PMIC clock
From: Matti Vaittinen @ 2018-06-01  7:31 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Matti Vaittinen, broonie, lee.jones, lgirdwood, mark.rutland,
	mazziesaccount, mturquette, robh+dt, linux-clk, devicetree,
	linux-kernel, mikko.mutanen, heikki.haikola
In-Reply-To: <152777943977.144038.10971658990200651749@swboyd.mtv.corp.google.com>

First of all - Thanks for looking at this!

On Thu, May 31, 2018 at 08:10:39AM -0700, Stephen Boyd wrote:
> Quoting Matti Vaittinen (2018-05-30 01:43:19)
> > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> > index 41492e980ef4..4b045699bb5e 100644
> > --- a/drivers/clk/Kconfig
> > +++ b/drivers/clk/Kconfig
> > @@ -279,6 +279,15 @@ config COMMON_CLK_STM32H7
> >         ---help---
> >           Support for stm32h7 SoC family clocks
> >  
> > +config COMMON_CLK_BD71837
> > +       tristate "Clock driver for ROHM BD71837 PMIC MFD"
> > +       depends on MFD_BD71837
> > +       depends on I2C=y
> 
> Why depend on I2C=y?

I added this because the PMIC is connected via i2c. But as you ask this
- and as you probably knew my answer - I guess this is not correct. So
I guess depends on MFD_BD71837 should be sufficient and the MFD portion
should hide the fact we sit on I2C? If this is the case - I will remove
this.

> 
> > +       depends on OF
> 
> Why depend on OF?

You're right. This is not needed. I'll remove this.

> 
> > +       help
> > +         This driver supports ROHM BD71837 PMIC clock.
> > +
> > +
> >  source "drivers/clk/bcm/Kconfig"
> >  source "drivers/clk/hisilicon/Kconfig"
> >  source "drivers/clk/imgtec/Kconfig"
> > diff --git a/drivers/clk/clk-bd71837.c b/drivers/clk/clk-bd71837.c
> > new file mode 100644
> > index 000000000000..91456d1077ac
> > --- /dev/null
> > +++ b/drivers/clk/clk-bd71837.c
> > @@ -0,0 +1,151 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (C) 2018 ROHM Semiconductors
> > +// bd71837.c  -- ROHM BD71837MWV clock driver
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/init.h>
> > +#include <linux/err.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>
> > +#include <linux/mfd/bd71837.h>
> > +#include <linux/clk-provider.h>
> > +#include <linux/clkdev.h>
> > +
> > +static int bd71837_clk_enable(struct clk_hw *hw);
> > +static void bd71837_clk_disable(struct clk_hw *hw);
> > +static int bd71837_clk_is_enabled(struct clk_hw *hw);
> > +
> > +struct bd71837_clk {
> > +       struct clk_hw hw;
> > +       uint8_t reg;
> > +       uint8_t mask;
> > +       unsigned long rate;
> > +       struct platform_device *pdev;
> > +       struct bd71837 *mfd;
> > +};
> > +
> > +static unsigned long bd71837_clk_recalc_rate(struct clk_hw *hw,
> > +                                            unsigned long parent_rate);
> > +
> > +static const struct clk_ops bd71837_clk_ops = {
> > +       .recalc_rate = &bd71837_clk_recalc_rate,
> > +       .prepare = &bd71837_clk_enable,
> > +       .unprepare = &bd71837_clk_disable,
> > +       .is_prepared = &bd71837_clk_is_enabled,
> > +};
> 
> Move this structure after the function definitions. And drop the forward
> declared functions.

Allright.

> 
> > +
> > +static int bd71837_clk_set(struct clk_hw *hw, int status)
> > +{
> > +       struct bd71837_clk *c = container_of(hw, struct bd71837_clk, hw);
> > +
> > +       return bd71837_update_bits(c->mfd, c->reg, c->mask, status);
> > +}
> > +
> > +static void bd71837_clk_disable(struct clk_hw *hw)
> > +{
> > +       int rv;
> > +       struct bd71837_clk *c = container_of(hw, struct bd71837_clk, hw);
> > +
> > +       rv = bd71837_clk_set(hw, 0);
> > +       if (rv)
> > +               dev_err(&c->pdev->dev, "Failed to disable 32K clk (%d)\n", rv);
> 
> Why is a disable error message more important than an enable error
> message?  Please drop this message and rely on callers to indicate if
> enabling their clk didn't work for some reason.

Reason is that the disable function is not returning error. So if I drop
the print there will be no indication of error, right?

> 
> > +}
> > +
> > +static int bd71837_clk_enable(struct clk_hw *hw)
> > +{
> > +       return bd71837_clk_set(hw, 1);
> > +}
> > +
> > +static int bd71837_clk_is_enabled(struct clk_hw *hw)
> > +{
> > +       struct bd71837_clk *c = container_of(hw, struct bd71837_clk, hw);
> > +
> > +       return c->mask & bd71837_reg_read(c->mfd, c->reg);
> 
> Please put this on two or more lines so we know what the type of
> bd71837_reg_read() returns:
> 
> 	u32 reg = bd71837_reg_read(....)
> 
> 	return c->mask & reg;
> 

Right. I'll do this.

> 
> > +}
> > +
> > +static unsigned long bd71837_clk_recalc_rate(struct clk_hw *hw,
> > +                                            unsigned long parent_rate)
> > +{
> > +       struct bd71837_clk *c = container_of(hw, struct bd71837_clk, hw);
> > +
> > +       return c->rate;
> 
> Recalc rate should read the hardware instead of returning a cached rate
> unless it can't actually read hardware.

We can't read the rate from HW. And actually, as this is fixed rate
clock generator - is the recalc_rate needed at all?

> 
> > +}
> > +
> > +static int bd71837_clk_probe(struct platform_device *pdev)
> > +{
> > +       struct bd71837_clk *c;
> > +       int rval = -ENOMEM;
> > +       struct bd71837 *mfd = dev_get_drvdata(pdev->dev.parent);
> > +       const char *errstr = "memory allocation for bd71837 data failed";
> 
> We don't need allocation error messages.

Correct. I'll remove.

> 
> > +       struct clk_init_data init = {
> > +               .name = "bd71837-32k-out",
> > +               .ops = &bd71837_clk_ops,
> > +       };
> > +
> > +       c = kzalloc(sizeof(struct bd71837_clk), GFP_KERNEL);
> 
> Use sizeof(*c) instead so we don't have to worry about type mismatches.

Ok.

> 
> > +       if (!c)
> > +               goto err_out;
> > +
> > +       c->reg = BD71837_REG_OUT32K;
> > +       c->mask = BD71837_OUT32K_EN;
> > +       c->rate = BD71837_CLK_RATE;
> 
> Is the plan to have more clks supported by this driver in the future?
> Because right now it seems to make a structure up and then hardcode the
> members for a single clk so I'm not sure why those registers aren't just
> hardcoded in the clk_ops functions that use them.

(At least) one more chip from this series is coming and I am planning to
support it with this driver. I am not sure if the registers will stay
the same. Hence I rather not hardcode the register values.

> 
> > +       c->mfd = mfd;
> > +       c->pdev = pdev;
> > +
> > +       if (pdev->dev.of_node)
> 
> If there isn't an of_node it would be NULL, and then calling the
> function below would cause it to not update the init name? Seems like it
> could be called unconditionally.

Right.

> 
> > +               of_property_read_string_index(pdev->dev.of_node,
> > +                                             "clock-output-names", 0,
> > +                                             &init.name);
> > +
> > +       c->hw.init = &init;
> > +
> > +       errstr = "failed to register 32K clk";
> 
> The 'errstr' thing is not standard. Please just call dev_err() directly
> with the string you want to print. And consider not printing anything at
> all.

I think this technique is actually used in many places at net side. I
guess i have adobted this habit from some netlink message handling code.
I can change this to in-place prints if it fits environment better
though.

> 
> > +       rval = clk_hw_register(&pdev->dev, &c->hw);
> > +       if (rval)
> > +               goto err_free;
> > +
> > +       errstr = "failed to register clkdev for bd71837";
> > +       rval = clk_hw_register_clkdev(&c->hw, init.name, NULL);
> 
> Are you using the clkdev lookup? Or this is just added for backup
> purposes? If this is a mostly DT driver then please drop this part of
> the patch and rely on of_clk_hw_add_provider() to handle the lookup
> instead.

I did this because this is how I get the clk_get to work in my test
driver. Problem is that I don't know how this clock is going to be used
- and I lack of knowledge how these things are usually done. I don't
know the clock consumer code so this was best I could think of. But if
this is not how things are usually handled I can remove the clkdev and
rely on of_clk_add_hw_provider. Would this be correct then:

> 
> > +       if (rval)
> > +               goto err_unregister;
> > +
> > +       platform_set_drvdata(pdev, c);
> > +       dev_dbg(&pdev->dev, "bd71837_clk successfully probed\n");
> 
> There's a pr_debug() in really_probe() for this.

Oh, thanks. I'll remove this debug.

> 
> > +
> > +       return 0;
> > +
> > +err_unregister:
> > +       clk_hw_unregister(&c->hw);
> > +err_free:
> > +       kfree(c);
> > +err_out:
> > +       dev_err(&pdev->dev, "%s\n", errstr);
> > +       return rval;
> > +}
> > +
> > +static int bd71837_clk_remove(struct platform_device *pdev)
> > +{
> > +       struct bd71837_clk *c = platform_get_drvdata(pdev);
> > +
> > +       if (c) {
> > +               clk_hw_unregister(&c->hw);
> 
> Use devm to register clks.
> 
> > +               kfree(c);
> 
> and devm_kzalloc()

Right. Thanks - devm makes this simpler.

> 
> > +               platform_set_drvdata(pdev, NULL);
> 
> This doesn't need to be done. Drop it.

Allright. Will drop.

> 
> > +       }
> > +       return 0;
> > +}
> > +



Br,
	Matti Vaittinen

^ permalink raw reply

* Re: [PATCH v2 5/5] arm64: dts: renesas: r8a7795: add ccree binding
From: Geert Uytterhoeven @ 2018-06-01  8:07 UTC (permalink / raw)
  To: Gilad Ben-Yossef
  Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
	Catalin Marinas, Will Deacon, Geert Uytterhoeven,
	Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
	Ofir Drang, Linux-Renesas,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux ARM, Linux Kernel Mailing List, linux-clk
In-Reply-To: <1527171551-21979-6-git-send-email-gilad@benyossef.com>

Hi Gilad,

On Thu, May 24, 2018 at 4:19 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> Add bindings for CryptoCell instance in the SoC.
>
> Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>

Thanks for the update!

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v2 5/5] arm64: dts: renesas: r8a7795: add ccree binding
From: Geert Uytterhoeven @ 2018-06-01  8:12 UTC (permalink / raw)
  To: Gilad Ben-Yossef
  Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
	Catalin Marinas, Will Deacon, Geert Uytterhoeven,
	Michael Turquette, Stephen Boyd, Herbert Xu, David S. Miller,
	Ofir Drang, Linux-Renesas,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux ARM, Linux kernel mailing list, linux-clk
In-Reply-To: <CAOtvUMeBCH+23c3FDWncMwPPe5N=7yjRKzhyLmFXrYazgGbcJw@mail.gmail.com>

Hi Gilad,

On Thu, May 31, 2018 at 1:55 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> On Tue, May 29, 2018 at 7:19 PM, Simon Horman <horms@verge.net.au> wrote:
>> On Thu, May 24, 2018 at 03:19:10PM +0100, Gilad Ben-Yossef wrote:
>>> Add bindings for CryptoCell instance in the SoC.
>>>
>>> Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
>>
>> In so far as I can review the details of this (which is not much) this
>> looks fine to me. I am, however, a little unclear in when it should be
>> accepted.
>
> Since Herbert Xu ACKed the driver changes, I would say the only gating
> commit is Geert's CR clock patch.

These are queued for v4.19.

> If that one is in, than I would say this one should go in as well.

As the device node now has a power-domains property, the genpd code will
try to attach it to the CPG/MSSR PM Domain, which is a clock domain.
In the absence of the clock patch, the device's module clock cannot be
found, and dev_pm_domain_attach() and thus platform_drv_probe() will fail,
before calling the device driver's .probe() function.

So there is no longer a dependency on the clock patch, and the DT patch can
go in in parallel (although I prefer its subject to be changed
s/binding/device device/).

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v2 4/8] dt-bindings: gnss: add u-blox binding
From: Johan Hovold @ 2018-06-01  8:15 UTC (permalink / raw)
  To: Rob Herring
  Cc: Johan Hovold, Greg Kroah-Hartman, Mark Rutland, Andreas Kemnade,
	Arnd Bergmann, H . Nikolaus Schaller, Pavel Machek,
	Marcel Holtmann, Sebastian Reichel, Tony Lindgren,
	linux-kernel@vger.kernel.org, devicetree
In-Reply-To: <CAL_JsqLfqgUbzMWdE_c96DrdgB31gSixbJo+JvsZhoJGJRWYMw@mail.gmail.com>

On Thu, May 31, 2018 at 10:58:59AM -0500, Rob Herring wrote:
> On Thu, May 31, 2018 at 9:34 AM, Johan Hovold <johan@kernel.org> wrote:
> > On Thu, May 31, 2018 at 08:55:10AM -0500, Rob Herring wrote:
> >> On Thu, May 31, 2018 at 3:22 AM, Johan Hovold <johan@kernel.org> wrote:
> >> > On Wed, May 30, 2018 at 10:58:05PM -0500, Rob Herring wrote:
> >> >> On Wed, May 30, 2018 at 12:32:38PM +0200, Johan Hovold wrote:
> >> >> > Add binding for u-blox GNSS receivers.

> >> >> > +Optional properties:
> >> >> > +
> >> >> > +- timepulse-gpios  : Time pulse GPIO
> >> >> > +- u-blox,extint-gpios      : External interrupt GPIO
> >> >>
> >> >> This should be interrupts property instead of a gpio.
> >> >
> >> > Contrary to what the name may suggest, this pin is actually an input
> >> > which can be used to control active power or to provide time or
> >> > frequency aiding data to the receiver (see section 1.13 in [1]).
> >> >
> >> > I only added it for completeness as the driver does not use it
> >> > currently. Remove, leave as is, or add "input" to the description as in:
> >> >
> >> >         - u-blox,extint-gpios   : External interrupt input GPIO
> >>
> >> Yes. You should also define the active level.
> >
> > The active level appears to be runtime configurable and dependent on the
> > selected function. For power control it can be used to control force-on
> > (active high), force-off (active low) or both; and for time aiding
> > sampling on falling or rising edge is also configurable. And who knows
> > what else this pin is used for next time they update the firmware. ;)
> 
> Okay.
> 
> > Shall I remove the property, or just add "input" as suggested above?
> 
> Just add "input".

I ended up rewording this as

	u-blox,extint-gpios  : GPIO connected to the "external
			       interrupt" input pin

to avoid any confusion about the direction of the GPIO.

Thanks,
Johan

^ permalink raw reply

* Re: [PATCH v7 1/5] dt-bindings: Add the r9a06g032-sysctrl.h file
From: Geert Uytterhoeven @ 2018-06-01  8:18 UTC (permalink / raw)
  To: M P
  Cc: Michel Pollet, Linux-Renesas, Simon Horman, Phil Edworthy,
	Michel Pollet, Michael Turquette, Stephen Boyd, Rob Herring,
	Mark Rutland, Geert Uytterhoeven, linux-clk,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux Kernel Mailing List
In-Reply-To: <CAMMfpEwnB26q+JzEfwby409XkUPWpNZAD8FhRWJfjW=0naY+pg@mail.gmail.com>

Hi Michel,

On Thu, May 31, 2018 at 12:01 PM, M P <buserror@gmail.com> wrote:
> On Thu, 31 May 2018 at 10:32, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> On Thu, May 31, 2018 at 11:11 AM, M P <buserror@gmail.com> wrote:
>> > On Fri, 25 May 2018 at 11:31, Geert Uytterhoeven <geert@linux-m68k.org>
>> > wrote:
>> >> On Thu, May 24, 2018 at 11:28 AM, Michel Pollet
>> >> <michel.pollet@bp.renesas.com> wrote:
>> >> > This adds the constants necessary to use the renesas,r9a06g032-sysctrl
>> > node.
>>
>> >> > @@ -0,0 +1,187 @@
>> >> > +/* SPDX-License-Identifier: GPL-2.0 */
>> >> > +/*
>> >> > + * R9A06G032 sysctrl IDs
>> >> > + *
>> >> > + * Copyright (C) 2018 Renesas Electronics Europe Limited
>> >> > + *
>> >> > + * Michel Pollet <michel.pollet@bp.renesas.com>, <buserror@gmail.com>
>> >> > + * Derived from zx-reboot.c
>> >> > + */
>> >> > +
>> >> > +#ifndef __DT_BINDINGS_R9A06G032_SYSCTRL_H__
>> >> > +#define __DT_BINDINGS_R9A06G032_SYSCTRL_H__
>> >> > +
>> >> > +#define R9A06G032_CLKOUT                       0
>> >> > +#define R9A06G032_CLK_PLL_USB                  1
>> >> > +#define R9A06G032_CLK_48                       1 /* AKA CLK_PLL_USB */
>> >> > +#define R9A06G032_CLKOUT_D10                   2
>> >> > +#define R9A06G032_CLKOUT_D16                   3
>> >> > +#define R9A06G032_MSEBIS_CLK                   3 /* AKA CLKOUT_D16 */
>> >> > +#define R9A06G032_MSEBIM_CLK                   3 /* AKA CLKOUT_D16 */
>> >> > +#define R9A06G032_CLKOUT_D160                  4
>> >> > +#define R9A06G032_CLKOUT_D1OR2                 5
>> >> > +#define R9A06G032_CLK_DDRPHY_PLLCLK            5 /* AKA CLKOUT_D1OR2 */
>> >
>> >> [...]
>> >
>> >> I have 3 comments:
>> >
>> >>    1. I had expected this list to match (both name- and order-wise)
>> > Appendix
>> >>       C ("Clock Tree Structure") in the RZ/N1[DSL] User’s Manual: System
>> >>       Introduction, Multiplexing, Electrical and Mechanical Information.
>> >>       That would make it easier to review.
>> >
>> > Well, that document was made a *long* time after the internal documentation
>> > we used to generate the clock lists. There are a few things we had to do:
>> >
>> > * Renumber peripherals. We decided a long time ago that u-boot and linux
>> > would stick to zero based peripherals, and not one based numbers. It's next
>> > to impossible to keep mixing number based across software base, so we use
>> > UART0 while the hardware manual mentions UART1 -- It *is* documented
>> > extensively with out SDK, and makes life using linux a lot easier. It's
>> > across all our SDK, u-boot, webapps readmes, howtos etc.
>> >
>> > * Rename some peripherals. Plenty of peripherals names made little sense
>> > and had zero consistency, in fact, many names were different depending on
>> > the place they were used! -- "flashnand"+"nand_flash"+"FNAND" etc,
>> > "QUADSPI"+"QSPI" etc etc so we also re-normalized the names to match linux
>> > conventions.
>> >
>> > * Other internal reasons I can't document here
>> >
>> > Also, the value here are made up anyway -- so I've decided to sort them to
>> > make sure any clock that has a parent is numbered *after* the parent...
>>
>> Well, all of that combines makes it very hard for us to review the list.
>
> I understand -- the previous format was a lot more readable, here I had to
> work to make the table as small as I could, and quite a few bits of readability
> were lost in the process. It's already a huge file.
>
> I'm not sure if that would help, but here is the link to the old table
> format I used
> https://github.com/renesas-rz/rzn1_linux/blob/rzn1-stable/drivers/clk/rzn1/rzn1-clkctrl-tables.h
>
> I had to throw that away to make up the new composite table that now contains
> what was in the device tree file before. However the names are the same.
>
> Perhaps I could change how I encode the register/bit pairs to use 8+8 bits to
> make the hex constants more readable?

Alternatively, you can use a macro to pack them, cfr. MOD_CLK_PACK()
in renesas-cpg-mssr.

>> >>    2. These definitions seems to be a mix of:
>> >>         1. Internal core clocks,
>> >>         2. Other core clocks,
>> >>         3. Module clocks.
>> >
>> >>       The internal clocks do not need to be referenced from DT, so I would
>> >>       not make them part of the DT bindings.
>> >
>> > Why? I'm told that "Bindings aren't for linux" -- why can't I imagine
>> > 'something' needing them? Why would I decide NOT to include them,
>> > as they are there? I 'factored' a few of them to the same number when
>>
>> Just general safety w.r.t. unchangeable DT definitions: anything that is
>> not listed here cannot be declared wrong later.
>
> Well, I need these #define *somewhere* as I use them in my driver. So what
> do you want me to do?
> + Remove the header, move the constants into the driver?
> + Use hard coded numbers in the DT and remove the header entirely?
> + Duplicate the header, keep the full one with the driver, and only list
> the CA7+UART0 one in the dt-binding and duplicate the ones I need as
> I go along?

None of these. You can just move the #defines that will never be needed
from DT into the driver source file.

Cfr. "Internal Core Clocks" in drivers/clk/renesas/r8a7795-cpg-mssr.c.

>> > applicable.
>>
>> You're 100% sure they're the same?
>
> Yes, the script logic hasn't changed in 2 years, and we've been using that
> hierarchy extensively since.
> Basically the logic is that if a clock doesn't have a gate and isn't a divisor
> of some sort, it's factored with it's parent clock... it used to be
> done with a DT
> alias of the same name.

OK.

>> > This is all done automatically BTW -- a script takes the original clocking
>> > spreadsheet, and converts it into a table, correcting 'human input' as it
>> > goes along.
>>
>> So the internal spreadsheet doesn't match the public documentation neither?
>
> Nope, as usual, people who wrote the documentation went their own way at
> some point, and didn't backport any changes they made.

Well, I guess we'll have to live with that...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v7 2/5] dt-bindings: clock: renesas,r9a06g032-sysctrl: documentation
From: Geert Uytterhoeven @ 2018-06-01  8:22 UTC (permalink / raw)
  To: M P
  Cc: Michel Pollet, Linux-Renesas, Simon Horman, Phil Edworthy,
	Michel Pollet, Michael Turquette, Stephen Boyd, Rob Herring,
	Mark Rutland, Geert Uytterhoeven, linux-clk,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux Kernel Mailing List
In-Reply-To: <CAMMfpExt6Kogw6T5FVrD94QuuN6BBNCXdW0qfnnWr2bEMfqjpA@mail.gmail.com>

Hi Michel,

On Thu, May 31, 2018 at 12:16 PM, M P <buserror@gmail.com> wrote:
> On Fri, 25 May 2018 at 10:23, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> On Thu, May 24, 2018 at 11:28 AM, Michel Pollet
>> <michel.pollet@bp.renesas.com> wrote:
>> > The Renesas R9A06G032 SYSCTRL node description.
>> >
>> > Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
>>
>> Thanks for your patch!
>>
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/clock/renesas,r9a06g032-sysctrl.txt
>> > @@ -0,0 +1,32 @@
>> > +* Renesas R9A06G032 SYSCTRL
>> > +
>> > +Required Properties:
>> > +
>> > +  - compatible: Must be:
>> > +    - "renesas,r9a06g032-sysctrl"
>> > +  - reg: Base address and length of the SYSCTRL IO block.
>> > +  - #clock-cells: Must be 1
>>
>> No clocks/clock-names for the external clock inputs?
>>
>> "RZ/N1 has 3 clock sources, 1 reference clock inputs for RGMII, and 2
>>  reference clock outputs for RMII/MII."
>>
>> Given the documentation explicitly mentions the module clocks are to be
>> used for power-management, you may want to add #power-domain-cells as well,
>> and let the driver register clock domain. But that can be added later
>> (although it will break backwards compatibility with old DTBs).
>>
>> As PWRCTRL_* registers allow to reset individual modules, #reset-cells is
>> another thing to add later.  It's good to start thinking early about how to
>> reference resets, though.
>> E.g. on other Renesas-SoCs, module resets uses the same numerical
>> references as module clocks.
>
> As you said, could we add all that later, as appropriate? Here I tried
> to trim it
> down to the the bare minimum -- my previous version of the driver had
> separate reset  descriptors, but this one has been all compacted to do just
> what it's supposed to do: clocks.

Yes, it can be added later.
I just wanted to mention it, so you could already think about it, and to avoid a
possible "but we could have nicely integrated reset and clock support if we
did ..." later.

> Or, so you want to add another DT index to refer to other reset indexes etc?
> ie not use the of_clk_src_onecell_get provider? That COULD work and yes, the
> indexes would stay the same, I'd just have to get the reset descriptor from the
> clock object. We haven't had a use for individual resets so far.

Reset indices come from DT, too, but the reset framework doesn't use an
xlate function itself, but just passes the index to the reset driver.
How you obtain the register and bits to reset the device is competlely up
to to the reset driver.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply


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