| Message ID | 20240217163201.32989-4-danila@jiaxyga.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm64: dts: qcom: pm6150: Add typec support for PM6150 | expand |
On Sat, 17 Feb 2024 at 18:32, Danila Tikhonov <danila@jiaxyga.com> wrote: > > Define VBUS regulator and the Type-C handling block as present on the > Quacomm PM6150 PMIC. > > Signed-off-by: Danila Tikhonov <danila@jiaxyga.com> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > arch/arm64/boot/dts/qcom/pm6150.dtsi | 46 ++++++++++++++++++++++++++++ > 1 file changed, 46 insertions(+)
On 17/02/2024 16:32, Danila Tikhonov wrote: > Define VBUS regulator and the Type-C handling block as present on the > Quacomm PM6150 PMIC. > > Signed-off-by: Danila Tikhonov <danila@jiaxyga.com> > + pm6150_typec: typec@1500 { > + compatible = "qcom,pm6150-typec, > + qcom,pm8150b-typec"; > + reg = <0x1500>, <0x1700>; > + interrupts = <0x0 0x15 0x00 IRQ_TYPE_EDGE_RISING>, > + <0x0 0x15 0x01 IRQ_TYPE_EDGE_RISING>, > + <0x0 0x15 0x02 IRQ_TYPE_EDGE_RISING>, > + <0x0 0x15 0x03 IRQ_TYPE_EDGE_RISING>, > + <0x0 0x15 0x04 IRQ_TYPE_EDGE_RISING>, > + <0x0 0x15 0x05 IRQ_TYPE_EDGE_RISING>, > + <0x0 0x15 0x06 IRQ_TYPE_EDGE_RISING>, > + <0x0 0x15 0x07 IRQ_TYPE_EDGE_RISING>, > + <0x0 0x17 0x00 IRQ_TYPE_EDGE_RISING>, > + <0x0 0x17 0x01 IRQ_TYPE_EDGE_RISING>, > + <0x0 0x17 0x02 IRQ_TYPE_EDGE_RISING>, > + <0x0 0x17 0x03 IRQ_TYPE_EDGE_RISING>, > + <0x0 0x17 0x04 IRQ_TYPE_EDGE_RISING>, > + <0x0 0x17 0x05 IRQ_TYPE_EDGE_RISING>, > + <0x0 0x17 0x06 IRQ_TYPE_EDGE_RISING>, > + <0x0 0x17 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", > + "sig-tx", > + "sig-rx", > + "msg-tx", > + "msg-rx", > + "msg-tx-failed", > + "msg-tx-discarded", > + "msg-rx-discarded", > + "fr-swap"; > + status = "disabled"; > + }; Should all of these be rising ? Looks incorrect to me. Please review: arch/arm64/boot/dts/qcom/pm8150b.dtsi pm8150b_typec: typec@1500 { compatible = "qcom,pm8150b-typec"; status = "disabled"; reg = <0x1500>, <0x1700>; 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>, <0x2 0x17 0x00 IRQ_TYPE_EDGE_RISING>, <0x2 0x17 0x01 IRQ_TYPE_EDGE_RISING>, <0x2 0x17 0x02 IRQ_TYPE_EDGE_RISING>, <0x2 0x17 0x03 IRQ_TYPE_EDGE_RISING>, <0x2 0x17 0x04 IRQ_TYPE_EDGE_RISING>, <0x2 0x17 0x05 IRQ_TYPE_EDGE_RISING>, <0x2 0x17 0x06 IRQ_TYPE_EDGE_RISING>, <0x2 0x17 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", "sig-tx", "sig-rx", "msg-tx", "msg-rx", "msg-tx-failed", "msg-tx-discarded", "msg-rx-discarded", "fr-swap"; } --- bod
I know that some interrupts have both for PM8150B, but for PM6150 all interrupts are rising. Please look at the downstream kernel: https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/187022f2721d584ac4ec92c0ac1af77da487521d/arch/arm64/boot/dts/qcom/pm6150.dtsi#L319 https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/187022f2721d584ac4ec92c0ac1af77da487521d/arch/arm64/boot/dts/qcom/pm8150b.dtsi#L292 --- Best wishes Danila On 2/18/24 02:19, Bryan O'Donoghue wrote: > On 17/02/2024 16:32, Danila Tikhonov wrote: >> Define VBUS regulator and the Type-C handling block as present on the >> Quacomm PM6150 PMIC. >> >> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com> > >> + pm6150_typec: typec@1500 { >> + compatible = "qcom,pm6150-typec, >> + qcom,pm8150b-typec"; >> + reg = <0x1500>, <0x1700>; >> + interrupts = <0x0 0x15 0x00 IRQ_TYPE_EDGE_RISING>, >> + <0x0 0x15 0x01 IRQ_TYPE_EDGE_RISING>, >> + <0x0 0x15 0x02 IRQ_TYPE_EDGE_RISING>, >> + <0x0 0x15 0x03 IRQ_TYPE_EDGE_RISING>, >> + <0x0 0x15 0x04 IRQ_TYPE_EDGE_RISING>, >> + <0x0 0x15 0x05 IRQ_TYPE_EDGE_RISING>, >> + <0x0 0x15 0x06 IRQ_TYPE_EDGE_RISING>, >> + <0x0 0x15 0x07 IRQ_TYPE_EDGE_RISING>, >> + <0x0 0x17 0x00 IRQ_TYPE_EDGE_RISING>, >> + <0x0 0x17 0x01 IRQ_TYPE_EDGE_RISING>, >> + <0x0 0x17 0x02 IRQ_TYPE_EDGE_RISING>, >> + <0x0 0x17 0x03 IRQ_TYPE_EDGE_RISING>, >> + <0x0 0x17 0x04 IRQ_TYPE_EDGE_RISING>, >> + <0x0 0x17 0x05 IRQ_TYPE_EDGE_RISING>, >> + <0x0 0x17 0x06 IRQ_TYPE_EDGE_RISING>, >> + <0x0 0x17 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", >> + "sig-tx", >> + "sig-rx", >> + "msg-tx", >> + "msg-rx", >> + "msg-tx-failed", >> + "msg-tx-discarded", >> + "msg-rx-discarded", >> + "fr-swap"; >> + status = "disabled"; >> + }; > > Should all of these be rising ? Looks incorrect to me. > > Please review: arch/arm64/boot/dts/qcom/pm8150b.dtsi > > pm8150b_typec: typec@1500 { > compatible = "qcom,pm8150b-typec"; > status = "disabled"; > reg = <0x1500>, > <0x1700>; > > 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>, > <0x2 0x17 0x00 IRQ_TYPE_EDGE_RISING>, > <0x2 0x17 0x01 IRQ_TYPE_EDGE_RISING>, > <0x2 0x17 0x02 IRQ_TYPE_EDGE_RISING>, > <0x2 0x17 0x03 IRQ_TYPE_EDGE_RISING>, > <0x2 0x17 0x04 IRQ_TYPE_EDGE_RISING>, > <0x2 0x17 0x05 IRQ_TYPE_EDGE_RISING>, > <0x2 0x17 0x06 IRQ_TYPE_EDGE_RISING>, > <0x2 0x17 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", > "sig-tx", > "sig-rx", > "msg-tx", > "msg-rx", > "msg-tx-failed", > "msg-tx-discarded", > "msg-rx-discarded", > "fr-swap"; > } > > --- > bod
On 18/02/2024 8:05 a.m., Danila Tikhonov wrote: > I know that some interrupts have both for PM8150B, but for PM6150 all > interrupts are rising. > Please look at the downstream kernel: > https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/187022f2721d584ac4ec92c0ac1af77da487521d/arch/arm64/boot/dts/qcom/pm6150.dtsi#L319 > https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/187022f2721d584ac4ec92c0ac1af77da487521d/arch/arm64/boot/dts/qcom/pm8150b.dtsi#L292 > Please take a look here, I think the same logic should apply to your patchset. https://www.spinics.net/lists/devicetree/msg665558.html --- bod
You are referring to Dmitry Baryshkov, as I see. But Dmitry has already reviewed my patch (message above). So it would be rude to change anything without his knowledge. Let's wait for his answer. --- Best wishes Danila On 2/18/24 20:14, Bryan O'Donoghue wrote: > On 18/02/2024 8:05 a.m., Danila Tikhonov wrote: >> I know that some interrupts have both for PM8150B, but for PM6150 all >> interrupts are rising. >> Please look at the downstream kernel: >> https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/187022f2721d584ac4ec92c0ac1af77da487521d/arch/arm64/boot/dts/qcom/pm6150.dtsi#L319 >> >> https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/187022f2721d584ac4ec92c0ac1af77da487521d/arch/arm64/boot/dts/qcom/pm8150b.dtsi#L292 >> >> > > > Please take a look here, I think the same logic should apply to your > patchset. > > https://www.spinics.net/lists/devicetree/msg665558.html > > --- > bod
On 18/02/2024 6:52 p.m., Danila Tikhonov wrote: > You are referring to Dmitry Baryshkov, as I see. But Dmitry has already > reviewed my patch (message above). Yes we previously debated and discussed verbatim copy of downstream versus the format we used for 8150b. The original driver I wrote for tcpm and the dts that went with it derived from 4.19 where the interrupt definition was already right, so in that case copy/paste of downstream is fine. However with earlier kernels, 4.14 in this case the signalling isn't right. Please read the discussion and reconsider your patch. > So it would be rude to change anything without his knowledge. Let's wait > for his answer He'd have to be arguing against his own patch..... One final nag - please use the kernel discussion format of bottom not top posting. https://git.codelinaro.org/bryan.odonoghue/kernel/-/blob/sc8280xp-v6.8-rc4-camss/Documentation/process/submitting-patches.rst?ref_type=heads --- bod
On Sun, 18 Feb 2024 at 20:53, Danila Tikhonov <danila@jiaxyga.com> wrote: > > You are referring to Dmitry Baryshkov, as I see. But Dmitry has already > reviewed my patch (message above). > So it would be rude to change anything without his knowledge. Let's wait > for his answer. I missed this point, so please update the IRQ flags accordingly to PM8150B, as Bryan has pointed out. > > --- > Best wishes > Danila > > On 2/18/24 20:14, Bryan O'Donoghue wrote: > > On 18/02/2024 8:05 a.m., Danila Tikhonov wrote: > >> I know that some interrupts have both for PM8150B, but for PM6150 all > >> interrupts are rising. > >> Please look at the downstream kernel: > >> https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/187022f2721d584ac4ec92c0ac1af77da487521d/arch/arm64/boot/dts/qcom/pm6150.dtsi#L319 > >> > >> https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/187022f2721d584ac4ec92c0ac1af77da487521d/arch/arm64/boot/dts/qcom/pm8150b.dtsi#L292 > >> > >> > > > > > > Please take a look here, I think the same logic should apply to your > > patchset. > > > > https://www.spinics.net/lists/devicetree/msg665558.html > > > > --- > > bod >
diff --git a/arch/arm64/boot/dts/qcom/pm6150.dtsi b/arch/arm64/boot/dts/qcom/pm6150.dtsi index ddbaf7280b03..bef5f28ba7cc 100644 --- a/arch/arm64/boot/dts/qcom/pm6150.dtsi +++ b/arch/arm64/boot/dts/qcom/pm6150.dtsi @@ -63,6 +63,52 @@ pm6150_resin: resin { }; }; + pm6150_vbus: usb-vbus-regulator@1100 { + compatible = "qcom,pm6150-vbus-reg, + qcom,pm8150b-vbus-reg"; + reg = <0x1100>; + status = "disabled"; + }; + + pm6150_typec: typec@1500 { + compatible = "qcom,pm6150-typec, + qcom,pm8150b-typec"; + reg = <0x1500>, <0x1700>; + interrupts = <0x0 0x15 0x00 IRQ_TYPE_EDGE_RISING>, + <0x0 0x15 0x01 IRQ_TYPE_EDGE_RISING>, + <0x0 0x15 0x02 IRQ_TYPE_EDGE_RISING>, + <0x0 0x15 0x03 IRQ_TYPE_EDGE_RISING>, + <0x0 0x15 0x04 IRQ_TYPE_EDGE_RISING>, + <0x0 0x15 0x05 IRQ_TYPE_EDGE_RISING>, + <0x0 0x15 0x06 IRQ_TYPE_EDGE_RISING>, + <0x0 0x15 0x07 IRQ_TYPE_EDGE_RISING>, + <0x0 0x17 0x00 IRQ_TYPE_EDGE_RISING>, + <0x0 0x17 0x01 IRQ_TYPE_EDGE_RISING>, + <0x0 0x17 0x02 IRQ_TYPE_EDGE_RISING>, + <0x0 0x17 0x03 IRQ_TYPE_EDGE_RISING>, + <0x0 0x17 0x04 IRQ_TYPE_EDGE_RISING>, + <0x0 0x17 0x05 IRQ_TYPE_EDGE_RISING>, + <0x0 0x17 0x06 IRQ_TYPE_EDGE_RISING>, + <0x0 0x17 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", + "sig-tx", + "sig-rx", + "msg-tx", + "msg-rx", + "msg-tx-failed", + "msg-tx-discarded", + "msg-rx-discarded", + "fr-swap"; + status = "disabled"; + }; + pm6150_temp: temp-alarm@2400 { compatible = "qcom,spmi-temp-alarm"; reg = <0x2400>;
Define VBUS regulator and the Type-C handling block as present on the Quacomm PM6150 PMIC. Signed-off-by: Danila Tikhonov <danila@jiaxyga.com> --- arch/arm64/boot/dts/qcom/pm6150.dtsi | 46 ++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+)