Re: [PATCH 2/2] rtc: mt6397: provide the 32 kHz output as a clock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] rtc: mt6397: NULL dereference of rtc->data in mtk_rtc_register_clkout()
--

commit 7a674b1fa23c59043354cce078b81b55651fd9c2
Author: Ryan Brue <ryanbrue.dev@gmail.com>

rtc: mt6397: provide the 32 kHz output as a clock

This commit registers the MT6397 RTC's 32 kHz output gated by RTC_CON.F32KOB
as a clock provider when the RTC node specifies #clock-cells, allowing client
devices such as Wi-Fi/Bluetooth combo chips to enable their sleep clock.

> diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> index d6e156516aa6c..5130fecffa7b4 100644
> --- a/drivers/rtc/rtc-mt6397.c
> +++ b/drivers/rtc/rtc-mt6397.c
[ ... ]
> @@ -317,6 +318,84 @@ static int mtk_rtc_nvram_write(void *priv, unsigned int offset, void *val,
[ ... ]
> +/* The 32 kHz output the PMIC drives to other chips, such as a combo chip */
> +static int mtk_rtc_register_clkout(struct device *dev, struct mt6397_rtc *rtc)
> +{
> +	struct clk_init_data init = {
> +		.name = "mt6397-rtc-32k",
> +		.ops = &mtk_rtc_clkout_ops,
> +	};
> +	int ret;
> +
> +	if (!rtc->data->con || !of_property_present(dev->of_node, "#clock-cells"))
> +		return 0;

[Severity: High]
Can this dereference a NULL rtc->data pointer during mtk_rtc_probe()?

In drivers/mfd/mt6397-core.c, mt6397_devs[] unconditionally registers the
mt6397-rtc MFD cell:

    {
        .name = "mt6397-rtc",
        .num_resources = ARRAY_SIZE(mt6397_rtc_resources),
        .resources = mt6397_rtc_resources,
        .of_compatible = "mediatek,mt6397-rtc",
    }

On MT6397 boards whose Device Tree omits the optional mediatek,mt6397-rtc
child node under the PMIC node (such as mt8173-evb.dts and mt8135-evbp1.dts),
mfd_add_device() registers the mt6397-rtc platform device with
pdev->dev.of_node set to NULL. Because mtk_rtc_driver has .driver.name set to
"mt6397-rtc" and no id_table, platform_match() binds the device via the
driver-name fallback and invokes mtk_rtc_probe():

drivers/rtc/rtc-mt6397.c:mtk_rtc_probe() {
    ...
    rtc->data = of_device_get_match_data(&pdev->dev);
    ...
    ret = mtk_rtc_register_clkout(&pdev->dev, rtc);
}

When pdev->dev.of_node is NULL, of_device_get_match_data() returns NULL.
Previously, mtk_rtc_probe() never dereferenced rtc->data during probe,
whereas mtk_rtc_register_clkout() now evaluates !rtc->data->con before
checking dev->of_node or whether rtc->data is NULL.

Should mtk_rtc_register_clkout() check !rtc->data (or check
of_property_present() first), or should mtk_rtc_probe() handle a NULL
rtc->data?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101-rbrue-suez-upstreaming-mt6397-rtc-32k-v1-0-9758f38d7053@gmail.com?part=2