Message ID | 20250318-xps13-no-pm8010-v1-1-c46236d96428@oss.qualcomm.com (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Series | arm64: dts: qcom: x1e80100-dell-xps13-9345: Disable PM8010 | expand |
On Tue, Mar 18, 2025 at 10:17:02PM -0500, Bjorn Andersson via B4 Relay wrote: > From: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com> > > The Qualcomm X Elite reference design uses the PM8010 PMIC for camera > use cases, but the Dell XPS13 doesn't. Disable this PMIC to avoid the > error in the kernel log caused by an attempt to access it during boot. > > Fixes: f5b788d0e8cd ("arm64: dts: qcom: Add support for X1-based Dell XPS 13 9345") > Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com> Usually we do the opposite: disable nodes by default that may or may not be there and enable them where needed. E.g. for the 4 SMB2360 instances in x1e80100-pmics.dtsi. I think the same approach would also be preferable here. You shouldn't get an error in the log just because you didn't go through all of your DT includes and checked if you really have all of the components listed there. I think it's okay to enable PMICs that are more or less guaranteed to be there, but clearly this is not the case for PM8010. Thanks, Stephan
On Wed, Mar 19, 2025 at 11:06:12AM +0100, Stephan Gerhold wrote: > On Tue, Mar 18, 2025 at 10:17:02PM -0500, Bjorn Andersson via B4 Relay wrote: > > From: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com> > > > > The Qualcomm X Elite reference design uses the PM8010 PMIC for camera > > use cases, but the Dell XPS13 doesn't. Disable this PMIC to avoid the > > error in the kernel log caused by an attempt to access it during boot. > > > > Fixes: f5b788d0e8cd ("arm64: dts: qcom: Add support for X1-based Dell XPS 13 9345") > > Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com> > > Usually we do the opposite: disable nodes by default that may or may not > be there and enable them where needed. E.g. for the 4 SMB2360 instances > in x1e80100-pmics.dtsi. > > I think the same approach would also be preferable here. You shouldn't > get an error in the log just because you didn't go through all of your > DT includes and checked if you really have all of the components listed > there. I think it's okay to enable PMICs that are more or less > guaranteed to be there, but clearly this is not the case for PM8010. I was just going to say the same. Johan
On Wed, Mar 19, 2025 at 11:06:12AM +0100, Stephan Gerhold wrote: > On Tue, Mar 18, 2025 at 10:17:02PM -0500, Bjorn Andersson via B4 Relay wrote: > > From: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com> > > > > The Qualcomm X Elite reference design uses the PM8010 PMIC for camera > > use cases, but the Dell XPS13 doesn't. Disable this PMIC to avoid the > > error in the kernel log caused by an attempt to access it during boot. > > > > Fixes: f5b788d0e8cd ("arm64: dts: qcom: Add support for X1-based Dell XPS 13 9345") > > Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com> > > Usually we do the opposite: disable nodes by default that may or may not > be there and enable them where needed. E.g. for the 4 SMB2360 instances > in x1e80100-pmics.dtsi. > > I think the same approach would also be preferable here. You shouldn't > get an error in the log just because you didn't go through all of your > DT includes and checked if you really have all of the components listed > there. I think it's okay to enable PMICs that are more or less > guaranteed to be there, but clearly this is not the case for PM8010. > That's reasonable. Have there been reports of this error from anyone else, or should I go ahead and enable &pm8010 on all !xps13 devices? Regards, Bjorn
diff --git a/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts b/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts index 967f6dba0878b51a985fd7c9570b8c4e71afe57d..684441bc3eb39ab2e8fd7dbb641a8ea75309901c 100644 --- a/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts +++ b/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts @@ -867,6 +867,10 @@ &pcie6a_phy { status = "okay"; }; +&pm8010 { + status = "disabled"; +}; + &pm8550_gpios { rtmr0_default: rtmr0-reset-n-active-state { pins = "gpio10";