Devicetree
 help / color / mirror / Atom feed
* [PATCH] arm64: dts: rockchip: Add dts for Leez RK3399 P710 SBC
From: Andy Yan @ 2019-08-03 11:46 UTC (permalink / raw)
  To: heiko
  Cc: devicetree, Andy Yan, linux-kernel, linux-rockchip, robh+dt,
	linux-arm-kernel

Leez P710 is a RK3399 based SBC, designed by Leez team
from lenovo [0].

Specification
- Rockchip RK3399
- 4/2GB LPDDR4
- TF sd scard slot
- eMMC
- M.2 B-Key for 4G LTE
- AP6256 for WiFi + BT
- Gigabit ethernet
- HDMI out
- 40 pin header
- TYPE-C Power supply

[0] https://leez.lenovo.com

Signed-off-by: Andy Yan <andyshrk@gmail.com>
---
 .../devicetree/bindings/arm/rockchip.yaml     |   5 +
 arch/arm64/boot/dts/rockchip/Makefile         |   1 +
 .../boot/dts/rockchip/rk3399-leez-p710.dts    | 635 ++++++++++++++++++
 3 files changed, 641 insertions(+)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts

diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml b/Documentation/devicetree/bindings/arm/rockchip.yaml
index 34865042f4e4..da9cd947abfa 100644
--- a/Documentation/devicetree/bindings/arm/rockchip.yaml
+++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
@@ -329,6 +329,11 @@ properties:
               - khadas,edge-v
           - const: rockchip,rk3399
 
+      - description: Leez RK3399 P710
+        items:
+          - const: leez,p710
+          - const: rockchip,rk3399
+
       - description: mqmaker MiQi
         items:
           - const: mqmaker,miqi
diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
index daa2c78e22c3..1f18a9392d15 100644
--- a/arch/arm64/boot/dts/rockchip/Makefile
+++ b/arch/arm64/boot/dts/rockchip/Makefile
@@ -20,6 +20,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-hugsun-x99.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-khadas-edge.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-khadas-edge-captain.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-khadas-edge-v.dtb
+dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-leez-p710.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopc-t4.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopi-m4.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopi-neo4.dtb
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts b/arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts
new file mode 100644
index 000000000000..b342f5e8692b
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts
@@ -0,0 +1,635 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2019 Andy Yan <andy.yan@gmail.com>
+ */
+
+/dts-v1/;
+#include <dt-bindings/input/linux-event-codes.h>
+#include <dt-bindings/pwm/pwm.h>
+#include "rk3399.dtsi"
+#include "rk3399-opp.dtsi"
+
+/ {
+	model = "Leez RK3399 P710";
+	compatible = "leez,p710", "rockchip,rk3399";
+
+	chosen {
+		stdout-path = "serial2:1500000n8";
+	};
+
+	clkin_gmac: external-gmac-clock {
+		compatible = "fixed-clock";
+		clock-frequency = <125000000>;
+		clock-output-names = "clkin_gmac";
+		#clock-cells = <0>;
+	};
+
+	sdio_pwrseq: sdio-pwrseq {
+		compatible = "mmc-pwrseq-simple";
+		clocks = <&rk808 1>;
+		clock-names = "ext_clock";
+		pinctrl-names = "default";
+		pinctrl-0 = <&wifi_enable_h>;
+		reset-gpios = <&gpio0 RK_PB2 GPIO_ACTIVE_LOW>;
+	};
+
+	dc5v_adp: dc-5v {
+		compatible = "regulator-fixed";
+		regulator-name = "dc5v_adapter";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+	};
+
+	vcc5v0_sys: vcc-sys {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc5v0_sys";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		vin-supply = <&dc5v_adp>;
+	};
+
+	vcc3v3_sys: vcc3v3-sys {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc3v3_sys";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		vin-supply = <&vcc5v0_sys>;
+	};
+
+	vcc5v0_host: vcc5v0-host-regulator {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio2 RK_PA2 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&vcc5v0_host_en>;
+		regulator-name = "vcc5v0_host";
+		regulator-always-on;
+		vin-supply = <&vcc5v0_sys>;
+	};
+
+	vcc_lan: vcc3v3-phy-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc_lan";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
+	};
+
+	vdd_log: vdd-log {
+		compatible = "pwm-regulator";
+		pwms = <&pwm2 0 25000 1>;
+		regulator-name = "vdd_log";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <800000>;
+		regulator-max-microvolt = <1400000>;
+		vin-supply = <&vcc5v0_sys>;
+	};
+};
+
+&cpu_l0 {
+	cpu-supply = <&vdd_cpu_l>;
+};
+
+&cpu_l1 {
+	cpu-supply = <&vdd_cpu_l>;
+};
+
+&cpu_l2 {
+	cpu-supply = <&vdd_cpu_l>;
+};
+
+&cpu_l3 {
+	cpu-supply = <&vdd_cpu_l>;
+};
+
+&cpu_b0 {
+	cpu-supply = <&vdd_cpu_b>;
+};
+
+&cpu_b1 {
+	cpu-supply = <&vdd_cpu_b>;
+};
+
+&emmc_phy {
+	status = "okay";
+};
+
+&gmac {
+	assigned-clocks = <&cru SCLK_RMII_SRC>;
+	assigned-clock-parents = <&clkin_gmac>;
+	clock_in_out = "input";
+	phy-supply = <&vcc_lan>;
+	phy-mode = "rgmii";
+	pinctrl-names = "default";
+	pinctrl-0 = <&rgmii_pins>;
+	snps,reset-gpio = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>;
+	snps,reset-active-low;
+	snps,reset-delays-us = <0 10000 50000>;
+	tx_delay = <0x28>;
+	rx_delay = <0x11>;
+	status = "okay";
+};
+
+&gpu {
+	mali-supply = <&vdd_gpu>;
+	status = "okay";
+};
+
+&hdmi {
+	ddc-i2c-bus = <&i2c3>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&hdmi_cec>;
+	status = "okay";
+};
+
+&hdmi_sound {
+	status = "okay";
+};
+
+&i2c0 {
+	clock-frequency = <400000>;
+	i2c-scl-rising-time-ns = <168>;
+	i2c-scl-falling-time-ns = <4>;
+	status = "okay";
+
+	rk808: pmic@1b {
+		compatible = "rockchip,rk808";
+		reg = <0x1b>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <21 IRQ_TYPE_LEVEL_LOW>;
+		#clock-cells = <1>;
+		clock-output-names = "xin32k", "rk808-clkout2";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pmic_int_l>;
+		rockchip,system-power-controller;
+		wakeup-source;
+
+		vcc1-supply = <&vcc5v0_sys>;
+		vcc2-supply = <&vcc5v0_sys>;
+		vcc3-supply = <&vcc5v0_sys>;
+		vcc4-supply = <&vcc5v0_sys>;
+		vcc6-supply = <&vcc5v0_sys>;
+		vcc7-supply = <&vcc5v0_sys>;
+		vcc8-supply = <&vcc3v3_sys>;
+		vcc9-supply = <&vcc5v0_sys>;
+		vcc10-supply = <&vcc5v0_sys>;
+		vcc11-supply = <&vcc5v0_sys>;
+		vcc12-supply = <&vcc3v3_sys>;
+		vddio-supply = <&vcc_1v8>;
+
+		regulators {
+			vdd_center: DCDC_REG1 {
+				regulator-name = "vdd_center";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <750000>;
+				regulator-max-microvolt = <1350000>;
+				regulator-ramp-delay = <6001>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdd_cpu_l: DCDC_REG2 {
+				regulator-name = "vdd_cpu_l";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <750000>;
+				regulator-max-microvolt = <1350000>;
+				regulator-ramp-delay = <6001>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc_ddr: DCDC_REG3 {
+				regulator-name = "vcc_ddr";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
+			};
+
+			vcc_1v8: DCDC_REG4 {
+				regulator-name = "vcc_1v8";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1800000>;
+				};
+			};
+
+			vcc1v8_dvp: LDO_REG1 {
+				regulator-name = "vcc1v8_dvp";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc1v8_hdmi: LDO_REG2 {
+				regulator-name = "vcc1v8_hdmi";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcca_1v8: LDO_REG3 {
+				regulator-name = "vcca_1v8";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1800000>;
+				};
+			};
+
+			vccio_sd: LDO_REG4 {
+				regulator-name = "vccio_sd";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <3000000>;
+				regulator-max-microvolt = <3000000>;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <3000000>;
+				};
+			};
+
+			vcca3v0_codec: LDO_REG5 {
+				regulator-name = "vcca3v0_codec";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <3000000>;
+				regulator-max-microvolt = <3000000>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc_1v5: LDO_REG6 {
+				regulator-name = "vcc_1v5";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1500000>;
+				regulator-max-microvolt = <1500000>;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1500000>;
+				};
+			};
+
+			vcc0v9_hdmi: LDO_REG7 {
+				regulator-name = "vcc0v9_hdmi";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <900000>;
+				regulator-max-microvolt = <900000>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc_3v0: LDO_REG8 {
+				regulator-name = "vcc_3v0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <3000000>;
+				regulator-max-microvolt = <3000000>;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <3000000>;
+				};
+			};
+
+		};
+	};
+
+	vdd_cpu_b: regulator@40 {
+		compatible = "silergy,syr827";
+		reg = <0x40>;
+		fcs,suspend-voltage-selector = <1>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&vsel1_gpio>;
+		regulator-name = "vdd_cpu_b";
+		regulator-min-microvolt = <712500>;
+		regulator-max-microvolt = <1500000>;
+		regulator-ramp-delay = <1000>;
+		regulator-always-on;
+		regulator-boot-on;
+		vin-supply = <&vcc5v0_sys>;
+
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
+	};
+
+	vdd_gpu: regulator@41 {
+		compatible = "silergy,syr828";
+		reg = <0x41>;
+		fcs,suspend-voltage-selector = <1>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&vsel2_gpio>;
+		regulator-name = "vdd_gpu";
+		regulator-min-microvolt = <712500>;
+		regulator-max-microvolt = <1500000>;
+		regulator-ramp-delay = <1000>;
+		regulator-always-on;
+		regulator-boot-on;
+		vin-supply = <&vcc5v0_sys>;
+
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
+	};
+};
+
+&i2c1 {
+	i2c-scl-rising-time-ns = <300>;
+	i2c-scl-falling-time-ns = <15>;
+	status = "okay";
+};
+
+&i2c3 {
+	i2c-scl-rising-time-ns = <450>;
+	i2c-scl-falling-time-ns = <15>;
+	status = "okay";
+};
+
+&i2c4 {
+	i2c-scl-rising-time-ns = <600>;
+	i2c-scl-falling-time-ns = <20>;
+	status = "okay";
+};
+
+&i2s0 {
+	rockchip,playback-channels = <8>;
+	rockchip,capture-channels = <8>;
+	status = "okay";
+};
+
+&i2s1 {
+	rockchip,playback-channels = <2>;
+	rockchip,capture-channels = <2>;
+	status = "okay";
+};
+
+&i2s2 {
+	status = "okay";
+};
+
+&io_domains {
+	status = "okay";
+
+	bt656-supply = <&vcc1v8_dvp>;
+	audio-supply = <&vcc_1v8>;
+	sdmmc-supply = <&vccio_sd>;
+	gpio1830-supply = <&vcc_3v0>;
+};
+
+&pmu_io_domains {
+	status = "okay";
+	pmu1830-supply = <&vcc_3v0>;
+};
+
+&pinctrl {
+	bt {
+		bt_enable_h: bt-enable-h {
+			rockchip,pins = <0 RK_PB1 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+
+		bt_host_wake_l: bt-host-wake-l {
+			rockchip,pins = <0 RK_PA4 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+
+		bt_wake_l: bt-wake-l {
+			rockchip,pins = <2 RK_PD2 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	pmic {
+		pmic_int_l: pmic-int-l {
+			rockchip,pins = <1 RK_PC5 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+
+		vsel1_gpio: vsel1-gpio {
+			rockchip,pins = <1 RK_PC1 RK_FUNC_GPIO &pcfg_pull_down>;
+		};
+
+		vsel2_gpio: vsel2-gpio {
+			rockchip,pins = <1 RK_PB6 RK_FUNC_GPIO &pcfg_pull_down>;
+		};
+	};
+
+	usb2 {
+		vcc5v0_host_en: vcc5v0-host-en {
+			rockchip,pins = <2 RK_PA2 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	wifi {
+		wifi_enable_h: wifi-enable-h {
+			rockchip,pins =
+				<0 RK_PB2 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+
+		wifi_host_wake_l: wifi-host-wake-l {
+			rockchip,pins = <0 RK_PA3 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+};
+
+&pwm2 {
+	status = "okay";
+};
+
+&saradc {
+	status = "okay";
+
+	vref-supply = <&vcc_1v8>;
+};
+
+&sdio0 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	bus-width = <4>;
+	clock-frequency = <50000000>;
+	cap-sdio-irq;
+	cap-sd-highspeed;
+	keep-power-in-suspend;
+	mmc-pwrseq = <&sdio_pwrseq>;
+	non-removable;
+	pinctrl-names = "default";
+	pinctrl-0 = <&sdio0_bus4 &sdio0_cmd &sdio0_clk>;
+	sd-uhs-sdr104;
+	status = "okay";
+
+	brcmf: wifi@1 {
+		compatible = "brcm,bcm4329-fmac";
+		reg = <1>;
+		interrupt-parent = <&gpio0>;
+		interrupts = <RK_PA3 GPIO_ACTIVE_HIGH>;
+		interrupt-names = "host-wake";
+		pinctrl-names = "default";
+		pinctrl-0 = <&wifi_host_wake_l>;
+	};
+};
+
+&sdmmc {
+	bus-width = <4>;
+	cap-mmc-highspeed;
+	cap-sd-highspeed;
+	cd-gpios = <&gpio0 RK_PA7 GPIO_ACTIVE_LOW>;
+	disable-wp;
+	max-frequency = <150000000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&sdmmc_clk &sdmmc_cd &sdmmc_cmd &sdmmc_bus4>;
+	status = "okay";
+};
+
+&sdhci {
+	bus-width = <8>;
+	mmc-hs400-1_8v;
+	mmc-hs400-enhanced-strobe;
+	non-removable;
+	status = "okay";
+};
+
+&tcphy0 {
+	status = "okay";
+};
+
+&tcphy1 {
+	status = "okay";
+};
+
+&tsadc {
+	status = "okay";
+
+	/* tshut mode 0:CRU 1:GPIO */
+	rockchip,hw-tshut-mode = <1>;
+	/* tshut polarity 0:LOW 1:HIGH */
+	rockchip,hw-tshut-polarity = <1>;
+};
+
+&u2phy0 {
+	status = "okay";
+
+	u2phy0_otg: otg-port {
+		status = "okay";
+	};
+
+	u2phy0_host: host-port {
+		phy-supply = <&vcc5v0_host>;
+		status = "okay";
+	};
+};
+
+&u2phy1 {
+	status = "okay";
+
+	u2phy1_otg: otg-port {
+		status = "okay";
+	};
+
+	u2phy1_host: host-port {
+		phy-supply = <&vcc5v0_host>;
+		status = "okay";
+	};
+};
+
+&uart0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart0_xfer &uart0_cts &uart0_rts>;
+	status = "okay";
+
+	bluetooth {
+		compatible = "brcm,bcm43438-bt";
+		clocks = <&rk808 1>;
+		clock-names = "ext_clock";
+		device-wakeup-gpios = <&gpio2 RK_PD2 GPIO_ACTIVE_HIGH>;
+		host-wakeup-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_HIGH>;
+		shutdown-gpios = <&gpio0 RK_PB1 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&bt_host_wake_l &bt_wake_l &bt_enable_h>;
+	};
+};
+
+&uart2 {
+	status = "okay";
+};
+
+&usb_host0_ehci {
+	status = "okay";
+};
+
+&usb_host0_ohci {
+	status = "okay";
+};
+
+&usb_host1_ehci {
+	status = "okay";
+};
+
+&usb_host1_ohci {
+	status = "okay";
+};
+
+&usbdrd3_0 {
+	status = "okay";
+};
+
+&usbdrd_dwc3_0 {
+	status = "okay";
+	dr_mode = "otg";
+};
+
+&usbdrd3_1 {
+	status = "okay";
+};
+
+&usbdrd_dwc3_1 {
+	status = "okay";
+	dr_mode = "host";
+};
+
+&vopb {
+	status = "okay";
+};
+
+&vopb_mmu {
+	status = "okay";
+};
+
+&vopl {
+	status = "okay";
+};
+
+&vopl_mmu {
+	status = "okay";
+};
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH v3 02/11] dt-bindings: clock: imx-lpcg: add support to parse clocks from device tree
From: Shawn Guo @ 2019-08-03 13:50 UTC (permalink / raw)
  To: Dong Aisheng
  Cc: devicetree, sboyd, mturquette, Rob Herring, linux-imx, kernel,
	fabio.estevam, linux-clk, linux-arm-kernel
In-Reply-To: <1563289265-10977-3-git-send-email-aisheng.dong@nxp.com>

On Tue, Jul 16, 2019 at 11:00:56PM +0800, Dong Aisheng wrote:
> MX8QM and MX8QXP LPCG Clocks are mostly the same except they may reside
> in different subsystems across CPUs and also vary a bit on the availability.
> 
> Same as SCU clock, we want to move the clock definition into device tree
> which can fully decouple the dependency of Clock ID definition from device
> tree and make us be able to write a fully generic lpcg clock driver.
> 
> And we can also use the existence of clock nodes in device tree to address
> the device and clock availability differences across different SoCs.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---
> ChangeLog:
> v2->v3:
>  * no changes
> v1->v2:
>  * Update example
>  * Add power domain property
> ---
>  .../devicetree/bindings/clock/imx8qxp-lpcg.txt     | 34 ++++++++++++++++++----
>  1 file changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/imx8qxp-lpcg.txt b/Documentation/devicetree/bindings/clock/imx8qxp-lpcg.txt
> index 965cfa4..6fc2fd8 100644
> --- a/Documentation/devicetree/bindings/clock/imx8qxp-lpcg.txt
> +++ b/Documentation/devicetree/bindings/clock/imx8qxp-lpcg.txt
> @@ -11,6 +11,21 @@ enabled by these control bits, it might still not be running based
>  on the base resource.
>  
>  Required properties:
> +- compatible:		Should be one of:
> +			  "fsl,imx8qxp-lpcg"
> +			  "fsl,imx8qm-lpcg" followed by "fsl,imx8qxp-lpcg".
> +- reg:			Address and length of the register set.
> +- #clock-cells:		Should be 1. One LPCG supports multiple clocks.
> +- clocks:		Input parent clocks phandle array for each clock.
> +- bit-offset:		An integer array indicating the bit offset for each clock.

I guess that the driver should be able to figure bit offset from
'clock-indices' property.

> +- hw-autogate:		Boolean array indicating whether supports HW autogate for
> +			each clock.

Not sure why it needs to be a property in DT.  Or asking it different
way, when it should be true and when false?

Shawn

> +- clock-output-names:	Shall be the corresponding names of the outputs.
> +			NOTE this property must be specified in the same order
> +			as the clock bit-offset and hw-autogate property.
> +- power-domains:	Should contain the power domain used by this clock.
> +
> +Legacy binding (DEPRECATED):
>  - compatible:	Should be one of:
>  		  "fsl,imx8qxp-lpcg-adma",
>  		  "fsl,imx8qxp-lpcg-conn",
> @@ -33,10 +48,17 @@ Examples:
>  
>  #include <dt-bindings/clock/imx8qxp-clock.h>
>  
> -conn_lpcg: clock-controller@5b200000 {
> -	compatible = "fsl,imx8qxp-lpcg-conn";
> -	reg = <0x5b200000 0xb0000>;
> +sdhc0_lpcg: clock-controller@5b200000 {
> +	compatible = "fsl,imx8qxp-lpcg";
> +	reg = <0x5b200000 0x10000>;
>  	#clock-cells = <1>;
> +	clocks = <&sdhc0_clk IMX_SC_PM_CLK_PER>,
> +		 <&conn_ipg_clk>, <&conn_axi_clk>;
> +	bit-offset = <0 16 20>;
> +	clock-output-names = "sdhc0_lpcg_per_clk",
> +			     "sdhc0_lpcg_ipg_clk",
> +			     "sdhc0_lpcg_ahb_clk";
> +	power-domains = <&pd IMX_SC_R_SDHC_0>;
>  };
>  
>  usdhc1: mmc@5b010000 {
> @@ -44,8 +66,8 @@ usdhc1: mmc@5b010000 {
>  	interrupt-parent = <&gic>;
>  	interrupts = <GIC_SPI 232 IRQ_TYPE_LEVEL_HIGH>;
>  	reg = <0x5b010000 0x10000>;
> -	clocks = <&conn_lpcg IMX8QXP_CONN_LPCG_SDHC0_IPG_CLK>,
> -		 <&conn_lpcg IMX8QXP_CONN_LPCG_SDHC0_PER_CLK>,
> -		 <&conn_lpcg IMX8QXP_CONN_LPCG_SDHC0_HCLK>;
> +	clocks = <&sdhc0_lpcg 1>,
> +		 <&sdhc0_lpcg 0>,
> +		 <&sdhc0_lpcg 2>;
>  	clock-names = "ipg", "per", "ahb";
>  };
> -- 
> 2.7.4
> 

^ permalink raw reply

* [PATCH v4 0/6] qcom: add OCMEM support
From: Brian Masney @ 2019-08-03 14:20 UTC (permalink / raw)
  To: agross, robdclark, sean, robh+dt, bjorn.andersson
  Cc: airlied, daniel, mark.rutland, jonathan, linux-arm-msm,
	linux-kernel, dri-devel, freedreno, devicetree, jcrouse

This patch series adds support for Qualcomm's On Chip MEMory (OCMEM)
that is needed in order to support some a3xx and a4xx-based GPUs
upstream. This is based on Rob Clark's patch series that he submitted
in October 2015 and I am resubmitting updated patches with his
permission. See the individual patches for the changelog.

This was tested with the GPU on a LG Nexus 5 (hammerhead) phone and
this will work on other msm8974-based systems. For a summary of what
currently works upstream on the Nexus 5, see my status page at
https://masneyb.github.io/nexus-5-upstream/.

Brian Masney (4):
  dt-bindings: soc: qcom: add On Chip MEMory (OCMEM) bindings
  dt-bindings: display: msm: gmu: add optional ocmem property
  soc: qcom: add OCMEM driver
  drm/msm/gpu: add ocmem init/cleanup functions

Rob Clark (2):
  firmware: qcom: scm: add OCMEM lock/unlock interface
  firmware: qcom: scm: add support to restore secure config to
    qcm_scm-32

 .../devicetree/bindings/display/msm/gmu.txt   |  50 ++
 .../devicetree/bindings/sram/qcom,ocmem.yaml  |  96 ++++
 drivers/firmware/qcom_scm-32.c                |  52 ++-
 drivers/firmware/qcom_scm-64.c                |  12 +
 drivers/firmware/qcom_scm.c                   |  53 +++
 drivers/firmware/qcom_scm.h                   |   9 +
 drivers/gpu/drm/msm/Kconfig                   |   1 +
 drivers/gpu/drm/msm/adreno/a3xx_gpu.c         |  28 +-
 drivers/gpu/drm/msm/adreno/a3xx_gpu.h         |   3 +-
 drivers/gpu/drm/msm/adreno/a4xx_gpu.c         |  25 +-
 drivers/gpu/drm/msm/adreno/a4xx_gpu.h         |   3 +-
 drivers/gpu/drm/msm/adreno/adreno_gpu.c       |  40 ++
 drivers/gpu/drm/msm/adreno/adreno_gpu.h       |  10 +
 drivers/soc/qcom/Kconfig                      |  10 +
 drivers/soc/qcom/Makefile                     |   1 +
 drivers/soc/qcom/ocmem.c                      | 433 ++++++++++++++++++
 include/linux/qcom_scm.h                      |  26 ++
 include/soc/qcom/ocmem.h                      |  62 +++
 18 files changed, 869 insertions(+), 45 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sram/qcom,ocmem.yaml
 create mode 100644 drivers/soc/qcom/ocmem.c
 create mode 100644 include/soc/qcom/ocmem.h

-- 
2.21.0

^ permalink raw reply

* [PATCH v4 1/6] dt-bindings: soc: qcom: add On Chip MEMory (OCMEM) bindings
From: Brian Masney @ 2019-08-03 14:20 UTC (permalink / raw)
  To: agross-DgEjT+Ai2ygdnm+yROfE0A, robdclark-Re5JQEeQqe8AvxtiuMwx3w,
	sean-p7yTbzM4H96eqtR555YLDQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A
  Cc: mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	jonathan-eSc4qw6YbEQ, airlied-cv59FeDIM0c,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	jcrouse-sgV2jX0FEOL9JmXXK+q4OQ, daniel-/w4YWyX8dFk,
	freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
In-Reply-To: <20190803142026.9647-1-masneyb-1iNe0GrtECGEi8DpZVb4nw@public.gmane.org>

Add device tree bindings for the On Chip Memory (OCMEM) that is present
on some Qualcomm Snapdragon SoCs.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
Changes since v3
- add ranges property
- remove unnecessary literal block |
- add #address-cells and #size-cells to binding
- rename path devicetree/bindings/sram/qcom/ to devicetree/bindings/sram/ since
  this is the only qcom binding in the sram namespace. That was a holdover from
  when I originally put this in the soc namespace.

Changes since v2:
- Add *-sram node and gmu-sram to example.

Changes since v1:
- Rename qcom,ocmem-msm8974 to qcom,msm8974-ocmem
- Renamed reg-names to ctrl and mem
- update hardware description
- moved from soc to sram namespace in the device tree bindings

 .../devicetree/bindings/sram/qcom,ocmem.yaml  | 96 +++++++++++++++++++
 1 file changed, 96 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sram/qcom,ocmem.yaml

diff --git a/Documentation/devicetree/bindings/sram/qcom,ocmem.yaml b/Documentation/devicetree/bindings/sram/qcom,ocmem.yaml
new file mode 100644
index 000000000000..1bb386fffa01
--- /dev/null
+++ b/Documentation/devicetree/bindings/sram/qcom,ocmem.yaml
@@ -0,0 +1,96 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sram/qcom/qcom,ocmem.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: On Chip Memory (OCMEM) that is present on some Qualcomm Snapdragon SoCs.
+
+maintainers:
+  - Brian Masney <masneyb@onstation.org>
+
+description: |
+  The On Chip Memory (OCMEM) is typically used by the GPU, camera/video, and
+  audio components on some Snapdragon SoCs.
+
+properties:
+  compatible:
+    const: qcom,msm8974-ocmem
+
+  reg:
+    items:
+      - description: Control registers
+      - description: OCMEM address range
+
+  reg-names:
+    items:
+      - const: ctrl
+      - const: mem
+
+  clocks:
+    items:
+      - description: Core clock
+      - description: Interface clock
+
+  clock-names:
+    items:
+      - const: core
+      - const: iface
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - clocks
+  - clock-names
+  - '#address-cells'
+  - '#size-cells'
+
+patternProperties:
+  "^.+-sram$":
+    type: object
+    description: A region of reserved memory.
+
+    properties:
+      reg:
+        maxItems: 1
+
+      ranges:
+        maxItems: 1
+
+    required:
+      - reg
+      - ranges
+
+examples:
+  - |
+      #include <dt-bindings/clock/qcom,rpmcc.h>
+      #include <dt-bindings/clock/qcom,mmcc-msm8974.h>
+
+      ocmem: ocmem@fdd00000 {
+        compatible = "qcom,msm8974-ocmem";
+
+        reg = <0xfdd00000 0x2000>,
+              <0xfec00000 0x180000>;
+        reg-names = "ctrl",
+                    "mem";
+
+        clocks = <&rpmcc RPM_SMD_OCMEMGX_CLK>,
+                 <&mmcc OCMEMCX_OCMEMNOC_CLK>;
+        clock-names = "core",
+                      "iface";
+
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        gmu-sram@0 {
+                reg = <0x0 0x100000>;
+                ranges = <0 0 0xfec00000 0x100000>;
+        };
+      };
-- 
2.21.0

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

^ permalink raw reply related

* [PATCH v4 2/6] dt-bindings: display: msm: gmu: add optional ocmem property
From: Brian Masney @ 2019-08-03 14:20 UTC (permalink / raw)
  To: agross-DgEjT+Ai2ygdnm+yROfE0A, robdclark-Re5JQEeQqe8AvxtiuMwx3w,
	sean-p7yTbzM4H96eqtR555YLDQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A
  Cc: mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	jonathan-eSc4qw6YbEQ, airlied-cv59FeDIM0c,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	jcrouse-sgV2jX0FEOL9JmXXK+q4OQ, daniel-/w4YWyX8dFk,
	freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
In-Reply-To: <20190803142026.9647-1-masneyb-1iNe0GrtECGEi8DpZVb4nw@public.gmane.org>

Some A3xx and A4xx Adreno GPUs do not have GMEM inside the GPU core and
must use the On Chip MEMory (OCMEM) in order to be functional. Add the
optional ocmem property to the Adreno Graphics Management Unit bindings.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
Changes since v3:
- correct link to qcom,ocmem.yaml

Changes since v2:
- Add a3xx example with OCMEM

Changes since v1:
- None

 .../devicetree/bindings/display/msm/gmu.txt   | 50 +++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/msm/gmu.txt b/Documentation/devicetree/bindings/display/msm/gmu.txt
index 90af5b0a56a9..672d557caba4 100644
--- a/Documentation/devicetree/bindings/display/msm/gmu.txt
+++ b/Documentation/devicetree/bindings/display/msm/gmu.txt
@@ -31,6 +31,10 @@ Required properties:
 - iommus: phandle to the adreno iommu
 - operating-points-v2: phandle to the OPP operating points
 
+Optional properties:
+- ocmem: phandle to the On Chip Memory (OCMEM) that's present on some Snapdragon
+         SoCs. See Documentation/devicetree/bindings/sram/qcom,ocmem.yaml.
+
 Example:
 
 / {
@@ -63,3 +67,49 @@ Example:
 		operating-points-v2 = <&gmu_opp_table>;
 	};
 };
+
+a3xx example with OCMEM support:
+
+/ {
+	...
+
+	gpu: adreno@fdb00000 {
+		compatible = "qcom,adreno-330.2",
+		             "qcom,adreno";
+		reg = <0xfdb00000 0x10000>;
+		reg-names = "kgsl_3d0_reg_memory";
+		interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "kgsl_3d0_irq";
+		clock-names = "core",
+		              "iface",
+		              "mem_iface";
+		clocks = <&mmcc OXILI_GFX3D_CLK>,
+		         <&mmcc OXILICX_AHB_CLK>,
+		         <&mmcc OXILICX_AXI_CLK>;
+		ocmem = <&ocmem>;
+		power-domains = <&mmcc OXILICX_GDSC>;
+		operating-points-v2 = <&gpu_opp_table>;
+		iommus = <&gpu_iommu 0>;
+	};
+
+	ocmem: ocmem@fdd00000 {
+		compatible = "qcom,msm8974-ocmem";
+
+		reg = <0xfdd00000 0x2000>,
+		      <0xfec00000 0x180000>;
+		reg-names = "ctrl",
+		             "mem";
+
+		clocks = <&rpmcc RPM_SMD_OCMEMGX_CLK>,
+		         <&mmcc OCMEMCX_OCMEMNOC_CLK>;
+		clock-names = "core",
+		              "iface";
+
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		gmu-sram@0 {
+			reg = <0x0 0x100000>;
+		};
+	};
+};
-- 
2.21.0

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

^ permalink raw reply related

* [PATCH v4 3/6] firmware: qcom: scm: add OCMEM lock/unlock interface
From: Brian Masney @ 2019-08-03 14:20 UTC (permalink / raw)
  To: agross-DgEjT+Ai2ygdnm+yROfE0A, robdclark-Re5JQEeQqe8AvxtiuMwx3w,
	sean-p7yTbzM4H96eqtR555YLDQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A
  Cc: mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	jonathan-eSc4qw6YbEQ, airlied-cv59FeDIM0c,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	jcrouse-sgV2jX0FEOL9JmXXK+q4OQ, daniel-/w4YWyX8dFk,
	freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
In-Reply-To: <20190803142026.9647-1-masneyb-1iNe0GrtECGEi8DpZVb4nw@public.gmane.org>

From: Rob Clark <robdclark@gmail.com>

Add support for the OCMEM lock/unlock interface that is needed by the
On Chip MEMory (OCMEM) that is present on some Snapdragon devices.

Signed-off-by: Rob Clark <robdclark@gmail.com>
[masneyb@onstation.org: ported to latest kernel; minor reformatting.]
Signed-off-by: Brian Masney <masneyb@onstation.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
Rob's last version of this patch:
https://patchwork.kernel.org/patch/7340711/

Changes since v3:
- None

Changes since v2:
- None

Changes since v1:
- None

 drivers/firmware/qcom_scm-32.c | 35 +++++++++++++++++++++++++++++
 drivers/firmware/qcom_scm-64.c | 12 ++++++++++
 drivers/firmware/qcom_scm.c    | 40 ++++++++++++++++++++++++++++++++++
 drivers/firmware/qcom_scm.h    |  9 ++++++++
 include/linux/qcom_scm.h       | 15 +++++++++++++
 5 files changed, 111 insertions(+)

diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
index 215061c581e1..4c2514e5e249 100644
--- a/drivers/firmware/qcom_scm-32.c
+++ b/drivers/firmware/qcom_scm-32.c
@@ -442,6 +442,41 @@ int __qcom_scm_hdcp_req(struct device *dev, struct qcom_scm_hdcp_req *req,
 		req, req_cnt * sizeof(*req), resp, sizeof(*resp));
 }
 
+int __qcom_scm_ocmem_lock(struct device *dev, u32 id, u32 offset, u32 size,
+			  u32 mode)
+{
+	struct ocmem_tz_lock {
+		__le32 id;
+		__le32 offset;
+		__le32 size;
+		__le32 mode;
+	} request;
+
+	request.id = cpu_to_le32(id);
+	request.offset = cpu_to_le32(offset);
+	request.size = cpu_to_le32(size);
+	request.mode = cpu_to_le32(mode);
+
+	return qcom_scm_call(dev, QCOM_SCM_OCMEM_SVC, QCOM_SCM_OCMEM_LOCK_CMD,
+			     &request, sizeof(request), NULL, 0);
+}
+
+int __qcom_scm_ocmem_unlock(struct device *dev, u32 id, u32 offset, u32 size)
+{
+	struct ocmem_tz_unlock {
+		__le32 id;
+		__le32 offset;
+		__le32 size;
+	} request;
+
+	request.id = cpu_to_le32(id);
+	request.offset = cpu_to_le32(offset);
+	request.size = cpu_to_le32(size);
+
+	return qcom_scm_call(dev, QCOM_SCM_OCMEM_SVC, QCOM_SCM_OCMEM_UNLOCK_CMD,
+			     &request, sizeof(request), NULL, 0);
+}
+
 void __qcom_scm_init(void)
 {
 }
diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c
index 91d5ad7cf58b..c3a3d9874def 100644
--- a/drivers/firmware/qcom_scm-64.c
+++ b/drivers/firmware/qcom_scm-64.c
@@ -241,6 +241,18 @@ int __qcom_scm_hdcp_req(struct device *dev, struct qcom_scm_hdcp_req *req,
 	return ret;
 }
 
+int __qcom_scm_ocmem_lock(struct device *dev, uint32_t id, uint32_t offset,
+			  uint32_t size, uint32_t mode)
+{
+	return -ENOTSUPP;
+}
+
+int __qcom_scm_ocmem_unlock(struct device *dev, uint32_t id, uint32_t offset,
+			    uint32_t size)
+{
+	return -ENOTSUPP;
+}
+
 void __qcom_scm_init(void)
 {
 	u64 cmd;
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 4802ab170fe5..7e285ff3961d 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -191,6 +191,46 @@ bool qcom_scm_pas_supported(u32 peripheral)
 }
 EXPORT_SYMBOL(qcom_scm_pas_supported);
 
+/**
+ * qcom_scm_ocmem_lock_available() - is OCMEM lock/unlock interface available
+ */
+bool qcom_scm_ocmem_lock_available(void)
+{
+	return __qcom_scm_is_call_available(__scm->dev, QCOM_SCM_OCMEM_SVC,
+					    QCOM_SCM_OCMEM_LOCK_CMD);
+}
+EXPORT_SYMBOL(qcom_scm_ocmem_lock_available);
+
+/**
+ * qcom_scm_ocmem_lock() - call OCMEM lock interface to assign an OCMEM
+ * region to the specified initiator
+ *
+ * @id:     tz initiator id
+ * @offset: OCMEM offset
+ * @size:   OCMEM size
+ * @mode:   access mode (WIDE/NARROW)
+ */
+int qcom_scm_ocmem_lock(enum qcom_scm_ocmem_client id, u32 offset, u32 size,
+			u32 mode)
+{
+	return __qcom_scm_ocmem_lock(__scm->dev, id, offset, size, mode);
+}
+EXPORT_SYMBOL(qcom_scm_ocmem_lock);
+
+/**
+ * qcom_scm_ocmem_unlock() - call OCMEM unlock interface to release an OCMEM
+ * region from the specified initiator
+ *
+ * @id:     tz initiator id
+ * @offset: OCMEM offset
+ * @size:   OCMEM size
+ */
+int qcom_scm_ocmem_unlock(enum qcom_scm_ocmem_client id, u32 offset, u32 size)
+{
+	return __qcom_scm_ocmem_unlock(__scm->dev, id, offset, size);
+}
+EXPORT_SYMBOL(qcom_scm_ocmem_unlock);
+
 /**
  * qcom_scm_pas_init_image() - Initialize peripheral authentication service
  *			       state machine for a given peripheral, using the
diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
index 99506bd873c0..ef293ee67ec1 100644
--- a/drivers/firmware/qcom_scm.h
+++ b/drivers/firmware/qcom_scm.h
@@ -42,6 +42,15 @@ extern int __qcom_scm_hdcp_req(struct device *dev,
 
 extern void __qcom_scm_init(void);
 
+#define QCOM_SCM_OCMEM_SVC			0xf
+#define QCOM_SCM_OCMEM_LOCK_CMD		0x1
+#define QCOM_SCM_OCMEM_UNLOCK_CMD		0x2
+
+extern int __qcom_scm_ocmem_lock(struct device *dev, u32 id, u32 offset,
+				 u32 size, u32 mode);
+extern int __qcom_scm_ocmem_unlock(struct device *dev, u32 id, u32 offset,
+				   u32 size);
+
 #define QCOM_SCM_SVC_PIL		0x2
 #define QCOM_SCM_PAS_INIT_IMAGE_CMD	0x1
 #define QCOM_SCM_PAS_MEM_SETUP_CMD	0x2
diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
index 2d5eff506e13..b49b734d662c 100644
--- a/include/linux/qcom_scm.h
+++ b/include/linux/qcom_scm.h
@@ -24,6 +24,16 @@ struct qcom_scm_vmperm {
 	int perm;
 };
 
+enum qcom_scm_ocmem_client {
+	QCOM_SCM_OCMEM_UNUSED_ID = 0x0,
+	QCOM_SCM_OCMEM_GRAPHICS_ID,
+	QCOM_SCM_OCMEM_VIDEO_ID,
+	QCOM_SCM_OCMEM_LP_AUDIO_ID,
+	QCOM_SCM_OCMEM_SENSORS_ID,
+	QCOM_SCM_OCMEM_OTHER_OS_ID,
+	QCOM_SCM_OCMEM_DEBUG_ID,
+};
+
 #define QCOM_SCM_VMID_HLOS       0x3
 #define QCOM_SCM_VMID_MSS_MSA    0xF
 #define QCOM_SCM_VMID_WLAN       0x18
@@ -41,6 +51,11 @@ extern bool qcom_scm_is_available(void);
 extern bool qcom_scm_hdcp_available(void);
 extern int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt,
 			     u32 *resp);
+extern bool qcom_scm_ocmem_lock_available(void);
+extern int qcom_scm_ocmem_lock(enum qcom_scm_ocmem_client id, u32 offset,
+			       u32 size, u32 mode);
+extern int qcom_scm_ocmem_unlock(enum qcom_scm_ocmem_client id, u32 offset,
+				 u32 size);
 extern bool qcom_scm_pas_supported(u32 peripheral);
 extern int qcom_scm_pas_init_image(u32 peripheral, const void *metadata,
 				   size_t size);
-- 
2.21.0

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

^ permalink raw reply related

* [PATCH v4 4/6] firmware: qcom: scm: add support to restore secure config to qcm_scm-32
From: Brian Masney @ 2019-08-03 14:20 UTC (permalink / raw)
  To: agross-DgEjT+Ai2ygdnm+yROfE0A, robdclark-Re5JQEeQqe8AvxtiuMwx3w,
	sean-p7yTbzM4H96eqtR555YLDQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A
  Cc: mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	jonathan-eSc4qw6YbEQ, airlied-cv59FeDIM0c,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	jcrouse-sgV2jX0FEOL9JmXXK+q4OQ, daniel-/w4YWyX8dFk,
	freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
In-Reply-To: <20190803142026.9647-1-masneyb-1iNe0GrtECGEi8DpZVb4nw@public.gmane.org>

From: Rob Clark <robdclark@gmail.com>

Add support to restore the secure configuration for qcm_scm-32.c. This
is needed by the On Chip MEMory (OCMEM) that is present on some
Snapdragon devices.

Signed-off-by: Rob Clark <robdclark@gmail.com>
[masneyb@onstation.org: ported to latest kernel; set ctx_bank_num to
 spare parameter.]
Signed-off-by: Brian Masney <masneyb@onstation.org>
---
Changes since v3:
- None

Changes since v2:
- None

Changes since v1:
- Use existing __qcom_scm_restore_sec_cfg() function stub in
  qcom_scm-32.c that was unimplemented
- Set the cfg.ctx_bank_num to the spare function parameter. It was
  previously set to the device_id.

 drivers/firmware/qcom_scm-32.c | 17 ++++++++++++++++-
 drivers/firmware/qcom_scm.c    | 13 +++++++++++++
 include/linux/qcom_scm.h       | 11 +++++++++++
 3 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
index 4c2514e5e249..5d90b7f5ab5a 100644
--- a/drivers/firmware/qcom_scm-32.c
+++ b/drivers/firmware/qcom_scm-32.c
@@ -617,7 +617,22 @@ int __qcom_scm_assign_mem(struct device *dev, phys_addr_t mem_region,
 int __qcom_scm_restore_sec_cfg(struct device *dev, u32 device_id,
 			       u32 spare)
 {
-	return -ENODEV;
+	struct msm_scm_sec_cfg {
+		__le32 id;
+		__le32 ctx_bank_num;
+	} cfg;
+	int ret, scm_ret = 0;
+
+	cfg.id = cpu_to_le32(device_id);
+	cfg.ctx_bank_num = cpu_to_le32(spare);
+
+	ret = qcom_scm_call(dev, QCOM_SCM_SVC_MP, QCOM_SCM_RESTORE_SEC_CFG,
+			    &cfg, sizeof(cfg), &scm_ret, sizeof(scm_ret));
+
+	if (ret || scm_ret)
+		return ret ? ret : -EINVAL;
+
+	return 0;
 }
 
 int __qcom_scm_iommu_secure_ptbl_size(struct device *dev, u32 spare,
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 7e285ff3961d..27c1d98a34e6 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -367,6 +367,19 @@ static const struct reset_control_ops qcom_scm_pas_reset_ops = {
 	.deassert = qcom_scm_pas_reset_deassert,
 };
 
+/**
+ * qcom_scm_restore_sec_cfg_available() - Check if secure environment
+ * supports restore security config interface.
+ *
+ * Return true if restore-cfg interface is supported, false if not.
+ */
+bool qcom_scm_restore_sec_cfg_available(void)
+{
+	return __qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_MP,
+					    QCOM_SCM_RESTORE_SEC_CFG);
+}
+EXPORT_SYMBOL(qcom_scm_restore_sec_cfg_available);
+
 int qcom_scm_restore_sec_cfg(u32 device_id, u32 spare)
 {
 	return __qcom_scm_restore_sec_cfg(__scm->dev, device_id, spare);
diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
index b49b734d662c..04382e1798e4 100644
--- a/include/linux/qcom_scm.h
+++ b/include/linux/qcom_scm.h
@@ -34,6 +34,16 @@ enum qcom_scm_ocmem_client {
 	QCOM_SCM_OCMEM_DEBUG_ID,
 };
 
+enum qcom_scm_sec_dev_id {
+	QCOM_SCM_MDSS_DEV_ID    = 1,
+	QCOM_SCM_OCMEM_DEV_ID   = 5,
+	QCOM_SCM_PCIE0_DEV_ID   = 11,
+	QCOM_SCM_PCIE1_DEV_ID   = 12,
+	QCOM_SCM_GFX_DEV_ID     = 18,
+	QCOM_SCM_UFS_DEV_ID     = 19,
+	QCOM_SCM_ICE_DEV_ID     = 20,
+};
+
 #define QCOM_SCM_VMID_HLOS       0x3
 #define QCOM_SCM_VMID_MSS_MSA    0xF
 #define QCOM_SCM_VMID_WLAN       0x18
@@ -70,6 +80,7 @@ extern int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz,
 extern void qcom_scm_cpu_power_down(u32 flags);
 extern u32 qcom_scm_get_version(void);
 extern int qcom_scm_set_remote_state(u32 state, u32 id);
+extern bool qcom_scm_restore_sec_cfg_available(void);
 extern int qcom_scm_restore_sec_cfg(u32 device_id, u32 spare);
 extern int qcom_scm_iommu_secure_ptbl_size(u32 spare, size_t *size);
 extern int qcom_scm_iommu_secure_ptbl_init(u64 addr, u32 size, u32 spare);
-- 
2.21.0

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

^ permalink raw reply related

* [PATCH v4 5/6] soc: qcom: add OCMEM driver
From: Brian Masney @ 2019-08-03 14:20 UTC (permalink / raw)
  To: agross-DgEjT+Ai2ygdnm+yROfE0A, robdclark-Re5JQEeQqe8AvxtiuMwx3w,
	sean-p7yTbzM4H96eqtR555YLDQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A
  Cc: mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	jonathan-eSc4qw6YbEQ, airlied-cv59FeDIM0c,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	jcrouse-sgV2jX0FEOL9JmXXK+q4OQ, daniel-/w4YWyX8dFk,
	freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
In-Reply-To: <20190803142026.9647-1-masneyb-1iNe0GrtECGEi8DpZVb4nw@public.gmane.org>

The OCMEM driver handles allocation and configuration of the On Chip
MEMory that is present on some Snapdragon SoCs. Devices which have
OCMEM do not have GMEM inside the GPU core, so the GPU must instead
use OCMEM to be functional. Since the GPU is currently the only OCMEM
user with an upstream driver, this is just a minimal implementation
sufficient for statically allocating to the GPU it's chunk of OCMEM.

This driver currently does not read the gmu-sram node that is described
in the device tree bindings. The starting memory address of the GPU's
reserved memory region is hardcoded to zero to match what the hardware
expects. The driver can be updated to read the reserved memory regions
from device tree once other users of OCMEM are added upstream.

Signed-off-by: Brian Masney <masneyb@onstation.org>
Co-developed-by: Rob Clark <robdclark@gmail.com>
Signed-off-by: Rob Clark <robdclark@gmail.com>
---
Changes since v3:
- None

Changes since v2
- Changed static inline stubs return -ENODEV when OCMEM is not
  configured into the kernel.

Changes since v1:
- ocmem_allocate(): check for alignment and minimum allocation size.
  The 64K values came from the downstream MSM kernel sources.
- add locking to memory allocations based on the client
- use clk_bulk_*() functions
- rename qcom,ocmem-msm8974 to qcom,msm8974-ocmem
- rename reg-names to ctrl and mem
- remove ocmem.xml.h file; use FIELD_PREP() instead for some nice cleanups
- add static inline noop versions of public-facing functions when ocmem
  is disabled to remove #ifdefs in adrenu_gpu.c
- use unsigned long for memory addresses
- move ocmem_dev_remove() below _probe() function
- remove error check from platform_get_resource_byname for ctrl resource
- add MODULE_DESCRIPTION() and MODULE_LICENSE()
- add description to top of ocmem.[ch]
- correct thin mode bit in update_ocmem()
- add 'WARN_ON(client != OCMEM_GRAPHICS)' to device_address()
- make of_get_ocmem return error codes via ERR_PTR instead of NULL
- ocmem_{allocate,free} - WARN_ON() if client != OCMEM_GRAPHICS.
  Simplify if statements.
- allow NULL to be passed into ocmem_free
- remove unnecessary initialization of i in update_ocmem()
- add dev_dbg to ocmem_allocate

Changes since Rob's last version of this patch from 2015:
https://patchwork.kernel.org/patch/7379801/
- reformatted driver to allow multiple instances
- updated logging of error paths during device probing
- remove unused psgsc_ctrl
- remove _clk from clock names
- propagate error code from devm_ioremap_resource()
- use device_get_match_data()
- SPDX license tags
- remove QCOM_SMD in Kconfig
- select ARCH_QCOM in Kconfig
- select ARCH_QCOM in Kconfig
- select QCOM_SCM in Kconfig
- longer description in Kconfig

 drivers/soc/qcom/Kconfig  |  10 +
 drivers/soc/qcom/Makefile |   1 +
 drivers/soc/qcom/ocmem.c  | 433 ++++++++++++++++++++++++++++++++++++++
 include/soc/qcom/ocmem.h  |  62 ++++++
 4 files changed, 506 insertions(+)
 create mode 100644 drivers/soc/qcom/ocmem.c
 create mode 100644 include/soc/qcom/ocmem.h

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index a6d1bfb17279..d18eb83b10da 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -74,6 +74,16 @@ config QCOM_MDT_LOADER
 	tristate
 	select QCOM_SCM
 
+config QCOM_OCMEM
+	tristate "Qualcomm On Chip Memory (OCMEM) driver"
+	depends on ARCH_QCOM
+	select QCOM_SCM
+	help
+          The On Chip Memory (OCMEM) allocator allows various clients to
+          allocate memory from OCMEM based on performance, latency and power
+          requirements. This is typically used by the GPU, camera/video, and
+          audio components on some Snapdragon SoCs.
+
 config QCOM_PM
 	bool "Qualcomm Power Management"
 	depends on ARCH_QCOM && !ARM64
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index eeb088beb15f..dfc378014a33 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o
 obj-$(CONFIG_QCOM_GLINK_SSR) +=	glink_ssr.o
 obj-$(CONFIG_QCOM_GSBI)	+=	qcom_gsbi.o
 obj-$(CONFIG_QCOM_MDT_LOADER)	+= mdt_loader.o
+obj-$(CONFIG_QCOM_OCMEM)	+= ocmem.o
 obj-$(CONFIG_QCOM_PM)	+=	spm.o
 obj-$(CONFIG_QCOM_QMI_HELPERS)	+= qmi_helpers.o
 qmi_helpers-y	+= qmi_encdec.o qmi_interface.o
diff --git a/drivers/soc/qcom/ocmem.c b/drivers/soc/qcom/ocmem.c
new file mode 100644
index 000000000000..7c28ad3108a6
--- /dev/null
+++ b/drivers/soc/qcom/ocmem.c
@@ -0,0 +1,433 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * The On Chip Memory (OCMEM) allocator allows various clients to allocate
+ * memory from OCMEM based on performance, latency and power requirements.
+ * This is typically used by the GPU, camera/video, and audio components on
+ * some Snapdragon SoCs.
+ *
+ * Copyright (C) 2019 Brian Masney <masneyb@onstation.org>
+ * Copyright (C) 2015 Red Hat. Author: Rob Clark <robdclark@gmail.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/qcom_scm.h>
+#include <linux/sizes.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <soc/qcom/ocmem.h>
+
+enum region_mode {
+	WIDE_MODE = 0x0,
+	THIN_MODE,
+	MODE_DEFAULT = WIDE_MODE,
+};
+
+enum ocmem_macro_state {
+	PASSTHROUGH = 0,
+	PERI_ON = 1,
+	CORE_ON = 2,
+	CLK_OFF = 4,
+};
+
+struct ocmem_region {
+	bool interleaved;
+	enum region_mode mode;
+	unsigned int num_macros;
+	enum ocmem_macro_state macro_state[4];
+	unsigned long macro_size;
+	unsigned long region_size;
+};
+
+struct ocmem_config {
+	uint8_t num_regions;
+	unsigned long macro_size;
+};
+
+struct ocmem {
+	struct device *dev;
+	const struct ocmem_config *config;
+	struct resource *memory;
+	void __iomem *mmio;
+	unsigned int num_ports;
+	unsigned int num_macros;
+	bool interleaved;
+	struct ocmem_region *regions;
+	unsigned long active_allocations;
+};
+
+#define OCMEM_MIN_ALIGN				SZ_64K
+#define OCMEM_MIN_ALLOC				SZ_64K
+
+#define OCMEM_REG_HW_VERSION			0x00000000
+#define OCMEM_REG_HW_PROFILE			0x00000004
+
+#define OCMEM_REG_REGION_MODE_CTL		0x00001000
+#define OCMEM_REGION_MODE_CTL_REG0_THIN		0x00000001
+#define OCMEM_REGION_MODE_CTL_REG1_THIN		0x00000002
+#define OCMEM_REGION_MODE_CTL_REG2_THIN		0x00000004
+#define OCMEM_REGION_MODE_CTL_REG3_THIN		0x00000008
+
+#define OCMEM_REG_GFX_MPU_START			0x00001004
+#define OCMEM_REG_GFX_MPU_END			0x00001008
+
+#define OCMEM_HW_PROFILE_NUM_PORTS(val)		FIELD_PREP(0x0000000f, (val))
+#define OCMEM_HW_PROFILE_NUM_MACROS(val)	FIELD_PREP(0x00003f00, (val))
+
+#define OCMEM_HW_PROFILE_LAST_REGN_HALFSIZE	0x00010000
+#define OCMEM_HW_PROFILE_INTERLEAVING		0x00020000
+#define OCMEM_REG_GEN_STATUS			0x0000000c
+
+#define OCMEM_REG_PSGSC_STATUS			0x00000038
+#define OCMEM_REG_PSGSC_CTL(i0)			(0x0000003c + 0x1*(i0))
+
+#define OCMEM_PSGSC_CTL_MACRO0_MODE(val)	FIELD_PREP(0x00000007, (val))
+#define OCMEM_PSGSC_CTL_MACRO1_MODE(val)	FIELD_PREP(0x00000070, (val))
+#define OCMEM_PSGSC_CTL_MACRO2_MODE(val)	FIELD_PREP(0x00000700, (val))
+#define OCMEM_PSGSC_CTL_MACRO3_MODE(val)	FIELD_PREP(0x00007000, (val))
+
+#define OCMEM_CLK_CORE_IDX			0
+static struct clk_bulk_data ocmem_clks[] = {
+	{
+		.id = "core",
+	},
+	{
+		.id = "iface",
+	},
+};
+
+static inline void ocmem_write(struct ocmem *ocmem, u32 reg, u32 data)
+{
+	writel(data, ocmem->mmio + reg);
+}
+
+static inline u32 ocmem_read(struct ocmem *ocmem, u32 reg)
+{
+	return readl(ocmem->mmio + reg);
+}
+
+static void update_ocmem(struct ocmem *ocmem)
+{
+	uint32_t region_mode_ctrl = 0x0;
+	int i;
+
+	if (!qcom_scm_ocmem_lock_available()) {
+		for (i = 0; i < ocmem->config->num_regions; i++) {
+			struct ocmem_region *region = &ocmem->regions[i];
+
+			if (region->mode == THIN_MODE)
+				region_mode_ctrl |= BIT(i);
+		}
+
+		dev_dbg(ocmem->dev, "ocmem_region_mode_control %x\n",
+			region_mode_ctrl);
+		ocmem_write(ocmem, OCMEM_REG_REGION_MODE_CTL, region_mode_ctrl);
+	}
+
+	for (i = 0; i < ocmem->config->num_regions; i++) {
+		struct ocmem_region *region = &ocmem->regions[i];
+		u32 data;
+
+		data = OCMEM_PSGSC_CTL_MACRO0_MODE(region->macro_state[0]) |
+			OCMEM_PSGSC_CTL_MACRO1_MODE(region->macro_state[1]) |
+			OCMEM_PSGSC_CTL_MACRO2_MODE(region->macro_state[2]) |
+			OCMEM_PSGSC_CTL_MACRO3_MODE(region->macro_state[3]);
+
+		ocmem_write(ocmem, OCMEM_REG_PSGSC_CTL(i), data);
+	}
+}
+
+static unsigned long phys_to_offset(struct ocmem *ocmem,
+				    unsigned long addr)
+{
+	if (addr < ocmem->memory->start || addr >= ocmem->memory->end)
+		return 0;
+
+	return addr - ocmem->memory->start;
+}
+
+static unsigned long device_address(struct ocmem *ocmem,
+				    enum ocmem_client client,
+				    unsigned long addr)
+{
+	WARN_ON(client != OCMEM_GRAPHICS);
+
+	/* TODO: gpu uses phys_to_offset, but others do not.. */
+	return phys_to_offset(ocmem, addr);
+}
+
+static void update_range(struct ocmem *ocmem, struct ocmem_buf *buf,
+			 enum ocmem_macro_state mstate, enum region_mode rmode)
+{
+	unsigned long offset = 0;
+	int i, j;
+
+	for (i = 0; i < ocmem->config->num_regions; i++) {
+		struct ocmem_region *region = &ocmem->regions[i];
+
+		if (buf->offset <= offset && offset < buf->offset + buf->len)
+			region->mode = rmode;
+
+		for (j = 0; j < region->num_macros; j++) {
+			if (buf->offset <= offset &&
+			    offset < buf->offset + buf->len)
+				region->macro_state[j] = mstate;
+
+			offset += region->macro_size;
+		}
+	}
+
+	update_ocmem(ocmem);
+}
+
+struct ocmem *of_get_ocmem(struct device *dev)
+{
+	struct platform_device *pdev;
+	struct device_node *devnode;
+
+	devnode = of_parse_phandle(dev->of_node, "ocmem", 0);
+	if (!devnode) {
+		dev_err(dev, "Cannot look up ocmem phandle\n");
+		return ERR_PTR(-ENODEV);
+	}
+
+	pdev = of_find_device_by_node(devnode);
+	if (!pdev) {
+		dev_err(dev, "Cannot find device node %s\n", devnode->name);
+		return ERR_PTR(-EPROBE_DEFER);
+	}
+
+	return platform_get_drvdata(pdev);
+}
+EXPORT_SYMBOL(of_get_ocmem);
+
+struct ocmem_buf *ocmem_allocate(struct ocmem *ocmem, enum ocmem_client client,
+				 unsigned long size)
+{
+	struct ocmem_buf *buf;
+	int ret;
+
+	/* TODO: add support for other clients... */
+	if (WARN_ON(client != OCMEM_GRAPHICS))
+		return ERR_PTR(-ENODEV);
+
+	if (size < OCMEM_MIN_ALLOC || !IS_ALIGNED(size, OCMEM_MIN_ALIGN))
+		return ERR_PTR(-EINVAL);
+
+	if (test_and_set_bit_lock(BIT(client), &ocmem->active_allocations))
+		return ERR_PTR(-EBUSY);
+
+	buf = kzalloc(sizeof(*buf), GFP_KERNEL);
+	if (!buf) {
+		ret = -ENOMEM;
+		goto err_unlock;
+	}
+
+	buf->offset = 0;
+	buf->addr = device_address(ocmem, client, buf->offset);
+	buf->len = size;
+
+	update_range(ocmem, buf, CORE_ON, WIDE_MODE);
+
+	if (qcom_scm_ocmem_lock_available()) {
+		ret = qcom_scm_ocmem_lock(QCOM_SCM_OCMEM_GRAPHICS_ID,
+					  buf->offset, buf->len, WIDE_MODE);
+		if (ret) {
+			dev_err(ocmem->dev, "could not lock: %d\n", ret);
+			ret = -EINVAL;
+			goto err_kfree;
+		}
+	} else {
+		ocmem_write(ocmem, OCMEM_REG_GFX_MPU_START, buf->offset);
+		ocmem_write(ocmem, OCMEM_REG_GFX_MPU_END,
+			    buf->offset + buf->len);
+	}
+
+	dev_dbg(ocmem->dev, "using %ldK of OCMEM at 0x%08lx for client %d\n",
+		size / 1024, buf->addr, client);
+
+	return buf;
+
+err_kfree:
+	kfree(buf);
+err_unlock:
+	clear_bit_unlock(BIT(client), &ocmem->active_allocations);
+
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL(ocmem_allocate);
+
+void ocmem_free(struct ocmem *ocmem, enum ocmem_client client,
+		struct ocmem_buf *buf)
+{
+	/* TODO: add support for other clients... */
+	if (WARN_ON(client != OCMEM_GRAPHICS))
+		return;
+
+	update_range(ocmem, buf, CLK_OFF, MODE_DEFAULT);
+
+	if (qcom_scm_ocmem_lock_available()) {
+		int ret;
+
+		ret = qcom_scm_ocmem_unlock(QCOM_SCM_OCMEM_GRAPHICS_ID,
+					    buf->offset, buf->len);
+		if (ret)
+			dev_err(ocmem->dev, "could not unlock: %d\n", ret);
+	} else {
+		ocmem_write(ocmem, OCMEM_REG_GFX_MPU_START, 0x0);
+		ocmem_write(ocmem, OCMEM_REG_GFX_MPU_END, 0x0);
+	}
+
+	kfree(buf);
+
+	clear_bit_unlock(BIT(client), &ocmem->active_allocations);
+}
+EXPORT_SYMBOL(ocmem_free);
+
+static int ocmem_dev_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	unsigned long reg, region_size;
+	int i, j, ret, num_banks;
+	struct resource *res;
+	struct ocmem *ocmem;
+
+	if (!qcom_scm_is_available())
+		return -EPROBE_DEFER;
+
+	ocmem = devm_kzalloc(dev, sizeof(*ocmem), GFP_KERNEL);
+	if (!ocmem)
+		return -ENOMEM;
+
+	ocmem->dev = dev;
+	ocmem->config = device_get_match_data(dev);
+
+	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(ocmem_clks), ocmem_clks);
+	if (ret) {
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Unable to get clocks\n");
+
+		return ret;
+	}
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ctrl");
+	ocmem->mmio = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(ocmem->mmio)) {
+		dev_err(&pdev->dev, "Failed to ioremap ocmem_ctrl resource\n");
+		return PTR_ERR(ocmem->mmio);
+	}
+
+	ocmem->memory = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+						     "mem");
+	if (!ocmem->memory) {
+		dev_err(dev, "Could not get mem region\n");
+		return -ENXIO;
+	}
+
+	/* The core clock is synchronous with graphics */
+	WARN_ON(clk_set_rate(ocmem_clks[OCMEM_CLK_CORE_IDX].clk, 1000) < 0);
+
+	ret = clk_bulk_prepare_enable(ARRAY_SIZE(ocmem_clks), ocmem_clks);
+	if (ret) {
+		dev_info(ocmem->dev, "Failed to enable clocks\n");
+		return ret;
+	}
+
+	if (qcom_scm_restore_sec_cfg_available()) {
+		dev_dbg(dev, "configuring scm\n");
+		ret = qcom_scm_restore_sec_cfg(QCOM_SCM_OCMEM_DEV_ID, 0);
+		if (ret) {
+			dev_err(dev, "Could not enable secure configuration\n");
+			goto err_clk_disable;
+		}
+	}
+
+	reg = ocmem_read(ocmem, OCMEM_REG_HW_PROFILE);
+	ocmem->num_ports = OCMEM_HW_PROFILE_NUM_PORTS(reg);
+	ocmem->num_macros = OCMEM_HW_PROFILE_NUM_MACROS(reg);
+	ocmem->interleaved = !!(reg & OCMEM_HW_PROFILE_INTERLEAVING);
+
+	num_banks = ocmem->num_ports / 2;
+	region_size = ocmem->config->macro_size * num_banks;
+
+	dev_info(dev, "%u ports, %u regions, %u macros, %sinterleaved\n",
+		 ocmem->num_ports, ocmem->config->num_regions,
+		 ocmem->num_macros, ocmem->interleaved ? "" : "not ");
+
+	ocmem->regions = devm_kcalloc(dev, ocmem->config->num_regions,
+				      sizeof(struct ocmem_region), GFP_KERNEL);
+	if (!ocmem->regions) {
+		ret = -ENOMEM;
+		goto err_clk_disable;
+	}
+
+	for (i = 0; i < ocmem->config->num_regions; i++) {
+		struct ocmem_region *region = &ocmem->regions[i];
+
+		if (WARN_ON(num_banks > ARRAY_SIZE(region->macro_state))) {
+			ret = -EINVAL;
+			goto err_clk_disable;
+		}
+
+		region->mode = MODE_DEFAULT;
+		region->num_macros = num_banks;
+
+		if (i == (ocmem->config->num_regions - 1) &&
+		    reg & OCMEM_HW_PROFILE_LAST_REGN_HALFSIZE) {
+			region->macro_size = ocmem->config->macro_size / 2;
+			region->region_size = region_size / 2;
+		} else {
+			region->macro_size = ocmem->config->macro_size;
+			region->region_size = region_size;
+		}
+
+		for (j = 0; j < ARRAY_SIZE(region->macro_state); j++)
+			region->macro_state[j] = CLK_OFF;
+	}
+
+	platform_set_drvdata(pdev, ocmem);
+
+	return 0;
+
+err_clk_disable:
+	clk_bulk_disable_unprepare(ARRAY_SIZE(ocmem_clks), ocmem_clks);
+	return ret;
+}
+
+static int ocmem_dev_remove(struct platform_device *pdev)
+{
+	clk_bulk_disable_unprepare(ARRAY_SIZE(ocmem_clks), ocmem_clks);
+
+	return 0;
+}
+
+static const struct ocmem_config ocmem_8974_config = {
+	.num_regions = 3,
+	.macro_size = SZ_128K,
+};
+
+static const struct of_device_id ocmem_of_match[] = {
+	{ .compatible = "qcom,msm8974-ocmem", .data = &ocmem_8974_config },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, ocmem_of_match);
+
+static struct platform_driver ocmem_driver = {
+	.probe = ocmem_dev_probe,
+	.remove = ocmem_dev_remove,
+	.driver = {
+		.name = "ocmem",
+		.of_match_table = ocmem_of_match,
+	},
+};
+
+module_platform_driver(ocmem_driver);
+
+MODULE_DESCRIPTION("On Chip Memory (OCMEM) allocator for some Snapdragon SoCs");
+MODULE_LICENSE("GPL");
diff --git a/include/soc/qcom/ocmem.h b/include/soc/qcom/ocmem.h
new file mode 100644
index 000000000000..a0ae336ba78b
--- /dev/null
+++ b/include/soc/qcom/ocmem.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * The On Chip Memory (OCMEM) allocator allows various clients to allocate
+ * memory from OCMEM based on performance, latency and power requirements.
+ * This is typically used by the GPU, camera/video, and audio components on
+ * some Snapdragon SoCs.
+ *
+ * Copyright (C) 2019 Brian Masney <masneyb@onstation.org>
+ * Copyright (C) 2015 Red Hat. Author: Rob Clark <robdclark@gmail.com>
+ */
+
+#ifndef __OCMEM_H__
+#define __OCMEM_H__
+
+enum ocmem_client {
+	/* GMEM clients */
+	OCMEM_GRAPHICS = 0x0,
+	/*
+	 * TODO add more once ocmem_allocate() is clever enough to
+	 * deal with multiple clients.
+	 */
+	OCMEM_CLIENT_MAX,
+};
+
+struct ocmem;
+
+struct ocmem_buf {
+	unsigned long offset;
+	unsigned long addr;
+	unsigned long len;
+};
+
+#if IS_ENABLED(CONFIG_QCOM_OCMEM)
+
+struct ocmem *of_get_ocmem(struct device *dev);
+struct ocmem_buf *ocmem_allocate(struct ocmem *ocmem, enum ocmem_client client,
+				 unsigned long size);
+void ocmem_free(struct ocmem *ocmem, enum ocmem_client client,
+		struct ocmem_buf *buf);
+
+#else /* IS_ENABLED(CONFIG_QCOM_OCMEM) */
+
+static inline struct ocmem *of_get_ocmem(struct device *dev)
+{
+	return ERR_PTR(-ENODEV);
+}
+
+static inline struct ocmem_buf *ocmem_allocate(struct ocmem *ocmem,
+					       enum ocmem_client client,
+					       unsigned long size)
+{
+	return ERR_PTR(-ENODEV);
+}
+
+static inline void ocmem_free(struct ocmem *ocmem, enum ocmem_client client,
+			      struct ocmem_buf *buf)
+{
+}
+
+#endif /* IS_ENABLED(CONFIG_QCOM_OCMEM) */
+
+#endif /* __OCMEM_H__ */
-- 
2.21.0

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

^ permalink raw reply related

* [PATCH v4 6/6] drm/msm/gpu: add ocmem init/cleanup functions
From: Brian Masney @ 2019-08-03 14:20 UTC (permalink / raw)
  To: agross-DgEjT+Ai2ygdnm+yROfE0A, robdclark-Re5JQEeQqe8AvxtiuMwx3w,
	sean-p7yTbzM4H96eqtR555YLDQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A
  Cc: mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	jonathan-eSc4qw6YbEQ, airlied-cv59FeDIM0c,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	jcrouse-sgV2jX0FEOL9JmXXK+q4OQ, daniel-/w4YWyX8dFk,
	freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
In-Reply-To: <20190803142026.9647-1-masneyb-1iNe0GrtECGEi8DpZVb4nw@public.gmane.org>

The files a3xx_gpu.c and a4xx_gpu.c have ifdefs for the OCMEM support
that was missing upstream. Add two new functions (adreno_gpu_ocmem_init
and adreno_gpu_ocmem_cleanup) that removes some duplicated code.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
Changes since v3:
- None

Changes since v2:
- Check for -ENODEV error of_get_ocmem()
- remove fail_cleanup_ocmem label in a[34]xx_gpu_init

Changes since v1:
- remove CONFIG_QCOM_OCMEM #ifdefs
- use unsigned long for memory addresses instead of uint32_t
- add 'depends on QCOM_OCMEM || QCOM_OCMEM=n' to Kconfig

 drivers/gpu/drm/msm/Kconfig             |  1 +
 drivers/gpu/drm/msm/adreno/a3xx_gpu.c   | 28 +++++------------
 drivers/gpu/drm/msm/adreno/a3xx_gpu.h   |  3 +-
 drivers/gpu/drm/msm/adreno/a4xx_gpu.c   | 25 ++++------------
 drivers/gpu/drm/msm/adreno/a4xx_gpu.h   |  3 +-
 drivers/gpu/drm/msm/adreno/adreno_gpu.c | 40 +++++++++++++++++++++++++
 drivers/gpu/drm/msm/adreno/adreno_gpu.h | 10 +++++++
 7 files changed, 66 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 9c37e4de5896..b3d3b2172659 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -7,6 +7,7 @@ config DRM_MSM
 	depends on OF && COMMON_CLK
 	depends on MMU
 	depends on INTERCONNECT || !INTERCONNECT
+	depends on QCOM_OCMEM || QCOM_OCMEM=n
 	select QCOM_MDT_LOADER if ARCH_QCOM
 	select REGULATOR
 	select DRM_KMS_HELPER
diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
index ec82aaa21734..07ddcc529573 100644
--- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
@@ -6,10 +6,6 @@
  * Copyright (c) 2014 The Linux Foundation. All rights reserved.
  */
 
-#ifdef CONFIG_MSM_OCMEM
-#  include <mach/ocmem.h>
-#endif
-
 #include "a3xx_gpu.h"
 
 #define A3XX_INT0_MASK \
@@ -195,9 +191,9 @@ static int a3xx_hw_init(struct msm_gpu *gpu)
 		gpu_write(gpu, REG_A3XX_RBBM_GPR0_CTL, 0x00000000);
 
 	/* Set the OCMEM base address for A330, etc */
-	if (a3xx_gpu->ocmem_hdl) {
+	if (a3xx_gpu->ocmem.hdl) {
 		gpu_write(gpu, REG_A3XX_RB_GMEM_BASE_ADDR,
-			(unsigned int)(a3xx_gpu->ocmem_base >> 14));
+			(unsigned int)(a3xx_gpu->ocmem.base >> 14));
 	}
 
 	/* Turn on performance counters: */
@@ -318,10 +314,7 @@ static void a3xx_destroy(struct msm_gpu *gpu)
 
 	adreno_gpu_cleanup(adreno_gpu);
 
-#ifdef CONFIG_MSM_OCMEM
-	if (a3xx_gpu->ocmem_base)
-		ocmem_free(OCMEM_GRAPHICS, a3xx_gpu->ocmem_hdl);
-#endif
+	adreno_gpu_ocmem_cleanup(&a3xx_gpu->ocmem);
 
 	kfree(a3xx_gpu);
 }
@@ -494,17 +487,10 @@ struct msm_gpu *a3xx_gpu_init(struct drm_device *dev)
 
 	/* if needed, allocate gmem: */
 	if (adreno_is_a330(adreno_gpu)) {
-#ifdef CONFIG_MSM_OCMEM
-		/* TODO this is different/missing upstream: */
-		struct ocmem_buf *ocmem_hdl =
-				ocmem_allocate(OCMEM_GRAPHICS, adreno_gpu->gmem);
-
-		a3xx_gpu->ocmem_hdl = ocmem_hdl;
-		a3xx_gpu->ocmem_base = ocmem_hdl->addr;
-		adreno_gpu->gmem = ocmem_hdl->len;
-		DBG("using %dK of OCMEM at 0x%08x", adreno_gpu->gmem / 1024,
-				a3xx_gpu->ocmem_base);
-#endif
+		ret = adreno_gpu_ocmem_init(&adreno_gpu->base.pdev->dev,
+					    adreno_gpu, &a3xx_gpu->ocmem);
+		if (ret)
+			goto fail;
 	}
 
 	if (!gpu->aspace) {
diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.h b/drivers/gpu/drm/msm/adreno/a3xx_gpu.h
index 5dc33e5ea53b..c555fb13e0d7 100644
--- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.h
@@ -19,8 +19,7 @@ struct a3xx_gpu {
 	struct adreno_gpu base;
 
 	/* if OCMEM is used for GMEM: */
-	uint32_t ocmem_base;
-	void *ocmem_hdl;
+	struct adreno_ocmem ocmem;
 };
 #define to_a3xx_gpu(x) container_of(x, struct a3xx_gpu, base)
 
diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
index ab2b752566d8..b01388a9e89e 100644
--- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
@@ -2,9 +2,6 @@
 /* Copyright (c) 2014 The Linux Foundation. All rights reserved.
  */
 #include "a4xx_gpu.h"
-#ifdef CONFIG_MSM_OCMEM
-#  include <soc/qcom/ocmem.h>
-#endif
 
 #define A4XX_INT0_MASK \
 	(A4XX_INT0_RBBM_AHB_ERROR |        \
@@ -188,7 +185,7 @@ static int a4xx_hw_init(struct msm_gpu *gpu)
 			(1 << 30) | 0xFFFF);
 
 	gpu_write(gpu, REG_A4XX_RB_GMEM_BASE_ADDR,
-			(unsigned int)(a4xx_gpu->ocmem_base >> 14));
+			(unsigned int)(a4xx_gpu->ocmem.base >> 14));
 
 	/* Turn on performance counters: */
 	gpu_write(gpu, REG_A4XX_RBBM_PERFCTR_CTL, 0x01);
@@ -318,10 +315,7 @@ static void a4xx_destroy(struct msm_gpu *gpu)
 
 	adreno_gpu_cleanup(adreno_gpu);
 
-#ifdef CONFIG_MSM_OCMEM
-	if (a4xx_gpu->ocmem_base)
-		ocmem_free(OCMEM_GRAPHICS, a4xx_gpu->ocmem_hdl);
-#endif
+	adreno_gpu_ocmem_cleanup(&a4xx_gpu->ocmem);
 
 	kfree(a4xx_gpu);
 }
@@ -578,17 +572,10 @@ struct msm_gpu *a4xx_gpu_init(struct drm_device *dev)
 
 	/* if needed, allocate gmem: */
 	if (adreno_is_a4xx(adreno_gpu)) {
-#ifdef CONFIG_MSM_OCMEM
-		/* TODO this is different/missing upstream: */
-		struct ocmem_buf *ocmem_hdl =
-				ocmem_allocate(OCMEM_GRAPHICS, adreno_gpu->gmem);
-
-		a4xx_gpu->ocmem_hdl = ocmem_hdl;
-		a4xx_gpu->ocmem_base = ocmem_hdl->addr;
-		adreno_gpu->gmem = ocmem_hdl->len;
-		DBG("using %dK of OCMEM at 0x%08x", adreno_gpu->gmem / 1024,
-				a4xx_gpu->ocmem_base);
-#endif
+		ret = adreno_gpu_ocmem_init(dev->dev, adreno_gpu,
+					    &a4xx_gpu->ocmem);
+		if (ret)
+			goto fail;
 	}
 
 	if (!gpu->aspace) {
diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.h b/drivers/gpu/drm/msm/adreno/a4xx_gpu.h
index d506311ee240..a01448cba2ea 100644
--- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.h
@@ -16,8 +16,7 @@ struct a4xx_gpu {
 	struct adreno_gpu base;
 
 	/* if OCMEM is used for GMEM: */
-	uint32_t ocmem_base;
-	void *ocmem_hdl;
+	struct adreno_ocmem ocmem;
 };
 #define to_a4xx_gpu(x) container_of(x, struct a4xx_gpu, base)
 
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 9acbbc0f3232..62e3ada0f7c8 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -14,6 +14,7 @@
 #include <linux/pm_opp.h>
 #include <linux/slab.h>
 #include <linux/soc/qcom/mdt_loader.h>
+#include <soc/qcom/ocmem.h>
 #include "adreno_gpu.h"
 #include "msm_gem.h"
 #include "msm_mmu.h"
@@ -892,6 +893,45 @@ static int adreno_get_pwrlevels(struct device *dev,
 	return 0;
 }
 
+int adreno_gpu_ocmem_init(struct device *dev, struct adreno_gpu *adreno_gpu,
+			  struct adreno_ocmem *adreno_ocmem)
+{
+	struct ocmem_buf *ocmem_hdl;
+	struct ocmem *ocmem;
+
+	ocmem = of_get_ocmem(dev);
+	if (IS_ERR(ocmem)) {
+		if (PTR_ERR(ocmem) == -ENODEV) {
+			/*
+			 * Return success since either the ocmem property was
+			 * not specified in device tree, or ocmem support is
+			 * not compiled into the kernel.
+			 */
+			return 0;
+		}
+
+		return PTR_ERR(ocmem);
+	}
+
+	ocmem_hdl = ocmem_allocate(ocmem, OCMEM_GRAPHICS, adreno_gpu->gmem);
+	if (IS_ERR(ocmem_hdl))
+		return PTR_ERR(ocmem_hdl);
+
+	adreno_ocmem->ocmem = ocmem;
+	adreno_ocmem->base = ocmem_hdl->addr;
+	adreno_ocmem->hdl = ocmem_hdl;
+	adreno_gpu->gmem = ocmem_hdl->len;
+
+	return 0;
+}
+
+void adreno_gpu_ocmem_cleanup(struct adreno_ocmem *adreno_ocmem)
+{
+	if (adreno_ocmem && adreno_ocmem->base)
+		ocmem_free(adreno_ocmem->ocmem, OCMEM_GRAPHICS,
+			   adreno_ocmem->hdl);
+}
+
 int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
 		struct adreno_gpu *adreno_gpu,
 		const struct adreno_gpu_funcs *funcs, int nr_rings)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index c7441fb8313e..d225a8a92e58 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -126,6 +126,12 @@ struct adreno_gpu {
 };
 #define to_adreno_gpu(x) container_of(x, struct adreno_gpu, base)
 
+struct adreno_ocmem {
+	struct ocmem *ocmem;
+	unsigned long base;
+	void *hdl;
+};
+
 /* platform config data (ie. from DT, or pdata) */
 struct adreno_platform_config {
 	struct adreno_rev rev;
@@ -236,6 +242,10 @@ void adreno_dump(struct msm_gpu *gpu);
 void adreno_wait_ring(struct msm_ringbuffer *ring, uint32_t ndwords);
 struct msm_ringbuffer *adreno_active_ring(struct msm_gpu *gpu);
 
+int adreno_gpu_ocmem_init(struct device *dev, struct adreno_gpu *adreno_gpu,
+			  struct adreno_ocmem *ocmem);
+void adreno_gpu_ocmem_cleanup(struct adreno_ocmem *ocmem);
+
 int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
 		struct adreno_gpu *gpu, const struct adreno_gpu_funcs *funcs,
 		int nr_rings);
-- 
2.21.0

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

^ permalink raw reply related

* Re: [PATCH v3 01/11] dt-bindings: firmware: imx-scu: new binding to parse clocks from device tree
From: Shawn Guo @ 2019-08-03 14:42 UTC (permalink / raw)
  To: Dong Aisheng
  Cc: devicetree, sboyd, mturquette, Rob Herring, linux-imx, kernel,
	fabio.estevam, linux-clk, linux-arm-kernel
In-Reply-To: <1563289265-10977-2-git-send-email-aisheng.dong@nxp.com>

On Tue, Jul 16, 2019 at 11:00:55PM +0800, Dong Aisheng wrote:
> There's a few limitations on the original one cell clock binding
> (#clock-cells = <1>) that we have to define some SW clock IDs for device
> tree to reference. This may cause troubles if we want to use common
> clock IDs for multi platforms support when the clock of those platforms
> are mostly the same.
> e.g. Current clock IDs name are defined with SS prefix.
> 
> However the device may reside in different SS across CPUs, that means the
> SS prefix may not valid anymore for a new SoC. Furthermore, the device
> availability of those clocks may also vary a bit.
> 
> For such situation, we want to eliminate the using of SW Clock IDs and
> change to use a more close to HW one instead.
> For SCU clocks usage, only two params required: Resource id + Clock Type.

If this is how SCU firmware addresses the clock, I agree that it's worth
witching to this new bindings, which describes the hardware (SCU
firmware in this case) better, IMO.

> Both parameters are platform independent. So we could use two cells binding
> to pass those parameters,
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---
> ChangeLog:
> v2->v3:
>  * Changed to two cells binding and register all clocks in driver
>    instead of parse from device tree.
> v1->v2:
>  * changed to one cell binding inspired by arm,scpi.txt
>    Documentation/devicetree/bindings/arm/arm,scpi.txt
>    Resource ID is encoded in 'reg' property.
>    Clock type is encoded in generic clock-indices property.
>    Then we don't have to search all the DT nodes to fetch
>    those two value to construct clocks which is relatively
>    low efficiency.
>  * Add required power-domain property as well.
> ---
>  .../devicetree/bindings/arm/freescale/fsl,scu.txt       | 12 +++++++-----
>  include/dt-bindings/firmware/imx/rsrc.h                 | 17 +++++++++++++++++
>  2 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> index 5d7dbab..351d335 100644
> --- a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> @@ -89,7 +89,10 @@ Required properties:
>  			  "fsl,imx8qm-clock"
>  			  "fsl,imx8qxp-clock"
>  			followed by "fsl,scu-clk"
> -- #clock-cells:		Should be 1. Contains the Clock ID value.
> +- #clock-cells:		Should be either
> +			2: Contains the Resource and Clock ID value.
> +			or
> +			1: Contains the Clock ID value. (DEPRECATED)
>  - clocks:		List of clock specifiers, must contain an entry for
>  			each required entry in clock-names
>  - clock-names:		Should include entries "xtal_32KHz", "xtal_24MHz"
> @@ -162,7 +165,7 @@ firmware {
>  
>  		clk: clk {
>  			compatible = "fsl,imx8qxp-clk", "fsl,scu-clk";
> -			#clock-cells = <1>;
> +			#clock-cells = <2>;
>  		};
>  
>  		iomuxc {
> @@ -192,8 +195,7 @@ serial@5a060000 {
>  	...
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_lpuart0>;
> -	clocks = <&clk IMX8QXP_UART0_CLK>,
> -		 <&clk IMX8QXP_UART0_IPG_CLK>;
> -	clock-names = "per", "ipg";
> +	clocks = <&uart0_clk IMX_SC_R_UART_0 IMX_SC_PM_CLK_PER>;
> +	clock-names = "ipg";
>  	power-domains = <&pd IMX_SC_R_UART_0>;
>  };
> diff --git a/include/dt-bindings/firmware/imx/rsrc.h b/include/dt-bindings/firmware/imx/rsrc.h
> index 4e61f64..fbeaca7 100644
> --- a/include/dt-bindings/firmware/imx/rsrc.h
> +++ b/include/dt-bindings/firmware/imx/rsrc.h
> @@ -547,4 +547,21 @@
>  #define IMX_SC_R_ATTESTATION		545
>  #define IMX_SC_R_LAST			546
>  
> +/*
> + * Defines for SC PM CLK
> + */
> +#define IMX_SC_PM_CLK_SLV_BUS		0	/* Slave bus clock */
> +#define IMX_SC_PM_CLK_MST_BUS		1	/* Master bus clock */
> +#define IMX_SC_PM_CLK_PER		2	/* Peripheral clock */
> +#define IMX_SC_PM_CLK_PHY		3	/* Phy clock */
> +#define IMX_SC_PM_CLK_MISC		4	/* Misc clock */
> +#define IMX_SC_PM_CLK_MISC0		0	/* Misc 0 clock */
> +#define IMX_SC_PM_CLK_MISC1		1	/* Misc 1 clock */
> +#define IMX_SC_PM_CLK_MISC2		2	/* Misc 2 clock */
> +#define IMX_SC_PM_CLK_MISC3		3	/* Misc 3 clock */
> +#define IMX_SC_PM_CLK_MISC4		4	/* Misc 4 clock */
> +#define IMX_SC_PM_CLK_CPU		2	/* CPU clock */
> +#define IMX_SC_PM_CLK_PLL		4	/* PLL */
> +#define IMX_SC_PM_CLK_BYPASS		4	/* Bypass clock */

It seems that there are several sets of clock type which apply to
different resources/devices?  If so, can you separate them a bit with
some comments to make the list easier for readers?

Shawn

> +
>  #endif /* __DT_BINDINGS_RSCRC_IMX_H */
> -- 
> 2.7.4
> 

^ permalink raw reply

* Re: [PATCH] ARM: dts: imx: Cleanup style around assignment operator
From: Shawn Guo @ 2019-08-03 14:50 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20190727142640.23014-1-krzk@kernel.org>

On Sat, Jul 27, 2019 at 04:26:40PM +0200, Krzysztof Kozlowski wrote:
> Use a space before and after assignment operator to have consistent
> style.
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] arm64: dts: imx8mq-evk: Unbypass audio_pll1
From: Shawn Guo @ 2019-08-03 14:56 UTC (permalink / raw)
  To: Daniel Baluta
  Cc: devicetree, baruch, abel.vesa, ccaione, andrew.smirnov, s.hauer,
	angus, linux-kernel, linux-imx, festevam, shengjiu.wang,
	linux-arm-kernel, l.stach
In-Reply-To: <20190728140817.12509-1-daniel.baluta@nxp.com>

On Sun, Jul 28, 2019 at 05:08:17PM +0300, Daniel Baluta wrote:
> Making audio_pll1 parent of audio_pll1_bypass, will allow
> setting rates multiple of 8000 for children.
> 
> After unbypass clk hierarchy looks like this:
>  * osc_25m
>    * audio_pll1
>      * audio_pll1_bypass
>        * audio_pll1_out
>          * sai2
>            * sai2_root_clk
> 
> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH v3 2/2] ARM: dts: imx6ul-kontron-n6310: Add Kontron i.MX6UL N6310 SoM and boards
From: Shawn Guo @ 2019-08-03 15:47 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Mark Rutland, devicetree, Sascha Hauer, linux-kernel,
	Schrempf Frieder, Rob Herring, NXP Linux Team,
	Pengutronix Kernel Team, Fabio Estevam, linux-arm-kernel
In-Reply-To: <20190729172007.3275-2-krzk@kernel.org>

On Mon, Jul 29, 2019 at 07:20:07PM +0200, Krzysztof Kozlowski wrote:
> Add support for i.MX6UL modules from Kontron Electronics GmbH (before
> acquisition: Exceet Electronics) and evalkit boards based on it:
> 
> 1. N6310 SOM: i.MX6 UL System-on-Module, a 25x25 mm solderable module
>    (LGA pads and pin castellations) with 256 MB RAM, 1 MB NOR-Flash,
>    256 MB NAND and other interfaces,
> 2. N6310 S: evalkit, w/wo eMMC, without display,
> 3. N6310 S 43: evalkit with 4.3" display,
> 4. N6310 S 50: evalkit with 5.0" display.
> 
> This includes device nodes for unsupported displays (Admatec
> T043C004800272T2A and T070P133T0S301).

Do not include unsupported devices.

> 
> The work is based on Exceet/Kontron source code (GPLv2) with numerous
> changes:
> 1. Reorganize files,
> 2. Rename Exceet -> Kontron,
> 3. Rename models/compatibles to match newest Kontron product naming,
> 4. Fix coding style errors and adjust to device tree coding guidelines,
> 5. Fix DTC warnings,
> 6. Extend compatibles so eval boards inherit the SoM compatible,
> 7. Use defines instead of GPIO and interrupt flag values,
> 8. Use proper vendor compatible for Macronix SPI NOR,
> 9. Sort nodes alphabetically.
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> 
> ---
> 
> Changes since v2, after Fabio's review:
> 1. Add "imx6ul" compatible to board name (that's what I understood from
>    review),
> 2. Add vendor/device prefix to eeprom and document the compatible,
> 3. Use "admatecde" as vendor compatible to avoid confusion with Admatec
>    AG in Switzerland (also making LCD panels),
> 4. Use generic names for nodes,
> 5. Use IRQ_TYPE_LEVEL_LOW,
> 6. Move iomux to the end of files,
> 7. Remove regulators node (include regulators in top level),
> 8. Remove cpu clock-frequency,
> 9. Other minor fixes pointed by Fabio.
> 
> Changes since v1, after Frieder's review:
> 1. Remove unneeded license notes,
> 2. Add Kontron copyright (2018),
> 3. Rename the files/models/compatibles to new naming - N6310,
> 4. Remove unneeded CPU operating points override,
> 5. Switch regulator nodes into simple children nodes without addresses
>    (so not simple bus),
> 6. Use proper vendor compatible for Macronix SPI NOR.
> ---
>  .../devicetree/bindings/arm/fsl.yaml          |   4 +
>  .../devicetree/bindings/eeprom/at25.txt       |   1 +

Please make them two separate patches.

>  arch/arm/boot/dts/Makefile                    |   3 +
>  .../boot/dts/imx6ul-kontron-n6310-s-43.dts    | 119 +++++
>  .../boot/dts/imx6ul-kontron-n6310-s-50.dts    | 119 +++++

Are they identical except the display node?  Please manage to save
duplicated data.

>  arch/arm/boot/dts/imx6ul-kontron-n6310-s.dts  | 420 ++++++++++++++++++
>  .../boot/dts/imx6ul-kontron-n6310-som.dtsi    | 134 ++++++
>  7 files changed, 800 insertions(+)
>  create mode 100644 arch/arm/boot/dts/imx6ul-kontron-n6310-s-43.dts
>  create mode 100644 arch/arm/boot/dts/imx6ul-kontron-n6310-s-50.dts
>  create mode 100644 arch/arm/boot/dts/imx6ul-kontron-n6310-s.dts
>  create mode 100644 arch/arm/boot/dts/imx6ul-kontron-n6310-som.dtsi
> 
> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> index 7294ac36f4c0..6a6c09d67dea 100644
> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> @@ -161,6 +161,10 @@ properties:
>          items:
>            - enum:
>                - fsl,imx6ul-14x14-evk      # i.MX6 UltraLite 14x14 EVK Board
> +              - kontron,imx6ul-n6310-som  # Kontron N6310 SOM
> +              - kontron,imx6ul-n6310-s    # Kontron N6310 S Board
> +              - kontron,imx6ul-n6310-s-43 # Kontron N6310 S 43 Board
> +              - kontron,imx6ul-n6310-s-50 # Kontron N6310 S 50 Board
>            - const: fsl,imx6ul
>  
>        - description: i.MX6ULL based Boards
> diff --git a/Documentation/devicetree/bindings/eeprom/at25.txt b/Documentation/devicetree/bindings/eeprom/at25.txt
> index b3bde97dc199..42577dd113dd 100644
> --- a/Documentation/devicetree/bindings/eeprom/at25.txt
> +++ b/Documentation/devicetree/bindings/eeprom/at25.txt
> @@ -3,6 +3,7 @@ EEPROMs (SPI) compatible with Atmel at25.
>  Required properties:
>  - compatible : Should be "<vendor>,<type>", and generic value "atmel,at25".
>    Example "<vendor>,<type>" values:
> +    "anvo,anv32e61w"
>      "microchip,25lc040"
>      "st,m95m02"
>      "st,m95256"
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 9159fa2cea90..28b6cb3454a3 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -569,6 +569,9 @@ dtb-$(CONFIG_SOC_IMX6UL) += \
>  	imx6ul-geam.dtb \
>  	imx6ul-isiot-emmc.dtb \
>  	imx6ul-isiot-nand.dtb \
> +	imx6ul-kontron-n6310-s.dtb \
> +	imx6ul-kontron-n6310-s-43.dtb \
> +	imx6ul-kontron-n6310-s-50.dtb \
>  	imx6ul-liteboard.dtb \
>  	imx6ul-opos6uldev.dtb \
>  	imx6ul-pico-hobbit.dtb \
> diff --git a/arch/arm/boot/dts/imx6ul-kontron-n6310-s-43.dts b/arch/arm/boot/dts/imx6ul-kontron-n6310-s-43.dts
> new file mode 100644
> index 000000000000..c83793725245
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6ul-kontron-n6310-s-43.dts
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2017 exceet electronics GmbH
> + * Copyright (C) 2018 Kontron Electronics GmbH
> + * Copyright (c) 2019 Krzysztof Kozlowski <krzk@kernel.org>
> + */
> +
> +#include "imx6ul-kontron-n6310-s.dts"
> +
> +/ {
> +	model = "Kontron N6310 S 43";
> +	compatible = "kontron,imx6ul-n6310-s-43", "kontron,imx6ul-n6310-s",
> +		     "kontron,imx6ul-n6310-som", "fsl,imx6ul";
> +
> +	backlight: backlight {
> +		compatible = "pwm-backlight";
> +		pwms = <&pwm7 0 5000000>;
> +		brightness-levels = <0 4 8 16 32 64 128 255>;
> +		default-brightness-level = <6>;
> +		status = "okay";
> +	};
> +
> +	panel {
> +		compatible = "admatecde,t043c004800272t2a";

Undocumented/unsupported compatible?

> +		backlight = <&backlight>;
> +
> +		port {
> +			panel_in: endpoint {
> +				remote-endpoint = <&display_out>;
> +			};
> +		};
> +	};
> +};
> +
> +&i2c4 {
> +	touchscreen@5d {
> +		compatible = "goodix,gt928";
> +		reg = <0x5d>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_cap_touch>;
> +		interrupt-parent = <&gpio5>;
> +		interrupts = <6 IRQ_TYPE_LEVEL_LOW>;
> +		reset-gpios = <&gpio5 8 GPIO_ACTIVE_HIGH>;
> +		irq-gpios = <&gpio5 6 GPIO_ACTIVE_HIGH>;
> +	};
> +};
> +
> +&lcdif {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_lcdif_dat &pinctrl_lcdif_ctrl>;
> +	status = "okay";
> +
> +	port {
> +		display_out: endpoint {
> +			remote-endpoint = <&panel_in>;
> +		};
> +	};
> +};
> +
> +&pwm7 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_pwm7>;
> +	status = "okay";
> +};
> +
> +&iomuxc {
> +	pinctrl_lcdif_dat: lcdifdatgrp {
> +		fsl,pins = <
> +			MX6UL_PAD_LCD_DATA00__LCDIF_DATA00	0x79
> +			MX6UL_PAD_LCD_DATA01__LCDIF_DATA01	0x79
> +			MX6UL_PAD_LCD_DATA02__LCDIF_DATA02	0x79
> +			MX6UL_PAD_LCD_DATA03__LCDIF_DATA03	0x79
> +			MX6UL_PAD_LCD_DATA04__LCDIF_DATA04	0x79
> +			MX6UL_PAD_LCD_DATA05__LCDIF_DATA05	0x79
> +			MX6UL_PAD_LCD_DATA06__LCDIF_DATA06	0x79
> +			MX6UL_PAD_LCD_DATA07__LCDIF_DATA07	0x79
> +			MX6UL_PAD_LCD_DATA08__LCDIF_DATA08	0x79
> +			MX6UL_PAD_LCD_DATA09__LCDIF_DATA09	0x79
> +			MX6UL_PAD_LCD_DATA10__LCDIF_DATA10	0x79
> +			MX6UL_PAD_LCD_DATA11__LCDIF_DATA11	0x79
> +			MX6UL_PAD_LCD_DATA12__LCDIF_DATA12	0x79
> +			MX6UL_PAD_LCD_DATA13__LCDIF_DATA13	0x79
> +			MX6UL_PAD_LCD_DATA14__LCDIF_DATA14	0x79
> +			MX6UL_PAD_LCD_DATA15__LCDIF_DATA15	0x79
> +			MX6UL_PAD_LCD_DATA16__LCDIF_DATA16	0x79
> +			MX6UL_PAD_LCD_DATA17__LCDIF_DATA17	0x79
> +			MX6UL_PAD_LCD_DATA18__LCDIF_DATA18	0x79
> +			MX6UL_PAD_LCD_DATA19__LCDIF_DATA19	0x79
> +			MX6UL_PAD_LCD_DATA20__LCDIF_DATA20	0x79
> +			MX6UL_PAD_LCD_DATA21__LCDIF_DATA21	0x79
> +			MX6UL_PAD_LCD_DATA22__LCDIF_DATA22	0x79
> +			MX6UL_PAD_LCD_DATA23__LCDIF_DATA23	0x79
> +		>;
> +	};
> +
> +	pinctrl_lcdif_ctrl: lcdifctrlgrp {
> +		fsl,pins = <
> +			MX6UL_PAD_LCD_CLK__LCDIF_CLK		0x79
> +			MX6UL_PAD_LCD_ENABLE__LCDIF_ENABLE	0x79
> +			MX6UL_PAD_LCD_HSYNC__LCDIF_HSYNC	0x79
> +			MX6UL_PAD_LCD_VSYNC__LCDIF_VSYNC	0x79
> +			MX6UL_PAD_LCD_RESET__LCDIF_RESET	0x79
> +		>;
> +	};
> +
> +	pinctrl_cap_touch: captouchgrp {
> +		fsl,pins = <
> +			MX6UL_PAD_SNVS_TAMPER6__GPIO5_IO06	0x1b0b0 /* Touch Interrupt */
> +			MX6UL_PAD_SNVS_TAMPER7__GPIO5_IO07	0x1b0b0 /* Touch Reset */
> +			MX6UL_PAD_SNVS_TAMPER8__GPIO5_IO08	0x1b0b0 /* Touch Wake */
> +		>;
> +	};
> +
> +	pinctrl_pwm7: pwm7grp {
> +		fsl,pins = <
> +			MX6UL_PAD_CSI_VSYNC__PWM7_OUT		0x110b0
> +		>;
> +	};
> +};
> diff --git a/arch/arm/boot/dts/imx6ul-kontron-n6310-s-50.dts b/arch/arm/boot/dts/imx6ul-kontron-n6310-s-50.dts
> new file mode 100644
> index 000000000000..f9c9afa58771
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6ul-kontron-n6310-s-50.dts
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2017 exceet electronics GmbH
> + * Copyright (C) 2018 Kontron Electronics GmbH
> + * Copyright (c) 2019 Krzysztof Kozlowski <krzk@kernel.org>
> + */
> +
> +#include "imx6ul-kontron-n6310-s.dts"
> +
> +/ {
> +	model = "Kontron N6310 S 50";
> +	compatible = "kontron,imx6ul-n6310-s-50", "kontron,imx6ul-n6310-s",
> +		     "kontron,imx6ul-n6310-som", "fsl,imx6ul";
> +
> +	backlight: backlight {
> +		compatible = "pwm-backlight";
> +		pwms = <&pwm7 0 5000000>;
> +		brightness-levels = <0 4 8 16 32 64 128 255>;
> +		default-brightness-level = <6>;
> +		status = "okay";
> +	};
> +
> +	panel {
> +		compatible = "admatecde,t070p133t0s301";
> +		backlight = <&backlight>;
> +
> +		port {
> +			panel_in: endpoint {
> +				remote-endpoint = <&display_out>;
> +			};
> +		};
> +	};
> +};
> +
> +&i2c4 {
> +	touchscreen@5d {
> +		compatible = "goodix,gt928";
> +		reg = <0x5d>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_cap_touch>;
> +		interrupt-parent = <&gpio5>;
> +		interrupts = <6 IRQ_TYPE_LEVEL_LOW>;
> +		reset-gpios = <&gpio5 8 GPIO_ACTIVE_HIGH>;
> +		irq-gpios = <&gpio5 6 GPIO_ACTIVE_HIGH>;
> +	};
> +};
> +
> +&lcdif {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_lcdif_dat &pinctrl_lcdif_ctrl>;
> +	status = "okay";
> +
> +	port {
> +		display_out: endpoint {
> +			remote-endpoint = <&panel_in>;
> +		};
> +	};
> +};
> +
> +&pwm7 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_pwm7>;
> +	status = "okay";
> +};
> +
> +&iomuxc {
> +	pinctrl_lcdif_dat: lcdifdatgrp {
> +		fsl,pins = <
> +			MX6UL_PAD_LCD_DATA00__LCDIF_DATA00	0x79
> +			MX6UL_PAD_LCD_DATA01__LCDIF_DATA01	0x79
> +			MX6UL_PAD_LCD_DATA02__LCDIF_DATA02	0x79
> +			MX6UL_PAD_LCD_DATA03__LCDIF_DATA03	0x79
> +			MX6UL_PAD_LCD_DATA04__LCDIF_DATA04	0x79
> +			MX6UL_PAD_LCD_DATA05__LCDIF_DATA05	0x79
> +			MX6UL_PAD_LCD_DATA06__LCDIF_DATA06	0x79
> +			MX6UL_PAD_LCD_DATA07__LCDIF_DATA07	0x79
> +			MX6UL_PAD_LCD_DATA08__LCDIF_DATA08	0x79
> +			MX6UL_PAD_LCD_DATA09__LCDIF_DATA09	0x79
> +			MX6UL_PAD_LCD_DATA10__LCDIF_DATA10	0x79
> +			MX6UL_PAD_LCD_DATA11__LCDIF_DATA11	0x79
> +			MX6UL_PAD_LCD_DATA12__LCDIF_DATA12	0x79
> +			MX6UL_PAD_LCD_DATA13__LCDIF_DATA13	0x79
> +			MX6UL_PAD_LCD_DATA14__LCDIF_DATA14	0x79
> +			MX6UL_PAD_LCD_DATA15__LCDIF_DATA15	0x79
> +			MX6UL_PAD_LCD_DATA16__LCDIF_DATA16	0x79
> +			MX6UL_PAD_LCD_DATA17__LCDIF_DATA17	0x79
> +			MX6UL_PAD_LCD_DATA18__LCDIF_DATA18	0x79
> +			MX6UL_PAD_LCD_DATA19__LCDIF_DATA19	0x79
> +			MX6UL_PAD_LCD_DATA20__LCDIF_DATA20	0x79
> +			MX6UL_PAD_LCD_DATA21__LCDIF_DATA21	0x79
> +			MX6UL_PAD_LCD_DATA22__LCDIF_DATA22	0x79
> +			MX6UL_PAD_LCD_DATA23__LCDIF_DATA23	0x79
> +		>;
> +	};
> +
> +	pinctrl_lcdif_ctrl: lcdifctrlgrp {
> +		fsl,pins = <
> +			MX6UL_PAD_LCD_CLK__LCDIF_CLK		0x79
> +			MX6UL_PAD_LCD_ENABLE__LCDIF_ENABLE	0x79
> +			MX6UL_PAD_LCD_HSYNC__LCDIF_HSYNC	0x79
> +			MX6UL_PAD_LCD_VSYNC__LCDIF_VSYNC	0x79
> +			MX6UL_PAD_LCD_RESET__LCDIF_RESET	0x79
> +		>;
> +	};
> +
> +	pinctrl_cap_touch: captouchgrp {
> +		fsl,pins = <
> +			MX6UL_PAD_SNVS_TAMPER6__GPIO5_IO06	0x1b0b0 /* Touch Interrupt */
> +			MX6UL_PAD_SNVS_TAMPER7__GPIO5_IO07	0x1b0b0 /* Touch Reset */
> +			MX6UL_PAD_SNVS_TAMPER8__GPIO5_IO08	0x1b0b0 /* Touch Wake */
> +		>;
> +	};
> +
> +	pinctrl_pwm7: pwm7grp {
> +		fsl,pins = <
> +			MX6UL_PAD_CSI_VSYNC__PWM7_OUT		0x110b0
> +		>;
> +	};
> +};
> diff --git a/arch/arm/boot/dts/imx6ul-kontron-n6310-s.dts b/arch/arm/boot/dts/imx6ul-kontron-n6310-s.dts
> new file mode 100644
> index 000000000000..4206a4b3f0df
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6ul-kontron-n6310-s.dts
> @@ -0,0 +1,420 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2017 exceet electronics GmbH
> + * Copyright (C) 2018 Kontron Electronics GmbH
> + * Copyright (c) 2019 Krzysztof Kozlowski <krzk@kernel.org>
> + */
> +
> +/dts-v1/;
> +
> +#include "imx6ul-kontron-n6310-som.dtsi"
> +
> +/ {
> +	model = "Kontron N6310 S";
> +	compatible = "kontron,imx6ul-n6310-s", "kontron,imx6ul-n6310-som",
> +		     "fsl,imx6ul";
> +
> +	pwm-beeper {
> +		compatible = "pwm-beeper";
> +		pwms = <&pwm8 0 5000>;
> +	};
> +
> +	gpio-leds {
> +		compatible = "gpio-leds";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_gpio_leds>;
> +
> +		led1 {
> +			label = "debug-led1";
> +			gpios = <&gpio1 30 GPIO_ACTIVE_LOW>;
> +			default-state = "off";
> +			linux,default-trigger = "heartbeat";
> +		};
> +
> +		led2 {
> +			label = "debug-led2";
> +			gpios = <&gpio5 3 GPIO_ACTIVE_LOW>;
> +			default-state = "off";
> +		};
> +
> +		led3 {
> +			label = "debug-led3";
> +			gpios = <&gpio5 2 GPIO_ACTIVE_LOW>;
> +			default-state = "off";
> +		};
> +	};
> +
> +	reg_3v3: regulator-3v3 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "3v3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +	};
> +
> +	reg_vref_adc: regulator-vref-adc {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vref-adc";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +	};
> +
> +	reg_usb_otg1_vbus: regulator-usb-otg1-vbus {
> +		compatible = "regulator-fixed";
> +		regulator-name = "usb_otg1_vbus";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		gpio = <&gpio1 4 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +};
> +
> +&adc1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_adc1>;
> +	num-channels = <3>;
> +	vref-supply = <&reg_vref_adc>;
> +	status = "okay";
> +};
> +
> +&can2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_flexcan2>;
> +	status = "okay";
> +};
> +
> +&ecspi1 {
> +	cs-gpios = <&gpio4 26 GPIO_ACTIVE_HIGH>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_ecspi1>;
> +	status = "okay";
> +
> +	eeprom@0 {
> +		compatible = "anvo,anv32e61w", "atmel,at25";
> +		reg = <0>;
> +		spi-max-frequency = <20000000>;
> +		spi-cpha;
> +		spi-cpol;
> +		pagesize = <1>;
> +		size = <8192>;
> +		address-width = <16>;
> +	};
> +};
> +
> +&fec1 {
> +	pinctrl-0 = <&pinctrl_enet1>;
> +	/delete-node/ mdio;
> +};
> +
> +&fec2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_enet2 &pinctrl_enet2_mdio>;
> +	phy-mode = "rmii";
> +	phy-handle = <&ethphy2>;
> +	status = "okay";
> +
> +	mdio {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		ethphy1: ethernet-phy@1 {
> +			reg = <1>;
> +			micrel,led-mode = <0>;
> +			clocks = <&clks IMX6UL_CLK_ENET_REF>;
> +			clock-names = "rmii-ref";
> +		};
> +
> +		ethphy2: ethernet-phy@2 {
> +			reg = <2>;
> +			micrel,led-mode = <0>;
> +			clocks = <&clks IMX6UL_CLK_ENET2_REF>;
> +			clock-names = "rmii-ref";
> +		};
> +	};
> +};
> +
> +&i2c1 {
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c1>;
> +	status = "okay";
> +};
> +
> +&i2c4 {
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c4>;
> +	status = "okay";
> +
> +	rtc@32 {
> +		compatible = "epson,rx8900";
> +		reg = <0x32>;
> +	};
> +};
> +
> +&pwm8 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_pwm8>;
> +	status = "okay";
> +};
> +
> +&snvs_poweroff {
> +	status = "okay";
> +};
> +
> +&uart1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart1>;
> +	status = "okay";
> +};
> +
> +&uart2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart2>;
> +	linux,rs485-enabled-at-boot-time;
> +	rs485-rx-during-tx;
> +	rs485-rts-active-low;
> +	uart-has-rtscts;
> +	status = "okay";
> +};
> +
> +&uart3 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart3>;
> +	fsl,uart-has-rtscts;
> +	status = "okay";
> +};
> +
> +&uart4 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart4>;
> +	status = "okay";
> +};
> +
> +&usbotg1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_usbotg1>;
> +	dr_mode = "otg";
> +	srp-disable;
> +	hnp-disable;
> +	adp-disable;
> +	vbus-supply = <&reg_usb_otg1_vbus>;
> +	status = "okay";
> +};
> +
> +&usbotg2 {
> +	dr_mode = "host";
> +	disable-over-current;
> +	status = "okay";
> +};
> +
> +&usdhc1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_usdhc1>;
> +	cd-gpios = <&gpio1 19 GPIO_ACTIVE_LOW>;
> +	keep-power-in-suspend;
> +	enable-sdio-wakeup;

