Re: [PATCH v2 1/2] dt-bindings: Documentation for qcom,eud

[Dwivedi, Avaneesh Kumar (avani)]

Hi Rob,

We would like to revive this thread, We also would like to address your 
comments on this patch set.

Please consider our reply against your comments so that we can proceed.

On 9/26/2018 12:55 AM, Rob Herring wrote:
> On Tue, Sep 04, 2018 at 02:34:12PM -0700, Prakruthi Deepak Heragu wrote:
> > Documentation for Embedded USB Debugger (EUD) device tree bindings.
> >
> > Signed-off-by: Satya Durga Srinivasu Prabhala <satyap@xxxxxxxxxxxxxx>
> > Signed-off-by: Prakruthi Deepak Heragu <pheragu@xxxxxxxxxxxxxx>
> > ---
> >   .../devicetree/bindings/soc/qcom/qcom,msm-eud.txt  | 41 ++++++++++++++++++++++
> >   1 file changed, 41 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt
> >
> > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt
> > new file mode 100644
> > index 0000000..a03021a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt
> > @@ -0,0 +1,41 @@
> > +* Qualcomm Technologies Inc Embedded USB Debugger (EUD)
> > +
> > +The EUD (Embedded USB Debugger) is a mini-USB hub implemented
> > +on chip to support the USB-based debug and trace capabilities.
> Is it just for debug and normally bypassed?
Yes, In normal mode EUD is bypassed.
> > +
> > +Required properties:
> > +
> > + - compatible:  Should be "qcom,msm-eud"
> Needs to be SoC specific (though a fallback is fine).

This IP will be present in all qcom SoC's that is why this is specific 
to qcom, let us know if this is anyway problematic?

> > + - interrupts:  Interrupt number
> > + - reg: Should be address and size of EUD register space
> > +
> > +Driver notifies clients for VBUS attach/detach and charger enable/disable
> Bindings are for h/w blocks, not drivers.
This has been addressed in patch set v3 which is pending for your review.
> > +events. The link between client and EUD is established via a directed
> > +graph. EUD driver has one endpoint of the graph link mentioning EUD as an
> > +output link and clients registered for these notifications from the EUD
> > +should have the other endpoint of the graph link as an input link.
> OF graph is for describing data flows (i.e. h/w connections), not
> clients wanting some event.

Will rephrasing above description as below would work?

"The link between event receiver and EUD is established via a directed 
graph. Where EUD act as output link and event receiver(ex. usb 
controller or charger h/w)Â as input link"

> > Each of
> > +these endpoints should contain a 'remote-endpoint' phandle property that
> > +points to the corresponding endpoint in the port of the remote device.
> You don't need to describe how the graph binding works. Just what the
> port assignments are.

patch set v3 has removed the part describing how graph binding works.

> I worry this is going to collide with using the graph binding for USB
> connectors.

Can you please elaborate your query little more? USB controller has 
input connection from EUD as well as USB connectors.

As mentioned earlier, usb controller receive event from EUD only in 
debug mode while in normal mode USB connector supplies the event.

does that address concern?

> > +
> > +An example for EUD device node:
> > +
> > +	eud: qcom,msm-eud@88e0000 {
> > +		compatible = "qcom,msm-eud";
> > +		interrupts = <GIC_SPI 492 IRQ_TYPE_LEVEL_HIGH>;
> > +		reg = <0x88e0000 0x4000>;
> > +		port {
> > +                	eud_output: endpoint {
> > +                        	remote-endpoint = <&usb3_input>;
> > +                        };
> > +        	};
> > +	};
> > +
> > +An example for EUD client:
> What are possible clients? Could we want to switch clients at runtime?

As of now clients are usb controller and charger hardware, and they are 
fixed.

EUD application decide events dynamically.

> > +
> > +	usb3 {
> > +		port {
> > +                	usb3_input: endpoint {
> > +                        	remote-endpoint = <&eud_output>;
> > +                        };
> > +        	};
> > +	};
> > --
> > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> > a Linux Foundation Collaborative Project
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.