Re: [PATCH v8 05/26] clocksource: Add driver for the Ingenic JZ47xx OST
Hi, Le lun. 25 févr. 2019 à 15:05, Stephen Boyd a écrit : Quoting Paul Cercueil (2019-02-22 19:17:25) Hi, Anything new on this? It still happens on 5.0-rc7. It probes with late_initcall, and not with device_initcall. I have no clue what's going on. I'm not sure what's going on either. You'll probably have to debug when the device is created and when it is probed by enabling the debug printing in the driver core or by adding in extra debug prints to narrow down the problem. For example, add a '#define DEBUG 1' at the top of drivers/base/dd.c and see if that helps give some info on what's going on with the drivers and devices. The doc of __platform_driver_probe says: "Use this instead of platform_driver_register() when you know the device is not hotpluggable and has already been registered". When the parent device and child device are both probed with builtin_platform_driver_probe(), and the parent calls devm_of_platform_populate(), it is not certain that the parent's probe will happen before the child's, and if it does not, the child device has not been registered and its probe is not allowed to defer. So it turned out not to be a core bug, rather a misuse of the API. So I will keep the builtin_platform_driver_probe() in the child, and use a subsys_initcall() in the parent. That works fine. Regards, -Paul
Re: [PATCH v8 05/26] clocksource: Add driver for the Ingenic JZ47xx OST
Quoting Paul Cercueil (2019-02-22 19:17:25) > Hi, > > Anything new on this? It still happens on 5.0-rc7. > It probes with late_initcall, and not with device_initcall. > I have no clue what's going on. > I'm not sure what's going on either. You'll probably have to debug when the device is created and when it is probed by enabling the debug printing in the driver core or by adding in extra debug prints to narrow down the problem. For example, add a '#define DEBUG 1' at the top of drivers/base/dd.c and see if that helps give some info on what's going on with the drivers and devices.
Re: [PATCH v8 05/26] clocksource: Add driver for the Ingenic JZ47xx OST
Hi, Le jeu. 24 janv. 2019 à 19:53, Paul Cercueil a écrit : Le jeu. 24 janv. 2019 à 19:46, Stephen Boyd a écrit : Quoting Paul Cercueil (2019-01-24 12:46:28) Le jeu. 24 janv. 2019 à 16:28, Stephen Boyd a écrit : > Quoting Guenter Roeck (2019-01-23 10:01:55) >> On Wed, Jan 23, 2019 at 02:25:53PM -0300, Paul Cercueil wrote: >> > Hi, >> > >> > Le mer. 23 janv. 2019 à 11:31, Guenter Roeck >> a écrit : >> > >On 1/23/19 4:58 AM, Mathieu Malaterre wrote: >> > >>On Wed, Dec 12, 2018 at 11:09 PM Paul Cercueil >> >> > >>wrote: >> > >>> >> > >>>From: Maarten ter Huurne >> > >>> >> > >>>OST is the OS Timer, a 64-bit timer/counter with buffered >> reading. >> > >>> >> > >>>SoCs before the JZ4770 had (if any) a 32-bit OST; the JZ4770 >> and >> > >>>JZ4780 have a 64-bit OST. >> > >>> >> > >>>This driver will register both a clocksource and a sched_clock >> to the >> > >>>system. >> > >>> >> > >>>Signed-off-by: Maarten ter Huurne >> > >>>Signed-off-by: Paul Cercueil >> > >>>--- >> > >>> >> > >>>Notes: >> > >>> v5: New patch >> > >>> >> > >>> v6: - Get rid of SoC IDs; pass pointer to >> ingenic_ost_soc_info >> > >>>as >> > >>>devicetree match data instead. >> > >>> - Use device_get_match_data() instead of the of_* >> variant >> > >>> - Handle error of dev_get_regmap() properly >> > >>> >> > >>> v7: Fix section mismatch by using >> > >>>builtin_platform_driver_probe() >> > >>> >> > >>> v8: builtin_platform_driver_probe() does not work >> anymore in >> > >>> 4.20-rc6? The probe function won't be called. Work >> around >> > >>>this >> > >>> for now by using late_initcall. >> > >>> >> > > >> > >Did anyone notice this ? Either something is wrong with the >> driver, or >> > >with the kernel core. Hacking around it seems like the worst >> possible >> > >"solution". >> > >> > I can confirm it still happens on 5.0-rc3. >> > >> > Just to explain what I'm doing: >> > >> > My ingenic-timer driver probes with builtin_platform_driver_probe >> (this >> > works), >> > and then calls of_platform_populate to probe its children. This >> driver, >> > ingenic-ost, is one of them, and will fail to probe with >> > builtin_platform_driver_probe. >> > >> >> The big question is _why_ it fails to probe. >> > > Are you sharing the device tree node between a 'normal' platform > device > driver and something more low level DT that marks the device's backing > DT node as OF_POPULATED early on? That's my only guess why it's not > working. I do, but I clear the OF_POPULATED flag so that it is then probed as a normal platform device, and it's not on this driver's node but its parent. Where do you clear the OF_POPULATED flag? In the ingenic-timer driver introduced in patch [04/26], inside the probe function. Anything new on this? It still happens on 5.0-rc7. It probes with late_initcall, and not with device_initcall. I have no clue what's going on. -Paul
Re: [PATCH v8 05/26] clocksource: Add driver for the Ingenic JZ47xx OST
Le jeu. 24 janv. 2019 à 19:46, Stephen Boyd a écrit : Quoting Paul Cercueil (2019-01-24 12:46:28) Le jeu. 24 janv. 2019 à 16:28, Stephen Boyd a écrit : > Quoting Guenter Roeck (2019-01-23 10:01:55) >> On Wed, Jan 23, 2019 at 02:25:53PM -0300, Paul Cercueil wrote: >> > Hi, >> > >> > Le mer. 23 janv. 2019 à 11:31, Guenter Roeck >> a écrit : >> > >On 1/23/19 4:58 AM, Mathieu Malaterre wrote: >> > >>On Wed, Dec 12, 2018 at 11:09 PM Paul Cercueil >> >> > >>wrote: >> > >>> >> > >>>From: Maarten ter Huurne >> > >>> >> > >>>OST is the OS Timer, a 64-bit timer/counter with buffered >> reading. >> > >>> >> > >>>SoCs before the JZ4770 had (if any) a 32-bit OST; the JZ4770 >> and >> > >>>JZ4780 have a 64-bit OST. >> > >>> >> > >>>This driver will register both a clocksource and a sched_clock >> to the >> > >>>system. >> > >>> >> > >>>Signed-off-by: Maarten ter Huurne >> > >>>Signed-off-by: Paul Cercueil >> > >>>--- >> > >>> >> > >>>Notes: >> > >>> v5: New patch >> > >>> >> > >>> v6: - Get rid of SoC IDs; pass pointer to >> ingenic_ost_soc_info >> > >>>as >> > >>>devicetree match data instead. >> > >>> - Use device_get_match_data() instead of the of_* >> variant >> > >>> - Handle error of dev_get_regmap() properly >> > >>> >> > >>> v7: Fix section mismatch by using >> > >>>builtin_platform_driver_probe() >> > >>> >> > >>> v8: builtin_platform_driver_probe() does not work >> anymore in >> > >>> 4.20-rc6? The probe function won't be called. Work >> around >> > >>>this >> > >>> for now by using late_initcall. >> > >>> >> > > >> > >Did anyone notice this ? Either something is wrong with the >> driver, or >> > >with the kernel core. Hacking around it seems like the worst >> possible >> > >"solution". >> > >> > I can confirm it still happens on 5.0-rc3. >> > >> > Just to explain what I'm doing: >> > >> > My ingenic-timer driver probes with builtin_platform_driver_probe >> (this >> > works), >> > and then calls of_platform_populate to probe its children. This >> driver, >> > ingenic-ost, is one of them, and will fail to probe with >> > builtin_platform_driver_probe. >> > >> >> The big question is _why_ it fails to probe. >> > > Are you sharing the device tree node between a 'normal' platform > device > driver and something more low level DT that marks the device's backing > DT node as OF_POPULATED early on? That's my only guess why it's not > working. I do, but I clear the OF_POPULATED flag so that it is then probed as a normal platform device, and it's not on this driver's node but its parent. Where do you clear the OF_POPULATED flag? In the ingenic-timer driver introduced in patch [04/26], inside the probe function.
Re: [PATCH v8 05/26] clocksource: Add driver for the Ingenic JZ47xx OST
Quoting Paul Cercueil (2019-01-24 12:46:28) > > > Le jeu. 24 janv. 2019 à 16:28, Stephen Boyd a > écrit : > > Quoting Guenter Roeck (2019-01-23 10:01:55) > >> On Wed, Jan 23, 2019 at 02:25:53PM -0300, Paul Cercueil wrote: > >> > Hi, > >> > > >> > Le mer. 23 janv. 2019 à 11:31, Guenter Roeck > >> a écrit : > >> > >On 1/23/19 4:58 AM, Mathieu Malaterre wrote: > >> > >>On Wed, Dec 12, 2018 at 11:09 PM Paul Cercueil > >> > >> > >>wrote: > >> > >>> > >> > >>>From: Maarten ter Huurne > >> > >>> > >> > >>>OST is the OS Timer, a 64-bit timer/counter with buffered > >> reading. > >> > >>> > >> > >>>SoCs before the JZ4770 had (if any) a 32-bit OST; the JZ4770 > >> and > >> > >>>JZ4780 have a 64-bit OST. > >> > >>> > >> > >>>This driver will register both a clocksource and a sched_clock > >> to the > >> > >>>system. > >> > >>> > >> > >>>Signed-off-by: Maarten ter Huurne > >> > >>>Signed-off-by: Paul Cercueil > >> > >>>--- > >> > >>> > >> > >>>Notes: > >> > >>> v5: New patch > >> > >>> > >> > >>> v6: - Get rid of SoC IDs; pass pointer to > >> ingenic_ost_soc_info > >> > >>>as > >> > >>>devicetree match data instead. > >> > >>> - Use device_get_match_data() instead of the of_* > >> variant > >> > >>> - Handle error of dev_get_regmap() properly > >> > >>> > >> > >>> v7: Fix section mismatch by using > >> > >>>builtin_platform_driver_probe() > >> > >>> > >> > >>> v8: builtin_platform_driver_probe() does not work > >> anymore in > >> > >>> 4.20-rc6? The probe function won't be called. Work > >> around > >> > >>>this > >> > >>> for now by using late_initcall. > >> > >>> > >> > > > >> > >Did anyone notice this ? Either something is wrong with the > >> driver, or > >> > >with the kernel core. Hacking around it seems like the worst > >> possible > >> > >"solution". > >> > > >> > I can confirm it still happens on 5.0-rc3. > >> > > >> > Just to explain what I'm doing: > >> > > >> > My ingenic-timer driver probes with builtin_platform_driver_probe > >> (this > >> > works), > >> > and then calls of_platform_populate to probe its children. This > >> driver, > >> > ingenic-ost, is one of them, and will fail to probe with > >> > builtin_platform_driver_probe. > >> > > >> > >> The big question is _why_ it fails to probe. > >> > > > > Are you sharing the device tree node between a 'normal' platform > > device > > driver and something more low level DT that marks the device's backing > > DT node as OF_POPULATED early on? That's my only guess why it's not > > working. > > I do, but I clear the OF_POPULATED flag so that it is then probed as a > normal platform device, and it's not on this driver's node but its > parent. > Where do you clear the OF_POPULATED flag?
Re: [PATCH v8 05/26] clocksource: Add driver for the Ingenic JZ47xx OST
Le jeu. 24 janv. 2019 à 16:28, Stephen Boyd a écrit : Quoting Guenter Roeck (2019-01-23 10:01:55) On Wed, Jan 23, 2019 at 02:25:53PM -0300, Paul Cercueil wrote: > Hi, > > Le mer. 23 janv. 2019 à 11:31, Guenter Roeck a écrit : > >On 1/23/19 4:58 AM, Mathieu Malaterre wrote: > >>On Wed, Dec 12, 2018 at 11:09 PM Paul Cercueil > >>wrote: > >>> > >>>From: Maarten ter Huurne > >>> > >>>OST is the OS Timer, a 64-bit timer/counter with buffered reading. > >>> > >>>SoCs before the JZ4770 had (if any) a 32-bit OST; the JZ4770 and > >>>JZ4780 have a 64-bit OST. > >>> > >>>This driver will register both a clocksource and a sched_clock to the > >>>system. > >>> > >>>Signed-off-by: Maarten ter Huurne > >>>Signed-off-by: Paul Cercueil > >>>--- > >>> > >>>Notes: > >>> v5: New patch > >>> > >>> v6: - Get rid of SoC IDs; pass pointer to ingenic_ost_soc_info > >>>as > >>>devicetree match data instead. > >>> - Use device_get_match_data() instead of the of_* variant > >>> - Handle error of dev_get_regmap() properly > >>> > >>> v7: Fix section mismatch by using > >>>builtin_platform_driver_probe() > >>> > >>> v8: builtin_platform_driver_probe() does not work anymore in > >>> 4.20-rc6? The probe function won't be called. Work around > >>>this > >>> for now by using late_initcall. > >>> > > > >Did anyone notice this ? Either something is wrong with the driver, or > >with the kernel core. Hacking around it seems like the worst possible > >"solution". > > I can confirm it still happens on 5.0-rc3. > > Just to explain what I'm doing: > > My ingenic-timer driver probes with builtin_platform_driver_probe (this > works), > and then calls of_platform_populate to probe its children. This driver, > ingenic-ost, is one of them, and will fail to probe with > builtin_platform_driver_probe. > The big question is _why_ it fails to probe. Are you sharing the device tree node between a 'normal' platform device driver and something more low level DT that marks the device's backing DT node as OF_POPULATED early on? That's my only guess why it's not working. I do, but I clear the OF_POPULATED flag so that it is then probed as a normal platform device, and it's not on this driver's node but its parent.
Re: [PATCH v8 05/26] clocksource: Add driver for the Ingenic JZ47xx OST
Quoting Guenter Roeck (2019-01-23 10:01:55) > On Wed, Jan 23, 2019 at 02:25:53PM -0300, Paul Cercueil wrote: > > Hi, > > > > Le mer. 23 janv. 2019 à 11:31, Guenter Roeck a écrit > > : > > >On 1/23/19 4:58 AM, Mathieu Malaterre wrote: > > >>On Wed, Dec 12, 2018 at 11:09 PM Paul Cercueil > > >>wrote: > > >>> > > >>>From: Maarten ter Huurne > > >>> > > >>>OST is the OS Timer, a 64-bit timer/counter with buffered reading. > > >>> > > >>>SoCs before the JZ4770 had (if any) a 32-bit OST; the JZ4770 and > > >>>JZ4780 have a 64-bit OST. > > >>> > > >>>This driver will register both a clocksource and a sched_clock to the > > >>>system. > > >>> > > >>>Signed-off-by: Maarten ter Huurne > > >>>Signed-off-by: Paul Cercueil > > >>>--- > > >>> > > >>>Notes: > > >>> v5: New patch > > >>> > > >>> v6: - Get rid of SoC IDs; pass pointer to ingenic_ost_soc_info > > >>>as > > >>>devicetree match data instead. > > >>> - Use device_get_match_data() instead of the of_* variant > > >>> - Handle error of dev_get_regmap() properly > > >>> > > >>> v7: Fix section mismatch by using > > >>>builtin_platform_driver_probe() > > >>> > > >>> v8: builtin_platform_driver_probe() does not work anymore in > > >>> 4.20-rc6? The probe function won't be called. Work around > > >>>this > > >>> for now by using late_initcall. > > >>> > > > > > >Did anyone notice this ? Either something is wrong with the driver, or > > >with the kernel core. Hacking around it seems like the worst possible > > >"solution". > > > > I can confirm it still happens on 5.0-rc3. > > > > Just to explain what I'm doing: > > > > My ingenic-timer driver probes with builtin_platform_driver_probe (this > > works), > > and then calls of_platform_populate to probe its children. This driver, > > ingenic-ost, is one of them, and will fail to probe with > > builtin_platform_driver_probe. > > > > The big question is _why_ it fails to probe. > Are you sharing the device tree node between a 'normal' platform device driver and something more low level DT that marks the device's backing DT node as OF_POPULATED early on? That's my only guess why it's not working.
Re: [PATCH v8 05/26] clocksource: Add driver for the Ingenic JZ47xx OST
On Wed, Jan 23, 2019 at 02:25:53PM -0300, Paul Cercueil wrote: > Hi, > > Le mer. 23 janv. 2019 à 11:31, Guenter Roeck a écrit : > >On 1/23/19 4:58 AM, Mathieu Malaterre wrote: > >>On Wed, Dec 12, 2018 at 11:09 PM Paul Cercueil > >>wrote: > >>> > >>>From: Maarten ter Huurne > >>> > >>>OST is the OS Timer, a 64-bit timer/counter with buffered reading. > >>> > >>>SoCs before the JZ4770 had (if any) a 32-bit OST; the JZ4770 and > >>>JZ4780 have a 64-bit OST. > >>> > >>>This driver will register both a clocksource and a sched_clock to the > >>>system. > >>> > >>>Signed-off-by: Maarten ter Huurne > >>>Signed-off-by: Paul Cercueil > >>>--- > >>> > >>>Notes: > >>> v5: New patch > >>> > >>> v6: - Get rid of SoC IDs; pass pointer to ingenic_ost_soc_info > >>>as > >>>devicetree match data instead. > >>> - Use device_get_match_data() instead of the of_* variant > >>> - Handle error of dev_get_regmap() properly > >>> > >>> v7: Fix section mismatch by using > >>>builtin_platform_driver_probe() > >>> > >>> v8: builtin_platform_driver_probe() does not work anymore in > >>> 4.20-rc6? The probe function won't be called. Work around > >>>this > >>> for now by using late_initcall. > >>> > > > >Did anyone notice this ? Either something is wrong with the driver, or > >with the kernel core. Hacking around it seems like the worst possible > >"solution". > > I can confirm it still happens on 5.0-rc3. > > Just to explain what I'm doing: > > My ingenic-timer driver probes with builtin_platform_driver_probe (this > works), > and then calls of_platform_populate to probe its children. This driver, > ingenic-ost, is one of them, and will fail to probe with > builtin_platform_driver_probe. > The big question is _why_ it fails to probe. Guenter
Re: [PATCH v8 05/26] clocksource: Add driver for the Ingenic JZ47xx OST
(re-send, in plain text this time, sorry for those who got it twice)
Le mer. 23 janv. 2019 à 9:58, Mathieu Malaterre a
écrit :
On Wed, Dec 12, 2018 at 11:09 PM Paul Cercueil
wrote:
From: Maarten ter Huurne
OST is the OS Timer, a 64-bit timer/counter with buffered reading.
SoCs before the JZ4770 had (if any) a 32-bit OST; the JZ4770 and
JZ4780 have a 64-bit OST.
This driver will register both a clocksource and a sched_clock to
the
system.
Signed-off-by: Maarten ter Huurne
Signed-off-by: Paul Cercueil
---
Notes:
v5: New patch
v6: - Get rid of SoC IDs; pass pointer to ingenic_ost_soc_info
as
devicetree match data instead.
- Use device_get_match_data() instead of the of_* variant
- Handle error of dev_get_regmap() properly
v7: Fix section mismatch by using
builtin_platform_driver_probe()
v8: builtin_platform_driver_probe() does not work anymore in
4.20-rc6? The probe function won't be called. Work around
this
for now by using late_initcall.
drivers/clocksource/Kconfig | 8 ++
drivers/clocksource/Makefile | 1 +
drivers/clocksource/ingenic-ost.c | 215
++
3 files changed, 224 insertions(+)
create mode 100644 drivers/clocksource/ingenic-ost.c
diff --git a/drivers/clocksource/Kconfig
b/drivers/clocksource/Kconfig
index 4e69af15c3e7..e0646878b0de 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -648,4 +648,12 @@ config INGENIC_TIMER
help
Support for the timer/counter unit of the Ingenic JZ SoCs.
+config INGENIC_OST
+ bool "Ingenic JZ47xx Operating System Timer"
+ depends on MIPS || COMPILE_TEST
+ depends on COMMON_CLK
+ select INGENIC_TIMER
+ help
+ Support for the OS Timer of the Ingenic JZ4770 or similar
SoC.
+
endmenu
diff --git a/drivers/clocksource/Makefile
b/drivers/clocksource/Makefile
index 7c8f790dcf67..7faa8108574a 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -75,6 +75,7 @@ obj-$(CONFIG_ASM9260_TIMER) +=
asm9260_timer.o
obj-$(CONFIG_H8300_TMR8) += h8300_timer8.o
obj-$(CONFIG_H8300_TMR16) += h8300_timer16.o
obj-$(CONFIG_H8300_TPU)+= h8300_tpu.o
+obj-$(CONFIG_INGENIC_OST) += ingenic-ost.o
obj-$(CONFIG_INGENIC_TIMER)+= ingenic-timer.o
obj-$(CONFIG_CLKSRC_ST_LPC)+= clksrc_st_lpc.o
obj-$(CONFIG_X86_NUMACHIP) += numachip.o
diff --git a/drivers/clocksource/ingenic-ost.c
b/drivers/clocksource/ingenic-ost.c
new file mode 100644
index ..cc0fee3efd29
--- /dev/null
+++ b/drivers/clocksource/ingenic-ost.c
@@ -0,0 +1,215 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * JZ47xx SoCs TCU Operating System Timer driver
+ *
+ * Copyright (C) 2016 Maarten ter Huurne
+ * Copyright (C) 2018 Paul Cercueil
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "ingenic-timer.h"
+
+#define TCU_OST_TCSR_MASK 0xc0
+#define TCU_OST_TCSR_CNT_MDBIT(15)
+
+#define TCU_OST_CHANNEL15
+
+struct ingenic_ost_soc_info {
+ bool is64bit;
+};
+
+struct ingenic_ost {
+ struct regmap *map;
+ struct clk *clk;
+
+ struct clocksource cs;
+};
+
+static u64 notrace ingenic_ost_read_cntl(void)
+{
+ /* Bypass the regmap here as we must return as soon as
possible */
+ return readl(ingenic_tcu_base + TCU_REG_OST_CNTL);
+}
+
+static u64 notrace ingenic_ost_read_cnth(void)
+{
+ /* Bypass the regmap here as we must return as soon as
possible */
+ return readl(ingenic_tcu_base + TCU_REG_OST_CNTH);
+}
+
+static u64 notrace ingenic_ost_clocksource_read(struct clocksource
*cs)
+{
+ u32 val1, val2;
+ u64 count, recount;
+ s64 diff;
+
+ /*
+* The buffering of the upper 32 bits of the timer prevents
wrong
+* results from the bottom 32 bits overflowing due to the
timer ticking
+* along. However, it does not prevent wrong results from
simultaneous
+* reads of the timer, which could reset the buffer
mid-read.
+* Since this kind of wrong read can happen only when the
bottom bits
+* overflow, there will be minutes between wrong reads, so
if we read
+* twice in succession, at least one of the reads will be
correct.
+*/
+
+ /* Bypass the regmap here as we must return as soon as
possible */
+ val1 = readl(ingenic_tcu_base + TCU_REG_OST_CNTL);
+ val2 = readl(ingenic_tcu_base + TCU_REG_OST_CNTHBUF);
+ count = (u64)val1 | (u64)val2 << 32;
+
+ val1 = readl(ingenic_tcu_base + TCU_REG_OST_CNTL);
+ val2 = readl(ingenic_tcu_base + TCU_REG_OST_CNTHBUF);
+ recount = (u64)val1 |
Re: [PATCH v8 05/26] clocksource: Add driver for the Ingenic JZ47xx OST
Hi, Le mer. 23 janv. 2019 à 11:31, Guenter Roeck a écrit : On 1/23/19 4:58 AM, Mathieu Malaterre wrote: On Wed, Dec 12, 2018 at 11:09 PM Paul Cercueil wrote: From: Maarten ter Huurne OST is the OS Timer, a 64-bit timer/counter with buffered reading. SoCs before the JZ4770 had (if any) a 32-bit OST; the JZ4770 and JZ4780 have a 64-bit OST. This driver will register both a clocksource and a sched_clock to the system. Signed-off-by: Maarten ter Huurne Signed-off-by: Paul Cercueil --- Notes: v5: New patch v6: - Get rid of SoC IDs; pass pointer to ingenic_ost_soc_info as devicetree match data instead. - Use device_get_match_data() instead of the of_* variant - Handle error of dev_get_regmap() properly v7: Fix section mismatch by using builtin_platform_driver_probe() v8: builtin_platform_driver_probe() does not work anymore in 4.20-rc6? The probe function won't be called. Work around this for now by using late_initcall. Did anyone notice this ? Either something is wrong with the driver, or with the kernel core. Hacking around it seems like the worst possible "solution". I can confirm it still happens on 5.0-rc3. Just to explain what I'm doing: My ingenic-timer driver probes with builtin_platform_driver_probe (this works), and then calls of_platform_populate to probe its children. This driver, ingenic-ost, is one of them, and will fail to probe with builtin_platform_driver_probe. -Paul
Re: [PATCH v8 05/26] clocksource: Add driver for the Ingenic JZ47xx OST
On 1/23/19 4:58 AM, Mathieu Malaterre wrote: On Wed, Dec 12, 2018 at 11:09 PM Paul Cercueil wrote: From: Maarten ter Huurne OST is the OS Timer, a 64-bit timer/counter with buffered reading. SoCs before the JZ4770 had (if any) a 32-bit OST; the JZ4770 and JZ4780 have a 64-bit OST. This driver will register both a clocksource and a sched_clock to the system. Signed-off-by: Maarten ter Huurne Signed-off-by: Paul Cercueil --- Notes: v5: New patch v6: - Get rid of SoC IDs; pass pointer to ingenic_ost_soc_info as devicetree match data instead. - Use device_get_match_data() instead of the of_* variant - Handle error of dev_get_regmap() properly v7: Fix section mismatch by using builtin_platform_driver_probe() v8: builtin_platform_driver_probe() does not work anymore in 4.20-rc6? The probe function won't be called. Work around this for now by using late_initcall. Did anyone notice this ? Either something is wrong with the driver, or with the kernel core. Hacking around it seems like the worst possible "solution". Guenter
Re: [PATCH v8 05/26] clocksource: Add driver for the Ingenic JZ47xx OST
On Wed, Dec 12, 2018 at 11:09 PM Paul Cercueil wrote:
>
> From: Maarten ter Huurne
>
> OST is the OS Timer, a 64-bit timer/counter with buffered reading.
>
> SoCs before the JZ4770 had (if any) a 32-bit OST; the JZ4770 and
> JZ4780 have a 64-bit OST.
>
> This driver will register both a clocksource and a sched_clock to the
> system.
>
> Signed-off-by: Maarten ter Huurne
> Signed-off-by: Paul Cercueil
> ---
>
> Notes:
> v5: New patch
>
> v6: - Get rid of SoC IDs; pass pointer to ingenic_ost_soc_info as
>devicetree match data instead.
> - Use device_get_match_data() instead of the of_* variant
> - Handle error of dev_get_regmap() properly
>
> v7: Fix section mismatch by using builtin_platform_driver_probe()
>
> v8: builtin_platform_driver_probe() does not work anymore in
> 4.20-rc6? The probe function won't be called. Work around this
> for now by using late_initcall.
>
> drivers/clocksource/Kconfig | 8 ++
> drivers/clocksource/Makefile | 1 +
> drivers/clocksource/ingenic-ost.c | 215
> ++
> 3 files changed, 224 insertions(+)
> create mode 100644 drivers/clocksource/ingenic-ost.c
>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 4e69af15c3e7..e0646878b0de 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -648,4 +648,12 @@ config INGENIC_TIMER
> help
> Support for the timer/counter unit of the Ingenic JZ SoCs.
>
> +config INGENIC_OST
> + bool "Ingenic JZ47xx Operating System Timer"
> + depends on MIPS || COMPILE_TEST
> + depends on COMMON_CLK
> + select INGENIC_TIMER
> + help
> + Support for the OS Timer of the Ingenic JZ4770 or similar SoC.
> +
> endmenu
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index 7c8f790dcf67..7faa8108574a 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -75,6 +75,7 @@ obj-$(CONFIG_ASM9260_TIMER) += asm9260_timer.o
> obj-$(CONFIG_H8300_TMR8) += h8300_timer8.o
> obj-$(CONFIG_H8300_TMR16) += h8300_timer16.o
> obj-$(CONFIG_H8300_TPU)+= h8300_tpu.o
> +obj-$(CONFIG_INGENIC_OST) += ingenic-ost.o
> obj-$(CONFIG_INGENIC_TIMER)+= ingenic-timer.o
> obj-$(CONFIG_CLKSRC_ST_LPC)+= clksrc_st_lpc.o
> obj-$(CONFIG_X86_NUMACHIP) += numachip.o
> diff --git a/drivers/clocksource/ingenic-ost.c
> b/drivers/clocksource/ingenic-ost.c
> new file mode 100644
> index ..cc0fee3efd29
> --- /dev/null
> +++ b/drivers/clocksource/ingenic-ost.c
> @@ -0,0 +1,215 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * JZ47xx SoCs TCU Operating System Timer driver
> + *
> + * Copyright (C) 2016 Maarten ter Huurne
> + * Copyright (C) 2018 Paul Cercueil
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "ingenic-timer.h"
> +
> +#define TCU_OST_TCSR_MASK 0xc0
> +#define TCU_OST_TCSR_CNT_MDBIT(15)
> +
> +#define TCU_OST_CHANNEL15
> +
> +struct ingenic_ost_soc_info {
> + bool is64bit;
> +};
> +
> +struct ingenic_ost {
> + struct regmap *map;
> + struct clk *clk;
> +
> + struct clocksource cs;
> +};
> +
> +static u64 notrace ingenic_ost_read_cntl(void)
> +{
> + /* Bypass the regmap here as we must return as soon as possible */
> + return readl(ingenic_tcu_base + TCU_REG_OST_CNTL);
> +}
> +
> +static u64 notrace ingenic_ost_read_cnth(void)
> +{
> + /* Bypass the regmap here as we must return as soon as possible */
> + return readl(ingenic_tcu_base + TCU_REG_OST_CNTH);
> +}
> +
> +static u64 notrace ingenic_ost_clocksource_read(struct clocksource *cs)
> +{
> + u32 val1, val2;
> + u64 count, recount;
> + s64 diff;
> +
> + /*
> +* The buffering of the upper 32 bits of the timer prevents wrong
> +* results from the bottom 32 bits overflowing due to the timer
> ticking
> +* along. However, it does not prevent wrong results from simultaneous
> +* reads of the timer, which could reset the buffer mid-read.
> +* Since this kind of wrong read can happen only when the bottom bits
> +* overflow, there will be minutes between wrong reads, so if we read
> +* twice in succession, at least one of the reads will be correct.
> +*/
> +
> + /* Bypass the regmap here as we must return as soon as possible */
> + val1 = readl(ingenic_tcu_base + TCU_REG_OST_CNTL);
> + val2 = readl(ingenic_tcu_base + TCU_REG_OST_CNTHBUF);
> + count = (u64)val1 | (u64)val2 << 32;
> +
> + val1 = readl(ingenic_tcu_base + TCU_REG_OST_CNTL);
> + val2 = readl(ingenic_tcu_base + TCU_REG_OST_CNTHBUF);
> + recount = (u64)val1 | (u64)val2 << 32;
> +
> +
[PATCH v8 05/26] clocksource: Add driver for the Ingenic JZ47xx OST
From: Maarten ter Huurne
OST is the OS Timer, a 64-bit timer/counter with buffered reading.
SoCs before the JZ4770 had (if any) a 32-bit OST; the JZ4770 and
JZ4780 have a 64-bit OST.
This driver will register both a clocksource and a sched_clock to the
system.
Signed-off-by: Maarten ter Huurne
Signed-off-by: Paul Cercueil
---
Notes:
v5: New patch
v6: - Get rid of SoC IDs; pass pointer to ingenic_ost_soc_info as
devicetree match data instead.
- Use device_get_match_data() instead of the of_* variant
- Handle error of dev_get_regmap() properly
v7: Fix section mismatch by using builtin_platform_driver_probe()
v8: builtin_platform_driver_probe() does not work anymore in
4.20-rc6? The probe function won't be called. Work around this
for now by using late_initcall.
drivers/clocksource/Kconfig | 8 ++
drivers/clocksource/Makefile | 1 +
drivers/clocksource/ingenic-ost.c | 215 ++
3 files changed, 224 insertions(+)
create mode 100644 drivers/clocksource/ingenic-ost.c
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 4e69af15c3e7..e0646878b0de 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -648,4 +648,12 @@ config INGENIC_TIMER
help
Support for the timer/counter unit of the Ingenic JZ SoCs.
+config INGENIC_OST
+ bool "Ingenic JZ47xx Operating System Timer"
+ depends on MIPS || COMPILE_TEST
+ depends on COMMON_CLK
+ select INGENIC_TIMER
+ help
+ Support for the OS Timer of the Ingenic JZ4770 or similar SoC.
+
endmenu
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 7c8f790dcf67..7faa8108574a 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -75,6 +75,7 @@ obj-$(CONFIG_ASM9260_TIMER) += asm9260_timer.o
obj-$(CONFIG_H8300_TMR8) += h8300_timer8.o
obj-$(CONFIG_H8300_TMR16) += h8300_timer16.o
obj-$(CONFIG_H8300_TPU)+= h8300_tpu.o
+obj-$(CONFIG_INGENIC_OST) += ingenic-ost.o
obj-$(CONFIG_INGENIC_TIMER)+= ingenic-timer.o
obj-$(CONFIG_CLKSRC_ST_LPC)+= clksrc_st_lpc.o
obj-$(CONFIG_X86_NUMACHIP) += numachip.o
diff --git a/drivers/clocksource/ingenic-ost.c
b/drivers/clocksource/ingenic-ost.c
new file mode 100644
index ..cc0fee3efd29
--- /dev/null
+++ b/drivers/clocksource/ingenic-ost.c
@@ -0,0 +1,215 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * JZ47xx SoCs TCU Operating System Timer driver
+ *
+ * Copyright (C) 2016 Maarten ter Huurne
+ * Copyright (C) 2018 Paul Cercueil
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "ingenic-timer.h"
+
+#define TCU_OST_TCSR_MASK 0xc0
+#define TCU_OST_TCSR_CNT_MDBIT(15)
+
+#define TCU_OST_CHANNEL15
+
+struct ingenic_ost_soc_info {
+ bool is64bit;
+};
+
+struct ingenic_ost {
+ struct regmap *map;
+ struct clk *clk;
+
+ struct clocksource cs;
+};
+
+static u64 notrace ingenic_ost_read_cntl(void)
+{
+ /* Bypass the regmap here as we must return as soon as possible */
+ return readl(ingenic_tcu_base + TCU_REG_OST_CNTL);
+}
+
+static u64 notrace ingenic_ost_read_cnth(void)
+{
+ /* Bypass the regmap here as we must return as soon as possible */
+ return readl(ingenic_tcu_base + TCU_REG_OST_CNTH);
+}
+
+static u64 notrace ingenic_ost_clocksource_read(struct clocksource *cs)
+{
+ u32 val1, val2;
+ u64 count, recount;
+ s64 diff;
+
+ /*
+* The buffering of the upper 32 bits of the timer prevents wrong
+* results from the bottom 32 bits overflowing due to the timer ticking
+* along. However, it does not prevent wrong results from simultaneous
+* reads of the timer, which could reset the buffer mid-read.
+* Since this kind of wrong read can happen only when the bottom bits
+* overflow, there will be minutes between wrong reads, so if we read
+* twice in succession, at least one of the reads will be correct.
+*/
+
+ /* Bypass the regmap here as we must return as soon as possible */
+ val1 = readl(ingenic_tcu_base + TCU_REG_OST_CNTL);
+ val2 = readl(ingenic_tcu_base + TCU_REG_OST_CNTHBUF);
+ count = (u64)val1 | (u64)val2 << 32;
+
+ val1 = readl(ingenic_tcu_base + TCU_REG_OST_CNTL);
+ val2 = readl(ingenic_tcu_base + TCU_REG_OST_CNTHBUF);
+ recount = (u64)val1 | (u64)val2 << 32;
+
+ /*
+* A wrong read will produce a result that is 1<<32 too high: the bottom
+* part from before overflow and the upper part from after overflow.
+* Therefore, the lower value of the two reads is the correct value.
+*/
+
+ diff = (s64)(recount - count);
+ if (unlikely(diff < 0))
+
