Hi Krzysztof, Thanks for your reply. On 2024/8/26 16:27, Krzysztof Kozlowski wrote:
[ EXTERNAL EMAIL ] On 23/08/2024 11:19, Xianwei Zhao via B4 Relay wrote:From: Yiting Deng <yiting.deng@xxxxxxxxxxx> Support for the on-chip RTC found in some of Amlogic's SoCs such as the A113L2 and A113X2. Signed-off-by: Yiting Deng <yiting.deng@xxxxxxxxxxx> Signed-off-by: Xianwei Zhao <xianwei.zhao@xxxxxxxxxxx> --- drivers/rtc/Kconfig | 12 + drivers/rtc/Makefile | 1 + drivers/rtc/rtc-amlogic.c | 589 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 602 insertions(+) diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig index 2a95b05982ad..0dd042701c3b 100644 --- a/drivers/rtc/Kconfig +++ b/drivers/rtc/Kconfig @@ -2043,4 +2043,16 @@ config RTC_DRV_SSD202D This driver can also be built as a module, if so, the module will be called "rtc-ssd20xd". +config RTC_DRV_AMLOGIC + tristate "Amlogic RTC" + depends on ARCH_MESON || COMPILE_TESTSo the third driver? What is wrong with existing ones? And why this one is named so differently?
This RTC hardware includes a timing function and an alarm function.But the existing has only timing function, alarm function is using the system clock to implement a virtual alarm. The "meson" string is meaningless, it just keeps going, and now the new hardware uses the normal naming.
+ select REGMAP_MMIO + default y + help + If you say yes here you get support for the RTC block on the + Amlogic A113L2(A4) and A113X2(A5) SoCs. + + This driver can also be built as a module. If so, the module + will be called "rtc-amlogic". + endif # RTC_CLASS diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile index 3004e372f25f..d4a56ddb4246 100644 --- a/drivers/rtc/Makefile +++ b/drivers/rtc/Makefile @@ -26,6 +26,7 @@ obj-$(CONFIG_RTC_DRV_ABB5ZES3) += rtc-ab-b5ze-s3.o obj-$(CONFIG_RTC_DRV_ABEOZ9) += rtc-ab-eoz9.o obj-$(CONFIG_RTC_DRV_ABX80X) += rtc-abx80x.o obj-$(CONFIG_RTC_DRV_AC100) += rtc-ac100.o +obj-$(CONFIG_RTC_DRV_AMLOGIC) += rtc-amlogic.o obj-$(CONFIG_RTC_DRV_ARMADA38X) += rtc-armada38x.o obj-$(CONFIG_RTC_DRV_AS3722) += rtc-as3722.o obj-$(CONFIG_RTC_DRV_ASM9260) += rtc-asm9260.o+ +static int aml_rtc_adjust_sec(struct device *dev, u32 match_counter, + int ops, int enable) +{ + struct aml_rtc_data *rtc = dev_get_drvdata(dev->parent); + u32 reg_val; + + if (!FIELD_FIT(RTC_MATCH_COUNTER, match_counter)) { + pr_err("%s: invalid match_counter\n", __func__);No, do not use pr_, but driver ones.
Will do.
+ return -EINVAL; + } + + reg_val = FIELD_PREP(RTC_SEC_ADJUST_CTRL, ops) + | FIELD_PREP(RTC_MATCH_COUNTER, match_counter) + | FIELD_PREP(RTC_ADJ_VALID, enable); + /* Set sec_adjust_ctrl, match_counter and Valid adjust */ + regmap_write(rtc->map, RTC_SEC_ADJUST_REG, reg_val); + + return 0; +} + +static int aml_rtc_set_calibration(struct device *dev, u32 calibration) +{ + int cal_ops, enable, match_counter; + int ret; + + match_counter = FIELD_GET(RTC_MATCH_COUNTER, calibration); + cal_ops = FIELD_GET(RTC_SEC_ADJUST_CTRL, calibration); + enable = FIELD_GET(RTC_ADJ_VALID, calibration); + + ret = aml_rtc_adjust_sec(dev, match_counter, cal_ops, enable); + dev_dbg(dev, "%s: Success to store RTC calibration attribute\n", + __func__); + + return ret; +} + +static int aml_rtc_get_calibration(struct device *dev, u32 *calibration) +{ + struct aml_rtc_data *rtc = dev_get_drvdata(dev->parent); + u32 reg_val; + + regmap_read(rtc->map, RTC_SEC_ADJUST_REG, ®_val); + *calibration = FIELD_GET(RTC_SEC_ADJUST_CTRL | RTC_MATCH_COUNTER, reg_val); + /* BIT is only UL defined，and GENMASK has no type, its' donot used together */ + *calibration |= FIELD_PREP(RTC_ADJ_VALID, FIELD_GET(RTC_ADJ_VALID, reg_val)); + + return 0; +} + +/** + * The calibration value transferred from buf takes bit[18:0] to represent + * match_counter, while takes bit[20:19] to represent sec_adjust_ctrl, bit[23] + * to represent adj_valid. enable/disable sec_adjust_ctrl and match_counter. + * @buf: Separate buf to match_counter, sec_adjust_ctrl and adj_valid + * match_counter: bit[18:0], value is 0 ~ 0x7fff + * sec_adjust_ctrl: bit[20:19], value is 0 ~ 2. 3 to insert a second once every + * match_counter+1 seconds, 2 to swallow a second once every match_counter+1 seconds + * 0 or 1 to no adjustment + * adj_valid: bit[23], value is 0 or 1, 0 to disable sec_adjust_ctrl and + * match_counter, and 1 to enable them.Messed kerneldoc. You have warning shere. Fix it.
I will delete it.
+ */ +static ssize_t rtc_calibration_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + int retval; + int calibration = 0; + + if (sscanf(buf, " %i ", &calibration) != 1) { + dev_err(dev, "Failed to store RTC calibration attribute\n");Where is the ABI documented?
I will move it to the standard RTC API to handle calibration. So here will delete it.
+ return -EINVAL; + } + retval = aml_rtc_set_calibration(dev, calibration); + + return retval ? retval : count; +} + +static ssize_t rtc_calibration_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + int retval = 0; + u32 calibration = 0; + + retval = aml_rtc_get_calibration(dev, &calibration); + if (retval < 0) { + dev_err(dev, "Failed to read RTC calibration attribute\n"); + sprintf(buf, "0\n"); + return retval; + } + + return sprintf(buf, "0x%x\n", calibration); +} +static DEVICE_ATTR_RW(rtc_calibration);Document the ABI.
I will move it to the standard RTC API to handle calibration. So here will delete it.
+ +static int rtc_set_div256_adjust(struct device *dev, u32 *value) +{ + struct aml_rtc_data *rtc = dev_get_drvdata(dev->parent); + u32 div256_adj; + + div256_adj = FIELD_PREP(RTC_DIV256_ADJ_DSR | RTC_DIV256_ADJ_VAL, *value); + /* + * AO_RTC_SEC_ADJUST_REG bit 24 insert/remove(1/0) a div256 cycle, + * bit 25 valid/invalid(1/0) div256_adj_val + */ + regmap_write_bits(rtc->map, RTC_SEC_ADJUST_REG, + RTC_DIV256_ADJ_DSR | RTC_DIV256_ADJ_VAL, div256_adj); + /* rtc need about 30ms to adjust its time after written */ + mdelay(30); + + return 0; +} + +static int rtc_get_div256_adjust(struct device *dev, u32 *value) +{ + struct aml_rtc_data *rtc = dev_get_drvdata(dev->parent); + u32 reg_val; + + regmap_read(rtc->map, RTC_SEC_ADJUST_REG, ®_val); + *value = FIELD_GET(RTC_DIV256_ADJ_DSR | RTC_DIV256_ADJ_VAL, reg_val); + + return 0; +} + +/** + * div256 adjust function is controlled using bit[24] and bit[25]. + * transferred buf takes bit[0] to represent div256 adj val, bit[1] + * to represent div256 adj enable/disable. div256 cycle means that adjust + * 1/32768/256 once by written once, it's val is equal to 1/128s + * @buf: 3: enable div256 adjust and insert a div256 cycle + * 2: enable div256 adjust and remove a div256 cycle + * 1 or 0: disable div256 adjustAgain incorrect kerneldoc.
This is not used functions, I will delete it.
+ */ +static ssize_t rtc_div256_adjust_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + int retval; + u32 value = 0; + + if (sscanf(buf, " %i ", &value) != 1) { + dev_err(dev, "Failed to store RTC div256 adjust attribute\n"); + return -EINVAL; + } + retval = rtc_set_div256_adjust(dev, &value); + + return retval ? retval : count; +} + +static ssize_t rtc_div256_adjust_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + int retval = 0; + u32 value = 0; + + retval = rtc_get_div256_adjust(dev, &value); + if (retval < 0) { + dev_err(dev, "Failed to read RTC div256 adjust attribute\n"); + sprintf(buf, "0\n"); + return retval; + } + + return sprintf(buf, "0x%x\n", value); +} +static DEVICE_ATTR_RW(rtc_div256_adjust); + +static struct attribute *aml_rtc_attrs[] = { + &dev_attr_rtc_calibration.attr, + &dev_attr_rtc_div256_adjust.attr, + NULL, +}; + +static const struct attribute_group aml_rtc_sysfs_files = { + .attrs = aml_rtc_attrs, +}; + +static void aml_rtc_init(struct device *dev, struct aml_rtc_data *rtc) +{ + u32 reg_val; + u32 rtc_enable; + + regmap_read(rtc->map, RTC_CTRL, ®_val); + rtc_enable = FIELD_GET(RTC_ENABLE, reg_val); + if (!rtc_enable) { + if (clk_get_rate(rtc->sclk) == OSC_24M) { + /* select 24M oscillator */ + regmap_update_bits(rtc->map, RTC_CTRL, RTC_OSC_SEL, RTC_OSC_SEL); + + /* + * Set RTC oscillator to freq_out to freq_in/((N0*M0+N1*M1)/(M0+M1)) + * Enable clock_in gate of oscillator 24MHz + * Set N0 to 733, N1 to 732 + */ + reg_val = FIELD_PREP(RTC_OSCIN_IN_EN, 1) + | FIELD_PREP(RTC_OSCIN_OUT_CFG, 1) + | FIELD_PREP(RTC_OSCIN_OUT_N0M0, RTC_OSCIN_OUT_32K_N0) + | FIELD_PREP(RTC_OSCIN_OUT_N1M1, RTC_OSCIN_OUT_32K_N1); + regmap_write_bits(rtc->map, RTC_OSCIN_CTRL0, RTC_OSCIN_IN_EN + | RTC_OSCIN_OUT_CFG | RTC_OSCIN_OUT_N0M0 + | RTC_OSCIN_OUT_N1M1, reg_val); + + /* Set M0 to 2, M1 to 3, so freq_out = 32768 Hz*/ + reg_val = FIELD_PREP(RTC_OSCIN_OUT_N0M0, RTC_OSCIN_OUT_32K_M0) + | FIELD_PREP(RTC_OSCIN_OUT_N1M1, RTC_OSCIN_OUT_32K_M1); + regmap_write_bits(rtc->map, RTC_OSCIN_CTRL1, RTC_OSCIN_OUT_N0M0 + | RTC_OSCIN_OUT_N1M1, reg_val); + } else { + /* select 32K oscillator */ + regmap_write_bits(rtc->map, RTC_CTRL, RTC_OSC_SEL, 0); + } + /* Enable RTC */ + regmap_write_bits(rtc->map, RTC_CTRL, RTC_ENABLE, RTC_ENABLE); + usleep_range(100, 200); + } + regmap_write_bits(rtc->map, RTC_INT_MASK, + RTC_ALRM0_IRQ_MSK, RTC_ALRM0_IRQ_MSK); + regmap_write_bits(rtc->map, RTC_CTRL, RTC_ALRM0_EN, 0); +} + +static int aml_rtc_probe(struct platform_device *pdev) +{ + struct aml_rtc_data *rtc; + void __iomem *base; + struct clk *core_clk; + int ret = 0; + + rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL); + if (!rtc) + return -ENOMEM; + + rtc->config = of_device_get_match_data(&pdev->dev); + if (!rtc->config) + return -ENODEV; + + base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(base)) { + dev_err(&pdev->dev, "resource ioremap failed\n"); + return PTR_ERR(base); + } + + rtc->map = devm_regmap_init_mmio(&pdev->dev, base, &aml_rtc_regmap_config); + if (IS_ERR(rtc->map)) { + dev_err(&pdev->dev, "regmap init failed\n"); + return PTR_ERR(rtc->map); + } + + rtc->irq = platform_get_irq(pdev, 0); + if (rtc->irq < 0) + return rtc->irq; + + rtc->sclk = devm_clk_get(&pdev->dev, "rtc_osc");Clock name should be: "osc"
Will do.
+ if (IS_ERR(rtc->sclk)) + return PTR_ERR(rtc->sclk); + if (clk_get_rate(rtc->sclk) != OSC_32K && clk_get_rate(rtc->sclk) != OSC_24M) { + dev_err(&pdev->dev, "Invalid clock configuration\n"); + return -EINVAL; + } + + core_clk = devm_clk_get(&pdev->dev, "rtc_sys_clk");Clock name: "sys"
Will do.
+ if (IS_ERR(core_clk)) { + dev_err(&pdev->dev, "failed to get rtc sys clk\n");Syntax is return dev_err_probe.
Will do.
+ return PTR_ERR(core_clk); + } + clk_prepare_enable(core_clk); + + aml_rtc_init(&pdev->dev, rtc); + + device_init_wakeup(&pdev->dev, 1); + platform_set_drvdata(pdev, rtc); + + rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev); + if (IS_ERR(rtc->rtc_dev)) + return PTR_ERR(rtc->rtc_dev); + + ret = devm_request_irq(&pdev->dev, rtc->irq, aml_rtc_handler, + IRQF_ONESHOT, "aml-rtc alarm", rtc); + if (ret) { + dev_err(&pdev->dev, "IRQ%d request failed, ret = %d\n", + rtc->irq, ret);Your code is buggy. You leave with prepared clock. Use devm_clk_get_enabled where applicable.
Will fix it.
+ return ret; + } + + rtc->rtc_dev->ops = &aml_rtc_ops; + rtc->rtc_dev->range_min = 0; + rtc->rtc_dev->range_max = U32_MAX; + + ret = rtc_add_group(rtc->rtc_dev, &aml_rtc_sysfs_files); + if (ret) { + dev_err(&pdev->dev, "Failed to create sysfs group: %d\n", ret); + return ret; + } + + return devm_rtc_register_device(rtc->rtc_dev); +} + +static int aml_rtc_suspend(struct device *dev) +{ + struct aml_rtc_data *rtc = dev_get_drvdata(dev); + + if (device_may_wakeup(dev)) + enable_irq_wake(rtc->irq); + + return 0; +} + +static int aml_rtc_resume(struct device *dev) +{ + struct aml_rtc_data *rtc = dev_get_drvdata(dev); + + if (device_may_wakeup(dev)) + disable_irq_wake(rtc->irq); + + return 0; +} +Where is the remove to cleanup?
Will add remove function.
+static SIMPLE_DEV_PM_OPS(aml_rtc_pm_ops, + aml_rtc_suspend, aml_rtc_resume); + +static const struct aml_rtc_config a5_rtc_config = { +}; + +static const struct aml_rtc_config a4_rtc_config = { + .gray_stored = true, +}; + +static const struct of_device_id aml_rtc_device_id[] = { + { + .compatible = "amlogic,a4-rtc", + .data = &a4_rtc_config, + }, + { + .compatible = "amlogic,a5-rtc", + .data = &a5_rtc_config, + }, +}; +MODULE_DEVICE_TABLE(of, aml_rtc_device_id); + +static struct platform_driver aml_rtc_driver = { + .probe = aml_rtc_probe, + .driver = { + .name = "aml-rtc", + .of_match_table = of_match_ptr(aml_rtc_device_id),Drop of_match_ptr. You have a warning here.
Will do.
+ .pm = &aml_rtc_pm_ops, + },Best regards, Krzysztof
