[v7] i2c: xiic: use generic device property accessors

[PATCH v7 1/6] i2c: xiic: skip input clock setup on non-OF systems

Posted by Abdurrahman Hussain via B4 Relay 1 week, 4 days ago

From: Abdurrahman Hussain <abdurrahman@nexthop.ai>

The xiic driver supports operation without explicit clock configuration
when clocks cannot be specified via firmware, such as on ACPI-based
systems. This behavior is implemented in xiic_setclk(), which returns
early when either i2c_clk or input_clk are zero.

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/i2c/busses/i2c-xiic.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 28015d77599d..912a94d4d080 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -1423,6 +1423,7 @@ MODULE_DEVICE_TABLE(of, xiic_of_match);
 
 static int xiic_i2c_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct xiic_i2c *i2c;
 	struct xiic_i2c_platform_data *pdata;
 	const struct of_device_id *match;
@@ -1464,10 +1465,12 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 	mutex_init(&i2c->lock);
 	spin_lock_init(&i2c->atomic_lock);
 
-	i2c->clk = devm_clk_get_enabled(&pdev->dev, NULL);
-	if (IS_ERR(i2c->clk))
-		return dev_err_probe(&pdev->dev, PTR_ERR(i2c->clk),
-				     "failed to enable input clock.\n");
+	if (is_of_node(dev->fwnode)) {
+		i2c->clk = devm_clk_get_enabled(dev, NULL);
+		if (IS_ERR(i2c->clk))
+			return dev_err_probe(&pdev->dev, PTR_ERR(i2c->clk),
+					"failed to enable input clock.\n");
+	}
 
 	i2c->dev = &pdev->dev;
 	pm_runtime_set_autosuspend_delay(i2c->dev, XIIC_PM_TIMEOUT);

-- 
2.52.0

Re: [PATCH v7 1/6] i2c: xiic: skip input clock setup on non-OF systems

Posted by Andy Shevchenko 1 week, 2 days ago

On Thu, Jan 29, 2026 at 09:43:13PM +0000, Abdurrahman Hussain via B4 Relay wrote:

> The xiic driver supports operation without explicit clock configuration
> when clocks cannot be specified via firmware, such as on ACPI-based
> systems. This behavior is implemented in xiic_setclk(), which returns
> early when either i2c_clk or input_clk are zero.

...

> -	i2c->clk = devm_clk_get_enabled(&pdev->dev, NULL);
> -	if (IS_ERR(i2c->clk))
> -		return dev_err_probe(&pdev->dev, PTR_ERR(i2c->clk),
> -				     "failed to enable input clock.\n");
> +	if (is_of_node(dev->fwnode)) {

Avoid dereferencing fwnode. Use dev_fwnode() API.

> +		i2c->clk = devm_clk_get_enabled(dev, NULL);
> +		if (IS_ERR(i2c->clk))
> +			return dev_err_probe(&pdev->dev, PTR_ERR(i2c->clk),
> +					"failed to enable input clock.\n");
> +	}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 1/6] i2c: xiic: skip input clock setup on non-OF systems

Posted by Andrew Lunn 1 week, 4 days ago

On Thu, Jan 29, 2026 at 09:43:13PM +0000, Abdurrahman Hussain via B4 Relay wrote:
> From: Abdurrahman Hussain <abdurrahman@nexthop.ai>
> 
> The xiic driver supports operation without explicit clock configuration
> when clocks cannot be specified via firmware, such as on ACPI-based
> systems.

Are you saying it is technically impossible to specify a clock in
ACPI?

Maybe a more accurate would be:

The xiic driver supports operation without explicit clock
configuration when the clocks are not specified via firmware, such as
when the ACPI tables are missing the description of the clocks.

	Andrew

Re: [PATCH v7 1/6] i2c: xiic: skip input clock setup on non-OF systems

Posted by Abdurrahman Hussain 1 week, 4 days ago

> On Jan 29, 2026, at 2:43 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> 
> On Thu, Jan 29, 2026 at 09:43:13PM +0000, Abdurrahman Hussain via B4 Relay wrote:
>> From: Abdurrahman Hussain <abdurrahman@nexthop.ai>
>> 
>> The xiic driver supports operation without explicit clock configuration
>> when clocks cannot be specified via firmware, such as on ACPI-based
>> systems.
> 
> Are you saying it is technically impossible to specify a clock in
> ACPI?
> 
> Maybe a more accurate would be:
> 
> The xiic driver supports operation without explicit clock
> configuration when the clocks are not specified via firmware, such as
> when the ACPI tables are missing the description of the clocks.
> 
> Andrew

Actually, ACPI (since 6.5) added a ClockInput() macro that can be added to
_CRS of a device node. The ACPI subsystem in kernel could parse these and
convert into proper clocks integrated with the CCF. But, AFAIK, this idea was
rejected in the past. So, technically, it's the kernel that lacks support on
ACPI systems.

What about this wording then:

The xiic driver supports operation without explicit clock configuration when
the clocks specified via firmware are ignored, such as on ACPI systems.

Abdurrahman

Re: [PATCH v7 1/6] i2c: xiic: skip input clock setup on non-OF systems

Posted by Andy Shevchenko 1 week, 2 days ago

On Thu, Jan 29, 2026 at 03:29:45PM -0800, Abdurrahman Hussain wrote:
> > On Jan 29, 2026, at 2:43 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> > On Thu, Jan 29, 2026 at 09:43:13PM +0000, Abdurrahman Hussain via B4 Relay wrote:

> >> The xiic driver supports operation without explicit clock configuration
> >> when clocks cannot be specified via firmware, such as on ACPI-based
> >> systems.
> > 
> > Are you saying it is technically impossible to specify a clock in
> > ACPI?
> > 
> > Maybe a more accurate would be:
> > 
> > The xiic driver supports operation without explicit clock
> > configuration when the clocks are not specified via firmware, such as
> > when the ACPI tables are missing the description of the clocks.
> 
> Actually, ACPI (since 6.5) added a ClockInput() macro that can be added to
> _CRS of a device node. The ACPI subsystem in kernel could parse these and
> convert into proper clocks integrated with the CCF. But, AFAIK, this idea was
> rejected in the past.

Rejected by which side? CCF?
Because specification still has that.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 1/6] i2c: xiic: skip input clock setup on non-OF systems

Posted by Abdurrahman Hussain 1 week, 1 day ago

On Sat Jan 31, 2026 at 10:12 AM UTC, Andy Shevchenko wrote:
> On Thu, Jan 29, 2026 at 03:29:45PM -0800, Abdurrahman Hussain wrote:
>> > On Jan 29, 2026, at 2:43 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> > On Thu, Jan 29, 2026 at 09:43:13PM +0000, Abdurrahman Hussain via B4 Relay wrote:
>
>> >> The xiic driver supports operation without explicit clock configuration
>> >> when clocks cannot be specified via firmware, such as on ACPI-based
>> >> systems.
>> >
>> > Are you saying it is technically impossible to specify a clock in
>> > ACPI?
>> >
>> > Maybe a more accurate would be:
>> >
>> > The xiic driver supports operation without explicit clock
>> > configuration when the clocks are not specified via firmware, such as
>> > when the ACPI tables are missing the description of the clocks.
>>
>> Actually, ACPI (since 6.5) added a ClockInput() macro that can be added to
>> _CRS of a device node. The ACPI subsystem in kernel could parse these and
>> convert into proper clocks integrated with the CCF. But, AFAIK, this idea was
>> rejected in the past.
>
> Rejected by which side? CCF?
> Because specification still has that.

I think the argument was that on ACPI based systems clocks are "owned"
by AML and there could be syncronizations issuebetween AML and the OS.

See https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1712165.html

Re: [PATCH v7 1/6] i2c: xiic: skip input clock setup on non-OF systems

Posted by Andrew Lunn 1 week ago

On Sat, Jan 31, 2026 at 08:30:40PM -0500, Abdurrahman Hussain wrote:
> On Sat Jan 31, 2026 at 10:12 AM UTC, Andy Shevchenko wrote:
> > On Thu, Jan 29, 2026 at 03:29:45PM -0800, Abdurrahman Hussain wrote:
> >> > On Jan 29, 2026, at 2:43 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> >> > On Thu, Jan 29, 2026 at 09:43:13PM +0000, Abdurrahman Hussain via B4 Relay wrote:
> >
> >> >> The xiic driver supports operation without explicit clock configuration
> >> >> when clocks cannot be specified via firmware, such as on ACPI-based
> >> >> systems.
> >> >
> >> > Are you saying it is technically impossible to specify a clock in
> >> > ACPI?
> >> >
> >> > Maybe a more accurate would be:
> >> >
> >> > The xiic driver supports operation without explicit clock
> >> > configuration when the clocks are not specified via firmware, such as
> >> > when the ACPI tables are missing the description of the clocks.
> >>
> >> Actually, ACPI (since 6.5) added a ClockInput() macro that can be added to
> >> _CRS of a device node. The ACPI subsystem in kernel could parse these and
> >> convert into proper clocks integrated with the CCF. But, AFAIK, this idea was
> >> rejected in the past.
> >
> > Rejected by which side? CCF?
> > Because specification still has that.
> 
> I think the argument was that on ACPI based systems clocks are "owned"
> by AML and there could be syncronizations issuebetween AML and the OS.
> 
> See https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1712165.html

Doesn't that just mean there needs to be a call into AML to request it
take an action on a clock? Otherwise, why even have ClockInput()? This
link is to quite an old thread, 2018, where as ClockInput seems to be
pretty new.

The fact ClockInput() exists, means at some point somebody will
implement it. Once it has been implemented, somebody might need to use
it with xiic? Because it is mandatory in DT, and there is no ACPI
binding document for xiic, they could make it mandatory in ACPI as
well. And then your device breaks.

By putting in the commit message something like:

Currently Linux does not implement ACPI ClockInput to describe clock
resources, unlike DT. However the xiic driver is happy if something
magically enables the clock before the driver probes, and does not
turn it off again. The clock should always be considered optional for
ACPI.

That should act has a hint to future developers hacking on xiic not to
make it mandatory.

     Andrew

Re: [PATCH v7 1/6] i2c: xiic: skip input clock setup on non-OF systems

Posted by Abdurrahman Hussain 1 week ago


> On Feb 2, 2026, at 5:21 AM, Andrew Lunn <andrew@lunn.ch> wrote:
> 
> On Sat, Jan 31, 2026 at 08:30:40PM -0500, Abdurrahman Hussain wrote:
>> On Sat Jan 31, 2026 at 10:12 AM UTC, Andy Shevchenko wrote:
>>> On Thu, Jan 29, 2026 at 03:29:45PM -0800, Abdurrahman Hussain wrote:
>>>>> On Jan 29, 2026, at 2:43 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>>>>> On Thu, Jan 29, 2026 at 09:43:13PM +0000, Abdurrahman Hussain via B4 Relay wrote:
>>> 
>>>>>> The xiic driver supports operation without explicit clock configuration
>>>>>> when clocks cannot be specified via firmware, such as on ACPI-based
>>>>>> systems.
>>>>> 
>>>>> Are you saying it is technically impossible to specify a clock in
>>>>> ACPI?
>>>>> 
>>>>> Maybe a more accurate would be:
>>>>> 
>>>>> The xiic driver supports operation without explicit clock
>>>>> configuration when the clocks are not specified via firmware, such as
>>>>> when the ACPI tables are missing the description of the clocks.
>>>> 
>>>> Actually, ACPI (since 6.5) added a ClockInput() macro that can be added to
>>>> _CRS of a device node. The ACPI subsystem in kernel could parse these and
>>>> convert into proper clocks integrated with the CCF. But, AFAIK, this idea was
>>>> rejected in the past.
>>> 
>>> Rejected by which side? CCF?
>>> Because specification still has that.
>> 
>> I think the argument was that on ACPI based systems clocks are "owned"
>> by AML and there could be syncronizations issuebetween AML and the OS.
>> 
>> See https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1712165.html
> 
> Doesn't that just mean there needs to be a call into AML to request it
> take an action on a clock? Otherwise, why even have ClockInput()? This
> link is to quite an old thread, 2018, where as ClockInput seems to be
> pretty new.
> 
> The fact ClockInput() exists, means at some point somebody will
> implement it. Once it has been implemented, somebody might need to use
> it with xiic? Because it is mandatory in DT, and there is no ACPI
> binding document for xiic, they could make it mandatory in ACPI as
> well. And then your device breaks.
> 

That makes sense. I might have misread the thread and came to the wrong
conclusion that converting ClockInput() into a CCF clocks was
undesirable. Thank you for clarifying. Maybe I can start looking into adding
support for this after this series is merged.

> By putting in the commit message something like:
> 
> Currently Linux does not implement ACPI ClockInput to describe clock
> resources, unlike DT. However the xiic driver is happy if something
> magically enables the clock before the driver probes, and does not
> turn it off again. The clock should always be considered optional for
> ACPI.
> 
> That should act has a hint to future developers hacking on xiic not to
> make it mandatory.
> 

Yes, that is much more clear and concise! I am going to use this paragraph
verbatim in the commit message. Thanks for all the feedback, Andrew!

Best regards,
Abdurrahman