| Message ID | 20240113-pmi632-typec-v2-13-182d9aa0a5b3@linaro.org |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | usb: typec: qcom-pmic-typec: enable support for PMI632 PMIC | expand |
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
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!
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"; };
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.
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>
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 >
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
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?
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 --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>;