Check Documentation/devicetree/bindings/power/wakeup-source.txt

> +	vmmc-supply = <&reg_3v3>;
> +	voltage-ranges = <3300 3300>;
> +	no-1-8-v;
> +	status = "okay";
> +};
> +
> +&usdhc2 {
> +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> +	pinctrl-0 = <&pinctrl_usdhc2>;
> +	pinctrl-1 = <&pinctrl_usdhc2_100mhz>;
> +	pinctrl-2 = <&pinctrl_usdhc2_200mhz>;
> +	non-removable;
> +	keep-power-in-suspend;
> +	enable-sdio-wakeup;
> +	vmmc-supply = <&reg_3v3>;
> +	voltage-ranges = <3300 3300>;
> +	no-1-8-v;
> +	status = "okay";
> +};
> +
> +&wdog1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_wdog>;
> +	status = "okay";

We usually put 'status' at the end of property list.

> +	fsl,ext-reset-output;
> +};
> +
> +&iomuxc {
> +	pinctrl-0 = <&pinctrl_reset_out &pinctrl_gpio>;
> +
> +	pinctrl_wdog: wdoggrp {
> +		fsl,pins = <
> +			MX6UL_PAD_GPIO1_IO09__WDOG1_WDOG_ANY	0x30b0
> +		>;
> +	};
> +
> +	pinctrl_gpio: gpio {

Please consistently name the node like:

	pinctrl_xxx: xxxgrp

And keep them well sorted alphabetically.

> +		fsl,pins = <
> +			MX6UL_PAD_SNVS_TAMPER5__GPIO5_IO05	0x1b0b0 /* DOUT1 */
> +			MX6UL_PAD_SNVS_TAMPER4__GPIO5_IO04	0x1b0b0 /* DIN1 */
> +			MX6UL_PAD_SNVS_TAMPER1__GPIO5_IO01	0x1b0b0 /* DOUT2 */
> +			MX6UL_PAD_SNVS_TAMPER0__GPIO5_IO00	0x1b0b0 /* DIN2 */
> +		>;
> +	};
> +
> +	pinctrl_usbotg1: usbotg1 {
> +		fsl,pins = <
> +			MX6UL_PAD_GPIO1_IO04__GPIO1_IO04	0x1b0b0
> +		>;
> +	};
> +
> +	pinctrl_gpio_leds: gpio_leds {
> +		fsl,pins = <
> +			MX6UL_PAD_UART5_TX_DATA__GPIO1_IO30	0x1b0b0	/* LED H14 */
> +			MX6UL_PAD_SNVS_TAMPER3__GPIO5_IO03	0x1b0b0	/* LED H15 */
> +			MX6UL_PAD_SNVS_TAMPER2__GPIO5_IO02	0x1b0b0	/* LED H16 */
> +		>;
> +	};
> +
> +	/* FRAM */
> +	pinctrl_ecspi1: ecspi1grp-1 {

Meaningless '-1' suffix.

Shawn

> +		fsl,pins = <
> +			MX6UL_PAD_CSI_DATA07__ECSPI1_MISO	0x100b1
> +			MX6UL_PAD_CSI_DATA06__ECSPI1_MOSI	0x100b1
> +			MX6UL_PAD_CSI_DATA04__ECSPI1_SCLK	0x100b1
> +			MX6UL_PAD_CSI_DATA05__GPIO4_IO26	0x100b1	/* ECSPI1-CS1 */
> +		>;
> +	};
> +
> +	pinctrl_enet2: enet2grp {
> +		fsl,pins = <
> +			MX6UL_PAD_ENET2_RX_EN__ENET2_RX_EN	0x1b0b0
> +			MX6UL_PAD_ENET2_RX_ER__ENET2_RX_ER	0x1b0b0
> +			MX6UL_PAD_ENET2_RX_DATA0__ENET2_RDATA00	0x1b0b0
> +			MX6UL_PAD_ENET2_RX_DATA1__ENET2_RDATA01	0x1b0b0
> +			MX6UL_PAD_ENET2_TX_EN__ENET2_TX_EN	0x1b0b0
> +			MX6UL_PAD_ENET2_TX_DATA0__ENET2_TDATA00	0x1b0b0
> +			MX6UL_PAD_ENET2_TX_DATA1__ENET2_TDATA01	0x1b0b0
> +			MX6UL_PAD_ENET2_TX_CLK__ENET2_REF_CLK2	0x4001b009
> +		>;
> +	};
> +
> +	pinctrl_enet2_mdio: enet2mdiogrp {
> +		fsl,pins = <
> +			MX6UL_PAD_GPIO1_IO07__ENET2_MDC         0x1b0b0
> +			MX6UL_PAD_GPIO1_IO06__ENET2_MDIO        0x1b0b0
> +		>;
> +	};
> +
> +	pinctrl_flexcan2: flexcan2grp{
> +		fsl,pins = <
> +			MX6UL_PAD_UART2_RTS_B__FLEXCAN2_RX	0x1b020
> +			MX6UL_PAD_UART2_CTS_B__FLEXCAN2_TX	0x1b020
> +		>;
> +	};
> +
> +	pinctrl_pwm8: pwm8grp {
> +		fsl,pins = <
> +			MX6UL_PAD_CSI_HSYNC__PWM8_OUT		0x110b0
> +		>;
> +	};
> +
> +	pinctrl_uart1: uart1grp {
> +		fsl,pins = <
> +			MX6UL_PAD_UART1_TX_DATA__UART1_DCE_TX	0x1b0b1
> +			MX6UL_PAD_UART1_RX_DATA__UART1_DCE_RX	0x1b0b1
> +		>;
> +	};
> +
> +	pinctrl_uart2: uart2grp {
> +		fsl,pins = <
> +			MX6UL_PAD_NAND_DATA04__UART2_DCE_TX	0x1b0b1
> +			MX6UL_PAD_NAND_DATA05__UART2_DCE_RX	0x1b0b1
> +			MX6UL_PAD_NAND_DATA06__UART2_DCE_CTS	0x1b0b1
> +			/*
> +			 * mux unused RTS to make sure it doesn't cause
> +			 * any interrupts when it is undefined
> +			 */
> +			MX6UL_PAD_NAND_DATA07__UART2_DCE_RTS	0x1b0b1
> +		>;
> +	};
> +
> +	pinctrl_uart3: uart3grp {
> +		fsl,pins = <
> +			MX6UL_PAD_UART3_TX_DATA__UART3_DCE_TX	0x1b0b1
> +			MX6UL_PAD_UART3_RX_DATA__UART3_DCE_RX	0x1b0b1
> +			MX6UL_PAD_UART3_CTS_B__UART3_DCE_CTS	0x1b0b1
> +			MX6UL_PAD_UART3_RTS_B__UART3_DCE_RTS	0x1b0b1
> +		>;
> +	};
> +
> +	pinctrl_uart4: uart4grp {
> +		fsl,pins = <
> +			MX6UL_PAD_UART4_TX_DATA__UART4_DCE_TX	0x1b0b1
> +			MX6UL_PAD_UART4_RX_DATA__UART4_DCE_RX	0x1b0b1
> +		>;
> +	};
> +
> +	pinctrl_usdhc1: usdhc1grp {
> +		fsl,pins = <
> +			MX6UL_PAD_SD1_CMD__USDHC1_CMD		0x17059
> +			MX6UL_PAD_SD1_CLK__USDHC1_CLK		0x10059
> +			MX6UL_PAD_SD1_DATA0__USDHC1_DATA0	0x17059
> +			MX6UL_PAD_SD1_DATA1__USDHC1_DATA1	0x17059
> +			MX6UL_PAD_SD1_DATA2__USDHC1_DATA2	0x17059
> +			MX6UL_PAD_SD1_DATA3__USDHC1_DATA3	0x17059
> +			MX6UL_PAD_UART1_RTS_B__GPIO1_IO19	0x100b1	/* SD1_CD */
> +		>;
> +	};
> +
> +	pinctrl_usdhc2: usdhc2grp {
> +		fsl,pins = <
> +			MX6UL_PAD_NAND_RE_B__USDHC2_CLK		0x10059
> +			MX6UL_PAD_NAND_WE_B__USDHC2_CMD		0x17059
> +			MX6UL_PAD_NAND_DATA00__USDHC2_DATA0	0x17059
> +			MX6UL_PAD_NAND_DATA01__USDHC2_DATA1	0x17059
> +			MX6UL_PAD_NAND_DATA02__USDHC2_DATA2	0x17059
> +			MX6UL_PAD_NAND_DATA03__USDHC2_DATA3	0x17059
> +		>;
> +	};
> +
> +	pinctrl_usdhc2_100mhz: usdhc2grp100mhz {
> +		fsl,pins = <
> +			MX6UL_PAD_NAND_RE_B__USDHC2_CLK		0x100b9
> +			MX6UL_PAD_NAND_WE_B__USDHC2_CMD		0x170b9
> +			MX6UL_PAD_NAND_DATA00__USDHC2_DATA0	0x170b9
> +			MX6UL_PAD_NAND_DATA01__USDHC2_DATA1	0x170b9
> +			MX6UL_PAD_NAND_DATA02__USDHC2_DATA2	0x170b9
> +			MX6UL_PAD_NAND_DATA03__USDHC2_DATA3	0x170b9
> +		>;
> +	};
> +
> +	pinctrl_usdhc2_200mhz: usdhc2grp200mhz {
> +		fsl,pins = <
> +			MX6UL_PAD_NAND_RE_B__USDHC2_CLK		0x100f9
> +			MX6UL_PAD_NAND_WE_B__USDHC2_CMD		0x170f9
> +			MX6UL_PAD_NAND_DATA00__USDHC2_DATA0	0x170f9
> +			MX6UL_PAD_NAND_DATA01__USDHC2_DATA1	0x170f9
> +			MX6UL_PAD_NAND_DATA02__USDHC2_DATA2	0x170f9
> +			MX6UL_PAD_NAND_DATA03__USDHC2_DATA3	0x170f9
> +		>;
> +	};
> +
> +	pinctrl_i2c1: i2c1grp {
> +		fsl,pins = <
> +			MX6UL_PAD_CSI_PIXCLK__I2C1_SCL		0x4001b8b0
> +			MX6UL_PAD_CSI_MCLK__I2C1_SDA		0x4001b8b0
> +		>;
> +	};
> +
> +	pinctrl_i2c4: i2c4grp {
> +		fsl,pins = <
> +			MX6UL_PAD_UART2_TX_DATA__I2C4_SCL	0x4001f8b0
> +			MX6UL_PAD_UART2_RX_DATA__I2C4_SDA	0x4001f8b0
> +		>;
> +	};
> +
> +	pinctrl_adc1: adc1grp {
> +		fsl,pins = <
> +			MX6UL_PAD_GPIO1_IO02__GPIO1_IO02	0xb0
> +			MX6UL_PAD_GPIO1_IO03__GPIO1_IO03	0xb0
> +			MX6UL_PAD_GPIO1_IO08__GPIO1_IO08	0xb0
> +		>;
> +	};
> +};
> diff --git a/arch/arm/boot/dts/imx6ul-kontron-n6310-som.dtsi b/arch/arm/boot/dts/imx6ul-kontron-n6310-som.dtsi
> new file mode 100644
> index 000000000000..798ff0028e29
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6ul-kontron-n6310-som.dtsi
> @@ -0,0 +1,134 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2017 exceet electronics GmbH
> + * Copyright (C) 2018 Kontron Electronics GmbH
> + * Copyright (c) 2019 Krzysztof Kozlowski <krzk@kernel.org>
> + */
> +
> +#include "imx6ul.dtsi"
> +#include <dt-bindings/gpio/gpio.h>
> +
> +/ {
> +	model = "Kontron N6310 SOM";
> +	compatible = "kontron,imx6ul-n6310-som", "fsl,imx6ul";
> +
> +	memory@80000000 {
> +		reg = <0x80000000 0x10000000>;
> +		device_type = "memory";
> +	};
> +};
> +
> +&ecspi2 {
> +	cs-gpios = <&gpio4 22 GPIO_ACTIVE_HIGH>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_ecspi2>;
> +	status = "okay";
> +
> +	spi-flash@0 {
> +		compatible = "mxicy,mx25v8035f", "jedec,spi-nor";
> +		spi-max-frequency = <50000000>;
> +		reg = <0>;
> +	};
> +};
> +
> +&fec1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_enet1 &pinctrl_enet1_mdio>;
> +	phy-mode = "rmii";
> +	phy-handle = <&ethphy1>;
> +	status = "okay";
> +
> +	mdio {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		ethphy1: ethernet-phy@1 {
> +			reg = <1>;
> +			micrel,led-mode = <0>;
> +			clocks = <&clks IMX6UL_CLK_ENET_REF>;
> +			clock-names = "rmii-ref";
> +		};
> +	};
> +};
> +
> +&fec2 {
> +	phy-mode = "rmii";
> +	status = "disabled";
> +};
> +
> +&qspi {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_qspi>;
> +	status = "okay";
> +
> +	spi-flash@0 {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "spi-nand";
> +		spi-max-frequency = <108000000>;
> +		spi-tx-bus-width = <4>;
> +		spi-rx-bus-width = <4>;
> +		reg = <0>;
> +
> +		partition@0 {
> +			label = "ubi1";
> +			reg = <0x00000000 0x08000000>;
> +		};
> +
> +		partition@8000000 {
> +			label = "ubi2";
> +			reg = <0x08000000 0x08000000>;
> +		};
> +	};
> +};
> +
> +&iomuxc {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_reset_out>;
> +
> +	pinctrl_reset_out: rstoutgrp {
> +		fsl,pins = <
> +			MX6UL_PAD_SNVS_TAMPER9__GPIO5_IO09      0x1b0b0
> +		>;
> +	};
> +
> +	pinctrl_ecspi2: ecspi2grp {
> +		fsl,pins = <
> +			MX6UL_PAD_CSI_DATA03__ECSPI2_MISO      0x100b1
> +			MX6UL_PAD_CSI_DATA02__ECSPI2_MOSI      0x100b1
> +			MX6UL_PAD_CSI_DATA00__ECSPI2_SCLK      0x100b1
> +			MX6UL_PAD_CSI_DATA01__GPIO4_IO22       0x100b1
> +		>;
> +	};
> +
> +	pinctrl_enet1: enet1grp {
> +		fsl,pins = <
> +			MX6UL_PAD_ENET1_RX_EN__ENET1_RX_EN      0x1b0b0
> +			MX6UL_PAD_ENET1_RX_ER__ENET1_RX_ER      0x1b0b0
> +			MX6UL_PAD_ENET1_RX_DATA0__ENET1_RDATA00 0x1b0b0
> +			MX6UL_PAD_ENET1_RX_DATA1__ENET1_RDATA01 0x1b0b0
> +			MX6UL_PAD_ENET1_TX_EN__ENET1_TX_EN      0x1b0b0
> +			MX6UL_PAD_ENET1_TX_DATA0__ENET1_TDATA00 0x1b0b0
> +			MX6UL_PAD_ENET1_TX_DATA1__ENET1_TDATA01 0x1b0b0
> +			MX6UL_PAD_ENET1_TX_CLK__ENET1_REF_CLK1  0x4001b009
> +		>;
> +	};
> +
> +	pinctrl_enet1_mdio: enet1mdiogrp {
> +		fsl,pins = <
> +			MX6UL_PAD_GPIO1_IO07__ENET1_MDC         0x1b0b0
> +			MX6UL_PAD_GPIO1_IO06__ENET1_MDIO        0x1b0b0
> +		>;
> +	};
> +
> +	pinctrl_qspi: qspigrp {
> +		fsl,pins = <
> +			MX6UL_PAD_NAND_WP_B__QSPI_A_SCLK        0x70a1
> +			MX6UL_PAD_NAND_READY_B__QSPI_A_DATA00   0x70a1
> +			MX6UL_PAD_NAND_CE0_B__QSPI_A_DATA01     0x70a1
> +			MX6UL_PAD_NAND_CE1_B__QSPI_A_DATA02     0x70a1
> +			MX6UL_PAD_NAND_CLE__QSPI_A_DATA03       0x70a1
> +			MX6UL_PAD_NAND_DQS__QSPI_A_SS0_B        0x70a1
> +		>;
> +	};
> +};
> -- 
> 2.17.1
> 

