Re: [PATCH V2 2/2] rtc: atcrtc100: Add andes atcrtc100 RTC driver

[Krzysztof Kozlowski <krzk@kernel.org>]

On 17/06/2024 10:25, CL Wang wrote:
> The atcrtc100 module includes a real time counter with alarm.
> Add a RTC driver for this function.
> 
> Signed-off-by: CL Wang <cl634@andestech.com>


...

> +
> +static int atc_rtc_probe(struct platform_device *pdev)
> +{
> +	struct atc_rtc *atc_rtc;
> +	void __iomem *reg_base;
> +	int ret = 0;
> +
> +	atc_rtc = devm_kzalloc(&pdev->dev, sizeof(*atc_rtc), GFP_KERNEL);
> +	if (!atc_rtc)
> +		return -ENOMEM;
> +	platform_set_drvdata(pdev, atc_rtc);
> +	spin_lock_init(&atc_rtc->lock);
> +
> +	reg_base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(reg_base)) {
> +		dev_err(&pdev->dev, "couldn't map io space\n");
> +		return PTR_ERR(atc_rtc->regmap);
> +	}
> +
> +	atc_rtc->regmap = devm_regmap_init_mmio(&pdev->dev, reg_base,
> +						&atc_rtc_regmap_config);
> +	if (IS_ERR(atc_rtc->regmap)) {
> +		dev_err(&pdev->dev, "regmap init failed\n");
> +		ret = PTR_ERR(atc_rtc->regmap);
> +	}
> +
> +	ret = atc_rtc_hw_init(atc_rtc);
> +	if (ret) {
> +		dev_err(&pdev->dev, "atc_rtc HW initialize failed\n");
> +		return ret;
> +	}
> +
> +	atc_rtc->alarm_irq = platform_get_irq(pdev, 1);
> +	if (atc_rtc->alarm_irq < 0) {
> +		dev_err(&pdev->dev, "failed to get irq %d for alarm\n",
> +			atc_rtc->alarm_irq);
> +	}
> +	atc_rtc->time_irq = platform_get_irq(pdev, 0);
> +	if (atc_rtc->time_irq < 0) {
> +		dev_err(&pdev->dev, "failed to get irq %d for RTC\n",
> +			atc_rtc->time_irq);
> +	}
> +
> +	ret = devm_request_irq(&pdev->dev, atc_rtc->alarm_irq, rtc_alarm,
> +			       0, "rtc alarm", atc_rtc);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to request irq %d: %d for alarm\n",
> +			atc_rtc->alarm_irq, ret);
> +		return ret;
> +	}
> +
> +	ret = devm_request_irq(&pdev->dev, atc_rtc->time_irq, rtc_interrupt,
> +			       0, "rtc time", atc_rtc);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to request irq %d: %d\n",
> +			atc_rtc->time_irq, ret);
> +		return ret;
> +	}
> +
> +	atc_rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
> +	if (IS_ERR(atc_rtc->rtc_dev)) {
> +		dev_err(&pdev->dev, "Could not allocate rtc device\n");
> +		return PTR_ERR(atc_rtc->rtc_dev);
> +	}
> +
> +	if (of_property_read_bool(pdev->dev.of_node, "wakeup-source")) {

Test your DTS (dtbs_check). This should fail.


> +		device_init_wakeup(&pdev->dev, true);
> +		ret = dev_pm_set_wake_irq(&pdev->dev, atc_rtc->alarm_irq);
> +		if (ret) {
> +			dev_err(&pdev->dev, "failed to enable irq wake\n");
> +			device_init_wakeup(&pdev->dev, false);
> +			atc_alarm_wakeup_enable(&pdev->dev, false);
> +			clear_bit(RTC_FEATURE_ALARM, atc_rtc->rtc_dev->features);
> +		} else {
> +			atc_alarm_wakeup_enable(&pdev->dev, true);
> +			set_bit(RTC_FEATURE_ALARM, atc_rtc->rtc_dev->features);
> +		}
> +	} else {
> +		atc_alarm_wakeup_enable(&pdev->dev, false);
> +		clear_bit(RTC_FEATURE_ALARM, atc_rtc->rtc_dev->features);
> +	}
> +	atc_rtc->rtc_dev->ops = &rtc_ops;
> +	atc_rtc->rtc_dev->range_min = ATCRTC_RTC_TIMESTAMP_BEGIN_2000;
> +	atc_rtc->rtc_dev->range_max = ATCRTC_RTC_TIMESTAMP_END_2089;
> +
> +	return devm_rtc_register_device(atc_rtc->rtc_dev);
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int atc_rtc_resume(struct device *dev)
> +{
> +	struct atc_rtc *rtc = dev_get_drvdata(dev);
> +
> +	if (device_may_wakeup(dev))
> +		disable_irq_wake(rtc->alarm_irq);
> +
> +	return 0;
> +}
> +
> +static int atc_rtc_suspend(struct device *dev)
> +{
> +	struct atc_rtc *rtc = dev_get_drvdata(dev);
> +
> +	if (device_may_wakeup(dev))
> +		enable_irq_wake(rtc->alarm_irq);
> +
> +	return 0;
> +}
> +#endif
> +static SIMPLE_DEV_PM_OPS(atc_rtc_pm_ops, atc_rtc_suspend, atc_rtc_resume);
> +
> +static const struct of_device_id atc_rtc_dt_match[] = {
> +	{ .compatible = "andestech,atcrtc100" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, atc_rtc_dt_match);
> +
> +static struct platform_driver atc_rtc_platform_driver = {
> +	.driver		= {
> +		.name	= "atcrtc100",
> +		.of_match_table	= of_match_ptr(atc_rtc_dt_match),

Drop of_match_ptr(), leads to warnings.

> +		.pm = &atc_rtc_pm_ops,
> +	},
> +	.probe		= atc_rtc_probe,
> +};
> +
> +module_platform_driver(atc_rtc_platform_driver);
> +
> +MODULE_AUTHOR("CL Wang <cl634@andestech.com>");
> +MODULE_DESCRIPTION("Andes ATCRTC100 driver");
> +MODULE_LICENSE("GPL");

Best regards,
Krzysztof