linux-rtc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] rtc: tegra: Dust off and deferred probe support
@ 2019-05-27 10:13 Thierry Reding
  2019-05-27 10:13 ` [PATCH 1/3] rtc: tegra: checkpatch and miscellaneous cleanups Thierry Reding
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Thierry Reding @ 2019-05-27 10:13 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: Jon Hunter, Kartik Kartik, linux-rtc, linux-tegra, linux-kernel

From: Thierry Reding <treding@nvidia.com>

The NVIDIA Tegra RTC driver has accumulated a bit of dust over the
years. Make a pass over it, addressing checkpatch warnings and fixing
some inconsistencies in comments and kernel messages as well as in
variable types and names.

Once cleaned up, also turn the driver into a regular driver in order
to support deferred probe which is needed to avoid a future regression
on Tegra186 and later.

Thierry

Thierry Reding (3):
  rtc: tegra: checkpatch and miscellaneous cleanups
  rtc: tegra: Use consistent variable names and types
  rtc: tegra: Turn into regular driver

 drivers/rtc/rtc-tegra.c | 254 ++++++++++++++++++++--------------------
 1 file changed, 128 insertions(+), 126 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] rtc: tegra: checkpatch and miscellaneous cleanups
  2019-05-27 10:13 [PATCH 0/3] rtc: tegra: Dust off and deferred probe support Thierry Reding
@ 2019-05-27 10:13 ` Thierry Reding
  2019-05-27 11:28   ` Dmitry Osipenko
  2019-05-27 10:13 ` [PATCH 2/3] rtc: tegra: Use consistent variable names and types Thierry Reding
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 6+ messages in thread
From: Thierry Reding @ 2019-05-27 10:13 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: Jon Hunter, Kartik Kartik, linux-rtc, linux-tegra, linux-kernel

From: Thierry Reding <treding@nvidia.com>

This set of changes fixes some checkpatch warnings as well as a number
of punctuation and padding inconsistencies.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/rtc/rtc-tegra.c | 154 +++++++++++++++++++++-------------------
 1 file changed, 79 insertions(+), 75 deletions(-)

diff --git a/drivers/rtc/rtc-tegra.c b/drivers/rtc/rtc-tegra.c
index f0ce76865434..a67c9d8be4f6 100644
--- a/drivers/rtc/rtc-tegra.c
+++ b/drivers/rtc/rtc-tegra.c
@@ -18,10 +18,10 @@
 #include <linux/rtc.h>
 #include <linux/slab.h>
 
-/* set to 1 = busy every eight 32kHz clocks during copy of sec+msec to AHB */
+/* Set to 1 = busy every eight 32 kHz clocks during copy of sec+msec to AHB. */
 #define TEGRA_RTC_REG_BUSY			0x004
 #define TEGRA_RTC_REG_SECONDS			0x008
-/* when msec is read, the seconds are buffered into shadow seconds. */
+/* When msec is read, the seconds are buffered into shadow seconds. */
 #define TEGRA_RTC_REG_SHADOW_SECONDS		0x00c
 #define TEGRA_RTC_REG_MILLI_SECONDS		0x010
 #define TEGRA_RTC_REG_SECONDS_ALARM0		0x014
@@ -46,44 +46,48 @@
 #define TEGRA_RTC_INTR_STATUS_SEC_ALARM0	(1<<0)
 
 struct tegra_rtc_info {
-	struct platform_device	*pdev;
-	struct rtc_device	*rtc_dev;
-	void __iomem		*rtc_base; /* NULL if not initialized. */
-	struct clk		*clk;
-	int			tegra_rtc_irq; /* alarm and periodic irq */
-	spinlock_t		tegra_rtc_lock;
+	struct platform_device *pdev;
+	struct rtc_device *rtc_dev;
+	void __iomem *rtc_base; /* NULL if not initialized */
+	struct clk *clk;
+	int tegra_rtc_irq; /* alarm and periodic IRQ */
+	spinlock_t tegra_rtc_lock;
 };
 
-/* RTC hardware is busy when it is updating its values over AHB once
- * every eight 32kHz clocks (~250uS).
- * outside of these updates the CPU is free to write.
- * CPU is always free to read.
+/*
+ * RTC hardware is busy when it is updating its values over AHB once every
+ * eight 32 kHz clocks (~250 us). Outside of these updates the CPU is free to
+ * write. CPU is always free to read.
  */
 static inline u32 tegra_rtc_check_busy(struct tegra_rtc_info *info)
 {
 	return readl(info->rtc_base + TEGRA_RTC_REG_BUSY) & 1;
 }
 
-/* Wait for hardware to be ready for writing.
- * This function tries to maximize the amount of time before the next update.
- * It does this by waiting for the RTC to become busy with its periodic update,
- * then returning once the RTC first becomes not busy.
+/*
+ * Wait for hardware to be ready for writing. This function tries to maximize
+ * the amount of time before the next update. It does this by waiting for the
+ * RTC to become busy with its periodic update, then returning once the RTC
+ * first becomes not busy.
+ *
  * This periodic update (where the seconds and milliseconds are copied to the
- * AHB side) occurs every eight 32kHz clocks (~250uS).
- * The behavior of this function allows us to make some assumptions without
- * introducing a race, because 250uS is plenty of time to read/write a value.
+ * AHB side) occurs every eight 32 kHz clocks (~250 us). The behavior of this
+ * function allows us to make some assumptions without introducing a race,
+ * because 250 us is plenty of time to read/write a value.
  */
 static int tegra_rtc_wait_while_busy(struct device *dev)
 {
 	struct tegra_rtc_info *info = dev_get_drvdata(dev);
+	int retries = 500; /* ~490 us is the worst case, ~250 us is best */
 
-	int retries = 500; /* ~490 us is the worst case, ~250 us is best. */
-
-	/* first wait for the RTC to become busy. this is when it
-	 * posts its updated seconds+msec registers to AHB side. */
+	/*
+	 * First wait for the RTC to become busy. This is when it posts its
+	 * updated seconds+msec registers to AHB side.
+	 */
 	while (tegra_rtc_check_busy(info)) {
 		if (!retries--)
 			goto retry_failed;
+
 		udelay(1);
 	}
 
@@ -91,7 +95,7 @@ static int tegra_rtc_wait_while_busy(struct device *dev)
 	return 0;
 
 retry_failed:
-	dev_err(dev, "write failed:retry count exceeded.\n");
+	dev_err(dev, "write failed: retry count exceeded\n");
 	return -ETIMEDOUT;
 }
 
@@ -101,8 +105,10 @@ static int tegra_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	unsigned long sec, msec;
 	unsigned long sl_irq_flags;
 
-	/* RTC hardware copies seconds to shadow seconds when a read
-	 * of milliseconds occurs. use a lock to keep other threads out. */
+	/*
+	 * RTC hardware copies seconds to shadow seconds when a read of
+	 * milliseconds occurs. use a lock to keep other threads out.
+	 */
 	spin_lock_irqsave(&info->tegra_rtc_lock, sl_irq_flags);
 
 	msec = readl(info->rtc_base + TEGRA_RTC_REG_MILLI_SECONDS);
@@ -112,7 +118,7 @@ static int tegra_rtc_read_time(struct device *dev, struct rtc_time *tm)
 
 	rtc_time64_to_tm(sec, tm);
 
