diff mbox series

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

Message ID 20240113-pmi632-typec-v2-13-182d9aa0a5b3@linaro.org
State Superseded
Headers show
Series usb: typec: qcom-pmic-typec: enable support for PMI632 PMIC | expand

Commit Message

Dmitry Baryshkov Jan. 13, 2024, 8:55 p.m. UTC
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(+)

Comments

Konrad Dybcio Jan. 15, 2024, 10 a.m. UTC | #1
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>;

Is it intended?

Thanks a lot for working on this!

Konrad
Dmitry Baryshkov Jan. 15, 2024, 10:43 a.m. UTC | #2
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?

> Is it intended?
>
> Thanks a lot for working on this!
Bryan O'Donoghue Jan. 17, 2024, 5:23 p.m. UTC | #3
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",
                       "typec-vpd-detect",
                       "typec-cc-state-change",
                       "typec-vconn-oc",
                       "typec-vbus-change",
                       "typec-attach-detach",
                       "typec-legacy-cable-detect",
                       "typec-try-snk-src-detect";
};
Dmitry Baryshkov Jan. 17, 2024, 6:05 p.m. UTC | #4
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.
Bryan O'Donoghue Jan. 17, 2024, 6:23 p.m. UTC | #5
On 17/01/2024 18:05, Dmitry Baryshkov wrote:
> 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.
> 

Ah.

The pm8150b state is more logical and if you are using it on pmi632 with 
the driver we have, I'm inclined to think also more correct.

Testing > theory.

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Bjorn Andersson Jan. 17, 2024, 10:01 p.m. UTC | #6
On Mon, Jan 15, 2024 at 11:00:53AM +0100, Konrad Dybcio 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>;
> 

Interrupt 1, 3, and 6 are level interrupts for which it's reasonable to
act on both edges. Interrupt 0, 2, 4, 5, and 7 are "pulse interrupts",
for which it seems reasonable to act on only one of the edges.

To me, Dmitry's proposed version makes more sense than downstream.

Regards,
Bjorn

> Is it intended?
> 
> Thanks a lot for working on this!
> 
> Konrad
>
Konrad Dybcio Jan. 18, 2024, 11:17 a.m. UTC | #7
On 1/17/24 23:01, Bjorn Andersson wrote:
> On Mon, Jan 15, 2024 at 11:00:53AM +0100, Konrad Dybcio 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>;
>>
> 
> Interrupt 1, 3, and 6 are level interrupts for which it's reasonable to
> act on both edges. Interrupt 0, 2, 4, 5, and 7 are "pulse interrupts",
> for which it seems reasonable to act on only one of the edges.
> 
> To me, Dmitry's proposed version makes more sense than downstream.

Thanks a lot for crosschecking!

Konrad
Dmitry Baryshkov Jan. 18, 2024, 1:38 p.m. UTC | #8
On Thu, 18 Jan 2024 at 13:17, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
>
>
> On 1/17/24 23:01, Bjorn Andersson wrote:
> > On Mon, Jan 15, 2024 at 11:00:53AM +0100, Konrad Dybcio 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>;
> >>
> >
> > Interrupt 1, 3, and 6 are level interrupts for which it's reasonable to
> > act on both edges. Interrupt 0, 2, 4, 5, and 7 are "pulse interrupts",
> > for which it seems reasonable to act on only one of the edges.
> >
> > To me, Dmitry's proposed version makes more sense than downstream.
>
> Thanks a lot for crosschecking!

Is there an ack?
Konrad Dybcio Jan. 18, 2024, 1:54 p.m. UTC | #9
On 1/18/24 14:38, Dmitry Baryshkov wrote:
> On Thu, 18 Jan 2024 at 13:17, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>
>>
>>
>> On 1/17/24 23:01, Bjorn Andersson wrote:
>>> On Mon, Jan 15, 2024 at 11:00:53AM +0100, Konrad Dybcio 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>
>>>>> ---

[...]

> Is there an ack?

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
diff mbox series

Patch

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>;
+			interrupt-names = "or-rid-detect-change",
+					  "vpd-detect",
+					  "cc-state-change",
+					  "vconn-oc",
+					  "vbus-change",
+					  "attach-detach",
+					  "legacy-cable-detect",
+					  "try-snk-src-detect";
+			vdd-vbus-supply = <&pmi632_vbus>;
+
+			status = "disabled";
+		};
+
 		pmi632_temp: temp-alarm@2400 {
 			compatible = "qcom,spmi-temp-alarm";
 			reg = <0x2400>;