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

The subject line should be: PHYTIUM: rtc: Add Phytium rtc device support

在 2021/3/20 18:30, Xin Hao 写道: Which API used this header file? Please remove it if unused.

Remove redundant blank line.

No need to initialize them. Can you add some comments to explain why we need read RTC_CDR_LOW register, but did not use the return value?

Ditto 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.

> + .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 {

_______________________________________________ Cloud Kernel mailing list -- cloud-kernel(a)lists.openanolis.org To unsubscribe send an email to cloud-kernel-leave(a)lists.openanolis.org