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

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

Posted by Hao Yao 2 weeks ago

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..0502876284ee 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_19M = 19.2MHz (default)
+ * PCH_CLK_FREQ_24M = 24MHz
+ */
+#define PCH_CLK_FREQ_19M	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_19M;
+		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] platform/x86: int3472: Use actual clock frequency for DSM method

Posted by Bingbu Cao 2 weeks ago

Hao,

Thanks for the patch.

On 12/5/25 5:51 PM, 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
>

What are the value 0 and 2? I think there are other frequencies.

> 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..0502876284ee 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_19M = 19.2MHz (default)
> + * PCH_CLK_FREQ_24M = 24MHz
> + */
> +#define PCH_CLK_FREQ_19M	1

I like 19P2MHZ.

> +#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_19M;
> +		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;
> 

-- 
Best regards,
Bingbu Cao

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

Posted by Hao Yao 1 week, 4 days ago

Hi Bingbu,

On 12/5/25 18:10, Bingbu Cao wrote:
> Hao,
> 
> Thanks for the patch.
> 
> On 12/5/25 5:51 PM, 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
>>
> 
> What are the value 0 and 2? I think there are other frequencies.

Seems 0 and 2 are reserved for future options, as I can't get the clock 
output by setting these values.

Best Regards,
Hao Yao

> 
>> 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..0502876284ee 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_19M = 19.2MHz (default)
>> + * PCH_CLK_FREQ_24M = 24MHz
>> + */
>> +#define PCH_CLK_FREQ_19M	1
> 
> I like 19P2MHZ.
> 
>> +#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_19M;
>> +		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] platform/x86: int3472: Use actual clock frequency for DSM method

Posted by Ilpo Järvinen 1 week, 3 days ago

On Mon, 8 Dec 2025, Hao Yao wrote:

> Hi Bingbu,
> 
> On 12/5/25 18:10, Bingbu Cao wrote:
> > Hao,
> > 
> > Thanks for the patch.
> > 
> > On 12/5/25 5:51 PM, 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
> > > 
> > 
> > What are the value 0 and 2? I think there are other frequencies.
> 
> Seems 0 and 2 are reserved for future options, as I can't get the clock output
> by setting these values.

Hi,

This looks like useful information to add into the changelog itself so if 
somebody later wonders this same thing, they can get the information 
easily.

--
 i.

> Best Regards,
> Hao Yao
> 
> > 
> > > 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..0502876284ee 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_19M = 19.2MHz (default)
> > > + * PCH_CLK_FREQ_24M = 24MHz
> > > + */
> > > +#define PCH_CLK_FREQ_19M	1
> > 
> > I like 19P2MHZ.
> > 
> > > +#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_19M;
> > > +		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] platform/x86: int3472: Use actual clock frequency for DSM method

Posted by Sakari Ailus 1 week, 6 days ago

On Fri, Dec 05, 2025 at 06:10:10PM +0800, Bingbu Cao wrote:
> > +#define PCH_CLK_FREQ_19M	1
> 
> I like 19P2MHZ.

How about simply PCH_CLK_FREQ_19M2?

-- 
Sakari Ailus

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

Posted by johannes.goede@oss.qualcomm.com 1 week, 4 days ago

Hi,

On 5-Dec-25 8:05 PM, Sakari Ailus wrote:
> On Fri, Dec 05, 2025 at 06:10:10PM +0800, Bingbu Cao wrote:
>>> +#define PCH_CLK_FREQ_19M	1
>>
>> I like 19P2MHZ.
> 
> How about simply PCH_CLK_FREQ_19M2?

+1 for this, this is the standard way to write fractional
MHz clocks.

With that fixed / so for v2:

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

Regards,

Hans

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

Posted by Hao Yao 1 week, 4 days ago

Hi Hans,

On 12/7/25 22:43, johannes.goede@oss.qualcomm.com wrote:
> Hi,
> 
> On 5-Dec-25 8:05 PM, Sakari Ailus wrote:
>> On Fri, Dec 05, 2025 at 06:10:10PM +0800, Bingbu Cao wrote:
>>>> +#define PCH_CLK_FREQ_19M	1
>>>
>>> I like 19P2MHZ.
>>
>> How about simply PCH_CLK_FREQ_19M2?
> 
> +1 for this, this is the standard way to write fractional
> MHz clocks.

Thank you. Will fix here in v2 patch.


Best Regards,
Hao Yao

> 
> With that fixed / so for v2:
> 
> Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> 
> Regards,
> 
> Hans
> 
>