Re: [PATCH v8 5/8] rtc: spacemit: support the SpacemiT P1 RTC

Alex Elder
2025-07-23T23:37:03Z

On 7/23/25 4:39 PM, Alexandre Belloni wrote:
> On 10/07/2025 12:51:03-0500, Alex Elder wrote:
>> +static int p1_rtc_read_time(struct device *dev, struct rtc_time *t)
>> +{
>> + struct p1_rtc *p1 = dev_get_drvdata(dev);
>> + struct regmap *regmap = p1->regmap;
>> + u32 count = RTC_READ_TRIES;
>> + u8 seconds;
>> + u8 time[6];
>> + int ret;
>> +
>> + if (!regmap_test_bits(regmap, RTC_CTRL, RTC_EN))
>> + return -ENODEV; /* RTC is disabled */
>
> That should be -EINVAL, as all the other drivers have standardized this.
>
> With this fixed,
> Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

OK thank you. I'll plan to send v9 with this change included.

-Alex

>
>
>> +
>> + ret = regmap_bulk_read(regmap, RTC_TIME, time, sizeof(time));
>> + if (ret)
>> + return ret;
>> +
>> + do {
>> + seconds = time[0];
>> + ret = regmap_bulk_read(regmap, RTC_TIME, time, sizeof(time));
>> + if (ret)
>> + return ret;
>> + } while (time[0] != seconds && --count);
>> +
>> + if (!count)
>> + return -EIO; /* Unable to get a consistent result */
>> +
>> + t->tm_sec = time[0] & GENMASK(5, 0);
>> + t->tm_min = time[1] & GENMASK(5, 0);
>> + t->tm_hour = time[2] & GENMASK(4, 0);
>> + t->tm_mday = (time[3] & GENMASK(4, 0)) + 1;
>> + t->tm_mon = time[4] & GENMASK(3, 0);
>> + t->tm_year = (time[5] & GENMASK(5, 0)) + 100;
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * The P1 hardware documentation states that values in the registers are
>> + * latched so when written they represent a consistent time snapshot.
>> + * Nevertheless, this is not guaranteed by the implementation, so we must
>> + * disable the RTC while updating it.
>> + */
>> +static int p1_rtc_set_time(struct device *dev, struct rtc_time *t)
>> +{
>> + struct p1_rtc *p1 = dev_get_drvdata(dev);
>> + struct regmap *regmap = p1->regmap;
>> + u8 time[6];
>> + int ret;
>> +
>> + time[0] = t->tm_sec;
>> + time[1] = t->tm_min;
>> + time[2] = t->tm_hour;
>> + time[3] = t->tm_mday - 1;
>> + time[4] = t->tm_mon;
>> + time[5] = t->tm_year - 100;
>> +
>> + /* Disable the RTC to update; re-enable again when done */
>> + ret = regmap_clear_bits(regmap, RTC_CTRL, RTC_EN);
>> + if (ret)
>> + return ret;
>> +
>> + /* If something goes wrong, leave the RTC disabled */
>> + ret = regmap_bulk_write(regmap, RTC_TIME, time, sizeof(time));
>> + if (ret)
>> + return ret;
>> +
>> + return regmap_set_bits(regmap, RTC_CTRL, RTC_EN);
>> +}
>> +
>> +static const struct rtc_class_ops p1_rtc_class_ops = {
>> + .read_time = p1_rtc_read_time,
>> + .set_time = p1_rtc_set_time,
>> +};
>> +
>> +static int p1_rtc_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct rtc_device *rtc;
>> + struct p1_rtc *p1;
>> +
>> + p1 = devm_kzalloc(dev, sizeof(*p1), GFP_KERNEL);
>> + if (!p1)
>> + return -ENOMEM;
>> + dev_set_drvdata(dev, p1);
>> +
>> + p1->regmap = dev_get_regmap(dev->parent, NULL);
>> + if (!p1->regmap)
>> + return dev_err_probe(dev, -ENODEV, "failed to get regmap\n");
>> +
>> + rtc = devm_rtc_allocate_device(dev);
>> + if (IS_ERR(rtc))
>> + return dev_err_probe(dev, PTR_ERR(rtc),
>> + "error allocating device\n");
>> + p1->rtc = rtc;
>> +
>> + rtc->ops = &p1_rtc_class_ops;
>> + rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
>> + rtc->range_max = RTC_TIMESTAMP_END_2063;
>> +
>> + clear_bit(RTC_FEATURE_ALARM, rtc->features);
>> + clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->features);
>> +
>> + return devm_rtc_register_device(rtc);
>> +}
>> +
>> +static struct platform_driver p1_rtc_driver = {
>> + .probe = p1_rtc_probe,
>> + .driver = {
>> + .name = MOD_NAME,
>> + },
>> +};
>> +
>> +module_platform_driver(p1_rtc_driver);
>> +
>> +MODULE_DESCRIPTION("SpacemiT P1 RTC driver");
>> +MODULE_LICENSE("GPL");
>> +MODULE_ALIAS("platform:" MOD_NAME);
>> --
>> 2.45.2
>>
>