-	dev_vdbg(dev, "time read as %lu. %ptR\n", sec, tm);
+	dev_vdbg(dev, "time read as %u, %ptR\n", sec, tm);
 
 	return 0;
 }
@@ -123,18 +129,18 @@ static int tegra_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	unsigned long sec;
 	int ret;
 
-	/* convert tm to seconds. */
+	/* convert tm to seconds */
 	sec = rtc_tm_to_time64(tm);
 
-	dev_vdbg(dev, "time set to %lu. %ptR\n", sec, tm);
+	dev_vdbg(dev, "time set to %u, %ptR\n", sec, tm);
 
-	/* seconds only written if wait succeeded. */
+	/* seconds only written if wait succeeded */
 	ret = tegra_rtc_wait_while_busy(dev);
 	if (!ret)
 		writel(sec, info->rtc_base + TEGRA_RTC_REG_SECONDS);
 
 	dev_vdbg(dev, "time read back as %d\n",
-		readl(info->rtc_base + TEGRA_RTC_REG_SECONDS));
+		 readl(info->rtc_base + TEGRA_RTC_REG_SECONDS));
 
 	return ret;
 }
@@ -143,15 +149,15 @@ static int tegra_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 {
 	struct tegra_rtc_info *info = dev_get_drvdata(dev);
 	unsigned long sec;
-	unsigned tmp;
+	unsigned int tmp;
 
 	sec = readl(info->rtc_base + TEGRA_RTC_REG_SECONDS_ALARM0);
 
 	if (sec == 0) {
-		/* alarm is disabled. */
+		/* alarm is disabled */
 		alarm->enabled = 0;
 	} else {
-		/* alarm is enabled. */
+		/* alarm is enabled */
 		alarm->enabled = 1;
 		rtc_time64_to_tm(sec, &alarm->time);
 	}
@@ -165,13 +171,13 @@ static int tegra_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 static int tegra_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
 {
 	struct tegra_rtc_info *info = dev_get_drvdata(dev);
-	unsigned status;
 	unsigned long sl_irq_flags;
+	unsigned int status;
 
 	tegra_rtc_wait_while_busy(dev);
 	spin_lock_irqsave(&info->tegra_rtc_lock, sl_irq_flags);
 
-	/* read the original value, and OR in the flag. */
+	/* read the original value, and OR in the flag */
 	status = readl(info->rtc_base + TEGRA_RTC_REG_INTR_MASK);
 	if (enabled)
 		status |= TEGRA_RTC_INTR_MASK_SEC_ALARM0; /* set it */
@@ -198,14 +204,14 @@ static int tegra_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 	tegra_rtc_wait_while_busy(dev);
 	writel(sec, info->rtc_base + TEGRA_RTC_REG_SECONDS_ALARM0);
 	dev_vdbg(dev, "alarm read back as %d\n",
-		readl(info->rtc_base + TEGRA_RTC_REG_SECONDS_ALARM0));
+		 readl(info->rtc_base + TEGRA_RTC_REG_SECONDS_ALARM0));
 
 	/* if successfully written and alarm is enabled ... */
 	if (sec) {
 		tegra_rtc_alarm_irq_enable(dev, 1);
-		dev_vdbg(dev, "alarm set as %lu. %ptR\n", sec, &alarm->time);
+		dev_vdbg(dev, "alarm set as %u, %ptR\n", sec, &alarm->time);
 	} else {
-		/* disable alarm if 0 or write error. */
+		/* disable alarm if 0 or write error */
 		dev_vdbg(dev, "alarm disabled\n");
 		tegra_rtc_alarm_irq_enable(dev, 0);
 	}
@@ -228,25 +234,26 @@ static irqreturn_t tegra_rtc_irq_handler(int irq, void *data)
 	struct device *dev = data;
 	struct tegra_rtc_info *info = dev_get_drvdata(dev);
 	unsigned long events = 0;
-	unsigned status;
 	unsigned long sl_irq_flags;
+	unsigned int status;
 
 	status = readl(info->rtc_base + TEGRA_RTC_REG_INTR_STATUS);
 	if (status) {
-		/* clear the interrupt masks and status on any irq. */
+		/* clear the interrupt masks and status on any IRQ */
 		tegra_rtc_wait_while_busy(dev);
+
 		spin_lock_irqsave(&info->tegra_rtc_lock, sl_irq_flags);
 		writel(0, info->rtc_base + TEGRA_RTC_REG_INTR_MASK);
 		writel(status, info->rtc_base + TEGRA_RTC_REG_INTR_STATUS);
 		spin_unlock_irqrestore(&info->tegra_rtc_lock, sl_irq_flags);
 	}
 
-	/* check if Alarm */
-	if ((status & TEGRA_RTC_INTR_STATUS_SEC_ALARM0))
+	/* check if alarm */
+	if (status & TEGRA_RTC_INTR_STATUS_SEC_ALARM0)
 		events |= RTC_IRQF | RTC_AF;
 
-	/* check if Periodic */
-	if ((status & TEGRA_RTC_INTR_STATUS_SEC_CDN_ALARM))
+	/* check if periodic */
+	if (status & TEGRA_RTC_INTR_STATUS_SEC_CDN_ALARM)
 		events |= RTC_IRQF | RTC_PF;
 
 	rtc_update_irq(info->rtc_dev, 1, events);
@@ -255,11 +262,11 @@ static irqreturn_t tegra_rtc_irq_handler(int irq, void *data)
 }
 
 static const struct rtc_class_ops tegra_rtc_ops = {
-	.read_time	= tegra_rtc_read_time,
-	.set_time	= tegra_rtc_set_time,
-	.read_alarm	= tegra_rtc_read_alarm,
-	.set_alarm	= tegra_rtc_set_alarm,
-	.proc		= tegra_rtc_proc,
+	.read_time = tegra_rtc_read_time,
+	.set_time = tegra_rtc_set_time,
+	.read_alarm = tegra_rtc_read_alarm,
+	.set_alarm = tegra_rtc_set_alarm,
+	.proc = tegra_rtc_proc,
 	.alarm_irq_enable = tegra_rtc_alarm_irq_enable,
 };
 
@@ -275,8 +282,7 @@ static int __init tegra_rtc_probe(struct platform_device *pdev)
 	struct resource *res;
 	int ret;
 
-	info = devm_kzalloc(&pdev->dev, sizeof(struct tegra_rtc_info),
-		GFP_KERNEL);
+	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
 	if (!info)
 		return -ENOMEM;
 
@@ -308,13 +314,13 @@ static int __init tegra_rtc_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
-	/* set context info. */
+	/* set context info */
 	info->pdev = pdev;
 	spin_lock_init(&info->tegra_rtc_lock);
 
 	platform_set_drvdata(pdev, info);
 
-	/* clear out the hardware. */
+	/* clear out the hardware */
 	writel(0, info->rtc_base + TEGRA_RTC_REG_SECONDS_ALARM0);
 	writel(0xffffffff, info->rtc_base + TEGRA_RTC_REG_INTR_STATUS);
 	writel(0, info->rtc_base + TEGRA_RTC_REG_INTR_MASK);
@@ -322,19 +328,16 @@ static int __init tegra_rtc_probe(struct platform_device *pdev)
 	device_init_wakeup(&pdev->dev, 1);
 
 	ret = devm_request_irq(&pdev->dev, info->tegra_rtc_irq,
-			tegra_rtc_irq_handler, IRQF_TRIGGER_HIGH,
-			dev_name(&pdev->dev), &pdev->dev);
+			       tegra_rtc_irq_handler, IRQF_TRIGGER_HIGH,
+			       dev_name(&pdev->dev), &pdev->dev);
 	if (ret) {
-		dev_err(&pdev->dev,
-			"Unable to request interrupt for device (err=%d).\n",
-			ret);
+		dev_err(&pdev->dev, "failed to request interrupt: %d\n", ret);
 		goto disable_clk;
 	}
 
 	ret = rtc_register_device(info->rtc_dev);
 	if (ret) {
-		dev_err(&pdev->dev, "Unable to register device (err=%d).\n",
-			ret);
+		dev_err(&pdev->dev, "failed to register device: %d\n", ret);
 		goto disable_clk;
 	}
 
@@ -363,18 +366,18 @@ static int tegra_rtc_suspend(struct device *dev)
 
 	tegra_rtc_wait_while_busy(dev);
 
-	/* only use ALARM0 as a wake source. */
+	/* only use ALARM0 as a wake source */
 	writel(0xffffffff, info->rtc_base + TEGRA_RTC_REG_INTR_STATUS);
 	writel(TEGRA_RTC_INTR_STATUS_SEC_ALARM0,
 		info->rtc_base + TEGRA_RTC_REG_INTR_MASK);
 
 	dev_vdbg(dev, "alarm sec = %d\n",
-		readl(info->rtc_base + TEGRA_RTC_REG_SECONDS_ALARM0));
+		 readl(info->rtc_base + TEGRA_RTC_REG_SECONDS_ALARM0));
 
-	dev_vdbg(dev, "Suspend (device_may_wakeup=%d) irq:%d\n",
-		device_may_wakeup(dev), info->tegra_rtc_irq);
+	dev_vdbg(dev, "Suspend (device_may_wakeup=%d) IRQ:%d\n",
+		 device_may_wakeup(dev), info->tegra_rtc_irq);
 
-	/* leave the alarms on as a wake source. */
+	/* leave the alarms on as a wake source */
 	if (device_may_wakeup(dev))
 		enable_irq_wake(info->tegra_rtc_irq);
 
@@ -386,8 +389,9 @@ static int tegra_rtc_resume(struct device *dev)
 	struct tegra_rtc_info *info = dev_get_drvdata(dev);
 
 	dev_vdbg(dev, "Resume (device_may_wakeup=%d)\n",
-		device_may_wakeup(dev));
-	/* alarms were left on as a wake source, turn them off. */
+		 device_may_wakeup(dev));
+
+	/* alarms were left on as a wake source, turn them off */
 	if (device_may_wakeup(dev))
 		disable_irq_wake(info->tegra_rtc_irq);
 
@@ -399,18 +403,17 @@ static SIMPLE_DEV_PM_OPS(tegra_rtc_pm_ops, tegra_rtc_suspend, tegra_rtc_resume);
 
 static void tegra_rtc_shutdown(struct platform_device *pdev)
 {
-	dev_vdbg(&pdev->dev, "disabling interrupts.\n");
+	dev_vdbg(&pdev->dev, "disabling interrupts\n");
 	tegra_rtc_alarm_irq_enable(&pdev->dev, 0);
 }
 
-MODULE_ALIAS("platform:tegra_rtc");
 static struct platform_driver tegra_rtc_driver = {
-	.remove		= tegra_rtc_remove,
-	.shutdown	= tegra_rtc_shutdown,
-	.driver		= {
-		.name	= "tegra_rtc",
+	.remove = tegra_rtc_remove,
+	.shutdown = tegra_rtc_shutdown,
+	.driver = {
+		.name = "tegra_rtc",
 		.of_match_table = tegra_rtc_dt_match,
-		.pm	= &tegra_rtc_pm_ops,
+		.pm = &tegra_rtc_pm_ops,
 	},
 };
 
@@ -418,4 +421,5 @@ module_platform_driver_probe(tegra_rtc_driver, tegra_rtc_probe);
 
 MODULE_AUTHOR("Jon Mayo <jmayo@nvidia.com>");
 MODULE_DESCRIPTION("driver for Tegra internal RTC");
+MODULE_ALIAS("platform:tegra_rtc");
 MODULE_LICENSE("GPL");
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] rtc: tegra: Use consistent variable names and types
  2019-05-27 10:13 [PATCH 0/3] rtc: tegra: Dust off and deferred probe support Thierry Reding
  2019-05-27 10:13 ` [PATCH 1/3] rtc: tegra: checkpatch and miscellaneous cleanups Thierry Reding
@ 2019-05-27 10:13 ` Thierry Reding
  2019-05-27 10:13 ` [PATCH 3/3] rtc: tegra: Turn into regular driver Thierry Reding
  2019-06-01 20:39 ` [PATCH 0/3] rtc: tegra: Dust off and deferred probe support Alexandre Belloni
  3 siblings, 0 replies; 6+ messages in thread
From: Thierry Reding @ 2019-05-27 10:13 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: Jon Hunter, Kartik Kartik, linux-rtc, linux-tegra, linux-kernel

From: Thierry Reding <treding@nvidia.com>

Many of the variables have redundant prefixes or suffixes. Drop all of
them where not necessary for context. Also make sure to use data types
consistently. For instance, values read from 32-bit register accessors
should be stored in u32.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/rtc/rtc-tegra.c | 118 ++++++++++++++++++++--------------------
 1 file changed, 58 insertions(+), 60 deletions(-)

diff --git a/drivers/rtc/rtc-tegra.c b/drivers/rtc/rtc-tegra.c
index a67c9d8be4f6..b68ba2dd1d36 100644
--- a/drivers/rtc/rtc-tegra.c
+++ b/drivers/rtc/rtc-tegra.c
@@ -47,11 +47,11 @@
 
 struct tegra_rtc_info {
 	struct platform_device *pdev;
-	struct rtc_device *rtc_dev;
-	void __iomem *rtc_base; /* NULL if not initialized */
+	struct rtc_device *rtc;
+	void __iomem *base; /* NULL if not initialized */
 	struct clk *clk;
-	int tegra_rtc_irq; /* alarm and periodic IRQ */
-	spinlock_t tegra_rtc_lock;
+	int irq; /* alarm and periodic IRQ */
+	spinlock_t lock;
 };
 
 /*
@@ -61,7 +61,7 @@ struct tegra_rtc_info {
  */
 static inline u32 tegra_rtc_check_busy(struct tegra_rtc_info *info)
 {
-	return readl(info->rtc_base + TEGRA_RTC_REG_BUSY) & 1;
+	return readl(info->base + TEGRA_RTC_REG_BUSY) & 1;
 }
 
 /*
@@ -102,19 +102,19 @@ static int tegra_rtc_wait_while_busy(struct device *dev)
 static int tegra_rtc_read_time(struct device *dev, struct rtc_time *tm)
 {
 	struct tegra_rtc_info *info = dev_get_drvdata(dev);
-	unsigned long sec, msec;
-	unsigned long sl_irq_flags;
+	unsigned long flags;
+	u32 sec, msec;
 
 	/*
 	 * RTC hardware copies seconds to shadow seconds when a read of
 	 * milliseconds occurs. use a lock to keep other threads out.
 	 */
-	spin_lock_irqsave(&info->tegra_rtc_lock, sl_irq_flags);
+	spin_lock_irqsave(&info->lock, flags);
 
-	msec = readl(info->rtc_base + TEGRA_RTC_REG_MILLI_SECONDS);
-	sec = readl(info->rtc_base + TEGRA_RTC_REG_SHADOW_SECONDS);
+	msec = readl(info->base + TEGRA_RTC_REG_MILLI_SECONDS);
+	sec = readl(info->base + TEGRA_RTC_REG_SHADOW_SECONDS);
 
-	spin_unlock_irqrestore(&info->tegra_rtc_lock, sl_irq_flags);
+	spin_unlock_irqrestore(&info->lock, flags);
 
 	rtc_time64_to_tm(sec, tm);
 
@@ -126,7 +126,7 @@ static int tegra_rtc_read_time(struct device *dev, struct rtc_time *tm)
 static int tegra_rtc_set_time(struct device *dev, struct rtc_time *tm)
 {
 	struct tegra_rtc_info *info = dev_get_drvdata(dev);
-	unsigned long sec;
+	u32 sec;
 	int ret;
 
 	/* convert tm to seconds */
@@ -137,10 +137,10 @@ static int tegra_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	/* seconds only written if wait succeeded */
 	ret = tegra_rtc_wait_while_busy(dev);
 	if (!ret)
-		writel(sec, info->rtc_base + TEGRA_RTC_REG_SECONDS);
+		writel(sec, info->base + TEGRA_RTC_REG_SECONDS);
 
 	dev_vdbg(dev, "time read back as %d\n",
-		 readl(info->rtc_base + TEGRA_RTC_REG_SECONDS));
+		 readl(info->base + TEGRA_RTC_REG_SECONDS));
 
 	return ret;
 }
@@ -148,10 +148,9 @@ static int tegra_rtc_set_time(struct device *dev, struct rtc_time *tm)
 static int tegra_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 {
 	struct tegra_rtc_info *info = dev_get_drvdata(dev);
-	unsigned long sec;
-	unsigned int tmp;
+	u32 sec, value;
 
-	sec = readl(info->rtc_base + TEGRA_RTC_REG_SECONDS_ALARM0);
+	sec = readl(info->base + TEGRA_RTC_REG_SECONDS_ALARM0);
 
 	if (sec == 0) {
 		/* alarm is disabled */
@@ -162,8 +161,8 @@ static int tegra_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 		rtc_time64_to_tm(sec, &alarm->time);
 	}
 
-	tmp = readl(info->rtc_base + TEGRA_RTC_REG_INTR_STATUS);
-	alarm->pending = (tmp & TEGRA_RTC_INTR_STATUS_SEC_ALARM0) != 0;
+	value = readl(info->base + TEGRA_RTC_REG_INTR_STATUS);
+	alarm->pending = (value & TEGRA_RTC_INTR_STATUS_SEC_ALARM0) != 0;
 
 	return 0;
 }
@@ -171,22 +170,22 @@ static int tegra_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 static int tegra_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
 {
 	struct tegra_rtc_info *info = dev_get_drvdata(dev);
-	unsigned long sl_irq_flags;
-	unsigned int status;
+	unsigned long flags;
+	u32 status;
 
 	tegra_rtc_wait_while_busy(dev);
-	spin_lock_irqsave(&info->tegra_rtc_lock, sl_irq_flags);
+	spin_lock_irqsave(&info->lock, flags);
 
 	/* read the original value, and OR in the flag */
-	status = readl(info->rtc_base + TEGRA_RTC_REG_INTR_MASK);
+	status = readl(info->base + TEGRA_RTC_REG_INTR_MASK);
 	if (enabled)
 		status |= TEGRA_RTC_INTR_MASK_SEC_ALARM0; /* set it */
 	else
 		status &= ~TEGRA_RTC_INTR_MASK_SEC_ALARM0; /* clear it */
 
-	writel(status, info->rtc_base + TEGRA_RTC_REG_INTR_MASK);
+	writel(status, info->base + TEGRA_RTC_REG_INTR_MASK);
 
-	spin_unlock_irqrestore(&info->tegra_rtc_lock, sl_irq_flags);
+	spin_unlock_irqrestore(&info->lock, flags);
 
 	return 0;
 }
@@ -194,7 +193,7 @@ static int tegra_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
 static int tegra_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 {
 	struct tegra_rtc_info *info = dev_get_drvdata(dev);
-	unsigned long sec;
+	u32 sec;
 
 	if (alarm->enabled)
 		sec = rtc_tm_to_time64(&alarm->time);
@@ -202,9 +201,9 @@ static int tegra_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 		sec = 0;
 
 	tegra_rtc_wait_while_busy(dev);
-	writel(sec, info->rtc_base + TEGRA_RTC_REG_SECONDS_ALARM0);
+	writel(sec, info->base + TEGRA_RTC_REG_SECONDS_ALARM0);
 	dev_vdbg(dev, "alarm read back as %d\n",
-		 readl(info->rtc_base + TEGRA_RTC_REG_SECONDS_ALARM0));
+		 readl(info->base + TEGRA_RTC_REG_SECONDS_ALARM0));
 
 	/* if successfully written and alarm is enabled ... */
 	if (sec) {
@@ -233,19 +232,18 @@ static irqreturn_t tegra_rtc_irq_handler(int irq, void *data)
 {
 	struct device *dev = data;
 	struct tegra_rtc_info *info = dev_get_drvdata(dev);
-	unsigned long events = 0;
-	unsigned long sl_irq_flags;
-	unsigned int status;
+	unsigned long events = 0, flags;
+	u32 status;
 
-	status = readl(info->rtc_base + TEGRA_RTC_REG_INTR_STATUS);
+	status = readl(info->base + TEGRA_RTC_REG_INTR_STATUS);
 	if (status) {
 		/* clear the interrupt masks and status on any IRQ */
 		tegra_rtc_wait_while_busy(dev);
 
-		spin_lock_irqsave(&info->tegra_rtc_lock, sl_irq_flags);
-		writel(0, info->rtc_base + TEGRA_RTC_REG_INTR_MASK);
-		writel(status, info->rtc_base + TEGRA_RTC_REG_INTR_STATUS);
-		spin_unlock_irqrestore(&info->tegra_rtc_lock, sl_irq_flags);
+		spin_lock_irqsave(&info->lock, flags);
+		writel(0, info->base + TEGRA_RTC_REG_INTR_MASK);
+		writel(status, info->base + TEGRA_RTC_REG_INTR_STATUS);
+		spin_unlock_irqrestore(&info->lock, flags);
 	}
 
 	/* check if alarm */
@@ -256,7 +254,7 @@ static irqreturn_t tegra_rtc_irq_handler(int irq, void *data)
 	if (status & TEGRA_RTC_INTR_STATUS_SEC_CDN_ALARM)
 		events |= RTC_IRQF | RTC_PF;
 
-	rtc_update_irq(info->rtc_dev, 1, events);
+	rtc_update_irq(info->rtc, 1, events);
 
 	return IRQ_HANDLED;
 }
@@ -287,9 +285,9 @@ static int __init tegra_rtc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	info->rtc_base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(info->rtc_base))
-		return PTR_ERR(info->rtc_base);
+	info->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(info->base))
+		return PTR_ERR(info->base);
 
 	ret = platform_get_irq(pdev, 0);
 	if (ret <= 0) {
@@ -297,14 +295,14 @@ static int __init tegra_rtc_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	info->tegra_rtc_irq = ret;
+	info->irq = ret;
 
-	info->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
-	if (IS_ERR(info->rtc_dev))
-		return PTR_ERR(info->rtc_dev);
+	info->rtc = devm_rtc_allocate_device(&pdev->dev);
+	if (IS_ERR(info->rtc))
+		return PTR_ERR(info->rtc);
 
-	info->rtc_dev->ops = &tegra_rtc_ops;
-	info->rtc_dev->range_max = U32_MAX;
+	info->rtc->ops = &tegra_rtc_ops;
+	info->rtc->range_max = U32_MAX;
 
 	info->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(info->clk))
@@ -316,26 +314,26 @@ static int __init tegra_rtc_probe(struct platform_device *pdev)
 
 	/* set context info */
 	info->pdev = pdev;
-	spin_lock_init(&info->tegra_rtc_lock);
+	spin_lock_init(&info->lock);
 
 	platform_set_drvdata(pdev, info);
 
 	/* clear out the hardware */
-	writel(0, info->rtc_base + TEGRA_RTC_REG_SECONDS_ALARM0);
-	writel(0xffffffff, info->rtc_base + TEGRA_RTC_REG_INTR_STATUS);
-	writel(0, info->rtc_base + TEGRA_RTC_REG_INTR_MASK);
+	writel(0, info->base + TEGRA_RTC_REG_SECONDS_ALARM0);
+	writel(0xffffffff, info->base + TEGRA_RTC_REG_INTR_STATUS);
+	writel(0, info->base + TEGRA_RTC_REG_INTR_MASK);
 
 	device_init_wakeup(&pdev->dev, 1);
 
-	ret = devm_request_irq(&pdev->dev, info->tegra_rtc_irq,
-			       tegra_rtc_irq_handler, IRQF_TRIGGER_HIGH,
-			       dev_name(&pdev->dev), &pdev->dev);
+	ret = devm_request_irq(&pdev->dev, info->irq, tegra_rtc_irq_handler,
+			       IRQF_TRIGGER_HIGH, dev_name(&pdev->dev),
+			       &pdev->dev);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to request interrupt: %d\n", ret);
 		goto disable_clk;
 	}
 
-	ret = rtc_register_device(info->rtc_dev);
+	ret = rtc_register_device(info->rtc);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to register device: %d\n", ret);
 		goto disable_clk;
@@ -367,19 +365,19 @@ static int tegra_rtc_suspend(struct device *dev)
 	tegra_rtc_wait_while_busy(dev);
 
 	/* only use ALARM0 as a wake source */
-	writel(0xffffffff, info->rtc_base + TEGRA_RTC_REG_INTR_STATUS);
+	writel(0xffffffff, info->base + TEGRA_RTC_REG_INTR_STATUS);
 	writel(TEGRA_RTC_INTR_STATUS_SEC_ALARM0,
-		info->rtc_base + TEGRA_RTC_REG_INTR_MASK);
+	       info->base + TEGRA_RTC_REG_INTR_MASK);
 
 	dev_vdbg(dev, "alarm sec = %d\n",
-		 readl(info->rtc_base + TEGRA_RTC_REG_SECONDS_ALARM0));
+		 readl(info->base + TEGRA_RTC_REG_SECONDS_ALARM0));
 
 	dev_vdbg(dev, "Suspend (device_may_wakeup=%d) IRQ:%d\n",
-		 device_may_wakeup(dev), info->tegra_rtc_irq);
+		 device_may_wakeup(dev), info->irq);
 
 	/* leave the alarms on as a wake source */
 	if (device_may_wakeup(dev))
-		enable_irq_wake(info->tegra_rtc_irq);
+		enable_irq_wake(info->irq);
 
 	return 0;
 }
@@ -393,7 +391,7 @@ static int tegra_rtc_resume(struct device *dev)
 
 	/* alarms were left on as a wake source, turn them off */
 	if (device_may_wakeup(dev))
-		disable_irq_wake(info->tegra_rtc_irq);
+		disable_irq_wake(info->irq);
 
 	return 0;
 }
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] rtc: tegra: Turn into regular driver
  2019-05-27 10:13 [PATCH 0/3] rtc: tegra: Dust off and deferred probe support Thierry Reding
  2019-05-27 10:13 ` [PATCH 1/3] rtc: tegra: checkpatch and miscellaneous cleanups Thierry Reding
  2019-05-27 10:13 ` [PATCH 2/3] rtc: tegra: Use consistent variable names and types Thierry Reding
@ 2019-05-27 10:13 ` Thierry Reding
  2019-06-01 20:39 ` [PATCH 0/3] rtc: tegra: Dust off and deferred probe support Alexandre Belloni
  3 siblings, 0 replies; 6+ messages in thread
From: Thierry Reding @ 2019-05-27 10:13 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: Jon Hunter, Kartik Kartik, linux-rtc, linux-tegra, linux-kernel

From: Thierry Reding <treding@nvidia.com>

Drivers registered with module_platform_driver_probe() are considered
non-hotpluggable, which among other things means that they don't support
deferred probe. However, recent changes in how the ARM SMMU works have
required the BPMP (which is the clock provider on Tegra186 and later) be
bound to the SMMU, which in turn means that the BPMP driver can defer
probe and hence clocks become available much later than they used to.
For most other drivers this is not a problem because they already
properly support deferred probe, but rtc-tegra is the odd one out that
now fails to probe and will therefore never be registered.

Fix this by making the driver a regular driver that supports unloading
and deferred probe.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/rtc/rtc-tegra.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/rtc/rtc-tegra.c b/drivers/rtc/rtc-tegra.c
index b68ba2dd1d36..8bbaea24926e 100644
--- a/drivers/rtc/rtc-tegra.c
+++ b/drivers/rtc/rtc-tegra.c
@@ -2,7 +2,7 @@
 /*
  * An RTC driver for the NVIDIA Tegra 200 series internal RTC.
  *
- * Copyright (c) 2010, NVIDIA Corporation.
+ * Copyright (c) 2010-2019, NVIDIA Corporation.
  */
 
 #include <linux/clk.h>
@@ -274,7 +274,7 @@ static const struct of_device_id tegra_rtc_dt_match[] = {
 };
 MODULE_DEVICE_TABLE(of, tegra_rtc_dt_match);
 
