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

[Bryan O'Donoghue]

On 17/01/2024 18:05, Dmitry Baryshkov wrote:
> On Wed, 17 Jan 2024 at 19:23, Bryan O'Donoghue
> <bryan.odonoghue at linaro.org> wrote:
>>
>> On 15/01/2024 10:43, Dmitry Baryshkov wrote:
>>> On Mon, 15 Jan 2024 at 12:00, Konrad Dybcio <konrad.dybcio at 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 at linaro.org>
>>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at 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 at 2 {
>>>>>                 #address-cells = <1>;
>>>>>                 #size-cells = <0>;
>>>>>
>>>>> +             pmi632_vbus: usb-vbus-regulator at 1100 {
>>>>> +                     compatible = "qcom,pmi632-vbus-reg", "qcom,pm8150b-vbus-reg";
>>>>> +                     reg = <0x1100>;
>>>>> +                     status = "disabled";
>>>>> +             };
>>>>> +
>>>>> +             pmi632_typec: typec at 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 at 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 at linaro.org>