[PATCH v2] platform/x86: int3472: Use actual clock frequency for DSM method

[PATCH v2] platform/x86: int3472: Use actual clock frequency for DSM method

[Hao Yao]

The third argument (args[2]) to the _DSM method was hardcoded to 1,
which corresponds to 19.2MHz. However, this argument should reflect
the actual clock frequency from the sensor's ACPI data.

According to the DSM specification:
- 1 = 19.2MHz
- 3 = 24MHz

Read the frequency from clk->frequency and set the DSM argument
accordingly, with 19.2MHz as the default for unsupported frequencies.

This ensures the sensor receives the correct clock frequency as
specified in its ACPI configuration.

Signed-off-by: Hao Yao <hao.yao@intel.com>
---
 .../x86/intel/int3472/clk_and_regulator.c     | 21 ++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel/int3472/clk_and_regulator.c b/drivers/platform/x86/intel/int3472/clk_and_regulator.c
index 9e052b164a1a..c834fd59ec0c 100644
--- a/drivers/platform/x86/intel/int3472/clk_and_regulator.c
+++ b/drivers/platform/x86/intel/int3472/clk_and_regulator.c
@@ -19,23 +19,42 @@ static const guid_t img_clk_guid =
 	GUID_INIT(0x82c0d13a, 0x78c5, 0x4244,
 		  0x9b, 0xb1, 0xeb, 0x8b, 0x53, 0x9a, 0x8d, 0x11);
 
+/*
+ * The PCH clock frequency argument to the _DSM method:
+ * PCH_CLK_FREQ_19M2 = 19.2MHz (default)
+ * PCH_CLK_FREQ_24M = 24MHz
+ */
+#define PCH_CLK_FREQ_19M2	1
+#define PCH_CLK_FREQ_24M	3
+
 static void skl_int3472_enable_clk(struct int3472_clock *clk, int enable)
 {
 	struct int3472_discrete_device *int3472 = to_int3472_device(clk);
 	union acpi_object args[3];
 	union acpi_object argv4;
+	u32 dsm_freq_arg;
 
 	if (clk->ena_gpio) {
 		gpiod_set_value_cansleep(clk->ena_gpio, enable);
 		return;
 	}
 
+	switch (clk->frequency) {
+	case 24000000:
+		dsm_freq_arg = PCH_CLK_FREQ_24M;
+		break;
+	case 19200000:
+	default:
+		dsm_freq_arg = PCH_CLK_FREQ_19M2;
+		break;
+	}
+
 	args[0].integer.type = ACPI_TYPE_INTEGER;
 	args[0].integer.value = clk->imgclk_index;
 	args[1].integer.type = ACPI_TYPE_INTEGER;
 	args[1].integer.value = enable;
 	args[2].integer.type = ACPI_TYPE_INTEGER;
-	args[2].integer.value = 1;
+	args[2].integer.value = dsm_freq_arg;
 
 	argv4.type = ACPI_TYPE_PACKAGE;
 	argv4.package.count = 3;
-- 
2.43.0

Re: [PATCH v2] platform/x86: int3472: Use actual clock frequency for DSM method

[Dan Scally]

Morning Hao, thanks for the patch

On 08/12/2025 03:51, Hao Yao wrote:
> The third argument (args[2]) to the _DSM method was hardcoded to 1,
> which corresponds to 19.2MHz. However, this argument should reflect
> the actual clock frequency from the sensor's ACPI data.
> 
> According to the DSM specification:
> - 1 = 19.2MHz
> - 3 = 24MHz
> 
> Read the frequency from clk->frequency and set the DSM argument
> accordingly, with 19.2MHz as the default for unsupported frequencies.
> 
> This ensures the sensor receives the correct clock frequency as
> specified in its ACPI configuration.
> 
> Signed-off-by: Hao Yao <hao.yao@intel.com>

Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

> ---
>   .../x86/intel/int3472/clk_and_regulator.c     | 21 ++++++++++++++++++-
>   1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/intel/int3472/clk_and_regulator.c b/drivers/platform/x86/intel/int3472/clk_and_regulator.c
> index 9e052b164a1a..c834fd59ec0c 100644
> --- a/drivers/platform/x86/intel/int3472/clk_and_regulator.c
> +++ b/drivers/platform/x86/intel/int3472/clk_and_regulator.c
> @@ -19,23 +19,42 @@ static const guid_t img_clk_guid =
>   	GUID_INIT(0x82c0d13a, 0x78c5, 0x4244,
>   		  0x9b, 0xb1, 0xeb, 0x8b, 0x53, 0x9a, 0x8d, 0x11);
>   
> +/*
> + * The PCH clock frequency argument to the _DSM method:
> + * PCH_CLK_FREQ_19M2 = 19.2MHz (default)
> + * PCH_CLK_FREQ_24M = 24MHz
> + */
> +#define PCH_CLK_FREQ_19M2	1
> +#define PCH_CLK_FREQ_24M	3
> +
>   static void skl_int3472_enable_clk(struct int3472_clock *clk, int enable)
>   {
>   	struct int3472_discrete_device *int3472 = to_int3472_device(clk);
>   	union acpi_object args[3];
>   	union acpi_object argv4;
> +	u32 dsm_freq_arg;
>   
>   	if (clk->ena_gpio) {
>   		gpiod_set_value_cansleep(clk->ena_gpio, enable);
>   		return;
>   	}
>   
> +	switch (clk->frequency) {
> +	case 24000000:
> +		dsm_freq_arg = PCH_CLK_FREQ_24M;
> +		break;
> +	case 19200000:
> +	default:
> +		dsm_freq_arg = PCH_CLK_FREQ_19M2;
> +		break;
> +	}
> +
>   	args[0].integer.type = ACPI_TYPE_INTEGER;
>   	args[0].integer.value = clk->imgclk_index;
>   	args[1].integer.type = ACPI_TYPE_INTEGER;
>   	args[1].integer.value = enable;
>   	args[2].integer.type = ACPI_TYPE_INTEGER;
> -	args[2].integer.value = 1;
> +	args[2].integer.value = dsm_freq_arg;
>   
>   	argv4.type = ACPI_TYPE_PACKAGE;
>   	argv4.package.count = 3;

Re: [PATCH v2] platform/x86: int3472: Use actual clock frequency for DSM method

[johannes.goede]

Hi,

On 8-Dec-25 4:51 AM, Hao Yao wrote:
> The third argument (args[2]) to the _DSM method was hardcoded to 1,
> which corresponds to 19.2MHz. However, this argument should reflect
> the actual clock frequency from the sensor's ACPI data.
> 
> According to the DSM specification:
> - 1 = 19.2MHz
> - 3 = 24MHz
> 
> Read the frequency from clk->frequency and set the DSM argument
> accordingly, with 19.2MHz as the default for unsupported frequencies.
> 
> This ensures the sensor receives the correct clock frequency as
> specified in its ACPI configuration.
> 
> Signed-off-by: Hao Yao <hao.yao@intel.com>

Thanks patch looks good to me:

Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>

Regards,

Hans


> ---
>  .../x86/intel/int3472/clk_and_regulator.c     | 21 ++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/intel/int3472/clk_and_regulator.c b/drivers/platform/x86/intel/int3472/clk_and_regulator.c
> index 9e052b164a1a..c834fd59ec0c 100644
> --- a/drivers/platform/x86/intel/int3472/clk_and_regulator.c
> +++ b/drivers/platform/x86/intel/int3472/clk_and_regulator.c
> @@ -19,23 +19,42 @@ static const guid_t img_clk_guid =
>  	GUID_INIT(0x82c0d13a, 0x78c5, 0x4244,
>  		  0x9b, 0xb1, 0xeb, 0x8b, 0x53, 0x9a, 0x8d, 0x11);
>  
> +/*
> + * The PCH clock frequency argument to the _DSM method:
> + * PCH_CLK_FREQ_19M2 = 19.2MHz (default)
> + * PCH_CLK_FREQ_24M = 24MHz
> + */
> +#define PCH_CLK_FREQ_19M2	1
> +#define PCH_CLK_FREQ_24M	3
> +
>  static void skl_int3472_enable_clk(struct int3472_clock *clk, int enable)
>  {
>  	struct int3472_discrete_device *int3472 = to_int3472_device(clk);
>  	union acpi_object args[3];
>  	union acpi_object argv4;
> +	u32 dsm_freq_arg;
>  
>  	if (clk->ena_gpio) {
>  		gpiod_set_value_cansleep(clk->ena_gpio, enable);
>  		return;
>  	}
>  
> +	switch (clk->frequency) {
> +	case 24000000:
> +		dsm_freq_arg = PCH_CLK_FREQ_24M;
> +		break;
> +	case 19200000:
> +	default:
> +		dsm_freq_arg = PCH_CLK_FREQ_19M2;
> +		break;
> +	}
> +
>  	args[0].integer.type = ACPI_TYPE_INTEGER;
>  	args[0].integer.value = clk->imgclk_index;
>  	args[1].integer.type = ACPI_TYPE_INTEGER;
>  	args[1].integer.value = enable;
>  	args[2].integer.type = ACPI_TYPE_INTEGER;
> -	args[2].integer.value = 1;
> +	args[2].integer.value = dsm_freq_arg;
>  
>  	argv4.type = ACPI_TYPE_PACKAGE;
>  	argv4.package.count = 3;

Re: [PATCH v2] platform/x86: int3472: Use actual clock frequency for DSM method

[Sakari Ailus]

Hi Hao,

On Mon, Dec 08, 2025 at 11:51:14AM +0800, Hao Yao wrote:
> The third argument (args[2]) to the _DSM method was hardcoded to 1,
> which corresponds to 19.2MHz. However, this argument should reflect
> the actual clock frequency from the sensor's ACPI data.
> 
> According to the DSM specification:
> - 1 = 19.2MHz
> - 3 = 24MHz
> 
> Read the frequency from clk->frequency and set the DSM argument
> accordingly, with 19.2MHz as the default for unsupported frequencies.
> 
> This ensures the sensor receives the correct clock frequency as
> specified in its ACPI configuration.
> 
> Signed-off-by: Hao Yao <hao.yao@intel.com>

Thanks for the update.

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

-- 
Kind regards,

Sakari Ailus

Re: [PATCH v2] platform/x86: int3472: Use actual clock frequency for DSM method

[Ilpo Järvinen]

On Mon, 8 Dec 2025, Hao Yao wrote:

> The third argument (args[2]) to the _DSM method was hardcoded to 1,
> which corresponds to 19.2MHz. However, this argument should reflect
> the actual clock frequency from the sensor's ACPI data.
> 
> According to the DSM specification:
> - 1 = 19.2MHz
> - 3 = 24MHz
> 
> Read the frequency from clk->frequency and set the DSM argument
> accordingly, with 19.2MHz as the default for unsupported frequencies.
> 
> This ensures the sensor receives the correct clock frequency as
> specified in its ACPI configuration.
> 
> Signed-off-by: Hao Yao <hao.yao@intel.com>

Should this have a Fixes tag?

Could you please also mention what you discovered about values 0 and 2 in 
the changelog as it seems useful information (I requested this in v1).

> ---
>  .../x86/intel/int3472/clk_and_regulator.c     | 21 ++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/intel/int3472/clk_and_regulator.c b/drivers/platform/x86/intel/int3472/clk_and_regulator.c
> index 9e052b164a1a..c834fd59ec0c 100644
> --- a/drivers/platform/x86/intel/int3472/clk_and_regulator.c
> +++ b/drivers/platform/x86/intel/int3472/clk_and_regulator.c
> @@ -19,23 +19,42 @@ static const guid_t img_clk_guid =
>  	GUID_INIT(0x82c0d13a, 0x78c5, 0x4244,
>  		  0x9b, 0xb1, 0xeb, 0x8b, 0x53, 0x9a, 0x8d, 0x11);
>  
> +/*
> + * The PCH clock frequency argument to the _DSM method:
> + * PCH_CLK_FREQ_19M2 = 19.2MHz (default)
> + * PCH_CLK_FREQ_24M = 24MHz
> + */
> +#define PCH_CLK_FREQ_19M2	1
> +#define PCH_CLK_FREQ_24M	3
> +
>  static void skl_int3472_enable_clk(struct int3472_clock *clk, int enable)
>  {
>  	struct int3472_discrete_device *int3472 = to_int3472_device(clk);
>  	union acpi_object args[3];
>  	union acpi_object argv4;
> +	u32 dsm_freq_arg;
>  
>  	if (clk->ena_gpio) {
>  		gpiod_set_value_cansleep(clk->ena_gpio, enable);
>  		return;
>  	}
>  
> +	switch (clk->frequency) {
> +	case 24000000:
> +		dsm_freq_arg = PCH_CLK_FREQ_24M;
> +		break;
> +	case 19200000:
> +	default:
> +		dsm_freq_arg = PCH_CLK_FREQ_19M2;
> +		break;
> +	}
> +
>  	args[0].integer.type = ACPI_TYPE_INTEGER;
>  	args[0].integer.value = clk->imgclk_index;
>  	args[1].integer.type = ACPI_TYPE_INTEGER;
>  	args[1].integer.value = enable;
>  	args[2].integer.type = ACPI_TYPE_INTEGER;
> -	args[2].integer.value = 1;
> +	args[2].integer.value = dsm_freq_arg;
>  
>  	argv4.type = ACPI_TYPE_PACKAGE;
>  	argv4.package.count = 3;
> 


-- 
 i.

Re: [PATCH v2] platform/x86: int3472: Use actual clock frequency for DSM method

[Hao Yao]

Hi,

Sorry for missing this mail,

On 12/29/25 20:06, Ilpo Järvinen wrote:
> On Mon, 8 Dec 2025, Hao Yao wrote:
> 
>> The third argument (args[2]) to the _DSM method was hardcoded to 1,
>> which corresponds to 19.2MHz. However, this argument should reflect
>> the actual clock frequency from the sensor's ACPI data.
>>
>> According to the DSM specification:
>> - 1 = 19.2MHz
>> - 3 = 24MHz
>>
>> Read the frequency from clk->frequency and set the DSM argument
>> accordingly, with 19.2MHz as the default for unsupported frequencies.
>>
>> This ensures the sensor receives the correct clock frequency as
>> specified in its ACPI configuration.
>>
>> Signed-off-by: Hao Yao <hao.yao@intel.com>
> 
> Should this have a Fixes tag?
> 

Fixes: e4543de8b6ff ("platform/x86: int3472: Evaluate device's _DSM 
method to control imaging clock")

> Could you please also mention what you discovered about values 0 and 2 in
> the changelog as it seems useful information (I requested this in v1).
> 

When I send value 0 or 2, the clock doesn't change (checked by 
oscilloscope). I guess that values other than 1 and 3 is ignored 
currently, maybe reserved for future usage.


Best Regards,
Hao Yao

>> ---
>>   .../x86/intel/int3472/clk_and_regulator.c     | 21 ++++++++++++++++++-
>>   1 file changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/platform/x86/intel/int3472/clk_and_regulator.c b/drivers/platform/x86/intel/int3472/clk_and_regulator.c
>> index 9e052b164a1a..c834fd59ec0c 100644
>> --- a/drivers/platform/x86/intel/int3472/clk_and_regulator.c
>> +++ b/drivers/platform/x86/intel/int3472/clk_and_regulator.c
>> @@ -19,23 +19,42 @@ static const guid_t img_clk_guid =
>>   	GUID_INIT(0x82c0d13a, 0x78c5, 0x4244,
>>   		  0x9b, 0xb1, 0xeb, 0x8b, 0x53, 0x9a, 0x8d, 0x11);
>>   
>> +/*
>> + * The PCH clock frequency argument to the _DSM method:
>> + * PCH_CLK_FREQ_19M2 = 19.2MHz (default)
>> + * PCH_CLK_FREQ_24M = 24MHz
>> + */
>> +#define PCH_CLK_FREQ_19M2	1
>> +#define PCH_CLK_FREQ_24M	3
>> +
>>   static void skl_int3472_enable_clk(struct int3472_clock *clk, int enable)
>>   {
>>   	struct int3472_discrete_device *int3472 = to_int3472_device(clk);
>>   	union acpi_object args[3];
>>   	union acpi_object argv4;
>> +	u32 dsm_freq_arg;
>>   
>>   	if (clk->ena_gpio) {
>>   		gpiod_set_value_cansleep(clk->ena_gpio, enable);
>>   		return;
>>   	}
>>   
>> +	switch (clk->frequency) {
>> +	case 24000000:
>> +		dsm_freq_arg = PCH_CLK_FREQ_24M;
>> +		break;
>> +	case 19200000:
>> +	default:
>> +		dsm_freq_arg = PCH_CLK_FREQ_19M2;
>> +		break;
>> +	}
>> +
>>   	args[0].integer.type = ACPI_TYPE_INTEGER;
>>   	args[0].integer.value = clk->imgclk_index;
>>   	args[1].integer.type = ACPI_TYPE_INTEGER;
>>   	args[1].integer.value = enable;
>>   	args[2].integer.type = ACPI_TYPE_INTEGER;
>> -	args[2].integer.value = 1;
>> +	args[2].integer.value = dsm_freq_arg;
>>   
>>   	argv4.type = ACPI_TYPE_PACKAGE;
>>   	argv4.package.count = 3;
>>
> 
>