[PATCH 1/3] ARM: dts: qcom: drop incorrect cell-index from SPMI

[PATCH 1/3] ARM: dts: qcom: drop incorrect cell-index from SPMI

[Krzysztof Kozlowski]

The SPMI controller (PMIC Arbiter) does not use nor allow 'cell-index'
property:

  qcom-sdx55-mtp.dtb: spmi@c440000: Unevaluated properties are not allowed ('cell-index' was unexpected)

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 arch/arm/boot/dts/qcom/qcom-sdx55.dtsi | 1 -
 arch/arm/boot/dts/qcom/qcom-sdx65.dtsi | 1 -
 2 files changed, 2 deletions(-)

diff --git a/arch/arm/boot/dts/qcom/qcom-sdx55.dtsi b/arch/arm/boot/dts/qcom/qcom-sdx55.dtsi
index 9c02d88e8eae..03da1ac9e43d 100644
--- a/arch/arm/boot/dts/qcom/qcom-sdx55.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-sdx55.dtsi
@@ -641,7 +641,6 @@ spmi_bus: spmi@c440000 {
 			#size-cells = <0>;
 			interrupt-controller;
 			#interrupt-cells = <4>;
-			cell-index = <0>;
 		};
 
 		tlmm: pinctrl@f100000 {
diff --git a/arch/arm/boot/dts/qcom/qcom-sdx65.dtsi b/arch/arm/boot/dts/qcom/qcom-sdx65.dtsi
index ff596215ae3c..49c16ee2e169 100644
--- a/arch/arm/boot/dts/qcom/qcom-sdx65.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-sdx65.dtsi
@@ -551,7 +551,6 @@ spmi_bus: spmi@c440000 {
 			#interrupt-cells = <4>;
 			#address-cells = <2>;
 			#size-cells = <0>;
-			cell-index = <0>;
 			qcom,channel = <0>;
 			qcom,ee = <0>;
 		};
-- 
2.34.1

[PATCH 2/3] arm64: dts: qcom: sc8180x: drop incorrect cell-index from SPMI

[Krzysztof Kozlowski]

The SPMI controller (PMIC Arbiter) does not use nor allow 'cell-index'
property:

  sc8180x-primus.dtb: spmi@c440000: Unevaluated properties are not allowed ('cell-index' was unexpected)

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 arch/arm64/boot/dts/qcom/sc8180x.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sc8180x.dtsi b/arch/arm64/boot/dts/qcom/sc8180x.dtsi
index 8fa296142eab..344ba444fcba 100644
--- a/arch/arm64/boot/dts/qcom/sc8180x.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8180x.dtsi
@@ -3299,7 +3299,6 @@ spmi_bus: spmi@c440000 {
 			#size-cells = <0>;
 			interrupt-controller;
 			#interrupt-cells = <4>;
-			cell-index = <0>;
 		};
 
 		apps_smmu: iommu@15000000 {
-- 
2.34.1

Re: [PATCH 2/3] arm64: dts: qcom: sc8180x: drop incorrect cell-index from SPMI

[Konrad Dybcio]

On 27.08.2023 14:28, Krzysztof Kozlowski wrote:
> The SPMI controller (PMIC Arbiter) does not use nor allow 'cell-index'
> property:
> 
>   sc8180x-primus.dtb: spmi@c440000: Unevaluated properties are not allowed ('cell-index' was unexpected)
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad

[PATCH 3/3] arm64: dts: qcom: sm7225-fp4: Revert "arm64: dts: qcom: sm7225-fairphone-fp4: Add AW8695 haptics"

[Krzysztof Kozlowski]

This reverts commit 413821b7777d062b57f8dc66ab088ed390cbc3ec because it
was never reviewed, was buggy (report from kernel test robot:
https://lore.kernel.org/all/202204090333.QZXMI2tu-lkp@intel.com/) and
used undocumented, broken bindings.  Half of the properties in this
device are questioned, thus adding DTS node causes only errors and does
not make the device usable without the bindings and driver part:

  sm7225-fairphone-fp4.dtb: haptics@5a: failed to match any schema with compatible: ['awinic,aw8695']
  sm7225-fairphone-fp4.dtb: haptics@5a: awinic,tset: b'\x12' is not of type 'object', 'array', 'boolean', 'null'
  sm7225-fairphone-fp4.dtb: haptics@5a: awinic,r-spare: b'h' is not of type 'object', 'array', 'boolean', 'null'

Since bindings were abandoned (4 months since review), revert the commit
to avoid false sense of supporting something which is not supported.

Cc: Luca Weiss <luca.weiss@fairphone.com>
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../boot/dts/qcom/sm7225-fairphone-fp4.dts    | 28 +------------------
 1 file changed, 1 insertion(+), 27 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts b/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
index 18171c5d8a38..568165f4f9e4 100644
--- a/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
+++ b/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
@@ -386,36 +386,10 @@ &i2c8 {
 };
 
 &i2c10 {
-	clock-frequency = <400000>;
-	status = "okay";
-
 	/* PM8008 PMIC @ 8 and 9 */
 	/* PX8618 @ 26 */
 	/* SMB1395 PMIC @ 34 */
-
-	haptics@5a {
-		compatible = "awinic,aw8695";
-		reg = <0x5a>;
-		interrupts-extended = <&tlmm 85 IRQ_TYPE_EDGE_FALLING>;
-		reset-gpios = <&tlmm 90 GPIO_ACTIVE_HIGH>;
-
-		awinic,f0-preset = <2350>;
-		awinic,f0-coefficient = <260>;
-		awinic,f0-calibration-percent = <7>;
-		awinic,drive-level = <125>;
-
-		awinic,f0-detection-play-time = <5>;
-		awinic,f0-detection-wait-time = <3>;
-		awinic,f0-detection-repeat = <2>;
-		awinic,f0-detection-trace = <15>;
-
-		awinic,boost-debug = /bits/ 8 <0x30 0xeb 0xd4>;
-		awinic,tset = /bits/ 8 <0x12>;
-		awinic,r-spare = /bits/ 8 <0x68>;
-
-		awinic,bemf-upper-threshold = <4104>;
-		awinic,bemf-lower-threshold = <1016>;
-	};
+	/* awinic,aw8695 @ 5a */
 };
 
 &ipa {
-- 
2.34.1

Re: [PATCH 3/3] arm64: dts: qcom: sm7225-fp4: Revert "arm64: dts: qcom: sm7225-fairphone-fp4: Add AW8695 haptics"

[Luca Weiss]

Hi Krzysztof,

On Sun Aug 27, 2023 at 2:28 PM CEST, Krzysztof Kozlowski wrote:
> This reverts commit 413821b7777d062b57f8dc66ab088ed390cbc3ec because it
> was never reviewed, was buggy (report from kernel test robot:
> https://lore.kernel.org/all/202204090333.QZXMI2tu-lkp@intel.com/) and

(I wouldn't say this part is accurate, the robot just didn't use a tree
with the i2c10 node already present, it was sent in an earlier patch
IIRC, but whatever)

> used undocumented, broken bindings.  Half of the properties in this
> device are questioned, thus adding DTS node causes only errors and does
> not make the device usable without the bindings and driver part:
>
>   sm7225-fairphone-fp4.dtb: haptics@5a: failed to match any schema with compatible: ['awinic,aw8695']
>   sm7225-fairphone-fp4.dtb: haptics@5a: awinic,tset: b'\x12' is not of type 'object', 'array', 'boolean', 'null'
>   sm7225-fairphone-fp4.dtb: haptics@5a: awinic,r-spare: b'h' is not of type 'object', 'array', 'boolean', 'null'
>
> Since bindings were abandoned (4 months since review), revert the commit
> to avoid false sense of supporting something which is not supported.

I've been avoiding touching this topic again since I'm really not sure
how to resolve.

There's a bunch of magic registers being written to in the downstream
driver, I don't have any documentation for that so I'm not exactly sure
what I can do to make nice bindings with proper properties.

Would you recommend just hardcoding some of these properties in the
driver, assuming they're constant for every AW8695, even though the
downstream driver has these properties in devicetree? Because of that I
assumed these properties could differ per implementation / usage of the
AW8695 in different devices.

Or do you have any other suggestion?

In any case:

Acked-by: Luca Weiss <luca.weiss@fairphone.com>

Regards
Luca

>
> Cc: Luca Weiss <luca.weiss@fairphone.com>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../boot/dts/qcom/sm7225-fairphone-fp4.dts    | 28 +------------------
>  1 file changed, 1 insertion(+), 27 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts b/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
> index 18171c5d8a38..568165f4f9e4 100644
> --- a/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
> +++ b/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
> @@ -386,36 +386,10 @@ &i2c8 {
>  };
>  
>  &i2c10 {
> -	clock-frequency = <400000>;
> -	status = "okay";
> -
>  	/* PM8008 PMIC @ 8 and 9 */
>  	/* PX8618 @ 26 */
>  	/* SMB1395 PMIC @ 34 */
> -
> -	haptics@5a {
> -		compatible = "awinic,aw8695";
> -		reg = <0x5a>;
> -		interrupts-extended = <&tlmm 85 IRQ_TYPE_EDGE_FALLING>;
> -		reset-gpios = <&tlmm 90 GPIO_ACTIVE_HIGH>;
> -
> -		awinic,f0-preset = <2350>;
> -		awinic,f0-coefficient = <260>;
> -		awinic,f0-calibration-percent = <7>;
> -		awinic,drive-level = <125>;
> -
> -		awinic,f0-detection-play-time = <5>;
> -		awinic,f0-detection-wait-time = <3>;
> -		awinic,f0-detection-repeat = <2>;
> -		awinic,f0-detection-trace = <15>;
> -
> -		awinic,boost-debug = /bits/ 8 <0x30 0xeb 0xd4>;
> -		awinic,tset = /bits/ 8 <0x12>;
> -		awinic,r-spare = /bits/ 8 <0x68>;
> -
> -		awinic,bemf-upper-threshold = <4104>;
> -		awinic,bemf-lower-threshold = <1016>;
> -	};
> +	/* awinic,aw8695 @ 5a */
>  };
>  
>  &ipa {

Re: [PATCH 3/3] arm64: dts: qcom: sm7225-fp4: Revert "arm64: dts: qcom: sm7225-fairphone-fp4: Add AW8695 haptics"

[Krzysztof Kozlowski]

On 28/08/2023 08:56, Luca Weiss wrote:
> Hi Krzysztof,
> 
> On Sun Aug 27, 2023 at 2:28 PM CEST, Krzysztof Kozlowski wrote:
>> This reverts commit 413821b7777d062b57f8dc66ab088ed390cbc3ec because it
>> was never reviewed, was buggy (report from kernel test robot:
>> https://lore.kernel.org/all/202204090333.QZXMI2tu-lkp@intel.com/) and
> 
> (I wouldn't say this part is accurate, the robot just didn't use a tree
> with the i2c10 node already present, it was sent in an earlier patch
> IIRC, but whatever)
> 
>> used undocumented, broken bindings.  Half of the properties in this
>> device are questioned, thus adding DTS node causes only errors and does
>> not make the device usable without the bindings and driver part:
>>
>>   sm7225-fairphone-fp4.dtb: haptics@5a: failed to match any schema with compatible: ['awinic,aw8695']
>>   sm7225-fairphone-fp4.dtb: haptics@5a: awinic,tset: b'\x12' is not of type 'object', 'array', 'boolean', 'null'
>>   sm7225-fairphone-fp4.dtb: haptics@5a: awinic,r-spare: b'h' is not of type 'object', 'array', 'boolean', 'null'
>>
>> Since bindings were abandoned (4 months since review), revert the commit
>> to avoid false sense of supporting something which is not supported.
> 
> I've been avoiding touching this topic again since I'm really not sure
> how to resolve.

Happens, but the DTS should not have been applied in such case.

> 
> There's a bunch of magic registers being written to in the downstream
> driver, I don't have any documentation for that so I'm not exactly sure
> what I can do to make nice bindings with proper properties.
> 
> Would you recommend just hardcoding some of these properties in the
> driver, assuming they're constant for every AW8695, even though the
> downstream driver has these properties in devicetree? Because of that I
> assumed these properties could differ per implementation / usage of the
> AW8695 in different devices.

Yes, keep them in the driver.

Best regards,
Krzysztof

Re: [PATCH 3/3] arm64: dts: qcom: sm7225-fp4: Revert "arm64: dts: qcom: sm7225-fairphone-fp4: Add AW8695 haptics"

[Luca Weiss]

On Mon Aug 28, 2023 at 9:00 AM CEST, Krzysztof Kozlowski wrote:
> On 28/08/2023 08:56, Luca Weiss wrote:
> > Hi Krzysztof,
> > 
> > On Sun Aug 27, 2023 at 2:28 PM CEST, Krzysztof Kozlowski wrote:
> >> This reverts commit 413821b7777d062b57f8dc66ab088ed390cbc3ec because it
> >> was never reviewed, was buggy (report from kernel test robot:
> >> https://lore.kernel.org/all/202204090333.QZXMI2tu-lkp@intel.com/) and
> > 
> > (I wouldn't say this part is accurate, the robot just didn't use a tree
> > with the i2c10 node already present, it was sent in an earlier patch
> > IIRC, but whatever)
> > 
> >> used undocumented, broken bindings.  Half of the properties in this
> >> device are questioned, thus adding DTS node causes only errors and does
> >> not make the device usable without the bindings and driver part:
> >>
> >>   sm7225-fairphone-fp4.dtb: haptics@5a: failed to match any schema with compatible: ['awinic,aw8695']
> >>   sm7225-fairphone-fp4.dtb: haptics@5a: awinic,tset: b'\x12' is not of type 'object', 'array', 'boolean', 'null'
> >>   sm7225-fairphone-fp4.dtb: haptics@5a: awinic,r-spare: b'h' is not of type 'object', 'array', 'boolean', 'null'
> >>
> >> Since bindings were abandoned (4 months since review), revert the commit
> >> to avoid false sense of supporting something which is not supported.
> > 
> > I've been avoiding touching this topic again since I'm really not sure
> > how to resolve.
>
> Happens, but the DTS should not have been applied in such case.

True, back when it was applied I told Bjorn but I also thought I'd get
the driver in soon also. Obviously this hasn't happened. So fine with me
to revert now, and I'll add it back once the new bindings are in.

>
> > 
> > There's a bunch of magic registers being written to in the downstream
> > driver, I don't have any documentation for that so I'm not exactly sure
> > what I can do to make nice bindings with proper properties.
> > 
> > Would you recommend just hardcoding some of these properties in the
> > driver, assuming they're constant for every AW8695, even though the
> > downstream driver has these properties in devicetree? Because of that I
> > assumed these properties could differ per implementation / usage of the
> > AW8695 in different devices.
>
> Yes, keep them in the driver.

Okay.. I'll make sure to document this in the driver or commit message
or somewhere so other people using the same AW8695 will know that this
is hardcoded.

Regards
Luca

>
> Best regards,
> Krzysztof

Re: [PATCH 3/3] arm64: dts: qcom: sm7225-fp4: Revert "arm64: dts: qcom: sm7225-fairphone-fp4: Add AW8695 haptics"

[Konrad Dybcio]

On 27.08.2023 14:28, Krzysztof Kozlowski wrote:
> This reverts commit 413821b7777d062b57f8dc66ab088ed390cbc3ec because it
> was never reviewed, was buggy (report from kernel test robot:
> https://lore.kernel.org/all/202204090333.QZXMI2tu-lkp@intel.com/) and
> used undocumented, broken bindings.  Half of the properties in this
> device are questioned, thus adding DTS node causes only errors and does
> not make the device usable without the bindings and driver part:
> 
>   sm7225-fairphone-fp4.dtb: haptics@5a: failed to match any schema with compatible: ['awinic,aw8695']
>   sm7225-fairphone-fp4.dtb: haptics@5a: awinic,tset: b'\x12' is not of type 'object', 'array', 'boolean', 'null'
>   sm7225-fairphone-fp4.dtb: haptics@5a: awinic,r-spare: b'h' is not of type 'object', 'array', 'boolean', 'null'
> 
> Since bindings were abandoned (4 months since review), revert the commit
> to avoid false sense of supporting something which is not supported.
> 
> Cc: Luca Weiss <luca.weiss@fairphone.com>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
Acked-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad

Re: [PATCH 1/3] ARM: dts: qcom: drop incorrect cell-index from SPMI

[Konrad Dybcio]

On 27.08.2023 14:28, Krzysztof Kozlowski wrote:
> The SPMI controller (PMIC Arbiter) does not use nor allow 'cell-index'
> property:
> 
>   qcom-sdx55-mtp.dtb: spmi@c440000: Unevaluated properties are not allowed ('cell-index' was unexpected)
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad

Re: (subset) [PATCH 1/3] ARM: dts: qcom: drop incorrect cell-index from SPMI

[Bjorn Andersson]

On Sun, 27 Aug 2023 14:28:40 +0200, Krzysztof Kozlowski wrote:
> The SPMI controller (PMIC Arbiter) does not use nor allow 'cell-index'
> property:
> 
>   qcom-sdx55-mtp.dtb: spmi@c440000: Unevaluated properties are not allowed ('cell-index' was unexpected)
> 
> 

Applied, thanks!

[1/3] ARM: dts: qcom: drop incorrect cell-index from SPMI
      commit: 340ed74de508e5d79599b67553ee1e8e8239e0d1

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>