* [PATCH V3 1/7] dt-bindings: Add the yaml bindings for EUD
2022-01-04 11:58 [PATCH V3 0/7] Add Embedded USB Debugger (EUD) driver Souradeep Chowdhury
@ 2022-01-04 11:58 ` Souradeep Chowdhury
2022-01-12 1:08 ` Rob Herring
2022-01-04 11:58 ` [PATCH V3 2/7] dt-bindings: connector: Add property for EUD type-C connector Souradeep Chowdhury
` (5 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Souradeep Chowdhury @ 2022-01-04 11:58 UTC (permalink / raw)
To: linux-arm-msm, linux-usb, devicetree, pure.logic, bjorn.andersson, greg
Cc: linux-kernel, tsoni, psodagud, satyap, pheragu, rnayak, sibis,
saiprakash.ranjan, quic_schowdhu
Documentation for Embedded USB Debugger(EUD) device tree
bindings in yaml format.
Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
---
.../devicetree/bindings/soc/qcom/qcom,eud.yaml | 50 ++++++++++++++++++++++
1 file changed, 50 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
new file mode 100644
index 0000000..3523932
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/soc/qcom/qcom,eud.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Qualcomm Embedded USB Debugger
+
+maintainers:
+ - Souradeep Chowdhury <quic_schowdhu@quicinc.com>
+
+description:
+ This binding is used to describe the Qualcomm Embedded USB Debugger, which is
+ mini USB-hub implemented on chip to support USB-based debug capabilities.
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - qcom,sc7280-eud
+ - const: qcom,eud
+
+ reg:
+ items:
+ - description: EUD Base Register Region
+ - description: EUD Mode Manager Register
+
+ interrupts:
+ description:
+ EUD interrupt
+
+ port:
+ description:
+ This port is to be attached to the endpoint of the connector child of
+ DWC3 controller node. The controller has the "usb-role-switch" property.
+ $ref: /schemas/graph.yaml#/properties/port
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ eud@88e0000 {
+ compatible = "qcom,sc7280-eud","qcom,eud";
+ reg = <0x88e0000 0x2000>,
+ <0x88e2000 0x1000>;
+ };
--
2.7.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH V3 1/7] dt-bindings: Add the yaml bindings for EUD
2022-01-04 11:58 ` [PATCH V3 1/7] dt-bindings: Add the yaml bindings for EUD Souradeep Chowdhury
@ 2022-01-12 1:08 ` Rob Herring
2022-01-17 5:12 ` Souradeep Chowdhury
0 siblings, 1 reply; 20+ messages in thread
From: Rob Herring @ 2022-01-12 1:08 UTC (permalink / raw)
To: Souradeep Chowdhury
Cc: linux-arm-msm, linux-usb, devicetree, pure.logic,
bjorn.andersson, greg, linux-kernel, tsoni, psodagud, satyap,
pheragu, rnayak, sibis, saiprakash.ranjan
On Tue, Jan 04, 2022 at 05:28:14PM +0530, Souradeep Chowdhury wrote:
> Documentation for Embedded USB Debugger(EUD) device tree
> bindings in yaml format.
I agree with Bjorn's comments.
>
> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> ---
> .../devicetree/bindings/soc/qcom/qcom,eud.yaml | 50 ++++++++++++++++++++++
> 1 file changed, 50 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
>
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
> new file mode 100644
> index 0000000..3523932
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/soc/qcom/qcom,eud.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Qualcomm Embedded USB Debugger
> +
> +maintainers:
> + - Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> +
> +description:
> + This binding is used to describe the Qualcomm Embedded USB Debugger, which is
> + mini USB-hub implemented on chip to support USB-based debug capabilities.
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - qcom,sc7280-eud
> + - const: qcom,eud
> +
> + reg:
> + items:
> + - description: EUD Base Register Region
> + - description: EUD Mode Manager Register
> +
> + interrupts:
> + description:
> + EUD interrupt
Just 'maxItems: 1' is sufficient here.
> +
> + port:
Based on Bjorn's comment, you will need 2 port nodes.
> + description:
> + This port is to be attached to the endpoint of the connector child of
> + DWC3 controller node. The controller has the "usb-role-switch" property.
> + $ref: /schemas/graph.yaml#/properties/port
> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + eud@88e0000 {
> + compatible = "qcom,sc7280-eud","qcom,eud";
> + reg = <0x88e0000 0x2000>,
> + <0x88e2000 0x1000>;
Are the ports really optional?
> + };
> --
> 2.7.4
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH V3 1/7] dt-bindings: Add the yaml bindings for EUD
2022-01-12 1:08 ` Rob Herring
@ 2022-01-17 5:12 ` Souradeep Chowdhury
0 siblings, 0 replies; 20+ messages in thread
From: Souradeep Chowdhury @ 2022-01-17 5:12 UTC (permalink / raw)
To: Rob Herring
Cc: linux-arm-msm, linux-usb, devicetree, pure.logic,
bjorn.andersson, greg, linux-kernel, tsoni, psodagud, satyap,
pheragu, rnayak, sibis, saiprakash.ranjan
On 1/12/2022 6:38 AM, Rob Herring wrote:
> On Tue, Jan 04, 2022 at 05:28:14PM +0530, Souradeep Chowdhury wrote:
>> Documentation for Embedded USB Debugger(EUD) device tree
>> bindings in yaml format.
> I agree with Bjorn's comments.
Ack
>
>> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>> ---
>> .../devicetree/bindings/soc/qcom/qcom,eud.yaml | 50 ++++++++++++++++++++++
>> 1 file changed, 50 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
>> new file mode 100644
>> index 0000000..3523932
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml
>> @@ -0,0 +1,50 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/soc/qcom/qcom,eud.yaml#"
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>> +
>> +title: Qualcomm Embedded USB Debugger
>> +
>> +maintainers:
>> + - Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>> +
>> +description:
>> + This binding is used to describe the Qualcomm Embedded USB Debugger, which is
>> + mini USB-hub implemented on chip to support USB-based debug capabilities.
>> +
>> +properties:
>> + compatible:
>> + items:
>> + - enum:
>> + - qcom,sc7280-eud
>> + - const: qcom,eud
>> +
>> + reg:
>> + items:
>> + - description: EUD Base Register Region
>> + - description: EUD Mode Manager Register
>> +
>> + interrupts:
>> + description:
>> + EUD interrupt
> Just 'maxItems: 1' is sufficient here.
Ack
>
>> +
>> + port:
> Based on Bjorn's comment, you will need 2 port nodes.
Ack
>
>> + description:
>> + This port is to be attached to the endpoint of the connector child of
>> + DWC3 controller node. The controller has the "usb-role-switch" property.
>> + $ref: /schemas/graph.yaml#/properties/port
>> +
>> +required:
>> + - compatible
>> + - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + eud@88e0000 {
>> + compatible = "qcom,sc7280-eud","qcom,eud";
>> + reg = <0x88e0000 0x2000>,
>> + <0x88e2000 0x1000>;
> Are the ports really optional?
Ack. Will make them mandatory.
>
>> + };
>> --
>> 2.7.4
>>
>>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH V3 2/7] dt-bindings: connector: Add property for EUD type-C connector
2022-01-04 11:58 [PATCH V3 0/7] Add Embedded USB Debugger (EUD) driver Souradeep Chowdhury
2022-01-04 11:58 ` [PATCH V3 1/7] dt-bindings: Add the yaml bindings for EUD Souradeep Chowdhury
@ 2022-01-04 11:58 ` Souradeep Chowdhury
2022-01-07 0:54 ` Bjorn Andersson
2022-01-04 11:58 ` [PATCH V3 3/7] bindings: usb: dwc3: Update dwc3 properties for EUD connector Souradeep Chowdhury
` (4 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Souradeep Chowdhury @ 2022-01-04 11:58 UTC (permalink / raw)
To: linux-arm-msm, linux-usb, devicetree, pure.logic, bjorn.andersson, greg
Cc: linux-kernel, tsoni, psodagud, satyap, pheragu, rnayak, sibis,
saiprakash.ranjan, quic_schowdhu
Added the property for EUD(Embedded USB Debugger) connector. EUD
is a mini usb hub with debug and trace capabilities and it has a
type C connector attached to it for role-switching. This connector
is attached to EUD via port.
Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
---
Documentation/devicetree/bindings/connector/usb-connector.yaml | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
index 7eb8659..417d39a 100644
--- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
+++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
@@ -30,6 +30,10 @@ properties:
- const: samsung,usb-connector-11pin
- const: usb-b-connector
+ - items:
+ - const: qcom,usb-connector-eud
+ - const: usb-c-connector
+
label:
description: Symbolic name for the connector.
@@ -179,7 +183,8 @@ properties:
properties:
port@0:
$ref: /schemas/graph.yaml#/properties/port
- description: High Speed (HS), present in all connectors.
+ description: High Speed (HS), present in all connectors. Also used as a
+ port to connect QCOM Embedded USB Debugger(EUD).
port@1:
$ref: /schemas/graph.yaml#/properties/port
--
2.7.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH V3 2/7] dt-bindings: connector: Add property for EUD type-C connector
2022-01-04 11:58 ` [PATCH V3 2/7] dt-bindings: connector: Add property for EUD type-C connector Souradeep Chowdhury
@ 2022-01-07 0:54 ` Bjorn Andersson
2022-01-17 5:08 ` Souradeep Chowdhury
0 siblings, 1 reply; 20+ messages in thread
From: Bjorn Andersson @ 2022-01-07 0:54 UTC (permalink / raw)
To: Souradeep Chowdhury
Cc: linux-arm-msm, linux-usb, devicetree, pure.logic, greg,
linux-kernel, tsoni, psodagud, satyap, pheragu, rnayak, sibis,
saiprakash.ranjan
On Tue 04 Jan 03:58 PST 2022, Souradeep Chowdhury wrote:
> Added the property for EUD(Embedded USB Debugger) connector. EUD
> is a mini usb hub with debug and trace capabilities and it has a
> type C connector attached to it for role-switching. This connector
> is attached to EUD via port.
>
Per my feedback on the dts patch, I don't think this binding is needed.
Regards,
Bjorn
> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> ---
> Documentation/devicetree/bindings/connector/usb-connector.yaml | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> index 7eb8659..417d39a 100644
> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> @@ -30,6 +30,10 @@ properties:
> - const: samsung,usb-connector-11pin
> - const: usb-b-connector
>
> + - items:
> + - const: qcom,usb-connector-eud
> + - const: usb-c-connector
> +
> label:
> description: Symbolic name for the connector.
>
> @@ -179,7 +183,8 @@ properties:
> properties:
> port@0:
> $ref: /schemas/graph.yaml#/properties/port
> - description: High Speed (HS), present in all connectors.
> + description: High Speed (HS), present in all connectors. Also used as a
> + port to connect QCOM Embedded USB Debugger(EUD).
>
> port@1:
> $ref: /schemas/graph.yaml#/properties/port
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH V3 2/7] dt-bindings: connector: Add property for EUD type-C connector
2022-01-07 0:54 ` Bjorn Andersson
@ 2022-01-17 5:08 ` Souradeep Chowdhury
0 siblings, 0 replies; 20+ messages in thread
From: Souradeep Chowdhury @ 2022-01-17 5:08 UTC (permalink / raw)
To: Bjorn Andersson
Cc: linux-arm-msm, linux-usb, devicetree, pure.logic, greg,
linux-kernel, tsoni, psodagud, satyap, pheragu, rnayak, sibis,
saiprakash.ranjan
On 1/7/2022 6:24 AM, Bjorn Andersson wrote:
> On Tue 04 Jan 03:58 PST 2022, Souradeep Chowdhury wrote:
>
>> Added the property for EUD(Embedded USB Debugger) connector. EUD
>> is a mini usb hub with debug and trace capabilities and it has a
>> type C connector attached to it for role-switching. This connector
>> is attached to EUD via port.
>>
> Per my feedback on the dts patch, I don't think this binding is needed.
Ack
>
> Regards,
> Bjorn
>
>> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>> ---
>> Documentation/devicetree/bindings/connector/usb-connector.yaml | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> index 7eb8659..417d39a 100644
>> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> @@ -30,6 +30,10 @@ properties:
>> - const: samsung,usb-connector-11pin
>> - const: usb-b-connector
>>
>> + - items:
>> + - const: qcom,usb-connector-eud
>> + - const: usb-c-connector
>> +
>> label:
>> description: Symbolic name for the connector.
>>
>> @@ -179,7 +183,8 @@ properties:
>> properties:
>> port@0:
>> $ref: /schemas/graph.yaml#/properties/port
>> - description: High Speed (HS), present in all connectors.
>> + description: High Speed (HS), present in all connectors. Also used as a
>> + port to connect QCOM Embedded USB Debugger(EUD).
>>
>> port@1:
>> $ref: /schemas/graph.yaml#/properties/port
>> --
>> 2.7.4
>>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH V3 3/7] bindings: usb: dwc3: Update dwc3 properties for EUD connector
2022-01-04 11:58 [PATCH V3 0/7] Add Embedded USB Debugger (EUD) driver Souradeep Chowdhury
2022-01-04 11:58 ` [PATCH V3 1/7] dt-bindings: Add the yaml bindings for EUD Souradeep Chowdhury
2022-01-04 11:58 ` [PATCH V3 2/7] dt-bindings: connector: Add property for EUD type-C connector Souradeep Chowdhury
@ 2022-01-04 11:58 ` Souradeep Chowdhury
2022-01-07 0:56 ` Bjorn Andersson
2022-01-04 11:58 ` [PATCH V3 4/7] soc: qcom: eud: Add driver support for Embedded USB Debugger(EUD) Souradeep Chowdhury
` (3 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Souradeep Chowdhury @ 2022-01-04 11:58 UTC (permalink / raw)
To: linux-arm-msm, linux-usb, devicetree, pure.logic, bjorn.andersson, greg
Cc: linux-kernel, tsoni, psodagud, satyap, pheragu, rnayak, sibis,
saiprakash.ranjan, quic_schowdhu
Add the connector property for dwc3 node. This connector can
be used to role-switch the controller from device to host and
vice versa.
Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
---
Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
index 078fb78..9382168 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -318,6 +318,12 @@ properties:
items:
enum: [1, 4, 8, 16, 32, 64, 128, 256]
+ connector:
+ type: object
+ $ref: /connector/usb-connector.yaml#
+ description:
+ Connector for dual role switch, especially for "eud-usb-c-connector"
+
unevaluatedProperties: false
required:
--
2.7.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH V3 3/7] bindings: usb: dwc3: Update dwc3 properties for EUD connector
2022-01-04 11:58 ` [PATCH V3 3/7] bindings: usb: dwc3: Update dwc3 properties for EUD connector Souradeep Chowdhury
@ 2022-01-07 0:56 ` Bjorn Andersson
2022-01-17 5:09 ` Souradeep Chowdhury
0 siblings, 1 reply; 20+ messages in thread
From: Bjorn Andersson @ 2022-01-07 0:56 UTC (permalink / raw)
To: Souradeep Chowdhury
Cc: linux-arm-msm, linux-usb, devicetree, pure.logic, greg,
linux-kernel, tsoni, psodagud, satyap, pheragu, rnayak, sibis,
saiprakash.ranjan
On Tue 04 Jan 03:58 PST 2022, Souradeep Chowdhury wrote:
> Add the connector property for dwc3 node. This connector can
> be used to role-switch the controller from device to host and
> vice versa.
>
> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> ---
> Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> index 078fb78..9382168 100644
> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> @@ -318,6 +318,12 @@ properties:
> items:
> enum: [1, 4, 8, 16, 32, 64, 128, 256]
>
> + connector:
The dwc3 isn't the connector, so I think you should put a ports {} in
the dwc3 and link that to the connector that is described elsewhere.
Regards,
Bjorn
> + type: object
> + $ref: /connector/usb-connector.yaml#
> + description:
> + Connector for dual role switch, especially for "eud-usb-c-connector"
> +
> unevaluatedProperties: false
>
> required:
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH V3 3/7] bindings: usb: dwc3: Update dwc3 properties for EUD connector
2022-01-07 0:56 ` Bjorn Andersson
@ 2022-01-17 5:09 ` Souradeep Chowdhury
0 siblings, 0 replies; 20+ messages in thread
From: Souradeep Chowdhury @ 2022-01-17 5:09 UTC (permalink / raw)
To: Bjorn Andersson
Cc: linux-arm-msm, linux-usb, devicetree, pure.logic, greg,
linux-kernel, tsoni, psodagud, satyap, pheragu, rnayak, sibis,
saiprakash.ranjan
On 1/7/2022 6:26 AM, Bjorn Andersson wrote:
> On Tue 04 Jan 03:58 PST 2022, Souradeep Chowdhury wrote:
>
>> Add the connector property for dwc3 node. This connector can
>> be used to role-switch the controller from device to host and
>> vice versa.
>>
>> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>> ---
>> Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> index 078fb78..9382168 100644
>> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> @@ -318,6 +318,12 @@ properties:
>> items:
>> enum: [1, 4, 8, 16, 32, 64, 128, 256]
>>
>> + connector:
> The dwc3 isn't the connector, so I think you should put a ports {} in
> the dwc3 and link that to the connector that is described elsewhere.
Ack
> Regards,
> Bjorn
>
>> + type: object
>> + $ref: /connector/usb-connector.yaml#
>> + description:
>> + Connector for dual role switch, especially for "eud-usb-c-connector"
>> +
>> unevaluatedProperties: false
>>
>> required:
>> --
>> 2.7.4
>>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH V3 4/7] soc: qcom: eud: Add driver support for Embedded USB Debugger(EUD)
2022-01-04 11:58 [PATCH V3 0/7] Add Embedded USB Debugger (EUD) driver Souradeep Chowdhury
` (2 preceding siblings ...)
2022-01-04 11:58 ` [PATCH V3 3/7] bindings: usb: dwc3: Update dwc3 properties for EUD connector Souradeep Chowdhury
@ 2022-01-04 11:58 ` Souradeep Chowdhury
2022-01-07 1:19 ` Bjorn Andersson
2022-01-04 11:58 ` [PATCH V3 5/7] arm64: dts: qcom: sc7280: Add EUD dt node and dwc3 connector Souradeep Chowdhury
` (2 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Souradeep Chowdhury @ 2022-01-04 11:58 UTC (permalink / raw)
To: linux-arm-msm, linux-usb, devicetree, pure.logic, bjorn.andersson, greg
Cc: linux-kernel, tsoni, psodagud, satyap, pheragu, rnayak, sibis,
saiprakash.ranjan, quic_schowdhu
Add support for control peripheral of EUD (Embedded USB Debugger) to
listen to events such as USB attach/detach, pet EUD to indicate software
is functional.Reusing the platform device kobj, sysfs entry 'enable' is
created to enable or disable EUD.
To enable the eud the following needs to be done
echo 1 > /sys/bus/platform/.../enable
To disable eud, following is the command
echo 0 > /sys/bus/platform/.../enable
Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
---
Documentation/ABI/testing/sysfs-driver-eud | 9 +
drivers/soc/qcom/Kconfig | 10 ++
drivers/soc/qcom/Makefile | 1 +
drivers/soc/qcom/qcom_eud.c | 260 +++++++++++++++++++++++++++++
4 files changed, 280 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-eud
create mode 100644 drivers/soc/qcom/qcom_eud.c
diff --git a/Documentation/ABI/testing/sysfs-driver-eud b/Documentation/ABI/testing/sysfs-driver-eud
new file mode 100644
index 0000000..2381552
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-eud
@@ -0,0 +1,9 @@
+What: /sys/bus/platform/drivers/eud/.../enable
+Date: January 2022
+Contact: Souradeep Chowdhury <schowdhu@codeaurora.org>
+Description:
+ The Enable/Disable sysfs interface for Embedded
+ USB Debugger(EUD). This enables and disables the
+ EUD based on a 1 or a 0 value. By enabling EUD,
+ the user is able to activate the mini-usb hub of
+ EUD for debug and trace capabilities.
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 79b568f..a4db41b 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -42,6 +42,16 @@ config QCOM_CPR
To compile this driver as a module, choose M here: the module will
be called qcom-cpr
+config QCOM_EUD
+ tristate "QCOM Embedded USB Debugger(EUD) Driver"
+ select USB_ROLE_SWITCH
+ help
+ This module enables support for Qualcomm Technologies, Inc.
+ Embedded USB Debugger (EUD). The EUD is a control peripheral
+ which reports VBUS attach/detach events and has USB-based
+ debug and trace capabilities. On selecting m, the module name
+ that is built is qcom_eud.ko
+
config QCOM_GENI_SE
tristate "QCOM GENI Serial Engine Driver"
depends on ARCH_QCOM || COMPILE_TEST
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index ad675a6..3331a40 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_QCOM_AOSS_QMP) += qcom_aoss.o
obj-$(CONFIG_QCOM_GENI_SE) += qcom-geni-se.o
obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o
obj-$(CONFIG_QCOM_CPR) += cpr.o
+obj-$(CONFIG_QCOM_EUD) += qcom_eud.o
obj-$(CONFIG_QCOM_GSBI) += qcom_gsbi.o
obj-$(CONFIG_QCOM_MDT_LOADER) += mdt_loader.o
obj-$(CONFIG_QCOM_OCMEM) += ocmem.o
diff --git a/drivers/soc/qcom/qcom_eud.c b/drivers/soc/qcom/qcom_eud.c
new file mode 100644
index 0000000..7c4cc7d
--- /dev/null
+++ b/drivers/soc/qcom/qcom_eud.c
@@ -0,0 +1,260 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2015-2021, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/bitops.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/sysfs.h>
+#include <linux/usb/role.h>
+
+#define EUD_REG_INT1_EN_MASK 0x0024
+#define EUD_REG_INT_STATUS_1 0x0044
+#define EUD_REG_CTL_OUT_1 0x0074
+#define EUD_REG_VBUS_INT_CLR 0x0080
+#define EUD_REG_CSR_EUD_EN 0x1014
+#define EUD_REG_SW_ATTACH_DET 0x1018
+#define EUD_REG_EUD_EN2 0x0000
+
+#define EUD_ENABLE BIT(0)
+#define EUD_INT_PET_EUD BIT(0)
+#define EUD_INT_VBUS BIT(2)
+#define EUD_INT_SAFE_MODE BIT(4)
+#define EUD_INT_ALL (EUD_INT_VBUS|EUD_INT_SAFE_MODE)
+
+struct eud_chip {
+ struct device *dev;
+ struct usb_role_switch *role_sw;
+ void __iomem *eud_reg_base;
+ void __iomem *eud_mode_mgr2_phys_base;
+ unsigned int int_status;
+ int eud_irq;
+ bool enable;
+ bool usb_attach;
+
+};
+
+static int enable_eud(struct eud_chip *priv)
+{
+ writel(EUD_ENABLE, priv->eud_reg_base + EUD_REG_CSR_EUD_EN);
+ writel(EUD_INT_VBUS | EUD_INT_SAFE_MODE,
+ priv->eud_reg_base + EUD_REG_INT1_EN_MASK);
+ writel(1, priv->eud_mode_mgr2_phys_base + EUD_REG_EUD_EN2);
+
+ return usb_role_switch_set_role(priv->role_sw, USB_ROLE_DEVICE);
+}
+
+static void disable_eud(struct eud_chip *priv)
+{
+ writel(0, priv->eud_reg_base + EUD_REG_CSR_EUD_EN);
+ writel(0, priv->eud_mode_mgr2_phys_base + EUD_REG_EUD_EN2);
+}
+
+static ssize_t enable_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct eud_chip *chip = dev_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%d\n", chip->enable);
+}
+
+static ssize_t enable_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct eud_chip *chip = dev_get_drvdata(dev);
+ bool enable;
+ int ret;
+
+ if (kstrtobool(buf, &enable))
+ return -EINVAL;
+
+ if (enable) {
+ ret = enable_eud(chip);
+ if (!ret)
+ chip->enable = enable;
+ } else {
+ disable_eud(chip);
+ }
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(enable);
+
+static struct attribute *eud_attrs[] = {
+ &dev_attr_enable.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(eud);
+
+static void usb_attach_detach(struct eud_chip *chip)
+{
+ u32 reg;
+
+ /* read ctl_out_1[4] to find USB attach or detach event */
+ reg = readl(chip->eud_reg_base + EUD_REG_CTL_OUT_1);
+ if (reg & EUD_INT_SAFE_MODE)
+ chip->usb_attach = true;
+ else
+ chip->usb_attach = false;
+
+ /* set and clear vbus_int_clr[0] to clear interrupt */
+ writel(BIT(0), chip->eud_reg_base + EUD_REG_VBUS_INT_CLR);
+ writel(0, chip->eud_reg_base + EUD_REG_VBUS_INT_CLR);
+}
+
+static void pet_eud(struct eud_chip *chip)
+{
+ u32 reg;
+ int ret;
+
+ /* read sw_attach_det[0] to find attach/detach event */
+ reg = readl(chip->eud_reg_base + EUD_REG_SW_ATTACH_DET);
+ if (reg & EUD_INT_PET_EUD) {
+ /* Detach & Attach pet for EUD */
+ writel(0, chip->eud_reg_base + EUD_REG_SW_ATTACH_DET);
+ /* Delay to make sure detach pet is done before attach pet */
+ ret = readl_poll_timeout(chip->eud_reg_base + EUD_REG_SW_ATTACH_DET,
+ reg, (reg == 0), 1, 100);
+ if (ret) {
+ dev_err(chip->dev, "Detach pet failed\n");
+ return;
+ }
+
+ writel(EUD_INT_PET_EUD, chip->eud_reg_base +
+ EUD_REG_SW_ATTACH_DET);
+ } else {
+ /* Attach pet for EUD */
+ writel(EUD_INT_PET_EUD, chip->eud_reg_base +
+ EUD_REG_SW_ATTACH_DET);
+ }
+}
+
+static irqreturn_t handle_eud_irq(int irq, void *data)
+{
+ struct eud_chip *chip = data;
+ u32 reg;
+
+ /* read status register and find out which interrupt triggered */
+ reg = readl(chip->eud_reg_base + EUD_REG_INT_STATUS_1);
+ switch (reg & EUD_INT_ALL) {
+ case EUD_INT_VBUS:
+ chip->int_status = EUD_INT_VBUS;
+ usb_attach_detach(chip);
+ return IRQ_WAKE_THREAD;
+ case EUD_INT_SAFE_MODE:
+ pet_eud(chip);
+ break;
+ default:
+ return IRQ_NONE;
+ }
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t handle_eud_irq_thread(int irq, void *data)
+{
+ struct eud_chip *chip = data;
+ int ret;
+
+ if (chip->int_status == EUD_INT_VBUS) {
+ if (chip->usb_attach)
+ ret = usb_role_switch_set_role(chip->role_sw, USB_ROLE_DEVICE);
+ else
+ ret = usb_role_switch_set_role(chip->role_sw, USB_ROLE_HOST);
+ if (ret)
+ dev_err(chip->dev, "failed to set role switch\n");
+ }
+
+ return IRQ_HANDLED;
+}
+
+static int eud_probe(struct platform_device *pdev)
+{
+ struct eud_chip *chip;
+ struct fwnode_handle *fwnode = pdev->dev.fwnode, *ep, *connector;
+ int ret;
+
+ chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
+ if (!chip)
+ return -ENOMEM;
+
+ chip->dev = &pdev->dev;
+
+ ep = fwnode_graph_get_next_endpoint(fwnode, NULL);
+ if (!ep)
+ return -ENODEV;
+
+ connector = fwnode_graph_get_remote_port_parent(ep);
+ fwnode_handle_put(ep);
+ if (!connector)
+ return -ENODEV;
+
+ chip->role_sw = fwnode_usb_role_switch_get(connector);
+ if (IS_ERR(chip->role_sw)) {
+ return dev_err_probe(chip->dev, PTR_ERR(chip->role_sw),
+ "failed to get role switch\n");
+ }
+
+ chip->eud_reg_base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(chip->eud_reg_base))
+ return PTR_ERR(chip->eud_reg_base);
+
+ chip->eud_mode_mgr2_phys_base = devm_platform_ioremap_resource(pdev, 1);
+ if (IS_ERR(chip->eud_mode_mgr2_phys_base))
+ return PTR_ERR(chip->eud_mode_mgr2_phys_base);
+
+ chip->eud_irq = platform_get_irq(pdev, 0);
+ ret = devm_request_threaded_irq(&pdev->dev, chip->eud_irq, handle_eud_irq,
+ handle_eud_irq_thread, IRQF_ONESHOT, NULL, chip);
+ if (ret)
+ return dev_err_probe(chip->dev, ret, "failed to allocate irq\n");
+
+ enable_irq_wake(chip->eud_irq);
+
+ platform_set_drvdata(pdev, chip);
+
+ return 0;
+}
+
+static int eud_remove(struct platform_device *pdev)
+{
+ struct eud_chip *chip = platform_get_drvdata(pdev);
+
+ if (chip->enable)
+ disable_eud(chip);
+
+ device_init_wakeup(&pdev->dev, false);
+ disable_irq_wake(chip->eud_irq);
+
+ return 0;
+}
+
+static const struct of_device_id eud_dt_match[] = {
+ { .compatible = "qcom,sc7280-eud" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, eud_dt_match);
+
+static struct platform_driver eud_driver = {
+ .probe = eud_probe,
+ .remove = eud_remove,
+ .driver = {
+ .name = "eud",
+ .dev_groups = eud_groups,
+ .of_match_table = eud_dt_match,
+ },
+};
+module_platform_driver(eud_driver);
+
+MODULE_DESCRIPTION("QTI EUD driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH V3 4/7] soc: qcom: eud: Add driver support for Embedded USB Debugger(EUD)
2022-01-04 11:58 ` [PATCH V3 4/7] soc: qcom: eud: Add driver support for Embedded USB Debugger(EUD) Souradeep Chowdhury
@ 2022-01-07 1:19 ` Bjorn Andersson
2022-01-17 4:58 ` Souradeep Chowdhury
0 siblings, 1 reply; 20+ messages in thread
From: Bjorn Andersson @ 2022-01-07 1:19 UTC (permalink / raw)
To: Souradeep Chowdhury
Cc: linux-arm-msm, linux-usb, devicetree, pure.logic, greg,
linux-kernel, tsoni, psodagud, satyap, pheragu, rnayak, sibis,
saiprakash.ranjan
On Tue 04 Jan 03:58 PST 2022, Souradeep Chowdhury wrote:
> Add support for control peripheral of EUD (Embedded USB Debugger) to
> listen to events such as USB attach/detach, pet EUD to indicate software
> is functional.Reusing the platform device kobj, sysfs entry 'enable' is
> created to enable or disable EUD.
>
> To enable the eud the following needs to be done
> echo 1 > /sys/bus/platform/.../enable
>
> To disable eud, following is the command
> echo 0 > /sys/bus/platform/.../enable
>
> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> ---
> Documentation/ABI/testing/sysfs-driver-eud | 9 +
> drivers/soc/qcom/Kconfig | 10 ++
> drivers/soc/qcom/Makefile | 1 +
> drivers/soc/qcom/qcom_eud.c | 260 +++++++++++++++++++++++++++++
> 4 files changed, 280 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-driver-eud
> create mode 100644 drivers/soc/qcom/qcom_eud.c
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-eud b/Documentation/ABI/testing/sysfs-driver-eud
> new file mode 100644
> index 0000000..2381552
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-eud
> @@ -0,0 +1,9 @@
> +What: /sys/bus/platform/drivers/eud/.../enable
> +Date: January 2022
> +Contact: Souradeep Chowdhury <schowdhu@codeaurora.org>
> +Description:
> + The Enable/Disable sysfs interface for Embedded
> + USB Debugger(EUD). This enables and disables the
> + EUD based on a 1 or a 0 value. By enabling EUD,
> + the user is able to activate the mini-usb hub of
> + EUD for debug and trace capabilities.
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index 79b568f..a4db41b 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -42,6 +42,16 @@ config QCOM_CPR
> To compile this driver as a module, choose M here: the module will
> be called qcom-cpr
>
> +config QCOM_EUD
> + tristate "QCOM Embedded USB Debugger(EUD) Driver"
> + select USB_ROLE_SWITCH
> + help
> + This module enables support for Qualcomm Technologies, Inc.
> + Embedded USB Debugger (EUD). The EUD is a control peripheral
> + which reports VBUS attach/detach events and has USB-based
> + debug and trace capabilities. On selecting m, the module name
> + that is built is qcom_eud.ko
> +
> config QCOM_GENI_SE
> tristate "QCOM GENI Serial Engine Driver"
> depends on ARCH_QCOM || COMPILE_TEST
> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> index ad675a6..3331a40 100644
> --- a/drivers/soc/qcom/Makefile
> +++ b/drivers/soc/qcom/Makefile
> @@ -4,6 +4,7 @@ obj-$(CONFIG_QCOM_AOSS_QMP) += qcom_aoss.o
> obj-$(CONFIG_QCOM_GENI_SE) += qcom-geni-se.o
> obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o
> obj-$(CONFIG_QCOM_CPR) += cpr.o
> +obj-$(CONFIG_QCOM_EUD) += qcom_eud.o
> obj-$(CONFIG_QCOM_GSBI) += qcom_gsbi.o
> obj-$(CONFIG_QCOM_MDT_LOADER) += mdt_loader.o
> obj-$(CONFIG_QCOM_OCMEM) += ocmem.o
> diff --git a/drivers/soc/qcom/qcom_eud.c b/drivers/soc/qcom/qcom_eud.c
> new file mode 100644
> index 0000000..7c4cc7d
> --- /dev/null
> +++ b/drivers/soc/qcom/qcom_eud.c
> @@ -0,0 +1,260 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2015-2021, The Linux Foundation. All rights reserved.
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
Is this needed?
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/sysfs.h>
> +#include <linux/usb/role.h>
> +
> +#define EUD_REG_INT1_EN_MASK 0x0024
> +#define EUD_REG_INT_STATUS_1 0x0044
> +#define EUD_REG_CTL_OUT_1 0x0074
> +#define EUD_REG_VBUS_INT_CLR 0x0080
> +#define EUD_REG_CSR_EUD_EN 0x1014
> +#define EUD_REG_SW_ATTACH_DET 0x1018
> +#define EUD_REG_EUD_EN2 0x0000
> +
> +#define EUD_ENABLE BIT(0)
> +#define EUD_INT_PET_EUD BIT(0)
> +#define EUD_INT_VBUS BIT(2)
> +#define EUD_INT_SAFE_MODE BIT(4)
> +#define EUD_INT_ALL (EUD_INT_VBUS|EUD_INT_SAFE_MODE)
> +
> +struct eud_chip {
> + struct device *dev;
> + struct usb_role_switch *role_sw;
> + void __iomem *eud_reg_base;
This is unnecessarily long, "base" or "eud" seems sufficient.
> + void __iomem *eud_mode_mgr2_phys_base;
"mode_mgr"
> + unsigned int int_status;
> + int eud_irq;
"irq"
> + bool enable;
"enable" is an action, but you use it to track if the state of the EUD,
i.e if it's "enabled".
> + bool usb_attach;
"attach" vs "attached"
> +
Empty line.
> +};
> +
> +static int enable_eud(struct eud_chip *priv)
> +{
> + writel(EUD_ENABLE, priv->eud_reg_base + EUD_REG_CSR_EUD_EN);
> + writel(EUD_INT_VBUS | EUD_INT_SAFE_MODE,
> + priv->eud_reg_base + EUD_REG_INT1_EN_MASK);
> + writel(1, priv->eud_mode_mgr2_phys_base + EUD_REG_EUD_EN2);
> +
> + return usb_role_switch_set_role(priv->role_sw, USB_ROLE_DEVICE);
When this fails, don't you want to disable the EUD again?
> +}
> +
> +static void disable_eud(struct eud_chip *priv)
> +{
> + writel(0, priv->eud_reg_base + EUD_REG_CSR_EUD_EN);
> + writel(0, priv->eud_mode_mgr2_phys_base + EUD_REG_EUD_EN2);
> +}
> +
> +static ssize_t enable_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct eud_chip *chip = dev_get_drvdata(dev);
> +
> + return sysfs_emit(buf, "%d\n", chip->enable);
> +}
> +
> +static ssize_t enable_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct eud_chip *chip = dev_get_drvdata(dev);
> + bool enable;
> + int ret;
> +
> + if (kstrtobool(buf, &enable))
> + return -EINVAL;
> +
> + if (enable) {
> + ret = enable_eud(chip);
> + if (!ret)
> + chip->enable = enable;
> + } else {
> + disable_eud(chip);
> + }
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR_RW(enable);
> +
> +static struct attribute *eud_attrs[] = {
> + &dev_attr_enable.attr,
> + NULL,
> +};
> +ATTRIBUTE_GROUPS(eud);
> +
> +static void usb_attach_detach(struct eud_chip *chip)
> +{
> + u32 reg;
> +
> + /* read ctl_out_1[4] to find USB attach or detach event */
> + reg = readl(chip->eud_reg_base + EUD_REG_CTL_OUT_1);
> + if (reg & EUD_INT_SAFE_MODE)
> + chip->usb_attach = true;
> + else
> + chip->usb_attach = false;
chip->usb_attach = reg & EUD_INT_SAFE_MODE;
> +
> + /* set and clear vbus_int_clr[0] to clear interrupt */
> + writel(BIT(0), chip->eud_reg_base + EUD_REG_VBUS_INT_CLR);
> + writel(0, chip->eud_reg_base + EUD_REG_VBUS_INT_CLR);
How come you do this before waking up the threaded handler?
> +}
> +
> +static void pet_eud(struct eud_chip *chip)
> +{
> + u32 reg;
> + int ret;
> +
> + /* read sw_attach_det[0] to find attach/detach event */
Please instead describe what this means, like:
/*
* When the EUD_INT_PET_EUD in SW_ATTACH_DET is set, the cable has been
* disconnected and we need to detach the something because something.
*/
> + reg = readl(chip->eud_reg_base + EUD_REG_SW_ATTACH_DET);
Please drop the double space.
> + if (reg & EUD_INT_PET_EUD) {
> + /* Detach & Attach pet for EUD */
> + writel(0, chip->eud_reg_base + EUD_REG_SW_ATTACH_DET);
> + /* Delay to make sure detach pet is done before attach pet */
> + ret = readl_poll_timeout(chip->eud_reg_base + EUD_REG_SW_ATTACH_DET,
> + reg, (reg == 0), 1, 100);
> + if (ret) {
> + dev_err(chip->dev, "Detach pet failed\n");
> + return;
> + }
> +
> + writel(EUD_INT_PET_EUD, chip->eud_reg_base +
> + EUD_REG_SW_ATTACH_DET);
Can you please help me see what differs between this and the write in
the else statement?
> + } else {
> + /* Attach pet for EUD */
> + writel(EUD_INT_PET_EUD, chip->eud_reg_base +
> + EUD_REG_SW_ATTACH_DET);
> + }
> +}
> +
> +static irqreturn_t handle_eud_irq(int irq, void *data)
> +{
> + struct eud_chip *chip = data;
> + u32 reg;
> +
> + /* read status register and find out which interrupt triggered */
That's what the next line says as well, please drop this comment.
> + reg = readl(chip->eud_reg_base + EUD_REG_INT_STATUS_1);
Please drop the double space.
> + switch (reg & EUD_INT_ALL) {
What if both of these bits are set? Won't you just end up with an
interrupt storm until someone disables the EUD again?
> + case EUD_INT_VBUS:
> + chip->int_status = EUD_INT_VBUS;
> + usb_attach_detach(chip);
> + return IRQ_WAKE_THREAD;
> + case EUD_INT_SAFE_MODE:
> + pet_eud(chip);
> + break;
This is the only case where you break to return IRQ_HANDLED. Please be
consistent.
> + default:
> + return IRQ_NONE;
> + }
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t handle_eud_irq_thread(int irq, void *data)
> +{
> + struct eud_chip *chip = data;
> + int ret;
> +
> + if (chip->int_status == EUD_INT_VBUS) {
I presume you have this here to signal that EUD_INT_VBUS was set in
EUD_REG_INT_STATUS_1 in handle_eud_irq().
But 1) unless this was true you wouldn't have woken up the threaded
handler and 2) you never clear int_status, so once you've received the
EUD_INT_VBUS it seems to forever be true.
> + if (chip->usb_attach)
> + ret = usb_role_switch_set_role(chip->role_sw, USB_ROLE_DEVICE);
> + else
> + ret = usb_role_switch_set_role(chip->role_sw, USB_ROLE_HOST);
> + if (ret)
> + dev_err(chip->dev, "failed to set role switch\n");
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int eud_probe(struct platform_device *pdev)
> +{
> + struct eud_chip *chip;
> + struct fwnode_handle *fwnode = pdev->dev.fwnode, *ep, *connector;
> + int ret;
> +
> + chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
> + if (!chip)
> + return -ENOMEM;
> +
> + chip->dev = &pdev->dev;
> +
> + ep = fwnode_graph_get_next_endpoint(fwnode, NULL);
> + if (!ep)
> + return -ENODEV;
> +
> + connector = fwnode_graph_get_remote_port_parent(ep);
> + fwnode_handle_put(ep);
> + if (!connector)
> + return -ENODEV;
> +
> + chip->role_sw = fwnode_usb_role_switch_get(connector);
Can't you just call usb_role_switch_get(&pdev->dev) to find the
"usb-role-switch", without having to traverse the fwnode_graph manually?
> + if (IS_ERR(chip->role_sw)) {
> + return dev_err_probe(chip->dev, PTR_ERR(chip->role_sw),
> + "failed to get role switch\n");
> + }
Note that from now on you must usb_role_switch_put(chip->role_sw);
> +
> + chip->eud_reg_base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(chip->eud_reg_base))
> + return PTR_ERR(chip->eud_reg_base);
> +
> + chip->eud_mode_mgr2_phys_base = devm_platform_ioremap_resource(pdev, 1);
> + if (IS_ERR(chip->eud_mode_mgr2_phys_base))
> + return PTR_ERR(chip->eud_mode_mgr2_phys_base);
> +
> + chip->eud_irq = platform_get_irq(pdev, 0);
> + ret = devm_request_threaded_irq(&pdev->dev, chip->eud_irq, handle_eud_irq,
> + handle_eud_irq_thread, IRQF_ONESHOT, NULL, chip);
> + if (ret)
> + return dev_err_probe(chip->dev, ret, "failed to allocate irq\n");
> +
> + enable_irq_wake(chip->eud_irq);
> +
> + platform_set_drvdata(pdev, chip);
> +
> + return 0;
> +}
> +
> +static int eud_remove(struct platform_device *pdev)
> +{
> + struct eud_chip *chip = platform_get_drvdata(pdev);
> +
> + if (chip->enable)
> + disable_eud(chip);
> +
> + device_init_wakeup(&pdev->dev, false);
> + disable_irq_wake(chip->eud_irq);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id eud_dt_match[] = {
> + { .compatible = "qcom,sc7280-eud" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, eud_dt_match);
> +
> +static struct platform_driver eud_driver = {
> + .probe = eud_probe,
I would prefer if you didn't indent the assignments.
> + .remove = eud_remove,
> + .driver = {
> + .name = "eud",
"qcom_eud" please.
Regards,
Bjorn
> + .dev_groups = eud_groups,
> + .of_match_table = eud_dt_match,
> + },
> +};
> +module_platform_driver(eud_driver);
> +
> +MODULE_DESCRIPTION("QTI EUD driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH V3 4/7] soc: qcom: eud: Add driver support for Embedded USB Debugger(EUD)
2022-01-07 1:19 ` Bjorn Andersson
@ 2022-01-17 4:58 ` Souradeep Chowdhury
0 siblings, 0 replies; 20+ messages in thread
From: Souradeep Chowdhury @ 2022-01-17 4:58 UTC (permalink / raw)
To: Bjorn Andersson
Cc: linux-arm-msm, linux-usb, devicetree, pure.logic, greg,
linux-kernel, tsoni, psodagud, satyap, pheragu, rnayak, sibis,
saiprakash.ranjan
On 1/7/2022 6:49 AM, Bjorn Andersson wrote:
> On Tue 04 Jan 03:58 PST 2022, Souradeep Chowdhury wrote:
>
>> Add support for control peripheral of EUD (Embedded USB Debugger) to
>> listen to events such as USB attach/detach, pet EUD to indicate software
>> is functional.Reusing the platform device kobj, sysfs entry 'enable' is
>> created to enable or disable EUD.
>>
>> To enable the eud the following needs to be done
>> echo 1 > /sys/bus/platform/.../enable
>>
>> To disable eud, following is the command
>> echo 0 > /sys/bus/platform/.../enable
>>
>> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>> ---
>> Documentation/ABI/testing/sysfs-driver-eud | 9 +
>> drivers/soc/qcom/Kconfig | 10 ++
>> drivers/soc/qcom/Makefile | 1 +
>> drivers/soc/qcom/qcom_eud.c | 260 +++++++++++++++++++++++++++++
>> 4 files changed, 280 insertions(+)
>> create mode 100644 Documentation/ABI/testing/sysfs-driver-eud
>> create mode 100644 drivers/soc/qcom/qcom_eud.c
>>
>> diff --git a/Documentation/ABI/testing/sysfs-driver-eud b/Documentation/ABI/testing/sysfs-driver-eud
>> new file mode 100644
>> index 0000000..2381552
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-driver-eud
>> @@ -0,0 +1,9 @@
>> +What: /sys/bus/platform/drivers/eud/.../enable
>> +Date: January 2022
>> +Contact: Souradeep Chowdhury <schowdhu@codeaurora.org>
>> +Description:
>> + The Enable/Disable sysfs interface for Embedded
>> + USB Debugger(EUD). This enables and disables the
>> + EUD based on a 1 or a 0 value. By enabling EUD,
>> + the user is able to activate the mini-usb hub of
>> + EUD for debug and trace capabilities.
>> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
>> index 79b568f..a4db41b 100644
>> --- a/drivers/soc/qcom/Kconfig
>> +++ b/drivers/soc/qcom/Kconfig
>> @@ -42,6 +42,16 @@ config QCOM_CPR
>> To compile this driver as a module, choose M here: the module will
>> be called qcom-cpr
>>
>> +config QCOM_EUD
>> + tristate "QCOM Embedded USB Debugger(EUD) Driver"
>> + select USB_ROLE_SWITCH
>> + help
>> + This module enables support for Qualcomm Technologies, Inc.
>> + Embedded USB Debugger (EUD). The EUD is a control peripheral
>> + which reports VBUS attach/detach events and has USB-based
>> + debug and trace capabilities. On selecting m, the module name
>> + that is built is qcom_eud.ko
>> +
>> config QCOM_GENI_SE
>> tristate "QCOM GENI Serial Engine Driver"
>> depends on ARCH_QCOM || COMPILE_TEST
>> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
>> index ad675a6..3331a40 100644
>> --- a/drivers/soc/qcom/Makefile
>> +++ b/drivers/soc/qcom/Makefile
>> @@ -4,6 +4,7 @@ obj-$(CONFIG_QCOM_AOSS_QMP) += qcom_aoss.o
>> obj-$(CONFIG_QCOM_GENI_SE) += qcom-geni-se.o
>> obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o
>> obj-$(CONFIG_QCOM_CPR) += cpr.o
>> +obj-$(CONFIG_QCOM_EUD) += qcom_eud.o
>> obj-$(CONFIG_QCOM_GSBI) += qcom_gsbi.o
>> obj-$(CONFIG_QCOM_MDT_LOADER) += mdt_loader.o
>> obj-$(CONFIG_QCOM_OCMEM) += ocmem.o
>> diff --git a/drivers/soc/qcom/qcom_eud.c b/drivers/soc/qcom/qcom_eud.c
>> new file mode 100644
>> index 0000000..7c4cc7d
>> --- /dev/null
>> +++ b/drivers/soc/qcom/qcom_eud.c
>> @@ -0,0 +1,260 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2015-2021, The Linux Foundation. All rights reserved.
>> + */
>> +
>> +#include <linux/bitops.h>
>> +#include <linux/delay.h>
> Is this needed?
Ack.
>
>> +#include <linux/err.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +#include <linux/sysfs.h>
>> +#include <linux/usb/role.h>
>> +
>> +#define EUD_REG_INT1_EN_MASK 0x0024
>> +#define EUD_REG_INT_STATUS_1 0x0044
>> +#define EUD_REG_CTL_OUT_1 0x0074
>> +#define EUD_REG_VBUS_INT_CLR 0x0080
>> +#define EUD_REG_CSR_EUD_EN 0x1014
>> +#define EUD_REG_SW_ATTACH_DET 0x1018
>> +#define EUD_REG_EUD_EN2 0x0000
>> +
>> +#define EUD_ENABLE BIT(0)
>> +#define EUD_INT_PET_EUD BIT(0)
>> +#define EUD_INT_VBUS BIT(2)
>> +#define EUD_INT_SAFE_MODE BIT(4)
>> +#define EUD_INT_ALL (EUD_INT_VBUS|EUD_INT_SAFE_MODE)
>> +
>> +struct eud_chip {
>> + struct device *dev;
>> + struct usb_role_switch *role_sw;
>> + void __iomem *eud_reg_base;
> This is unnecessarily long, "base" or "eud" seems sufficient.
Ack
>
>> + void __iomem *eud_mode_mgr2_phys_base;
> "mode_mgr"
Ack
>
>> + unsigned int int_status;
>> + int eud_irq;
> "irq"
Ack
>
>> + bool enable;
> "enable" is an action, but you use it to track if the state of the EUD,
> i.e if it's "enabled".
Ack
>
>> + bool usb_attach;
> "attach" vs "attached"
Ack
>
>> +
> Empty line.
Ack
>
>> +};
>> +
>> +static int enable_eud(struct eud_chip *priv)
>> +{
>> + writel(EUD_ENABLE, priv->eud_reg_base + EUD_REG_CSR_EUD_EN);
>> + writel(EUD_INT_VBUS | EUD_INT_SAFE_MODE,
>> + priv->eud_reg_base + EUD_REG_INT1_EN_MASK);
>> + writel(1, priv->eud_mode_mgr2_phys_base + EUD_REG_EUD_EN2);
>> +
>> + return usb_role_switch_set_role(priv->role_sw, USB_ROLE_DEVICE);
> When this fails, don't you want to disable the EUD again?
Ack
>
>> +}
>> +
>> +static void disable_eud(struct eud_chip *priv)
>> +{
>> + writel(0, priv->eud_reg_base + EUD_REG_CSR_EUD_EN);
>> + writel(0, priv->eud_mode_mgr2_phys_base + EUD_REG_EUD_EN2);
>> +}
>> +
>> +static ssize_t enable_show(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct eud_chip *chip = dev_get_drvdata(dev);
>> +
>> + return sysfs_emit(buf, "%d\n", chip->enable);
>> +}
>> +
>> +static ssize_t enable_store(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + struct eud_chip *chip = dev_get_drvdata(dev);
>> + bool enable;
>> + int ret;
>> +
>> + if (kstrtobool(buf, &enable))
>> + return -EINVAL;
>> +
>> + if (enable) {
>> + ret = enable_eud(chip);
>> + if (!ret)
>> + chip->enable = enable;
>> + } else {
>> + disable_eud(chip);
>> + }
>> +
>> + return count;
>> +}
>> +
>> +static DEVICE_ATTR_RW(enable);
>> +
>> +static struct attribute *eud_attrs[] = {
>> + &dev_attr_enable.attr,
>> + NULL,
>> +};
>> +ATTRIBUTE_GROUPS(eud);
>> +
>> +static void usb_attach_detach(struct eud_chip *chip)
>> +{
>> + u32 reg;
>> +
>> + /* read ctl_out_1[4] to find USB attach or detach event */
>> + reg = readl(chip->eud_reg_base + EUD_REG_CTL_OUT_1);
>> + if (reg & EUD_INT_SAFE_MODE)
>> + chip->usb_attach = true;
>> + else
>> + chip->usb_attach = false;
> chip->usb_attach = reg & EUD_INT_SAFE_MODE;
Ack
>> +
>> + /* set and clear vbus_int_clr[0] to clear interrupt */
>> + writel(BIT(0), chip->eud_reg_base + EUD_REG_VBUS_INT_CLR);
>> + writel(0, chip->eud_reg_base + EUD_REG_VBUS_INT_CLR);
> How come you do this before waking up the threaded handler?
Ack
>
>> +}
>> +
>> +static void pet_eud(struct eud_chip *chip)
>> +{
>> + u32 reg;
>> + int ret;
>> +
>> + /* read sw_attach_det[0] to find attach/detach event */
> Please instead describe what this means, like:
>
> /*
> * When the EUD_INT_PET_EUD in SW_ATTACH_DET is set, the cable has been
> * disconnected and we need to detach the something because something.
> */
Ack
>
>> + reg = readl(chip->eud_reg_base + EUD_REG_SW_ATTACH_DET);
> Please drop the double space.
Ack
>
>> + if (reg & EUD_INT_PET_EUD) {
>> + /* Detach & Attach pet for EUD */
>> + writel(0, chip->eud_reg_base + EUD_REG_SW_ATTACH_DET);
>> + /* Delay to make sure detach pet is done before attach pet */
>> + ret = readl_poll_timeout(chip->eud_reg_base + EUD_REG_SW_ATTACH_DET,
>> + reg, (reg == 0), 1, 100);
>> + if (ret) {
>> + dev_err(chip->dev, "Detach pet failed\n");
>> + return;
>> + }
>> +
>> + writel(EUD_INT_PET_EUD, chip->eud_reg_base +
>> + EUD_REG_SW_ATTACH_DET);
> Can you please help me see what differs between this and the write in
> the else statement?
Ack. Will move this out of the if else statement.
>
>> + } else {
>> + /* Attach pet for EUD */
>> + writel(EUD_INT_PET_EUD, chip->eud_reg_base +
>> + EUD_REG_SW_ATTACH_DET);
>> + }
>> +}
>> +
>> +static irqreturn_t handle_eud_irq(int irq, void *data)
>> +{
>> + struct eud_chip *chip = data;
>> + u32 reg;
>> +
>> + /* read status register and find out which interrupt triggered */
> That's what the next line says as well, please drop this comment.
Ack
>
>> + reg = readl(chip->eud_reg_base + EUD_REG_INT_STATUS_1);
> Please drop the double space.
Ack
>
>> + switch (reg & EUD_INT_ALL) {
> What if both of these bits are set? Won't you just end up with an
> interrupt storm until someone disables the EUD again?
So EUD can either be in safe mode or vbus mode , both bits will not be
set together.
>
>> + case EUD_INT_VBUS:
>> + chip->int_status = EUD_INT_VBUS;
>> + usb_attach_detach(chip);
>> + return IRQ_WAKE_THREAD;
>> + case EUD_INT_SAFE_MODE:
>> + pet_eud(chip);
>> + break;
> This is the only case where you break to return IRQ_HANDLED. Please be
> consistent.
Ack
>
>> + default:
>> + return IRQ_NONE;
>> + }
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static irqreturn_t handle_eud_irq_thread(int irq, void *data)
>> +{
>> + struct eud_chip *chip = data;
>> + int ret;
>> +
>> + if (chip->int_status == EUD_INT_VBUS) {
> I presume you have this here to signal that EUD_INT_VBUS was set in
> EUD_REG_INT_STATUS_1 in handle_eud_irq().
>
> But 1) unless this was true you wouldn't have woken up the threaded
> handler and 2) you never clear int_status, so once you've received the
> EUD_INT_VBUS it seems to forever be true.
Ack. Will be clearing the int bits once this is done.
>
>> + if (chip->usb_attach)
>> + ret = usb_role_switch_set_role(chip->role_sw, USB_ROLE_DEVICE);
>> + else
>> + ret = usb_role_switch_set_role(chip->role_sw, USB_ROLE_HOST);
>> + if (ret)
>> + dev_err(chip->dev, "failed to set role switch\n");
>> + }
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int eud_probe(struct platform_device *pdev)
>> +{
>> + struct eud_chip *chip;
>> + struct fwnode_handle *fwnode = pdev->dev.fwnode, *ep, *connector;
>> + int ret;
>> +
>> + chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
>> + if (!chip)
>> + return -ENOMEM;
>> +
>> + chip->dev = &pdev->dev;
>> +
>> + ep = fwnode_graph_get_next_endpoint(fwnode, NULL);
>> + if (!ep)
>> + return -ENODEV;
>> +
>> + connector = fwnode_graph_get_remote_port_parent(ep);
>> + fwnode_handle_put(ep);
>> + if (!connector)
>> + return -ENODEV;
>> +
>> + chip->role_sw = fwnode_usb_role_switch_get(connector);
> Can't you just call usb_role_switch_get(&pdev->dev) to find the
> "usb-role-switch", without having to traverse the fwnode_graph manually?
"usb_role_switch_get" finds the role_switch from the current or the
parent node, however
in this case the usb-role-switch property is present at the parent of
the connector node
which is attached to the eud node via port end point.
>
>> + if (IS_ERR(chip->role_sw)) {
>> + return dev_err_probe(chip->dev, PTR_ERR(chip->role_sw),
>> + "failed to get role switch\n");
>> + }
> Note that from now on you must usb_role_switch_put(chip->role_sw);
Ack
>
>> +
>> + chip->eud_reg_base = devm_platform_ioremap_resource(pdev, 0);
>> + if (IS_ERR(chip->eud_reg_base))
>> + return PTR_ERR(chip->eud_reg_base);
>> +
>> + chip->eud_mode_mgr2_phys_base = devm_platform_ioremap_resource(pdev, 1);
>> + if (IS_ERR(chip->eud_mode_mgr2_phys_base))
>> + return PTR_ERR(chip->eud_mode_mgr2_phys_base);
>> +
>> + chip->eud_irq = platform_get_irq(pdev, 0);
>> + ret = devm_request_threaded_irq(&pdev->dev, chip->eud_irq, handle_eud_irq,
>> + handle_eud_irq_thread, IRQF_ONESHOT, NULL, chip);
>> + if (ret)
>> + return dev_err_probe(chip->dev, ret, "failed to allocate irq\n");
>> +
>> + enable_irq_wake(chip->eud_irq);
>> +
>> + platform_set_drvdata(pdev, chip);
>> +
>> + return 0;
>> +}
>> +
>> +static int eud_remove(struct platform_device *pdev)
>> +{
>> + struct eud_chip *chip = platform_get_drvdata(pdev);
>> +
>> + if (chip->enable)
>> + disable_eud(chip);
>> +
>> + device_init_wakeup(&pdev->dev, false);
>> + disable_irq_wake(chip->eud_irq);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id eud_dt_match[] = {
>> + { .compatible = "qcom,sc7280-eud" },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(of, eud_dt_match);
>> +
>> +static struct platform_driver eud_driver = {
>> + .probe = eud_probe,
> I would prefer if you didn't indent the assignments.
Ack
>
>> + .remove = eud_remove,
>> + .driver = {
>> + .name = "eud",
> "qcom_eud" please.
Ack
>
> Regards,
> Bjorn
>
>> + .dev_groups = eud_groups,
>> + .of_match_table = eud_dt_match,
>> + },
>> +};
>> +module_platform_driver(eud_driver);
>> +
>> +MODULE_DESCRIPTION("QTI EUD driver");
>> +MODULE_LICENSE("GPL v2");
>> --
>> 2.7.4
>>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH V3 5/7] arm64: dts: qcom: sc7280: Add EUD dt node and dwc3 connector
2022-01-04 11:58 [PATCH V3 0/7] Add Embedded USB Debugger (EUD) driver Souradeep Chowdhury
` (3 preceding siblings ...)
2022-01-04 11:58 ` [PATCH V3 4/7] soc: qcom: eud: Add driver support for Embedded USB Debugger(EUD) Souradeep Chowdhury
@ 2022-01-04 11:58 ` Souradeep Chowdhury
2022-01-04 19:24 ` Dmitry Baryshkov
2022-01-07 0:53 ` Bjorn Andersson
2022-01-04 11:58 ` [PATCH V3 6/7] arm64: dts: qcom: sc7280: Set the default dr_mode for usb2 Souradeep Chowdhury
2022-01-04 11:58 ` [PATCH V3 7/7] MAINTAINERS: Add maintainer entry for EUD Souradeep Chowdhury
6 siblings, 2 replies; 20+ messages in thread
From: Souradeep Chowdhury @ 2022-01-04 11:58 UTC (permalink / raw)
To: linux-arm-msm, linux-usb, devicetree, pure.logic, bjorn.andersson, greg
Cc: linux-kernel, tsoni, psodagud, satyap, pheragu, rnayak, sibis,
saiprakash.ranjan, quic_schowdhu
Add the Embedded USB Debugger(EUD) device tree node. The
node contains EUD base register region and EUD mode
manager register regions along with the interrupt entry.
Also add the connector to EUD which is mapped as the child
of dwc3. The connector is attached to EUD via port. Also add
the role-switch property to dwc3 node.
Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
---
arch/arm64/boot/dts/qcom/sc7280.dtsi | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 53a21d0..2d14e5c 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -1315,6 +1315,18 @@
phys = <&usb_2_hsphy>;
phy-names = "usb2-phy";
maximum-speed = "high-speed";
+ usb-role-switch;
+ usb_con: eud_usb_connector {
+ compatible = "qcom,usb-connector-eud",
+ "usb-c-connector";
+ ports {
+ port@0 {
+ usb2_role_switch: endpoint {
+ remote-endpoint = <&eud_ep>;
+ };
+ };
+ };
+ };
};
};
@@ -1339,6 +1351,19 @@
interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>;
};
+ eud: eud@88e0000 {
+ compatible = "qcom,sc7280-eud","qcom,eud";
+ reg = <0 0x88e0000 0 0x2000>,
+ <0 0x88e2000 0 0x1000>;
+ interrupt-parent = <&pdc>;
+ interrupts = <11 IRQ_TYPE_LEVEL_HIGH>;
+ port {
+ eud_ep: endpoint {
+ remote-endpoint = <&usb2_role_switch>;
+ };
+ };
+ };
+
nsp_noc: interconnect@a0c0000 {
reg = <0 0x0a0c0000 0 0x10000>;
compatible = "qcom,sc7280-nsp-noc";
--
2.7.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH V3 5/7] arm64: dts: qcom: sc7280: Add EUD dt node and dwc3 connector
2022-01-04 11:58 ` [PATCH V3 5/7] arm64: dts: qcom: sc7280: Add EUD dt node and dwc3 connector Souradeep Chowdhury
@ 2022-01-04 19:24 ` Dmitry Baryshkov
2022-01-07 0:53 ` Bjorn Andersson
1 sibling, 0 replies; 20+ messages in thread
From: Dmitry Baryshkov @ 2022-01-04 19:24 UTC (permalink / raw)
To: Souradeep Chowdhury
Cc: linux-arm-msm, linux-usb, devicetree, pure.logic,
bjorn.andersson, greg, linux-kernel, tsoni, psodagud, satyap,
pheragu, rnayak, sibis, saiprakash.ranjan
On Tue, 4 Jan 2022 at 15:00, Souradeep Chowdhury
<quic_schowdhu@quicinc.com> wrote:
>
> Add the Embedded USB Debugger(EUD) device tree node. The
> node contains EUD base register region and EUD mode
> manager register regions along with the interrupt entry.
> Also add the connector to EUD which is mapped as the child
> of dwc3. The connector is attached to EUD via port. Also add
> the role-switch property to dwc3 node.
>
> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> ---
> arch/arm64/boot/dts/qcom/sc7280.dtsi | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 53a21d0..2d14e5c 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -1315,6 +1315,18 @@
> phys = <&usb_2_hsphy>;
> phy-names = "usb2-phy";
> maximum-speed = "high-speed";
> + usb-role-switch;
> + usb_con: eud_usb_connector {
Is this node meant to be named connector instead?
> + compatible = "qcom,usb-connector-eud",
> + "usb-c-connector";
> + ports {
> + port@0 {
> + usb2_role_switch: endpoint {
> + remote-endpoint = <&eud_ep>;
> + };
> + };
> + };
> + };
> };
> };
>
> @@ -1339,6 +1351,19 @@
> interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>;
> };
>
> + eud: eud@88e0000 {
> + compatible = "qcom,sc7280-eud","qcom,eud";
> + reg = <0 0x88e0000 0 0x2000>,
> + <0 0x88e2000 0 0x1000>;
> + interrupt-parent = <&pdc>;
> + interrupts = <11 IRQ_TYPE_LEVEL_HIGH>;
> + port {
> + eud_ep: endpoint {
> + remote-endpoint = <&usb2_role_switch>;
> + };
> + };
> + };
> +
> nsp_noc: interconnect@a0c0000 {
> reg = <0 0x0a0c0000 0 0x10000>;
> compatible = "qcom,sc7280-nsp-noc";
> --
> 2.7.4
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH V3 5/7] arm64: dts: qcom: sc7280: Add EUD dt node and dwc3 connector
2022-01-04 11:58 ` [PATCH V3 5/7] arm64: dts: qcom: sc7280: Add EUD dt node and dwc3 connector Souradeep Chowdhury
2022-01-04 19:24 ` Dmitry Baryshkov
@ 2022-01-07 0:53 ` Bjorn Andersson
2022-01-17 5:07 ` Souradeep Chowdhury
1 sibling, 1 reply; 20+ messages in thread
From: Bjorn Andersson @ 2022-01-07 0:53 UTC (permalink / raw)
To: Souradeep Chowdhury
Cc: linux-arm-msm, linux-usb, devicetree, pure.logic, greg,
linux-kernel, tsoni, psodagud, satyap, pheragu, rnayak, sibis,
saiprakash.ranjan
On Tue 04 Jan 03:58 PST 2022, Souradeep Chowdhury wrote:
> Add the Embedded USB Debugger(EUD) device tree node. The
> node contains EUD base register region and EUD mode
> manager register regions along with the interrupt entry.
> Also add the connector to EUD which is mapped as the child
> of dwc3. The connector is attached to EUD via port. Also add
> the role-switch property to dwc3 node.
>
> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> ---
> arch/arm64/boot/dts/qcom/sc7280.dtsi | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 53a21d0..2d14e5c 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -1315,6 +1315,18 @@
> phys = <&usb_2_hsphy>;
> phy-names = "usb2-phy";
> maximum-speed = "high-speed";
> + usb-role-switch;
> + usb_con: eud_usb_connector {
I think this is backwards.
Iiuc you have dwc3 <-> EUD <-> connector, where the connector typically
would be represented in relation to a Type-C controller.
So I think this graph should be looking something like:
dwc3 {
ports {
port {
dwc3_eud: endpoint {
remote-endpoint = <&eud_ep>;
};
};
};
};
eud {
ports {
port {
eud_dwc3: endpoint {
remote-endpoint = <&dwc3_eud>;
};
};
port {
eud_con: endpoint {
remote-endpoint = <&con_eud>;
};
};
};
};
connector {
compatible = "usb-c-connector";
ports {
port {
con_eud: endpoint {
remote-endpoint = <&eud_con>;
};
};
};
};
With EUD "forwarding" the role switching requests from the connector, or
overriding them if the EUD is enabled.
Alternatively the dwc3 would be in a graph with both the eud and the
connector, but I don't know how to ensure that the EUD decision takes
precedence over the type-c controllers role request...
In the case of us having a type-c controller, such as the pmic_glink,
the connector node would then be a child of the controller.
Regards,
Bjorn
> + compatible = "qcom,usb-connector-eud",
> + "usb-c-connector";
> + ports {
> + port@0 {
> + usb2_role_switch: endpoint {
> + remote-endpoint = <&eud_ep>;
> + };
> + };
> + };
> + };
> };
> };
>
> @@ -1339,6 +1351,19 @@
> interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>;
> };
>
> + eud: eud@88e0000 {
> + compatible = "qcom,sc7280-eud","qcom,eud";
> + reg = <0 0x88e0000 0 0x2000>,
> + <0 0x88e2000 0 0x1000>;
> + interrupt-parent = <&pdc>;
> + interrupts = <11 IRQ_TYPE_LEVEL_HIGH>;
> + port {
> + eud_ep: endpoint {
> + remote-endpoint = <&usb2_role_switch>;
> + };
> + };
> + };
> +
> nsp_noc: interconnect@a0c0000 {
> reg = <0 0x0a0c0000 0 0x10000>;
> compatible = "qcom,sc7280-nsp-noc";
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH V3 5/7] arm64: dts: qcom: sc7280: Add EUD dt node and dwc3 connector
2022-01-07 0:53 ` Bjorn Andersson
@ 2022-01-17 5:07 ` Souradeep Chowdhury
0 siblings, 0 replies; 20+ messages in thread
From: Souradeep Chowdhury @ 2022-01-17 5:07 UTC (permalink / raw)
To: Bjorn Andersson
Cc: linux-arm-msm, linux-usb, devicetree, pure.logic, greg,
linux-kernel, tsoni, psodagud, satyap, pheragu, rnayak, sibis,
saiprakash.ranjan
On 1/7/2022 6:23 AM, Bjorn Andersson wrote:
> On Tue 04 Jan 03:58 PST 2022, Souradeep Chowdhury wrote:
>
>> Add the Embedded USB Debugger(EUD) device tree node. The
>> node contains EUD base register region and EUD mode
>> manager register regions along with the interrupt entry.
>> Also add the connector to EUD which is mapped as the child
>> of dwc3. The connector is attached to EUD via port. Also add
>> the role-switch property to dwc3 node.
>>
>> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>> ---
>> arch/arm64/boot/dts/qcom/sc7280.dtsi | 25 +++++++++++++++++++++++++
>> 1 file changed, 25 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> index 53a21d0..2d14e5c 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> @@ -1315,6 +1315,18 @@
>> phys = <&usb_2_hsphy>;
>> phy-names = "usb2-phy";
>> maximum-speed = "high-speed";
>> + usb-role-switch;
>> + usb_con: eud_usb_connector {
> I think this is backwards.
>
> Iiuc you have dwc3 <-> EUD <-> connector, where the connector typically
> would be represented in relation to a Type-C controller.
>
> So I think this graph should be looking something like:
>
>
> dwc3 {
> ports {
> port {
> dwc3_eud: endpoint {
> remote-endpoint = <&eud_ep>;
> };
> };
> };
> };
>
> eud {
> ports {
> port {
> eud_dwc3: endpoint {
> remote-endpoint = <&dwc3_eud>;
> };
> };
>
> port {
> eud_con: endpoint {
> remote-endpoint = <&con_eud>;
> };
> };
> };
> };
>
> connector {
> compatible = "usb-c-connector";
>
> ports {
> port {
> con_eud: endpoint {
> remote-endpoint = <&eud_con>;
> };
> };
> };
> };
>
> With EUD "forwarding" the role switching requests from the connector, or
> overriding them if the EUD is enabled.
>
> Alternatively the dwc3 would be in a graph with both the eud and the
> connector, but I don't know how to ensure that the EUD decision takes
> precedence over the type-c controllers role request...
>
>
> In the case of us having a type-c controller, such as the pmic_glink,
> the connector node would then be a child of the controller.
Ack. Will align the device tree with this structure.
>
> Regards,
> Bjorn
>
>> + compatible = "qcom,usb-connector-eud",
>> + "usb-c-connector";
>> + ports {
>> + port@0 {
>> + usb2_role_switch: endpoint {
>> + remote-endpoint = <&eud_ep>;
>> + };
>> + };
>> + };
>> + };
>> };
>> };
>>
>> @@ -1339,6 +1351,19 @@
>> interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>;
>> };
>>
>> + eud: eud@88e0000 {
>> + compatible = "qcom,sc7280-eud","qcom,eud";
>> + reg = <0 0x88e0000 0 0x2000>,
>> + <0 0x88e2000 0 0x1000>;
>> + interrupt-parent = <&pdc>;
>> + interrupts = <11 IRQ_TYPE_LEVEL_HIGH>;
>> + port {
>> + eud_ep: endpoint {
>> + remote-endpoint = <&usb2_role_switch>;
>> + };
>> + };
>> + };
>> +
>> nsp_noc: interconnect@a0c0000 {
>> reg = <0 0x0a0c0000 0 0x10000>;
>> compatible = "qcom,sc7280-nsp-noc";
>> --
>> 2.7.4
>>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH V3 6/7] arm64: dts: qcom: sc7280: Set the default dr_mode for usb2
2022-01-04 11:58 [PATCH V3 0/7] Add Embedded USB Debugger (EUD) driver Souradeep Chowdhury
` (4 preceding siblings ...)
2022-01-04 11:58 ` [PATCH V3 5/7] arm64: dts: qcom: sc7280: Add EUD dt node and dwc3 connector Souradeep Chowdhury
@ 2022-01-04 11:58 ` Souradeep Chowdhury
2022-01-04 11:58 ` [PATCH V3 7/7] MAINTAINERS: Add maintainer entry for EUD Souradeep Chowdhury
6 siblings, 0 replies; 20+ messages in thread
From: Souradeep Chowdhury @ 2022-01-04 11:58 UTC (permalink / raw)
To: linux-arm-msm, linux-usb, devicetree, pure.logic, bjorn.andersson, greg
Cc: linux-kernel, tsoni, psodagud, satyap, pheragu, rnayak, sibis,
saiprakash.ranjan, quic_schowdhu
Set the default dr_mode for usb2 node to "otg" to enable.
role-switch for EUD(Embedded USB Debugger) connector node.
Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
---
arch/arm64/boot/dts/qcom/sc7280-idp.dts | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
index 64fc22a..7c69c78 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
@@ -61,6 +61,10 @@
modem-init;
};
+&usb_2_dwc3 {
+ dr_mode = "otg";
+};
+
&pmk8350_vadc {
pmr735a_die_temp {
reg = <PMR735A_ADC7_DIE_TEMP>;
--
2.7.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH V3 7/7] MAINTAINERS: Add maintainer entry for EUD
2022-01-04 11:58 [PATCH V3 0/7] Add Embedded USB Debugger (EUD) driver Souradeep Chowdhury
` (5 preceding siblings ...)
2022-01-04 11:58 ` [PATCH V3 6/7] arm64: dts: qcom: sc7280: Set the default dr_mode for usb2 Souradeep Chowdhury
@ 2022-01-04 11:58 ` Souradeep Chowdhury
2022-01-06 10:52 ` Pavel Machek
6 siblings, 1 reply; 20+ messages in thread
From: Souradeep Chowdhury @ 2022-01-04 11:58 UTC (permalink / raw)
To: linux-arm-msm, linux-usb, devicetree, pure.logic, bjorn.andersson, greg
Cc: linux-kernel, tsoni, psodagud, satyap, pheragu, rnayak, sibis,
saiprakash.ranjan, quic_schowdhu
Add the entry for maintainer for EUD driver
and other associated files.
Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
---
MAINTAINERS | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 0227e15..54f616a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7018,6 +7018,14 @@ F: include/trace/events/mdio.h
F: include/uapi/linux/mdio.h
F: include/uapi/linux/mii.h
+EUD-QCOM
+M: Souradeep Chowdhury <schowdhu@codeaurora.org>
+L: linux-arm-msm@vger.kernel.org
+S: Maintained
+F: Documentation/ABI/testing/sysfs-driver-eud
+F: Documentation/devicetree/bindings/arm/msm/qcom,eud.yaml
+F: drivers/usb/common/qcom_eud.c
+
EXFAT FILE SYSTEM
M: Namjae Jeon <linkinjeon@kernel.org>
M: Sungjong Seo <sj1557.seo@samsung.com>
--
2.7.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH V3 7/7] MAINTAINERS: Add maintainer entry for EUD
2022-01-04 11:58 ` [PATCH V3 7/7] MAINTAINERS: Add maintainer entry for EUD Souradeep Chowdhury
@ 2022-01-06 10:52 ` Pavel Machek
0 siblings, 0 replies; 20+ messages in thread
From: Pavel Machek @ 2022-01-06 10:52 UTC (permalink / raw)
To: Souradeep Chowdhury
Cc: linux-arm-msm, linux-usb, devicetree, pure.logic,
bjorn.andersson, greg, linux-kernel, tsoni, psodagud, satyap,
pheragu, rnayak, sibis, saiprakash.ranjan
[-- Attachment #1: Type: text/plain, Size: 740 bytes --]
On Tue 2022-01-04 17:28:20, Souradeep Chowdhury wrote:
> Add the entry for maintainer for EUD driver
> and other associated files.
>
> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> ---
> MAINTAINERS | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0227e15..54f616a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7018,6 +7018,14 @@ F: include/trace/events/mdio.h
> F: include/uapi/linux/mdio.h
> F: include/uapi/linux/mii.h
>
> +EUD-QCOM
This should really explain the "EUD" and "QCOM". Something like
EMBEDDED USB DEBUGGER (EUD) ?
Is it Qualcomm specific?
Best regards,
Pavel
--
http://www.livejournal.com/~pavelmachek
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread