Hi Aleksandar,
You've not CC'ed any of the maintainers or mailing lists other than
linux-mips, so it's likely that reviewers will be unaware of your patchset.
scripts/get_maintainer.pl will show you which email addresses to send to.
On 19/06/17 16:49, Aleksandar Markovic wrote:
From: Aleksandar Markovic <aleksandar.markovic@xxxxxxxxxx>
Add device driver for a virtual Goldfish RTC clock.
The driver can be build only if CONFIG_MIPS and CONFIG_GOLDFISH
s/build/built
are set. The device tree key used by OS for binding the driver is
defined as "google,goldfish-rtc".
This is usually referred to as the compatible string
[...]
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+
+enum {
+ TIMER_TIME_LOW = 0x00, /* get low bits of current time */
+ /* and update TIMER_TIME_HIGH */
+ TIMER_TIME_HIGH = 0x04, /* get high bits of time at last */
+ /* TIMER_TIME_LOW read */
+ TIMER_ALARM_LOW = 0x08, /* set low bits of alarm and */
+ /* activate it */
+ TIMER_ALARM_HIGH = 0x0c, /* set high bits of next alarm */
+ TIMER_CLEAR_INTERRUPT = 0x10,
+ TIMER_CLEAR_ALARM = 0x14
+};
Why an enum rather than defines?
+
+struct goldfish_rtc {
+ void __iomem *base;
+ uint32_t irq;
s/uint32_t/u32
+ struct rtc_device *rtc;
+};
+
+static irqreturn_t
+goldfish_rtc_interrupt(int irq, void *dev_id)
You could make this into one line
+{
+ struct goldfish_rtc *qrtc = dev_id;
+ unsigned long events = 0;
+ void __iomem *base = qrtc->base;
+
+ writel(1, base + TIMER_CLEAR_INTERRUPT);
+ events = RTC_IRQF | RTC_AF;
+
+ rtc_update_irq(qrtc->rtc, 1, events);
+
+ return IRQ_HANDLED;
+}
+
+static int goldfish_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+ uint64_t time;
+ uint64_t time_low;
+ uint64_t time_high;
+ uint64_t time_high_prev;
s/uint64_t/u64
+ struct goldfish_rtc *qrtc =
+ platform_get_drvdata(to_platform_device(dev));
+ void __iomem *base = qrtc->base;
+
+ time_high = readl(base + TIMER_TIME_HIGH);
+ do {
+ time_high_prev = time_high;
+ time_low = readl(base + TIMER_TIME_LOW);
+ time_high = readl(base + TIMER_TIME_HIGH);
+ } while (time_high != time_high_prev);
+ time = ((int64_t)time_high << 32) | time_low;
Why a signed type?
+
+ do_div(time, NSEC_PER_SEC);
+
+ rtc_time_to_tm(time, tm);
+ return 0;
+}
+
+static const struct rtc_class_ops goldfish_rtc_ops = {
+ .read_time = goldfish_rtc_read_time,
+};
+
+static int goldfish_rtc_probe(struct platform_device *pdev)
+{
+ int ret;
You can remove this variable (see later for justification)
+ struct resource *r;
+ struct goldfish_rtc *qrtc;
+ unsigned long rtc_dev_len;
+ unsigned long rtc_dev_addr;
+
+ qrtc = devm_kzalloc(&pdev->dev, sizeof(*qrtc), GFP_KERNEL);
+ if (qrtc == NULL) {
if (!qrtc) {
+ ret = -ENOMEM;
+ goto error;
There's no need to goto error, so this can become:
return -ENOMEM;
+ }
+ platform_set_drvdata(pdev, qrtc);
+
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (r == NULL) {
if (!r) {
+ ret = -ENODEV;
+ goto error;
There's no need to goto error, so this can become:
return -ENODEV;
+ }
+
+ rtc_dev_addr = r->start;
+ rtc_dev_len = resource_size(r);
+ qrtc->base = devm_ioremap(&pdev->dev, rtc_dev_addr, rtc_dev_len);
+ if (IS_ERR(qrtc->base)) {
+ ret = -ENODEV;
+ goto error;
Ditto
+ }
+
+ qrtc->irq = platform_get_irq(pdev, 0);
+ if (qrtc->irq < 0) {
+ ret = -ENODEV;
+ goto error;
Ditto
+ }
+
+ qrtc->rtc = devm_rtc_device_register(&pdev->dev, pdev->name,
+ &goldfish_rtc_ops, THIS_MODULE);
+ if (IS_ERR(qrtc->rtc)) {
+ ret = PTR_ERR(qrtc->rtc);
+ goto error;
Ditto
+ }
+
+ ret = devm_request_irq(&pdev->dev, qrtc->irq, goldfish_rtc_interrupt,
+ 0, pdev->name, qrtc);
+ if (ret)
+ goto error;
Ditto
+
+ return 0;
+
+error:
+ return ret;
This label can be removed
+}
+
+static const struct of_device_id goldfish_rtc_of_match[] = {
+ { .compatible = "google,goldfish-rtc", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, goldfish_rtc_of_match);
+
+static struct platform_driver goldfish_rtc = {
+ .probe = goldfish_rtc_probe,
+ .driver = {
+ .name = "goldfish_rtc",
+ .of_match_table = goldfish_rtc_of_match,
+ }
+};
+
+module_platform_driver(goldfish_rtc);
Thanks,
Harvey
