Re: [PATCH v3 5/7] dt-bindings: usb: Add Qualcomm PMIC TCPM YAML schema

[Rob Herring <robh@kernel.org>]

On Mon, Nov 08, 2021 at 07:36:27PM +0000, Bryan O'Donoghue wrote:
> On 08/11/2021 19:22, Rob Herring wrote:
> > On Mon, Nov 8, 2021 at 12:58 PM Bryan O'Donoghue
> > <bryan.odonoghue@linaro.org> wrote:
> > > 
> > > On 08/11/2021 17:13, Rob Herring wrote:
> > > > Looks like the h/w is all part of a
> > > > PMIC, so it should be part of the PMIC binding and probably merged with
> > > > one of the nodes these phandles point to.
> > > 
> > > Not sure I really follow you here.
> > > 
> > > The existing PMIC dts arch/arm64/boot/dts/qcom/pm8150b.dtsi has:
> > > 
> > > pm8150b_gpios: gpio@c000 {
> > >       compatible = "qcom,pm8150b-gpio";
> > > }
> > > Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml
> > > 
> > > and
> > > 
> > > pm8150b_adc_tm: adc-tm@3500 {
> > >       compatible = "qcom,spmi-adc-tm5";
> > > };
> > > Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml
> > > 
> > > to which I'm adding :
> > > 
> > > pm8150b_typec: typec@1500 {
> > >       compatible = "qcom,pm8150b-typec";
> > > };
> > > 
> > > Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml
> > > 
> > > pm8150b_pdphy: pdphy@1700 {
> > >       compatible = "qcom,pm8150b-pdphy";
> > > };
> > > 
> > > Documentation/devicetree/bindings/usb/qcom,pmic-pdphy.yaml
> > 
> >  From what I gather, there is not a 3rd h/w device this binding
> > describes, but it is just a collection of all the data you happen to
> > want for your driver.
> 
> The TCPM "virtual" driver presents as a device to the TCPM API and then uses
> phandle to talk to the PDPHY and typec devices yes.

That's nice, but it doesn't belong in DT.

>  That's assuming a specific structure for a
> > specific OS. Why can't most of this binding be part of
> > "qcom,pm8150b-typec" instead of making up some virtual device?
> 
> I thought it was a better model to have the TCPM be a separate device with
> the pdphy and typec blocks as their own devices.
> 
> #1 Because the address space spans over more than just the pdphy and typec
> device, there's a charger block in between
>
> #2 Because the pdphy and typec have separate IRQ lines and register spaces

I didn't say combine them. That would be once again structuring your h/w 
description to match your driver architecture. Bind your driver to 
"qcom,pm8150b-typec" and then it can retrieve the "qcom,pm8150b-pdphy" 
node which doesn't have a driver. There's no rule that nodes and drivers 
are 1:1, or that a driver can't access DT data in another node.

Your other option is instantiate your own device from the virtual 
driver's initcall based on presence of the 2 nodes above. 

Rob