-static int __init tegra_rtc_probe(struct platform_device *pdev)
+static int tegra_rtc_probe(struct platform_device *pdev)
 {
 	struct tegra_rtc_info *info;
 	struct resource *res;
@@ -406,6 +406,7 @@ static void tegra_rtc_shutdown(struct platform_device *pdev)
 }
 
 static struct platform_driver tegra_rtc_driver = {
+	.probe = tegra_rtc_probe,
 	.remove = tegra_rtc_remove,
 	.shutdown = tegra_rtc_shutdown,
 	.driver = {
@@ -414,8 +415,7 @@ static struct platform_driver tegra_rtc_driver = {
 		.pm = &tegra_rtc_pm_ops,
 	},
 };
-
-module_platform_driver_probe(tegra_rtc_driver, tegra_rtc_probe);
+module_platform_driver(tegra_rtc_driver);
 
 MODULE_AUTHOR("Jon Mayo <jmayo@nvidia.com>");
 MODULE_DESCRIPTION("driver for Tegra internal RTC");
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] rtc: tegra: checkpatch and miscellaneous cleanups
  2019-05-27 10:13 ` [PATCH 1/3] rtc: tegra: checkpatch and miscellaneous cleanups Thierry Reding
@ 2019-05-27 11:28   ` Dmitry Osipenko
  0 siblings, 0 replies; 6+ messages in thread
From: Dmitry Osipenko @ 2019-05-27 11:28 UTC (permalink / raw)
  To: Thierry Reding, Alessandro Zummo, Alexandre Belloni
  Cc: Jon Hunter, Kartik Kartik, linux-rtc, linux-tegra, linux-kernel

