[PATCH v9 0/7] Add Qualcomm SM6115 / SM4250 EUD dt-bindings & driver support

[PATCH v9 0/7] Add Qualcomm SM6115 / SM4250 EUD dt-bindings & driver support

[Bhupesh Sharma]

Changes since v8:
-------------------
- v8 can be viewed here: https://lore.kernel.org/linux-arm-msm/20230717103236.1246771-1-bhupesh.sharma@linaro.org/
- Konrad and Stephan pointed that I should define 'tcsr syscon' node for sm6115.dtsi,
  and use phandle for the same inside the EUD node, which would eventually be used
  inside the eud driver. Added [PATCH 1/7] and [PATCH 5/7] for the same
  in this series.
- Rebased on latest linux-next/master.

Changes since v6/v7:
-------------------
- v6 can be viewed here: https://lore.kernel.org/linux-arm-msm/20230517211756.2483552-1-bhupesh.sharma@linaro.org/
- Konrad and Krzysztof had different suggestions on how to tackle
  different SoCs inside the eud driver which require access to secure mode
  manager register space. While Konrad's suggestion was to use a dt property,
  other comments suggested using optional platform data for determining
  the same. Modified [PATCH 2/4] accordingly to use the optional
  platform data for now.
- Added Krzysztof's RB for [PATCH 1/4] and also addressed his review comments
  received on v5.
- Dropped eud cleanup patches (which were sent a v7) as they have been accepted in linux-next.
- Rebased on latest linux-next/master.

Changes since v5:
----------------
- v5 can be viewed here: https://lore.kernel.org/linux-arm-msm/20230516213308.2432018-1-bhupesh.sharma@linaro.org/
- Addressed Mani's comment and added Fixes tag for [PATCH 1/6].
  Also collected his Ack for this patch.
- Fixed [PATCH 4/6] as per Greg's comments and added a separate patch
  for identation issues -> [PATCH 3/6].

Changes since v4:
----------------
- v4 can be viewed here: https://lore.kernel.org/linux-arm-msm/20230505064039.1630025-1-bhupesh.sharma@linaro.org/
- Addressed Konrad's review comments regarding EUD driver code.
- Also collected his R-B for [PATCH 4/5 and 5/5].
- Fixed the dt-bindings as per Krzysztof's comments.

Changes since v3:
----------------
- v3 can be viewed here: https://www.spinics.net/lists/linux-arm-msm/msg137025.html 
- Addressed Konrad's review comments regarding mainly the driver code.
  Also fixed the .dtsi as per his comments.
- Also collected his R-B for [PATCH 1/5].

Changes since v2:
----------------
- v2 can be viewed here: https://www.spinics.net/lists/linux-arm-msm/msg137025.html 
- Addressed Bjorn and Krzysztof's comments.
- Added [PATCH 1/5] which fixes the 'qcom_eud' sysfs path. 
- Added [PATCH 5/5] to enable EUD for Qualcomm QRB4210-RB2 boards.

Changes since v1:
----------------
- v1 can be viewed here: https://lore.kernel.org/linux-arm-msm/20221231130743.3285664-1-bhupesh.sharma@linaro.org
- Added Krzysztof in Cc list.
- Fixed the following issue reported by kernel test bot:
  >> ERROR: modpost: "qcom_scm_io_writel" [drivers/usb/misc/qcom_eud.ko] undefined!

This series adds the dt-binding and driver support for SM6115 / SM4250
EUD (Embedded USB Debugger) block available on Qualcomm SoCs.

It also enables the same for QRB4210-RB2 boards by default (the user
still needs to enable the same via sysfs).

The EUD is a mini-USB hub implemented on chip to support the USB-based debug
and trace capabilities.

EUD driver listens to events like USB attach or detach and then
informs the USB about these events via ROLE-SWITCH.

Bhupesh Sharma (7):
  dt-bindings: mfd: qcom,tcsr: Add the compatible for SM6115
  dt-bindings: soc: qcom: eud: Document vendor-specific 'secure mode'
    property
  dt-bindings: soc: qcom: eud: Add SM6115 / SM4250 support
  usb: misc: eud: Add driver support for SM6115 / SM4250
  arm64: dts: qcom: sm6115: Add tcsr syscon node
  arm64: dts: qcom: sm6115: Add EUD dt node and dwc3 connector
  arm64: dts: qcom: qrb4210-rb2: Enable EUD debug peripheral

 .../devicetree/bindings/mfd/qcom,tcsr.yaml    |  1 +
 .../bindings/soc/qcom/qcom,eud.yaml           |  9 +++
 arch/arm64/boot/dts/qcom/qrb4210-rb2.dts      | 27 ++++++-
 arch/arm64/boot/dts/qcom/sm6115.dtsi          | 56 ++++++++++++++
 drivers/usb/misc/Kconfig                      |  2 +-
 drivers/usb/misc/qcom_eud.c                   | 76 +++++++++++++++++--
 6 files changed, 164 insertions(+), 7 deletions(-)

-- 
2.38.1

[PATCH v9 1/7] dt-bindings: mfd: qcom,tcsr: Add the compatible for SM6115

[Bhupesh Sharma]

