------------------------------------------------------------------

发送时间：2021年3月22日(星期一) 15:37

收件人：cloud-kernel <cloud-kernel@lists.openanolis.cn>

主　题：[ck] Re: [PATCH openAnolis v1 2/4] PHYTIUM: rtc: new driver: rtc-phytium add phytium rtc device support

在 2021/3/22 上午10:28, Baolin Wang 写道:
>
> The subject line should be:
> PHYTIUM: rtc: Add Phytium rtc device support
done
>
> 在 2021/3/20 18:30, Xin Hao 写道:
>> Add phytium ft25000 platform rtc driver.
>>
>> Signed-off-by: Xin Hao <xingxin.hx@openanolis.cn>
>> ---
>>   drivers/i2c/busses/i2c-designware-platdrv.c |   1 +
>>   drivers/rtc/Kconfig                         |  10 +
>>   drivers/rtc/Makefile                        |   1 +
>>   drivers/rtc/rtc-phytium.c                   | 316 ++++++++++++++++++++
>>   4 files changed, 328 insertions(+)
>>   create mode 100644 drivers/rtc/rtc-phytium.c
>>
>> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
>> b/drivers/i2c/busses/i2c-designware-platdrv.c
>> index b5750fd85125..3818b7a0a847 100644
>> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
>> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
>> @@ -151,6 +151,7 @@ static const struct acpi_device_id
>> dw_i2c_acpi_match[] = {
>>       { "APMC0D0F", 0 },
>>       { "HISI02A1", 0 },
>>       { "HISI02A2", 0 },
>> +    { "PHYT0003", 0 },
>>       { }
>>   };
>>   MODULE_DEVICE_TABLE(acpi, dw_i2c_acpi_match);
>> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
>> index 7d7be60a2413..3e4effbf31ef 100644
>> --- a/drivers/rtc/Kconfig
>> +++ b/drivers/rtc/Kconfig
>> @@ -1793,6 +1793,16 @@ config RTC_DRV_RTD119X
>>         If you say yes here, you get support for the RTD1295 SoC
>>         Real Time Clock.
>>   +config RTC_DRV_PHYTIUM
>> +    tristate "Phytium RTC"
>> +    depends on ARCH_PHYTIUM
>> +    default y if ARCH_PHYTIUM
>> +    help
>> +      Say yes here to support the Phytium SoC real time clock.
>> +
>> +      This driver can also be built as a module, if so, the module
>> +      will be called "rtc-phytium".
>> +
>>   comment "HID Sensor RTC drivers"
>>     config RTC_DRV_HID_SENSOR_TIME
>> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
>> index 5ff2fc0c361a..289447233c6b 100644
>> --- a/drivers/rtc/Makefile
>> +++ b/drivers/rtc/Makefile
>> @@ -120,6 +120,7 @@ obj-$(CONFIG_RTC_DRV_PCF85363)    += rtc-pcf85363.o
>>   obj-$(CONFIG_RTC_DRV_PCF8523)    += rtc-pcf8523.o
>>   obj-$(CONFIG_RTC_DRV_PCF8563)    += rtc-pcf8563.o
>>   obj-$(CONFIG_RTC_DRV_PCF8583)    += rtc-pcf8583.o
>> +obj-$(CONFIG_RTC_DRV_PHYTIUM)    += rtc-phytium.o
>>   obj-$(CONFIG_RTC_DRV_PIC32)    += rtc-pic32.o
>>   obj-$(CONFIG_RTC_DRV_PL030)    += rtc-pl030.o
>>   obj-$(CONFIG_RTC_DRV_PL031)    += rtc-pl031.o
>> diff --git a/drivers/rtc/rtc-phytium.c b/drivers/rtc/rtc-phytium.c
>> new file mode 100644
>> index 000000000000..a9b5f52a9e8f
>> --- /dev/null
>> +++ b/drivers/rtc/rtc-phytium.c
>> @@ -0,0 +1,316 @@
>> +/*
>> + * Phytium Real Time Clock Driver
>> + *
>> + * Copyright (c) 2019, Phytium Technology Co., Ltd.
>> + *
>> + * Chen Baozi <chenbaozi@phytium.com.cn>
>> + *
>> + * This program is free software; you can redistribute  it and/or
>> modify it
>> + * under  the terms of  the GNU General  Public License as published
>> by the
>> + * Free Software Foundation;  either version 2 of the  License, or
>> (at your
>> + * option) any later version.
>> + *
>> + */
>> +
>> +#include <linux/init.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/io.h>
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>
> Which API used this header file? Please remove it if unused.
done
>
>> +#include <linux/acpi.h>
>> + /* registers and masks need comments or No one knows what these are*/
>> +#define RTC_CMR                 0x04
>> +#define RTC_AES_SEL             0x08
>> +#define RTC_AES_SEL_COUNTER     0x100
>> +#define RTC_CCR                 0x0C
>> +#define RTC_CCR_IE              BIT(0)
>> +#define RTC_CCR_MASK            BIT(1)
>> +#define RTC_CCR_EN              BIT(2)
>> +#define RTC_CCR_WEN             BIT(3)
>> +#define RTC_STAT                0x10
>> +#define RTC_STAT_BIT            BIT(0)
>> +#define RTC_RSTAT               0x14
>> +#define RTC_EOI                 0x18
>> +#define RTC_VER                 0x1C
>> +#define RTC_CDR_LOW             0x20
>> +#define RTC_CCVR                0x24
>> +#define RTC_CLR_LOW             0x28
>> +#define RTC_CLR                 0x2c
>> +#define RTC_COUNTER_HB_OFFSET   15
>> +#define RTC_COUNTER_LB_MASK     GENMASK(14, 0)
>> +
>> +struct phytium_rtc_dev {
>> +    struct rtc_device *rtc;
>> +    struct device *dev;
>> +    unsigned long alarm_time;
>> +    void __iomem *csr_base;
>> +    struct clk *clk;
>> +    unsigned int irq_wake;
>> +    unsigned int irq_enabled;
>> +};
>> +
>> +static int phytium_rtc_read_time(struct device *dev, struct rtc_time
>> *tm)
>> +{
>> +    struct phytium_rtc_dev *pdata = dev_get_drvdata(dev);
>> +
>
> Remove redundant blank line.
done
>> +    unsigned long counter = 0;
>> +    unsigned long tmp = 0;
>
> No need to initialize them.
>
>> +
>> +    writel(RTC_AES_SEL_COUNTER, pdata->csr_base + RTC_AES_SEL);
>> +    counter = readl(pdata->csr_base + RTC_CCVR);
>> +    tmp = readl(pdata->csr_base + RTC_CDR_LOW); //why? I am confusing
>
> Can you add some comments to explain why we need read RTC_CDR_LOW
> register, but did not use the return value?
the PHYTIUM engineer can explain it ? thanks
>
>> +
>> +    rtc_time_to_tm(counter, tm);
>> +    return rtc_valid_tm(tm);
>> +}
>> +
>> +static int phytium_rtc_set_mmss(struct device *dev, unsigned long secs)
>> +{
>> +    struct phytium_rtc_dev *pdata = dev_get_drvdata(dev);
>> +    unsigned long counter = 0;
>> +    unsigned long tmp = 0;
>
> Ditto
>
>> +
>> +    writel(RTC_AES_SEL_COUNTER, pdata->csr_base + RTC_AES_SEL);
>> +    writel(0x00000000, pdata->csr_base + RTC_CLR_LOW);
>> +    writel((u32)secs, pdata->csr_base + RTC_CLR);
>> +    writel(RTC_AES_SEL_COUNTER, pdata->csr_base + RTC_AES_SEL);
>> +    counter = readl(pdata->csr_base + RTC_CLR);   // what does these mean?
>> +    tmp = readl(pdata->csr_base + RTC_CLR_LOW);
>> +
>> +    return 0;
>> +}
>> +
>> +static int phytium_rtc_read_alarm(struct device *dev, struct
>> rtc_wkalrm *alrm)
>> +{
>> +    struct phytium_rtc_dev *pdata = dev_get_drvdata(dev);
>> +
>> +    rtc_time_to_tm(pdata->alarm_time, &alrm->time);
>> +    alrm->enabled = readl(pdata->csr_base + RTC_CCR) & RTC_CCR_IE;
>> +
>> +    return 0;
>> +}
>> +
>> +static int phytium_rtc_alarm_irq_enable(struct device *dev, u32
>> enabled)
>> +{
>> +    struct phytium_rtc_dev *pdata = dev_get_drvdata(dev);
>> +    u32 ccr;
>> +
>> +    ccr = readl(pdata->csr_base + RTC_CCR);
>> +    if (enabled) {
>> +        ccr &= ~RTC_CCR_MASK;
>> +        ccr |= RTC_CCR_IE;
>> +    } else {
>> +        ccr &= ~RTC_CCR_IE;
>> +        ccr |= RTC_CCR_MASK;
>> +    }
>> +    writel(ccr, pdata->csr_base + RTC_CCR);
>> +
>> +    return 0;
>> +}
>> + // it is very stange here. I think you made a mistake.
>> +static int phytium_rtc_alarm_irq_enabled(struct device *dev)
>> +{
>> +    struct phytium_rtc_dev *pdata = dev_get_drvdata(dev);
>> +
>> +    return readl(pdata->csr_base + RTC_CCR) & RTC_CCR_IE ? 1 : 0;
>> +}
>> +
>> +static int phytium_rtc_set_alarm(struct device *dev, struct
>> rtc_wkalrm *alrm)
>> +{
>> +    struct phytium_rtc_dev *pdata = dev_get_drvdata(dev);
>> +    unsigned long alarm_time; //the variable is never read in the other functions, why not to put it in private data instead of as a local variable

>> +
>> +    rtc_tm_to_time(&alrm->time, &alarm_time);
>> +
>> +    pdata->alarm_time = alarm_time;
>> +    writel((u32) pdata->alarm_time, pdata->csr_base + RTC_CMR);
>> +
>> +    phytium_rtc_alarm_irq_enable(dev, alrm->enabled);
>> +
>> +    return 0;
>> +}
>> +
>> +static const struct rtc_class_ops phytium_rtc_ops = {
>> +    .read_time    = phytium_rtc_read_time,
>> +    .set_mmss    = phytium_rtc_set_mmss,
>
> I still think we should change to implement the 'set_time' ops, since
> the set_mmss is deprecated in CK 5.10 and mainline kernel.
no change
>
>> +    .read_alarm    = phytium_rtc_read_alarm,
>> +    .set_alarm    = phytium_rtc_set_alarm,
>> +    .alarm_irq_enable = phytium_rtc_alarm_irq_enable,
>> +};
>> + /* the error handling is really ugly, you should handle these error at the end of the function and you miss some resources 。I advise u to learn to use goto to handle errors*/