27.05.2019 13:13, Thierry Reding пишет:
> From: Thierry Reding <treding@nvidia.com>
> 
> This set of changes fixes some checkpatch warnings as well as a number
> of punctuation and padding inconsistencies.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/rtc/rtc-tegra.c | 154 +++++++++++++++++++++-------------------
>  1 file changed, 79 insertions(+), 75 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-tegra.c b/drivers/rtc/rtc-tegra.c
> index f0ce76865434..a67c9d8be4f6 100644
> --- a/drivers/rtc/rtc-tegra.c
> +++ b/drivers/rtc/rtc-tegra.c
> @@ -18,10 +18,10 @@
>  #include <linux/rtc.h>
>  #include <linux/slab.h>
>  
> -/* set to 1 = busy every eight 32kHz clocks during copy of sec+msec to AHB */
> +/* Set to 1 = busy every eight 32 kHz clocks during copy of sec+msec to AHB. */
>  #define TEGRA_RTC_REG_BUSY			0x004
>  #define TEGRA_RTC_REG_SECONDS			0x008
> -/* when msec is read, the seconds are buffered into shadow seconds. */
> +/* When msec is read, the seconds are buffered into shadow seconds. */
>  #define TEGRA_RTC_REG_SHADOW_SECONDS		0x00c
>  #define TEGRA_RTC_REG_MILLI_SECONDS		0x010
>  #define TEGRA_RTC_REG_SECONDS_ALARM0		0x014
> @@ -46,44 +46,48 @@
>  #define TEGRA_RTC_INTR_STATUS_SEC_ALARM0	(1<<0)
>  
>  struct tegra_rtc_info {
> -	struct platform_device	*pdev;
> -	struct rtc_device	*rtc_dev;
> -	void __iomem		*rtc_base; /* NULL if not initialized. */
> -	struct clk		*clk;
> -	int			tegra_rtc_irq; /* alarm and periodic irq */
> -	spinlock_t		tegra_rtc_lock;
> +	struct platform_device *pdev;
> +	struct rtc_device *rtc_dev;
> +	void __iomem *rtc_base; /* NULL if not initialized */
> +	struct clk *clk;
> +	int tegra_rtc_irq; /* alarm and periodic IRQ */
> +	spinlock_t tegra_rtc_lock;
>  };
>  
> -/* RTC hardware is busy when it is updating its values over AHB once
> - * every eight 32kHz clocks (~250uS).
> - * outside of these updates the CPU is free to write.
> - * CPU is always free to read.
> +/*
> + * RTC hardware is busy when it is updating its values over AHB once every
> + * eight 32 kHz clocks (~250 us). Outside of these updates the CPU is free to
> + * write. CPU is always free to read.
>   */
>  static inline u32 tegra_rtc_check_busy(struct tegra_rtc_info *info)
>  {
>  	return readl(info->rtc_base + TEGRA_RTC_REG_BUSY) & 1;
>  }
>  
> -/* Wait for hardware to be ready for writing.
> - * This function tries to maximize the amount of time before the next update.
> - * It does this by waiting for the RTC to become busy with its periodic update,
> - * then returning once the RTC first becomes not busy.
> +/*
> + * Wait for hardware to be ready for writing. This function tries to maximize
> + * the amount of time before the next update. It does this by waiting for the
> + * RTC to become busy with its periodic update, then returning once the RTC
> + * first becomes not busy.
> + *
>   * This periodic update (where the seconds and milliseconds are copied to the
> - * AHB side) occurs every eight 32kHz clocks (~250uS).
> - * The behavior of this function allows us to make some assumptions without
> - * introducing a race, because 250uS is plenty of time to read/write a value.
> + * AHB side) occurs every eight 32 kHz clocks (~250 us). The behavior of this
> + * function allows us to make some assumptions without introducing a race,
> + * because 250 us is plenty of time to read/write a value.
>   */
>  static int tegra_rtc_wait_while_busy(struct device *dev)
>  {
>  	struct tegra_rtc_info *info = dev_get_drvdata(dev);
> +	int retries = 500; /* ~490 us is the worst case, ~250 us is best */
>  
> -	int retries = 500; /* ~490 us is the worst case, ~250 us is best. */
> -
> -	/* first wait for the RTC to become busy. this is when it
> -	 * posts its updated seconds+msec registers to AHB side. */
> +	/*
> +	 * First wait for the RTC to become busy. This is when it posts its
> +	 * updated seconds+msec registers to AHB side.
> +	 */
>  	while (tegra_rtc_check_busy(info)) {
>  		if (!retries--)
>  			goto retry_failed;
> +
>  		udelay(1);
>  	}
>  
> @@ -91,7 +95,7 @@ static int tegra_rtc_wait_while_busy(struct device *dev)
>  	return 0;
>  
>  retry_failed:
> -	dev_err(dev, "write failed:retry count exceeded.\n");
> +	dev_err(dev, "write failed: retry count exceeded\n");
>  	return -ETIMEDOUT;
>  }
>  
> @@ -101,8 +105,10 @@ static int tegra_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  	unsigned long sec, msec;
>  	unsigned long sl_irq_flags;
>  
> -	/* RTC hardware copies seconds to shadow seconds when a read
> -	 * of milliseconds occurs. use a lock to keep other threads out. */
> +	/*
> +	 * RTC hardware copies seconds to shadow seconds when a read of
> +	 * milliseconds occurs. use a lock to keep other threads out.
> +	 */
>  	spin_lock_irqsave(&info->tegra_rtc_lock, sl_irq_flags);
>  
>  	msec = readl(info->rtc_base + TEGRA_RTC_REG_MILLI_SECONDS);
> @@ -112,7 +118,7 @@ static int tegra_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  
>  	rtc_time64_to_tm(sec, tm);
>  
> -	dev_vdbg(dev, "time read as %lu. %ptR\n", sec, tm);
> +	dev_vdbg(dev, "time read as %u, %ptR\n", sec, tm);
>  
>  	return 0;
>  }
> @@ -123,18 +129,18 @@ static int tegra_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  	unsigned long sec;
>  	int ret;
>  
> -	/* convert tm to seconds. */
> +	/* convert tm to seconds */
>  	sec = rtc_tm_to_time64(tm);
>  
> -	dev_vdbg(dev, "time set to %lu. %ptR\n", sec, tm);
> +	dev_vdbg(dev, "time set to %u, %ptR\n", sec, tm);
>  
> -	/* seconds only written if wait succeeded. */
> +	/* seconds only written if wait succeeded */
>  	ret = tegra_rtc_wait_while_busy(dev);
>  	if (!ret)
>  		writel(sec, info->rtc_base + TEGRA_RTC_REG_SECONDS);
>  
>  	dev_vdbg(dev, "time read back as %d\n",
> -		readl(info->rtc_base + TEGRA_RTC_REG_SECONDS));
> +		 readl(info->rtc_base + TEGRA_RTC_REG_SECONDS));
>  
>  	return ret;
>  }
> @@ -143,15 +149,15 @@ static int tegra_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
>  {
>  	struct tegra_rtc_info *info = dev_get_drvdata(dev);
>  	unsigned long sec;
> -	unsigned tmp;
> +	unsigned int tmp;
>  
>  	sec = readl(info->rtc_base + TEGRA_RTC_REG_SECONDS_ALARM0);
>  
>  	if (sec == 0) {
> -		/* alarm is disabled. */
> +		/* alarm is disabled */
>  		alarm->enabled = 0;
>  	} else {
> -		/* alarm is enabled. */
> +		/* alarm is enabled */
>  		alarm->enabled = 1;
>  		rtc_time64_to_tm(sec, &alarm->time);
>  	}
> @@ -165,13 +171,13 @@ static int tegra_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
>  static int tegra_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
>  {
>  	struct tegra_rtc_info *info = dev_get_drvdata(dev);
> -	unsigned status;
>  	unsigned long sl_irq_flags;
> +	unsigned int status;
>  
>  	tegra_rtc_wait_while_busy(dev);
>  	spin_lock_irqsave(&info->tegra_rtc_lock, sl_irq_flags);
>  
> -	/* read the original value, and OR in the flag. */
> +	/* read the original value, and OR in the flag */
>  	status = readl(info->rtc_base + TEGRA_RTC_REG_INTR_MASK);
>  	if (enabled)
>  		status |= TEGRA_RTC_INTR_MASK_SEC_ALARM0; /* set it */
> @@ -198,14 +204,14 @@ static int tegra_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
>  	tegra_rtc_wait_while_busy(dev);
>  	writel(sec, info->rtc_base + TEGRA_RTC_REG_SECONDS_ALARM0);
>  	dev_vdbg(dev, "alarm read back as %d\n",
> -		readl(info->rtc_base + TEGRA_RTC_REG_SECONDS_ALARM0));
> +		 readl(info->rtc_base + TEGRA_RTC_REG_SECONDS_ALARM0));
>  
>  	/* if successfully written and alarm is enabled ... */
>  	if (sec) {
>  		tegra_rtc_alarm_irq_enable(dev, 1);
> -		dev_vdbg(dev, "alarm set as %lu. %ptR\n", sec, &alarm->time);
> +		dev_vdbg(dev, "alarm set as %u, %ptR\n", sec, &alarm->time);
>  	} else {
> -		/* disable alarm if 0 or write error. */
> +		/* disable alarm if 0 or write error */
>  		dev_vdbg(dev, "alarm disabled\n");
>  		tegra_rtc_alarm_irq_enable(dev, 0);
>  	}
> @@ -228,25 +234,26 @@ static irqreturn_t tegra_rtc_irq_handler(int irq, void *data)
>  	struct device *dev = data;
>  	struct tegra_rtc_info *info = dev_get_drvdata(dev);
>  	unsigned long events = 0;
> -	unsigned status;
>  	unsigned long sl_irq_flags;
> +	unsigned int status;
>  
>  	status = readl(info->rtc_base + TEGRA_RTC_REG_INTR_STATUS);
>  	if (status) {
> -		/* clear the interrupt masks and status on any irq. */
> +		/* clear the interrupt masks and status on any IRQ */
>  		tegra_rtc_wait_while_busy(dev);
> +
>  		spin_lock_irqsave(&info->tegra_rtc_lock, sl_irq_flags);
>  		writel(0, info->rtc_base + TEGRA_RTC_REG_INTR_MASK);
>  		writel(status, info->rtc_base + TEGRA_RTC_REG_INTR_STATUS);
>  		spin_unlock_irqrestore(&info->tegra_rtc_lock, sl_irq_flags);
>  	}
>  
> -	/* check if Alarm */
> -	if ((status & TEGRA_RTC_INTR_STATUS_SEC_ALARM0))
> +	/* check if alarm */
> +	if (status & TEGRA_RTC_INTR_STATUS_SEC_ALARM0)
>  		events |= RTC_IRQF | RTC_AF;
>  
> -	/* check if Periodic */
> -	if ((status & TEGRA_RTC_INTR_STATUS_SEC_CDN_ALARM))
> +	/* check if periodic */
> +	if (status & TEGRA_RTC_INTR_STATUS_SEC_CDN_ALARM)
>  		events |= RTC_IRQF | RTC_PF;
>  
>  	rtc_update_irq(info->rtc_dev, 1, events);
> @@ -255,11 +262,11 @@ static irqreturn_t tegra_rtc_irq_handler(int irq, void *data)
>  }
>  
>  static const struct rtc_class_ops tegra_rtc_ops = {
> -	.read_time	= tegra_rtc_read_time,
> -	.set_time	= tegra_rtc_set_time,
> -	.read_alarm	= tegra_rtc_read_alarm,
> -	.set_alarm	= tegra_rtc_set_alarm,
> -	.proc		= tegra_rtc_proc,
> +	.read_time = tegra_rtc_read_time,
> +	.set_time = tegra_rtc_set_time,
> +	.read_alarm = tegra_rtc_read_alarm,
> +	.set_alarm = tegra_rtc_set_alarm,
> +	.proc = tegra_rtc_proc,
>  	.alarm_irq_enable = tegra_rtc_alarm_irq_enable,
>  };
>  
> @@ -275,8 +282,7 @@ static int __init tegra_rtc_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	int ret;
>  
> -	info = devm_kzalloc(&pdev->dev, sizeof(struct tegra_rtc_info),
> -		GFP_KERNEL);
> +	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
>  	if (!info)
>  		return -ENOMEM;
>  
> @@ -308,13 +314,13 @@ static int __init tegra_rtc_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		return ret;
>  
> -	/* set context info. */
> +	/* set context info */
>  	info->pdev = pdev;
>  	spin_lock_init(&info->tegra_rtc_lock);
>  
>  	platform_set_drvdata(pdev, info);
>  
> -	/* clear out the hardware. */
> +	/* clear out the hardware */
>  	writel(0, info->rtc_base + TEGRA_RTC_REG_SECONDS_ALARM0);
>  	writel(0xffffffff, info->rtc_base + TEGRA_RTC_REG_INTR_STATUS);
>  	writel(0, info->rtc_base + TEGRA_RTC_REG_INTR_MASK);
> @@ -322,19 +328,16 @@ static int __init tegra_rtc_probe(struct platform_device *pdev)
>  	device_init_wakeup(&pdev->dev, 1);
>  
>  	ret = devm_request_irq(&pdev->dev, info->tegra_rtc_irq,
> -			tegra_rtc_irq_handler, IRQF_TRIGGER_HIGH,
> -			dev_name(&pdev->dev), &pdev->dev);
> +			       tegra_rtc_irq_handler, IRQF_TRIGGER_HIGH,
> +			       dev_name(&pdev->dev), &pdev->dev);
>  	if (ret) {
> -		dev_err(&pdev->dev,
> -			"Unable to request interrupt for device (err=%d).\n",
> -			ret);
> +		dev_err(&pdev->dev, "failed to request interrupt: %d\n", ret);
>  		goto disable_clk;
>  	}
>  
>  	ret = rtc_register_device(info->rtc_dev);
>  	if (ret) {
> -		dev_err(&pdev->dev, "Unable to register device (err=%d).\n",
> -			ret);
> +		dev_err(&pdev->dev, "failed to register device: %d\n", ret);
>  		goto disable_clk;
>  	}
>  
> @@ -363,18 +366,18 @@ static int tegra_rtc_suspend(struct device *dev)
>  
>  	tegra_rtc_wait_while_busy(dev);
>  
> -	/* only use ALARM0 as a wake source. */
> +	/* only use ALARM0 as a wake source */
>  	writel(0xffffffff, info->rtc_base + TEGRA_RTC_REG_INTR_STATUS);
>  	writel(TEGRA_RTC_INTR_STATUS_SEC_ALARM0,
>  		info->rtc_base + TEGRA_RTC_REG_INTR_MASK);
>  
>  	dev_vdbg(dev, "alarm sec = %d\n",
> -		readl(info->rtc_base + TEGRA_RTC_REG_SECONDS_ALARM0));
> +		 readl(info->rtc_base + TEGRA_RTC_REG_SECONDS_ALARM0));
>  
> -	dev_vdbg(dev, "Suspend (device_may_wakeup=%d) irq:%d\n",
> -		device_may_wakeup(dev), info->tegra_rtc_irq);
> +	dev_vdbg(dev, "Suspend (device_may_wakeup=%d) IRQ:%d\n",
> +		 device_may_wakeup(dev), info->tegra_rtc_irq);
>  
> -	/* leave the alarms on as a wake source. */
> +	/* leave the alarms on as a wake source */
>  	if (device_may_wakeup(dev))
>  		enable_irq_wake(info->tegra_rtc_irq);
>  
> @@ -386,8 +389,9 @@ static int tegra_rtc_resume(struct device *dev)
>  	struct tegra_rtc_info *info = dev_get_drvdata(dev);
>  
>  	dev_vdbg(dev, "Resume (device_may_wakeup=%d)\n",
> -		device_may_wakeup(dev));
> -	/* alarms were left on as a wake source, turn them off. */
> +		 device_may_wakeup(dev));
> +
> +	/* alarms were left on as a wake source, turn them off */
>  	if (device_may_wakeup(dev))
>  		disable_irq_wake(info->tegra_rtc_irq);
>  
> @@ -399,18 +403,17 @@ static SIMPLE_DEV_PM_OPS(tegra_rtc_pm_ops, tegra_rtc_suspend, tegra_rtc_resume);
>  
>  static void tegra_rtc_shutdown(struct platform_device *pdev)
>  {
> -	dev_vdbg(&pdev->dev, "disabling interrupts.\n");
> +	dev_vdbg(&pdev->dev, "disabling interrupts\n");
>  	tegra_rtc_alarm_irq_enable(&pdev->dev, 0);
>  }
>  
> -MODULE_ALIAS("platform:tegra_rtc");
>  static struct platform_driver tegra_rtc_driver = {
> -	.remove		= tegra_rtc_remove,
> -	.shutdown	= tegra_rtc_shutdown,
> -	.driver		= {
> -		.name	= "tegra_rtc",
> +	.remove = tegra_rtc_remove,
> +	.shutdown = tegra_rtc_shutdown,
> +	.driver = {
> +		.name = "tegra_rtc",
>  		.of_match_table = tegra_rtc_dt_match,
> -		.pm	= &tegra_rtc_pm_ops,
> +		.pm = &tegra_rtc_pm_ops,
>  	},
>  };
>  
> @@ -418,4 +421,5 @@ module_platform_driver_probe(tegra_rtc_driver, tegra_rtc_probe);
>  
>  MODULE_AUTHOR("Jon Mayo <jmayo@nvidia.com>");
>  MODULE_DESCRIPTION("driver for Tegra internal RTC");
> +MODULE_ALIAS("platform:tegra_rtc");
>  MODULE_LICENSE("GPL");
> 

You could drop MODULE_ALIAS entirely since this is an OF-driver and
hence the modalias is overridden anyway by MODULE_DEVICE_TABLE.

# cat /sys/devices/soc0/7000e000.rtc/modalias
of:NrtcT(null)Cnvidia,tegra20-rtc

-- 
Dmitry

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/3] rtc: tegra: Dust off and deferred probe support
  2019-05-27 10:13 [PATCH 0/3] rtc: tegra: Dust off and deferred probe support Thierry Reding
                   ` (2 preceding siblings ...)
  2019-05-27 10:13 ` [PATCH 3/3] rtc: tegra: Turn into regular driver Thierry Reding