Document the qcom,sm6115-tcsr compatible.

Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
---
 Documentation/devicetree/bindings/mfd/qcom,tcsr.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mfd/qcom,tcsr.yaml b/Documentation/devicetree/bindings/mfd/qcom,tcsr.yaml
index 5ad9d5deaaf8a..b580398041d87 100644
--- a/Documentation/devicetree/bindings/mfd/qcom,tcsr.yaml
+++ b/Documentation/devicetree/bindings/mfd/qcom,tcsr.yaml
@@ -27,6 +27,7 @@ properties:
           - qcom,sdm845-tcsr
           - qcom,sdx55-tcsr
           - qcom,sdx65-tcsr
+          - qcom,sm6115-tcsr
           - qcom,sm8150-tcsr
           - qcom,sm8450-tcsr
           - qcom,tcsr-apq8064
-- 
2.38.1

[PATCH v9 2/7] dt-bindings: soc: qcom: eud: Document vendor-specific 'secure mode' property

[Bhupesh Sharma]

On some SoCs (like the SM6115 / SM4250 SoC), the enable bit inside
'tcsr_check_reg' needs to be set first to 'enable' EUD mode.

So introduce a vendor-specific dt-property 'qcom,secure-eud-reg'
which specifies the base address of the TCSR reg space and the offset
of the 'tcsr_check_reg'.

Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
---
 Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
index f2c5ec7e6437b..ca38d219e57d5 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
@@ -45,6 +45,14 @@ properties:
         $ref: /schemas/graph.yaml#/properties/port
         description: This port is to be attached to the type C connector.
 
+  qcom,secure-eud-reg:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      - items:
+          - description: phandle to TCSR hardware block
+          - description: offset of the secure mode manager register
+    description: TCSR hardware block
+
 required:
   - compatible
   - reg
-- 
2.38.1

[PATCH v9 3/7] dt-bindings: soc: qcom: eud: Add SM6115 / SM4250 support

[Bhupesh Sharma]

Add dt-bindings for EUD found on Qualcomm SM6115 / SM4250 SoC.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
---
 Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
index ca38d219e57d5..bb9b97f03585f 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
@@ -18,6 +18,7 @@ properties:
     items:
       - enum:
           - qcom,sc7280-eud
+          - qcom,sm6115-eud
       - const: qcom,eud
 
   reg:
-- 
2.38.1

[PATCH v9 4/7] usb: misc: eud: Add driver support for SM6115 / SM4250

[Bhupesh Sharma]

Add SM6115 / SM4250 SoC EUD support in qcom_eud driver.

On some SoCs (like the SM6115 / SM4250 SoC), the mode manager
needs to be accessed only via the secure world (through 'scm'
calls).

Also, the enable bit inside 'tcsr_check_reg' needs to be set
first to set the eud in 'enable' mode on these SoCs.

Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
---
 drivers/usb/misc/Kconfig    |  2 +-
 drivers/usb/misc/qcom_eud.c | 76 ++++++++++++++++++++++++++++++++++---
 2 files changed, 72 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
index 99b15b77dfd57..51eb5140caa14 100644
--- a/drivers/usb/misc/Kconfig
+++ b/drivers/usb/misc/Kconfig
@@ -146,7 +146,7 @@ config USB_APPLEDISPLAY
 
 config USB_QCOM_EUD
 	tristate "QCOM Embedded USB Debugger(EUD) Driver"
-	depends on ARCH_QCOM || COMPILE_TEST
+	depends on (ARCH_QCOM && QCOM_SCM) || COMPILE_TEST
 	select USB_ROLE_SWITCH
 	help
 	  This module enables support for Qualcomm Technologies, Inc.
diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
index 7f371ea1248c3..a5b28fc24116a 100644
--- a/drivers/usb/misc/qcom_eud.c
+++ b/drivers/usb/misc/qcom_eud.c
@@ -11,9 +11,12 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/sysfs.h>
+#include <linux/firmware/qcom/qcom_scm.h>
 #include <linux/usb/role.h>
 
 #define EUD_REG_INT1_EN_MASK	0x0024
@@ -30,6 +33,10 @@
 #define EUD_INT_SAFE_MODE	BIT(4)
 #define EUD_INT_ALL		(EUD_INT_VBUS | EUD_INT_SAFE_MODE)
 
+#define EUD_EN2_EN		BIT(0)
+#define EUD_EN2_DISABLE		(0)
+#define TCSR_CHECK_EN		BIT(0)
+
 struct eud_chip {
 	struct device			*dev;
 	struct usb_role_switch		*role_sw;
@@ -39,6 +46,7 @@ struct eud_chip {
 	int				irq;
 	bool				enabled;
 	bool				usb_attached;
+	phys_addr_t			secure_mode_mgr;
 };
 
 static int enable_eud(struct eud_chip *priv)
@@ -46,7 +54,11 @@ static int enable_eud(struct eud_chip *priv)
 	writel(EUD_ENABLE, priv->base + EUD_REG_CSR_EUD_EN);
 	writel(EUD_INT_VBUS | EUD_INT_SAFE_MODE,
 			priv->base + EUD_REG_INT1_EN_MASK);
-	writel(1, priv->mode_mgr + EUD_REG_EUD_EN2);
+
+	if (priv->secure_mode_mgr)
+		qcom_scm_io_writel(priv->secure_mode_mgr + EUD_REG_EUD_EN2, EUD_EN2_EN);
+	else
+		writel(EUD_EN2_EN, priv->mode_mgr + EUD_REG_EUD_EN2);
 
 	return usb_role_switch_set_role(priv->role_sw, USB_ROLE_DEVICE);
 }
