Re: [PATCH 1/2] [RFC] rtc: expose direct access to hardware alarm time in debugfs

["Mateusz Jończyk" <mat.jonczyk@o2.pl>]

W dniu 31.03.2022 o 21:21, Greg KH pisze:
> On Thu, Mar 31, 2022 at 09:06:11PM +0200, Mateusz Jończyk wrote:
>> Before Linux 5.17, there was a problem with the CMOS RTC driver:
>> cmos_read_alarm() and cmos_set_alarm() did not check for the UIP (Update
>> in progress) bit, which could have caused it to sometimes fail silently
>> and read bogus values or do not set the alarm correctly.
>> Luckily, this issue was masked by cmos_read_time() invocations in core
>> RTC code - see https://marc.info/?l=linux-rtc&m=164858416511425&w=4
[snip]
>> diff --git a/drivers/rtc/debugfs.c b/drivers/rtc/debugfs.c
>> new file mode 100644
>> index 000000000000..5ceed5504033
>> --- /dev/null
>> +++ b/drivers/rtc/debugfs.c
>> @@ -0,0 +1,112 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +
>> +/*
>> + * Debugfs interface for testing RTC alarms.
>> + */
>> +#include <linux/debugfs.h>
>> +#include <linux/err.h>
>> +#include <linux/rtc.h>
>> +
>> +static struct dentry *rtc_main_debugfs_dir;
>> +
>> +void rtc_debugfs_init(void)
>> +{
>> +	struct dentry *ret = debugfs_create_dir("rtc", NULL);
>> +
>> +	// No error is critical here
> What do you mean by this?

I meant that even if debugfs_create_dir() fails, this does not matter here and the
caller of rtc_debugfs_init() may continue execution.

>> +	if (!IS_ERR(ret))
>> +		rtc_main_debugfs_dir = ret;
> This should not be needed.
>
> Just set the directory and away you go.
>
> But why even save it?  If you need it you can always look it up, right?
OK, I did not know about easy lookup of debugfs directories.
> And when are you removing this directory you created?
Never, as the RTC subsystem cannot be compiled as a module.
This is the main debugfs directory of RTC subsystem, directories for specific devices
(rtc0, etc.) are created below it.
Please correct me if I'm wrong, but I thought that empty debugfs directories do not hurt.


Thanks for reviewing,

Mateusz