Re: [PATCH v2 13/15] arm64: dts: qcom: pmi632: define USB-C related blocks

[Dmitry Baryshkov <dmitry.baryshkov@linaro.org>]

On Wed, 17 Jan 2024 at 19:23, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> On 15/01/2024 10:43, Dmitry Baryshkov wrote:
> > On Mon, 15 Jan 2024 at 12:00, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
> >>
> >> On 13.01.2024 21:55, Dmitry Baryshkov wrote:
> >>> Define VBUS regulator and the Type-C handling block as present on the
> >>> Quacomm PMI632 PMIC.
> >>>
> >>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >>> ---
> >>>   arch/arm64/boot/dts/qcom/pmi632.dtsi | 30 ++++++++++++++++++++++++++++++
> >>>   1 file changed, 30 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/qcom/pmi632.dtsi b/arch/arm64/boot/dts/qcom/pmi632.dtsi
> >>> index 4eb79e0ce40a..d6832f0b7b80 100644
> >>> --- a/arch/arm64/boot/dts/qcom/pmi632.dtsi
> >>> +++ b/arch/arm64/boot/dts/qcom/pmi632.dtsi
> >>> @@ -45,6 +45,36 @@ pmic@2 {
> >>>                #address-cells = <1>;
> >>>                #size-cells = <0>;
> >>>
> >>> +             pmi632_vbus: usb-vbus-regulator@1100 {
> >>> +                     compatible = "qcom,pmi632-vbus-reg", "qcom,pm8150b-vbus-reg";
> >>> +                     reg = <0x1100>;
> >>> +                     status = "disabled";
> >>> +             };
> >>> +
> >>> +             pmi632_typec: typec@1500 {
> >>> +                     compatible = "qcom,pmi632-typec";
> >>> +                     reg = <0x1500>;
> >>> +                     interrupts = <0x2 0x15 0x00 IRQ_TYPE_EDGE_RISING>,
> >>> +                                  <0x2 0x15 0x01 IRQ_TYPE_EDGE_BOTH>,
> >>> +                                  <0x2 0x15 0x02 IRQ_TYPE_EDGE_RISING>,
> >>> +                                  <0x2 0x15 0x03 IRQ_TYPE_EDGE_BOTH>,
> >>> +                                  <0x2 0x15 0x04 IRQ_TYPE_EDGE_RISING>,
> >>> +                                  <0x2 0x15 0x05 IRQ_TYPE_EDGE_RISING>,
> >>> +                                  <0x2 0x15 0x06 IRQ_TYPE_EDGE_BOTH>,
> >>> +                                  <0x2 0x15 0x07 IRQ_TYPE_EDGE_RISING>;
> >> This differs from the downstream irq types:
> >>
> >> <0x2 0x15 0x0 IRQ_TYPE_EDGE_BOTH>,
> >> <0x2 0x15 0x1 IRQ_TYPE_EDGE_BOTH>,
> >> <0x2 0x15 0x2 IRQ_TYPE_EDGE_RISING>,
> >> <0x2 0x15 0x3 IRQ_TYPE_EDGE_RISING>,
> >> <0x2 0x15 0x4 IRQ_TYPE_EDGE_BOTH>,
> >> <0x2 0x15 0x5 IRQ_TYPE_EDGE_RISING>,
> >> <0x2 0x15 0x6 IRQ_TYPE_EDGE_RISING>,
> >> <0x2 0x15 0x7 IRQ_TYPE_EDGE_RISING>;
> >
> > I must admit, I copied the IRQs from the pm8150b rather than from the
> > vendor kernel.
> >
> > Bryan, any idea which set of flags is more correct?
>
> My € says 1:1 with the downstream pmi632.dtsi
>
> qcom,typec@1500 {
>      reg = <0x1500 0x100>;
>      interrupts = <0x2 0x15 0x0 IRQ_TYPE_EDGE_BOTH>,
>                   <0x2 0x15 0x1 IRQ_TYPE_EDGE_BOTH>,
>                   <0x2 0x15 0x2 IRQ_TYPE_EDGE_RISING>,
>                   <0x2 0x15 0x3 IRQ_TYPE_EDGE_RISING>,
>                   <0x2 0x15 0x4 IRQ_TYPE_EDGE_BOTH>,
>                   <0x2 0x15 0x5 IRQ_TYPE_EDGE_RISING>,
>                   <0x2 0x15 0x6 IRQ_TYPE_EDGE_RISING>,
>                   <0x2 0x15 0x7 IRQ_TYPE_EDGE_RISING>;
>
>      interrupt-names = "typec-or-rid-detect-change",

My 2c say that EDGE_BOTH doesn't make sense for or-rid-detect-change
at least. It is an "or" of several _pulse_ interrupts, so there is no
need to detect the falling edge.

>                        "typec-vpd-detect",

both, correct in both cases

>                        "typec-cc-state-change",

pulse interrupt, raising.

>                        "typec-vconn-oc",

It is a 'level' interrupt, so we probably want to detect both rising
and falling edges.

>                        "typec-vbus-change",

"pulse" interrupt => rising, not both, correct in PM8150B.

>                        "typec-attach-detach",

pulse interrupt, rising only.

>                        "typec-legacy-cable-detect",

level, should be both as in PM8150B.

>                        "typec-try-snk-src-detect";
> };

Pulse interrupt being raised when there is either successful or
unsuccessful try.SNK or try.SRC.

So, after consulting the documentation, I believe the flags being a
part of this patch (and in pm8150b.dtsi) are correct.

-- 
With best wishes
Dmitry