^ permalink raw reply

* Re: [PATCH v3] ARM: dts: vf610-bk4: Fix qspi node description
From: Shawn Guo @ 2019-08-03 15:53 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Fabio Estevam, Sascha Hauer, Stefan Agner, Rob Herring,
	Mark Rutland, linux-arm-kernel, devicetree, linux-kernel
In-Reply-To: <20190731141151.7196-1-lukma@denx.de>

On Wed, Jul 31, 2019 at 04:11:51PM +0200, Lukasz Majewski wrote:
> Before this change the device tree description of qspi node for
> second memory on BK4 board was wrong (applicable to old, removed
> fsl-quadspi.c driver).
> 
> As a result this memory was not recognized correctly when used
> with the new spi-fsl-qspi.c driver.
> 
> From the dt-bindings:
> 
> "Required SPI slave node properties:
>   - reg: There are two buses (A and B) with two chip selects each.
> This encodes to which bus and CS the flash is connected:
> <0>: Bus A, CS 0
> <1>: Bus A, CS 1
> <2>: Bus B, CS 0
> <3>: Bus B, CS 1"
> 
> According to above with new driver the second SPI-NOR memory shall
> have reg=<2> as it is connected to Bus B, CS 0.
> 
> Fixes: a67d2c52a82f ("ARM: dts: Add support for Liebherr's BK4 device (vf610 based)")
> Suggested-by: Fabio Estevam <festevam@gmail.com>
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> Reviewed-by: Fabio Estevam <festevam@gmail.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Sowjanya Komatineni @ 2019-08-03 17:01 UTC (permalink / raw)
  To: Dmitry Osipenko, thierry.reding, jonathanh, tglx, jason,
	marc.zyngier, linus.walleij, stefan, mark.rutland
  Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
	josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
	robh+dt, devicetree