>> +static irqreturn_t phytium_rtc_interrupt(int irq, void *id)
>> +{
>> +    struct phytium_rtc_dev *pdata = (struct phytium_rtc_dev *) id;
>> +
>> +    /* Check if interrupt asserted */
>> +    if (!(readl(pdata->csr_base + RTC_STAT) & RTC_STAT_BIT))
>> +        return IRQ_NONE;
>> +
>> +    /* Clear interrupt */
>> +    readl(pdata->csr_base + RTC_EOI);
>> +
>> +    rtc_update_irq(pdata->rtc, 1, RTC_IRQF | RTC_AF);
>> +
>> +    return IRQ_HANDLED;
>> +}
>> +
>> +static int phytium_rtc_probe(struct platform_device *pdev)
>> +{
>> +    struct phytium_rtc_dev *pdata;
>> +    struct resource *res;
>> +    int ret;
>> +    int irq;
>> +
>> +    pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
>> +    if (!pdata)
>> +        return -ENOMEM;
>> +    platform_set_drvdata(pdev, pdata);
>> +    pdata->dev = &pdev->dev;
>> +
>> +    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +    pdata->csr_base = devm_ioremap_resource(&pdev->dev, res);
>> +    if (IS_ERR(pdata->csr_base))
>> +        return PTR_ERR(pdata->csr_base);
>> +
>> +    irq = platform_get_irq(pdev, 0);
>> +    if (irq < 0) {
>> +        dev_err(&pdev->dev, "No IRQ resource\n");
>> +        return irq;
>> +    }
>> +    ret = devm_request_irq(&pdev->dev, irq, phytium_rtc_interrupt, 0,
>> +                   dev_name(&pdev->dev), pdata);
>> +    if (ret) {
>> +        dev_err(&pdev->dev, "Could not request IRQ\n");
>> +        return ret;
>> +    }
>> +
>> +#ifndef CONFIG_ACPI
>> +    pdata->clk = devm_clk_get(&pdev->dev, NULL);
>> +    if (IS_ERR(pdata->clk)) {
>> +        dev_err(&pdev->dev, "Couldn't get the clock for RTC\n");
>> +        return -ENODEV;
>> +    }
>> +
>> +    ret = clk_prepare_enable(pdata->clk);
>> +    if (ret)
>> +        return ret;
>> +#endif
>> +
>> +    /* Turn on the clock and the crystal */
>> +    writel(RTC_CCR_EN, pdata->csr_base + RTC_CCR);
>> +
>> +    ret = device_init_wakeup(&pdev->dev, 1);
>> +    if (ret) {
>> +        clk_disable_unprepare(pdata->clk);
>> +        return ret;
>> +    }
>> +
>> +    pdata->rtc = devm_rtc_device_register(&pdev->dev, pdev->name,
>> +                     &phytium_rtc_ops, THIS_MODULE);
>> +    if (IS_ERR(pdata->rtc)) {
>> +        clk_disable_unprepare(pdata->clk);
>> +        return PTR_ERR(pdata->rtc);
>> +    }
>> +
>> +    /* HW does not support update faster than 1 seconds this variable is never used */
>> +    pdata->rtc->uie_unsupported = 1;
>> +
>> +    return 0;
>> +}
>> +
>> +static int phytium_rtc_remove(struct platform_device *pdev)
>> +{
>> +    struct phytium_rtc_dev *pdata = platform_get_drvdata(pdev);
>> +    //why enable this irq hers?
>> +    phytium_rtc_alarm_irq_enable(&pdev->dev, 0);
>> +    device_init_wakeup(&pdev->dev, 0);
>> +    clk_disable_unprepare(pdata->clk);
>> +    return 0;
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int phytium_rtc_suspend(struct device *dev)
>> +{   //the same as the phytium_rtc_resume, all the drive private information should be got at probe time and stored for later use

>> +    struct platform_device *pdev = to_platform_device(dev);
>> +    struct phytium_rtc_dev *pdata = platform_get_drvdata(pdev);
>> +    int irq;
>> +
>> +    /*
>> +     * If this RTC alarm will be used for waking the system up,
>> +     * don't disable it of course. Else we just disable the alarm
>> +     * and await suspension.
>> +     */
>> +    irq = platform_get_irq(pdev, 0);
>> +    if (device_may_wakeup(&pdev->dev)) {
>> +        if (!enable_irq_wake(irq))
>> +            pdata->irq_wake = 1;
>> +    } else {
>> +        pdata->irq_enabled = phytium_rtc_alarm_irq_enabled(dev);
>> +        phytium_rtc_alarm_irq_enable(dev, 0);
>> +        clk_disable_unprepare(pdata->clk);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int phytium_rtc_resume(struct device *dev)
>> +{
>> +    struct platform_device *pdev = to_platform_device(dev);
>> +    struct phytium_rtc_dev *pdata = platform_get_drvdata(pdev);
>> +    int irq;
>> +    int rc;
>> +    //This irq should be got this irq in probe procedure and be stored in your driver private data.
>> +    irq = platform_get_irq(pdev, 0);
>> +    if (device_may_wakeup(&pdev->dev)) {
>> +        if (pdata->irq_wake) {
>> +            disable_irq_wake(irq);
>> +            pdata->irq_wake = 0;
>> +        }
>> +    } else {

//This clock is the same as above.

>> +        rc = clk_prepare_enable(pdata->clk);
>> +        if (rc) {
>> +            dev_err(dev, "Unable to enable clock error %d\n", rc);
>> +            return rc;
>> +        }
>> +        phytium_rtc_alarm_irq_enable(dev, pdata->irq_enabled);
>> +    }
>> +
>> +    return 0;
>> +}
>> +#endif
>> +
>> +static SIMPLE_DEV_PM_OPS(phytium_rtc_pm_ops, phytium_rtc_suspend,
>> phytium_rtc_resume);
>> +
>> +#ifdef CONFIG_OF
>> +static const struct of_device_id phytium_rtc_of_match[] = {
>> +    { .compatible = "phytium,rtc" },
>> +    { }
>> +};
>> +MODULE_DEVICE_TABLE(of, phytium_rtc_of_match);
>> +#endif
>> +
>> +#ifdef CONFIG_ACPI
>> +static const struct acpi_device_id phytium_rtc_acpi_match[] = {
>> +    { "PHYT0002", 0 },
>> +    { }
>> +};
>> +#endif
>> +
>> +static struct platform_driver phytium_rtc_driver = {
>> +    .probe        = phytium_rtc_probe,
>> +    .remove        = phytium_rtc_remove,
>> +    .driver        = {
>> +        .name    = "phytium-rtc",
>> +        .pm = &phytium_rtc_pm_ops,
>> +        .of_match_table    = of_match_ptr(phytium_rtc_of_match),
>> +        .acpi_match_table = ACPI_PTR(phytium_rtc_acpi_match),
>> +    },
>> +};
>> +
>> +module_platform_driver(phytium_rtc_driver);
>> +
>> +MODULE_DESCRIPTION("Phytium RTC driver");
>> +MODULE_AUTHOR("Chen Baozi <chenbaozi@phytium.com.cn>");
>> +MODULE_LICENSE("GPL");
>>
> _______________________________________________
> Cloud Kernel mailing list -- cloud-kernel@lists.openanolis.cn
> To unsubscribe send an email to cloud-kernel-leave@lists.openanolis.cn

--
Best Regards!

_______________________________________________
Cloud Kernel mailing list -- cloud-kernel@lists.openanolis.cn
To unsubscribe send an email to cloud-kernel-leave@lists.openanolis.cn