| Message ID | 20230318121828.739424-10-bryan.odonoghue@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Add Qualcomm PMIC TPCM support | expand |
On 19/03/2023 11:58, Krzysztof Kozlowski wrote: >> + >> +maintainers: >> + - Bryan O'Donoghue<bryan.odonoghue@linaro.org> >> + >> +description: | >> + Qualcomm PMIC Virtual Type-C Port Manager Driver >> + A virtual device which manages Qualcomm PMIC provided Type-C port and >> + Power Delivery in one place. > OK, so it looks like bindings for driver, so a no-go. Unless there is > such device as "manager", this does not look like hardware description. > >> + >> +properties: >> + compatible: >> + const: qcom,pmic-virt-tcpm >> + >> + connector: >> + type: object >> + $ref: /schemas/connector/usb-connector.yaml# >> + unevaluatedProperties: false >> + >> + port: >> + $ref: /schemas/graph.yaml#/properties/port >> + description: >> + Contains a port which consumes data-role switching messages. >> + >> + qcom,pmic-typec: >> + $ref: /schemas/types.yaml#/definitions/phandle >> + description: >> + A phandle to the typec port hardware driver. >> + >> + qcom,pmic-pdphy: >> + $ref: /schemas/types.yaml#/definitions/phandle > Having typec and phy as phandles - not children - also suggests this is > some software construct, not hardware description. So probably I didn't interpret Rob's comment correctly here. For a pure software device - a virtual device - there should be no dts representation at all - not even at the firmware{}, chosen{}, rpm{} level, it wouldn't be possible/acceptable to have a tcpm {} with a compat pointing to the two phandles I have here ? --- bod
On 19/03/2023 15:10, Krzysztof Kozlowski wrote: > On 19/03/2023 15:59, Bryan O'Donoghue wrote: >> On 19/03/2023 11:58, Krzysztof Kozlowski wrote: >>>> + >>>> +maintainers: >>>> + - Bryan O'Donoghue<bryan.odonoghue@linaro.org> >>>> + >>>> +description: | >>>> + Qualcomm PMIC Virtual Type-C Port Manager Driver >>>> + A virtual device which manages Qualcomm PMIC provided Type-C port and >>>> + Power Delivery in one place. >>> OK, so it looks like bindings for driver, so a no-go. Unless there is >>> such device as "manager", this does not look like hardware description. >>> >>>> + >>>> +properties: >>>> + compatible: >>>> + const: qcom,pmic-virt-tcpm >>>> + >>>> + connector: >>>> + type: object >>>> + $ref: /schemas/connector/usb-connector.yaml# >>>> + unevaluatedProperties: false >>>> + >>>> + port: >>>> + $ref: /schemas/graph.yaml#/properties/port >>>> + description: >>>> + Contains a port which consumes data-role switching messages. >>>> + >>>> + qcom,pmic-typec: >>>> + $ref: /schemas/types.yaml#/definitions/phandle >>>> + description: >>>> + A phandle to the typec port hardware driver. >>>> + >>>> + qcom,pmic-pdphy: >>>> + $ref: /schemas/types.yaml#/definitions/phandle >>> Having typec and phy as phandles - not children - also suggests this is >>> some software construct, not hardware description. >> >> So probably I didn't interpret Rob's comment correctly here. > > He proposed to merge it with other node: > "probably merged with > one of the nodes these phandles point to." > > "Why can't most of this binding be part of" > > I don't see how you implemented his comments. Actually, nothing improved > here in this regard - you still have these phandles. So this comment from Rob is what I was aiming for "Your other option is instantiate your own device from the virtual driver's initcall based on presence of the 2 nodes above. " rather than two mush the pdphy and typec into one device, which they are not. I guess what I'm trying to understand is how you guys would suggest that is actually done. Could I trouble you for an example ? --- bod
On 19/03/2023 16:44, Bryan O'Donoghue wrote: > On 19/03/2023 15:10, Krzysztof Kozlowski wrote: >> On 19/03/2023 15:59, Bryan O'Donoghue wrote: >>> On 19/03/2023 11:58, Krzysztof Kozlowski wrote: >>>>> + >>>>> +maintainers: >>>>> + - Bryan O'Donoghue<bryan.odonoghue@linaro.org> >>>>> + >>>>> +description: | >>>>> + Qualcomm PMIC Virtual Type-C Port Manager Driver >>>>> + A virtual device which manages Qualcomm PMIC provided Type-C port and >>>>> + Power Delivery in one place. >>>> OK, so it looks like bindings for driver, so a no-go. Unless there is >>>> such device as "manager", this does not look like hardware description. >>>> >>>>> + >>>>> +properties: >>>>> + compatible: >>>>> + const: qcom,pmic-virt-tcpm >>>>> + >>>>> + connector: >>>>> + type: object >>>>> + $ref: /schemas/connector/usb-connector.yaml# >>>>> + unevaluatedProperties: false >>>>> + >>>>> + port: >>>>> + $ref: /schemas/graph.yaml#/properties/port >>>>> + description: >>>>> + Contains a port which consumes data-role switching messages. >>>>> + >>>>> + qcom,pmic-typec: >>>>> + $ref: /schemas/types.yaml#/definitions/phandle >>>>> + description: >>>>> + A phandle to the typec port hardware driver. >>>>> + >>>>> + qcom,pmic-pdphy: >>>>> + $ref: /schemas/types.yaml#/definitions/phandle >>>> Having typec and phy as phandles - not children - also suggests this is >>>> some software construct, not hardware description. >>> >>> So probably I didn't interpret Rob's comment correctly here. >> >> He proposed to merge it with other node: >> "probably merged with >> one of the nodes these phandles point to." >> >> "Why can't most of this binding be part of" >> >> I don't see how you implemented his comments. Actually, nothing improved >> here in this regard - you still have these phandles. > > So this comment from Rob is what I was aiming for > > "Your other option is instantiate your own device from the virtual > driver's initcall based on presence of the 2 nodes above. " > > rather than two mush the pdphy and typec into one device, which they are > not. Sure, but you did not instantiate anything based on these two or one nodes. You added virtual device node. > I guess what I'm trying to understand is how you guys would suggest that > is actually done. You have there already node for the PMIC USB Type-C, so this should be part of it. I really do not understand why this is separate device lying around in parallel like: pmic { usb { }; }; virtual- pmic-tcpm { }; What hardware piece does such description represent? > > Could I trouble you for an example ? > > --- > bod Best regards, Krzysztof
On 19/03/2023 21:31, Caleb Connolly wrote: > The pdphy is fairly well encapsulated (3 tcpm callbacks go to it, that's > all?), I think the tcpm part could be merged in with the typec driver > and it could just have a phandle to the pdphy node to represent the > dependency. > > Then in the typec driver you can get the device with > spmi_device_from_of() and call into it that way for the few tcpm > callbacks that it needs to handle and to pass in the tcpm_port. Yes or just have one "typec" device own both register banks --- bod
diff --git a/Documentation/devicetree/bindings/usb/qcom,pmic-virt-tcpm.yaml b/Documentation/devicetree/bindings/usb/qcom,pmic-virt-tcpm.yaml new file mode 100644 index 0000000000000..576842c8b65b4 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/qcom,pmic-virt-tcpm.yaml @@ -0,0 +1,88 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/usb/qcom,pmic-virt-tcpm.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Qualcomm PMIC Virtual TCPM Driver + +maintainers: + - Bryan O'Donoghue <bryan.odonoghue@linaro.org> + +description: | + Qualcomm PMIC Virtual Type-C Port Manager Driver + A virtual device which manages Qualcomm PMIC provided Type-C port and + Power Delivery in one place. + +properties: + compatible: + const: qcom,pmic-virt-tcpm + + connector: + type: object + $ref: /schemas/connector/usb-connector.yaml# + unevaluatedProperties: false + + port: + $ref: /schemas/graph.yaml#/properties/port + description: + Contains a port which consumes data-role switching messages. + + qcom,pmic-typec: + $ref: /schemas/types.yaml#/definitions/phandle + description: + A phandle to the typec port hardware driver. + + qcom,pmic-pdphy: + $ref: /schemas/types.yaml#/definitions/phandle + description: + A phandle to the type-c pdphy hardware driver. + +required: + - compatible + - connector + - port + - qcom,pmic-typec + - qcom,pmic-pdphy + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + #include <dt-bindings/usb/pd.h> + #include <dt-bindings/usb/typec/qcom,pmic-typec.h> + #include <dt-bindings/usb/typec/qcom,pmic-pdphy.h> + + pm8150b_tcpm: pmic-tcpm { + compatible = "qcom,pmic-virt-tcpm"; + + qcom,pmic-typec = <&pm8150b_typec>; + qcom,pmic-pdphy = <&pm8150b_pdphy>; + + port { + usb3_role: endpoint { + remote-endpoint = <&dwc3_drd_switch>; + }; + }; + + connector { + compatible = "usb-c-connector"; + + power-role = "source"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + pmic_tcpm_ss_mux: endpoint { + remote-endpoint = <&qmp_ss_mux>; + }; + }; + }; + }; + }; + +...
Add a YAML description for the pm8150b-tcpm driver. The pm8150b-tcpm encapsulates a type-c block and a pdphy block into one block presented to the TCPM Linux API. Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> --- .../bindings/usb/qcom,pmic-virt-tcpm.yaml | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/qcom,pmic-virt-tcpm.yaml