On 19/03/2023 17:50, Krzysztof Kozlowski wrote: > 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@xxxxxxxxxx> >>>>>> + >>>>>> +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: 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. > > pmic { > usb { > }; > }; > > virtual- pmic-tcpm { > }; > > What hardware piece does such description represent? > >> >> Could I trouble you for an example ? >> >> --- >> bod > > Best regards, > Krzysztof > -- Kind Regards, Caleb (they/them)