In-Reply-To: <8d30f325-1c63-3802-7c21-f313115f8e55@gmail.com>


On 8/3/19 3:33 AM, Dmitry Osipenko wrote:
> 03.08.2019 2:51, Sowjanya Komatineni пишет:
>> On 8/2/19 2:15 PM, Dmitry Osipenko wrote:
>>> 02.08.2019 23:32, Sowjanya Komatineni пишет:
>>>> On 8/2/19 1:17 PM, Dmitry Osipenko wrote:
>>>>> 02.08.2019 23:13, Dmitry Osipenko пишет:
>>>>>> 02.08.2019 21:33, Sowjanya Komatineni пишет:
>>>>>>> On 8/2/19 5:38 AM, Dmitry Osipenko wrote:
>>>>>>>> 02.08.2019 2:49, Sowjanya Komatineni пишет:
>>>>>>>>> On 8/1/19 4:19 PM, Sowjanya Komatineni wrote:
>>>>>>>>>> On 8/1/19 2:30 PM, Sowjanya Komatineni wrote:
>>>>>>>>>>> On 8/1/19 1:54 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>> 01.08.2019 23:31, Sowjanya Komatineni пишет:
>>>>>>>>>>>>> On 8/1/19 1:17 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>> 01.08.2019 22:42, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>> On 8/1/19 12:00 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>> 01.08.2019 20:58, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>>> On 7/31/19 4:09 PM, Sowjanya Komatineni wrote:
>>>>>>>>>>>>>>>>>> On 7/31/19 3:44 AM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>>>>> 31.07.2019 12:50, Dmitry Osipenko пишет:
>>>>>>>>>>>>>>>>>>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>>>>>>> This patch implements save and restore context for
>>>>>>>>>>>>>>>>>>>>> peripheral
>>>>>>>>>>>>>>>>>>>>> fixed
>>>>>>>>>>>>>>>>>>>>> clock ops, peripheral gate clock ops, sdmmc mux clock
>>>>>>>>>>>>>>>>>>>>> ops, and
>>>>>>>>>>>>>>>>>>>>> peripheral clock ops.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> During system suspend, core power goes off and
>>>>>>>>>>>>>>>>>>>>> looses the
>>>>>>>>>>>>>>>>>>>>> settings
>>>>>>>>>>>>>>>>>>>>> of the Tegra CAR controller registers.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> So during suspend entry clock and reset state of
>>>>>>>>>>>>>>>>>>>>> peripherals is
>>>>>>>>>>>>>>>>>>>>> saved
>>>>>>>>>>>>>>>>>>>>> and on resume they are restored to have clocks back to
>>>>>>>>>>>>>>>>>>>>> same
>>>>>>>>>>>>>>>>>>>>> rate and
>>>>>>>>>>>>>>>>>>>>> state as before suspend.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>>>>>>>>>>>>>>>>>> Signed-off-by: Sowjanya Komatineni
>>>>>>>>>>>>>>>>>>>>> <skomatineni@nvidia.com>
>>>>>>>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>>>>>>>          drivers/clk/tegra/clk-periph-fixed.c | 33
>>>>>>>>>>>>>>>>>>>>> ++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>>          drivers/clk/tegra/clk-periph-gate.c  | 34
>>>>>>>>>>>>>>>>>>>>> +++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>>          drivers/clk/tegra/clk-periph.c       | 37
>>>>>>>>>>>>>>>>>>>>> ++++++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>>          drivers/clk/tegra/clk-sdmmc-mux.c    | 28
>>>>>>>>>>>>>>>>>>>>> +++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>>          drivers/clk/tegra/clk.h              | 6 ++++++
>>>>>>>>>>>>>>>>>>>>>          5 files changed, 138 insertions(+)
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>> index c088e7a280df..21b24530fa00 100644
>>>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>> @@ -60,11 +60,44 @@
>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_recalc_rate(struct
>>>>>>>>>>>>>>>>>>>>> clk_hw *hw,
>>>>>>>>>>>>>>>>>>>>>              return (unsigned long)rate;
>>>>>>>>>>>>>>>>>>>>>          }
>>>>>>>>>>>>>>>>>>>>>          +static int
>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_save_context(struct
>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>> +    struct tegra_clk_periph_fixed *fixed =
>>>>>>>>>>>>>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>>>>>>>>>>>>>> +    u32 mask = 1 << (fixed->num % 32);
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> +    fixed->enb_ctx = readl_relaxed(fixed->base +
>>>>>>>>>>>>>>>>>>>>> fixed->regs->enb_reg) &
>>>>>>>>>>>>>>>>>>>>> +             mask;
>>>>>>>>>>>>>>>>>>>>> +    fixed->rst_ctx = readl_relaxed(fixed->base +
>>>>>>>>>>>>>>>>>>>>> fixed->regs->rst_reg) &
>>>>>>>>>>>>>>>>>>>>> +             mask;
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> +    return 0;
>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> +static void
>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_restore_context(struct
>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>> +    struct tegra_clk_periph_fixed *fixed =
>>>>>>>>>>>>>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>>>>>>>>>>>>>> +    u32 mask = 1 << (fixed->num % 32);
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> +    if (fixed->enb_ctx)
>>>>>>>>>>>>>>>>>>>>> +        writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>>>> fixed->regs->enb_set_reg);
>>>>>>>>>>>>>>>>>>>>> +    else
>>>>>>>>>>>>>>>>>>>>> +        writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>>>> fixed->regs->enb_clr_reg);
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> +    udelay(2);
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> +    if (!fixed->rst_ctx) {
>>>>>>>>>>>>>>>>>>>>> +        udelay(5); /* reset propogation delay */
>>>>>>>>>>>>>>>>>>>>> +        writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>>>> fixed->regs->rst_reg);
>>>>>>>>>>>>>>>>>>>>> +    }
>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>          static const struct clk_ops
>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_ops
>>>>>>>>>>>>>>>>>>>>> = {
>>>>>>>>>>>>>>>>>>>>>              .is_enabled =
>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_is_enabled,
>>>>>>>>>>>>>>>>>>>>>              .enable = tegra_clk_periph_fixed_enable,
>>>>>>>>>>>>>>>>>>>>>              .disable = tegra_clk_periph_fixed_disable,
>>>>>>>>>>>>>>>>>>>>>              .recalc_rate =
>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_recalc_rate,
>>>>>>>>>>>>>>>>>>>>> +    .save_context =
>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_save_context,
>>>>>>>>>>>>>>>>>>>>> +    .restore_context =
>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_restore_context,
>>>>>>>>>>>>>>>>>>>>>          };
>>>>>>>>>>>>>>>>>>>>>            struct clk
>>>>>>>>>>>>>>>>>>>>> *tegra_clk_register_periph_fixed(const
>>>>>>>>>>>>>>>>>>>>> char
>>>>>>>>>>>>>>>>>>>>> *name,
>>>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>> index 4b31beefc9fc..6ba5b08e0787 100644
>>>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>> @@ -25,6 +25,8 @@ static
>>>>>>>>>>>>>>>>>>>>> DEFINE_SPINLOCK(periph_ref_lock);
>>>>>>>>>>>>>>>>>>>>>            #define read_rst(gate) \
>>>>>>>>>>>>>>>>>>>>>              readl_relaxed(gate->clk_base +
>>>>>>>>>>>>>>>>>>>>> (gate->regs->rst_reg))
>>>>>>>>>>>>>>>>>>>>> +#define write_rst_set(val, gate) \
>>>>>>>>>>>>>>>>>>>>> +    writel_relaxed(val, gate->clk_base +
>>>>>>>>>>>>>>>>>>>>> (gate->regs->rst_set_reg))
>>>>>>>>>>>>>>>>>>>>>          #define write_rst_clr(val, gate) \
>>>>>>>>>>>>>>>>>>>>>              writel_relaxed(val, gate->clk_base +
>>>>>>>>>>>>>>>>>>>>> (gate->regs->rst_clr_reg))
>>>>>>>>>>>>>>>>>>>>>          @@ -110,10 +112,42 @@ static void
>>>>>>>>>>>>>>>>>>>>> clk_periph_disable(struct
>>>>>>>>>>>>>>>>>>>>> clk_hw *hw)
>>>>>>>>>>>>>>>>>>>>> spin_unlock_irqrestore(&periph_ref_lock, flags);
>>>>>>>>>>>>>>>>>>>>>          }
>>>>>>>>>>>>>>>>>>>>>          +static int clk_periph_gate_save_context(struct
>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>> +    struct tegra_clk_periph_gate *gate =
>>>>>>>>>>>>>>>>>>>>> to_clk_periph_gate(hw);
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> +    gate->clk_state_ctx = read_enb(gate) &
>>>>>>>>>>>>>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>>>>>>>>>>>>>> +    gate->rst_state_ctx = read_rst(gate) &
>>>>>>>>>>>>>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> +    return 0;
>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> +static void clk_periph_gate_restore_context(struct
>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>> +    struct tegra_clk_periph_gate *gate =
>>>>>>>>>>>>>>>>>>>>> to_clk_periph_gate(hw);
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> +    if (gate->clk_state_ctx)
>>>>>>>>>>>>>>>>>>>>> + write_enb_set(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>> +    else
>>>>>>>>>>>>>>>>>>>>> + write_enb_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> +    udelay(5);
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> +    if (!(gate->flags & TEGRA_PERIPH_NO_RESET) &&
>>>>>>>>>>>>>>>>>>>>> +        !(gate->flags & TEGRA_PERIPH_MANUAL_RESET)) {
>>>>>>>>>>>>>>>>>>>>> +        if (gate->rst_state_ctx)
>>>>>>>>>>>>>>>>>>>>> + write_rst_set(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>> +        else
>>>>>>>>>>>>>>>>>>>>> + write_rst_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>> +    }
>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>          const struct clk_ops
>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_gate_ops = {
>>>>>>>>>>>>>>>>>>>>>              .is_enabled = clk_periph_is_enabled,
>>>>>>>>>>>>>>>>>>>>>              .enable = clk_periph_enable,
>>>>>>>>>>>>>>>>>>>>>              .disable = clk_periph_disable,
>>>>>>>>>>>>>>>>>>>>> +    .save_context = clk_periph_gate_save_context,
>>>>>>>>>>>>>>>>>>>>> +    .restore_context =
>>>>>>>>>>>>>>>>>>>>> clk_periph_gate_restore_context,
>>>>>>>>>>>>>>>>>>>>>          };
>>>>>>>>>>>>>>>>>>>>>            struct clk
>>>>>>>>>>>>>>>>>>>>> *tegra_clk_register_periph_gate(const
>>>>>>>>>>>>>>>>>>>>> char *name,
>>>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>> index 58437da25156..06fb62955768 100644
>>>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>> @@ -99,6 +99,37 @@ static void
>>>>>>>>>>>>>>>>>>>>> clk_periph_disable(struct
>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>              gate_ops->disable(gate_hw);
>>>>>>>>>>>>>>>>>>>>>          }
>>>>>>>>>>>>>>>>>>>>>          +static int clk_periph_save_context(struct
>>>>>>>>>>>>>>>>>>>>> clk_hw *hw)
>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>> +    struct tegra_clk_periph *periph =
>>>>>>>>>>>>>>>>>>>>> to_clk_periph(hw);
>>>>>>>>>>>>>>>>>>>>> +    const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>>>>>>>>>>>>>>>> +    struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> +    if (!(periph->gate.flags & TEGRA_PERIPH_NO_GATE))
>>>>>>>>>>>>>>>>>>>>> +        gate_ops->save_context(gate_hw);
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> +    periph->parent_ctx = clk_periph_get_parent(hw);
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> +    return 0;
>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> +static void clk_periph_restore_context(struct clk_hw
>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>> +    struct tegra_clk_periph *periph =
>>>>>>>>>>>>>>>>>>>>> to_clk_periph(hw);
>>>>>>>>>>>>>>>>>>>>> +    const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>>>>>>>>>>>>>>>> +    struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>>>>>>>>>>>>>> +    const struct clk_ops *div_ops = periph->div_ops;
>>>>>>>>>>>>>>>>>>>>> +    struct clk_hw *div_hw = &periph->divider.hw;
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> +    clk_periph_set_parent(hw, periph->parent_ctx);
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> +    if (!(periph->gate.flags & TEGRA_PERIPH_NO_DIV))
>>>>>>>>>>>>>>>>>>>>> + div_ops->restore_context(div_hw);
>>>>>>>>>>>>>>>>>>>> Could you please point to where the divider's
>>>>>>>>>>>>>>>>>>>> save_context()
>>>>>>>>>>>>>>>>>>>> happens?
>>>>>>>>>>>>>>>>>>>> Because I can't see it.
>>>>>>>>>>>>>>>>>>> Ah, I now see that there is no need to save the dividers
>>>>>>>>>>>>>>>>>>> context
>>>>>>>>>>>>>>>>>>> because
>>>>>>>>>>>>>>>>>>> clk itself has enough info that is needed for the
>>>>>>>>>>>>>>>>>>> context's
>>>>>>>>>>>>>>>>>>> restoring
>>>>>>>>>>>>>>>>>>> (like I pointed in the review to v6).
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Looks like you could also implement a new
>>>>>>>>>>>>>>>>>>> clk_hw_get_parent_index()
>>>>>>>>>>>>>>>>>>> generic helper to get the index instead of storing it
>>>>>>>>>>>>>>>>>>> manually.
>>>>>>>>>>>>>>>>>> clk_periph_get_parent basically invokes existing
>>>>>>>>>>>>>>>>>> clk_mux_ops
>>>>>>>>>>>>>>>>>> get_parent() which is then saved in tegra_clk_periph.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> All existing drivers are using directly get_parent() from
>>>>>>>>>>>>>>>>>> clk_mux
>>>>>>>>>>>>>>>>>> which actually gets index from the register read.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> To have this more generic w.r.t save/restore context
>>>>>>>>>>>>>>>>>> point of
>>>>>>>>>>>>>>>>>> view,
>>>>>>>>>>>>>>>>>> probably instead of implementing new get_parent_index
>>>>>>>>>>>>>>>>>> helper,
>>>>>>>>>>>>>>>>>> I think
>>>>>>>>>>>>>>>>>> its better to implement save_context and
>>>>>>>>>>>>>>>>>> restore_context to
>>>>>>>>>>>>>>>>>> clk_mux_ops along with creating parent_index field into
>>>>>>>>>>>>>>>>>> clk_mux to
>>>>>>>>>>>>>>>>>> cache index during set_parent.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> So we just need to invoke mux_ops save_context and
>>>>>>>>>>>>>>>>>> restore_context.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I hope its ok to add save/restore context to clk_mux_ops
>>>>>>>>>>>>>>>>> to be
>>>>>>>>>>>>>>>>> more
>>>>>>>>>>>>>>>>> generic w.r.t save/restore context rather than
>>>>>>>>>>>>>>>>> get_parent_index
>>>>>>>>>>>>>>>>> API.
>>>>>>>>>>>>>>>>> Please confirm if you agree.
>>>>>>>>>>>>>>>> Sounds like a good idea. I see that there is a 'restoring'
>>>>>>>>>>>>>>>> helper for
>>>>>>>>>>>>>>>> the generic clk_gate, seems something similar could be done
>>>>>>>>>>>>>>>> for the
>>>>>>>>>>>>>>>> clk_mux. And looks like anyway you'll need to associate the
>>>>>>>>>>>>>>>> parent
>>>>>>>>>>>>>>>> clock
>>>>>>>>>>>>>>>> with the hw index in order to restore the muxing.
>>>>>>>>>>>>>>> by 'restoring' helper for generic clk_gate, are you
>>>>>>>>>>>>>>> referring to
>>>>>>>>>>>>>>> clk_gate_restore_context API?
>>>>>>>>>>>>>> Yes.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> clk_gate_restore_context is API that's any clk drivers can
>>>>>>>>>>>>>>> use for
>>>>>>>>>>>>>>> clk_gate operation restore for custom gate clk_ops.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> But clk-periph is directly using generic clk_mux ops from
>>>>>>>>>>>>>>> clk_mux
>>>>>>>>>>>>>>> so I
>>>>>>>>>>>>>>> think we should add .restore_context to clk_mux_ops and then
>>>>>>>>>>>>>>> during
>>>>>>>>>>>>>>> clk-periph restore need to invoke mux_ops->restore_context.
>>>>>>>>>>>>>> I'm not sure whether it will be good for every driver that
>>>>>>>>>>>>>> uses
>>>>>>>>>>>>>> generic
>>>>>>>>>>>>>> clk_mux ops. Should be more flexible to have a generic helper
>>>>>>>>>>>>>> function
>>>>>>>>>>>>>> that any driver could use in order to restore the clock's
>>>>>>>>>>>>>> parent.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The clk-periph restoring also includes case of combining
>>>>>>>>>>>>>> divider
>>>>>>>>>>>>>> and
>>>>>>>>>>>>>> parent restoring, so generic helper could be useful in that
>>>>>>>>>>>>>> case
>>>>>>>>>>>>>> as well.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> It also looks like you could actually use the
>>>>>>>>>>>>>> clk_gate_restore_context()
>>>>>>>>>>>>>> instead of manually saving the clock's enable-state, couldn't
>>>>>>>>>>>>>> you?
>>>>>>>>>>>>> ok for clk_mux, can add generic clk_mux_restore_context API
>>>>>>>>>>>>> rather
>>>>>>>>>>>>> than
>>>>>>>>>>>>> using restore_context in clk_ops and will invoke that during
>>>>>>>>>>>>> clk_periph
>>>>>>>>>>>>> restore.
>>>>>>>>>>>>>
>>>>>>>>> digging thru looks like for clk_periph source restore instead of
>>>>>>>>> clk_mux_restore_context, i can directly do clk_hw_get_parent and
>>>>>>>>> clk_set_parent with mux_hw as they invoke mux_ops get/set parent
>>>>>>>>> anyway.
>>>>>>>>> Will do this for periph clk mux
>> Just to be clear, clk_mux don't have cached parent. get_parent is by
>> register read. So, cant directly use get_parent and then set during
>> restore.
>>
>> So will create both clk_mux_save/restore_context in generic clk driver
>> and will invoke them during tegra peripheral clock suspend/resume.
> Why MUX clock doesn't have a cached parent? What MUX clock you're
> talking about?
>
> [snip]

Please ignore got it.

Will send next version after giving few more days for feedback.

^ permalink raw reply

* Re: [PATCH 3/5] MIPS: lantiq: add an irq_domain and irq_chip for EBU
From: Martin Blumenstingl @ 2019-08-03 17:33 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: tglx, jason, ralf, paul.burton, jhogan, robh+dt, linux-mips,
	devicetree, linux-kernel, mark.rutland, john, Hauke Mehrtens
In-Reply-To: <86o916mx2m.wl-maz@kernel.org>

Hi Marc,

On Sat, Aug 3, 2019 at 11:12 AM Marc Zyngier <maz@kernel.org> wrote:
>
> Hi Martin,
>
> On Thu, 01 Aug 2019 18:42:42 +0100,
> Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:
>
> [...]
>
> > > > +static void ltq_ebu_irq_handler(struct irq_desc *desc)
> > > > +{
> > > > +     struct irq_domain *domain = irq_desc_get_handler_data(desc);
> > > > +     struct irq_chip *irqchip = irq_desc_get_chip(desc);
> > > > +
> > > > +     chained_irq_enter(irqchip, desc);
> > > > +
> > > > +     generic_handle_irq(irq_find_mapping(domain, 0));
> > >
> > > Having an irqdomain for a single interrupt is a bit over the top... Is
> > > that for the convenience of the DT infrastructure?
> > yes, I did it to get DT support
> > please let me know if there's a "better" way (preferably with another
> > driver as example)
>
> To be honest, the chained handler is what troubles me the most. You
> normally would use such a construct if you had a multiplexer. In your
> case, you have a 1:1 relationship between input and output. It is just
> that this irqchip allows the trigger to be adapted, which normally
> calls for a hierarchical implementation.
>
> In your case, with only a single interrupt, it doesn't matter much
> though.
I see, thank you for the explanation

can you name a driver for a hierarchical irqchip driver that you
consider "clean" which I could use as reference?
I am considering to still convert it to a hierarchical irqchip driver
to keep it consistent with at least two more upcoming Lantiq irqchip
drivers (which both seem to be similar use-cases as this one, they
just provide more than one interrupt):
- there's a PCI legacy interrupt controller in the PCIe controller's
app registers. it takes 4 parent interrupts and provides
PCI_INT{A,B,C,D}. the interrupts need to be enabled and ACK'ed in the
PCIe app registers as well as in the parent interrupt controller
- the EIU (External Interrupt Unit) in my own words is the GPIO
interrupt controller. it takes up to 6 parent interrupts and provides
one interrupt for each "EXIN GPIO". setting the IRQ type and ACK need
to happen through the EIU registers as well as the parent interrupt
controller

my initial thought is that it's best to follow one programming model
(which based on your suggestion would be a hierarchical irqchip) for
all three IRQ controllers

> >
> > [...]
> > > > +     irq_create_mapping(domain, 0);
> > >
> > > Why do you need to perform this eagerly? I'd expect this interrupt to
> > > be mapped when it is actually claimed by a driver.
> > I don't remember why I added it, it may be left-over from copying from
> > another driver
> > in v2 I'll try to drop it
> >
> > > > +
> > > > +     irq_set_chained_handler_and_data(irq, ltq_ebu_irq_handler, domain);
> > >
> > > And there is no HW initialisation whatsoever? I'd expect, at the very
> > > least, the sole interrupt to be configured as disabled/masked.
> > I can add that. is there any "best practice" on what I should
> > initialize (just disable it or also set a "default" mode like
> > LEVEL_LOW)?
>
> Whichever default state makes sense. What you want to avoid is to boot
> the kernel with a screaming interrupt because some firmware has left
> it enabled.
noted, thank you


Martin

^ permalink raw reply

* Re: [PATCH v2 3/6] arm64: dts: amlogic: g12: add temperature sensor
From: Martin Blumenstingl @ 2019-08-03 17:52 UTC (permalink / raw)
  To: Guillaume La Roque
  Cc: daniel.lezcano, khilman, devicetree, linux-amlogic, linux-kernel,
	linux-arm-kernel, linux-pm
In-Reply-To: <20190731153529.30159-4-glaroque@baylibre.com>

Hi Guillaume,

On Wed, Jul 31, 2019 at 5:36 PM Guillaume La Roque
<glaroque@baylibre.com> wrote:
>
> Add cpu and ddr temperature sensors for G12 Socs
>
> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
with the nit-pick below addressed:
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

> ---
>  .../boot/dts/amlogic/meson-g12-common.dtsi    | 22 +++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
> index 06e186ca41e3..7f862a3490fb 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
> @@ -1353,6 +1353,28 @@
>                                 };
>                         };
>
> +                       cpu_temp: temperature-sensor@34800 {
> +                               compatible = "amlogic,g12-cpu-thermal",
> +                                            "amlogic,g12-thermal";
> +                               reg = <0x0 0x34800 0x0 0x50>;
> +                               interrupts = <GIC_SPI 35 IRQ_TYPE_EDGE_RISING>;
> +                               clocks = <&clkc CLKID_TS>;
> +                               status = "okay";
I believe nodes are enabled automatically if they don't have a status property

> +                               #thermal-sensor-cells = <0>;
> +                               amlogic,ao-secure = <&sec_AO>;
> +                       };
> +
> +                       ddr_temp: temperature-sensor@34c00 {
> +                               compatible = "amlogic,g12-ddr-thermal",
> +                                            "amlogic,g12-thermal";
> +                               reg = <0x0 0x34c00 0x0 0x50>;
> +                               interrupts = <GIC_SPI 36 IRQ_TYPE_EDGE_RISING>;
> +                               clocks = <&clkc CLKID_TS>;
> +                               status = "okay";
same here


Martin

^ permalink raw reply

* Re: [PATCH v2 2/6] thermal: amlogic: Add thermal driver to support G12 SoCs
From: Martin Blumenstingl @ 2019-08-03 18:24 UTC (permalink / raw)
  To: Guillaume La Roque
  Cc: daniel.lezcano, khilman, devicetree, linux-amlogic, linux-kernel,
	linux-arm-kernel, linux-pm
In-Reply-To: <20190731153529.30159-3-glaroque@baylibre.com>

Hi Guillaume,

(I still don't have any experience with the thermal framework, so
below are some general comments)

On Wed, Jul 31, 2019 at 5:36 PM Guillaume La Roque
<glaroque@baylibre.com> wrote:
I would add a patch description here:
"
Amlogic G12A and G12B SoCs integrate two thermal sensors with the same design.
One is located close to the DDR (controller?) and the other one is
located close to the PLLs (between the CPU and GPU).

The calibration data for each of the thermal sensors instance is
stored in a different location within the AO region.

Implement reading the temperature from each thermal sensor.

The IP block has more functionality, which may be added to this driver
in the future:
- reading up to 16 stored temperature samples
- chip reset when the temperature exceeds a configurable threshold
- up to four interrupts when the temperature has risen above a
configurable threshold
- up to four interrupts when the temperature has fallen below a
configurable threshold
"

> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
> ---
>  drivers/thermal/Kconfig           |  11 +
>  drivers/thermal/Makefile          |   1 +
>  drivers/thermal/amlogic_thermal.c | 332 ++++++++++++++++++++++++++++++
>  3 files changed, 344 insertions(+)
>  create mode 100644 drivers/thermal/amlogic_thermal.c
>
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 9966364a6deb..0f31bb4bc372 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -348,6 +348,17 @@ config MTK_THERMAL
>           Enable this option if you want to have support for thermal management
>           controller present in Mediatek SoCs
>
> +config AMLOGIC_THERMAL
we typically use "MESON" in the Kconfig symbols:
$ grep -c AMLOGIC .config
1
$ grep -c MESON .config
33

I also wonder if we should add G12 or G12A so we don't conflict with
upcoming thermal sensors with a different design (assuming that this
will be a thing).
for example we already have three different USB2 PHY drivers

[...]
> +/*
> + * Calculate a temperature value from a temperature code.
> + * The unit of the temperature is degree Celsius.
is it really degree Celsius or millicelsius?

> + */
> +static int code_to_temp(struct amlogic_thermal *pdata, int temp_code)
PREFIX_thermal_code_to_millicelsius?

[...]
> +static int amlogic_thermal_enable(struct amlogic_thermal *data)
> +{
> +       clk_prepare_enable(data->clk);
no clock error handling?

> +       regmap_update_bits(data->regmap, TSENSOR_CFG_REG1,
> +                          TSENSOR_CFG_REG1_ENABLE, TSENSOR_CFG_REG1_ENABLE);
> +
> +       return 0;
> +}
> +
> +static int amlogic_thermal_disable(struct amlogic_thermal *data)
> +{
> +       regmap_update_bits(data->regmap, TSENSOR_CFG_REG1,
> +                          TSENSOR_CFG_REG1_ENABLE, 0);
> +       clk_disable(data->clk);
either clk_disable_unprepare here or use only clk_enable in
amlogic_thermal_enable and move prepare/unprepare somewhere else

> +
> +       return 0;
> +}
> +
> +static int amlogic_thermal_get_temp(void *data, int *temp)
> +{
> +       unsigned int tvalue;
> +       struct amlogic_thermal *pdata = data;
> +
> +       if (!data)
> +               return -EINVAL;
> +
> +       regmap_read(pdata->regmap, TSENSOR_STAT0, &tvalue);
> +       *temp = code_to_temp(pdata,
> +                            tvalue & TSENSOR_READ_TEMP_MASK);
maybe simply move the implementation from code_to_temp here?

[...]
> +static const struct amlogic_thermal_data amlogic_thermal_g12_cpu_param = {
> +       .u_efuse_off = 0x128,
> +       .soc = &amlogic_thermal_g12,
based on the variable name I expected this to be an enum of some sort.
I would have expected it to be called calibration_parameters or
similar to match the explanation in the driver description
(no need to change it if you prefer it like this, I just want to make
you aware of this)

> +       .regmap_config = &amlogic_thermal_regmap_config_g12,
if regmap_config is always the same you may also pass it directly to
devm_regmap_init_mmio

> +};
> +
> +static const struct amlogic_thermal_data amlogic_thermal_g12_ddr_param = {
> +       .u_efuse_off = 0xF0,
I believe we use lower-case hex everywhere else

[...]
> +static const struct of_device_id of_amlogic_thermal_match[] = {
> +       {
> +               .compatible = "amlogic,g12-ddr-thermal",
> +               .data = &amlogic_thermal_g12_ddr_param,
> +       },
> +       {
> +               .compatible = "amlogic,g12-cpu-thermal",
> +               .data = &amlogic_thermal_g12_cpu_param,
> +       },
> +       { /* end */ }
other drivers use "sentinel", but personally I have no preference here

[...]
> +       pdata->tzd = devm_thermal_zone_of_sensor_register
> +                               (&pdev->dev,
> +                                0,
> +                                pdata,
> +                                &amlogic_thermal_ops);
I believe the opening brace has to go onto the same line as the function name

[...]
> +       ret = amlogic_thermal_enable(pdata);
> +       if (ret)
> +               clk_unprepare(pdata->clk);
clk_disable_unprepare, else you'll leave the clock prepared

> +#ifdef CONFIG_PM_SLEEP
> +static int amlogic_thermal_suspend(struct device *dev)
> +{
> +       struct amlogic_thermal *data = dev_get_drvdata(dev);
> +
> +       return amlogic_thermal_disable(data);
> +}
> +
> +static int amlogic_thermal_resume(struct device *dev)
> +{
> +       struct amlogic_thermal *data = dev_get_drvdata(dev);
> +
> +       return amlogic_thermal_enable(data);
> +}
> +#endif
instead of using an #ifdef here annotate the suspend/resume functions
with __maybe_unused, see [0]


Martin


[0] https://lore.kernel.org/patchwork/patch/732981/

^ permalink raw reply

* Re: [PATCH v2 4/6] arm64: dts: meson: sei510: Add minimal thermal zone
From: Martin Blumenstingl @ 2019-08-03 18:29 UTC (permalink / raw)
  To: Guillaume La Roque
  Cc: daniel.lezcano, khilman, devicetree, linux-amlogic, linux-kernel,
	linux-arm-kernel, linux-pm
In-Reply-To: <20190731153529.30159-5-glaroque@baylibre.com>

Hi Guillaume,

On Wed, Jul 31, 2019 at 5:36 PM Guillaume La Roque
<glaroque@baylibre.com> wrote:
>
> Add minimal thermal zone for DDR and CPU sensor
so high DDR (controller?) temperatures will throttle Mali and high PLL
temperatures will throttle the CPU?
to get things started I'm fine with this, but I think it should be
mentioned here

> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
> ---
>  .../boot/dts/amlogic/meson-g12a-sei510.dts    | 56 +++++++++++++++++++
>  1 file changed, 56 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts b/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts
> index 979449968a5f..2c16a2cba0a3 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts
> @@ -10,6 +10,7 @@
>  #include <dt-bindings/input/input.h>
>  #include <dt-bindings/gpio/meson-g12a-gpio.h>
>  #include <dt-bindings/sound/meson-g12a-tohdmitx.h>
> +#include <dt-bindings/thermal/thermal.h>
>
>  / {
>         compatible = "seirobotics,sei510", "amlogic,g12a";
> @@ -33,6 +34,53 @@
>                 ethernet0 = &ethmac;
>         };
>
> +       thermal-zones {
> +               cpu-thermal {
> +                       polling-delay = <1000>;
> +                       polling-delay-passive = <100>;
> +                       thermal-sensors = <&cpu_temp>;
> +
> +                       trips {
> +                               cpu_critical: cpu-critical {
> +                                       temperature = <110000>; /* millicelsius */
> +                                       hysteresis = <2000>; /* millicelsius */
> +                                       type = "critical";
> +                               };
> +                       };
> +
> +                       cooling-maps {
> +                               map {
> +                                       trip = <&cpu_critical>;
> +                                       cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +                                                        <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +                                                        <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +                                                        <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +                               };
> +                       };
> +               };
> +
> +               ddr-thermal {
> +                       polling-delay = <1000>;
> +                       polling-delay-passive = <100>;
> +                       thermal-sensors = <&ddr_temp>;
> +
> +                       trips {
> +                               ddr_critical: ddr-critical {
> +                                       temperature = <110000>; /* millicelsius */
> +                                       hysteresis = <2000>; /* millicelsius */
> +                                       type = "critical";
> +                               };
> +                       };
> +
> +                       cooling-maps {
> +                               map {
> +                                       trip = <&ddr_critical>;
> +                                       cooling-device = <&mali THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +                               };
> +                       };
> +               };
> +       };
> +
>         mono_dac: audio-codec-0 {
>                 compatible = "maxim,max98357a";
>                 #sound-dai-cells = <0>;
> @@ -321,6 +369,7 @@
>         operating-points-v2 = <&cpu_opp_table>;
>         clocks = <&clkc CLKID_CPU_CLK>;
>         clock-latency = <50000>;
> +       #cooling-cells = <2>;
>  };
>
>  &cpu1 {
> @@ -328,6 +377,7 @@
>         operating-points-v2 = <&cpu_opp_table>;
>         clocks = <&clkc CLKID_CPU_CLK>;
>         clock-latency = <50000>;
> +       #cooling-cells = <2>;
>  };
>
>  &cpu2 {
> @@ -335,6 +385,7 @@
>         operating-points-v2 = <&cpu_opp_table>;
>         clocks = <&clkc CLKID_CPU_CLK>;
>         clock-latency = <50000>;
> +       #cooling-cells = <2>;
>  };
>
>  &cpu3 {
> @@ -342,6 +393,7 @@
>         operating-points-v2 = <&cpu_opp_table>;
>         clocks = <&clkc CLKID_CPU_CLK>;
>         clock-latency = <50000>;
> +       #cooling-cells = <2>;
>  };
>
>  &cvbs_vdac_port {
> @@ -368,6 +420,10 @@
>         status = "okay";
>  };
>
> +&mali {
> +       #cooling-cells = <2>;
> +};
is there something device-specific in this patch? I'm wondering
whether we can move all of this go g12a.dtsi to simplify maintenance
in the future


Martin

^ permalink raw reply

* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Sowjanya Komatineni @ 2019-08-03 23:44 UTC (permalink / raw)
  To: Dmitry Osipenko, thierry.reding, jonathanh, tglx, jason,
	marc.zyngier, linus.walleij, stefan, mark.rutland
  Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
	josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
	robh+dt, devicetree
In-Reply-To: <274e67b1-16b8-2475-d026-68bd89b090ec@nvidia.com>


On 8/3/19 10:01 AM, Sowjanya Komatineni wrote:
>
> On 8/3/19 3:33 AM, Dmitry Osipenko wrote:
>> 03.08.2019 2:51, Sowjanya Komatineni пишет:
>>> On 8/2/19 2:15 PM, Dmitry Osipenko wrote:
>>>> 02.08.2019 23:32, Sowjanya Komatineni пишет:
>>>>> On 8/2/19 1:17 PM, Dmitry Osipenko wrote:
>>>>>> 02.08.2019 23:13, Dmitry Osipenko пишет:
>>>>>>> 02.08.2019 21:33, Sowjanya Komatineni пишет:
>>>>>>>> On 8/2/19 5:38 AM, Dmitry Osipenko wrote:
>>>>>>>>> 02.08.2019 2:49, Sowjanya Komatineni пишет:
>>>>>>>>>> On 8/1/19 4:19 PM, Sowjanya Komatineni wrote:
>>>>>>>>>>> On 8/1/19 2:30 PM, Sowjanya Komatineni wrote:
>>>>>>>>>>>> On 8/1/19 1:54 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>> 01.08.2019 23:31, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>> On 8/1/19 1:17 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>> 01.08.2019 22:42, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>> On 8/1/19 12:00 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>>> 01.08.2019 20:58, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>>>> On 7/31/19 4:09 PM, Sowjanya Komatineni wrote:
>>>>>>>>>>>>>>>>>>> On 7/31/19 3:44 AM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>>>>>> 31.07.2019 12:50, Dmitry Osipenko пишет:
>>>>>>>>>>>>>>>>>>>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>>>>>>>> This patch implements save and restore context for
>>>>>>>>>>>>>>>>>>>>>> peripheral
>>>>>>>>>>>>>>>>>>>>>> fixed
>>>>>>>>>>>>>>>>>>>>>> clock ops, peripheral gate clock ops, sdmmc mux 
>>>>>>>>>>>>>>>>>>>>>> clock
>>>>>>>>>>>>>>>>>>>>>> ops, and
>>>>>>>>>>>>>>>>>>>>>> peripheral clock ops.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> During system suspend, core power goes off and
>>>>>>>>>>>>>>>>>>>>>> looses the
>>>>>>>>>>>>>>>>>>>>>> settings
>>>>>>>>>>>>>>>>>>>>>> of the Tegra CAR controller registers.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> So during suspend entry clock and reset state of
>>>>>>>>>>>>>>>>>>>>>> peripherals is
>>>>>>>>>>>>>>>>>>>>>> saved
>>>>>>>>>>>>>>>>>>>>>> and on resume they are restored to have clocks 
>>>>>>>>>>>>>>>>>>>>>> back to
>>>>>>>>>>>>>>>>>>>>>> same
>>>>>>>>>>>>>>>>>>>>>> rate and
>>>>>>>>>>>>>>>>>>>>>> state as before suspend.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>>>>>>>>>>>>>>>>>>> Signed-off-by: Sowjanya Komatineni
>>>>>>>>>>>>>>>>>>>>>> <skomatineni@nvidia.com>
>>>>>>>>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph-fixed.c | 33
>>>>>>>>>>>>>>>>>>>>>> ++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph-gate.c | 34
>>>>>>>>>>>>>>>>>>>>>> +++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph.c | 37
>>>>>>>>>>>>>>>>>>>>>> ++++++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-sdmmc-mux.c | 28
>>>>>>>>>>>>>>>>>>>>>> +++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk.h | 6 ++++++
>>>>>>>>>>>>>>>>>>>>>>          5 files changed, 138 insertions(+)
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>>> index c088e7a280df..21b24530fa00 100644
>>>>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>>> @@ -60,11 +60,44 @@
>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_recalc_rate(struct
>>>>>>>>>>>>>>>>>>>>>> clk_hw *hw,
>>>>>>>>>>>>>>>>>>>>>>              return (unsigned long)rate;
>>>>>>>>>>>>>>>>>>>>>>          }
>>>>>>>>>>>>>>>>>>>>>>          +static int
>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_save_context(struct
>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>> +    struct tegra_clk_periph_fixed *fixed =
>>>>>>>>>>>>>>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>>>>>>>>>>>>>>> +    u32 mask = 1 << (fixed->num % 32);
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> +    fixed->enb_ctx = readl_relaxed(fixed->base +
>>>>>>>>>>>>>>>>>>>>>> fixed->regs->enb_reg) &
>>>>>>>>>>>>>>>>>>>>>> +             mask;
>>>>>>>>>>>>>>>>>>>>>> +    fixed->rst_ctx = readl_relaxed(fixed->base +
>>>>>>>>>>>>>>>>>>>>>> fixed->regs->rst_reg) &
>>>>>>>>>>>>>>>>>>>>>> +             mask;
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> +    return 0;
>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> +static void
>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_restore_context(struct
>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>> +    struct tegra_clk_periph_fixed *fixed =
>>>>>>>>>>>>>>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>>>>>>>>>>>>>>> +    u32 mask = 1 << (fixed->num % 32);
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> +    if (fixed->enb_ctx)
>>>>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>>>>> fixed->regs->enb_set_reg);
>>>>>>>>>>>>>>>>>>>>>> +    else
>>>>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>>>>> fixed->regs->enb_clr_reg);
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> +    udelay(2);
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> +    if (!fixed->rst_ctx) {
>>>>>>>>>>>>>>>>>>>>>> +        udelay(5); /* reset propogation delay */
>>>>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>>>>> fixed->regs->rst_reg);
>>>>>>>>>>>>>>>>>>>>>> +    }
>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>          static const struct clk_ops
>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_ops
>>>>>>>>>>>>>>>>>>>>>> = {
>>>>>>>>>>>>>>>>>>>>>>              .is_enabled =
>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_is_enabled,
>>>>>>>>>>>>>>>>>>>>>>              .enable = 
>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_enable,
>>>>>>>>>>>>>>>>>>>>>>              .disable = 
>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_disable,
>>>>>>>>>>>>>>>>>>>>>> .recalc_rate =
>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_recalc_rate,
>>>>>>>>>>>>>>>>>>>>>> +    .save_context =
>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_save_context,
>>>>>>>>>>>>>>>>>>>>>> +    .restore_context =
>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_restore_context,
>>>>>>>>>>>>>>>>>>>>>>          };
>>>>>>>>>>>>>>>>>>>>>>            struct clk
>>>>>>>>>>>>>>>>>>>>>> *tegra_clk_register_periph_fixed(const
>>>>>>>>>>>>>>>>>>>>>> char
>>>>>>>>>>>>>>>>>>>>>> *name,
>>>>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>>> index 4b31beefc9fc..6ba5b08e0787 100644
>>>>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>>> @@ -25,6 +25,8 @@ static
>>>>>>>>>>>>>>>>>>>>>> DEFINE_SPINLOCK(periph_ref_lock);
>>>>>>>>>>>>>>>>>>>>>>            #define read_rst(gate) \
>>>>>>>>>>>>>>>>>>>>>> readl_relaxed(gate->clk_base +
>>>>>>>>>>>>>>>>>>>>>> (gate->regs->rst_reg))
>>>>>>>>>>>>>>>>>>>>>> +#define write_rst_set(val, gate) \
>>>>>>>>>>>>>>>>>>>>>> +    writel_relaxed(val, gate->clk_base +
>>>>>>>>>>>>>>>>>>>>>> (gate->regs->rst_set_reg))
>>>>>>>>>>>>>>>>>>>>>>          #define write_rst_clr(val, gate) \
>>>>>>>>>>>>>>>>>>>>>> writel_relaxed(val, gate->clk_base +
>>>>>>>>>>>>>>>>>>>>>> (gate->regs->rst_clr_reg))
>>>>>>>>>>>>>>>>>>>>>>          @@ -110,10 +112,42 @@ static void
>>>>>>>>>>>>>>>>>>>>>> clk_periph_disable(struct
>>>>>>>>>>>>>>>>>>>>>> clk_hw *hw)
>>>>>>>>>>>>>>>>>>>>>> spin_unlock_irqrestore(&periph_ref_lock, flags);
>>>>>>>>>>>>>>>>>>>>>>          }
>>>>>>>>>>>>>>>>>>>>>>          +static int 
>>>>>>>>>>>>>>>>>>>>>> clk_periph_gate_save_context(struct
>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>> +    struct tegra_clk_periph_gate *gate =
>>>>>>>>>>>>>>>>>>>>>> to_clk_periph_gate(hw);
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> + gate->clk_state_ctx = read_enb(gate) &
>>>>>>>>>>>>>>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>>>>>>>>>>>>>>> + gate->rst_state_ctx = read_rst(gate) &
>>>>>>>>>>>>>>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> +    return 0;
>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> +static void clk_periph_gate_restore_context(struct
>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>> +    struct tegra_clk_periph_gate *gate =
>>>>>>>>>>>>>>>>>>>>>> to_clk_periph_gate(hw);
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> +    if (gate->clk_state_ctx)
>>>>>>>>>>>>>>>>>>>>>> + write_enb_set(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>>> +    else
>>>>>>>>>>>>>>>>>>>>>> + write_enb_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> +    udelay(5);
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> +    if (!(gate->flags & TEGRA_PERIPH_NO_RESET) &&
>>>>>>>>>>>>>>>>>>>>>> + !(gate->flags & TEGRA_PERIPH_MANUAL_RESET)) {
>>>>>>>>>>>>>>>>>>>>>> +        if (gate->rst_state_ctx)
>>>>>>>>>>>>>>>>>>>>>> + write_rst_set(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>>> +        else
>>>>>>>>>>>>>>>>>>>>>> + write_rst_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>>> +    }
>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>          const struct clk_ops
>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_gate_ops = {
>>>>>>>>>>>>>>>>>>>>>>              .is_enabled = clk_periph_is_enabled,
>>>>>>>>>>>>>>>>>>>>>>              .enable = clk_periph_enable,
>>>>>>>>>>>>>>>>>>>>>>              .disable = clk_periph_disable,
>>>>>>>>>>>>>>>>>>>>>> +    .save_context = clk_periph_gate_save_context,
>>>>>>>>>>>>>>>>>>>>>> +    .restore_context =
>>>>>>>>>>>>>>>>>>>>>> clk_periph_gate_restore_context,
>>>>>>>>>>>>>>>>>>>>>>          };
>>>>>>>>>>>>>>>>>>>>>>            struct clk
>>>>>>>>>>>>>>>>>>>>>> *tegra_clk_register_periph_gate(const
>>>>>>>>>>>>>>>>>>>>>> char *name,
>>>>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>>> index 58437da25156..06fb62955768 100644
>>>>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>>> @@ -99,6 +99,37 @@ static void
>>>>>>>>>>>>>>>>>>>>>> clk_periph_disable(struct
>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>> gate_ops->disable(gate_hw);
>>>>>>>>>>>>>>>>>>>>>>          }
>>>>>>>>>>>>>>>>>>>>>>          +static int clk_periph_save_context(struct
>>>>>>>>>>>>>>>>>>>>>> clk_hw *hw)
>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>> +    struct tegra_clk_periph *periph =
>>>>>>>>>>>>>>>>>>>>>> to_clk_periph(hw);
>>>>>>>>>>>>>>>>>>>>>> +    const struct clk_ops *gate_ops = 
>>>>>>>>>>>>>>>>>>>>>> periph->gate_ops;
>>>>>>>>>>>>>>>>>>>>>> +    struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> +    if (!(periph->gate.flags & 
>>>>>>>>>>>>>>>>>>>>>> TEGRA_PERIPH_NO_GATE))
>>>>>>>>>>>>>>>>>>>>>> + gate_ops->save_context(gate_hw);
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> + periph->parent_ctx = clk_periph_get_parent(hw);
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> +    return 0;
>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> +static void clk_periph_restore_context(struct 
>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>> +    struct tegra_clk_periph *periph =
>>>>>>>>>>>>>>>>>>>>>> to_clk_periph(hw);
>>>>>>>>>>>>>>>>>>>>>> +    const struct clk_ops *gate_ops = 
>>>>>>>>>>>>>>>>>>>>>> periph->gate_ops;
>>>>>>>>>>>>>>>>>>>>>> +    struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>>>>>>>>>>>>>>> +    const struct clk_ops *div_ops = 
>>>>>>>>>>>>>>>>>>>>>> periph->div_ops;
>>>>>>>>>>>>>>>>>>>>>> +    struct clk_hw *div_hw = &periph->divider.hw;
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> + clk_periph_set_parent(hw, periph->parent_ctx);
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> +    if (!(periph->gate.flags & 
>>>>>>>>>>>>>>>>>>>>>> TEGRA_PERIPH_NO_DIV))
>>>>>>>>>>>>>>>>>>>>>> + div_ops->restore_context(div_hw);
>>>>>>>>>>>>>>>>>>>>> Could you please point to where the divider's
>>>>>>>>>>>>>>>>>>>>> save_context()
>>>>>>>>>>>>>>>>>>>>> happens?
>>>>>>>>>>>>>>>>>>>>> Because I can't see it.
>>>>>>>>>>>>>>>>>>>> Ah, I now see that there is no need to save the 
>>>>>>>>>>>>>>>>>>>> dividers
>>>>>>>>>>>>>>>>>>>> context
>>>>>>>>>>>>>>>>>>>> because
>>>>>>>>>>>>>>>>>>>> clk itself has enough info that is needed for the
>>>>>>>>>>>>>>>>>>>> context's
>>>>>>>>>>>>>>>>>>>> restoring
>>>>>>>>>>>>>>>>>>>> (like I pointed in the review to v6).
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Looks like you could also implement a new
>>>>>>>>>>>>>>>>>>>> clk_hw_get_parent_index()
>>>>>>>>>>>>>>>>>>>> generic helper to get the index instead of storing it
>>>>>>>>>>>>>>>>>>>> manually.
>>>>>>>>>>>>>>>>>>> clk_periph_get_parent basically invokes existing
>>>>>>>>>>>>>>>>>>> clk_mux_ops
>>>>>>>>>>>>>>>>>>> get_parent() which is then saved in tegra_clk_periph.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> All existing drivers are using directly get_parent() 
>>>>>>>>>>>>>>>>>>> from
>>>>>>>>>>>>>>>>>>> clk_mux
>>>>>>>>>>>>>>>>>>> which actually gets index from the register read.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> To have this more generic w.r.t save/restore context
>>>>>>>>>>>>>>>>>>> point of
>>>>>>>>>>>>>>>>>>> view,
>>>>>>>>>>>>>>>>>>> probably instead of implementing new get_parent_index
>>>>>>>>>>>>>>>>>>> helper,
>>>>>>>>>>>>>>>>>>> I think
>>>>>>>>>>>>>>>>>>> its better to implement save_context and
>>>>>>>>>>>>>>>>>>> restore_context to
>>>>>>>>>>>>>>>>>>> clk_mux_ops along with creating parent_index field into
>>>>>>>>>>>>>>>>>>> clk_mux to
>>>>>>>>>>>>>>>>>>> cache index during set_parent.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> So we just need to invoke mux_ops save_context and
>>>>>>>>>>>>>>>>>>> restore_context.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> I hope its ok to add save/restore context to clk_mux_ops
>>>>>>>>>>>>>>>>>> to be
>>>>>>>>>>>>>>>>>> more
>>>>>>>>>>>>>>>>>> generic w.r.t save/restore context rather than
>>>>>>>>>>>>>>>>>> get_parent_index
>>>>>>>>>>>>>>>>>> API.
>>>>>>>>>>>>>>>>>> Please confirm if you agree.
>>>>>>>>>>>>>>>>> Sounds like a good idea. I see that there is a 
>>>>>>>>>>>>>>>>> 'restoring'
>>>>>>>>>>>>>>>>> helper for
>>>>>>>>>>>>>>>>> the generic clk_gate, seems something similar could be 
>>>>>>>>>>>>>>>>> done
>>>>>>>>>>>>>>>>> for the
>>>>>>>>>>>>>>>>> clk_mux. And looks like anyway you'll need to 
>>>>>>>>>>>>>>>>> associate the
>>>>>>>>>>>>>>>>> parent
>>>>>>>>>>>>>>>>> clock
>>>>>>>>>>>>>>>>> with the hw index in order to restore the muxing.
>>>>>>>>>>>>>>>> by 'restoring' helper for generic clk_gate, are you
>>>>>>>>>>>>>>>> referring to
>>>>>>>>>>>>>>>> clk_gate_restore_context API?
>>>>>>>>>>>>>>> Yes.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> clk_gate_restore_context is API that's any clk drivers can
>>>>>>>>>>>>>>>> use for
>>>>>>>>>>>>>>>> clk_gate operation restore for custom gate clk_ops.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> But clk-periph is directly using generic clk_mux ops from
>>>>>>>>>>>>>>>> clk_mux
>>>>>>>>>>>>>>>> so I
>>>>>>>>>>>>>>>> think we should add .restore_context to clk_mux_ops and 
>>>>>>>>>>>>>>>> then
>>>>>>>>>>>>>>>> during
>>>>>>>>>>>>>>>> clk-periph restore need to invoke 
>>>>>>>>>>>>>>>> mux_ops->restore_context.
>>>>>>>>>>>>>>> I'm not sure whether it will be good for every driver that
>>>>>>>>>>>>>>> uses
>>>>>>>>>>>>>>> generic
>>>>>>>>>>>>>>> clk_mux ops. Should be more flexible to have a generic 
>>>>>>>>>>>>>>> helper
>>>>>>>>>>>>>>> function
>>>>>>>>>>>>>>> that any driver could use in order to restore the clock's
>>>>>>>>>>>>>>> parent.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The clk-periph restoring also includes case of combining
>>>>>>>>>>>>>>> divider
>>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>> parent restoring, so generic helper could be useful in that
>>>>>>>>>>>>>>> case
>>>>>>>>>>>>>>> as well.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> It also looks like you could actually use the
>>>>>>>>>>>>>>> clk_gate_restore_context()
>>>>>>>>>>>>>>> instead of manually saving the clock's enable-state, 
>>>>>>>>>>>>>>> couldn't
>>>>>>>>>>>>>>> you?
>>>>>>>>>>>>>> ok for clk_mux, can add generic clk_mux_restore_context API
>>>>>>>>>>>>>> rather
>>>>>>>>>>>>>> than
>>>>>>>>>>>>>> using restore_context in clk_ops and will invoke that during
>>>>>>>>>>>>>> clk_periph
>>>>>>>>>>>>>> restore.
>>>>>>>>>>>>>>
>>>>>>>>>> digging thru looks like for clk_periph source restore instead of
>>>>>>>>>> clk_mux_restore_context, i can directly do clk_hw_get_parent and
>>>>>>>>>> clk_set_parent with mux_hw as they invoke mux_ops get/set parent
>>>>>>>>>> anyway.
>>>>>>>>>> Will do this for periph clk mux
>>> Just to be clear, clk_mux don't have cached parent. get_parent is by
>>> register read. So, cant directly use get_parent and then set during
>>> restore.
>>>
>>> So will create both clk_mux_save/restore_context in generic clk driver
>>> and will invoke them during tegra peripheral clock suspend/resume.
>> Why MUX clock doesn't have a cached parent? What MUX clock you're
>> talking about?
>>
>> [snip]
>
> Please ignore got it.
>
> Will send next version after giving few more days for feedback.
>
Couple of issues:

1.) I see clk-tegra-periph driver periph_clks init_data entries for some 
peripherals are not correct for Tegra 114 and later chips.

Eg I2C TEGRA_INIT_DATA_TABLE entries in clk-tegra-periph are used for 
all Tegra chipsets currently in the driver.

These entries are using MUX shift of 30 and MUX mask only for 2 bits 
which is correct for T30 and prior.
But for later Tegra chips, it should be MUX shift 29 and MASK(3).

Also, I2C parent idx entries in mux_pllp_clkm_idx are different from 
Tegra114 onwards.

As we are using only PLLP and CLKM sources only for I2C, their 
corresponding mux values from register spec by using upper 2 bits for 
T114 onwards match actual 2 bits of MUX value on T30 and prior.

Not sure if this something known pending to port actual clock MUX table 
changes for Tegra114 onwards?

Or

Are we purposely using upper 2 bits only for clock source for T114 and 
later as the upper 2 bit values of the limited clock source we are using 
match with previous Tegra peripheral clock source mux values?

Peter/Thierry, Can you please help comment on this?


2.) Other issue is regarding using clk_set_parent directly during 
clk_peripheral restore is clk_core_set_parent checks new parent with 
current parent and if its same, it just returns as success which is good 
in normal operation.

But during restore, we can't use clk_set_parent as new parent is from 
clk_get_parent on restore and this is same as cached parent.

So clk_set_parent returns 0 but acutal register value for clk source is 
different as it gets reset on SC7 entry/exit and to restore need to 
invoke mux_ops set_parent with parent_index.

So this need parent index for cached parent and without using context 
variable to store this, need an API like you were originally suggesting 
for get_parent_index to get parent index for the specified parent clk_hw.

As we decided not to use save/restore for clk_mux ops as its generic for 
other drivers, looks like we need get_parent_index API to use for 
restoring peripheral source and use this with clk_mux_ops set_parent.

clk core driver already has clk_fetch_parent_index but is it OK to 
export this?

Otherwise, will create separate API in clk driver which returns parent 
index from parent clk_hw by using this existing clk_fetch_parent_index 
so this API can be used by other drivers.

^ permalink raw reply

* Re: [PATCH] arm64: dts: rockchip: Add dts for Leez RK3399 P710 SBC
From: Heiko Stuebner @ 2019-08-04  0:34 UTC (permalink / raw)
  To: Andy Yan
  Cc: robh+dt, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel
In-Reply-To: <20190803114612.4830-1-andyshrk@gmail.com>

Hi Andy,

Am Samstag, 3. August 2019, 13:46:12 CEST schrieb Andy Yan:
> Leez P710 is a RK3399 based SBC, designed by Leez team
> from lenovo [0].
> 
> Specification
> - Rockchip RK3399
> - 4/2GB LPDDR4
> - TF sd scard slot
> - eMMC
> - M.2 B-Key for 4G LTE
> - AP6256 for WiFi + BT
> - Gigabit ethernet
> - HDMI out
> - 40 pin header
> - TYPE-C Power supply
> 
> [0] https://leez.lenovo.com
> 
> Signed-off-by: Andy Yan <andyshrk@gmail.com>
> ---
>  .../devicetree/bindings/arm/rockchip.yaml     |   5 +
>  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
>  .../boot/dts/rockchip/rk3399-leez-p710.dts    | 635 ++++++++++++++++++
>  3 files changed, 641 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts
> 
> diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml b/Documentation/devicetree/bindings/arm/rockchip.yaml
> index 34865042f4e4..da9cd947abfa 100644
> --- a/Documentation/devicetree/bindings/arm/rockchip.yaml
> +++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
> @@ -329,6 +329,11 @@ properties:
>                - khadas,edge-v
>            - const: rockchip,rk3399
>  
> +      - description: Leez RK3399 P710
> +        items:
> +          - const: leez,p710

Is "leez" really the vendor?
Part of me would assume something like
	lenovo,leez-p710

So please clarify :-)
And also please make sure the decided vendor is part of the vendor-prefixes
binding in Documentation/devicestree/bindings/vendor-prefixes.yaml

> +          - const: rockchip,rk3399
> +
>        - description: mqmaker MiQi
>          items:
>            - const: mqmaker,miqi
> diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> index daa2c78e22c3..1f18a9392d15 100644
> --- a/arch/arm64/boot/dts/rockchip/Makefile
> +++ b/arch/arm64/boot/dts/rockchip/Makefile
> @@ -20,6 +20,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-hugsun-x99.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-khadas-edge.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-khadas-edge-captain.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-khadas-edge-v.dtb
> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-leez-p710.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopc-t4.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopi-m4.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopi-neo4.dtb
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts b/arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts
> new file mode 100644
> index 000000000000..b342f5e8692b
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts
> @@ -0,0 +1,635 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2019 Andy Yan <andy.yan@gmail.com>
> + */
> +
> +/dts-v1/;
> +#include <dt-bindings/input/linux-event-codes.h>
> +#include <dt-bindings/pwm/pwm.h>
> +#include "rk3399.dtsi"
> +#include "rk3399-opp.dtsi"
> +
> +/ {
> +	model = "Leez RK3399 P710";
> +	compatible = "leez,p710", "rockchip,rk3399";

same comment as above, so maybe:
	model = "Lenovo Leez RK3399 P710";
	compatible = "lenovo,leez-p710", "rockchip,rk3399";



> +
> +	chosen {
> +		stdout-path = "serial2:1500000n8";
> +	};
> +
> +	clkin_gmac: external-gmac-clock {
> +		compatible = "fixed-clock";
> +		clock-frequency = <125000000>;
> +		clock-output-names = "clkin_gmac";
> +		#clock-cells = <0>;
> +	};
> +
> +	sdio_pwrseq: sdio-pwrseq {
> +		compatible = "mmc-pwrseq-simple";
> +		clocks = <&rk808 1>;
> +		clock-names = "ext_clock";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&wifi_enable_h>;
> +		reset-gpios = <&gpio0 RK_PB2 GPIO_ACTIVE_LOW>;
> +	};
> +
> +	dc5v_adp: dc-5v {
> +		compatible = "regulator-fixed";
> +		regulator-name = "dc5v_adapter";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +	};
> +
> +	vcc5v0_sys: vcc-sys {

vcc5v0_sys: vcc5v0-sys ?

> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc5v0_sys";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		vin-supply = <&dc5v_adp>;
> +	};
> +
> +	vcc3v3_sys: vcc3v3-sys {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc3v3_sys";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		vin-supply = <&vcc5v0_sys>;
> +	};
> +
> +	vcc5v0_host: vcc5v0-host-regulator {
> +		compatible = "regulator-fixed";
> +		enable-active-high;
> +		gpio = <&gpio2 RK_PA2 GPIO_ACTIVE_HIGH>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&vcc5v0_host_en>;
> +		regulator-name = "vcc5v0_host";
> +		regulator-always-on;
> +		vin-supply = <&vcc5v0_sys>;
> +	};
> +
> +	vcc_lan: vcc3v3-phy-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc_lan";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +
> +		regulator-state-mem {
> +			regulator-off-in-suspend;
> +		};
> +	};

In general, please model an actual regulator-tree and do not copy the
unspecific Rockchip vendor tree. These unconnected regulators are a very
good indicator that the real power-tree got ignored (missing vin-supply here)

I found schematics on https://github.com/leezsbc/resources/wiki/Leez-P710:
链接: https://pan.baidu.com/s/1NPWbuI5csT4zftKUCnRs7g
提取码: rvrh

and there the power-tree is described in a complete way.

regulator/regulator_summaray in the kernels debugfs should
show a nice tree structure starting from the dc-adapter input.

Also please use names matching the supply names from the schematics.

Same for pinctrl names, please use names as used in the board schematics.


> +	vdd_log: vdd-log {
> +		compatible = "pwm-regulator";
> +		pwms = <&pwm2 0 25000 1>;
> +		regulator-name = "vdd_log";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <800000>;
> +		regulator-max-microvolt = <1400000>;
> +		vin-supply = <&vcc5v0_sys>;
> +	};
> +};
> +
> +&cpu_l0 {
> +	cpu-supply = <&vdd_cpu_l>;
> +};
> +
> +&cpu_l1 {
> +	cpu-supply = <&vdd_cpu_l>;
> +};
> +
> +&cpu_l2 {
> +	cpu-supply = <&vdd_cpu_l>;
> +};
> +
> +&cpu_l3 {
> +	cpu-supply = <&vdd_cpu_l>;
> +};
> +
> +&cpu_b0 {
> +	cpu-supply = <&vdd_cpu_b>;
> +};
> +
> +&cpu_b1 {
> +	cpu-supply = <&vdd_cpu_b>;
> +};
> +
> +&emmc_phy {
> +	status = "okay";
> +};
> +
> +&gmac {
> +	assigned-clocks = <&cru SCLK_RMII_SRC>;
> +	assigned-clock-parents = <&clkin_gmac>;
> +	clock_in_out = "input";
> +	phy-supply = <&vcc_lan>;
> +	phy-mode = "rgmii";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&rgmii_pins>;
> +	snps,reset-gpio = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>;
> +	snps,reset-active-low;
> +	snps,reset-delays-us = <0 10000 50000>;
> +	tx_delay = <0x28>;
> +	rx_delay = <0x11>;
> +	status = "okay";
> +};
> +
> +&gpu {
> +	mali-supply = <&vdd_gpu>;
> +	status = "okay";
> +};
> +
> +&hdmi {
> +	ddc-i2c-bus = <&i2c3>;

can this also use the internal i2c inside the dw-hdmi?


> +	pinctrl-names = "default";
> +	pinctrl-0 = <&hdmi_cec>;
> +	status = "okay";
> +};
> +
> +&hdmi_sound {
> +	status = "okay";
> +};
> +
> +&i2c0 {
> +	clock-frequency = <400000>;
> +	i2c-scl-rising-time-ns = <168>;
> +	i2c-scl-falling-time-ns = <4>;
> +	status = "okay";
> +
> +	rk808: pmic@1b {
> +		compatible = "rockchip,rk808";
> +		reg = <0x1b>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <21 IRQ_TYPE_LEVEL_LOW>;
> +		#clock-cells = <1>;
> +		clock-output-names = "xin32k", "rk808-clkout2";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pmic_int_l>;
> +		rockchip,system-power-controller;
> +		wakeup-source;
> +
> +		vcc1-supply = <&vcc5v0_sys>;
> +		vcc2-supply = <&vcc5v0_sys>;
> +		vcc3-supply = <&vcc5v0_sys>;
> +		vcc4-supply = <&vcc5v0_sys>;
> +		vcc6-supply = <&vcc5v0_sys>;
> +		vcc7-supply = <&vcc5v0_sys>;
> +		vcc8-supply = <&vcc3v3_sys>;
> +		vcc9-supply = <&vcc5v0_sys>;
> +		vcc10-supply = <&vcc5v0_sys>;
> +		vcc11-supply = <&vcc5v0_sys>;
> +		vcc12-supply = <&vcc3v3_sys>;
> +		vddio-supply = <&vcc_1v8>;
> +
> +		regulators {
> +			vdd_center: DCDC_REG1 {
> +				regulator-name = "vdd_center";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <750000>;
> +				regulator-max-microvolt = <1350000>;
> +				regulator-ramp-delay = <6001>;
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vdd_cpu_l: DCDC_REG2 {
> +				regulator-name = "vdd_cpu_l";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <750000>;
> +				regulator-max-microvolt = <1350000>;
> +				regulator-ramp-delay = <6001>;
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcc_ddr: DCDC_REG3 {
> +				regulator-name = "vcc_ddr";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +				};
> +			};
> +
> +			vcc_1v8: DCDC_REG4 {
> +				regulator-name = "vcc_1v8";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-microvolt = <1800000>;
> +				};
> +			};
> +
> +			vcc1v8_dvp: LDO_REG1 {
> +				regulator-name = "vcc1v8_dvp";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcc1v8_hdmi: LDO_REG2 {
> +				regulator-name = "vcc1v8_hdmi";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcca_1v8: LDO_REG3 {
> +				regulator-name = "vcca_1v8";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-microvolt = <1800000>;
> +				};
> +			};
> +
> +			vccio_sd: LDO_REG4 {
> +				regulator-name = "vccio_sd";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <3000000>;
> +				regulator-max-microvolt = <3000000>;
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-microvolt = <3000000>;
> +				};
> +			};
> +
> +			vcca3v0_codec: LDO_REG5 {
> +				regulator-name = "vcca3v0_codec";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <3000000>;
> +				regulator-max-microvolt = <3000000>;
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcc_1v5: LDO_REG6 {
> +				regulator-name = "vcc_1v5";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1500000>;
> +				regulator-max-microvolt = <1500000>;
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-microvolt = <1500000>;
> +				};
> +			};
> +
> +			vcc0v9_hdmi: LDO_REG7 {
> +				regulator-name = "vcc0v9_hdmi";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <900000>;
> +				regulator-max-microvolt = <900000>;
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcc_3v0: LDO_REG8 {
> +				regulator-name = "vcc_3v0";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <3000000>;
> +				regulator-max-microvolt = <3000000>;
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-microvolt = <3000000>;
> +				};
> +			};
> +

unneeded blank line

> +		};
> +	};
> +


Heiko

^ permalink raw reply

* Re: [PATCH v4 3/4] net: phy: realtek: Add helpers for accessing RTL8211E extension pages
From: Heiner Kallweit @ 2019-08-04  8:33 UTC (permalink / raw)
  To: Matthias Kaehlcke, David S . Miller, Rob Herring, Mark Rutland,
	Andrew Lunn, Florian Fainelli
  Cc: netdev, devicetree, linux-kernel, Douglas Anderson
In-Reply-To: <20190801190759.28201-4-mka@chromium.org>

On 01.08.2019 21:07, Matthias Kaehlcke wrote:
> The RTL8211E has extension pages, which can be accessed after
> selecting a page through a custom method. Add a function to
> modify bits in a register of an extension page and a helper for
> selecting an ext page. Use rtl8211e_modify_ext_paged() in
> rtl8211e_config_init() instead of doing things 'manually'.
> 
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> Changes in v4:
> - don't add constant RTL8211E_EXT_PAGE, it's only used once,
>   use a literal instead
> - pass 'oldpage' to phy_restore_page() in rtl8211e_select_ext_page(),
>   not 'page'
> - return 'oldpage' in rtl8211e_select_ext_page()
> - use __phy_modify() in rtl8211e_modify_ext_paged() instead of
>   reimplementing __phy_modify_changed()
> - in rtl8211e_modify_ext_paged() return directly when
>   rtl8211e_select_ext_page() fails
> ---
>  drivers/net/phy/realtek.c | 48 +++++++++++++++++++++++++++------------
>  1 file changed, 34 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> index a669945eb829..e09d3b0da2c7 100644
> --- a/drivers/net/phy/realtek.c
> +++ b/drivers/net/phy/realtek.c
> @@ -53,6 +53,36 @@ static int rtl821x_write_page(struct phy_device *phydev, int page)
>  	return __phy_write(phydev, RTL821x_PAGE_SELECT, page);
>  }
>  
> +static int rtl8211e_select_ext_page(struct phy_device *phydev, int page)

The "extended page" mechanism doesn't exist on RTL8211E only. A prefix
rtl821x like in other functions may be better therefore.

> +{
> +	int ret, oldpage;
> +
> +	oldpage = phy_select_page(phydev, 7);
> +	if (oldpage < 0)
> +		return oldpage;
> +
> +	ret = __phy_write(phydev, RTL821x_EXT_PAGE_SELECT, page);
> +	if (ret)
> +		return phy_restore_page(phydev, oldpage, ret);
> +
> +	return oldpage;
> +}
> +
> +static int rtl8211e_modify_ext_paged(struct phy_device *phydev, int page,
> +				     u32 regnum, u16 mask, u16 set)
> +{
> +	int ret = 0;
> +	int oldpage;
> +
> +	oldpage = rtl8211e_select_ext_page(phydev, page);
> +	if (oldpage < 0)
> +		return oldpage;
> +
> +	ret = __phy_modify(phydev, regnum, mask, set);
> +
> +	return phy_restore_page(phydev, oldpage, ret);
> +}
> +
>  static int rtl8201_ack_interrupt(struct phy_device *phydev)
>  {
>  	int err;
> @@ -184,7 +214,7 @@ static int rtl8211f_config_init(struct phy_device *phydev)
>  
>  static int rtl8211e_config_init(struct phy_device *phydev)
>  {
> -	int ret = 0, oldpage;
> +	int ret;
>  	u16 val;
>  
>  	/* enable TX/RX delay for rgmii-* modes, and disable them for rgmii. */
> @@ -213,19 +243,9 @@ static int rtl8211e_config_init(struct phy_device *phydev)
>  	 * 2 = RX Delay, 1 = TX Delay, 0 = SELRGV (see original PHY datasheet
>  	 * for details).
>  	 */
> -	oldpage = phy_select_page(phydev, 0x7);
> -	if (oldpage < 0)
> -		goto err_restore_page;
> -
> -	ret = __phy_write(phydev, RTL821x_EXT_PAGE_SELECT, 0xa4);
> -	if (ret)
> -		goto err_restore_page;
> -
> -	ret = __phy_modify(phydev, 0x1c, RTL8211E_TX_DELAY | RTL8211E_RX_DELAY,
> -			   val);
> -
> -err_restore_page:
> -	return phy_restore_page(phydev, oldpage, ret);
> +	return rtl8211e_modify_ext_paged(phydev, 0xa4, 0x1c,
> +					 RTL8211E_TX_DELAY | RTL8211E_RX_DELAY,
> +					 val);
>  }
>  
>  static int rtl8211b_suspend(struct phy_device *phydev)
> 

^ permalink raw reply

* Re: [PATCH] arm64: dts: rockchip: Add dts for Leez RK3399 P710 SBC
From: Andy Yan @ 2019-08-04  8:38 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: robh+dt, devicetree, linux-arm-kernel,
	open list:ARM/Rockchip SoC..., linux-kernel
In-Reply-To: <22687582.BTWJvYJJdG@phil>

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

Hi Heiko:

Heiko Stuebner <heiko@sntech.de> 于2019年8月4日周日 上午8:34写道:

> Hi Andy,
>
> Am Samstag, 3. August 2019, 13:46:12 CEST schrieb Andy Yan:
> > Leez P710 is a RK3399 based SBC, designed by Leez team
> > from lenovo [0].
> >
> > Specification
> > - Rockchip RK3399
> > - 4/2GB LPDDR4
> > - TF sd scard slot
> > - eMMC
> > - M.2 B-Key for 4G LTE
> > - AP6256 for WiFi + BT
> > - Gigabit ethernet
> > - HDMI out
> > - 40 pin header
> > - TYPE-C Power supply
> >
> > [0] https://leez.lenovo.com
> >
> > Signed-off-by: Andy Yan <andyshrk@gmail.com>
> > ---
> >  .../devicetree/bindings/arm/rockchip.yaml     |   5 +
> >  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
> >  .../boot/dts/rockchip/rk3399-leez-p710.dts    | 635 ++++++++++++++++++
> >  3 files changed, 641 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts
> >
> > diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml
> b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > index 34865042f4e4..da9cd947abfa 100644
> > --- a/Documentation/devicetree/bindings/arm/rockchip.yaml
> > +++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > @@ -329,6 +329,11 @@ properties:
> >                - khadas,edge-v
> >            - const: rockchip,rk3399
> >
> > +      - description: Leez RK3399 P710
> > +        items:
> > +          - const: leez,p710
>
> Is "leez" really the vendor?
> Part of me would assume something like
>         lenovo,leez-p710
>
>
According to Leez team, they want to treat Leez an independent vendor here。



> So please clarify :-)
> And also please make sure the decided vendor is part of the vendor-prefixes
> binding in Documentation/devicestree/bindings/vendor-prefixes.yaml
>
> > +          - const: rockchip,rk3399
> > +
> >        - description: mqmaker MiQi
> >          items:
> >            - const: mqmaker,miqi
> > diff --git a/arch/arm64/boot/dts/rockchip/Makefile
> b/arch/arm64/boot/dts/rockchip/Makefile
> > index daa2c78e22c3..1f18a9392d15 100644
> > --- a/arch/arm64/boot/dts/rockchip/Makefile
> > +++ b/arch/arm64/boot/dts/rockchip/Makefile
> > @@ -20,6 +20,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-hugsun-x99.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-khadas-edge.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-khadas-edge-captain.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-khadas-edge-v.dtb
> > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-leez-p710.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopc-t4.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopi-m4.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopi-neo4.dtb
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts
> b/arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts
> > new file mode 100644
> > index 000000000000..b342f5e8692b
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts
> > @@ -0,0 +1,635 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright (c) 2019 Andy Yan <andy.yan@gmail.com>
> > + */
> > +
> > +/dts-v1/;
> > +#include <dt-bindings/input/linux-event-codes.h>
> > +#include <dt-bindings/pwm/pwm.h>
> > +#include "rk3399.dtsi"
> > +#include "rk3399-opp.dtsi"
> > +
> > +/ {
> > +     model = "Leez RK3399 P710";
> > +     compatible = "leez,p710", "rockchip,rk3399";
>
> same comment as above, so maybe:
>         model = "Lenovo Leez RK3399 P710";
>         compatible = "lenovo,leez-p710", "rockchip,rk3399";
>
>
>
> > +
> > +     chosen {
> > +             stdout-path = "serial2:1500000n8";
> > +     };
> > +
> > +     clkin_gmac: external-gmac-clock {
> > +             compatible = "fixed-clock";
> > +             clock-frequency = <125000000>;
> > +             clock-output-names = "clkin_gmac";
> > +             #clock-cells = <0>;
> > +     };
> > +
> > +     sdio_pwrseq: sdio-pwrseq {
> > +             compatible = "mmc-pwrseq-simple";
> > +             clocks = <&rk808 1>;
> > +             clock-names = "ext_clock";
> > +             pinctrl-names = "default";
> > +             pinctrl-0 = <&wifi_enable_h>;
> > +             reset-gpios = <&gpio0 RK_PB2 GPIO_ACTIVE_LOW>;
> > +     };
> > +
> > +     dc5v_adp: dc-5v {
> > +             compatible = "regulator-fixed";
> > +             regulator-name = "dc5v_adapter";
> > +             regulator-always-on;
> > +             regulator-boot-on;
> > +             regulator-min-microvolt = <5000000>;
> > +             regulator-max-microvolt = <5000000>;
> > +     };
> > +
> > +     vcc5v0_sys: vcc-sys {
>
> vcc5v0_sys: vcc5v0-sys ?
>
> > +             compatible = "regulator-fixed";
> > +             regulator-name = "vcc5v0_sys";
> > +             regulator-always-on;
> > +             regulator-boot-on;
> > +             regulator-min-microvolt = <5000000>;
> > +             regulator-max-microvolt = <5000000>;
> > +             vin-supply = <&dc5v_adp>;
> > +     };
> > +
> > +     vcc3v3_sys: vcc3v3-sys {
> > +             compatible = "regulator-fixed";
> > +             regulator-name = "vcc3v3_sys";
> > +             regulator-always-on;
> > +             regulator-boot-on;
> > +             regulator-min-microvolt = <3300000>;
> > +             regulator-max-microvolt = <3300000>;
> > +             vin-supply = <&vcc5v0_sys>;
> > +     };
> > +
> > +     vcc5v0_host: vcc5v0-host-regulator {
> > +             compatible = "regulator-fixed";
> > +             enable-active-high;
> > +             gpio = <&gpio2 RK_PA2 GPIO_ACTIVE_HIGH>;
> > +             pinctrl-names = "default";
> > +             pinctrl-0 = <&vcc5v0_host_en>;
> > +             regulator-name = "vcc5v0_host";
> > +             regulator-always-on;
> > +             vin-supply = <&vcc5v0_sys>;
> > +     };
> > +
> > +     vcc_lan: vcc3v3-phy-regulator {
> > +             compatible = "regulator-fixed";
> > +             regulator-name = "vcc_lan";
> > +             regulator-always-on;
> > +             regulator-boot-on;
> > +             regulator-min-microvolt = <3300000>;
> > +             regulator-max-microvolt = <3300000>;
> > +
> > +             regulator-state-mem {
> > +                     regulator-off-in-suspend;
> > +             };
> > +     };
>
> In general, please model an actual regulator-tree and do not copy the
> unspecific Rockchip vendor tree. These unconnected regulators are a very
> good indicator that the real power-tree got ignored (missing vin-supply
> here)
>
> I found schematics on https://github.com/leezsbc/resources/wiki/Leez-P710:
> 链接: https://pan.baidu.com/s/1NPWbuI5csT4zftKUCnRs7g
> 提取码: rvrh
>
> and there the power-tree is described in a complete way.
>
> regulator/regulator_summaray in the kernels debugfs should
> show a nice tree structure starting from the dc-adapter input.
>
> Also please use names matching the supply names from the schematics.
>
> Same for pinctrl names, please use names as used in the board schematics.
>
>
> > +     vdd_log: vdd-log {
> > +             compatible = "pwm-regulator";
> > +             pwms = <&pwm2 0 25000 1>;
> > +             regulator-name = "vdd_log";
> > +             regulator-always-on;
> > +             regulator-boot-on;
> > +             regulator-min-microvolt = <800000>;
> > +             regulator-max-microvolt = <1400000>;
> > +             vin-supply = <&vcc5v0_sys>;
> > +     };
> > +};
> > +
> > +&cpu_l0 {
> > +     cpu-supply = <&vdd_cpu_l>;
> > +};
> > +
> > +&cpu_l1 {
> > +     cpu-supply = <&vdd_cpu_l>;
> > +};
> > +
> > +&cpu_l2 {
> > +     cpu-supply = <&vdd_cpu_l>;
> > +};
> > +
> > +&cpu_l3 {
> > +     cpu-supply = <&vdd_cpu_l>;
> > +};
> > +
> > +&cpu_b0 {
> > +     cpu-supply = <&vdd_cpu_b>;
> > +};
> > +
> > +&cpu_b1 {
> > +     cpu-supply = <&vdd_cpu_b>;
> > +};
> > +
> > +&emmc_phy {
> > +     status = "okay";
> > +};
> > +
> > +&gmac {
> > +     assigned-clocks = <&cru SCLK_RMII_SRC>;
> > +     assigned-clock-parents = <&clkin_gmac>;
> > +     clock_in_out = "input";
> > +     phy-supply = <&vcc_lan>;
> > +     phy-mode = "rgmii";
> > +     pinctrl-names = "default";
> > +     pinctrl-0 = <&rgmii_pins>;
> > +     snps,reset-gpio = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>;
> > +     snps,reset-active-low;
> > +     snps,reset-delays-us = <0 10000 50000>;
> > +     tx_delay = <0x28>;
> > +     rx_delay = <0x11>;
> > +     status = "okay";
> > +};
> > +
> > +&gpu {
> > +     mali-supply = <&vdd_gpu>;
> > +     status = "okay";
> > +};
> > +
> > +&hdmi {
> > +     ddc-i2c-bus = <&i2c3>;
>
> can this also use the internal i2c inside the dw-hdmi?
>
>
> > +     pinctrl-names = "default";
> > +     pinctrl-0 = <&hdmi_cec>;
> > +     status = "okay";
> > +};
> > +
> > +&hdmi_sound {
> > +     status = "okay";
> > +};
> > +
> > +&i2c0 {
> > +     clock-frequency = <400000>;
> > +     i2c-scl-rising-time-ns = <168>;
> > +     i2c-scl-falling-time-ns = <4>;
> > +     status = "okay";
> > +
> > +     rk808: pmic@1b {
> > +             compatible = "rockchip,rk808";
> > +             reg = <0x1b>;
> > +             interrupt-parent = <&gpio1>;
> > +             interrupts = <21 IRQ_TYPE_LEVEL_LOW>;
> > +             #clock-cells = <1>;
> > +             clock-output-names = "xin32k", "rk808-clkout2";
> > +             pinctrl-names = "default";
> > +             pinctrl-0 = <&pmic_int_l>;
> > +             rockchip,system-power-controller;
> > +             wakeup-source;
> > +
> > +             vcc1-supply = <&vcc5v0_sys>;
> > +             vcc2-supply = <&vcc5v0_sys>;
> > +             vcc3-supply = <&vcc5v0_sys>;
> > +             vcc4-supply = <&vcc5v0_sys>;
> > +             vcc6-supply = <&vcc5v0_sys>;
> > +             vcc7-supply = <&vcc5v0_sys>;
> > +             vcc8-supply = <&vcc3v3_sys>;
> > +             vcc9-supply = <&vcc5v0_sys>;
> > +             vcc10-supply = <&vcc5v0_sys>;
> > +             vcc11-supply = <&vcc5v0_sys>;
> > +             vcc12-supply = <&vcc3v3_sys>;
> > +             vddio-supply = <&vcc_1v8>;
> > +
> > +             regulators {
> > +                     vdd_center: DCDC_REG1 {
> > +                             regulator-name = "vdd_center";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <750000>;
> > +                             regulator-max-microvolt = <1350000>;
> > +                             regulator-ramp-delay = <6001>;
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vdd_cpu_l: DCDC_REG2 {
> > +                             regulator-name = "vdd_cpu_l";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <750000>;
> > +                             regulator-max-microvolt = <1350000>;
> > +                             regulator-ramp-delay = <6001>;
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vcc_ddr: DCDC_REG3 {
> > +                             regulator-name = "vcc_ddr";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-state-mem {
> > +                                     regulator-on-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vcc_1v8: DCDC_REG4 {
> > +                             regulator-name = "vcc_1v8";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <1800000>;
> > +                             regulator-max-microvolt = <1800000>;
> > +                             regulator-state-mem {
> > +                                     regulator-on-in-suspend;
> > +                                     regulator-suspend-microvolt =
> <1800000>;
> > +                             };
> > +                     };
> > +
> > +                     vcc1v8_dvp: LDO_REG1 {
> > +                             regulator-name = "vcc1v8_dvp";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <1800000>;
> > +                             regulator-max-microvolt = <1800000>;
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vcc1v8_hdmi: LDO_REG2 {
> > +                             regulator-name = "vcc1v8_hdmi";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <1800000>;
> > +                             regulator-max-microvolt = <1800000>;
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vcca_1v8: LDO_REG3 {
> > +                             regulator-name = "vcca_1v8";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <1800000>;
> > +                             regulator-max-microvolt = <1800000>;
> > +                             regulator-state-mem {
> > +                                     regulator-on-in-suspend;
> > +                                     regulator-suspend-microvolt =
> <1800000>;
> > +                             };
> > +                     };
> > +
> > +                     vccio_sd: LDO_REG4 {
> > +                             regulator-name = "vccio_sd";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <3000000>;
> > +                             regulator-max-microvolt = <3000000>;
> > +                             regulator-state-mem {
> > +                                     regulator-on-in-suspend;
> > +                                     regulator-suspend-microvolt =
> <3000000>;
> > +                             };
> > +                     };
> > +
> > +                     vcca3v0_codec: LDO_REG5 {
> > +                             regulator-name = "vcca3v0_codec";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <3000000>;
> > +                             regulator-max-microvolt = <3000000>;
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vcc_1v5: LDO_REG6 {
> > +                             regulator-name = "vcc_1v5";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <1500000>;
> > +                             regulator-max-microvolt = <1500000>;
> > +                             regulator-state-mem {
> > +                                     regulator-on-in-suspend;
> > +                                     regulator-suspend-microvolt =
> <1500000>;
> > +                             };
> > +                     };
> > +
> > +                     vcc0v9_hdmi: LDO_REG7 {
> > +                             regulator-name = "vcc0v9_hdmi";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <900000>;
> > +                             regulator-max-microvolt = <900000>;
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vcc_3v0: LDO_REG8 {
> > +                             regulator-name = "vcc_3v0";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <3000000>;
> > +                             regulator-max-microvolt = <3000000>;
> > +                             regulator-state-mem {
> > +                                     regulator-on-in-suspend;
> > +                                     regulator-suspend-microvolt =
> <3000000>;
> > +                             };
> > +                     };
> > +
>
> unneeded blank line
>
> > +             };
> > +     };
> > +
>
>
> Heiko
>
>
>

[-- Attachment #2: Type: text/html, Size: 24466 bytes --]

^ permalink raw reply

* Re: [PATCH] arm64: dts: rockchip: Add dts for Leez RK3399 P710 SBC
From: Heiko Stuebner @ 2019-08-04  9:59 UTC (permalink / raw)
  To: Andy Yan
  Cc: devicetree, robh+dt, linux-kernel, linux-arm-kernel,
	open list:ARM/Rockchip SoC...
In-Reply-To: <CANbgqATNLO=wJfKNDY74qmQUqAP_9Xv29nGhLj+Y0Cc7CAMQyg@mail.gmail.com>

Hi Andy,

Am Sonntag, 4. August 2019, 10:38:26 CEST schrieb Andy Yan:
> Heiko Stuebner <heiko@sntech.de> 于2019年8月4日周日 上午8:34写道:
> > Am Samstag, 3. August 2019, 13:46:12 CEST schrieb Andy Yan:
> > > Leez P710 is a RK3399 based SBC, designed by Leez team
> > > from lenovo [0].
> > >
> > > Specification
> > > - Rockchip RK3399
> > > - 4/2GB LPDDR4
> > > - TF sd scard slot
> > > - eMMC
> > > - M.2 B-Key for 4G LTE
> > > - AP6256 for WiFi + BT
> > > - Gigabit ethernet
> > > - HDMI out
> > > - 40 pin header
> > > - TYPE-C Power supply
> > >
> > > [0] https://leez.lenovo.com
> > >
> > > Signed-off-by: Andy Yan <andyshrk@gmail.com>
> > > ---
> > >  .../devicetree/bindings/arm/rockchip.yaml     |   5 +
> > >  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
> > >  .../boot/dts/rockchip/rk3399-leez-p710.dts    | 635 ++++++++++++++++++
> > >  3 files changed, 641 insertions(+)
> > >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts
> > >
> > > diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml
> > b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > index 34865042f4e4..da9cd947abfa 100644
> > > --- a/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > +++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > @@ -329,6 +329,11 @@ properties:
> > >                - khadas,edge-v
> > >            - const: rockchip,rk3399
> > >
> > > +      - description: Leez RK3399 P710
> > > +        items:
> > > +          - const: leez,p710
> >
> > Is "leez" really the vendor?
> > Part of me would assume something like
> >         lenovo,leez-p710
> >
> >
> According to Leez team, they want to treat Leez an independent vendor here。

ok, that should be fine then. You'll just have to also add
an entry for them into the vendor-prefixes.yaml file then.

(And of course rework the regulator tree)

Thanks
Heiko

> > So please clarify :-)
> > And also please make sure the decided vendor is part of the vendor-prefixes
> > binding in Documentation/devicestree/bindings/vendor-prefixes.yaml
> >
> > > +          - const: rockchip,rk3399
> > > +
> > >        - description: mqmaker MiQi
> > >          items:
> > >            - const: mqmaker,miqi
> > > diff --git a/arch/arm64/boot/dts/rockchip/Makefile
> > b/arch/arm64/boot/dts/rockchip/Makefile
> > > index daa2c78e22c3..1f18a9392d15 100644
> > > --- a/arch/arm64/boot/dts/rockchip/Makefile
> > > +++ b/arch/arm64/boot/dts/rockchip/Makefile
> > > @@ -20,6 +20,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-hugsun-x99.dtb
> > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-khadas-edge.dtb
> > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-khadas-edge-captain.dtb
> > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-khadas-edge-v.dtb
> > > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-leez-p710.dtb
> > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopc-t4.dtb
> > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopi-m4.dtb
> > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopi-neo4.dtb
> > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts
> > b/arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts
> > > new file mode 100644
> > > index 000000000000..b342f5e8692b
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts
> > > @@ -0,0 +1,635 @@
> > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > > +/*
> > > + * Copyright (c) 2019 Andy Yan <andy.yan@gmail.com>
> > > + */
> > > +
> > > +/dts-v1/;
> > > +#include <dt-bindings/input/linux-event-codes.h>
> > > +#include <dt-bindings/pwm/pwm.h>
> > > +#include "rk3399.dtsi"
> > > +#include "rk3399-opp.dtsi"
> > > +
> > > +/ {
> > > +     model = "Leez RK3399 P710";
> > > +     compatible = "leez,p710", "rockchip,rk3399";
> >
> > same comment as above, so maybe:
> >         model = "Lenovo Leez RK3399 P710";
> >         compatible = "lenovo,leez-p710", "rockchip,rk3399";
> >
> >
> >
> > > +
> > > +     chosen {
> > > +             stdout-path = "serial2:1500000n8";
> > > +     };
> > > +
> > > +     clkin_gmac: external-gmac-clock {
> > > +             compatible = "fixed-clock";
> > > +             clock-frequency = <125000000>;
> > > +             clock-output-names = "clkin_gmac";
> > > +             #clock-cells = <0>;
> > > +     };
> > > +
> > > +     sdio_pwrseq: sdio-pwrseq {
> > > +             compatible = "mmc-pwrseq-simple";
> > > +             clocks = <&rk808 1>;
> > > +             clock-names = "ext_clock";
> > > +             pinctrl-names = "default";
> > > +             pinctrl-0 = <&wifi_enable_h>;
> > > +             reset-gpios = <&gpio0 RK_PB2 GPIO_ACTIVE_LOW>;
> > > +     };
> > > +
> > > +     dc5v_adp: dc-5v {
> > > +             compatible = "regulator-fixed";
> > > +             regulator-name = "dc5v_adapter";
> > > +             regulator-always-on;
> > > +             regulator-boot-on;
> > > +             regulator-min-microvolt = <5000000>;
> > > +             regulator-max-microvolt = <5000000>;
> > > +     };
> > > +
> > > +     vcc5v0_sys: vcc-sys {
> >
> > vcc5v0_sys: vcc5v0-sys ?
> >
> > > +             compatible = "regulator-fixed";
> > > +             regulator-name = "vcc5v0_sys";
> > > +             regulator-always-on;
> > > +             regulator-boot-on;
> > > +             regulator-min-microvolt = <5000000>;
> > > +             regulator-max-microvolt = <5000000>;
> > > +             vin-supply = <&dc5v_adp>;
> > > +     };
> > > +
> > > +     vcc3v3_sys: vcc3v3-sys {
> > > +             compatible = "regulator-fixed";
> > > +             regulator-name = "vcc3v3_sys";
> > > +             regulator-always-on;
> > > +             regulator-boot-on;
> > > +             regulator-min-microvolt = <3300000>;
> > > +             regulator-max-microvolt = <3300000>;
> > > +             vin-supply = <&vcc5v0_sys>;
> > > +     };
> > > +
> > > +     vcc5v0_host: vcc5v0-host-regulator {
> > > +             compatible = "regulator-fixed";
> > > +             enable-active-high;
> > > +             gpio = <&gpio2 RK_PA2 GPIO_ACTIVE_HIGH>;
> > > +             pinctrl-names = "default";
> > > +             pinctrl-0 = <&vcc5v0_host_en>;
> > > +             regulator-name = "vcc5v0_host";
> > > +             regulator-always-on;
> > > +             vin-supply = <&vcc5v0_sys>;
> > > +     };
> > > +
> > > +     vcc_lan: vcc3v3-phy-regulator {
> > > +             compatible = "regulator-fixed";
> > > +             regulator-name = "vcc_lan";
> > > +             regulator-always-on;
> > > +             regulator-boot-on;
> > > +             regulator-min-microvolt = <3300000>;
> > > +             regulator-max-microvolt = <3300000>;
> > > +
> > > +             regulator-state-mem {
> > > +                     regulator-off-in-suspend;
> > > +             };
> > > +     };
> >
> > In general, please model an actual regulator-tree and do not copy the
> > unspecific Rockchip vendor tree. These unconnected regulators are a very
> > good indicator that the real power-tree got ignored (missing vin-supply
> > here)
> >
> > I found schematics on https://github.com/leezsbc/resources/wiki/Leez-P710:
> > 链接: https://pan.baidu.com/s/1NPWbuI5csT4zftKUCnRs7g
> > 提取码: rvrh
> >
> > and there the power-tree is described in a complete way.
> >
> > regulator/regulator_summaray in the kernels debugfs should
> > show a nice tree structure starting from the dc-adapter input.
> >
> > Also please use names matching the supply names from the schematics.
> >
> > Same for pinctrl names, please use names as used in the board schematics.
> >
> >
> > > +     vdd_log: vdd-log {
> > > +             compatible = "pwm-regulator";
> > > +             pwms = <&pwm2 0 25000 1>;
> > > +             regulator-name = "vdd_log";
> > > +             regulator-always-on;
> > > +             regulator-boot-on;
> > > +             regulator-min-microvolt = <800000>;
> > > +             regulator-max-microvolt = <1400000>;
> > > +             vin-supply = <&vcc5v0_sys>;
> > > +     };
> > > +};
> > > +
> > > +&cpu_l0 {
> > > +     cpu-supply = <&vdd_cpu_l>;
> > > +};
> > > +
> > > +&cpu_l1 {
> > > +     cpu-supply = <&vdd_cpu_l>;
> > > +};
> > > +
> > > +&cpu_l2 {
> > > +     cpu-supply = <&vdd_cpu_l>;
> > > +};
> > > +
> > > +&cpu_l3 {
> > > +     cpu-supply = <&vdd_cpu_l>;
> > > +};
> > > +
> > > +&cpu_b0 {
> > > +     cpu-supply = <&vdd_cpu_b>;
> > > +};
> > > +
> > > +&cpu_b1 {
> > > +     cpu-supply = <&vdd_cpu_b>;
> > > +};
> > > +
> > > +&emmc_phy {
> > > +     status = "okay";
> > > +};
> > > +
> > > +&gmac {
> > > +     assigned-clocks = <&cru SCLK_RMII_SRC>;
> > > +     assigned-clock-parents = <&clkin_gmac>;
> > > +     clock_in_out = "input";
> > > +     phy-supply = <&vcc_lan>;
> > > +     phy-mode = "rgmii";
> > > +     pinctrl-names = "default";
> > > +     pinctrl-0 = <&rgmii_pins>;
> > > +     snps,reset-gpio = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>;
> > > +     snps,reset-active-low;
> > > +     snps,reset-delays-us = <0 10000 50000>;
> > > +     tx_delay = <0x28>;
> > > +     rx_delay = <0x11>;
> > > +     status = "okay";
> > > +};
> > > +
> > > +&gpu {
> > > +     mali-supply = <&vdd_gpu>;
> > > +     status = "okay";
> > > +};
> > > +
> > > +&hdmi {
> > > +     ddc-i2c-bus = <&i2c3>;
> >
> > can this also use the internal i2c inside the dw-hdmi?
> >
> >
> > > +     pinctrl-names = "default";
> > > +     pinctrl-0 = <&hdmi_cec>;
> > > +     status = "okay";
> > > +};
> > > +
> > > +&hdmi_sound {
> > > +     status = "okay";
> > > +};
> > > +
> > > +&i2c0 {
> > > +     clock-frequency = <400000>;
> > > +     i2c-scl-rising-time-ns = <168>;
> > > +     i2c-scl-falling-time-ns = <4>;
> > > +     status = "okay";
> > > +
> > > +     rk808: pmic@1b {
> > > +             compatible = "rockchip,rk808";
> > > +             reg = <0x1b>;
> > > +             interrupt-parent = <&gpio1>;
> > > +             interrupts = <21 IRQ_TYPE_LEVEL_LOW>;
> > > +             #clock-cells = <1>;
> > > +             clock-output-names = "xin32k", "rk808-clkout2";
> > > +             pinctrl-names = "default";
> > > +             pinctrl-0 = <&pmic_int_l>;
> > > +             rockchip,system-power-controller;
> > > +             wakeup-source;
> > > +
> > > +             vcc1-supply = <&vcc5v0_sys>;
> > > +             vcc2-supply = <&vcc5v0_sys>;
> > > +             vcc3-supply = <&vcc5v0_sys>;
> > > +             vcc4-supply = <&vcc5v0_sys>;
> > > +             vcc6-supply = <&vcc5v0_sys>;
> > > +             vcc7-supply = <&vcc5v0_sys>;
> > > +             vcc8-supply = <&vcc3v3_sys>;
> > > +             vcc9-supply = <&vcc5v0_sys>;
> > > +             vcc10-supply = <&vcc5v0_sys>;
> > > +             vcc11-supply = <&vcc5v0_sys>;
> > > +             vcc12-supply = <&vcc3v3_sys>;
> > > +             vddio-supply = <&vcc_1v8>;
> > > +
> > > +             regulators {
> > > +                     vdd_center: DCDC_REG1 {
> > > +                             regulator-name = "vdd_center";
> > > +                             regulator-always-on;
> > > +                             regulator-boot-on;
> > > +                             regulator-min-microvolt = <750000>;
> > > +                             regulator-max-microvolt = <1350000>;
> > > +                             regulator-ramp-delay = <6001>;
> > > +                             regulator-state-mem {
> > > +                                     regulator-off-in-suspend;
> > > +                             };
> > > +                     };
> > > +
> > > +                     vdd_cpu_l: DCDC_REG2 {
> > > +                             regulator-name = "vdd_cpu_l";
> > > +                             regulator-always-on;
> > > +                             regulator-boot-on;
> > > +                             regulator-min-microvolt = <750000>;
> > > +                             regulator-max-microvolt = <1350000>;
> > > +                             regulator-ramp-delay = <6001>;
> > > +                             regulator-state-mem {
> > > +                                     regulator-off-in-suspend;
> > > +                             };
> > > +                     };
> > > +
> > > +                     vcc_ddr: DCDC_REG3 {
> > > +                             regulator-name = "vcc_ddr";
> > > +                             regulator-always-on;
> > > +                             regulator-boot-on;
> > > +                             regulator-state-mem {
> > > +                                     regulator-on-in-suspend;
> > > +                             };
> > > +                     };
> > > +
> > > +                     vcc_1v8: DCDC_REG4 {
> > > +                             regulator-name = "vcc_1v8";
> > > +                             regulator-always-on;
> > > +                             regulator-boot-on;
> > > +                             regulator-min-microvolt = <1800000>;
> > > +                             regulator-max-microvolt = <1800000>;
> > > +                             regulator-state-mem {
> > > +                                     regulator-on-in-suspend;
> > > +                                     regulator-suspend-microvolt =
> > <1800000>;
> > > +                             };
> > > +                     };
> > > +
> > > +                     vcc1v8_dvp: LDO_REG1 {
> > > +                             regulator-name = "vcc1v8_dvp";
> > > +                             regulator-always-on;
> > > +                             regulator-boot-on;
> > > +                             regulator-min-microvolt = <1800000>;
> > > +                             regulator-max-microvolt = <1800000>;
> > > +                             regulator-state-mem {
> > > +                                     regulator-off-in-suspend;
> > > +                             };
> > > +                     };
> > > +
> > > +                     vcc1v8_hdmi: LDO_REG2 {
> > > +                             regulator-name = "vcc1v8_hdmi";
> > > +                             regulator-always-on;
> > > +                             regulator-boot-on;
> > > +                             regulator-min-microvolt = <1800000>;
> > > +                             regulator-max-microvolt = <1800000>;
> > > +                             regulator-state-mem {
> > > +                                     regulator-off-in-suspend;
> > > +                             };
> > > +                     };
> > > +
> > > +                     vcca_1v8: LDO_REG3 {
> > > +                             regulator-name = "vcca_1v8";
> > > +                             regulator-always-on;
> > > +                             regulator-boot-on;
> > > +                             regulator-min-microvolt = <1800000>;
> > > +                             regulator-max-microvolt = <1800000>;
> > > +                             regulator-state-mem {
> > > +                                     regulator-on-in-suspend;
> > > +                                     regulator-suspend-microvolt =
> > <1800000>;
> > > +                             };
> > > +                     };
> > > +
> > > +                     vccio_sd: LDO_REG4 {
> > > +                             regulator-name = "vccio_sd";
> > > +                             regulator-always-on;
> > > +                             regulator-boot-on;
> > > +                             regulator-min-microvolt = <3000000>;
> > > +                             regulator-max-microvolt = <3000000>;
> > > +                             regulator-state-mem {
> > > +                                     regulator-on-in-suspend;
> > > +                                     regulator-suspend-microvolt =
> > <3000000>;
> > > +                             };
> > > +                     };
> > > +
> > > +                     vcca3v0_codec: LDO_REG5 {
> > > +                             regulator-name = "vcca3v0_codec";
> > > +                             regulator-always-on;
> > > +                             regulator-boot-on;
> > > +                             regulator-min-microvolt = <3000000>;
> > > +                             regulator-max-microvolt = <3000000>;
> > > +                             regulator-state-mem {
> > > +                                     regulator-off-in-suspend;
> > > +                             };
> > > +                     };
> > > +
> > > +                     vcc_1v5: LDO_REG6 {
> > > +                             regulator-name = "vcc_1v5";
> > > +                             regulator-always-on;
> > > +                             regulator-boot-on;
> > > +                             regulator-min-microvolt = <1500000>;
> > > +                             regulator-max-microvolt = <1500000>;
> > > +                             regulator-state-mem {
> > > +                                     regulator-on-in-suspend;
> > > +                                     regulator-suspend-microvolt =
> > <1500000>;
> > > +                             };
> > > +                     };
> > > +
> > > +                     vcc0v9_hdmi: LDO_REG7 {
> > > +                             regulator-name = "vcc0v9_hdmi";
> > > +                             regulator-always-on;
> > > +                             regulator-boot-on;
> > > +                             regulator-min-microvolt = <900000>;
> > > +                             regulator-max-microvolt = <900000>;
> > > +                             regulator-state-mem {
> > > +                                     regulator-off-in-suspend;
> > > +                             };
> > > +                     };
> > > +
> > > +                     vcc_3v0: LDO_REG8 {
> > > +                             regulator-name = "vcc_3v0";
> > > +                             regulator-always-on;
> > > +                             regulator-boot-on;
> > > +                             regulator-min-microvolt = <3000000>;
> > > +                             regulator-max-microvolt = <3000000>;
> > > +                             regulator-state-mem {
> > > +                                     regulator-on-in-suspend;
> > > +                                     regulator-suspend-microvolt =
> > <3000000>;
> > > +                             };
> > > +                     };
> > > +
> >
> > unneeded blank line
> >
> > > +             };
> > > +     };
> > > +
> >
> >
> > Heiko
> >
> >
> >





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

^ permalink raw reply

* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Dmitry Osipenko @ 2019-08-04 12:24 UTC (permalink / raw)
  To: Sowjanya Komatineni, thierry.reding, jonathanh, tglx, jason,
	marc.zyngier, linus.walleij, stefan, mark.rutland
  Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
	josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
	robh+dt, devicetree
In-Reply-To: <4dec8efb-bc3b-0275-8dea-7600a8f9e478@nvidia.com>

04.08.2019 2:44, Sowjanya Komatineni пишет:
> 
> On 8/3/19 10:01 AM, Sowjanya Komatineni wrote:
>>
>> On 8/3/19 3:33 AM, Dmitry Osipenko wrote:
>>> 03.08.2019 2:51, Sowjanya Komatineni пишет:
>>>> On 8/2/19 2:15 PM, Dmitry Osipenko wrote:
>>>>> 02.08.2019 23:32, Sowjanya Komatineni пишет:
>>>>>> On 8/2/19 1:17 PM, Dmitry Osipenko wrote:
>>>>>>> 02.08.2019 23:13, Dmitry Osipenko пишет:
>>>>>>>> 02.08.2019 21:33, Sowjanya Komatineni пишет:
>>>>>>>>> On 8/2/19 5:38 AM, Dmitry Osipenko wrote:
>>>>>>>>>> 02.08.2019 2:49, Sowjanya Komatineni пишет:
>>>>>>>>>>> On 8/1/19 4:19 PM, Sowjanya Komatineni wrote:
>>>>>>>>>>>> On 8/1/19 2:30 PM, Sowjanya Komatineni wrote:
>>>>>>>>>>>>> On 8/1/19 1:54 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>> 01.08.2019 23:31, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>> On 8/1/19 1:17 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>> 01.08.2019 22:42, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>>> On 8/1/19 12:00 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>>>> 01.08.2019 20:58, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>>>>> On 7/31/19 4:09 PM, Sowjanya Komatineni wrote:
>>>>>>>>>>>>>>>>>>>> On 7/31/19 3:44 AM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>>>>>>> 31.07.2019 12:50, Dmitry Osipenko пишет:
>>>>>>>>>>>>>>>>>>>>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>>>>>>>>> This patch implements save and restore context for
>>>>>>>>>>>>>>>>>>>>>>> peripheral
>>>>>>>>>>>>>>>>>>>>>>> fixed
>>>>>>>>>>>>>>>>>>>>>>> clock ops, peripheral gate clock ops, sdmmc mux clock
>>>>>>>>>>>>>>>>>>>>>>> ops, and
>>>>>>>>>>>>>>>>>>>>>>> peripheral clock ops.
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> During system suspend, core power goes off and
>>>>>>>>>>>>>>>>>>>>>>> looses the
>>>>>>>>>>>>>>>>>>>>>>> settings
>>>>>>>>>>>>>>>>>>>>>>> of the Tegra CAR controller registers.
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> So during suspend entry clock and reset state of
>>>>>>>>>>>>>>>>>>>>>>> peripherals is
>>>>>>>>>>>>>>>>>>>>>>> saved
>>>>>>>>>>>>>>>>>>>>>>> and on resume they are restored to have clocks back to
>>>>>>>>>>>>>>>>>>>>>>> same
>>>>>>>>>>>>>>>>>>>>>>> rate and
>>>>>>>>>>>>>>>>>>>>>>> state as before suspend.
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>>>>>>>>>>>>>>>>>>>> Signed-off-by: Sowjanya Komatineni
>>>>>>>>>>>>>>>>>>>>>>> <skomatineni@nvidia.com>
>>>>>>>>>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph-fixed.c | 33
>>>>>>>>>>>>>>>>>>>>>>> ++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph-gate.c | 34
>>>>>>>>>>>>>>>>>>>>>>> +++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph.c | 37
>>>>>>>>>>>>>>>>>>>>>>> ++++++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-sdmmc-mux.c | 28
>>>>>>>>>>>>>>>>>>>>>>> +++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk.h | 6 ++++++
>>>>>>>>>>>>>>>>>>>>>>>          5 files changed, 138 insertions(+)
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>>>> index c088e7a280df..21b24530fa00 100644
>>>>>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>>>> @@ -60,11 +60,44 @@
>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_recalc_rate(struct
>>>>>>>>>>>>>>>>>>>>>>> clk_hw *hw,
>>>>>>>>>>>>>>>>>>>>>>>              return (unsigned long)rate;
>>>>>>>>>>>>>>>>>>>>>>>          }
>>>>>>>>>>>>>>>>>>>>>>>          +static int
>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_save_context(struct
>>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>>> +    struct tegra_clk_periph_fixed *fixed =
>>>>>>>>>>>>>>>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>>>>>>>>>>>>>>>> +    u32 mask = 1 << (fixed->num % 32);
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> +    fixed->enb_ctx = readl_relaxed(fixed->base +
>>>>>>>>>>>>>>>>>>>>>>> fixed->regs->enb_reg) &
>>>>>>>>>>>>>>>>>>>>>>> +             mask;
>>>>>>>>>>>>>>>>>>>>>>> +    fixed->rst_ctx = readl_relaxed(fixed->base +
>>>>>>>>>>>>>>>>>>>>>>> fixed->regs->rst_reg) &
>>>>>>>>>>>>>>>>>>>>>>> +             mask;
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> +    return 0;
>>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> +static void
>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_restore_context(struct
>>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>>> +    struct tegra_clk_periph_fixed *fixed =
>>>>>>>>>>>>>>>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>>>>>>>>>>>>>>>> +    u32 mask = 1 << (fixed->num % 32);
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> +    if (fixed->enb_ctx)
>>>>>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>>>>>> fixed->regs->enb_set_reg);
>>>>>>>>>>>>>>>>>>>>>>> +    else
>>>>>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>>>>>> fixed->regs->enb_clr_reg);
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> +    udelay(2);
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> +    if (!fixed->rst_ctx) {
>>>>>>>>>>>>>>>>>>>>>>> +        udelay(5); /* reset propogation delay */
>>>>>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>>>>>> fixed->regs->rst_reg);
>>>>>>>>>>>>>>>>>>>>>>> +    }
>>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>>          static const struct clk_ops
>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_ops
>>>>>>>>>>>>>>>>>>>>>>> = {
>>>>>>>>>>>>>>>>>>>>>>>              .is_enabled =
>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_is_enabled,
>>>>>>>>>>>>>>>>>>>>>>>              .enable = tegra_clk_periph_fixed_enable,
>>>>>>>>>>>>>>>>>>>>>>>              .disable = tegra_clk_periph_fixed_disable,
>>>>>>>>>>>>>>>>>>>>>>> .recalc_rate =
>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_recalc_rate,
>>>>>>>>>>>>>>>>>>>>>>> +    .save_context =
>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_save_context,
>>>>>>>>>>>>>>>>>>>>>>> +    .restore_context =
>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_restore_context,
>>>>>>>>>>>>>>>>>>>>>>>          };
>>>>>>>>>>>>>>>>>>>>>>>            struct clk
>>>>>>>>>>>>>>>>>>>>>>> *tegra_clk_register_periph_fixed(const
>>>>>>>>>>>>>>>>>>>>>>> char
>>>>>>>>>>>>>>>>>>>>>>> *name,
>>>>>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>>>> index 4b31beefc9fc..6ba5b08e0787 100644
>>>>>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>>>> @@ -25,6 +25,8 @@ static
>>>>>>>>>>>>>>>>>>>>>>> DEFINE_SPINLOCK(periph_ref_lock);
>>>>>>>>>>>>>>>>>>>>>>>            #define read_rst(gate) \
>>>>>>>>>>>>>>>>>>>>>>> readl_relaxed(gate->clk_base +
>>>>>>>>>>>>>>>>>>>>>>> (gate->regs->rst_reg))
>>>>>>>>>>>>>>>>>>>>>>> +#define write_rst_set(val, gate) \
>>>>>>>>>>>>>>>>>>>>>>> +    writel_relaxed(val, gate->clk_base +
>>>>>>>>>>>>>>>>>>>>>>> (gate->regs->rst_set_reg))
>>>>>>>>>>>>>>>>>>>>>>>          #define write_rst_clr(val, gate) \
>>>>>>>>>>>>>>>>>>>>>>> writel_relaxed(val, gate->clk_base +
>>>>>>>>>>>>>>>>>>>>>>> (gate->regs->rst_clr_reg))
>>>>>>>>>>>>>>>>>>>>>>>          @@ -110,10 +112,42 @@ static void
>>>>>>>>>>>>>>>>>>>>>>> clk_periph_disable(struct
>>>>>>>>>>>>>>>>>>>>>>> clk_hw *hw)
>>>>>>>>>>>>>>>>>>>>>>> spin_unlock_irqrestore(&periph_ref_lock, flags);
>>>>>>>>>>>>>>>>>>>>>>>          }
>>>>>>>>>>>>>>>>>>>>>>>          +static int clk_periph_gate_save_context(struct
>>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>>> +    struct tegra_clk_periph_gate *gate =
>>>>>>>>>>>>>>>>>>>>>>> to_clk_periph_gate(hw);
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> + gate->clk_state_ctx = read_enb(gate) &
>>>>>>>>>>>>>>>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>>>>>>>>>>>>>>>> + gate->rst_state_ctx = read_rst(gate) &
>>>>>>>>>>>>>>>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> +    return 0;
>>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> +static void clk_periph_gate_restore_context(struct
>>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>>> +    struct tegra_clk_periph_gate *gate =
>>>>>>>>>>>>>>>>>>>>>>> to_clk_periph_gate(hw);
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> +    if (gate->clk_state_ctx)
>>>>>>>>>>>>>>>>>>>>>>> + write_enb_set(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>>>> +    else
>>>>>>>>>>>>>>>>>>>>>>> + write_enb_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> +    udelay(5);
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> +    if (!(gate->flags & TEGRA_PERIPH_NO_RESET) &&
>>>>>>>>>>>>>>>>>>>>>>> + !(gate->flags & TEGRA_PERIPH_MANUAL_RESET)) {
>>>>>>>>>>>>>>>>>>>>>>> +        if (gate->rst_state_ctx)
>>>>>>>>>>>>>>>>>>>>>>> + write_rst_set(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>>>> +        else
>>>>>>>>>>>>>>>>>>>>>>> + write_rst_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>>>> +    }
>>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>>          const struct clk_ops
>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_gate_ops = {
>>>>>>>>>>>>>>>>>>>>>>>              .is_enabled = clk_periph_is_enabled,
>>>>>>>>>>>>>>>>>>>>>>>              .enable = clk_periph_enable,
>>>>>>>>>>>>>>>>>>>>>>>              .disable = clk_periph_disable,
>>>>>>>>>>>>>>>>>>>>>>> +    .save_context = clk_periph_gate_save_context,
>>>>>>>>>>>>>>>>>>>>>>> +    .restore_context =
>>>>>>>>>>>>>>>>>>>>>>> clk_periph_gate_restore_context,
>>>>>>>>>>>>>>>>>>>>>>>          };
>>>>>>>>>>>>>>>>>>>>>>>            struct clk
>>>>>>>>>>>>>>>>>>>>>>> *tegra_clk_register_periph_gate(const
>>>>>>>>>>>>>>>>>>>>>>> char *name,
>>>>>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>>>> index 58437da25156..06fb62955768 100644
>>>>>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>>>> @@ -99,6 +99,37 @@ static void
>>>>>>>>>>>>>>>>>>>>>>> clk_periph_disable(struct
>>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>>> gate_ops->disable(gate_hw);
>>>>>>>>>>>>>>>>>>>>>>>          }
>>>>>>>>>>>>>>>>>>>>>>>          +static int clk_periph_save_context(struct
>>>>>>>>>>>>>>>>>>>>>>> clk_hw *hw)
>>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>>> +    struct tegra_clk_periph *periph =
>>>>>>>>>>>>>>>>>>>>>>> to_clk_periph(hw);
>>>>>>>>>>>>>>>>>>>>>>> +    const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>>>>>>>>>>>>>>>>>> +    struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> +    if (!(periph->gate.flags & TEGRA_PERIPH_NO_GATE))
>>>>>>>>>>>>>>>>>>>>>>> + gate_ops->save_context(gate_hw);
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> + periph->parent_ctx = clk_periph_get_parent(hw);
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> +    return 0;
>>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> +static void clk_periph_restore_context(struct clk_hw
>>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>>> +    struct tegra_clk_periph *periph =
>>>>>>>>>>>>>>>>>>>>>>> to_clk_periph(hw);
>>>>>>>>>>>>>>>>>>>>>>> +    const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>>>>>>>>>>>>>>>>>> +    struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>>>>>>>>>>>>>>>> +    const struct clk_ops *div_ops = periph->div_ops;
>>>>>>>>>>>>>>>>>>>>>>> +    struct clk_hw *div_hw = &periph->divider.hw;
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> + clk_periph_set_parent(hw, periph->parent_ctx);
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> +    if (!(periph->gate.flags & TEGRA_PERIPH_NO_DIV))
>>>>>>>>>>>>>>>>>>>>>>> + div_ops->restore_context(div_hw);
>>>>>>>>>>>>>>>>>>>>>> Could you please point to where the divider's
>>>>>>>>>>>>>>>>>>>>>> save_context()
>>>>>>>>>>>>>>>>>>>>>> happens?
>>>>>>>>>>>>>>>>>>>>>> Because I can't see it.
>>>>>>>>>>>>>>>>>>>>> Ah, I now see that there is no need to save the dividers
>>>>>>>>>>>>>>>>>>>>> context
>>>>>>>>>>>>>>>>>>>>> because
>>>>>>>>>>>>>>>>>>>>> clk itself has enough info that is needed for the
>>>>>>>>>>>>>>>>>>>>> context's
>>>>>>>>>>>>>>>>>>>>> restoring
>>>>>>>>>>>>>>>>>>>>> (like I pointed in the review to v6).
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Looks like you could also implement a new
>>>>>>>>>>>>>>>>>>>>> clk_hw_get_parent_index()
>>>>>>>>>>>>>>>>>>>>> generic helper to get the index instead of storing it
>>>>>>>>>>>>>>>>>>>>> manually.
>>>>>>>>>>>>>>>>>>>> clk_periph_get_parent basically invokes existing
>>>>>>>>>>>>>>>>>>>> clk_mux_ops
>>>>>>>>>>>>>>>>>>>> get_parent() which is then saved in tegra_clk_periph.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> All existing drivers are using directly get_parent() from
>>>>>>>>>>>>>>>>>>>> clk_mux
>>>>>>>>>>>>>>>>>>>> which actually gets index from the register read.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> To have this more generic w.r.t save/restore context
>>>>>>>>>>>>>>>>>>>> point of
>>>>>>>>>>>>>>>>>>>> view,
>>>>>>>>>>>>>>>>>>>> probably instead of implementing new get_parent_index
>>>>>>>>>>>>>>>>>>>> helper,
>>>>>>>>>>>>>>>>>>>> I think
>>>>>>>>>>>>>>>>>>>> its better to implement save_context and
>>>>>>>>>>>>>>>>>>>> restore_context to
>>>>>>>>>>>>>>>>>>>> clk_mux_ops along with creating parent_index field into
>>>>>>>>>>>>>>>>>>>> clk_mux to
>>>>>>>>>>>>>>>>>>>> cache index during set_parent.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> So we just need to invoke mux_ops save_context and
>>>>>>>>>>>>>>>>>>>> restore_context.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> I hope its ok to add save/restore context to clk_mux_ops
>>>>>>>>>>>>>>>>>>> to be
>>>>>>>>>>>>>>>>>>> more
>>>>>>>>>>>>>>>>>>> generic w.r.t save/restore context rather than
>>>>>>>>>>>>>>>>>>> get_parent_index
>>>>>>>>>>>>>>>>>>> API.
>>>>>>>>>>>>>>>>>>> Please confirm if you agree.
>>>>>>>>>>>>>>>>>> Sounds like a good idea. I see that there is a 'restoring'
>>>>>>>>>>>>>>>>>> helper for
>>>>>>>>>>>>>>>>>> the generic clk_gate, seems something similar could be done
>>>>>>>>>>>>>>>>>> for the
>>>>>>>>>>>>>>>>>> clk_mux. And looks like anyway you'll need to associate the
>>>>>>>>>>>>>>>>>> parent
>>>>>>>>>>>>>>>>>> clock
>>>>>>>>>>>>>>>>>> with the hw index in order to restore the muxing.
>>>>>>>>>>>>>>>>> by 'restoring' helper for generic clk_gate, are you
>>>>>>>>>>>>>>>>> referring to
>>>>>>>>>>>>>>>>> clk_gate_restore_context API?
>>>>>>>>>>>>>>>> Yes.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> clk_gate_restore_context is API that's any clk drivers can
>>>>>>>>>>>>>>>>> use for
>>>>>>>>>>>>>>>>> clk_gate operation restore for custom gate clk_ops.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> But clk-periph is directly using generic clk_mux ops from
>>>>>>>>>>>>>>>>> clk_mux
>>>>>>>>>>>>>>>>> so I
>>>>>>>>>>>>>>>>> think we should add .restore_context to clk_mux_ops and then
>>>>>>>>>>>>>>>>> during
>>>>>>>>>>>>>>>>> clk-periph restore need to invoke mux_ops->restore_context.
>>>>>>>>>>>>>>>> I'm not sure whether it will be good for every driver that
>>>>>>>>>>>>>>>> uses
>>>>>>>>>>>>>>>> generic
>>>>>>>>>>>>>>>> clk_mux ops. Should be more flexible to have a generic helper
>>>>>>>>>>>>>>>> function
>>>>>>>>>>>>>>>> that any driver could use in order to restore the clock's
>>>>>>>>>>>>>>>> parent.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> The clk-periph restoring also includes case of combining
>>>>>>>>>>>>>>>> divider
>>>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>>> parent restoring, so generic helper could be useful in that
>>>>>>>>>>>>>>>> case
>>>>>>>>>>>>>>>> as well.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> It also looks like you could actually use the
>>>>>>>>>>>>>>>> clk_gate_restore_context()
>>>>>>>>>>>>>>>> instead of manually saving the clock's enable-state, couldn't
>>>>>>>>>>>>>>>> you?
>>>>>>>>>>>>>>> ok for clk_mux, can add generic clk_mux_restore_context API
>>>>>>>>>>>>>>> rather
>>>>>>>>>>>>>>> than
>>>>>>>>>>>>>>> using restore_context in clk_ops and will invoke that during
>>>>>>>>>>>>>>> clk_periph
>>>>>>>>>>>>>>> restore.
>>>>>>>>>>>>>>>
>>>>>>>>>>> digging thru looks like for clk_periph source restore instead of
>>>>>>>>>>> clk_mux_restore_context, i can directly do clk_hw_get_parent and
>>>>>>>>>>> clk_set_parent with mux_hw as they invoke mux_ops get/set parent
>>>>>>>>>>> anyway.
>>>>>>>>>>> Will do this for periph clk mux
>>>> Just to be clear, clk_mux don't have cached parent. get_parent is by
>>>> register read. So, cant directly use get_parent and then set during
>>>> restore.
>>>>
>>>> So will create both clk_mux_save/restore_context in generic clk driver
>>>> and will invoke them during tegra peripheral clock suspend/resume.
>>> Why MUX clock doesn't have a cached parent? What MUX clock you're
>>> talking about?
>>>
>>> [snip]
>>
>> Please ignore got it.
>>
>> Will send next version after giving few more days for feedback.
>>
> Couple of issues:
> 
> 1.) I see clk-tegra-periph driver periph_clks init_data entries for some
> peripherals are not correct for Tegra 114 and later chips.
> 
> Eg I2C TEGRA_INIT_DATA_TABLE entries in clk-tegra-periph are used for all Tegra
> chipsets currently in the driver.
> 
> These entries are using MUX shift of 30 and MUX mask only for 2 bits which is
> correct for T30 and prior.
> But for later Tegra chips, it should be MUX shift 29 and MASK(3).
> 
> Also, I2C parent idx entries in mux_pllp_clkm_idx are different from Tegra114
> onwards.
> 
> As we are using only PLLP and CLKM sources only for I2C, their corresponding mux
> values from register spec by using upper 2 bits for T114 onwards match actual 2
> bits of MUX value on T30 and prior.
> 
> Not sure if this something known pending to port actual clock MUX table changes
> for Tegra114 onwards?
> 
> Or
> 
> Are we purposely using upper 2 bits only for clock source for T114 and later as
> the upper 2 bit values of the limited clock source we are using match with
> previous Tegra peripheral clock source mux values?

The actual hardware values are compatible on all Tegra SoCs and PLLC2 and PLLC3
are just unused on T114+, so should be okay.

> Peter/Thierry, Can you please help comment on this?
> 
> 
> 2.) Other issue is regarding using clk_set_parent directly during clk_peripheral
> restore is clk_core_set_parent checks new parent with current parent and if its
> same, it just returns as success which is good in normal operation.
> 
> But during restore, we can't use clk_set_parent as new parent is from
> clk_get_parent on restore and this is same as cached parent.
> 
> So clk_set_parent returns 0 but acutal register value for clk source is different
> as it gets reset on SC7 entry/exit and to restore need to invoke mux_ops
> set_parent with parent_index.
> 
> So this need parent index for cached parent and without using context variable to
> store this, need an API like you were originally suggesting for get_parent_index
> to get parent index for the specified parent clk_hw.
> 
> As we decided not to use save/restore for clk_mux ops as its generic for other
> drivers, looks like we need get_parent_index API to use for restoring peripheral
> source and use this with clk_mux_ops set_parent.
> 
> clk core driver already has clk_fetch_parent_index but is it OK to export this?
> 
> Otherwise, will create separate API in clk driver which returns parent index from
> parent clk_hw by using this existing clk_fetch_parent_index so this API can be
> used by other drivers.

The clk_fetch_parent_index can't be used directly as you can see because it uses
clk_core which is internal to clk.c, you'll need to make a wrapper for getting the
cached index.

Something like this:

int clk_hw_get_parent_index(struct clk_hw *hw, struct *parent_hw)
{
	if (!hw || !parent_hw)
		return -EINVAL;

	return clk_fetch_parent_index(hw->core, parent_hw->core);
}

^ 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