Re: [PATCH v2] ARM: dts: qcom: msm8960: expressatt: Add coreriver,tc360-touchkey

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `pm8921_l29` regulator provides a wide voltage range instead of a fixed voltage, which is incompatible with the `tc360-touchkey` driver that expects a fixed regulator.
- [Low] The newly added nodes `i2c-gpio-touchkey` and `touchkey-enable` violate standard devicetree node naming schemas and will trigger `dtbs_check` warnings.
--

commit df1b82e005a06f9e0f91c39a21c42b56f537d34d
Author: Rudraksha Gupta <guptarud@gmail.com>

ARM: dts: qcom: msm8960: expressatt: Add coreriver,tc360-touchkey

This commit adds the tc360 touchkey using an i2c-gpio bus for the msm8960
samsung expressatt device. It includes the necessary pin configurations,
regulators, and interrupt mappings.

Link: https://github.com/LineageOS/android_kernel_samsung_d2/blob/stable/cm-12.0-YNG4N/drivers/input/keyboard/cypress_touchkey_236/Makefile#L5

> diff --git a/arch/arm/boot/dts/qcom/qcom-msm8960-samsung-expressatt.dts b/arch/arm/boot/dts/qcom/qcom-msm8960-samsung-expressatt.dts
> --- a/arch/arm/boot/dts/qcom/qcom-msm8960-samsung-expressatt.dts
> +++ b/arch/arm/boot/dts/qcom/qcom-msm8960-samsung-expressatt.dts
> @@ -52,6 +52,40 @@ key-volume-down {
>  			linux,code = <KEY_VOLUMEDOWN>;
>  		};
>  	};
> +
> +	touchkey_enable: touchkey-enable {

[Severity: Low]
Will this node name trigger dtbs_check warnings? The fixed regulator schema
typically requires nodes to match the regular expression
^regulator(-[0-9]+v[0-9]+|-[0-9a-z-]+)?$. Would renaming this to something
like regulator-touchkey be more compliant?

> +		compatible = "regulator-fixed";
> +		regulator-name = "touchkey_enable";
> +		gpio = <&tlmm 51 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +		regulator-boot-on;
> +	};
> +
> +	i2c-gpio-touchkey {

[Severity: Low]
Does this node name violate the standard I2C controller schema? The standard
schema expects the bus node to match the regular expression ^i2c(@.*)?$.
Should this be named just i2c instead?

> +		compatible = "i2c-gpio";
> +		#address-cells = <1>;
> +		#size-cells = <0>;

[ ... ]

> @@ -420,6 +468,12 @@ pm8921_l25: l25 {
>  			bias-pull-down;
>  		};
>  
> +		pm8921_l29: l29 {
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <3300000>;

[Severity: High]
Will the wide voltage range here cause the touchkey to fail?

The tc360-touchkey driver is configured as a fixed regulator and only enables
it without calling regulator_set_voltage(). If the core regulator framework
enables this at the minimum voltage of 1.8V, will it be below the typical
voltage expected by the hardware? 

Should the min and max microvolts be constrained to the exact required fixed
voltage?

> +			bias-pull-down;
> +		};
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260527-expressatt-touchkey-v2-1-049dca41fc3a@gmail.com?part=1