@@ -54,7 +66,11 @@ static int enable_eud(struct eud_chip *priv)
 static void disable_eud(struct eud_chip *priv)
 {
 	writel(0, priv->base + EUD_REG_CSR_EUD_EN);
-	writel(0, priv->mode_mgr + EUD_REG_EUD_EN2);
+
+	if (priv->secure_mode_mgr)
+		qcom_scm_io_writel(priv->secure_mode_mgr + EUD_REG_EUD_EN2, EUD_EN2_DISABLE);
+	else
+		writel(EUD_EN2_DISABLE, priv->mode_mgr + EUD_REG_EUD_EN2);
 }
 
 static ssize_t enable_show(struct device *dev,
@@ -175,9 +191,37 @@ static void eud_role_switch_release(void *data)
 	usb_role_switch_put(chip->role_sw);
 }
 
+static int eud_find_secure_reg_addr(struct device *dev, u64 *addr)
+{
+	struct device_node *tcsr;
+	struct device_node *np = dev->of_node;
+	struct resource res;
+	u32 offset;
+	int ret;
+
+	tcsr = of_parse_phandle(np, "qcom,secure-eud-reg", 0);
+	if (!tcsr)
+		return 0;
+
+	ret = of_address_to_resource(tcsr, 0, &res);
+	of_node_put(tcsr);
+	if (ret)
+		return ret;
+
+	ret = of_property_read_u32_index(np, "qcom,secure-eud-reg", 1, &offset);
+	if (ret < 0)
+		return ret;
+
+	*addr = res.start + offset;
+
+	return 0;
+}
+
 static int eud_probe(struct platform_device *pdev)
 {
 	struct eud_chip *chip;
+	struct resource *res;
+	phys_addr_t tcsr_check = 0;
 	int ret;
 
 	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
@@ -200,9 +244,30 @@ static int eud_probe(struct platform_device *pdev)
 	if (IS_ERR(chip->base))
 		return PTR_ERR(chip->base);
 
-	chip->mode_mgr = devm_platform_ioremap_resource(pdev, 1);
-	if (IS_ERR(chip->mode_mgr))
-		return PTR_ERR(chip->mode_mgr);
+	/*
+	 * EUD block on a few Qualcomm SoCs needs secure register access.
+	 * Check for the same via vendor-specific dt property.
+	 */
+	ret = eud_find_secure_reg_addr(&pdev->dev, &tcsr_check);
+	if (ret < 0)
+		return ret;
+
+	if (tcsr_check) {
+		res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+		if (!res)
+			return dev_err_probe(chip->dev, -ENODEV,
+					     "failed to get secure_mode_mgr reg base\n");
+
+		chip->secure_mode_mgr = res->start;
+
+		ret = qcom_scm_io_writel(tcsr_check, TCSR_CHECK_EN);
+		if (ret)
+			return dev_err_probe(chip->dev, ret, "failed to write tcsr check reg\n");
+	} else {
+		chip->mode_mgr = devm_platform_ioremap_resource(pdev, 1);
+		if (IS_ERR(chip->mode_mgr))
+			return PTR_ERR(chip->mode_mgr);
+	}
 
 	chip->irq = platform_get_irq(pdev, 0);
 	ret = devm_request_threaded_irq(&pdev->dev, chip->irq, handle_eud_irq,
@@ -230,6 +295,7 @@ static void eud_remove(struct platform_device *pdev)
 
 static const struct of_device_id eud_dt_match[] = {
 	{ .compatible = "qcom,sc7280-eud" },
+	{ .compatible = "qcom,sm6115-eud" },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, eud_dt_match);
-- 
2.38.1

[PATCH v9 5/7] arm64: dts: qcom: sm6115: Add tcsr syscon node

[Bhupesh Sharma]

Add the TCSR syscon dt node for SM6115 / SM4250 SoC.

Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm6115.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
index 839c603512403..3be0b54fc3628 100644
--- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
@@ -591,6 +591,11 @@ tcsr_mutex: hwlock@340000 {
 			#hwlock-cells = <1>;
 		};
 
+		tcsr_regs: syscon@3c0000 {
+			compatible = "qcom,sm6115-tcsr", "syscon";
+			reg = <0x0 0x003c0000 0x0 0x40000>;
+		};
+
 		tlmm: pinctrl@500000 {
 			compatible = "qcom,sm6115-tlmm";
 			reg = <0x0 0x00500000 0x0 0x400000>,
-- 
2.38.1

[PATCH v9 6/7] arm64: dts: qcom: sm6115: Add EUD dt node and dwc3 connector

[Bhupesh Sharma]

Add the Embedded USB Debugger(EUD) device tree node for
SM6115 / SM4250 SoC.

The node contains EUD base register region, EUD mode manager
register region.

The TCSR Base register region is defined via a vendor specific
dt-property.

Also add the typec connector node for EUD which is attached to
EUD node via port. EUD is also attached to DWC3 node via port.

To enable the role switch, we need to set dr_mode = "otg" property
for 'usb_dwc3' sub-node in the board dts file.

Also the EUD device can be enabled on a board once linux is boot'ed
by setting:
 $ echo 1 > /sys/bus/platform/drivers/qcom_eud/../enable

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm6115.dtsi | 51 ++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
index 3be0b54fc3628..cd5b85b63a9ec 100644
--- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
@@ -260,6 +260,18 @@ CLUSTER_1_SLEEP_1: cluster-sleep-1-1 {
 		};
 	};
 
+	eud_typec: connector {
+		compatible = "usb-c-connector";
+
+		ports {
+			port@0 {
+				con_eud: endpoint {
+					remote-endpoint = <&eud_con>;
+				};
+			};
+		};
+	};
+
 	firmware {
 		scm: scm {
 			compatible = "qcom,scm-sm6115", "qcom,scm";
@@ -794,6 +806,38 @@ gcc: clock-controller@1400000 {
 			#power-domain-cells = <1>;
 		};
 
+		eud: eud@1610000 {
+			compatible = "qcom,sm6115-eud", "qcom,eud";
+			reg = <0x0 0x01610000 0x0 0x2000>,
+			      <0x0 0x01612000 0x0 0x1000>;
+			reg-names = "eud-base", "eud-mode-mgr";
+			interrupts = <GIC_SPI 189 IRQ_TYPE_LEVEL_HIGH>;
+
+			qcom,secure-eud-reg = <&tcsr_regs 0x25018>;
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+
+					eud_ep: endpoint {
+						remote-endpoint = <&usb2_role_switch>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+
+					eud_con: endpoint {
+						remote-endpoint = <&con_eud>;
+					};
+				};
+			};
+		};
+
 		usb_hsphy: phy@1613000 {
 			compatible = "qcom,sm6115-qusb2-phy";
 			reg = <0x0 0x01613000 0x0 0x180>;
@@ -1327,6 +1371,13 @@ usb_dwc3: usb@4e00000 {
 				snps,has-lpm-erratum;
 				snps,hird-threshold = /bits/ 8 <0x10>;
 				snps,usb3_lpm_capable;
+				usb-role-switch;
+
+				port {
+					usb2_role_switch: endpoint {
+						remote-endpoint = <&eud_ep>;
+					};
+				};
 			};
 		};
 
-- 
2.38.1

[PATCH v9 7/7] arm64: dts: qcom: qrb4210-rb2: Enable EUD debug peripheral

[Bhupesh Sharma]

Since the USB-C type port on the Qualcomm QRB4210-RB2 board
can be set primarily in a 'device' configuration (with the default
DIP switch settings), it makes sense to enable the EUD debug
peripheral on the board by default by setting the USB 'dr_mode' property
as 'otg'.

Now, the EUD debug peripheral can be enabled by executing:
 $ echo 1 > /sys/bus/platform/drivers/qcom_eud/1610000.eud/enable

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
---
 arch/arm64/boot/dts/qcom/qrb4210-rb2.dts | 27 +++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts b/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts
index a7278a9472ed9..640668960deb0 100644
--- a/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts
+++ b/arch/arm64/boot/dts/qcom/qrb4210-rb2.dts
@@ -264,6 +264,10 @@ &pon_resin {
 	status = "okay";
 };
 
+&eud {
+	status = "okay";
+};
+
 &qupv3_id_0 {
 	status = "okay";
 };
@@ -518,7 +522,28 @@ &usb {
 
 &usb_dwc3 {
 	maximum-speed = "super-speed";
-	dr_mode = "peripheral";
+
+	/*
+	 * There is only one USB DWC3 controller on QRB4210 board and it is connected
+	 * via a DIP Switch:
+	 * - to either an USB - C type connector or an USB - A type connector
+	 *   (via a GL3590-S hub), and
+	 * - to either an USB - A type connector (via a GL3590-S hub) or a connector
+	 *   for further connection with a mezzanine board.
+	 *
+	 * All of the above hardware muxes would allow us to hook things up in
+	 * different ways to some potential benefit for static configurations (for e.g.
+	 * on one hand we can have two USB - A type connectors and a USB - Ethernet
+	 * connection available and on the other we can use the USB - C type in
+	 * peripheral mode).
+	 *
+	 * Note that since the USB - C type can be used only in peripehral mode,
+	 * so hardcoding the mode to 'peripheral' here makes sense.
+	 *
+	 * However since we want to use the EUD debug device, we set the mode as
+	 * 'otg' here.
+	 */
+	dr_mode = "otg";
 };
 
 &usb_hsphy {
-- 
2.38.1

Re: [PATCH v9 1/7] dt-bindings: mfd: qcom,tcsr: Add the compatible for SM6115

[Krzysztof Kozlowski]

On 18/07/2023 08:10, Bhupesh Sharma wrote:
> Document the qcom,sm6115-tcsr compatible.
> 
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> ---
>  Documentation/devicetree/bindings/mfd/qcom,tcsr.yaml | 1 +

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v9 2/7] dt-bindings: soc: qcom: eud: Document vendor-specific 'secure mode' property

[Krzysztof Kozlowski]

On 18/07/2023 08:10, Bhupesh Sharma wrote:
> On some SoCs (like the SM6115 / SM4250 SoC), the enable bit inside
> 'tcsr_check_reg' needs to be set first to 'enable' EUD mode.
> 
> So introduce a vendor-specific dt-property 'qcom,secure-eud-reg'
> which specifies the base address of the TCSR reg space and the offset
> of the 'tcsr_check_reg'.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v9 4/7] usb: misc: eud: Add driver support for SM6115 / SM4250

[Krzysztof Kozlowski]

On 18/07/2023 08:10, Bhupesh Sharma wrote:
> Add SM6115 / SM4250 SoC EUD support in qcom_eud driver.
> 
> On some SoCs (like the SM6115 / SM4250 SoC), the mode manager
> needs to be accessed only via the secure world (through 'scm'
> calls).
> 
> Also, the enable bit inside 'tcsr_check_reg' needs to be set
> first to set the eud in 'enable' mode on these SoCs.
> 
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> ---
>  drivers/usb/misc/Kconfig    |  2 +-
>  drivers/usb/misc/qcom_eud.c | 76 ++++++++++++++++++++++++++++++++++---
>  2 files changed, 72 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
> index 99b15b77dfd57..51eb5140caa14 100644
> --- a/drivers/usb/misc/Kconfig
> +++ b/drivers/usb/misc/Kconfig
> @@ -146,7 +146,7 @@ config USB_APPLEDISPLAY
>  
>  config USB_QCOM_EUD
>  	tristate "QCOM Embedded USB Debugger(EUD) Driver"
> -	depends on ARCH_QCOM || COMPILE_TEST
> +	depends on (ARCH_QCOM && QCOM_SCM) || COMPILE_TEST
>  	select USB_ROLE_SWITCH
>  	help
>  	  This module enables support for Qualcomm Technologies, Inc.
> diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
> index 7f371ea1248c3..a5b28fc24116a 100644
> --- a/drivers/usb/misc/qcom_eud.c
> +++ b/drivers/usb/misc/qcom_eud.c
> @@ -11,9 +11,12 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/sysfs.h>
> +#include <linux/firmware/qcom/qcom_scm.h>
>  #include <linux/usb/role.h>
>  
>  #define EUD_REG_INT1_EN_MASK	0x0024
> @@ -30,6 +33,10 @@
>  #define EUD_INT_SAFE_MODE	BIT(4)
>  #define EUD_INT_ALL		(EUD_INT_VBUS | EUD_INT_SAFE_MODE)
>  
> +#define EUD_EN2_EN		BIT(0)
> +#define EUD_EN2_DISABLE		(0)
> +#define TCSR_CHECK_EN		BIT(0)
> +
>  struct eud_chip {
>  	struct device			*dev;
>  	struct usb_role_switch		*role_sw;
> @@ -39,6 +46,7 @@ struct eud_chip {
>  	int				irq;
>  	bool				enabled;
>  	bool				usb_attached;
> +	phys_addr_t			secure_mode_mgr;
>  };
>  
>  static int enable_eud(struct eud_chip *priv)
> @@ -46,7 +54,11 @@ static int enable_eud(struct eud_chip *priv)
>  	writel(EUD_ENABLE, priv->base + EUD_REG_CSR_EUD_EN);
>  	writel(EUD_INT_VBUS | EUD_INT_SAFE_MODE,
>  			priv->base + EUD_REG_INT1_EN_MASK);
> -	writel(1, priv->mode_mgr + EUD_REG_EUD_EN2);
> +
> +	if (priv->secure_mode_mgr)
> +		qcom_scm_io_writel(priv->secure_mode_mgr + EUD_REG_EUD_EN2, EUD_EN2_EN);
> +	else
> +		writel(EUD_EN2_EN, priv->mode_mgr + EUD_REG_EUD_EN2);
>  
>  	return usb_role_switch_set_role(priv->role_sw, USB_ROLE_DEVICE);
>  }
> @@ -54,7 +66,11 @@ static int enable_eud(struct eud_chip *priv)
>  static void disable_eud(struct eud_chip *priv)
>  {
>  	writel(0, priv->base + EUD_REG_CSR_EUD_EN);
> -	writel(0, priv->mode_mgr + EUD_REG_EUD_EN2);
> +
> +	if (priv->secure_mode_mgr)
> +		qcom_scm_io_writel(priv->secure_mode_mgr + EUD_REG_EUD_EN2, EUD_EN2_DISABLE);
> +	else
> +		writel(EUD_EN2_DISABLE, priv->mode_mgr + EUD_REG_EUD_EN2);
>  }
>  
>  static ssize_t enable_show(struct device *dev,
> @@ -175,9 +191,37 @@ static void eud_role_switch_release(void *data)
>  	usb_role_switch_put(chip->role_sw);
>  }
>  
> +static int eud_find_secure_reg_addr(struct device *dev, u64 *addr)
> +{
> +	struct device_node *tcsr;
> +	struct device_node *np = dev->of_node;
> +	struct resource res;
> +	u32 offset;
> +	int ret;
> +
> +	tcsr = of_parse_phandle(np, "qcom,secure-eud-reg", 0);
> +	if (!tcsr)
> +		return 0;
> +
> +	ret = of_address_to_resource(tcsr, 0, &res);
> +	of_node_put(tcsr);
> +	if (ret)
> +		return ret;
> +
> +	ret = of_property_read_u32_index(np, "qcom,secure-eud-reg", 1, &offset);
> +	if (ret < 0)
> +		return ret;
> +
> +	*addr = res.start + offset;
> +
> +	return 0;
> +}
> +
>  static int eud_probe(struct platform_device *pdev)
>  {
>  	struct eud_chip *chip;
> +	struct resource *res;
> +	phys_addr_t tcsr_check = 0;
>  	int ret;
>  
>  	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
> @@ -200,9 +244,30 @@ static int eud_probe(struct platform_device *pdev)
>  	if (IS_ERR(chip->base))
>  		return PTR_ERR(chip->base);
>  
> -	chip->mode_mgr = devm_platform_ioremap_resource(pdev, 1);
> -	if (IS_ERR(chip->mode_mgr))
> -		return PTR_ERR(chip->mode_mgr);
> +	/*
> +	 * EUD block on a few Qualcomm SoCs needs secure register access.
> +	 * Check for the same via vendor-specific dt property.
> +	 */
> +	ret = eud_find_secure_reg_addr(&pdev->dev, &tcsr_check);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (tcsr_check) {
> +		res = platform_get_resource(pdev, IORESOURCE_MEM, 1);

I don't understand this code. If you have syscon to eud reg, then why
you are not using it, but instead map again second address space?

> +		if (!res)
> +			return dev_err_probe(chip->dev, -ENODEV,
> +					     "failed to get secure_mode_mgr reg base\n");
> +
> +		chip->secure_mode_mgr = res->start;
> +
> +		ret = qcom_scm_io_writel(tcsr_check, TCSR_CHECK_EN);

That's not how syscon is used. Your bindings defined it as syscon, so
are you sure you are using it correctly?



Best regards,
Krzysztof

Re: [PATCH v9 4/7] usb: misc: eud: Add driver support for SM6115 / SM4250

[Souradeep Chowdhury]

Hi Bhupesh,

On 7/18/2023 11:40 AM, Bhupesh Sharma wrote:
> Add SM6115 / SM4250 SoC EUD support in qcom_eud driver.
> 
> On some SoCs (like the SM6115 / SM4250 SoC), the mode manager
> needs to be accessed only via the secure world (through 'scm'
> calls).
> 
> Also, the enable bit inside 'tcsr_check_reg' needs to be set
> first to set the eud in 'enable' mode on these SoCs.
> 
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> ---
>   drivers/usb/misc/Kconfig    |  2 +-
>   drivers/usb/misc/qcom_eud.c | 76 ++++++++++++++++++++++++++++++++++---
>   2 files changed, 72 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
> index 99b15b77dfd57..51eb5140caa14 100644
> --- a/drivers/usb/misc/Kconfig
> +++ b/drivers/usb/misc/Kconfig
> @@ -146,7 +146,7 @@ config USB_APPLEDISPLAY
>   
>   config USB_QCOM_EUD
>   	tristate "QCOM Embedded USB Debugger(EUD) Driver"
> -	depends on ARCH_QCOM || COMPILE_TEST
> +	depends on (ARCH_QCOM && QCOM_SCM) || COMPILE_TEST
>   	select USB_ROLE_SWITCH
>   	help
>   	  This module enables support for Qualcomm Technologies, Inc.
> diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
> index 7f371ea1248c3..a5b28fc24116a 100644
> --- a/drivers/usb/misc/qcom_eud.c
> +++ b/drivers/usb/misc/qcom_eud.c
> @@ -11,9 +11,12 @@
>   #include <linux/kernel.h>
>   #include <linux/module.h>
>   #include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
>   #include <linux/platform_device.h>
>   #include <linux/slab.h>
>   #include <linux/sysfs.h>
> +#include <linux/firmware/qcom/qcom_scm.h>
>   #include <linux/usb/role.h>
>   
>   #define EUD_REG_INT1_EN_MASK	0x0024
> @@ -30,6 +33,10 @@
>   #define EUD_INT_SAFE_MODE	BIT(4)
>   #define EUD_INT_ALL		(EUD_INT_VBUS | EUD_INT_SAFE_MODE)
>   
> +#define EUD_EN2_EN		BIT(0)
> +#define EUD_EN2_DISABLE		(0)
> +#define TCSR_CHECK_EN		BIT(0)
> +
>   struct eud_chip {
>   	struct device			*dev;
>   	struct usb_role_switch		*role_sw;
> @@ -39,6 +46,7 @@ struct eud_chip {
>   	int				irq;
>   	bool				enabled;
>   	bool				usb_attached;
> +	phys_addr_t			secure_mode_mgr;
>   };
>   
>   static int enable_eud(struct eud_chip *priv)
> @@ -46,7 +54,11 @@ static int enable_eud(struct eud_chip *priv)
>   	writel(EUD_ENABLE, priv->base + EUD_REG_CSR_EUD_EN);
>   	writel(EUD_INT_VBUS | EUD_INT_SAFE_MODE,
>   			priv->base + EUD_REG_INT1_EN_MASK);
> -	writel(1, priv->mode_mgr + EUD_REG_EUD_EN2);
> +
> +	if (priv->secure_mode_mgr)
> +		qcom_scm_io_writel(priv->secure_mode_mgr + EUD_REG_EUD_EN2, EUD_EN2_EN);
> +	else
> +		writel(EUD_EN2_EN, priv->mode_mgr + EUD_REG_EUD_EN2);
>   
>   	return usb_role_switch_set_role(priv->role_sw, USB_ROLE_DEVICE);
>   }
> @@ -54,7 +66,11 @@ static int enable_eud(struct eud_chip *priv)
>   static void disable_eud(struct eud_chip *priv)
>   {
>   	writel(0, priv->base + EUD_REG_CSR_EUD_EN);
> -	writel(0, priv->mode_mgr + EUD_REG_EUD_EN2);
> +
> +	if (priv->secure_mode_mgr)
> +		qcom_scm_io_writel(priv->secure_mode_mgr + EUD_REG_EUD_EN2, EUD_EN2_DISABLE);
> +	else
> +		writel(EUD_EN2_DISABLE, priv->mode_mgr + EUD_REG_EUD_EN2);
>   }
>   
>   static ssize_t enable_show(struct device *dev,
> @@ -175,9 +191,37 @@ static void eud_role_switch_release(void *data)
>   	usb_role_switch_put(chip->role_sw);
>   }
>   
> +static int eud_find_secure_reg_addr(struct device *dev, u64 *addr)
> +{
> +	struct device_node *tcsr;
> +	struct device_node *np = dev->of_node;
> +	struct resource res;
> +	u32 offset;
> +	int ret;
> +
> +	tcsr = of_parse_phandle(np, "qcom,secure-eud-reg", 0);
> +	if (!tcsr)
> +		return 0;
> +
> +	ret = of_address_to_resource(tcsr, 0, &res);
> +	of_node_put(tcsr);
> +	if (ret)
> +		return ret;
> +
> +	ret = of_property_read_u32_index(np, "qcom,secure-eud-reg", 1, &offset);
> +	if (ret < 0)
> +		return ret;
> +
> +	*addr = res.start + offset;
> +
> +	return 0;
> +}
> +
>   static int eud_probe(struct platform_device *pdev)
>   {
>   	struct eud_chip *chip;
> +	struct resource *res;
> +	phys_addr_t tcsr_check = 0;
>   	int ret;
>   
>   	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
> @@ -200,9 +244,30 @@ static int eud_probe(struct platform_device *pdev)
>   	if (IS_ERR(chip->base))
>   		return PTR_ERR(chip->base);
>   
> -	chip->mode_mgr = devm_platform_ioremap_resource(pdev, 1);
> -	if (IS_ERR(chip->mode_mgr))
> -		return PTR_ERR(chip->mode_mgr);
> +	/*
> +	 * EUD block on a few Qualcomm SoCs needs secure register access.
> +	 * Check for the same via vendor-specific dt property.
> +	 */
> +	ret = eud_find_secure_reg_addr(&pdev->dev, &tcsr_check);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (tcsr_check) {
> +		res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +		if (!res)
> +			return dev_err_probe(chip->dev, -ENODEV,
> +					     "failed to get secure_mode_mgr reg base\n");
> +
> +		chip->secure_mode_mgr = res->start;

There are multiple instances where the addresses are being mapped from 
the dt property without using the devm version. Either we should switch 
to the later or ensure that these addresses are unmapped in the removal
path of the driver.

Thanks,
Souradeep

> +
> +		ret = qcom_scm_io_writel(tcsr_check, TCSR_CHECK_EN);
> +		if (ret)
> +			return dev_err_probe(chip->dev, ret, "failed to write tcsr check reg\n");
> +	} else {
> +		chip->mode_mgr = devm_platform_ioremap_resource(pdev, 1);
> +		if (IS_ERR(chip->mode_mgr))
> +			return PTR_ERR(chip->mode_mgr);
> +	}
>   
>   	chip->irq = platform_get_irq(pdev, 0);
>   	ret = devm_request_threaded_irq(&pdev->dev, chip->irq, handle_eud_irq,
> @@ -230,6 +295,7 @@ static void eud_remove(struct platform_device *pdev)
>   
>   static const struct of_device_id eud_dt_match[] = {
>   	{ .compatible = "qcom,sc7280-eud" },
> +	{ .compatible = "qcom,sm6115-eud" },
>   	{ }
>   };
>   MODULE_DEVICE_TABLE(of, eud_dt_match);

Re: [PATCH v9 4/7] usb: misc: eud: Add driver support for SM6115 / SM4250

[kernel test robot]

Hi Bhupesh,

kernel test robot noticed the following build errors:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus lee-mfd/for-mfd-next linus/master v6.5-rc2 next-20230718]
[cannot apply to robh/for-next lee-mfd/for-mfd-fixes]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Bhupesh-Sharma/dt-bindings-mfd-qcom-tcsr-Add-the-compatible-for-SM6115/20230718-190223
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20230718061052.1332993-5-bhupesh.sharma%40linaro.org
patch subject: [PATCH v9 4/7] usb: misc: eud: Add driver support for SM6115 / SM4250
config: arm-allmodconfig (https://download.01.org/0day-ci/archive/20230719/202307191045.FMNOhyay-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230719/202307191045.FMNOhyay-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307191045.FMNOhyay-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/usb/misc/qcom_eud.c: In function 'eud_probe':
>> drivers/usb/misc/qcom_eud.c:251:52: error: passing argument 2 of 'eud_find_secure_reg_addr' from incompatible pointer type [-Werror=incompatible-pointer-types]
     251 |         ret = eud_find_secure_reg_addr(&pdev->dev, &tcsr_check);
         |                                                    ^~~~~~~~~~~
         |                                                    |
         |                                                    phys_addr_t * {aka unsigned int *}
   drivers/usb/misc/qcom_eud.c:194:62: note: expected 'u64 *' {aka 'long long unsigned int *'} but argument is of type 'phys_addr_t *' {aka 'unsigned int *'}
     194 | static int eud_find_secure_reg_addr(struct device *dev, u64 *addr)
         |                                                         ~~~~~^~~~
   cc1: some warnings being treated as errors

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for SM_GCC_8350
   Depends on [n]: COMMON_CLK [=y] && COMMON_CLK_QCOM [=m] && (ARM64 || COMPILE_TEST [=n])
   Selected by [m]:
   - SM_VIDEOCC_8350 [=m] && COMMON_CLK [=y] && COMMON_CLK_QCOM [=m]
   WARNING: unmet direct dependencies detected for SM_GCC_8450
   Depends on [n]: COMMON_CLK [=y] && COMMON_CLK_QCOM [=m] && (ARM64 || COMPILE_TEST [=n])
   Selected by [m]:
   - SM_GPUCC_8450 [=m] && COMMON_CLK [=y] && COMMON_CLK_QCOM [=m]
   - SM_VIDEOCC_8450 [=m] && COMMON_CLK [=y] && COMMON_CLK_QCOM [=m]
   WARNING: unmet direct dependencies detected for SM_GCC_8550
   Depends on [n]: COMMON_CLK [=y] && COMMON_CLK_QCOM [=m] && (ARM64 || COMPILE_TEST [=n])
   Selected by [m]:
   - SM_GPUCC_8550 [=m] && COMMON_CLK [=y] && COMMON_CLK_QCOM [=m]
   - SM_VIDEOCC_8550 [=m] && COMMON_CLK [=y] && COMMON_CLK_QCOM [=m]


vim +/eud_find_secure_reg_addr +251 drivers/usb/misc/qcom_eud.c

   219	
   220	static int eud_probe(struct platform_device *pdev)
   221	{
   222		struct eud_chip *chip;
   223		struct resource *res;
   224		phys_addr_t tcsr_check = 0;
   225		int ret;
   226	
   227		chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
   228		if (!chip)
   229			return -ENOMEM;
   230	
   231		chip->dev = &pdev->dev;
   232	
   233		chip->role_sw = usb_role_switch_get(&pdev->dev);
   234		if (IS_ERR(chip->role_sw))
   235			return dev_err_probe(chip->dev, PTR_ERR(chip->role_sw),
   236						"failed to get role switch\n");
   237	
   238		ret = devm_add_action_or_reset(chip->dev, eud_role_switch_release, chip);
   239		if (ret)
   240			return dev_err_probe(chip->dev, ret,
   241					"failed to add role switch release action\n");
   242	
   243		chip->base = devm_platform_ioremap_resource(pdev, 0);
   244		if (IS_ERR(chip->base))
   245			return PTR_ERR(chip->base);
   246	
   247		/*
   248		 * EUD block on a few Qualcomm SoCs needs secure register access.
   249		 * Check for the same via vendor-specific dt property.
   250		 */
 > 251		ret = eud_find_secure_reg_addr(&pdev->dev, &tcsr_check);
   252		if (ret < 0)
   253			return ret;
   254	
   255		if (tcsr_check) {
   256			res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
   257			if (!res)
   258				return dev_err_probe(chip->dev, -ENODEV,
   259						     "failed to get secure_mode_mgr reg base\n");
   260	
   261			chip->secure_mode_mgr = res->start;
   262	
   263			ret = qcom_scm_io_writel(tcsr_check, TCSR_CHECK_EN);
   264			if (ret)
   265				return dev_err_probe(chip->dev, ret, "failed to write tcsr check reg\n");
   266		} else {
   267			chip->mode_mgr = devm_platform_ioremap_resource(pdev, 1);
   268			if (IS_ERR(chip->mode_mgr))
   269				return PTR_ERR(chip->mode_mgr);
   270		}
   271	
   272		chip->irq = platform_get_irq(pdev, 0);
   273		ret = devm_request_threaded_irq(&pdev->dev, chip->irq, handle_eud_irq,
   274				handle_eud_irq_thread, IRQF_ONESHOT, NULL, chip);
   275		if (ret)
   276			return dev_err_probe(chip->dev, ret, "failed to allocate irq\n");
   277	
   278		enable_irq_wake(chip->irq);
   279	
   280		platform_set_drvdata(pdev, chip);
   281	
   282		return 0;
   283	}
   284	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v9 5/7] arm64: dts: qcom: sm6115: Add tcsr syscon node

[Konrad Dybcio]

On 18.07.2023 08:10, Bhupesh Sharma wrote:
> Add the TCSR syscon dt node for SM6115 / SM4250 SoC.
> 
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad