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>
---
 arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts | 4 ++++
 1 file changed, 4 insertions(+)

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";

---
base-commit: c4d4884b67802c41fd67399747165d65c770621a
change-id: 20250318-xps13-no-pm8010-3822155f8f13

Best regards,
-- 
Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>

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.
> 

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

On 3/19/25 15:00, Bjorn Andersson wrote:
> 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?


Hi all,


Revisiting this change in context of Dell XPS 9345 and now Lenovo 
ThinkBook 16 [1] (and upcoming Lenovo Ideapad 5) as these do not have 
pm8010.


Perhaps safest and easiest is to simply disable pm8010 in 
`x1e80100-pmics.dtsi` as proposed and _not_ enable it on any devices, 
since its known to not be used anywhere _yet_? As its a camera PMIC, the 
only submitted upstream (did not land yet) change that utilizes pm8010 
on X1/X1E is Bryan's CAMSS series that enables camera on CRD [2]. There 
are a few other laptops that have patches to enable the camera (my 
Zenbook, Bryan's branch on Gitlab for few more laptops) but those were 
not submitted yet, so can be easily changed to explicitly enable pm8010 
before submission. This way we could simplify this change and not have 
to figure out which of the upstream devices do in fact have pm8010 onboard.


On XPS 9345 specifically failure to probe pm8010 leads to power button's 
short press not working for shutdown/suspend (only long press kill 
laptop power), which is a minor but not irrelevant detail, hence 
bringing this up.


Thanks,

Alex


[1] 
https://lore.kernel.org/all/20250607-tb16-dt-v6-4-61a31914ee72@oldschoolsolutions.biz/

[2] 
https://lore.kernel.org/all/20250417-b4-linux-next-25-03-13-dtsi-x1e80100-camss-v7-5-3fd4124cf35a@linaro.org/

>
> Regards,
> Bjorn
>

On 15/06/2025 19:29, Aleksandrs Vinarskis wrote:
> Hi all,
> 
> 
> Revisiting this change in context of Dell XPS 9345 and now Lenovo 
> ThinkBook 16 [1] (and upcoming Lenovo Ideapad 5) as these do not have 
> pm8010.
> 
> 
> Perhaps safest and easiest is to simply disable pm8010 in `x1e80100- 
> pmics.dtsi` as proposed and _not_ enable it on any devices, since its 
> known to not be used anywhere _yet_? As its a camera PMIC, the only 
> submitted upstream (did not land yet) change that utilizes pm8010 on X1/ 
> X1E is Bryan's CAMSS series that enables camera on CRD [2]. There are a 
> few other laptops that have patches to enable the camera (my Zenbook, 
> Bryan's branch on Gitlab for few more laptops) but those were not 
> submitted yet, so can be easily changed to explicitly enable pm8010 
> before submission. This way we could simplify this change and not have 
> to figure out which of the upstream devices do in fact have pm8010 onboard.

As a general principle, unless its used it should be disabled.

pm8010 is a camera specific PMIC and Dell has its own way of powering 
the camera sensors on XPS, Inspiron and I'd guess other models too, 
presumably as a result of reusing the x86 design with Qcom with minimal 
changes.

Disable by default. +1

---
bod

On 6/16/25 10:09 AM, Bryan O'Donoghue wrote:
> On 15/06/2025 19:29, Aleksandrs Vinarskis wrote:
>> Hi all,
>>
>>
>> Revisiting this change in context of Dell XPS 9345 and now Lenovo ThinkBook 16 [1] (and upcoming Lenovo Ideapad 5) as these do not have pm8010.
>>
>>
>> Perhaps safest and easiest is to simply disable pm8010 in `x1e80100- pmics.dtsi` as proposed and _not_ enable it on any devices, since its known to not be used anywhere _yet_? As its a camera PMIC, the only submitted upstream (did not land yet) change that utilizes pm8010 on X1/ X1E is Bryan's CAMSS series that enables camera on CRD [2]. There are a few other laptops that have patches to enable the camera (my Zenbook, Bryan's branch on Gitlab for few more laptops) but those were not submitted yet, so can be easily changed to explicitly enable pm8010 before submission. This way we could simplify this change and not have to figure out which of the upstream devices do in fact have pm8010 onboard.
> 
> As a general principle, unless its used it should be disabled.
> 
> pm8010 is a camera specific PMIC and Dell has its own way of powering the camera sensors on XPS, Inspiron and I'd guess other models too, presumably as a result of reusing the x86 design with Qcom with minimal changes.
> 
> Disable by default. +1

Sure, we can do that

Konrad

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