[PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node

Sibi Sankar posted 5 patches 1 month ago
There is a newer version of this series
[PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node
Posted by Sibi Sankar 1 month ago
Add embedded controller node for Hamoa/Purwa CRDs which adds fan control,
temperature sensors, access to EC internal state changes and suspend
entry/exit notifications to the EC.

Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/x1-crd.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
index ded96fb43489..29a1aeb98353 100644
--- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
@@ -1042,6 +1042,16 @@ eusb6_repeater: redriver@4f {
 
 		#phy-cells = <0>;
 	};
+
+	embedded-controller@76 {
+		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";
+		reg = <0x76>;
+
+		interrupts-extended = <&tlmm 66 IRQ_TYPE_EDGE_FALLING>;
+
+		pinctrl-0 = <&ec_int_n_default>;
+		pinctrl-names = "default";
+	};
 };
 
 &i2c7 {
@@ -1485,6 +1495,12 @@ &tlmm {
 			       <44 4>, /* SPI (TPM) */
 			       <238 1>; /* UFS Reset */
 
+	ec_int_n_default: ec-int-n-state {
+		pins = "gpio66";
+		function = "gpio";
+		bias-disable;
+	};
+
 	edp_reg_en: edp-reg-en-state {
 		pins = "gpio70";
 		function = "gpio";
-- 
2.34.1
Re: [PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node
Posted by Krzysztof Kozlowski 1 month ago
On Mon, Mar 09, 2026 at 05:06:45AM +0530, Sibi Sankar wrote:
> Add embedded controller node for Hamoa/Purwa CRDs which adds fan control,
> temperature sensors, access to EC internal state changes and suspend
> entry/exit notifications to the EC.
> 
> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/x1-crd.dtsi | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
> index ded96fb43489..29a1aeb98353 100644
> --- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
> @@ -1042,6 +1042,16 @@ eusb6_repeater: redriver@4f {
>  
>  		#phy-cells = <0>;
>  	};
> +
> +	embedded-controller@76 {
> +		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";

I don't see updates to other x1e boards, thus my arguments from v2 stay
valid. It's wrong to call it "hamoa-ec" since only one Hamoa board has
it. All of other Hamoa boards apparently do not have it.

Best regards,
Krzysztof
Re: [PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node
Posted by Sibi Sankar 1 month ago
On 3/9/2026 12:55 PM, Krzysztof Kozlowski wrote:
> On Mon, Mar 09, 2026 at 05:06:45AM +0530, Sibi Sankar wrote:
>> Add embedded controller node for Hamoa/Purwa CRDs which adds fan control,
>> temperature sensors, access to EC internal state changes and suspend
>> entry/exit notifications to the EC.
>>
>> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>> ---
>>   arch/arm64/boot/dts/qcom/x1-crd.dtsi | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>> index ded96fb43489..29a1aeb98353 100644
>> --- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>> @@ -1042,6 +1042,16 @@ eusb6_repeater: redriver@4f {
>>   
>>   		#phy-cells = <0>;
>>   	};
>> +
>> +	embedded-controller@76 {
>> +		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";
> I don't see updates to other x1e boards, thus my arguments from v2 stay
> valid. It's wrong to call it "hamoa-ec" since only one Hamoa board has
> it. All of other Hamoa boards apparently do not have it.


Hey Krzysztof,
Thanks for taking time to review the series :)

What other Hamoa boards are you referring to? The series
mentions that the driver and bindings is meant for Qualcomm
Hamoa/Purwa/Glymur "reference" devices, so it only covers
CRD and IOT-EVK. It definitely does not cover all Hamoa boards
boards like you are assuming.

> Best regards,
> Krzysztof
>
Re: [PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node
Posted by Krzysztof Kozlowski 1 month ago
On 09/03/2026 10:03, Sibi Sankar wrote:
> 
> On 3/9/2026 12:55 PM, Krzysztof Kozlowski wrote:
>> On Mon, Mar 09, 2026 at 05:06:45AM +0530, Sibi Sankar wrote:
>>> Add embedded controller node for Hamoa/Purwa CRDs which adds fan control,
>>> temperature sensors, access to EC internal state changes and suspend
>>> entry/exit notifications to the EC.
>>>
>>> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>>> ---
>>>   arch/arm64/boot/dts/qcom/x1-crd.dtsi | 16 ++++++++++++++++
>>>   1 file changed, 16 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>> index ded96fb43489..29a1aeb98353 100644
>>> --- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>> @@ -1042,6 +1042,16 @@ eusb6_repeater: redriver@4f {
>>>   
>>>   		#phy-cells = <0>;
>>>   	};
>>> +
>>> +	embedded-controller@76 {
>>> +		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";
>> I don't see updates to other x1e boards, thus my arguments from v2 stay
>> valid. It's wrong to call it "hamoa-ec" since only one Hamoa board has
>> it. All of other Hamoa boards apparently do not have it.
> 
> 
> Hey Krzysztof,
> Thanks for taking time to review the series :)
> 
> What other Hamoa boards are you referring to? The series
> mentions that the driver and bindings is meant for Qualcomm
> Hamoa/Purwa/Glymur "reference" devices, so it only covers
> CRD and IOT-EVK. It definitely does not cover all Hamoa boards
> boards like you are assuming.

hamoa-ec compatible implies that and that's something I raised in v2
already. You need a specific compatible.


Best regards,
Krzysztof
Re: [PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node
Posted by Sibi Sankar 1 month ago
On 3/9/2026 2:39 PM, Krzysztof Kozlowski wrote:
> On 09/03/2026 10:03, Sibi Sankar wrote:
>> On 3/9/2026 12:55 PM, Krzysztof Kozlowski wrote:
>>> On Mon, Mar 09, 2026 at 05:06:45AM +0530, Sibi Sankar wrote:
>>>> Add embedded controller node for Hamoa/Purwa CRDs which adds fan control,
>>>> temperature sensors, access to EC internal state changes and suspend
>>>> entry/exit notifications to the EC.
>>>>
>>>> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>>>> ---
>>>>    arch/arm64/boot/dts/qcom/x1-crd.dtsi | 16 ++++++++++++++++
>>>>    1 file changed, 16 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>> index ded96fb43489..29a1aeb98353 100644
>>>> --- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>> @@ -1042,6 +1042,16 @@ eusb6_repeater: redriver@4f {
>>>>    
>>>>    		#phy-cells = <0>;
>>>>    	};
>>>> +
>>>> +	embedded-controller@76 {
>>>> +		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";
>>> I don't see updates to other x1e boards, thus my arguments from v2 stay
>>> valid. It's wrong to call it "hamoa-ec" since only one Hamoa board has
>>> it. All of other Hamoa boards apparently do not have it.
>>
>> Hey Krzysztof,
>> Thanks for taking time to review the series :)
>>
>> What other Hamoa boards are you referring to? The series
>> mentions that the driver and bindings is meant for Qualcomm
>> Hamoa/Purwa/Glymur "reference" devices, so it only covers
>> CRD and IOT-EVK. It definitely does not cover all Hamoa boards
>> boards like you are assuming.
> hamoa-ec compatible implies that and that's something I raised in v2
> already. You need a specific compatible.


Hamoa/Glymur reference devices can have different EC MCUs
depending on the SKU. This introduces the need to deal with
possibility of quirks and bugs introduced by these differences.
Hamoa/Purwa CRDs and IOT EVK runs on IT8987, while Glymur
reference devices run on NPCX498/488. This pretty much was
the rationale to make the MCU part of the compatible. Anyway
I can keep it as qcom,hamoa-ec and qcom,glymur-ec for now
and add specific compatibles when we upstream those boards?

>
>
> Best regards,
> Krzysztof
Re: [PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node
Posted by Dmitry Baryshkov 1 month ago
On Mon, Mar 09, 2026 at 04:21:50PM +0530, Sibi Sankar wrote:
> 
> On 3/9/2026 2:39 PM, Krzysztof Kozlowski wrote:
> > On 09/03/2026 10:03, Sibi Sankar wrote:
> > > On 3/9/2026 12:55 PM, Krzysztof Kozlowski wrote:
> > > > On Mon, Mar 09, 2026 at 05:06:45AM +0530, Sibi Sankar wrote:
> > > > > Add embedded controller node for Hamoa/Purwa CRDs which adds fan control,
> > > > > temperature sensors, access to EC internal state changes and suspend
> > > > > entry/exit notifications to the EC.
> > > > > 
> > > > > Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
> > > > > ---
> > > > >    arch/arm64/boot/dts/qcom/x1-crd.dtsi | 16 ++++++++++++++++
> > > > >    1 file changed, 16 insertions(+)
> > > > > 
> > > > > diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
> > > > > index ded96fb43489..29a1aeb98353 100644
> > > > > --- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
> > > > > +++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
> > > > > @@ -1042,6 +1042,16 @@ eusb6_repeater: redriver@4f {
> > > > >    		#phy-cells = <0>;
> > > > >    	};
> > > > > +
> > > > > +	embedded-controller@76 {
> > > > > +		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";
> > > > I don't see updates to other x1e boards, thus my arguments from v2 stay
> > > > valid. It's wrong to call it "hamoa-ec" since only one Hamoa board has
> > > > it. All of other Hamoa boards apparently do not have it.
> > > 
> > > Hey Krzysztof,
> > > Thanks for taking time to review the series :)
> > > 
> > > What other Hamoa boards are you referring to? The series
> > > mentions that the driver and bindings is meant for Qualcomm
> > > Hamoa/Purwa/Glymur "reference" devices, so it only covers
> > > CRD and IOT-EVK. It definitely does not cover all Hamoa boards
> > > boards like you are assuming.
> > hamoa-ec compatible implies that and that's something I raised in v2
> > already. You need a specific compatible.
> 
> 
> Hamoa/Glymur reference devices can have different EC MCUs
> depending on the SKU. This introduces the need to deal with
> possibility of quirks and bugs introduced by these differences.
> Hamoa/Purwa CRDs and IOT EVK runs on IT8987, while Glymur
> reference devices run on NPCX498/488. This pretty much was
> the rationale to make the MCU part of the compatible. Anyway
> I can keep it as qcom,hamoa-ec and qcom,glymur-ec for now
> and add specific compatibles when we upstream those boards?

Why do you need a generic compat at all? Glancing at the database, at
least the following laptops seem to have similar protocol (I might be
wrong, this is based on a very quick glance):

acer-sfa14-11
asus-vivobook-s-15
asus-vivobook-s15-q5507
asus-vivobook-s15-s5507
honor-magicbook-art-14
honor-mro-521
hp-elitebook-6-g1q
hp-omnibook-5-14-he0xxx
lenovo-ideacentre-01q8x10
lenovo-ideapad-slim3x-15q8x10-WCN7850
lenovo-thinkpad-t14s-120hz-64gb
lenovo-thinkpad-t14s
lenovo-yoga-slim-7x
medion-14-s1-elite-sprchrgd
medion-14-s1-sprchrgd
qualcomm-snapdragon-dev-kit
tuxedo-elite-14-gen1

I assume that some of them might be false positives and others will use
vendor (or device) extensions to your protocol.

I'd suggest implicitly using "qcom,hamoa-crd-ec" / "qcom,glymyr-crd-ec",
because then we can use something like "asus,vivobook-s15-ec" to
identify the EC on VivoBook S15.

> 
> > 
> > 
> > Best regards,
> > Krzysztof

-- 
With best wishes
Dmitry
Re: [PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node
Posted by Konrad Dybcio 1 month ago
On 3/9/26 10:13 PM, Dmitry Baryshkov wrote:
> On Mon, Mar 09, 2026 at 04:21:50PM +0530, Sibi Sankar wrote:
>>
>> On 3/9/2026 2:39 PM, Krzysztof Kozlowski wrote:
>>> On 09/03/2026 10:03, Sibi Sankar wrote:
>>>> On 3/9/2026 12:55 PM, Krzysztof Kozlowski wrote:
>>>>> On Mon, Mar 09, 2026 at 05:06:45AM +0530, Sibi Sankar wrote:
>>>>>> Add embedded controller node for Hamoa/Purwa CRDs which adds fan control,
>>>>>> temperature sensors, access to EC internal state changes and suspend
>>>>>> entry/exit notifications to the EC.
>>>>>>
>>>>>> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>>>>>> ---
>>>>>>    arch/arm64/boot/dts/qcom/x1-crd.dtsi | 16 ++++++++++++++++
>>>>>>    1 file changed, 16 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>> index ded96fb43489..29a1aeb98353 100644
>>>>>> --- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>> +++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>> @@ -1042,6 +1042,16 @@ eusb6_repeater: redriver@4f {
>>>>>>    		#phy-cells = <0>;
>>>>>>    	};
>>>>>> +
>>>>>> +	embedded-controller@76 {
>>>>>> +		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";
>>>>> I don't see updates to other x1e boards, thus my arguments from v2 stay
>>>>> valid. It's wrong to call it "hamoa-ec" since only one Hamoa board has
>>>>> it. All of other Hamoa boards apparently do not have it.
>>>>
>>>> Hey Krzysztof,
>>>> Thanks for taking time to review the series :)
>>>>
>>>> What other Hamoa boards are you referring to? The series
>>>> mentions that the driver and bindings is meant for Qualcomm
>>>> Hamoa/Purwa/Glymur "reference" devices, so it only covers
>>>> CRD and IOT-EVK. It definitely does not cover all Hamoa boards
>>>> boards like you are assuming.
>>> hamoa-ec compatible implies that and that's something I raised in v2
>>> already. You need a specific compatible.
>>
>>
>> Hamoa/Glymur reference devices can have different EC MCUs
>> depending on the SKU. This introduces the need to deal with
>> possibility of quirks and bugs introduced by these differences.
>> Hamoa/Purwa CRDs and IOT EVK runs on IT8987, while Glymur
>> reference devices run on NPCX498/488. This pretty much was
>> the rationale to make the MCU part of the compatible. Anyway
>> I can keep it as qcom,hamoa-ec and qcom,glymur-ec for now
>> and add specific compatibles when we upstream those boards?
> 
> Why do you need a generic compat at all? Glancing at the database, at
> least the following laptops seem to have similar protocol (I might be
> wrong, this is based on a very quick glance):
> 
> acer-sfa14-11
> asus-vivobook-s-15
> asus-vivobook-s15-q5507
> asus-vivobook-s15-s5507
> honor-magicbook-art-14
> honor-mro-521
> hp-elitebook-6-g1q
> hp-omnibook-5-14-he0xxx
> lenovo-ideacentre-01q8x10
> lenovo-ideapad-slim3x-15q8x10-WCN7850
> lenovo-thinkpad-t14s-120hz-64gb
> lenovo-thinkpad-t14s
> lenovo-yoga-slim-7x
> medion-14-s1-elite-sprchrgd
> medion-14-s1-sprchrgd
> qualcomm-snapdragon-dev-kit
> tuxedo-elite-14-gen1
> 
> I assume that some of them might be false positives and others will use
> vendor (or device) extensions to your protocol.
> 
> I'd suggest implicitly using "qcom,hamoa-crd-ec" / "qcom,glymyr-crd-ec",
> because then we can use something like "asus,vivobook-s15-ec" to
> identify the EC on VivoBook S15.

+1

Konrad
Re: [PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node
Posted by Sibi Sankar 1 month ago
On 3/10/2026 2:43 AM, Dmitry Baryshkov wrote:
> On Mon, Mar 09, 2026 at 04:21:50PM +0530, Sibi Sankar wrote:
>> On 3/9/2026 2:39 PM, Krzysztof Kozlowski wrote:
>>> On 09/03/2026 10:03, Sibi Sankar wrote:
>>>> On 3/9/2026 12:55 PM, Krzysztof Kozlowski wrote:
>>>>> On Mon, Mar 09, 2026 at 05:06:45AM +0530, Sibi Sankar wrote:
>>>>>> Add embedded controller node for Hamoa/Purwa CRDs which adds fan control,
>>>>>> temperature sensors, access to EC internal state changes and suspend
>>>>>> entry/exit notifications to the EC.
>>>>>>
>>>>>> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>>>>>> ---
>>>>>>     arch/arm64/boot/dts/qcom/x1-crd.dtsi | 16 ++++++++++++++++
>>>>>>     1 file changed, 16 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>> index ded96fb43489..29a1aeb98353 100644
>>>>>> --- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>> +++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>> @@ -1042,6 +1042,16 @@ eusb6_repeater: redriver@4f {
>>>>>>     		#phy-cells = <0>;
>>>>>>     	};
>>>>>> +
>>>>>> +	embedded-controller@76 {
>>>>>> +		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";
>>>>> I don't see updates to other x1e boards, thus my arguments from v2 stay
>>>>> valid. It's wrong to call it "hamoa-ec" since only one Hamoa board has
>>>>> it. All of other Hamoa boards apparently do not have it.
>>>> Hey Krzysztof,
>>>> Thanks for taking time to review the series :)
>>>>
>>>> What other Hamoa boards are you referring to? The series
>>>> mentions that the driver and bindings is meant for Qualcomm
>>>> Hamoa/Purwa/Glymur "reference" devices, so it only covers
>>>> CRD and IOT-EVK. It definitely does not cover all Hamoa boards
>>>> boards like you are assuming.
>>> hamoa-ec compatible implies that and that's something I raised in v2
>>> already. You need a specific compatible.
>>
>> Hamoa/Glymur reference devices can have different EC MCUs
>> depending on the SKU. This introduces the need to deal with
>> possibility of quirks and bugs introduced by these differences.
>> Hamoa/Purwa CRDs and IOT EVK runs on IT8987, while Glymur
>> reference devices run on NPCX498/488. This pretty much was
>> the rationale to make the MCU part of the compatible. Anyway
>> I can keep it as qcom,hamoa-ec and qcom,glymur-ec for now
>> and add specific compatibles when we upstream those boards?
> Why do you need a generic compat at all? Glancing at the database, at
> least the following laptops seem to have similar protocol (I might be
> wrong, this is based on a very quick glance):
>
> acer-sfa14-11
> asus-vivobook-s-15
> asus-vivobook-s15-q5507
> asus-vivobook-s15-s5507
> honor-magicbook-art-14
> honor-mro-521
> hp-elitebook-6-g1q
> hp-omnibook-5-14-he0xxx
> lenovo-ideacentre-01q8x10
> lenovo-ideapad-slim3x-15q8x10-WCN7850
> lenovo-thinkpad-t14s-120hz-64gb
> lenovo-thinkpad-t14s
> lenovo-yoga-slim-7x
> medion-14-s1-elite-sprchrgd
> medion-14-s1-sprchrgd
> qualcomm-snapdragon-dev-kit
> tuxedo-elite-14-gen1
>
> I assume that some of them might be false positives and others will use
> vendor (or device) extensions to your protocol.
>
> I'd suggest implicitly using "qcom,hamoa-crd-ec" / "qcom,glymyr-crd-ec",
> because then we can use something like "asus,vivobook-s15-ec" to
> identify the EC on VivoBook S15.

Ack, this was the consensus reached as well.

>
>>>
>>> Best regards,
>>> Krzysztof
Re: [PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node
Posted by Krzysztof Kozlowski 1 month ago
On 09/03/2026 11:51, Sibi Sankar wrote:
> 
> On 3/9/2026 2:39 PM, Krzysztof Kozlowski wrote:
>> On 09/03/2026 10:03, Sibi Sankar wrote:
>>> On 3/9/2026 12:55 PM, Krzysztof Kozlowski wrote:
>>>> On Mon, Mar 09, 2026 at 05:06:45AM +0530, Sibi Sankar wrote:
>>>>> Add embedded controller node for Hamoa/Purwa CRDs which adds fan control,
>>>>> temperature sensors, access to EC internal state changes and suspend
>>>>> entry/exit notifications to the EC.
>>>>>
>>>>> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>>>>> ---
>>>>>    arch/arm64/boot/dts/qcom/x1-crd.dtsi | 16 ++++++++++++++++
>>>>>    1 file changed, 16 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>> index ded96fb43489..29a1aeb98353 100644
>>>>> --- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>> +++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>> @@ -1042,6 +1042,16 @@ eusb6_repeater: redriver@4f {
>>>>>    
>>>>>    		#phy-cells = <0>;
>>>>>    	};
>>>>> +
>>>>> +	embedded-controller@76 {
>>>>> +		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";
>>>> I don't see updates to other x1e boards, thus my arguments from v2 stay
>>>> valid. It's wrong to call it "hamoa-ec" since only one Hamoa board has
>>>> it. All of other Hamoa boards apparently do not have it.
>>>
>>> Hey Krzysztof,
>>> Thanks for taking time to review the series :)
>>>
>>> What other Hamoa boards are you referring to? The series
>>> mentions that the driver and bindings is meant for Qualcomm
>>> Hamoa/Purwa/Glymur "reference" devices, so it only covers
>>> CRD and IOT-EVK. It definitely does not cover all Hamoa boards
>>> boards like you are assuming.
>> hamoa-ec compatible implies that and that's something I raised in v2
>> already. You need a specific compatible.
> 
> 
> Hamoa/Glymur reference devices can have different EC MCUs
> depending on the SKU. This introduces the need to deal with
> possibility of quirks and bugs introduced by these differences.
> Hamoa/Purwa CRDs and IOT EVK runs on IT8987, while Glymur
> reference devices run on NPCX498/488. This pretty much was

None of these answer my comments from here and v2.

> the rationale to make the MCU part of the compatible. Anyway
> I can keep it as qcom,hamoa-ec and qcom,glymur-ec for now

No. You cannot add a generic compatible when you claim it is not even
correct - "different EC depending on the SKU".

Best regards,
Krzysztof
Re: [PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node
Posted by Sibi Sankar 1 month ago
On 3/9/2026 4:23 PM, Krzysztof Kozlowski wrote:
> On 09/03/2026 11:51, Sibi Sankar wrote:
>> On 3/9/2026 2:39 PM, Krzysztof Kozlowski wrote:
>>> On 09/03/2026 10:03, Sibi Sankar wrote:
>>>> On 3/9/2026 12:55 PM, Krzysztof Kozlowski wrote:
>>>>> On Mon, Mar 09, 2026 at 05:06:45AM +0530, Sibi Sankar wrote:
>>>>>> Add embedded controller node for Hamoa/Purwa CRDs which adds fan control,
>>>>>> temperature sensors, access to EC internal state changes and suspend
>>>>>> entry/exit notifications to the EC.
>>>>>>
>>>>>> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>>>>>> ---
>>>>>>     arch/arm64/boot/dts/qcom/x1-crd.dtsi | 16 ++++++++++++++++
>>>>>>     1 file changed, 16 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>> index ded96fb43489..29a1aeb98353 100644
>>>>>> --- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>> +++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>> @@ -1042,6 +1042,16 @@ eusb6_repeater: redriver@4f {
>>>>>>     
>>>>>>     		#phy-cells = <0>;
>>>>>>     	};
>>>>>> +
>>>>>> +	embedded-controller@76 {
>>>>>> +		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";
>>>>> I don't see updates to other x1e boards, thus my arguments from v2 stay
>>>>> valid. It's wrong to call it "hamoa-ec" since only one Hamoa board has
>>>>> it. All of other Hamoa boards apparently do not have it.
>>>> Hey Krzysztof,
>>>> Thanks for taking time to review the series :)
>>>>
>>>> What other Hamoa boards are you referring to? The series
>>>> mentions that the driver and bindings is meant for Qualcomm
>>>> Hamoa/Purwa/Glymur "reference" devices, so it only covers
>>>> CRD and IOT-EVK. It definitely does not cover all Hamoa boards
>>>> boards like you are assuming.
>>> hamoa-ec compatible implies that and that's something I raised in v2
>>> already. You need a specific compatible.
>>
>> Hamoa/Glymur reference devices can have different EC MCUs
>> depending on the SKU. This introduces the need to deal with
>> possibility of quirks and bugs introduced by these differences.
>> Hamoa/Purwa CRDs and IOT EVK runs on IT8987, while Glymur
>> reference devices run on NPCX498/488. This pretty much was
> None of these answer my comments from here and v2.
>
>> the rationale to make the MCU part of the compatible. Anyway
>> I can keep it as qcom,hamoa-ec and qcom,glymur-ec for now
> No. You cannot add a generic compatible when you claim it is not even
> correct - "different EC depending on the SKU".

https://lore.kernel.org/lkml/1336e1c3-6ee9-4a19-976b-0bfdc02fc8e6@quicinc.com/

The explanation mentioned ^^ was the motivation
to have a extremely generic compatible as a fallback
since the firmware basically implements the same
exact specification.

https://lore.kernel.org/lkml/def949f2-301d-4edc-b303-0fbe02a18c3c@kernel.org/

I'll stick to your suggestion mentioned ^^ and use the
following compatibles instead:

qcom,hamoa-crd-it8987-ec
qcom,hamoa-iot-evk-it8987-ec
qcom,glymur-crd-ncpx498s-ec

I'll continue to use the hamoa-crd based compatible
as the fallback since they implement the same firmware
on different MCUs.

+properties: + compatible: + items: + - enum: + - 
qcom,glymur-crd-npcx498s-ec + - qcom,hamoa-iot-evk-it8987-ec + - const: 
qcom,hamoa-crd-it8987-ec Thanks again!

>
> Best regards,
> Krzysztof
Re: [PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node
Posted by Konrad Dybcio 1 month ago
On 3/9/26 11:53 AM, Krzysztof Kozlowski wrote:
> On 09/03/2026 11:51, Sibi Sankar wrote:
>>
>> On 3/9/2026 2:39 PM, Krzysztof Kozlowski wrote:
>>> On 09/03/2026 10:03, Sibi Sankar wrote:
>>>> On 3/9/2026 12:55 PM, Krzysztof Kozlowski wrote:
>>>>> On Mon, Mar 09, 2026 at 05:06:45AM +0530, Sibi Sankar wrote:
>>>>>> Add embedded controller node for Hamoa/Purwa CRDs which adds fan control,
>>>>>> temperature sensors, access to EC internal state changes and suspend
>>>>>> entry/exit notifications to the EC.
>>>>>>
>>>>>> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>>>>>> ---
>>>>>>    arch/arm64/boot/dts/qcom/x1-crd.dtsi | 16 ++++++++++++++++
>>>>>>    1 file changed, 16 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>> index ded96fb43489..29a1aeb98353 100644
>>>>>> --- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>> +++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>> @@ -1042,6 +1042,16 @@ eusb6_repeater: redriver@4f {
>>>>>>    
>>>>>>    		#phy-cells = <0>;
>>>>>>    	};
>>>>>> +
>>>>>> +	embedded-controller@76 {
>>>>>> +		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";
>>>>> I don't see updates to other x1e boards, thus my arguments from v2 stay
>>>>> valid. It's wrong to call it "hamoa-ec" since only one Hamoa board has
>>>>> it. All of other Hamoa boards apparently do not have it.
>>>>
>>>> Hey Krzysztof,
>>>> Thanks for taking time to review the series :)
>>>>
>>>> What other Hamoa boards are you referring to? The series
>>>> mentions that the driver and bindings is meant for Qualcomm
>>>> Hamoa/Purwa/Glymur "reference" devices, so it only covers
>>>> CRD and IOT-EVK. It definitely does not cover all Hamoa boards
>>>> boards like you are assuming.
>>> hamoa-ec compatible implies that and that's something I raised in v2
>>> already. You need a specific compatible.
>>
>>
>> Hamoa/Glymur reference devices can have different EC MCUs
>> depending on the SKU. This introduces the need to deal with
>> possibility of quirks and bugs introduced by these differences.
>> Hamoa/Purwa CRDs and IOT EVK runs on IT8987, while Glymur
>> reference devices run on NPCX498/488. This pretty much was
> 
> None of these answer my comments from here and v2.
> 
>> the rationale to make the MCU part of the compatible. Anyway
>> I can keep it as qcom,hamoa-ec and qcom,glymur-ec for now
> 
> No. You cannot add a generic compatible when you claim it is not even
> correct - "different EC depending on the SKU".

I agree, this name isn't really the best. We don't really have a better
"official one" though. Perhaps something like "qcom,compute-ec"?

"qcom,reference-compute-ec-that-happens-to-be-found-on-boards-featuring-
hamoa-glymur-and-derivative-socs-running-windows-by-design"?

Konrad
Re: [PATCH V3 4/5] arm64: dts: qcom: x1-crd: Add Embedded controller node
Posted by Krzysztof Kozlowski 1 month ago
On 09/03/2026 12:48, Konrad Dybcio wrote:
> On 3/9/26 11:53 AM, Krzysztof Kozlowski wrote:
>> On 09/03/2026 11:51, Sibi Sankar wrote:
>>>
>>> On 3/9/2026 2:39 PM, Krzysztof Kozlowski wrote:
>>>> On 09/03/2026 10:03, Sibi Sankar wrote:
>>>>> On 3/9/2026 12:55 PM, Krzysztof Kozlowski wrote:
>>>>>> On Mon, Mar 09, 2026 at 05:06:45AM +0530, Sibi Sankar wrote:
>>>>>>> Add embedded controller node for Hamoa/Purwa CRDs which adds fan control,
>>>>>>> temperature sensors, access to EC internal state changes and suspend
>>>>>>> entry/exit notifications to the EC.
>>>>>>>
>>>>>>> Signed-off-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
>>>>>>> ---
>>>>>>>    arch/arm64/boot/dts/qcom/x1-crd.dtsi | 16 ++++++++++++++++
>>>>>>>    1 file changed, 16 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>>> index ded96fb43489..29a1aeb98353 100644
>>>>>>> --- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>>> +++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
>>>>>>> @@ -1042,6 +1042,16 @@ eusb6_repeater: redriver@4f {
>>>>>>>    
>>>>>>>    		#phy-cells = <0>;
>>>>>>>    	};
>>>>>>> +
>>>>>>> +	embedded-controller@76 {
>>>>>>> +		compatible = "qcom,hamoa-it8987-ec", "qcom,hamoa-ec";
>>>>>> I don't see updates to other x1e boards, thus my arguments from v2 stay
>>>>>> valid. It's wrong to call it "hamoa-ec" since only one Hamoa board has
>>>>>> it. All of other Hamoa boards apparently do not have it.
>>>>>
>>>>> Hey Krzysztof,
>>>>> Thanks for taking time to review the series :)
>>>>>
>>>>> What other Hamoa boards are you referring to? The series
>>>>> mentions that the driver and bindings is meant for Qualcomm
>>>>> Hamoa/Purwa/Glymur "reference" devices, so it only covers
>>>>> CRD and IOT-EVK. It definitely does not cover all Hamoa boards
>>>>> boards like you are assuming.
>>>> hamoa-ec compatible implies that and that's something I raised in v2
>>>> already. You need a specific compatible.
>>>
>>>
>>> Hamoa/Glymur reference devices can have different EC MCUs
>>> depending on the SKU. This introduces the need to deal with
>>> possibility of quirks and bugs introduced by these differences.
>>> Hamoa/Purwa CRDs and IOT EVK runs on IT8987, while Glymur
>>> reference devices run on NPCX498/488. This pretty much was
>>
>> None of these answer my comments from here and v2.
>>
>>> the rationale to make the MCU part of the compatible. Anyway
>>> I can keep it as qcom,hamoa-ec and qcom,glymur-ec for now
>>
>> No. You cannot add a generic compatible when you claim it is not even
>> correct - "different EC depending on the SKU".
> 
> I agree, this name isn't really the best. We don't really have a better
> "official one" though. Perhaps something like "qcom,compute-ec"?
> 
> "qcom,reference-compute-ec-that-happens-to-be-found-on-boards-featuring-
> hamoa-glymur-and-derivative-socs-running-windows-by-design"?

I asked at v2 and here I asked twice, so three times already. You need
compatible for board. Nothing else.


Best regards,
Krzysztof