@ 2019-06-01 20:39 ` Alexandre Belloni
  3 siblings, 0 replies; 6+ messages in thread
From: Alexandre Belloni @ 2019-06-01 20:39 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Alessandro Zummo, Jon Hunter, Kartik Kartik, linux-rtc,
	linux-tegra, linux-kernel

On 27/05/2019 12:13:56+0200, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> The NVIDIA Tegra RTC driver has accumulated a bit of dust over the
> years. Make a pass over it, addressing checkpatch warnings and fixing
> some inconsistencies in comments and kernel messages as well as in
> variable types and names.
> 
> Once cleaned up, also turn the driver into a regular driver in order
> to support deferred probe which is needed to avoid a future regression
> on Tegra186 and later.
> 
> Thierry
> 
> Thierry Reding (3):
>   rtc: tegra: checkpatch and miscellaneous cleanups
>   rtc: tegra: Use consistent variable names and types
>   rtc: tegra: Turn into regular driver
> 
>  drivers/rtc/rtc-tegra.c | 254 ++++++++++++++++++++--------------------
>  1 file changed, 128 insertions(+), 126 deletions(-)
> 

All applied, thanks.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-06-01 20:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-27 10:13 [PATCH 0/3] rtc: tegra: Dust off and deferred probe support Thierry Reding
2019-05-27 10:13 ` [PATCH 1/3] rtc: tegra: checkpatch and miscellaneous cleanups Thierry Reding
2019-05-27 11:28   ` Dmitry Osipenko
2019-05-27 10:13 ` [PATCH 2/3] rtc: tegra: Use consistent variable names and types Thierry Reding
2019-05-27 10:13 ` [PATCH 3/3] rtc: tegra: Turn into regular driver Thierry Reding
2019-06-01 20:39 ` [PATCH 0/3] rtc: tegra: Dust off and deferred probe support Alexandre Belloni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).