Re: [PATCH] rtc: omap: Add missing kernel-doc for @dev parameter

Thank you for your contribution! Sashiko AI review found 2 potential issue(=
s) to consider:

Pre-existing issues:
- [High] omap_rtc_power_off_program() ignores its 'dev' parameter and deref=
erences a potentially NULL global pointer, masking an API inconsistency and=
 risking a kernel panic.
- [High] omap_rtc_power_off_program() reads time registers without waiting =
for the RTC to be un-busy, risking a corrupted time read and system hang.
--

--- Patch [1]: [PATCH] rtc: omap: Add missing kernel-doc for @dev parameter=
 ---
commit 1c4ccb1ef3cb61576a4f7529af03a3fdc744f227
Author: Rosen Penev <[email protected]>

rtc: omap: Add missing kernel-doc for @dev parameter

This commit fixes a kernel-doc warning for omap_rtc_power_off_program() by
adding the missing @dev parameter description.

> diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
> index 0f90065e352cb..39dd91d1bf86a 100644
> --- a/drivers/rtc/rtc-omap.c
> +++ b/drivers/rtc/rtc-omap.c
> @@ -407,6 +407,7 @@ static struct omap_rtc *omap_rtc_power_off_rtc;
>   * omap_rtc_power_off_program: Set the pmic power off sequence. The RTC
>   * generates pmic_pwr_enable control, which can be used to control an ex=
ternal
>   * PMIC.
> + * @dev: pointer to RTC device structure

[Severity: High]
This isn't a bug introduced by this patch, but the documentation solidifies=
 a
broken API. Does this documentation hide a type confusion?

Looking at omap_rtc_power_off(), it passes the parent platform device:

drivers/rtc/rtc-omap.c:omap_rtc_power_off() {
    ...
    omap_rtc_power_off_program(rtc->dev.parent);
}

However, am33xx_rtc_only_idle() passes the RTC device pointer:

drivers/soc/ti/pm33xx.c:am33xx_rtc_only_idle() {
    ...
    omap_rtc_power_off_program(&omap_rtc->dev);
}

Inside omap_rtc_power_off_program(), the dev parameter is ignored entirely
and a global variable is unconditionally dereferenced:

drivers/rtc/rtc-omap.c:omap_rtc_power_off_program() {
    struct omap_rtc *rtc =3D omap_rtc_power_off_rtc;
    ...
    rtc->type->unlock(rtc);
}

This global is only populated in omap_rtc_probe() if the
system-power-controller property is present:

drivers/rtc/rtc-omap.c:omap_rtc_probe() {
    if (rtc->is_pmic_controller) {
        if (!pm_power_off) {
            omap_rtc_power_off_rtc =3D rtc;
        }
    }
}

If the property is missing but pm33xx.c triggers this path during suspend,
will this cause a kernel panic due to a NULL pointer dereference of
omap_rtc_power_off_rtc?

>   */
>  int omap_rtc_power_off_program(struct device *dev)
>  {

[Severity: High]
This is a pre-existing issue, but does this function risk a corrupted time
read by not waiting for the RTC to be un-busy?

The function calls omap_rtc_read_time_raw() without first synchronizing via
rtc_wait_not_busy():

drivers/rtc/rtc-omap.c:omap_rtc_power_off_program() {
    ...
    /* set alarm one second from now */
    omap_rtc_read_time_raw(rtc, &tm);
    seconds =3D tm.tm_sec;
    ...
}

If the RTC is actively updating time registers, could we read transitional
garbage data (e.g. seconds updated to 0, but minutes still at 59 instead of
0)?

The rollover check further down only verifies the seconds register:

drivers/rtc/rtc-omap.c:omap_rtc_power_off_program() {
    ...
    /* Retry in case roll over happened before alarm was armed. */
    if (rtc_read(rtc, OMAP_RTC_SECONDS_REG) !=3D seconds) {
        ...
    }
}

If the seconds register was read correctly but others were corrupted, will
the check falsely succeed and arm the alarm with the corrupted time?

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260728003545.5713=
[email protected]?part=3D1