* Re: [PATCH v2] platform/x86: int3472: Use actual clock frequency for DSM method
2025-12-08 3:51 [PATCH v2] platform/x86: int3472: Use actual clock frequency for DSM method Hao Yao
@ 2025-12-08 7:48 ` Dan Scally
2025-12-08 9:31 ` johannes.goede
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Dan Scally @ 2025-12-08 7:48 UTC (permalink / raw)
To: Hao Yao, platform-driver-x86, johannes.goede, sakari.ailus,
ilpo.jarvinen
Cc: bingbu.cao, linux-media, linux-kernel
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;
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] platform/x86: int3472: Use actual clock frequency for DSM method
2025-12-08 3:51 [PATCH v2] platform/x86: int3472: Use actual clock frequency for DSM method Hao Yao
2025-12-08 7:48 ` Dan Scally
@ 2025-12-08 9:31 ` johannes.goede
2025-12-08 11:27 ` Sakari Ailus
2025-12-29 12:06 ` Ilpo Järvinen
3 siblings, 0 replies; 6+ messages in thread
From: johannes.goede @ 2025-12-08 9:31 UTC (permalink / raw)
To: Hao Yao, platform-driver-x86, dan.scally, sakari.ailus, ilpo.jarvinen
Cc: bingbu.cao, linux-media, linux-kernel
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;
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] platform/x86: int3472: Use actual clock frequency for DSM method
2025-12-08 3:51 [PATCH v2] platform/x86: int3472: Use actual clock frequency for DSM method Hao Yao
2025-12-08 7:48 ` Dan Scally
2025-12-08 9:31 ` johannes.goede
@ 2025-12-08 11:27 ` Sakari Ailus
2025-12-29 12:06 ` Ilpo Järvinen
3 siblings, 0 replies; 6+ messages in thread
From: Sakari Ailus @ 2025-12-08 11:27 UTC (permalink / raw)
To: Hao Yao
Cc: platform-driver-x86, johannes.goede, dan.scally, ilpo.jarvinen,
bingbu.cao, linux-media, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] platform/x86: int3472: Use actual clock frequency for DSM method
2025-12-08 3:51 [PATCH v2] platform/x86: int3472: Use actual clock frequency for DSM method Hao Yao
` (2 preceding siblings ...)
2025-12-08 11:27 ` Sakari Ailus
@ 2025-12-29 12:06 ` Ilpo Järvinen
2026-04-02 6:56 ` Hao Yao
3 siblings, 1 reply; 6+ messages in thread
From: Ilpo Järvinen @ 2025-12-29 12:06 UTC (permalink / raw)
To: Hao Yao
Cc: platform-driver-x86, johannes.goede, dan.scally, sakari.ailus,
bingbu.cao, linux-media, LKML
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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] platform/x86: int3472: Use actual clock frequency for DSM method
2025-12-29 12:06 ` Ilpo Järvinen
@ 2026-04-02 6:56 ` Hao Yao
0 siblings, 0 replies; 6+ messages in thread
From: Hao Yao @ 2026-04-02 6:56 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: platform-driver-x86, johannes.goede, dan.scally, sakari.ailus,
bingbu.cao, linux-media, LKML
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;
>>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread