* [PATCH AUTOSEL 5.4 02/32] ARM: dts: rockchip: fix phy nodename for rk3229-xms6
From: Sasha Levin @ 2020-05-22 14:50 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, devicetree, Heiko Stuebner, linux-rockchip,
Johan Jonker, linux-arm-kernel
In-Reply-To: <20200522145044.434677-1-sashal@kernel.org>
From: Johan Jonker <jbx6244@gmail.com>
[ Upstream commit 621c8d0c233e260232278a4cfd3380caa3c1da29 ]
A test with the command below gives for example this error:
arch/arm/boot/dts/rk3229-xms6.dt.yaml: phy@0:
'#phy-cells' is a required property
The phy nodename is normally used by a phy-handle.
This node is however compatible with
"ethernet-phy-id1234.d400", "ethernet-phy-ieee802.3-c22"
which is just been added to 'ethernet-phy.yaml'.
So change nodename to 'ethernet-phy' for which '#phy-cells'
is not a required property
make ARCH=arm dtbs_check
DT_SCHEMA_FILES=~/.local/lib/python3.5/site-packages/dtschema/schemas/
phy/phy-provider.yaml
Signed-off-by: Johan Jonker <jbx6244@gmail.com>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Link: https://lore.kernel.org/r/20200416170321.4216-2-jbx6244@gmail.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/arm/boot/dts/rk3229-xms6.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/rk3229-xms6.dts b/arch/arm/boot/dts/rk3229-xms6.dts
index 679fc2b00e5a..933ef69da32a 100644
--- a/arch/arm/boot/dts/rk3229-xms6.dts
+++ b/arch/arm/boot/dts/rk3229-xms6.dts
@@ -150,7 +150,7 @@
#address-cells = <1>;
#size-cells = <0>;
- phy: phy@0 {
+ phy: ethernet-phy@0 {
compatible = "ethernet-phy-id1234.d400",
"ethernet-phy-ieee802.3-c22";
reg = <0>;
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH AUTOSEL 5.4 04/32] arm64: dts: rockchip: swap interrupts interrupt-names rk3399 gpu node
From: Sasha Levin @ 2020-05-22 14:50 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, devicetree, Heiko Stuebner, linux-rockchip,
Johan Jonker, linux-arm-kernel
In-Reply-To: <20200522145044.434677-1-sashal@kernel.org>
From: Johan Jonker <jbx6244@gmail.com>
[ Upstream commit c604fd810bda667bdc20b2c041917baa7803e0fb ]
Dts files with Rockchip rk3399 'gpu' nodes were manually verified.
In order to automate this process arm,mali-midgard.txt
has been converted to yaml. In the new setup dtbs_check with
arm,mali-midgard.yaml expects interrupts and interrupt-names values
in the same order. Fix this for rk3399.
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/gpu/
arm,mali-midgard.yaml
Signed-off-by: Johan Jonker <jbx6244@gmail.com>
Link: https://lore.kernel.org/r/20200425143837.18706-1-jbx6244@gmail.com
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index cd97016b7c18..c5d8d1c58291 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1881,10 +1881,10 @@
gpu: gpu@ff9a0000 {
compatible = "rockchip,rk3399-mali", "arm,mali-t860";
reg = <0x0 0xff9a0000 0x0 0x10000>;
- interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH 0>,
- <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH 0>,
- <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH 0>;
- interrupt-names = "gpu", "job", "mmu";
+ interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH 0>,
+ <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH 0>,
+ <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH 0>;
+ interrupt-names = "job", "mmu", "gpu";
clocks = <&cru ACLK_GPU>;
power-domains = <&power RK3399_PD_GPU>;
status = "disabled";
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH AUTOSEL 5.4 01/32] ARM: dts: rockchip: fix phy nodename for rk3228-evb
From: Sasha Levin @ 2020-05-22 14:50 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, devicetree, Heiko Stuebner, linux-rockchip,
Johan Jonker, linux-arm-kernel
From: Johan Jonker <jbx6244@gmail.com>
[ Upstream commit 287e0d538fcec2f6e8eb1e565bf0749f3b90186d ]
A test with the command below gives for example this error:
arch/arm/boot/dts/rk3228-evb.dt.yaml: phy@0:
'#phy-cells' is a required property
The phy nodename is normally used by a phy-handle.
This node is however compatible with
"ethernet-phy-id1234.d400", "ethernet-phy-ieee802.3-c22"
which is just been added to 'ethernet-phy.yaml'.
So change nodename to 'ethernet-phy' for which '#phy-cells'
is not a required property
make ARCH=arm dtbs_check
DT_SCHEMA_FILES=~/.local/lib/python3.5/site-packages/dtschema/schemas/
phy/phy-provider.yaml
Signed-off-by: Johan Jonker <jbx6244@gmail.com>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Link: https://lore.kernel.org/r/20200416170321.4216-1-jbx6244@gmail.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/arm/boot/dts/rk3228-evb.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/rk3228-evb.dts b/arch/arm/boot/dts/rk3228-evb.dts
index 5670b33fd1bd..aed879db6c15 100644
--- a/arch/arm/boot/dts/rk3228-evb.dts
+++ b/arch/arm/boot/dts/rk3228-evb.dts
@@ -46,7 +46,7 @@
#address-cells = <1>;
#size-cells = <0>;
- phy: phy@0 {
+ phy: ethernet-phy@0 {
compatible = "ethernet-phy-id1234.d400", "ethernet-phy-ieee802.3-c22";
reg = <0>;
clocks = <&cru SCLK_MAC_PHY>;
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH AUTOSEL 5.6 11/41] ARM: dts: omap4-droid4: Fix occasional lost wakeirq for uart1
From: Sasha Levin @ 2020-05-22 14:49 UTC (permalink / raw)
To: linux-kernel, stable
Cc: maemo-leste, Sasha Levin, devicetree, Tony Lindgren,
Merlijn Wajer, Sebastian Reichel, Pavel Machek, linux-arm-kernel
In-Reply-To: <20200522144959.434379-1-sashal@kernel.org>
From: Tony Lindgren <tony@atomide.com>
[ Upstream commit 738b150ecefbffb6e55cfa8a3b66a844f777d8fb ]
Looks like using the UART CTS pin does not always trigger for a wake-up
when the SoC is idle.
This is probably because the modem first uses gpio_149 to signal the SoC
that data will be sent, and the CTS will only get used later when the
data transfer is starting.
Let's fix the issue by configuring the gpio_149 pad as the wakeirq for
UART. We have gpio_149 managed by the USB PHY for powering up the right
USB mode, and after that, the gpio gets recycled as the modem wake-up
pin. If needeed, the USB PHY can also later on be configured to use
gpio_149 pad as the wakeirq as a shared irq.
Let's also configure the missing properties for uart-has-rtscts and
current-speed for the modem port while at it. We already configure the
hardware flow control pins with uart1_pins pinctrl setting.
Cc: maemo-leste@lists.dyne.org
Cc: Merlijn Wajer <merlijn@wizzup.org>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Sebastian Reichel <sre@kernel.org>
Signed-off-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/arm/boot/dts/motorola-mapphone-common.dtsi | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/motorola-mapphone-common.dtsi b/arch/arm/boot/dts/motorola-mapphone-common.dtsi
index 01ea9a1e2c86..06fbffa81636 100644
--- a/arch/arm/boot/dts/motorola-mapphone-common.dtsi
+++ b/arch/arm/boot/dts/motorola-mapphone-common.dtsi
@@ -723,14 +723,18 @@
};
/*
- * As uart1 is wired to mdm6600 with rts and cts, we can use the cts pin for
- * uart1 wakeirq.
+ * The uart1 port is wired to mdm6600 with rts and cts. The modem uses gpio_149
+ * for wake-up events for both the USB PHY and the UART. We can use gpio_149
+ * pad as the shared wakeirq for the UART rather than the RX or CTS pad as we
+ * have gpio_149 trigger before the UART transfer starts.
*/
&uart1 {
pinctrl-names = "default";
pinctrl-0 = <&uart1_pins>;
interrupts-extended = <&wakeupgen GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH
- &omap4_pmx_core 0xfc>;
+ &omap4_pmx_core 0x110>;
+ uart-has-rtscts;
+ current-speed = <115200>;
};
&uart3 {
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH AUTOSEL 5.6 10/41] ARM: dts: omap4-droid4: Fix flakey wlan by disabling internal pull for gpio
From: Sasha Levin @ 2020-05-22 14:49 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Tony Lindgren, Sasha Levin, linux-arm-kernel, devicetree
In-Reply-To: <20200522144959.434379-1-sashal@kernel.org>
From: Tony Lindgren <tony@atomide.com>
[ Upstream commit 30fa60c678eaa27b8f2a531920d77f7184658f73 ]
The wlan on droid4 is flakey on some devices, and experiments have shown this
gets fixed if we disable the internal pull for wlan gpio interrupt line.
The symptoms are that the wlan connection is very slow and almost useless
with lots of wlcore firmware reboot warnings in the dmesg.
In addition to configuring the wlan gpio pulls, let's also configure the rest
of the wlan sd pins. We have not configured those eariler as we're booting
using kexec.
Signed-off-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
.../boot/dts/motorola-mapphone-common.dtsi | 33 +++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/arch/arm/boot/dts/motorola-mapphone-common.dtsi b/arch/arm/boot/dts/motorola-mapphone-common.dtsi
index 9067e0ef4240..01ea9a1e2c86 100644
--- a/arch/arm/boot/dts/motorola-mapphone-common.dtsi
+++ b/arch/arm/boot/dts/motorola-mapphone-common.dtsi
@@ -367,6 +367,8 @@
};
&mmc3 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&mmc3_pins>;
vmmc-supply = <&wl12xx_vmmc>;
/* uart2_tx.sdmmc3_dat1 pad as wakeirq */
interrupts-extended = <&wakeupgen GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH
@@ -472,6 +474,37 @@
>;
};
+ /*
+ * Android uses PIN_OFF_INPUT_PULLDOWN | PIN_INPUT_PULLUP | MUX_MODE3
+ * for gpio_100, but the internal pull makes wlan flakey on some
+ * devices. Off mode value should be tested if we have off mode working
+ * later on.
+ */
+ mmc3_pins: pinmux_mmc3_pins {
+ pinctrl-single,pins = <
+ /* 0x4a10008e gpmc_wait2.gpio_100 d23 */
+ OMAP4_IOPAD(0x08e, PIN_INPUT | MUX_MODE3)
+
+ /* 0x4a100102 abe_mcbsp1_dx.sdmmc3_dat2 ab25 */
+ OMAP4_IOPAD(0x102, PIN_INPUT_PULLUP | MUX_MODE1)
+
+ /* 0x4a100104 abe_mcbsp1_fsx.sdmmc3_dat3 ac27 */
+ OMAP4_IOPAD(0x104, PIN_INPUT_PULLUP | MUX_MODE1)
+
+ /* 0x4a100118 uart2_cts.sdmmc3_clk ab26 */
+ OMAP4_IOPAD(0x118, PIN_INPUT | MUX_MODE1)
+
+ /* 0x4a10011a uart2_rts.sdmmc3_cmd ab27 */
+ OMAP4_IOPAD(0x11a, PIN_INPUT_PULLUP | MUX_MODE1)
+
+ /* 0x4a10011c uart2_rx.sdmmc3_dat0 aa25 */
+ OMAP4_IOPAD(0x11c, PIN_INPUT_PULLUP | MUX_MODE1)
+
+ /* 0x4a10011e uart2_tx.sdmmc3_dat1 aa26 */
+ OMAP4_IOPAD(0x11e, PIN_INPUT_PULLUP | MUX_MODE1)
+ >;
+ };
+
/* gpmc_ncs0.gpio_50 */
poweroff_gpio: pinmux_poweroff_pins {
pinctrl-single,pins = <
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH AUTOSEL 5.6 07/41] ARM: dts: rockchip: fix pinctrl sub nodename for spi in rk322x.dtsi
From: Sasha Levin @ 2020-05-22 14:49 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, devicetree, Heiko Stuebner, linux-rockchip,
Johan Jonker, linux-arm-kernel
In-Reply-To: <20200522144959.434379-1-sashal@kernel.org>
From: Johan Jonker <jbx6244@gmail.com>
[ Upstream commit 855bdca1781c79eb661f89c8944c4a719ce720e8 ]
A test with the command below gives these errors:
arch/arm/boot/dts/rk3229-evb.dt.yaml: spi-0:
'#address-cells' is a required property
arch/arm/boot/dts/rk3229-evb.dt.yaml: spi-1:
'#address-cells' is a required property
arch/arm/boot/dts/rk3229-xms6.dt.yaml: spi-0:
'#address-cells' is a required property
arch/arm/boot/dts/rk3229-xms6.dt.yaml: spi-1:
'#address-cells' is a required property
The $nodename pattern for spi nodes is
"^spi(@.*|-[0-9a-f])*$". To prevent warnings rename
'spi-0' and 'spi-1' pinctrl sub nodenames to
'spi0' and 'spi1' in 'rk322x.dtsi'.
make ARCH=arm dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/spi-controller.yaml
Signed-off-by: Johan Jonker <jbx6244@gmail.com>
Link: https://lore.kernel.org/r/20200424123923.8192-1-jbx6244@gmail.com
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/arm/boot/dts/rk322x.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/rk322x.dtsi b/arch/arm/boot/dts/rk322x.dtsi
index 729119952c68..a83f65486ad4 100644
--- a/arch/arm/boot/dts/rk322x.dtsi
+++ b/arch/arm/boot/dts/rk322x.dtsi
@@ -1033,7 +1033,7 @@
};
};
- spi-0 {
+ spi0 {
spi0_clk: spi0-clk {
rockchip,pins = <0 RK_PB1 2 &pcfg_pull_up>;
};
@@ -1051,7 +1051,7 @@
};
};
- spi-1 {
+ spi1 {
spi1_clk: spi1-clk {
rockchip,pins = <0 RK_PC7 2 &pcfg_pull_up>;
};
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH AUTOSEL 5.6 06/41] ARM: dts: rockchip: swap clock-names of gpu nodes
From: Sasha Levin @ 2020-05-22 14:49 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, devicetree, Heiko Stuebner, linux-rockchip,
Johan Jonker, linux-arm-kernel
In-Reply-To: <20200522144959.434379-1-sashal@kernel.org>
From: Johan Jonker <jbx6244@gmail.com>
[ Upstream commit b14f3898d2c25a9b47a61fb879d0b1f3af92c59b ]
Dts files with Rockchip 'gpu' nodes were manually verified.
In order to automate this process arm,mali-utgard.txt
has been converted to yaml. In the new setup dtbs_check with
arm,mali-utgard.yaml expects clock-names values
in the same order, so fix that.
Signed-off-by: Johan Jonker <jbx6244@gmail.com>
Link: https://lore.kernel.org/r/20200425192500.1808-1-jbx6244@gmail.com
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/arm/boot/dts/rk3036.dtsi | 2 +-
arch/arm/boot/dts/rk322x.dtsi | 2 +-
arch/arm/boot/dts/rk3xxx.dtsi | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/rk3036.dtsi b/arch/arm/boot/dts/rk3036.dtsi
index cf36e25195b4..8c4b8f56c9e0 100644
--- a/arch/arm/boot/dts/rk3036.dtsi
+++ b/arch/arm/boot/dts/rk3036.dtsi
@@ -128,7 +128,7 @@
assigned-clocks = <&cru SCLK_GPU>;
assigned-clock-rates = <100000000>;
clocks = <&cru SCLK_GPU>, <&cru SCLK_GPU>;
- clock-names = "core", "bus";
+ clock-names = "bus", "core";
resets = <&cru SRST_GPU>;
status = "disabled";
};
diff --git a/arch/arm/boot/dts/rk322x.dtsi b/arch/arm/boot/dts/rk322x.dtsi
index 4e90efdc9630..729119952c68 100644
--- a/arch/arm/boot/dts/rk322x.dtsi
+++ b/arch/arm/boot/dts/rk322x.dtsi
@@ -561,7 +561,7 @@
"pp1",
"ppmmu1";
clocks = <&cru ACLK_GPU>, <&cru ACLK_GPU>;
- clock-names = "core", "bus";
+ clock-names = "bus", "core";
resets = <&cru SRST_GPU_A>;
status = "disabled";
};
diff --git a/arch/arm/boot/dts/rk3xxx.dtsi b/arch/arm/boot/dts/rk3xxx.dtsi
index 241f43e29c77..bb5ff10b9110 100644
--- a/arch/arm/boot/dts/rk3xxx.dtsi
+++ b/arch/arm/boot/dts/rk3xxx.dtsi
@@ -84,7 +84,7 @@
compatible = "arm,mali-400";
reg = <0x10090000 0x10000>;
clocks = <&cru ACLK_GPU>, <&cru ACLK_GPU>;
- clock-names = "core", "bus";
+ clock-names = "bus", "core";
assigned-clocks = <&cru ACLK_GPU>;
assigned-clock-rates = <100000000>;
resets = <&cru SRST_GPU>;
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH AUTOSEL 5.6 04/41] arm64: dts: rockchip: fix status for &gmac2phy in rk3328-evb.dts
From: Sasha Levin @ 2020-05-22 14:49 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, devicetree, Heiko Stuebner, linux-rockchip,
Johan Jonker, linux-arm-kernel
In-Reply-To: <20200522144959.434379-1-sashal@kernel.org>
From: Johan Jonker <jbx6244@gmail.com>
[ Upstream commit c617ed88502d0b05149e7f32f3b3fd8a0663f7e2 ]
The status was removed of the '&gmac2phy' node with the apply
of a patch long time ago, so fix status for '&gmac2phy'
in 'rk3328-evb.dts'.
Signed-off-by: Johan Jonker <jbx6244@gmail.com>
Link: https://lore.kernel.org/r/20200425122345.12902-2-jbx6244@gmail.com
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/arm64/boot/dts/rockchip/rk3328-evb.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3328-evb.dts b/arch/arm64/boot/dts/rockchip/rk3328-evb.dts
index 6abc6f4a86cf..05265b38cc02 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-evb.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-evb.dts
@@ -86,7 +86,7 @@
assigned-clock-rate = <50000000>;
assigned-clocks = <&cru SCLK_MAC2PHY>;
assigned-clock-parents = <&cru SCLK_MAC2PHY_SRC>;
-
+ status = "okay";
};
&i2c1 {
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH AUTOSEL 5.6 05/41] arm64: dts: rockchip: swap interrupts interrupt-names rk3399 gpu node
From: Sasha Levin @ 2020-05-22 14:49 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, devicetree, Heiko Stuebner, linux-rockchip,
Johan Jonker, linux-arm-kernel
In-Reply-To: <20200522144959.434379-1-sashal@kernel.org>
From: Johan Jonker <jbx6244@gmail.com>
[ Upstream commit c604fd810bda667bdc20b2c041917baa7803e0fb ]
Dts files with Rockchip rk3399 'gpu' nodes were manually verified.
In order to automate this process arm,mali-midgard.txt
has been converted to yaml. In the new setup dtbs_check with
arm,mali-midgard.yaml expects interrupts and interrupt-names values
in the same order. Fix this for rk3399.
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/gpu/
arm,mali-midgard.yaml
Signed-off-by: Johan Jonker <jbx6244@gmail.com>
Link: https://lore.kernel.org/r/20200425143837.18706-1-jbx6244@gmail.com
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 5c4238a80144..c341172ec208 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1890,10 +1890,10 @@
gpu: gpu@ff9a0000 {
compatible = "rockchip,rk3399-mali", "arm,mali-t860";
reg = <0x0 0xff9a0000 0x0 0x10000>;
- interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH 0>,
- <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH 0>,
- <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH 0>;
- interrupt-names = "gpu", "job", "mmu";
+ interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH 0>,
+ <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH 0>,
+ <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH 0>;
+ interrupt-names = "job", "mmu", "gpu";
clocks = <&cru ACLK_GPU>;
#cooling-cells = <2>;
power-domains = <&power RK3399_PD_GPU>;
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH AUTOSEL 5.6 03/41] ARM: dts: rockchip: fix phy nodename for rk3229-xms6
From: Sasha Levin @ 2020-05-22 14:49 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, devicetree, Heiko Stuebner, linux-rockchip,
Johan Jonker, linux-arm-kernel
In-Reply-To: <20200522144959.434379-1-sashal@kernel.org>
From: Johan Jonker <jbx6244@gmail.com>
[ Upstream commit 621c8d0c233e260232278a4cfd3380caa3c1da29 ]
A test with the command below gives for example this error:
arch/arm/boot/dts/rk3229-xms6.dt.yaml: phy@0:
'#phy-cells' is a required property
The phy nodename is normally used by a phy-handle.
This node is however compatible with
"ethernet-phy-id1234.d400", "ethernet-phy-ieee802.3-c22"
which is just been added to 'ethernet-phy.yaml'.
So change nodename to 'ethernet-phy' for which '#phy-cells'
is not a required property
make ARCH=arm dtbs_check
DT_SCHEMA_FILES=~/.local/lib/python3.5/site-packages/dtschema/schemas/
phy/phy-provider.yaml
Signed-off-by: Johan Jonker <jbx6244@gmail.com>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Link: https://lore.kernel.org/r/20200416170321.4216-2-jbx6244@gmail.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/arm/boot/dts/rk3229-xms6.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/rk3229-xms6.dts b/arch/arm/boot/dts/rk3229-xms6.dts
index 679fc2b00e5a..933ef69da32a 100644
--- a/arch/arm/boot/dts/rk3229-xms6.dts
+++ b/arch/arm/boot/dts/rk3229-xms6.dts
@@ -150,7 +150,7 @@
#address-cells = <1>;
#size-cells = <0>;
- phy: phy@0 {
+ phy: ethernet-phy@0 {
compatible = "ethernet-phy-id1234.d400",
"ethernet-phy-ieee802.3-c22";
reg = <0>;
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH AUTOSEL 5.6 02/41] ARM: dts: rockchip: fix phy nodename for rk3228-evb
From: Sasha Levin @ 2020-05-22 14:49 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, devicetree, Heiko Stuebner, linux-rockchip,
Johan Jonker, linux-arm-kernel
In-Reply-To: <20200522144959.434379-1-sashal@kernel.org>
From: Johan Jonker <jbx6244@gmail.com>
[ Upstream commit 287e0d538fcec2f6e8eb1e565bf0749f3b90186d ]
A test with the command below gives for example this error:
arch/arm/boot/dts/rk3228-evb.dt.yaml: phy@0:
'#phy-cells' is a required property
The phy nodename is normally used by a phy-handle.
This node is however compatible with
"ethernet-phy-id1234.d400", "ethernet-phy-ieee802.3-c22"
which is just been added to 'ethernet-phy.yaml'.
So change nodename to 'ethernet-phy' for which '#phy-cells'
is not a required property
make ARCH=arm dtbs_check
DT_SCHEMA_FILES=~/.local/lib/python3.5/site-packages/dtschema/schemas/
phy/phy-provider.yaml
Signed-off-by: Johan Jonker <jbx6244@gmail.com>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Link: https://lore.kernel.org/r/20200416170321.4216-1-jbx6244@gmail.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/arm/boot/dts/rk3228-evb.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/rk3228-evb.dts b/arch/arm/boot/dts/rk3228-evb.dts
index 5670b33fd1bd..aed879db6c15 100644
--- a/arch/arm/boot/dts/rk3228-evb.dts
+++ b/arch/arm/boot/dts/rk3228-evb.dts
@@ -46,7 +46,7 @@
#address-cells = <1>;
#size-cells = <0>;
- phy: phy@0 {
+ phy: ethernet-phy@0 {
compatible = "ethernet-phy-id1234.d400", "ethernet-phy-ieee802.3-c22";
reg = <0>;
clocks = <&cru SCLK_MAC_PHY>;
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH AUTOSEL 5.6 01/41] arm64: dts: qcom: db820c: fix audio configuration
From: Sasha Levin @ 2020-05-22 14:49 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, devicetree, Bjorn Andersson, Vinod Koul,
Srinivas Kandagatla, linux-arm-kernel
From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
[ Upstream commit 7710f80ecd9c74544a22557ab581cf603e713f51 ]
After patch f864edff110d ("ASoC: qdsp6: q6routing: remove default routing")
and 9b60441692d9 ("ASoC: qdsp6: q6asm-dai: only enable dais from device tree")
asm dais and routing needs to be properly specified at device tree level.
This patch fixes this.
Tested-by: Vinod Koul <vkoul@kernel.org>
Reviewed-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Link: https://lore.kernel.org/r/20200422101922.8894-1-srinivas.kandagatla@linaro.org
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 19 ++++++++++++++++++-
arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 ++
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
index a85b85d85a5f..3c7c9b52623c 100644
--- a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
@@ -908,10 +908,27 @@
status = "okay";
};
+&q6asmdai {
+ dai@0 {
+ reg = <0>;
+ };
+
+ dai@1 {
+ reg = <1>;
+ };
+
+ dai@2 {
+ reg = <2>;
+ };
+};
+
&sound {
compatible = "qcom,apq8096-sndcard";
model = "DB820c";
- audio-routing = "RX_BIAS", "MCLK";
+ audio-routing = "RX_BIAS", "MCLK",
+ "MM_DL1", "MultiMedia1 Playback",
+ "MM_DL2", "MultiMedia2 Playback",
+ "MultiMedia3 Capture", "MM_UL3";
mm1-dai-link {
link-name = "MultiMedia1";
diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 7ae082ea14ea..f925a6c7d293 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -2053,6 +2053,8 @@
reg = <APR_SVC_ASM>;
q6asmdai: dais {
compatible = "qcom,q6asm-dais";
+ #address-cells = <1>;
+ #size-cells = <0>;
#sound-dai-cells = <1>;
iommus = <&lpass_q6_smmu 1>;
};
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH 2/2] firmware: smccc: Add ARCH_SOC_ID support
From: Arnd Bergmann @ 2020-05-22 14:46 UTC (permalink / raw)
To: Sudeep Holla
Cc: Mark Rutland, Francois Ozog, Lorenzo Pieralisi,
Greg Kroah-Hartman, linux-kernel@vger.kernel.org, harb,
Will Deacon, Linux ARM
In-Reply-To: <20200522124951.35776-3-sudeep.holla@arm.com>
On Fri, May 22, 2020 at 2:50 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> +
> + soc_id_rev = res.a0;
> +
> + soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> + if (!soc_dev_attr)
> + return -ENOMEM;
> +
> + sprintf(soc_id_str, "0x%04x", IMP_DEF_SOC_ID(soc_id_version));
> + sprintf(soc_id_rev_str, "0x%08x", soc_id_rev);
> + sprintf(soc_id_jep106_id_str, "0x%02x%02x",
> + JEP106_BANK_CONT_CODE(soc_id_version),
> + JEP106_ID_CODE(soc_id_version));
> +
> + soc_dev_attr->soc_id = soc_id_str;
> + soc_dev_attr->revision = soc_id_rev_str;
> + soc_dev_attr->jep106_id = soc_id_jep106_id_str;
Ok, let me try to understand how this maps the 64-bit ID into the
six strings in user space:
For a chip that identifies as
JEP106_BANK_CONT_CODE = 12
JEP106_ID_CODE = 34
IMP_DEF_SOC_ID = 5678
soc_id_rev = 9abcdef0
the normal sysfs attributes contain these strings:
machine = ""
family = ""
revision = "0x9abcdef0
serial_number = ""
soc_id = "0x5678"
and the new attribute is
jep106_identification_code = "0x1234"
This still looks like a rather poorly designed interface to me, with a
number of downsides:
- Nothing in those strings identifies the numbers as using jep106
numbers rather than some something else that might use strings
with hexadecimal numbers.
- I think we should have something unique in "family" just because
existing scripts can use that as the primary indentifier
- It seems odd that there is no way to read the serial number through
the same interface and publish it the usual way. Francois Ozog
recently asked for a generic way to find out a serial number for
inventory management, and this would be the obvious place to have it.
It can of course be added later when the next revision of the spec
is there, it just seems like a surprising omission.
How about making the contents:
machine = "" /* could be a future addition, but board specific */
family = "jep106:1234"
revision = "0x9abcdef0
serial_number = "0xfedcba987654321" /* to be implemented later */
soc_id = "jep106:1234:5678" /* duplicates family but makes it unique*/
That would work without any new properties, dropping the other patch,
and be easier to use for identification from user space.
Arnd
_______________________________________________
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 v12 2/3] i2c: npcm7xx: Add Nuvoton NPCM I2C controller driver
From: Wolfram Sang @ 2020-05-22 14:45 UTC (permalink / raw)
To: Tali Perry
Cc: devicetree, tmaimon77, yuenn, avifishman70, venture, openbmc,
brendanhiggins, ofery, linux-kernel, kfting, robh+dt, linux-i2c,
andriy.shevchenko, linux-arm-kernel, benjaminfair
In-Reply-To: <20200521110910.45518-3-tali.perry1@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 13108 bytes --]
On Thu, May 21, 2020 at 02:09:09PM +0300, Tali Perry wrote:
> Add Nuvoton NPCM BMC I2C controller driver.
>
> Signed-off-by: Tali Perry <tali.perry1@gmail.com>
This is a very complex driver, so I can really comment only about high
level things. Thank you very much for keeping at it!
My code checkers say:
CHECKPATCH:
CHECK: usleep_range is preferred over udelay; see Documentation/timers/timers-howto.rst
#1210: FILE: drivers/i2c/busses/i2c-npcm7xx.c:1161:
+ udelay(200);
(a few of them)
GCC:
CC drivers/i2c/busses/i2c-npcm7xx.o
drivers/i2c/busses/i2c-npcm7xx.c: In function ‘npcm_i2c_reset’:
drivers/i2c/busses/i2c-npcm7xx.c:521:5: warning: variable ‘i2cctl2’ set but not used [-Wunused-but-set-variable]
> +/* Status of one I2C module */
> +struct npcm_i2c {
> + struct i2c_adapter adap;
> + struct device *dev;
> + unsigned char __iomem *reg;
> + spinlock_t lock; /* IRQ synchronization */
> + struct completion cmd_complete;
> + int irq;
> + int cmd_err;
> + struct i2c_msg *msgs;
> + int msgs_num;
> + int num;
> + u32 apb_clk;
> + struct i2c_bus_recovery_info rinfo;
> + enum i2c_state state;
> + enum i2c_oper operation;
> + enum i2c_mode master_or_slave;
> + enum i2c_state_ind stop_ind;
> + u8 dest_addr;
> + u8 *rd_buf;
> + u16 rd_size;
> + u16 rd_ind;
> + u8 *wr_buf;
> + u16 wr_size;
> + u16 wr_ind;
> + bool fifo_use;
> + u16 PEC_mask; /* PEC bit mask per slave address */
> + bool PEC_use;
> + bool read_block_use;
> + u8 int_cnt;
What is this for? It is written to but never read.
> + u32 clk_period_us;
Not used? Seems this struct could need some cleaning up.
> + unsigned long int_time_stamp;
> + unsigned long bus_freq; /* in kHz */
> + u32 xmits;
> +#ifdef CONFIG_DEBUG_FS
> + struct dentry *debugfs; /* debugfs device directory */
> + u64 ber_cnt;
> + u64 rec_succ_cnt;
> + u64 rec_fail_cnt;
> + u64 nack_cnt;
> + u64 timeout_cnt;
> +#endif
> +};
> +
...
> +static inline u16 npcm_i2c_get_index(struct npcm_i2c *bus)
> +{
> + if (bus->operation == I2C_READ_OPER)
> + return bus->rd_ind;
> + if (bus->operation == I2C_WRITE_OPER)
> + return bus->wr_ind;
> + return 0;
I2C_NO_OPER?
...
> +/* recovery using bit banging functionality of the module */
> +static int npcm_i2c_recovery_init(struct i2c_adapter *_adap)
> +{
> + struct npcm_i2c *bus = container_of(_adap, struct npcm_i2c, adap);
> + struct i2c_bus_recovery_info *rinfo = &bus->rinfo;
> +
> + rinfo->recover_bus = npcm_i2c_recovery_tgclk;
> + rinfo->prepare_recovery = NULL;
> + rinfo->unprepare_recovery = NULL;
> + rinfo->set_scl = NULL;
> + rinfo->set_sda = NULL;
'bus' is kzalloced, so no need for these NULLs.
What I wonder more, though, is if you can't populate {set|get}_{scl|sda}
and use the internal i2c_generic_scl_recovery()? Are there any issues
with it?
> +
> + dev_dbg(bus->dev, "init i2c recovery using TGCLK\n");
There is no error path here, so I think this message is not useful.
Means also this function could be 'void'.
> +
> + rinfo->get_scl = npcm_i2c_get_SCL;
> + rinfo->get_sda = npcm_i2c_get_SDA;
Not needed when you have a custom function.
> +
> + _adap->bus_recovery_info = rinfo;
> +
> + return 0;
> +}
> +
...
> +static int npcm_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> + int num)
> +{
> + struct npcm_i2c *bus = container_of(adap, struct npcm_i2c, adap);
> + struct i2c_msg *msg0, *msg1;
> + unsigned long time_left, flags;
> + u16 nwrite, nread;
> + u8 *write_data, *read_data;
> + u8 slave_addr;
> + int timeout;
> + int ret = 0;
> + bool read_block = false;
> + bool read_PEC = false;
> + u8 bus_busy;
> + unsigned long timeout_usec;
> +
> + if (bus->state == I2C_DISABLE) {
> + dev_err(bus->dev, "I2C%d module is disabled", bus->num);
> + return -EINVAL;
> + }
> +
> + if (num > 2 || num < 1) {
> + dev_err(bus->dev, "I2C cmd not supported num of msgs=%d", num);
> + return -EINVAL;
> + }
Since you have an 'i2c_adapter_quirks' struct filled, the core will
I2C check that for you.
> +
> + msg0 = &msgs[0];
> + slave_addr = msg0->addr;
> + if (msg0->flags & I2C_M_RD) { /* read */
> + if (num == 2) {
> + dev_err(bus->dev, "num=2 but 1st msg rd instead of wr");
> + return -EINVAL;
Ditto.
> + }
> + nwrite = 0;
> + write_data = NULL;
> + read_data = msg0->buf;
> + if (msg0->flags & I2C_M_RECV_LEN) {
> + nread = 1;
> + read_block = true;
> + if (msg0->flags & I2C_CLIENT_PEC)
> + read_PEC = true;
> + } else {
> + nread = msg0->len;
> + }
> + } else { /* write */
> + nwrite = msg0->len;
> + write_data = msg0->buf;
> + nread = 0;
> + read_data = NULL;
> + if (num == 2) {
> + msg1 = &msgs[1];
> + read_data = msg1->buf;
> + if (slave_addr != msg1->addr) {
> + dev_err(bus->dev,
> + "SA==%02x but msg1->addr==%02x\n",
> + slave_addr, msg1->addr);
> + return -EINVAL;
Ditto.
> + }
> + if ((msg1->flags & I2C_M_RD) == 0) {
> + dev_err(bus->dev,
> + "num = 2 but both msg are write.\n");
> + return -EINVAL;
> + }
Ditto.
> + if (msg1->flags & I2C_M_RECV_LEN) {
> + nread = 1;
> + read_block = true;
> + if (msg1->flags & I2C_CLIENT_PEC)
> + read_PEC = true;
> + } else {
> + nread = msg1->len;
> + read_block = false;
> + }
> + }
> + }
> +
> + /* Adaptive TimeOut: astimated time in usec + 100% margin */
> + timeout_usec = (2 * 10000 / bus->bus_freq) * (2 + nread + nwrite);
> + timeout = max(msecs_to_jiffies(35), usecs_to_jiffies(timeout_usec));
> + if (nwrite >= 32 * 1024 || nread >= 32 * 1024) {
> + dev_err(bus->dev, "i2c%d buffer too big\n", bus->num);
> + return -EINVAL;
> + }
Ditto.
> +
> + time_left = jiffies + msecs_to_jiffies(DEFAULT_STALL_COUNT) + 1;
> + do {
> + /*
> + * we must clear slave address immediately when the bus is not
> + * busy, so we spinlock it, but we don't keep the lock for the
> + * entire while since it is too long.
> + */
> + spin_lock_irqsave(&bus->lock, flags);
> + bus_busy = ioread8(bus->reg + NPCM_I2CCST) & NPCM_I2CCST_BB;
> + spin_unlock_irqrestore(&bus->lock, flags);
> +
> + } while (time_is_after_jiffies(time_left) && bus_busy);
> +
> + if (bus_busy) {
> + iowrite8(NPCM_I2CCST_BB, bus->reg + NPCM_I2CCST);
> + npcm_i2c_reset(bus);
> + i2c_recover_bus(adap);
> + return -EAGAIN;
> + }
> +
> + npcm_i2c_init_params(bus);
> + bus->dest_addr = slave_addr;
> + bus->msgs = msgs;
> + bus->msgs_num = num;
> + bus->cmd_err = 0;
> + bus->read_block_use = read_block;
> +
> + reinit_completion(&bus->cmd_complete);
> + if (!npcm_i2c_master_start_xmit(bus, slave_addr, nwrite, nread,
> + write_data, read_data, read_PEC,
> + read_block))
> + ret = -EBUSY;
> +
> + if (ret != -EBUSY) {
> + time_left = wait_for_completion_timeout(&bus->cmd_complete,
> + timeout);
> +
> + if (time_left == 0) {
> +#ifdef CONFIG_DEBUG_FS
> + if (bus->timeout_cnt == ULLONG_MAX) {
> + dev_dbg(bus->dev,
> + "timeout_cnt reach max, reset to 0");
> + bus->timeout_cnt = 0;
> + }
> + bus->timeout_cnt++;
> +#endif
> + if (bus->master_or_slave == I2C_MASTER) {
> + i2c_recover_bus(adap);
> + bus->cmd_err = -EIO;
> + bus->state = I2C_IDLE;
> + }
> + }
> + }
> + ret = bus->cmd_err;
> +
> + /* if there was BER, check if need to recover the bus: */
> + if (bus->cmd_err == -EAGAIN)
> + ret = i2c_recover_bus(adap);
> +
> + return bus->cmd_err;
> +}
> +
> +static u32 npcm_i2c_functionality(struct i2c_adapter *adap)
> +{
> + return I2C_FUNC_I2C |
> + I2C_FUNC_SMBUS_EMUL |
> + I2C_FUNC_SMBUS_BLOCK_DATA |
> + I2C_FUNC_SMBUS_PEC |
> + I2C_FUNC_SLAVE;
> +}
> +
> +static const struct i2c_adapter_quirks npcm_i2c_quirks = {
> + .max_read_len = 32768,
> + .max_write_len = 32768,
These limits are for simple reads/writes with num_msgs == 1. If you have
limits also for num_msgs == 2, then you also need to fill
'max_comb_1st_msg_len' and 'max_comb_2nd_msg_len'. (Because for some HW
these are different values then)
> + .max_num_msgs = 2,
You can drop this because I2C_AQ_COMB_WRITE_THEN_READ implies it.
> + .flags = I2C_AQ_COMB_WRITE_THEN_READ,
> +};
> +
> +static const struct i2c_algorithm npcm_i2c_algo = {
> + .master_xfer = npcm_i2c_master_xfer,
> + .functionality = npcm_i2c_functionality,
> +};
> +
> +#ifdef CONFIG_DEBUG_FS
> +/* i2c debugfs directory: used to keep health monitor of i2c devices */
> +static struct dentry *npcm_i2c_debugfs_dir;
> +
> +static void i2c_init_debugfs(struct platform_device *pdev, struct npcm_i2c *bus)
> +{
> + struct dentry *d;
> +
> + if (!npcm_i2c_debugfs_dir)
> + return;
> +
> + d = debugfs_create_dir(dev_name(&pdev->dev), npcm_i2c_debugfs_dir);
> + if (IS_ERR_OR_NULL(d))
> + return;
> +
> + debugfs_create_u64("ber_cnt", 0444, d, &bus->ber_cnt);
> + debugfs_create_u64("nack_cnt", 0444, d, &bus->nack_cnt);
> + debugfs_create_u64("rec_succ_cnt", 0444, d, &bus->rec_succ_cnt);
> + debugfs_create_u64("rec_fail_cnt", 0444, d, &bus->rec_fail_cnt);
> + debugfs_create_u64("timeout_cnt", 0444, d, &bus->timeout_cnt);
> +
> + bus->debugfs = d;
> +}
> +#else
> +static void i2c_init_debugfs(struct platform_device *pdev, struct npcm_i2c *bus)
> +{
> +}
> +#endif
> +
> +static int npcm_i2c_probe_bus(struct platform_device *pdev)
> +{
> + struct npcm_i2c *bus;
> + struct i2c_adapter *adap;
> + struct clk *i2c_clk;
> + static struct regmap *gcr_regmap;
> + static struct regmap *clk_regmap;
> + int ret;
> + int num;
> +
> + bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
> + if (!bus)
> + return -ENOMEM;
> +
> + bus->dev = &pdev->dev;
> +
> + num = of_alias_get_id(pdev->dev.of_node, "i2c");
> + bus->num = num;
Why not assigning it directly and save the 'num' variable?
> + /* core clk must be acquired to calculate module timing settings */
> + i2c_clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(i2c_clk))
> + return PTR_ERR(i2c_clk);
> + bus->apb_clk = clk_get_rate(i2c_clk);
> +
> + gcr_regmap = syscon_regmap_lookup_by_compatible("nuvoton,npcm750-gcr");
> + if (IS_ERR(gcr_regmap))
> + return IS_ERR(gcr_regmap);
> + regmap_write(gcr_regmap, NPCM_I2CSEGCTL, NPCM_I2CSEGCTL_INIT_VAL);
> +
> + clk_regmap = syscon_regmap_lookup_by_compatible("nuvoton,npcm750-clk");
> + if (IS_ERR(clk_regmap))
> + return IS_ERR(clk_regmap);
> +
> + bus->reg = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(bus->reg))
> + return PTR_ERR((bus)->reg);
> +
> + spin_lock_init(&bus->lock);
> + init_completion(&bus->cmd_complete);
> +
> + adap = &bus->adap;
> + adap->owner = THIS_MODULE;
> + adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD | I2C_CLIENT_SLAVE;
Since you have a DT compatible, you won't need classes. Just drop them.
> + adap->retries = 3;
> + adap->timeout = HZ;
> + adap->algo = &npcm_i2c_algo;
> + adap->quirks = &npcm_i2c_quirks;
> + adap->algo_data = bus;
> + adap->dev.parent = &pdev->dev;
> + adap->dev.of_node = pdev->dev.of_node;
> + adap->nr = pdev->id;
> +
> + bus->irq = platform_get_irq(pdev, 0);
> + if (bus->irq < 0)
> + return bus->irq;
> +
> + ret = devm_request_irq(bus->dev, bus->irq, npcm_i2c_bus_irq, 0,
> + dev_name(bus->dev), bus);
> + if (ret)
> + return ret;
> +
> + ret = __npcm_i2c_init(bus, pdev);
> + if (ret)
> + return ret;
> +
> + ret = npcm_i2c_recovery_init(adap);
> + if (ret)
> + return ret;
> +
> + i2c_set_adapdata(adap, bus);
> +
> + snprintf(bus->adap.name, sizeof(bus->adap.name), "Nuvoton i2c");
Maybe you want to add something more specific in case you have multiple
instances of this driver at runtime.
> + ret = i2c_add_numbered_adapter(&bus->adap);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to add numbered adapter %d\n", ret);
The I2C core will print warnings for you.
> + return ret;
> + }
> + platform_set_drvdata(pdev, bus);
> +
> + i2c_init_debugfs(pdev, bus);
> + return 0;
> +}
> +
...
> +#ifdef CONFIG_DEBUG_FS
> +static int __init npcm_i2c_init(void)
> +{
> + struct dentry *dir;
> +
> + dir = debugfs_create_dir("i2c", NULL);
Okay, the GPIO fault injector could also need such a directory. I will
add this to the core. And then send an incremental patch for your
driver.
> + if (IS_ERR_OR_NULL(dir))
> + return 0;
> +
> + npcm_i2c_debugfs_dir = dir;
> + return 0;
> +}
> +
> +static void __exit npcm_i2c_exit(void)
> +{
> + debugfs_remove_recursive(npcm_i2c_debugfs_dir);
> +}
> +
> +module_init(npcm_i2c_init);
> +module_exit(npcm_i2c_exit);
> +#endif
> +
> +MODULE_AUTHOR("Avi Fishman <avi.fishman@gmail.com>");
> +MODULE_AUTHOR("Tali Perry <tali.perry@nuvoton.com>");
> +MODULE_AUTHOR("Tyrone Ting <kfting@nuvoton.com>");
> +MODULE_DESCRIPTION("Nuvoton I2C Bus Driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_VERSION("0.1.3");
> +
> --
> 2.22.0
>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
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 09/12] dt-bindings: arm: fsl: Add msi-map device-tree binding for fsl-mc bus
From: Robin Murphy @ 2020-05-22 14:34 UTC (permalink / raw)
To: Rob Herring, Diana Craciun OSS
Cc: devicetree, Marc Zyngier, PCI, Hanjun Guo, Rafael J. Wysocki,
Makarand Pawagi, linux-acpi, Linux IOMMU, Catalin Marinas,
Sudeep Holla, Bjorn Helgaas, Will Deacon,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <CAL_JsqKf+cq9Nhs+M8ihC-Ls24YH-WEofW8H4kkFPWMhZw=unA@mail.gmail.com>
On 2020-05-22 15:08, Rob Herring wrote:
> On Fri, May 22, 2020 at 3:57 AM Diana Craciun OSS
> <diana.craciun@oss.nxp.com> wrote:
>>
>> On 5/22/2020 12:42 PM, Robin Murphy wrote:
>>> On 2020-05-22 00:10, Rob Herring wrote:
>>>> On Thu, May 21, 2020 at 7:00 AM Lorenzo Pieralisi
>>>> <lorenzo.pieralisi@arm.com> wrote:
>>>>>
>>>>> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>>>>>
>>>>> The existing bindings cannot be used to specify the relationship
>>>>> between fsl-mc devices and GIC ITSes.
>>>>>
>>>>> Add a generic binding for mapping fsl-mc devices to GIC ITSes, using
>>>>> msi-map property.
>>>>>
>>>>> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>>>>> Cc: Rob Herring <robh+dt@kernel.org>
>>>>> ---
>>>>> .../devicetree/bindings/misc/fsl,qoriq-mc.txt | 30
>>>>> +++++++++++++++++--
>>>>> 1 file changed, 27 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
>>>>> b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
>>>>> index 9134e9bcca56..b0813b2d0493 100644
>>>>> --- a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
>>>>> +++ b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
>>>>> @@ -18,9 +18,9 @@ same hardware "isolation context" and a 10-bit
>>>>> value called an ICID
>>>>> the requester.
>>>>>
>>>>> The generic 'iommus' property is insufficient to describe the
>>>>> relationship
>>>>> -between ICIDs and IOMMUs, so an iommu-map property is used to define
>>>>> -the set of possible ICIDs under a root DPRC and how they map to
>>>>> -an IOMMU.
>>>>> +between ICIDs and IOMMUs, so the iommu-map and msi-map properties
>>>>> are used
>>>>> +to define the set of possible ICIDs under a root DPRC and how they
>>>>> map to
>>>>> +an IOMMU and a GIC ITS respectively.
>>>>>
>>>>> For generic IOMMU bindings, see
>>>>> Documentation/devicetree/bindings/iommu/iommu.txt.
>>>>> @@ -28,6 +28,9 @@ Documentation/devicetree/bindings/iommu/iommu.txt.
>>>>> For arm-smmu binding, see:
>>>>> Documentation/devicetree/bindings/iommu/arm,smmu.yaml.
>>>>>
>>>>> +For GICv3 and GIC ITS bindings, see:
>>>>> +Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml.
>>>>>
>>>>> +
>>>>> Required properties:
>>>>>
>>>>> - compatible
>>>>> @@ -119,6 +122,15 @@ Optional properties:
>>>>> associated with the listed IOMMU, with the iommu-specifier
>>>>> (i - icid-base + iommu-base).
>>>>>
>>>>> +- msi-map: Maps an ICID to a GIC ITS and associated iommu-specifier
>>>>> + data.
>>>>> +
>>>>> + The property is an arbitrary number of tuples of
>>>>> + (icid-base,iommu,iommu-base,length).
>>>>
>>>> I'm confused because the example has GIC ITS phandle, not an IOMMU.
>>>>
>>>> What is an iommu-base?
>>>
>>> Right, I was already halfway through writing a reply to say that all
>>> the copy-pasted "iommu" references here should be using the
>>> terminology from the pci-msi.txt binding instead.
>>
>> Right, will change it.
>>
>>>
>>>>> +
>>>>> + Any ICID in the interval [icid-base, icid-base + length) is
>>>>> + associated with the listed GIC ITS, with the iommu-specifier
>>>>> + (i - icid-base + iommu-base).
>>>>> Example:
>>>>>
>>>>> smmu: iommu@5000000 {
>>>>> @@ -128,6 +140,16 @@ Example:
>>>>> ...
>>>>> };
>>>>>
>>>>> + gic: interrupt-controller@6000000 {
>>>>> + compatible = "arm,gic-v3";
>>>>> + ...
>>>>> + its: gic-its@6020000 {
>>>>> + compatible = "arm,gic-v3-its";
>>>>> + msi-controller;
>>>>> + ...
>>>>> + };
>>>>> + };
>>>>> +
>>>>> fsl_mc: fsl-mc@80c000000 {
>>>>> compatible = "fsl,qoriq-mc";
>>>>> reg = <0x00000008 0x0c000000 0 0x40>, /* MC
>>>>> portal base */
>>>>> @@ -135,6 +157,8 @@ Example:
>>>>> msi-parent = <&its>;
>>>
>>> Side note: is it right to keep msi-parent here? It rather implies that
>>> the MC itself has a 'native' Device ID rather than an ICID, which I
>>> believe is not strictly true. Plus it's extra-confusing that it
>>> doesn't specify an ID either way, since that makes it look like the
>>> legacy PCI case that gets treated implicitly as an identity msi-map,
>>> which makes no sense at all to combine with an actual msi-map.
>>
>> Before adding msi-map, the fsl-mc code assumed that ICID and streamID
>> are equal and used msi-parent just to get the reference to the ITS node.
>> Removing msi-parent will break the backward compatibility of the already
>> existing systems. Maybe we should mention that this is legacy and not to
>> be used for newer device trees.
>
> If ids are 1:1, then the DT should use msi-parent. If there is
> remapping, then use msi-map. A given system should use one or the
> other. I suppose if some ids are 1:1 and the msi-map was added to add
> additional support for ids not 1:1, then you could end up with both.
> That's fine in dts files, but examples should reflect the 'right' way.
Is that defined anywhere? The generic MSI binding just has some weaselly
wording about buses:
"When #msi-cells is non-zero, busses with an msi-parent will require
additional properties to describe the relationship between devices on
the bus and the set of MSIs they can potentially generate."
which appears at odds with its own definition of msi-parent as including
an msi-specifier (or at best very unclear about what value that
specifier should take in this case).
The PCI MSI binding goes even further and specifically reserves
msi-parent for cases where there is no sideband data. As far as I'm
aware, the fact that the ITS driver implements a bodge for the "empty
msi-parent even though #msi-cells > 0" case is merely a compatibility
thing for old DTs from before this was really thought through, not an
officially-specified behaviour.
Robin.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [GIT PULL v2 11/11] arm64: tegra: Device tree changes for v5.8-rc1
From: Thierry Reding @ 2020-05-22 14:28 UTC (permalink / raw)
To: arm, soc; +Cc: linux-tegra, Thierry Reding, linux-arm-kernel, Jon Hunter
In-Reply-To: <20200515145311.1580134-12-thierry.reding@gmail.com>
Hi ARM SoC maintainers,
The following changes since commit 8f3d9f354286745c751374f5f1fcafee6b3f3136:
Linux 5.7-rc1 (2020-04-12 12:35:55 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git tags/tegra-for-5.8-arm64-dt-v2
for you to fetch changes up to 358a6777f5dd293bc3c075af6fc8c0dd18774c96:
arm64: tegra: Make the RTC a wakeup source on Jetson Nano and TX1 (2020-05-22 14:56:54 +0200)
This is a slightly rebased version of the prior pull request which has
the commit log cleaned up as I mentioned earlier. I've also included a
patch to enable resume from suspend via the PMIC on Jetson TX1 and
Jetson Nano, which will help us to test suspend/resume in our test
system in a unified way.
Thanks,
Thierry
----------------------------------------------------------------
arm64: tegra: Device tree changes for v5.8-rc1
This contains a couple of fixes for minor issues, enables XUDC support
on Tegra194, and enables EMC frequency scaling and video capture on
Tegra210.
----------------------------------------------------------------
Jon Hunter (3):
arm64: tegra: Fix ethernet phy-mode for Jetson Xavier
arm64: tegra: Allow the PMIC RTC to wakeup Jetson Xavier
arm64: tegra: Make the RTC a wakeup source on Jetson Nano and TX1
Joseph Lo (1):
arm64: tegra: Add external memory controller node for Tegra210
Nagarjuna Kristam (1):
arm64: tegra: Add XUDC node on Tegra194
Rob Herring (1):
arm64: tegra: Kill off "simple-panel" compatibles
Sowjanya Komatineni (4):
dt-bindings: clock: tegra: Add clock ID for CSI TPG clock
arm64: tegra: Fix SOR powergate clocks and reset
arm64: tegra: Add reset-cells to memory controller
arm64: tegra: Add Tegra VI CSI support in device tree
Thierry Reding (4):
Merge branch 'for-5.8/dt-bindings' into for-5.8/arm64/dt
arm64: tegra: Hook up EMC cooling device
arm64: tegra: Enable VI I2C on Jetson Nano
arm64: tegra: Make the RTC a wakeup source on Jetson TX2
Vidya Sagar (1):
arm64: tegra: Fix flag for 64-bit resources in 'ranges' property
arch/arm64/boot/dts/nvidia/tegra132-norrin.dts | 2 +-
arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi | 3 +-
arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi | 5 +-
arch/arm64/boot/dts/nvidia/tegra194.dtsi | 30 ++++++--
arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi | 3 +-
arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi | 10 +++
arch/arm64/boot/dts/nvidia/tegra210-p3450-0000.dts | 7 +-
arch/arm64/boot/dts/nvidia/tegra210.dtsi | 89 ++++++++++++++++++++--
include/dt-bindings/clock/tegra210-car.h | 2 +-
9 files changed, 131 insertions(+), 20 deletions(-)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [GIT PULL v2 08/11] soc/tegra: Changes for v5.8-rc1
From: Thierry Reding @ 2020-05-22 14:28 UTC (permalink / raw)
To: arm, soc; +Cc: linux-tegra, Thierry Reding, linux-arm-kernel, Jon Hunter
In-Reply-To: <20200515145311.1580134-9-thierry.reding@gmail.com>
Hi ARM SoC maintainers,
The following changes since commit 8f3d9f354286745c751374f5f1fcafee6b3f3136:
Linux 5.7-rc1 (2020-04-12 12:35:55 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git tags/tegra-for-5.8-soc-v2
for you to fetch changes up to df701a76a6419e66b566457a5b3fcdd314e48fd9:
soc/tegra: pmc: Enable PMIC wake event on Tegra210 (2020-05-22 14:55:54 +0200)
On top of the previous pull request this contains a missing patch to
enable resume from suspend by the PMIC, like we already do an Tegra186
and Tegra194.
Thanks,
Thierry
----------------------------------------------------------------
soc/tegra: Changes for v5.8-rc1
Enables Tegra210, Tegra186 and Tegra194 to be woken from suspend by the
PMIC and exports a bit more information about SoCs via sysfs.
----------------------------------------------------------------
Arnd Bergmann (1):
soc: tegra: Fix tegra_pmc_get_suspend_mode definition
Corentin Labbe (1):
soc/tegra: pmc: Select GENERIC_PINCONF
Jon Hunter (5):
soc/tegra: fuse: Add custom SoC attributes
soc/tegra: fuse: Trivial clean-up of tegra_init_revision()
soc/tegra: fuse: Update the SoC revision attribute to display a name
soc/tegra: pmc: Enable PMIC wake event on Tegra194
soc/tegra: pmc: Enable PMIC wake event on Tegra210
Thierry Reding (1):
soc/tegra: pmc: Enable PMIC wake event on Tegra186
drivers/soc/tegra/Kconfig | 1 +
drivers/soc/tegra/fuse/fuse-tegra.c | 57 +++++++++++++++++++++++++++++++++-
drivers/soc/tegra/fuse/fuse-tegra20.c | 1 +
drivers/soc/tegra/fuse/fuse-tegra30.c | 6 ++++
drivers/soc/tegra/fuse/fuse.h | 8 +++++
drivers/soc/tegra/fuse/tegra-apbmisc.c | 32 +++++++++++--------
drivers/soc/tegra/pmc.c | 3 ++
include/soc/tegra/pmc.h | 15 +++++----
8 files changed, 103 insertions(+), 20 deletions(-)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [GIT PULL v2 05/11] firmware: tegra: Changes for v5.8-rc1
From: Thierry Reding @ 2020-05-22 14:28 UTC (permalink / raw)
To: arm, soc; +Cc: linux-tegra, Thierry Reding, linux-arm-kernel, Jon Hunter
In-Reply-To: <20200515145311.1580134-6-thierry.reding@gmail.com>
Hi ARM SoC maintainers,
The following changes since commit 8f3d9f354286745c751374f5f1fcafee6b3f3136:
Linux 5.7-rc1 (2020-04-12 12:35:55 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git tags/tegra-for-5.8-firmware-v2
for you to fetch changes up to 10b2006681333a14a907a06a0e213c568fa8e7f4:
firmware: tegra: Defer BPMP probe if shared memory not available (2020-05-22 14:57:14 +0200)
On top of the previous pull request this contains an additional fix from
Jon for an issue that our test system discovered in a recent linux-next.
Thanks,
Thierry
----------------------------------------------------------------
firmware: tegra: Changes for v5.8-rc1
This contains a change that makes the BPMP driver a regular driver,
which fixes some weird suspend/resume ordering issues. Another fix is
also included to implement another way of enabling the L2 cache after
LP2 suspend.
----------------------------------------------------------------
Dmitry Osipenko (1):
firmware: tf: Different way of L2 cache enabling after LP2 suspend
Jon Hunter (1):
firmware: tegra: Defer BPMP probe if shared memory not available
Thierry Reding (1):
firmware: tegra: Make BPMP a regular driver
drivers/firmware/tegra/bpmp-tegra186.c | 4 ++--
drivers/firmware/tegra/bpmp.c | 9 +++------
drivers/firmware/trusted_foundations.c | 21 +++++++++++++++++++--
include/linux/firmware/trusted_foundations.h | 1 +
4 files changed, 25 insertions(+), 10 deletions(-)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH V4 4/4] platform: mtk-isp: Add Mediatek FD driver
From: Jerry-ch Chen @ 2020-05-22 14:10 UTC (permalink / raw)
To: Tomasz Figa
Cc: devicetree, Sean.Cheng, laurent.pinchart+renesas, Rynn.Wu,
zwisler, srv_heupstream, christie.yu, jerry-ch.chen, hverkuil,
jungo.lin, sj.huang, yuzhao, hans.verkuil, pihsun, frederic.chen,
matthias.bgg, linux-mediatek, mchehab, linux-arm-kernel,
linux-media
In-Reply-To: <20200521182825.GA249683@chromium.org>
Hi Tomasz,
On Thu, 2020-05-21 at 18:28 +0000, Tomasz Figa wrote:
> Hi Jerry,
>
> On Wed, Dec 04, 2019 at 08:47:32PM +0800, Jerry-ch Chen wrote:
> > From: Jerry-ch Chen <jerry-ch.chen@mediatek.com>
> >
> > This patch adds the driver of Face Detection (FD) unit in
> > Mediatek camera system, providing face detection function.
> >
> > The mtk-isp directory will contain drivers for multiple IP
> > blocks found in Mediatek ISP system. It will include ISP Pass 1
> > driver (CAM), sensor interface driver, DIP driver and face
> > detection driver.
> >
> > Signed-off-by: Jerry-ch Chen <jerry-ch.chen@mediatek.com>
> > ---
> > drivers/media/platform/Kconfig | 2 +
> > drivers/media/platform/Makefile | 2 +
> > drivers/media/platform/mtk-isp/fd/Kconfig | 19 +
> > drivers/media/platform/mtk-isp/fd/Makefile | 5 +
> > drivers/media/platform/mtk-isp/fd/mtk_fd.h | 149 ++
> > drivers/media/platform/mtk-isp/fd/mtk_fd_40.c | 1279 +++++++++++++++++
> > include/uapi/linux/v4l2-controls.h | 4 +
> > include/uapi/linux/videodev2.h | 3 +
> > 8 files changed, 1463 insertions(+)
> > create mode 100644 drivers/media/platform/mtk-isp/fd/Kconfig
> > create mode 100644 drivers/media/platform/mtk-isp/fd/Makefile
> > create mode 100644 drivers/media/platform/mtk-isp/fd/mtk_fd.h
> > create mode 100644 drivers/media/platform/mtk-isp/fd/mtk_fd_40.c
> >
>
> Thank you for the patch. Please see my comments inline. Really sorry for
> the much delayed review.
>
I appreciate your comments, here's the reply
> [snip]
> > +config VIDEO_MEDIATEK_FD
> > + tristate "Mediatek face detection processing function"
> > + depends on VIDEO_V4L2
> > + depends on ARCH_MEDIATEK
> > + select VIDEOBUF2_DMA_CONTIG
> > + select VIDEOBUF2_CORE
> > + select VIDEOBUF2_V4L2
> > + select VIDEOBUF2_MEMOPS
> > + select MEDIA_CONTROLLER
> > + select MTK_SCP
> > +
> > + default n
> > + help
> > + Support the Face Detection (FD) feature in the Mediatek
> > + mt8183 Soc.
>
> MT8183?
Fixed.
>
> > +
> > + FD driver is a V4L2 memory-to-memory device driver which
> > + provides hardware accelerated face detection function,
> > + it can detect different sizes of faces in a raw image.
>
> A YUV image I guess?
Fixed
>
> [snip]
> > diff --git a/drivers/media/platform/mtk-isp/fd/mtk_fd.h b/drivers/media/platform/mtk-isp/fd/mtk_fd.h
> > new file mode 100644
> > index 000000000000..d85bdcb70d6d
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-isp/fd/mtk_fd.h
> > @@ -0,0 +1,149 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +//
> > +// Copyright (c) 2018 MediaTek Inc.
> > +
> > +#ifndef __MTK_FD_HW_H__
> > +#define __MTK_FD_HW_H__
> > +
> > +#include <linux/completion.h>
> > +#include <linux/io.h>
> > +#include <linux/types.h>
> > +#include <linux/platform_device.h>
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-device.h>
> > +#include <media/videobuf2-v4l2.h>
> > +
> > +#define MTK_FD_OUTPUT_MIN_WIDTH 26U
> > +#define MTK_FD_OUTPUT_MIN_HEIGHT 26U
> > +#define MTK_FD_OUTPUT_MAX_WIDTH 640U
> > +#define MTK_FD_OUTPUT_MAX_HEIGHT 480U
> > +
> > +#define MTK_FD_HW_FMT_VYUY 2
> > +#define MTK_FD_HW_FMT_UYVY 3
> > +#define MTK_FD_HW_FMT_YVYU 4
> > +#define MTK_FD_HW_FMT_YUYV 5
> > +#define MTK_FD_HW_FMT_YVU_2P 6
> > +#define MTK_FD_HW_FMT_YUV_2P 7
> > +#define MTK_FD_HW_FMT_UNKNOWN 8
>
> What is an unknown format?
It's used in translate v4l2 pixel format to the value that FD HW
recognized. But I think the case of UNKNWOWN shouldn't happened.
I will remove it
>
> > +
> > +#define MTK_FD_IPI_CMD_INIT 0
> > +#define MTK_FD_IPI_CMD_INIT_ACK 1
> > +#define MTK_FD_IPI_CMD_ENQUEUE 2
> > +#define MTK_FD_IPI_CMD_ENQ_ACK 3
> > +#define MTK_FD_IPI_CMD_EXIT 4
> > +#define MTK_FD_IPI_CMD_EXIT_ACK 5
> > +#define MTK_FD_IPI_CMD_RESET 6
> > +#define MTK_FD_IPI_CMD_RESET_ACK 7
> > +
> > +#define MTK_FD_REG_OFFSET_HW_ENABLE 0x4
> > +#define MTK_FD_REG_OFFSET_INT_EN 0x15c
> > +#define MTK_FD_REG_OFFSET_INT_VAL 0x168
> > +#define MTK_FD_REG_OFFSET_RESULT 0x178
> > +
> > +#define MTK_FD_SET_HW_ENABLE 0x111
> > +#define MTK_FD_RS_BUF_SIZE 2289664
> > +#define MTK_FD_HW_WORK_BUF_SIZE 0x100000
>
> How about using the SZ_1M?
>
yes, fixed.
> > +#define MTK_FD_MAX_SPEEDUP 7
> > +#define MTK_FD_MAX_RESULT_NUM 1026
>
> Perhaps NUM_RESULTS?
>
fixed as MTK_FD_MAX_NUM_RESULT.
> > +
> > +/* Max scale size counts */
> > +#define MTK_FD_SCALE_ARR_NUM 15
>
> Perhaps NUM_SCALE_SIZES?
>
fixed as MTK_FD_NUM_SCALE_SIZE
> > +
> > +#define MTK_FD_HW_TIMEOUT 1000
>
> What's the unit?
it's in milliseconds, I would like to rename it to
MTK_FD_HW_TIMEOUT_MSEC
>
> > +
> > +enum face_angle {
> > + MTK_FD_FACE_FRONT,
> > + MTK_FD_FACE_RIGHT_50,
> > + MTK_FD_FACE_LEFT_50,
> > + MTK_FD_FACE_RIGHT_90,
> > + MTK_FD_FACE_LEFT_90,
> > + MTK_FD_FACE_ANGLE_NUM,
> > +};
>
> This enum seems to define values for the V4L2_CID_MTK_FD_DETECT_POSE
> control. Considering that this is an enumeration and the values are
> actually integers (-90, -50, 0, 50, 90), perhaps this should be an
> INTEGER_MENU control instead?
>
this ioctl let user select multiple face positions(combination of angles
and directions) to be detected. so I thought I am not able to use the
INTEGER_MENU for this purpose.
A bit-field as following should be used by user.
I consider adding it to uapi.
struct face_direction_def {
__u16 MTK_FD_FACE_DIR_0 : 1,
MTK_FD_FACE_DIR_30 : 1,
MTK_FD_FACE_DIR_60 : 1,
MTK_FD_FACE_DIR_90 : 1,
MTK_FD_FACE_DIR_120 : 1,
MTK_FD_FACE_DIR_150 : 1,
MTK_FD_FACE_DIR_180 : 1,
MTK_FD_FACE_DIR_210 : 1,
MTK_FD_FACE_DIR_240 : 1,
MTK_FD_FACE_DIR_270 : 1,
MTK_FD_FACE_DIR_300 : 1,
MTK_FD_FACE_DIR_330 : 1,
: 4;
};
User can also select some face directions of each face angle in one
ioctl, for example:
/*
* u16 face_directions[MTK_FD_FACE_ANGLE_NUM] = {0};
*
* face_directions[MTK_FD_FACE_FRONT] = 0x7; //angle:0, dir:0,30,60
* face_directions[MTK_FACE_RIGHT_50] = 0x2; //angle:50, dir:30
*
*/
> > +
> > +struct fd_buffer {
> > + __u32 scp_addr; /* used by SCP */
> > + __u32 dma_addr; /* used by DMA HW */
> > +} __packed;
fd buffer is used for scp ipi
> > +
> > +struct fd_face_result {
> > + char data[16];
> > +};
fd_face_result is used for user, so it should be moved to
include/uapi/linux.
In fact, it has bit-field definition for user, so I would like to define
it in include/uapi/linux as following:
struct fd_face_result {
__u64 face_idx : 12,
type : 1,
x0 : 10,
y0 : 10,
x1 : 10,
y1 : 10,
fcv1 : 11;
__u64 fcv2 : 7,
rip_dir : 4,
rop_dir : 3,
det_size : 5;
};
> > +
> > +struct fd_user_output {
> > + struct fd_face_result results[MTK_FD_MAX_RESULT_NUM];
> > + __u16 number;
>
> Is this perhaps the number of results? If so, would num_results be a better
> name?
>
yes, fixed.
> > +};
>
> Since this struct is the meta buffer format, it is a part of the userspace
> interface and should be defined in a header under include/uapi/linux/.
>
Ok, I will create include/uapi/linux/mtk_fd_40.h
which suppose to include structures that userspace will use.
should the private IOCTLs be placed in it together?
> > +
> > +struct user_param {
> > + u8 fd_speedup;
> > + u8 fd_extra_model;
> > + u8 scale_img_num;
> > + u8 src_img_fmt;
> > + __u16 scale_img_width[MTK_FD_SCALE_ARR_NUM];
> > + __u16 scale_img_height[MTK_FD_SCALE_ARR_NUM];
> > + __u16 face_directions[MTK_FD_FACE_ANGLE_NUM];
>
> Is this a user-facing definition or an interface between the kernel driver
> and firmware? If the latter, the __ types shouldn't be used.
>
It's the later, I'll fix it.
> > +} __packed;
> > +
> > +struct fd_init_param {
> > + struct fd_buffer fd_manager;
> > + __u32 rs_dma_addr;
>
> Ditto.
>
done.
> > +} __packed;
> > +
> > +struct fd_enq_param {
> > + __u64 output_vaddr;
>
> Ditto.
done.
>
> > + struct fd_buffer src_img[2];
> > + struct fd_buffer user_result;
> > + struct user_param user_param;
> > +} __packed;
> > +
> > +struct fd_ack_param {
> > + __u32 ret_code;
> > + __u32 ret_msg;
>
> Ditto.
done.
>
> > +} __packed;
> [snip]
> > +/* */
>
> Was there supposed to be a comment here? :)
Ooops, no, it should be removed.
>
> > +static int mtk_fd_hw_alloc_rs_dma_addr(struct mtk_fd_dev *fd)
> > +{
> > + struct device *dev = fd->dev;
> > + void *va;
> > + dma_addr_t dma_handle;
> > +
> > + va = dma_alloc_coherent(dev, MTK_FD_RS_BUF_SIZE, &dma_handle,
> > + GFP_KERNEL);
> > + if (!va) {
> > + dev_err(dev, "dma_alloc null va\n");
>
> No need to print errors for memory allocation failures, because one will be
> printed automatically.
Ok, log removed.
>
> [snip]
> > +static int mtk_fd_hw_connect(struct mtk_fd_dev *fd)
> > +{
> > + int ret;
> > +
> > + ret = rproc_boot(fd->rproc_handle);
> > +
>
> nit: Unnecessary blank line.
>
fixed.
> [snip]
> > +static int mtk_fd_vb2_queue_setup(struct vb2_queue *vq,
> > + unsigned int *num_buffers,
> > + unsigned int *num_planes,
> > + unsigned int sizes[],
> > + struct device *alloc_devs[])
> > +{
> > + struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > + unsigned int size[2];
> > + unsigned int plane;
> > +
> > + switch (vq->type) {
> > + case V4L2_BUF_TYPE_META_CAPTURE:
> > + size[0] = ctx->dst_fmt.buffersize;
> > + break;
> > + case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> > + size[0] = ctx->src_fmt.plane_fmt[0].sizeimage;
> > + if (*num_planes == 2)
> > + size[1] = ctx->src_fmt.plane_fmt[1].sizeimage;
> > + break;
> > + }
>
> Is this code above needed? The code below sets sizes[] and it uses a for loop,
> without opencoded assignment for the second plane.
>
Looks like not really useful here,
it should check sizes and num_planes if num_plane not zero,
and for V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, it will at most have 2
planes, maybe no need for loop as well.
I will refine this function as following:
mtk_fd_vb2_queue_setup(...)
{
struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
if (*num_planes == 0) {
if (vq->type == V4L2_BUF_TYPE_META_CAPTURE) {
sizes[0] = ctx->dst_fmt.buffersize;
*num_planes = 1;
return 0;
} else if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
*num_planes = ctx->src_fmt.num_planes;
sizes[0] = ctx->src_fmt.plane_fmt[0].sizeimage;
if (*num_planes == 2)
sizes[1] = ctx->src_fmt.plane_fmt[1].sizeimage;
return 0;
}
return -EINVAL;
}
/* If num_plane not zero, check the num_plane and sizes*/
if (vq->type == V4L2_BUF_TYPE_META_CAPTURE) {
if ((*num_planes == 1) &&
(sizes[0] <= ctx->dst_fmt.buffersize))
return 0;
else
return -EINVAL;
}
if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
if ((*num_planes == 1) &&
(sizes[0] <= ctx->src_fmt.plane_fmt[0].sizeimage))
return 0;
else if ((*num_planes == 2) &&
(sizes[0] <= ctx->src_fmt.plane_fmt[0].sizeimage) &&
(sizes[1] <= ctx->src_fmt.plane_fmt[1].sizeimage))
return 0;
else
return -EINVAL;
}
return 0;
}
> > +
> > + if (*num_planes > 2)
> > + return -EINVAL;
> > + if (*num_planes == 0) {
> > + if (vq->type == V4L2_BUF_TYPE_META_CAPTURE) {
> > + sizes[0] = ctx->dst_fmt.buffersize;
> > + *num_planes = 1;
> > + return 0;
> > + }
> > +
> > + *num_planes = ctx->src_fmt.num_planes;
> > + for (plane = 0; plane < *num_planes; plane++)
> > + sizes[plane] = ctx->src_fmt.plane_fmt[plane].sizeimage;
> > + return 0;
> > + }
> > +
> > + for (plane = 0; plane < *num_planes; plane++) {
> > + if (sizes[plane] < size[plane])
> > + return -EINVAL;
> > + }
> > + return 0;
> > +}
> [snip]
> > +static int mtk_fd_enum_fmt_out_mp(struct file *file, void *fh,
> > + struct v4l2_fmtdesc *f)
> > +{
> > + if (f->index >= NUM_FORMATS)
>
> Please use ARRAY_SIZE().
fixed.
>
> > + return -EINVAL;
> > +
> > + f->pixelformat = mtk_fd_img_fmts[f->index].pixelformat;
> > + return 0;
> > +}
> > +
> > +static void mtk_fd_fill_pixfmt_mp(struct v4l2_pix_format_mplane *dfmt,
> > + const struct v4l2_pix_format_mplane *sfmt)
> > +{
> > + dfmt->field = V4L2_FIELD_NONE;
> > + dfmt->colorspace = V4L2_COLORSPACE_BT2020;
> > + dfmt->num_planes = sfmt->num_planes;
> > + dfmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> > + dfmt->quantization = V4L2_QUANTIZATION_DEFAULT;
> > + dfmt->xfer_func =
> > + V4L2_MAP_XFER_FUNC_DEFAULT(dfmt->colorspace);
> > +
> > + /* Keep user setting as possible */
> > + dfmt->width = clamp(dfmt->width,
> > + MTK_FD_OUTPUT_MIN_WIDTH,
> > + MTK_FD_OUTPUT_MAX_WIDTH);
> > + dfmt->height = clamp(dfmt->height,
> > + MTK_FD_OUTPUT_MIN_HEIGHT,
> > + MTK_FD_OUTPUT_MAX_HEIGHT);
> > +
> > + if (sfmt->num_planes == 2) {
> > + /* NV16M and NV61M has 1 byte per pixel */
> > + dfmt->plane_fmt[0].bytesperline = dfmt->width;
> > + dfmt->plane_fmt[1].bytesperline = dfmt->width;
> > + } else {
> > + /* 2 bytes per pixel */
> > + dfmt->plane_fmt[0].bytesperline = dfmt->width * 2;
> > + }
> > +
> > + dfmt->plane_fmt[0].sizeimage =
> > + dfmt->height * dfmt->plane_fmt[0].bytesperline;
>
> Could some of the code above be replaced with v4l2_fill_pixfmt_mp()?
>
I would like to refine as following
mtk_fd_fill_pixfmt_mp(...){
v4l2_fill_pixfmt_mp(dfmt, sfmt->pixelformat, dfmt->width,
dfmt->height);
dfmt->field = V4L2_FIELD_NONE;
dfmt->colorspace = V4L2_COLORSPACE_BT2020;
dfmt->num_planes = sfmt->num_planes;
dfmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
dfmt->quantization = V4L2_QUANTIZATION_DEFAULT;
dfmt->xfer_func =
V4L2_MAP_XFER_FUNC_DEFAULT(dfmt->colorspace);
}
> > +}
> > +
> > +static const struct v4l2_pix_format_mplane *mtk_fd_find_fmt(u32 format)
> > +{
> > + unsigned int i;
> > + const struct v4l2_pix_format_mplane *dev_fmt;
> > +
> > + for (i = 0; i < NUM_FORMATS; i++) {
>
> Please use ARRAY_SIZE rather than a custom macro.
fixed.
>
> > + dev_fmt = &mtk_fd_img_fmts[i];
> > + if (dev_fmt->pixelformat == format)
> > + return dev_fmt;
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > +static int mtk_fd_try_fmt_out_mp(struct file *file,
> > + void *fh,
> > + struct v4l2_format *f)
> > +{
> > + struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> > + const struct v4l2_pix_format_mplane *fmt;
> > +
> > + fmt = mtk_fd_find_fmt(pix_mp->pixelformat);
> > + if (!fmt)
> > + fmt = &mtk_fd_img_fmts[0]; /* Get default img fmt */
>
> nit: Please move the comment to a separate line and add braces.
>
fixed.
> > +
> > + mtk_fd_fill_pixfmt_mp(pix_mp, fmt);
> > + return 0;
> > +}
> [snip]
> > +static unsigned int get_fd_img_fmt(unsigned int fourcc)
> > +{
> > + switch (fourcc) {
> > + case V4L2_PIX_FMT_VYUY:
> > + return MTK_FD_HW_FMT_VYUY;
> > + case V4L2_PIX_FMT_YUYV:
> > + return MTK_FD_HW_FMT_YUYV;
> > + case V4L2_PIX_FMT_YVYU:
> > + return MTK_FD_HW_FMT_YVYU;
> > + case V4L2_PIX_FMT_UYVY:
> > + return MTK_FD_HW_FMT_UYVY;
> > + case V4L2_PIX_FMT_NV16M:
> > + return MTK_FD_HW_FMT_YUV_2P;
> > + case V4L2_PIX_FMT_NV61M:
> > + return MTK_FD_HW_FMT_YVU_2P;
> > + default:
> > + return MTK_FD_HW_FMT_UNKNOWN;
>
> If we want to be paranoid, we can just add a WARN here and return 0.
> Shouldn't be a need to define an unknown format.
Ok, I will refine as following:
default:
dev_warn(dev, "%s: Unsupported V4L2_PIX_FMT\n", __func__);
return 0;
>
> [snip]
> > +static void mtk_fd_fill_user_param(struct user_param *user_param,
> > + struct v4l2_ctrl_handler *hdl)
> > +{
> > + struct v4l2_ctrl *ctrl;
> > + int i;
> > +
> > + ctrl = v4l2_ctrl_find(hdl, V4L2_CID_MTK_FD_SCALE_DOWN_IMG_WIDTH);
> > + if (ctrl)
> > + for (i = 0; i < ctrl->elems; i++)
> > + user_param->scale_img_width[i] = ctrl->p_new.p_u16[i];
> > + ctrl = v4l2_ctrl_find(hdl, V4L2_CID_MTK_FD_SCALE_DOWN_IMG_HEIGHT);
> > + if (ctrl)
> > + for (i = 0; i < ctrl->elems; i++)
> > + user_param->scale_img_height[i] = ctrl->p_new.p_u16[i];
> > + ctrl = v4l2_ctrl_find(hdl, V4L2_CID_MTK_FD_SCALE_IMG_NUM);
> > + if (ctrl)
> > + user_param->scale_img_num = ctrl->val;
> > +
>
> nit: Either separate the code dealing with all controls from each other, or
> none.
>
Fixed.
> > + ctrl = v4l2_ctrl_find(hdl, V4L2_CID_MTK_FD_DETECT_POSE);
> > + if (ctrl)
> > + for (i = 0; i < ctrl->elems; i++)
> > + user_param->face_directions[i] = ctrl->p_new.p_u16[i];
> > + ctrl = v4l2_ctrl_find(hdl, V4L2_CID_MTK_FD_DETECT_SPEED);
> > + if (ctrl)
> > + user_param->fd_speedup = ctrl->val;
> > + ctrl = v4l2_ctrl_find(hdl, V4L2_CID_MTK_FD_DETECTION_MODEL);
> > + if (ctrl)
> > + user_param->fd_extra_model = ctrl->val;
> > +}
> > +
> > +static void mtk_fd_device_run(void *priv)
> > +{
> > + struct mtk_fd_ctx *ctx = priv;
> > + struct mtk_fd_dev *fd = ctx->fd_dev;
> > + struct vb2_v4l2_buffer *src_buf, *dst_buf;
> > + struct fd_enq_param fd_param;
> > + void *plane_vaddr;
> > +
> > + src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> > + dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> > +
> > + fd_param.src_img[0].dma_addr =
> > + vb2_dma_contig_plane_dma_addr(&src_buf->vb2_buf, 0);
> > + fd_param.user_result.dma_addr =
> > + vb2_dma_contig_plane_dma_addr(&dst_buf->vb2_buf, 0);
> > + plane_vaddr = vb2_plane_vaddr(&dst_buf->vb2_buf, 0);
> > + fd_param.output_vaddr = (u64)(unsigned long)plane_vaddr;
>
> Why is the vaddr needed here? Specfically, it could pose a security problem
> if CPU virtual addresses are exchanged with firmware.
>
This should be removed... SCP side as well.
> > + fd_param.user_param.src_img_fmt =
> > + get_fd_img_fmt(ctx->src_fmt.pixelformat);
> > + if (ctx->src_fmt.num_planes == 2)
> > + fd_param.src_img[1].dma_addr =
> > + vb2_dma_contig_plane_dma_addr(&src_buf->vb2_buf, 1);
>
> nit: Could this be moved above, to be just below src_img[0] initialization,
> for readability reasons?
>
Ok, this function will be refined as
static void mtk_fd_device_run(void *priv)
{
struct mtk_fd_ctx *ctx = priv;
struct mtk_fd_dev *fd = ctx->fd_dev;
struct vb2_v4l2_buffer *src_buf, *dst_buf;
struct fd_enq_param fd_param;
src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
fd_param.src_img[0].dma_addr =
vb2_dma_contig_plane_dma_addr(&src_buf->vb2_buf, 0);
if (ctx->src_fmt.num_planes == 2)
fd_param.src_img[1].dma_addr =
vb2_dma_contig_plane_dma_addr(&src_buf->vb2_buf, 1);
fd_param.user_result.dma_addr =
vb2_dma_contig_plane_dma_addr(&dst_buf->vb2_buf, 0);
fd_param.user_param.src_img_fmt =
get_fd_img_fmt(fd->dev, ctx->src_fmt.pixelformat);
mtk_fd_fill_user_param(&fd_param.user_param, &ctx->hdl);
/* Complete request controls if any */
v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req, &ctx->hdl);
fd->output = vb2_plane_vaddr(&dst_buf->vb2_buf, 0);
mtk_fd_hw_job_exec(fd, &fd_param);
}
> > + mtk_fd_fill_user_param(&fd_param.user_param, &ctx->hdl);
> > +
> > + /* Complete request controls if any */
> > + v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req, &ctx->hdl);
> > +
> > + fd->output = plane_vaddr;
> > + mtk_fd_hw_job_exec(fd, &fd_param);
> > +}
> > +
> > +static struct v4l2_m2m_ops fd_m2m_ops = {
> > + .device_run = mtk_fd_device_run,
> > +};
> > +
> > +static const struct media_device_ops fd_m2m_media_ops = {
> > + .req_validate = vb2_request_validate,
> > + .req_queue = v4l2_m2m_request_queue,
> > +};
> > +
> > +static int mtk_fd_video_device_register(struct mtk_fd_dev *fd)
> > +{
> > + struct video_device *vfd = &fd->vfd;
> > + struct v4l2_m2m_dev *m2m_dev = fd->m2m_dev;
> > + struct device *dev = fd->dev;
> > + int ret;
> > +
> > + vfd->fops = &fd_video_fops;
> > + vfd->release = video_device_release;
> > + vfd->lock = &fd->vfd_lock;
> > + vfd->v4l2_dev = &fd->v4l2_dev;
> > + vfd->vfl_dir = VFL_DIR_M2M;
> > + vfd->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_OUTPUT_MPLANE |
> > + V4L2_CAP_META_CAPTURE;
> > + vfd->ioctl_ops = &mtk_fd_v4l2_video_out_ioctl_ops;
> > +
> > + strscpy(vfd->name, dev_driver_string(dev), sizeof(vfd->name));
> > +
> > + video_set_drvdata(vfd, fd);
> > +
> > + ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
> > + if (ret) {
> > + dev_err(dev, "Failed to register video device\n");
> > + goto err_free_dev;
> > + }
> > +
> > + ret = v4l2_m2m_register_media_controller(m2m_dev, vfd,
> > + MEDIA_ENT_F_PROC_VIDEO_STATISTICS);
> > + if (ret) {
> > + dev_err(dev, "Failed to init mem2mem media controller\n");
> > + goto err_unreg_video;
> > + }
> > + return 0;
> > +
> > +err_unreg_video:
> > + video_unregister_device(vfd);
> > +err_free_dev:
> > + video_device_release(vfd);
> > + return ret;
> > +}
> > +
> > +static int mtk_fd_dev_v4l2_init(struct mtk_fd_dev *fd)
> > +{
> > + struct media_device *mdev = &fd->mdev;
> > + struct device *dev = fd->dev;
> > + int ret;
> > +
> > + ret = v4l2_device_register(dev, &fd->v4l2_dev);
> > + if (ret) {
> > + dev_err(dev, "Failed to register v4l2 device\n");
> > + return ret;
> > + }
> > +
> > + fd->m2m_dev = v4l2_m2m_init(&fd_m2m_ops);
> > + if (IS_ERR(fd->m2m_dev)) {
> > + dev_err(dev, "Failed to init mem2mem device\n");
> > + ret = PTR_ERR(fd->m2m_dev);
> > + goto err_unreg_v4l2_dev;
> > + }
> > +
> > + mdev->dev = dev;
> > + strscpy(mdev->model, dev_driver_string(dev), sizeof(mdev->model));
> > + snprintf(mdev->bus_info, sizeof(mdev->bus_info),
> > + "platform:%s", dev_name(dev));
> > + media_device_init(mdev);
> > + mdev->ops = &fd_m2m_media_ops;
> > + fd->v4l2_dev.mdev = mdev;
> > +
> > + ret = mtk_fd_video_device_register(fd);
> > + if (ret) {
> > + dev_err(dev, "Failed to register video device\n");
> > + goto err_cleanup_mdev;
> > + }
> > +
> > + ret = media_device_register(mdev);
> > + if (ret) {
> > + dev_err(dev, "Failed to register mem2mem media device\n");
> > + goto err_unreg_vdev;
> > + }
> > +
> > + return 0;
> > +
> > +err_unreg_vdev:
> > + v4l2_m2m_unregister_media_controller(fd->m2m_dev);
> > + video_unregister_device(&fd->vfd);
> > + video_device_release(&fd->vfd);
>
> This is inconsistent with the registration, which registers all of these in
> a function. Perhaps it would be worth to move this cleanup to a
> mtk_fd_video_device_unregister() function?
>
Fixed.
> > +err_cleanup_mdev:
> > + media_device_cleanup(mdev);
> > + v4l2_m2m_release(fd->m2m_dev);
> > +err_unreg_v4l2_dev:
> > + v4l2_device_unregister(&fd->v4l2_dev);
> > + return ret;
> > +}
> > +
> > +static void mtk_fd_dev_v4l2_release(struct mtk_fd_dev *fd)
> > +{
> > + v4l2_m2m_unregister_media_controller(fd->m2m_dev);
> > + video_unregister_device(&fd->vfd);
> > + video_device_release(&fd->vfd);
> > + media_device_cleanup(&fd->mdev);
> > + v4l2_m2m_release(fd->m2m_dev);
> > + v4l2_device_unregister(&fd->v4l2_dev);
> > +}
> > +
> > +static irqreturn_t mtk_fd_irq(int irq, void *data)
> > +{
> > + struct mtk_fd_dev *fd = (struct mtk_fd_dev *)data;
> > +
> > + /* must read this register otherwise HW will keep sending irq */
> > + readl(fd->fd_base + MTK_FD_REG_OFFSET_INT_VAL);
> > + fd->output->number = readl(fd->fd_base + MTK_FD_REG_OFFSET_RESULT);
> > + dev_dbg(fd->dev, "mtk_fd_face_num:%d\n", fd->output->number);
> > +
> > + mtk_fd_hw_done(fd, VB2_BUF_STATE_DONE);
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int mtk_fd_hw_get_scp_mem(struct mtk_fd_dev *fd)
> > +{
> > + struct device *dev = fd->dev;
> > + dma_addr_t addr;
> > + void *ptr;
> > + u32 ret;
> > +
> > + /*
> > + * Allocate coherent reserved memory for SCP firmware usage.
> > + * The size of SCP composer's memory is fixed to 0x100000
> > + * for the requirement of firmware.
> > + */
> > + ptr = dma_alloc_coherent(&fd->scp_pdev->dev,
> > + MTK_FD_HW_WORK_BUF_SIZE, &addr, GFP_KERNEL);
> > + if (!ptr)
> > + return -ENOMEM;
> > +
> > + fd->scp_mem.scp_addr = addr;
> > + fd->scp_mem_virt_addr = ptr;
> > + dev_info(dev, "scp addr:%pad va:%pK\n", &addr, ptr);
>
> These addresses are nothing a user should be concerned about, so please
> don't use the _info level here. If you think this is an important debugging
> information, please make it dev_dbg(). Otherwise, please just remove.
>
Removed.
> > +
> > + /*
> > + * This reserved memory is also be used by FD HW.
> > + * Need to get iova address for FD DMA.
> > + */
> > + addr = dma_map_resource(dev, addr, MTK_FD_HW_WORK_BUF_SIZE,
> > + DMA_TO_DEVICE, DMA_ATTR_SKIP_CPU_SYNC);
>
> Note that the second argument to dma_map_resource() is phys_addr_t, but the
> code above passes a DMA address. This works only by luck, because both
> physical and SCP DMA address space have the same addresses. To be fully
> correct, dma_map_single() needs to be used, with the kernel virtual address
> passed to it, but currently it doesn't handle the memory from a reserved
> pool. Thus, we can only keep the hack as is, but please add a comment
> explaining it, e.g.
>
> /*
> * FIXME: Assume SCP DMA and physical addresses are the same until
> * dma_map_single() is fixed to handle reserved memory allocations.
> */
>
Ok, I've insert it above the comments there.
as following:
/*
* FIXME: Assume SCP DMA and physical addresses are the same until
* dma_map_single() is fixed to handle reserved memory allocations.
*
* This reserved memory will also be used by FD HW.
* Need to get iova address for FD DMA.
*/
> > + if (dma_mapping_error(dev, addr)) {
> > + dev_err(dev, "Failed to map scp iova\n");
> > + ret = -ENOMEM;
> > + goto fail_free_mem;
> > + }
> > + fd->scp_mem.dma_addr = addr;
> > + dev_info(dev, "scp iova addr:%pad\n", &addr);
>
> Ditto.
fixed.
>
> Best regards,
> Tomasz
Thanks and best regards,
Jerry
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH v12 07/11] psci: Add hypercall service for kvm ptp.
From: Steven Price @ 2020-05-22 14:18 UTC (permalink / raw)
To: Jianyong Wu, netdev, yangbo.lu, john.stultz, tglx, pbonzini,
sean.j.christopherson, maz, richardcochran, Mark.Rutland, will,
suzuki.poulose
Cc: justin.he, Wei.Chen, kvm, Steve.Capper, linux-kernel, Kaly.Xin,
nd, kvmarm, linux-arm-kernel
In-Reply-To: <20200522083724.38182-8-jianyong.wu@arm.com>
On 22/05/2020 09:37, Jianyong Wu wrote:
> ptp_kvm modules will get this service through smccc call.
> The service offers real time and counter cycle of host for guest.
> Also let caller determine which cycle of virtual counter or physical counter
> to return.
>
> Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
> ---
> include/linux/arm-smccc.h | 14 ++++++++++++
> virt/kvm/Kconfig | 4 ++++
> virt/kvm/arm/hypercalls.c | 47 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 65 insertions(+)
>
> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> index bdc0124a064a..badadc390809 100644
> --- a/include/linux/arm-smccc.h
> +++ b/include/linux/arm-smccc.h
> @@ -94,6 +94,8 @@
>
> /* KVM "vendor specific" services */
> #define ARM_SMCCC_KVM_FUNC_FEATURES 0
> +#define ARM_SMCCC_KVM_FUNC_KVM_PTP 1
> +#define ARM_SMCCC_KVM_FUNC_KVM_PTP_PHY 2
> #define ARM_SMCCC_KVM_FUNC_FEATURES_2 127
> #define ARM_SMCCC_KVM_NUM_FUNCS 128
>
> @@ -103,6 +105,18 @@
> ARM_SMCCC_OWNER_VENDOR_HYP, \
> ARM_SMCCC_KVM_FUNC_FEATURES)
>
> +#define ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID \
> + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
> + ARM_SMCCC_SMC_32, \
> + ARM_SMCCC_OWNER_VENDOR_HYP, \
> + ARM_SMCCC_KVM_FUNC_KVM_PTP)
> +
> +#define ARM_SMCCC_VENDOR_HYP_KVM_PTP_PHY_FUNC_ID \
> + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
> + ARM_SMCCC_SMC_32, \
> + ARM_SMCCC_OWNER_VENDOR_HYP, \
> + ARM_SMCCC_KVM_FUNC_KVM_PTP_PHY)
> +
> #ifndef __ASSEMBLY__
>
> #include <linux/linkage.h>
> diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
> index aad9284c043a..bf820811e815 100644
> --- a/virt/kvm/Kconfig
> +++ b/virt/kvm/Kconfig
> @@ -60,3 +60,7 @@ config HAVE_KVM_VCPU_RUN_PID_CHANGE
>
> config HAVE_KVM_NO_POLL
> bool
> +
> +config ARM64_KVM_PTP_HOST
> + def_bool y
> + depends on ARM64 && KVM
> diff --git a/virt/kvm/arm/hypercalls.c b/virt/kvm/arm/hypercalls.c
> index db6dce3d0e23..c964122f8dae 100644
> --- a/virt/kvm/arm/hypercalls.c
> +++ b/virt/kvm/arm/hypercalls.c
> @@ -3,6 +3,7 @@
>
> #include <linux/arm-smccc.h>
> #include <linux/kvm_host.h>
> +#include <linux/clocksource_ids.h>
>
> #include <asm/kvm_emulate.h>
>
> @@ -11,6 +12,10 @@
>
> int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
> {
> +#ifdef CONFIG_ARM64_KVM_PTP_HOST
> + struct system_time_snapshot systime_snapshot;
> + u64 cycles;
> +#endif
> u32 func_id = smccc_get_function(vcpu);
> u32 val[4] = {SMCCC_RET_NOT_SUPPORTED};
> u32 feature;
> @@ -70,7 +75,49 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
> break;
> case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
> val[0] = BIT(ARM_SMCCC_KVM_FUNC_FEATURES);
> +
> +#ifdef CONFIG_ARM64_KVM_PTP_HOST
> + val[0] |= BIT(ARM_SMCCC_KVM_FUNC_KVM_PTP);
> +#endif
> break;
> +
> +#ifdef CONFIG_ARM64_KVM_PTP_HOST
> + /*
> + * This serves virtual kvm_ptp.
> + * Four values will be passed back.
> + * reg0 stores high 32-bit host ktime;
> + * reg1 stores low 32-bit host ktime;
> + * reg2 stores high 32-bit difference of host cycles and cntvoff;
> + * reg3 stores low 32-bit difference of host cycles and cntvoff.
> + */
> + case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID:
> + /*
> + * system time and counter value must captured in the same
> + * time to keep consistency and precision.
> + */
> + ktime_get_snapshot(&systime_snapshot);
> + if (systime_snapshot.cs_id != CSID_ARM_ARCH_COUNTER)
> + break;
> + val[0] = upper_32_bits(systime_snapshot.real);
> + val[1] = lower_32_bits(systime_snapshot.real);
> + /*
> + * which of virtual counter or physical counter being
> + * asked for is decided by the first argument.
> + */
> + feature = smccc_get_arg1(vcpu);
> + switch (feature) {
> + case ARM_SMCCC_VENDOR_HYP_KVM_PTP_PHY_FUNC_ID:
> + cycles = systime_snapshot.cycles;
> + break;
> + default:
There's something a bit odd here.
ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID and
ARM_SMCCC_VENDOR_HYP_KVM_PTP_PHY_FUNC_ID look like they should be names
of separate (top-level) functions, but actually the _PHY_ one is a
parameter for the first. If the intention is to have a parameter then it
would be better to pick a better name for the _PHY_ define and not
define it using ARM_SMCCC_CALL_VAL.
Second the use of "default:" means that there's no possibility to later
extend this interface for more clocks if needed in the future.
Alternatively you could indeed implement as two top-level functions and
change this to a...
switch (func_id)
... along with multiple case labels as the functions would obviously be
mostly the same.
Also a minor style issue - you might want to consider splitting this
into it's own function.
Finally I do think it would be useful to add some documentation of the
new SMC calls. It would be easier to review the interface based on that
documentation rather than trying to reverse-engineer the interface from
the code.
Steve
> + cycles = systime_snapshot.cycles -
> + vcpu_vtimer(vcpu)->cntvoff;
> + }
> + val[2] = upper_32_bits(cycles);
> + val[3] = lower_32_bits(cycles);
> + break;
> +#endif
> +
> default:
> return kvm_psci_call(vcpu);
> }
>
_______________________________________________
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 0/5] STM32 CRC update
From: Herbert Xu @ 2020-05-22 14:13 UTC (permalink / raw)
To: Nicolas Toromanoff
Cc: Alexandre Torgue, linux-kernel, David S . Miller, linux-crypto,
Maxime Coquelin, linux-stm32, linux-arm-kernel
In-Reply-To: <20200512141113.18972-1-nicolas.toromanoff@st.com>
On Tue, May 12, 2020 at 04:11:08PM +0200, Nicolas Toromanoff wrote:
> This set of patches update the STM32 CRC driver.
> It contains bug fix.
>
> First fixes issue if we enable STM32 CRC32 hardware accelerator with
> ext4 (with metadata-chksum enable) and other fs that use same direct
> access to update crc32 API without previous init.
> Second fixes some issues raise by the extra self-test.
> Third fixes wrong hw usage if there is multiple IP on the SOC.
> Forth fixes "sleep while atomic" in tcrypt test, and some other places
> (ext4)
> Last fixes concurrent accesses. As state is saved in the hardware cell
> and not in stack as other CRC32 drivers, we need to create atomic
> section to protect concurrent CRC32 calculus.
>
> This patch series applies to cryptodev/master.
>
> Nicolas Toromanoff (5):
> crypto: stm32/crc: fix ext4 chksum BUG_ON()
> crypto: stm32/crc: fix run-time self test issue.
> crypto: stm32/crc: fix multi-instance
> crypto: stm32/crc: don't sleep in runtime pm
> crypto: stm32/crc: protect from concurrent accesses
>
> drivers/crypto/stm32/stm32-crc32.c | 230 ++++++++++++++++++++---------
> 1 file changed, 161 insertions(+), 69 deletions(-)
All applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
_______________________________________________
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 09/12] dt-bindings: arm: fsl: Add msi-map device-tree binding for fsl-mc bus
From: Rob Herring @ 2020-05-22 14:08 UTC (permalink / raw)
To: Diana Craciun OSS
Cc: devicetree, Hanjun Guo, Lorenzo Pieralisi, Marc Zyngier,
Will Deacon, PCI, Sudeep Holla, Rafael J. Wysocki, Linux IOMMU,
linux-acpi, Makarand Pawagi, Catalin Marinas, Bjorn Helgaas,
Robin Murphy,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <ee7a5c04-814e-215f-ec74-52c2f3b881d0@oss.nxp.com>
On Fri, May 22, 2020 at 3:57 AM Diana Craciun OSS
<diana.craciun@oss.nxp.com> wrote:
>
> On 5/22/2020 12:42 PM, Robin Murphy wrote:
> > On 2020-05-22 00:10, Rob Herring wrote:
> >> On Thu, May 21, 2020 at 7:00 AM Lorenzo Pieralisi
> >> <lorenzo.pieralisi@arm.com> wrote:
> >>>
> >>> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> >>>
> >>> The existing bindings cannot be used to specify the relationship
> >>> between fsl-mc devices and GIC ITSes.
> >>>
> >>> Add a generic binding for mapping fsl-mc devices to GIC ITSes, using
> >>> msi-map property.
> >>>
> >>> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> >>> Cc: Rob Herring <robh+dt@kernel.org>
> >>> ---
> >>> .../devicetree/bindings/misc/fsl,qoriq-mc.txt | 30
> >>> +++++++++++++++++--
> >>> 1 file changed, 27 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> >>> b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> >>> index 9134e9bcca56..b0813b2d0493 100644
> >>> --- a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> >>> +++ b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> >>> @@ -18,9 +18,9 @@ same hardware "isolation context" and a 10-bit
> >>> value called an ICID
> >>> the requester.
> >>>
> >>> The generic 'iommus' property is insufficient to describe the
> >>> relationship
> >>> -between ICIDs and IOMMUs, so an iommu-map property is used to define
> >>> -the set of possible ICIDs under a root DPRC and how they map to
> >>> -an IOMMU.
> >>> +between ICIDs and IOMMUs, so the iommu-map and msi-map properties
> >>> are used
> >>> +to define the set of possible ICIDs under a root DPRC and how they
> >>> map to
> >>> +an IOMMU and a GIC ITS respectively.
> >>>
> >>> For generic IOMMU bindings, see
> >>> Documentation/devicetree/bindings/iommu/iommu.txt.
> >>> @@ -28,6 +28,9 @@ Documentation/devicetree/bindings/iommu/iommu.txt.
> >>> For arm-smmu binding, see:
> >>> Documentation/devicetree/bindings/iommu/arm,smmu.yaml.
> >>>
> >>> +For GICv3 and GIC ITS bindings, see:
> >>> +Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml.
> >>>
> >>> +
> >>> Required properties:
> >>>
> >>> - compatible
> >>> @@ -119,6 +122,15 @@ Optional properties:
> >>> associated with the listed IOMMU, with the iommu-specifier
> >>> (i - icid-base + iommu-base).
> >>>
> >>> +- msi-map: Maps an ICID to a GIC ITS and associated iommu-specifier
> >>> + data.
> >>> +
> >>> + The property is an arbitrary number of tuples of
> >>> + (icid-base,iommu,iommu-base,length).
> >>
> >> I'm confused because the example has GIC ITS phandle, not an IOMMU.
> >>
> >> What is an iommu-base?
> >
> > Right, I was already halfway through writing a reply to say that all
> > the copy-pasted "iommu" references here should be using the
> > terminology from the pci-msi.txt binding instead.
>
> Right, will change it.
>
> >
> >>> +
> >>> + Any ICID in the interval [icid-base, icid-base + length) is
> >>> + associated with the listed GIC ITS, with the iommu-specifier
> >>> + (i - icid-base + iommu-base).
> >>> Example:
> >>>
> >>> smmu: iommu@5000000 {
> >>> @@ -128,6 +140,16 @@ Example:
> >>> ...
> >>> };
> >>>
> >>> + gic: interrupt-controller@6000000 {
> >>> + compatible = "arm,gic-v3";
> >>> + ...
> >>> + its: gic-its@6020000 {
> >>> + compatible = "arm,gic-v3-its";
> >>> + msi-controller;
> >>> + ...
> >>> + };
> >>> + };
> >>> +
> >>> fsl_mc: fsl-mc@80c000000 {
> >>> compatible = "fsl,qoriq-mc";
> >>> reg = <0x00000008 0x0c000000 0 0x40>, /* MC
> >>> portal base */
> >>> @@ -135,6 +157,8 @@ Example:
> >>> msi-parent = <&its>;
> >
> > Side note: is it right to keep msi-parent here? It rather implies that
> > the MC itself has a 'native' Device ID rather than an ICID, which I
> > believe is not strictly true. Plus it's extra-confusing that it
> > doesn't specify an ID either way, since that makes it look like the
> > legacy PCI case that gets treated implicitly as an identity msi-map,
> > which makes no sense at all to combine with an actual msi-map.
>
> Before adding msi-map, the fsl-mc code assumed that ICID and streamID
> are equal and used msi-parent just to get the reference to the ITS node.
> Removing msi-parent will break the backward compatibility of the already
> existing systems. Maybe we should mention that this is legacy and not to
> be used for newer device trees.
If ids are 1:1, then the DT should use msi-parent. If there is
remapping, then use msi-map. A given system should use one or the
other. I suppose if some ids are 1:1 and the msi-map was added to add
additional support for ids not 1:1, then you could end up with both.
That's fine in dts files, but examples should reflect the 'right' way.
Rob
_______________________________________________
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 09/12] dt-bindings: arm: fsl: Add msi-map device-tree binding for fsl-mc bus
From: Rob Herring @ 2020-05-22 14:02 UTC (permalink / raw)
To: Robin Murphy
Cc: devicetree, Hanjun Guo, Lorenzo Pieralisi, Catalin Marinas, PCI,
Sudeep Holla, Rafael J. Wysocki, Linux IOMMU, linux-acpi,
Makarand Pawagi, Marc Zyngier, Diana Craciun, Bjorn Helgaas,
Will Deacon,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <abca6ecb-5d93-832f-ff7c-de53bb6203f3@arm.com>
On Fri, May 22, 2020 at 3:42 AM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2020-05-22 00:10, Rob Herring wrote:
> > On Thu, May 21, 2020 at 7:00 AM Lorenzo Pieralisi
> > <lorenzo.pieralisi@arm.com> wrote:
> >>
> >> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> >>
> >> The existing bindings cannot be used to specify the relationship
> >> between fsl-mc devices and GIC ITSes.
> >>
> >> Add a generic binding for mapping fsl-mc devices to GIC ITSes, using
> >> msi-map property.
> >>
> >> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> >> Cc: Rob Herring <robh+dt@kernel.org>
> >> ---
> >> .../devicetree/bindings/misc/fsl,qoriq-mc.txt | 30 +++++++++++++++++--
> >> 1 file changed, 27 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> >> index 9134e9bcca56..b0813b2d0493 100644
> >> --- a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> >> +++ b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> >> @@ -18,9 +18,9 @@ same hardware "isolation context" and a 10-bit value called an ICID
> >> the requester.
> >>
> >> The generic 'iommus' property is insufficient to describe the relationship
> >> -between ICIDs and IOMMUs, so an iommu-map property is used to define
> >> -the set of possible ICIDs under a root DPRC and how they map to
> >> -an IOMMU.
> >> +between ICIDs and IOMMUs, so the iommu-map and msi-map properties are used
> >> +to define the set of possible ICIDs under a root DPRC and how they map to
> >> +an IOMMU and a GIC ITS respectively.
> >>
> >> For generic IOMMU bindings, see
> >> Documentation/devicetree/bindings/iommu/iommu.txt.
> >> @@ -28,6 +28,9 @@ Documentation/devicetree/bindings/iommu/iommu.txt.
> >> For arm-smmu binding, see:
> >> Documentation/devicetree/bindings/iommu/arm,smmu.yaml.
> >>
> >> +For GICv3 and GIC ITS bindings, see:
> >> +Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml.
> >> +
> >> Required properties:
> >>
> >> - compatible
> >> @@ -119,6 +122,15 @@ Optional properties:
> >> associated with the listed IOMMU, with the iommu-specifier
> >> (i - icid-base + iommu-base).
> >>
> >> +- msi-map: Maps an ICID to a GIC ITS and associated iommu-specifier
> >> + data.
> >> +
> >> + The property is an arbitrary number of tuples of
> >> + (icid-base,iommu,iommu-base,length).
> >
> > I'm confused because the example has GIC ITS phandle, not an IOMMU.
> >
> > What is an iommu-base?
>
> Right, I was already halfway through writing a reply to say that all the
> copy-pasted "iommu" references here should be using the terminology from
> the pci-msi.txt binding instead.
>
> >> +
> >> + Any ICID in the interval [icid-base, icid-base + length) is
> >> + associated with the listed GIC ITS, with the iommu-specifier
> >> + (i - icid-base + iommu-base).
> >> Example:
> >>
> >> smmu: iommu@5000000 {
> >> @@ -128,6 +140,16 @@ Example:
> >> ...
> >> };
> >>
> >> + gic: interrupt-controller@6000000 {
> >> + compatible = "arm,gic-v3";
> >> + ...
> >> + its: gic-its@6020000 {
> >> + compatible = "arm,gic-v3-its";
> >> + msi-controller;
> >> + ...
> >> + };
> >> + };
> >> +
> >> fsl_mc: fsl-mc@80c000000 {
> >> compatible = "fsl,qoriq-mc";
> >> reg = <0x00000008 0x0c000000 0 0x40>, /* MC portal base */
> >> @@ -135,6 +157,8 @@ Example:
> >> msi-parent = <&its>;
>
> Side note: is it right to keep msi-parent here? It rather implies that
> the MC itself has a 'native' Device ID rather than an ICID, which I
> believe is not strictly true. Plus it's extra-confusing that it doesn't
> specify an ID either way, since that makes it look like the legacy PCI
> case that gets treated implicitly as an identity msi-map, which makes no
> sense at all to combine with an actual msi-map.
No, it doesn't make sense from a binding perspective.
>
> >> /* define map for ICIDs 23-64 */
> >> iommu-map = <23 &smmu 23 41>;
> >> + /* define msi map for ICIDs 23-64 */
> >> + msi-map = <23 &its 23 41>;
> >
> > Seeing 23 twice is odd. The numbers to the right of 'its' should be an
> > ITS number space.
>
> On about 99% of systems the values in the SMMU Stream ID and ITS Device
> ID spaces are going to be the same. Nobody's going to bother carrying
> *two* sets of sideband data across the interconnect if they don't have to ;)
I'm referring to the 23 on the left and right, not between the msi and
iommu. If the left and right are the same, then what are we remapping
exactly?
Rob
_______________________________________________
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 v2 1/3] ARM: pxa: remove Compulab pxa2xx boards
From: Arnd Bergmann @ 2020-05-22 13:22 UTC (permalink / raw)
To: Miquel Raynal
Cc: linux-mtd, Boris Brezillon, Robert Jarzmik, Linux ARM,
Mike Rapoport
In-Reply-To: <20200522151750.369dd4d9@xps13>
On Fri, May 22, 2020 at 3:17 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Arnd Bergmann <arnd@arndb.de> wrote on Fri, 22 May 2020 15:14:43 +0200:
>
> > On Fri, May 22, 2020 at 1:18 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > Boris Brezillon <boris.brezillon@collabora.com> wrote on Thu, 21 May
> > > Actually I was about the merge this patch in the MTD tree because I
> > > have a big cleanup series which depends on it. Arnd could you confirm
> > > that it's okay for you?
> >
> > Yes, that sounds good. I assume the patch is basically identical to the one
> > that Mike just sent, right? If there are changes to arch/arm, I guess those can
> > be dropped now as I'm merging Robert's patches to remove those board
> > files.
> >
> > Arnd
>
> Here is the diffstat:
>
> drivers/mtd/nand/raw/Kconfig | 4 -
> drivers/mtd/nand/raw/Makefile | 1 -
> drivers/mtd/nand/raw/cmx270_nand.c | 236 -----------------------------
> 3 files changed, 241 deletions(-)
> delete mode 100644 drivers/mtd/nand/raw/cmx270_nand.c
>
> So it should be fine.
Agreed, please apply it.
Arnd
_______________________________________________
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 v2 1/3] ARM: pxa: remove Compulab pxa2xx boards
From: Miquel Raynal @ 2020-05-22 13:17 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-mtd, Boris Brezillon, Robert Jarzmik, Linux ARM,
Mike Rapoport
In-Reply-To: <CAK8P3a2Z-UxktGn+_aBVaaBTStT7f177V2ED2RfVCQS8QDBamg@mail.gmail.com>
Hi Arnd,
Arnd Bergmann <arnd@arndb.de> wrote on Fri, 22 May 2020 15:14:43 +0200:
> On Fri, May 22, 2020 at 1:18 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > Boris Brezillon <boris.brezillon@collabora.com> wrote on Thu, 21 May
> > 2020 21:10:16 +0200:
> >
> > > On Thu, 21 May 2020 22:01:58 +0300
> > > Mike Rapoport <rppt@kernel.org> wrote:
> > >
> > > > On Thu, May 21, 2020 at 09:26:10AM +0200, Arnd Bergmann wrote:
> > > > > On Thu, May 21, 2020 at 9:12 AM Mike Rapoport <rppt@kernel.org> wrote:
> > > > > >
> > > > > > On Wed, May 20, 2020 at 05:21:51PM +0200, Arnd Bergmann wrote:
> > > > > > > On Wed, May 20, 2020 at 12:21 PM Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> > > > > > > >
> > > > > > > > As these boards have no more users nor testers, and patching them has
> > > > > > > > become a burden, be that because of the PCI part or the MTD NAND
> > > > > > > > support, let's remove them.
> > > > > > > >
> > > > > > > > The cm-x300 will for now remain and represent Compulab boards at its
> > > > > > > > best in the PXA department.
> > > > > >
> > > > > > I think this series missed the cm-x270 NAND driver, so this could be
> > > > > > PATCH 4/3 (not even compile tested).
> > > > > >
> > > > > > From 56a11987f82ef8b32b25dfc17b849f9bbbf03e4d Mon Sep 17 00:00:00 2001
> > > > > > From: Mike Rapoport <rppt@linux.ibm.com>
> > > > > > Date: Thu, 21 May 2020 10:09:47 +0300
> > > > > > Subject: [PATCH] mtd: rawnand: remove CM-X270 NAND driver
> > > > > >
> > > > > > The cm-x270 board have been removed and theres is no point to keep this
> > > > > > driver.
> > > > > >
> > > > > > Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> > > > >
> > > > > Thanks! Removing that driver was actually how the discussion started,
> > > > > so we definitely want to do that. Boris earlier patch "mtd: rawnand: Get
> > > > > rid of the cmx270 driver" did it by changing the board file, now your
> > > > > patch is the right thing to do, and we should remember to do the same
> > > > > for mbxfb.
> > > >
> > > > Arnd, will you pick this to the SoC tree or should I resend?
> > >
> > > Actually I sent a v2 removing only the MTD driver. You should probably
> > > sync with Miquel to make sure the same patch is not applied twice.
> >
> > Actually I was about the merge this patch in the MTD tree because I
> > have a big cleanup series which depends on it. Arnd could you confirm
> > that it's okay for you?
>
> Yes, that sounds good. I assume the patch is basically identical to the one
> that Mike just sent, right? If there are changes to arch/arm, I guess those can
> be dropped now as I'm merging Robert's patches to remove those board
> files.
>
> Arnd
Here is the diffstat:
drivers/mtd/nand/raw/Kconfig | 4 -
drivers/mtd/nand/raw/Makefile | 1 -
drivers/mtd/nand/raw/cmx270_nand.c | 236 -----------------------------
3 files changed, 241 deletions(-)
delete mode 100644 drivers/mtd/nand/raw/cmx270_nand.c
So it should be fine.
Thanks,
Miquèl
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox