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

[Hao Yao <hao.yao@intel.com>]

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