linux-kernel - Re: [PATCH v15] rtc: sophgo: add rtc support for Sophgo CV1800 SoC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <u6ofni4aabs4jy4unh5nudgm6qieg5hsk3xt4725yiuumlspen@ypery534oe4q>
Date: Sun, 11 May 2025 06:34:09 +0800
From: Inochi Amaoto <inochiama@...il.com>
To: Alexander Sverdlin <alexander.sverdlin@...il.com>, 
	Inochi Amaoto <inochiama@...il.com>, sophgo@...ts.linux.dev, linux-rtc@...r.kernel.org
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH v15] rtc: sophgo: add rtc support for Sophgo CV1800 SoC

On Sat, May 10, 2025 at 04:30:07PM +0200, Alexander Sverdlin wrote:
> Hi Inochi!
> 
> On Fri, 2025-05-09 at 06:21 +0800, Inochi Amaoto wrote:
> > On Wed, May 07, 2025 at 09:56:20PM +0200, Alexander Sverdlin wrote:
> > > From: Jingbao Qiu <qiujingbao.dlmu@...il.com>
> > > 
> > > Implement the RTC driver for CV1800, which able to provide time alarm.
> > > 
> > > Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@...il.com>
> > > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@...il.com>
> 
> ...
> 
> > > +static int cv1800_rtc_probe(struct platform_device *pdev)
> > > +{
> > > +	struct cv1800_rtc_priv *rtc;
> > > +	int ret;
> > > +
> > > +	rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
> > > +	if (!rtc)
> > > +		return -ENOMEM;
> > > +
> > > +	rtc->rtc_map = device_node_to_regmap(pdev->dev.parent->of_node);
> > > +	if (IS_ERR(rtc->rtc_map))
> > > +		return dev_err_probe(&pdev->dev, PTR_ERR(rtc->rtc_map),
> > > +				     "cannot get parent regmap\n");
> > > +
> > > +	rtc->irq = platform_get_irq(pdev, 0);
> > > +	if (rtc->irq < 0)
> > > +		return rtc->irq;
> > > +
> > > +	rtc->clk = devm_clk_get_enabled(pdev->dev.parent, "rtc");
> > > +	if (IS_ERR(rtc->clk))
> > > +		return dev_err_probe(&pdev->dev, PTR_ERR(rtc->clk),
> > > +				     "rtc clk not found\n");
> > > +
> > > +	platform_set_drvdata(pdev, rtc);
> > > +
> > > +	device_init_wakeup(&pdev->dev, 1);
> > > +
> > > +	rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
> > > +	if (IS_ERR(rtc->rtc_dev))
> > > +		return PTR_ERR(rtc->rtc_dev);
> > > +
> > > +	rtc->rtc_dev->ops = &cv1800_rtc_ops;
> > > +	rtc->rtc_dev->range_max = U32_MAX;
> > > +
> > > +	ret = devm_request_irq(&pdev->dev, rtc->irq, cv1800_rtc_irq_handler,
> > > +			       IRQF_TRIGGER_HIGH, "rtc alarm", rtc);
> > > +	if (ret)
> > > +		return dev_err_probe(&pdev->dev, ret,
> > > +				     "cannot register interrupt handler\n");
> > > +
> > > +	return devm_rtc_register_device(rtc->rtc_dev);
> > > +}
> > > +
> > 
> > I wonder whether the rtc driver may need reset (maybe optional) for this?
> > If so, please add it.
> 
> I'm not sure which reset you are referring to... RTC module can carry out
> system-wide resets, but cannot be reset itself (as I understand).
> 

This is fine for me.

> Initially I was thinking about providing a reboot driver for Linux utilizing
> the RTC module but it turns out PSCI interface is not optional on ARM64, which
> means PSCI reset interface has to be provided by the firmware (I'm thinking
> about U-Boot) and Linux will rely on PSCI reboot.
> 

I am not familiar with PSCI, but there is a fact that preserve uboot may
be costly as the ram is limited. I think it may be fine to have a reboot
driver to provide power function at the same time. But if you find there
is a way to implement PSCI reboot, just do the thing you prefer.

Regards,
Inochi