public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Hao Yao <hao.yao@intel.com>
Cc: Bingbu Cao <bingbu.cao@linux.intel.com>,
	 platform-driver-x86@vger.kernel.org,
	dan.scally@ideasonboard.com,  sakari.ailus@linux.intel.com,
	bingbu.cao@intel.com,  linux-media@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] platform/x86: int3472: Use actual clock frequency for DSM method
Date: Mon, 8 Dec 2025 18:22:12 +0200 (EET)	[thread overview]
Message-ID: <5cbe91d2-fde2-5676-e4c7-ade2f7ec5184@linux.intel.com> (raw)
In-Reply-To: <9890dbd7-ee91-41fe-8436-03f60bd97419@intel.com>

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

      reply	other threads:[~2025-12-08 16:22 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-05  9:51 [PATCH] platform/x86: int3472: Use actual clock frequency for DSM method Hao Yao
2025-12-05 10:10 ` Bingbu Cao
2025-12-05 19:05   ` Sakari Ailus
2025-12-07 14:43     ` johannes.goede
2025-12-08  3:55       ` Hao Yao
2025-12-08  3:53   ` Hao Yao
2025-12-08 16:22     ` Ilpo Järvinen [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5cbe91d2-fde2-5676-e4c7-ade2f7ec5184@linux.intel.com \
    --to=ilpo.jarvinen@linux.intel.com \
    --cc=bingbu.cao@intel.com \
    --cc=bingbu.cao@linux.intel.com \
    --cc=dan.scally@ideasonboard.com \
    --cc=hao.yao@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox