[PATCH v2] rtc: tegra: Add ACPI support

[PATCH v2] rtc: tegra: Add ACPI support

[Kartik Rajput]

Add ACPI support for Tegra RTC, which is available on Tegra241 and
Tegra410. Both Tegra241 and Tegra410 use the same ACPI ID 'NVDA0280'.
The RTC clock is configured by UEFI before the kernel boots.

Signed-off-by: Kartik Rajput <kkartik@nvidia.com>
---
v1 -> v2:
	* Dropped "linux/acpi.h" from includes.
	* Dropped redundant ', 0' part from tegra_rtc_acpi_match.
	* Replaced "is_of_node(dev_fwnode(&pdev->dev))" with
	  "dev_of_node(&pdev->dev)" to check device of node.
	* Dropped redundant of_node checks before accessing clock
	  related APIs.
---
 drivers/rtc/rtc-tegra.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-tegra.c b/drivers/rtc/rtc-tegra.c
index 46788db89953..a34f0c80fc37 100644
--- a/drivers/rtc/rtc-tegra.c
+++ b/drivers/rtc/rtc-tegra.c
@@ -274,6 +274,12 @@ static const struct of_device_id tegra_rtc_dt_match[] = {
 };
 MODULE_DEVICE_TABLE(of, tegra_rtc_dt_match);
 
+static const struct acpi_device_id tegra_rtc_acpi_match[] = {
+	{ "NVDA0280" },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, tegra_rtc_acpi_match);
+
 static int tegra_rtc_probe(struct platform_device *pdev)
 {
 	struct tegra_rtc_info *info;
@@ -300,9 +306,11 @@ static int tegra_rtc_probe(struct platform_device *pdev)
 	info->rtc->ops = &tegra_rtc_ops;
 	info->rtc->range_max = U32_MAX;
 
-	info->clk = devm_clk_get(&pdev->dev, NULL);
-	if (IS_ERR(info->clk))
-		return PTR_ERR(info->clk);
+	if (dev_of_node(&pdev->dev)) {
+		info->clk = devm_clk_get(&pdev->dev, NULL);
+		if (IS_ERR(info->clk))
+			return PTR_ERR(info->clk);
+	}
 
 	ret = clk_prepare_enable(info->clk);
 	if (ret < 0)
@@ -404,6 +412,7 @@ static struct platform_driver tegra_rtc_driver = {
 	.driver = {
 		.name = "tegra_rtc",
 		.of_match_table = tegra_rtc_dt_match,
+		.acpi_match_table = tegra_rtc_acpi_match,
 		.pm = &tegra_rtc_pm_ops,
 	},
 };
-- 
2.43.0

Re: [PATCH v2] rtc: tegra: Add ACPI support

[Andy Shevchenko]

On Wed, Oct 22, 2025 at 12:06:45PM +0530, Kartik Rajput wrote:
> Add ACPI support for Tegra RTC, which is available on Tegra241 and
> Tegra410. Both Tegra241 and Tegra410 use the same ACPI ID 'NVDA0280'.
> The RTC clock is configured by UEFI before the kernel boots.

Thanks for an update, looks much better now!
A comment below, though.

...

> -	info->clk = devm_clk_get(&pdev->dev, NULL);
> -	if (IS_ERR(info->clk))
> -		return PTR_ERR(info->clk);
> +	if (dev_of_node(&pdev->dev)) {
> +		info->clk = devm_clk_get(&pdev->dev, NULL);
> +		if (IS_ERR(info->clk))
> +			return PTR_ERR(info->clk);
> +	}
>  
>  	ret = clk_prepare_enable(info->clk);

Since we still call CLK APIs unconditionally here, shouldn't be the whole
approach just to move to _optional() CLK API?

	info->clk = devm_clk_get_optional(&pdev->dev, NULL);

I haven't checked the code below, but maybe even one can incorporate _enabled
to this as well (in a separate change as it's not related to this patch
directly).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] rtc: tegra: Add ACPI support

[Kartik Rajput]

On 22/10/25 22:38, Andy Shevchenko wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Wed, Oct 22, 2025 at 12:06:45PM +0530, Kartik Rajput wrote:
>> Add ACPI support for Tegra RTC, which is available on Tegra241 and
>> Tegra410. Both Tegra241 and Tegra410 use the same ACPI ID 'NVDA0280'.
>> The RTC clock is configured by UEFI before the kernel boots.
> 
> Thanks for an update, looks much better now!
> A comment below, though.
> 
> ...
> 
>> -     info->clk = devm_clk_get(&pdev->dev, NULL);
>> -     if (IS_ERR(info->clk))
>> -             return PTR_ERR(info->clk);
>> +     if (dev_of_node(&pdev->dev)) {
>> +             info->clk = devm_clk_get(&pdev->dev, NULL);
>> +             if (IS_ERR(info->clk))
>> +                     return PTR_ERR(info->clk);
>> +     }
>>
>>        ret = clk_prepare_enable(info->clk);
> 
> Since we still call CLK APIs unconditionally here, shouldn't be the whole
> approach just to move to _optional() CLK API?
> 
>          info->clk = devm_clk_get_optional(&pdev->dev, NULL);
> 
> I haven't checked the code below, but maybe even one can incorporate _enabled
> to this as well (in a separate change as it's not related to this patch
> directly).
> 

Hi Andy,

The reason I did not use the _optional API is because the clocks are required
for the device-tree. Therefore, it must fail if clocks are not provided on
device-tree boot.

Thanks,
Kartik


> --
> With Best Regards,
> Andy Shevchenko
> 
> 

Re: [PATCH v2] rtc: tegra: Add ACPI support

[Andy Shevchenko]

On Thu, Oct 23, 2025 at 12:14:13PM +0530, Kartik Rajput wrote:
> On 22/10/25 22:38, Andy Shevchenko wrote:
> > On Wed, Oct 22, 2025 at 12:06:45PM +0530, Kartik Rajput wrote:

...

> > > -     info->clk = devm_clk_get(&pdev->dev, NULL);
> > > -     if (IS_ERR(info->clk))
> > > -             return PTR_ERR(info->clk);
> > > +     if (dev_of_node(&pdev->dev)) {
> > > +             info->clk = devm_clk_get(&pdev->dev, NULL);
> > > +             if (IS_ERR(info->clk))
> > > +                     return PTR_ERR(info->clk);
> > > +     }
> > > 
> > >        ret = clk_prepare_enable(info->clk);
> > 
> > Since we still call CLK APIs unconditionally here, shouldn't be the whole
> > approach just to move to _optional() CLK API?
> > 
> >          info->clk = devm_clk_get_optional(&pdev->dev, NULL);
> > 
> > I haven't checked the code below, but maybe even one can incorporate _enabled
> > to this as well (in a separate change as it's not related to this patch
> > directly).
> 
> The reason I did not use the _optional API is because the clocks are required
> for the device-tree. Therefore, it must fail if clocks are not provided on
> device-tree boot.

I see, please mention this in the commit message. And perhaps add a patch to
convert to devm_clk_get_enabled().

On top of that you also can convert driver to use pm_sleep_ptr() and drop ugly
ifdeffery. But this is really out of scope, and up to you to decide.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] rtc: tegra: Add ACPI support

[Kartik Rajput]

On 23/10/25 13:05, Andy Shevchenko wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Thu, Oct 23, 2025 at 12:14:13PM +0530, Kartik Rajput wrote:
>> On 22/10/25 22:38, Andy Shevchenko wrote:
>>> On Wed, Oct 22, 2025 at 12:06:45PM +0530, Kartik Rajput wrote:
> 
> ...
> 
>>>> -     info->clk = devm_clk_get(&pdev->dev, NULL);
>>>> -     if (IS_ERR(info->clk))
>>>> -             return PTR_ERR(info->clk);
>>>> +     if (dev_of_node(&pdev->dev)) {
>>>> +             info->clk = devm_clk_get(&pdev->dev, NULL);
>>>> +             if (IS_ERR(info->clk))
>>>> +                     return PTR_ERR(info->clk);
>>>> +     }
>>>>
>>>>         ret = clk_prepare_enable(info->clk);
>>>
>>> Since we still call CLK APIs unconditionally here, shouldn't be the whole
>>> approach just to move to _optional() CLK API?
>>>
>>>           info->clk = devm_clk_get_optional(&pdev->dev, NULL);
>>>
>>> I haven't checked the code below, but maybe even one can incorporate _enabled
>>> to this as well (in a separate change as it's not related to this patch
>>> directly).
>>
>> The reason I did not use the _optional API is because the clocks are required
>> for the device-tree. Therefore, it must fail if clocks are not provided on
>> device-tree boot.
> 
> I see, please mention this in the commit message. And perhaps add a patch to
> convert to devm_clk_get_enabled().
> 
> On top of that you also can convert driver to use pm_sleep_ptr() and drop ugly
> ifdeffery. But this is really out of scope, and up to you to decide.
> 
> --
> With Best Regards,
> Andy Shevchenko
> 
> 

Thanks for the suggestions, Andy! I have posted v3 with all these changes here:
https://lore.kernel.org/linux-tegra/20251023093042.770798-1-kkartik@nvidia.com/T/#t

Regards,
Kartik