diff mbox series

[2/2] drivers: misc: add ripple counter driver

Message ID 20210226141411.2517368-3-linux@rasmusvillemoes.dk
State New
Headers show
Series add ripple counter dt binding and driver | expand

Commit Message

Rasmus Villemoes Feb. 26, 2021, 2:14 p.m. UTC
The only purpose of this driver is to serve as a consumer of the input
clock, to prevent it from being disabled by clk_disable_unused().

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/misc/Kconfig      |  7 +++++++
 drivers/misc/Makefile     |  1 +
 drivers/misc/ripple-ctr.c | 31 +++++++++++++++++++++++++++++++
 3 files changed, 39 insertions(+)
 create mode 100644 drivers/misc/ripple-ctr.c

Comments

Chen, Mike Ximing Feb. 28, 2021, 5:47 a.m. UTC | #1
> -----Original Message-----

> From: Rasmus Villemoes <linux@rasmusvillemoes.dk>

> Sent: Friday, February 26, 2021 9:14 AM

> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Rob Herring

> <robh+dt@kernel.org>

> Cc: devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Arnd Bergmann

> <arnd@arndb.de>; linux-clk@vger.kernel.org; Rasmus Villemoes

> <linux@rasmusvillemoes.dk>

> Subject: [PATCH 2/2] drivers: misc: add ripple counter driver

> 

> The only purpose of this driver is to serve as a consumer of the input

> clock, to prevent it from being disabled by clk_disable_unused().

> 

> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>

> ---

>  drivers/misc/Kconfig      |  7 +++++++

>  drivers/misc/Makefile     |  1 +

>  drivers/misc/ripple-ctr.c | 31 +++++++++++++++++++++++++++++++

>  3 files changed, 39 insertions(+)

>  create mode 100644 drivers/misc/ripple-ctr.c

> 

> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig

> index f532c59bb59b..44b0b6ce42df 100644

> --- a/drivers/misc/Kconfig

> +++ b/drivers/misc/Kconfig

> @@ -445,6 +445,13 @@ config HISI_HIKEY_USB

>  	  switching between the dual-role USB-C port and the USB-A host ports

>  	  using only one USB controller.

> 

> +config RIPPLE_CTR

> +	tristate "Trivial ripple counter driver"

> +	help

> +	  This provides a stub driver for a ripple counter, whose

> +	  only purpose is to request and enable the clock source

> +	  driving the counter.

> +

>  source "drivers/misc/c2port/Kconfig"

>  source "drivers/misc/eeprom/Kconfig"

>  source "drivers/misc/cb710/Kconfig"

> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile

> index 99b6f15a3c70..d560163068a9 100644

> --- a/drivers/misc/Makefile

> +++ b/drivers/misc/Makefile

> @@ -56,3 +56,4 @@ obj-$(CONFIG_HABANA_AI)		+= habanalabs/

>  obj-$(CONFIG_UACCE)		+= uacce/

>  obj-$(CONFIG_XILINX_SDFEC)	+= xilinx_sdfec.o

>  obj-$(CONFIG_HISI_HIKEY_USB)	+= hisi_hikey_usb.o

> +obj-$(CONFIG_RIPPLE_CTR)	+= ripple-ctr.o

> diff --git a/drivers/misc/ripple-ctr.c b/drivers/misc/ripple-ctr.c

> new file mode 100644

> index 000000000000..f086eaf335df

> --- /dev/null

> +++ b/drivers/misc/ripple-ctr.c

> @@ -0,0 +1,31 @@

> +// SPDX-License-Identifier: GPL-2.0-or-later

> +#include <linux/clk.h>

> +#include <linux/err.h>

> +#include <linux/mod_devicetable.h>

> +#include <linux/module.h>

> +#include <linux/platform_device.h>

> +

> +static int ripple_ctr_probe(struct platform_device *pdev)

> +{

> +	struct clk *clk;

> +

> +	clk = devm_clk_get(&pdev->dev, NULL);

> +	if (IS_ERR(clk))

> +		return PTR_ERR(clk);

> +	return clk_prepare_enable(clk);

> +}

> +

> +static const struct of_device_id ripple_ctr_ids[] = {

> +	{ .compatible = "linux,ripple-counter", },

> +	{ }

> +};

> +MODULE_DEVICE_TABLE(of, ripple_ctr_ids);

> +

> +static struct platform_driver ripple_ctr_driver = {

> +	.driver	= {

> +		.name		= "ripple-counter",

> +		.of_match_table	= ripple_ctr_ids,

> +	},

> +	.probe	= ripple_ctr_probe,

> +};

> +module_platform_driver(ripple_ctr_driver);


Missing MODULE_LICENSE() tag?
See https://www.kernel.org/doc/html/latest/process/license-rules.html#license-identifiers

> --

> 2.29.2
Andy Shevchenko Feb. 28, 2021, 9:29 a.m. UTC | #2
On Sun, Feb 28, 2021 at 11:07 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Friday, February 26, 2021, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:

>>

>> The only purpose of this driver is to serve as a consumer of the input

>> clock, to prevent it from being disabled by clk_disable_unused().

>

> We have a clock API to do the same (something like marking it used or so) why do you need a driver?


Example:
https://elixir.bootlin.com/linux/latest/source/drivers/platform/x86/pmc_atom.c#L365

If it's a DT based platform I think you can make it somehow work thru DT.

-- 
With Best Regards,
Andy Shevchenko
Andy Shevchenko Feb. 28, 2021, 9:33 a.m. UTC | #3
On Sun, Feb 28, 2021 at 11:29 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>

> On Sun, Feb 28, 2021 at 11:07 AM Andy Shevchenko

> <andy.shevchenko@gmail.com> wrote:

> > On Friday, February 26, 2021, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:

> >>

> >> The only purpose of this driver is to serve as a consumer of the input

> >> clock, to prevent it from being disabled by clk_disable_unused().

> >

> > We have a clock API to do the same (something like marking it used or so) why do you need a driver?

>

> Example:

> https://elixir.bootlin.com/linux/latest/source/drivers/platform/x86/pmc_atom.c#L365

>

> If it's a DT based platform I think you can make it somehow work thru DT.


Okay, briefly looking at the state of affairs [1] seems like you need
to hack it into clock provider.

[1]: https://elixir.bootlin.com/linux/latest/C/ident/CLK_IS_CRITICAL

-- 
With Best Regards,
Andy Shevchenko
Rasmus Villemoes March 1, 2021, 8:29 a.m. UTC | #4
On 28/02/2021 10.33, Andy Shevchenko wrote:
> On Sun, Feb 28, 2021 at 11:29 AM Andy Shevchenko

> <andy.shevchenko@gmail.com> wrote:

>>

>> On Sun, Feb 28, 2021 at 11:07 AM Andy Shevchenko

>> <andy.shevchenko@gmail.com> wrote:

>>> On Friday, February 26, 2021, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:

>>>>

>>>> The only purpose of this driver is to serve as a consumer of the input

>>>> clock, to prevent it from being disabled by clk_disable_unused().

>>>

>>> We have a clock API to do the same (something like marking it used or so) why do you need a driver?

>>

>> Example:

>> https://elixir.bootlin.com/linux/latest/source/drivers/platform/x86/pmc_atom.c#L365

>>

>> If it's a DT based platform I think you can make it somehow work thru DT.

> 

> Okay, briefly looking at the state of affairs [1] seems like you need

> to hack it into clock provider.

> 

> [1]: https://elixir.bootlin.com/linux/latest/C/ident/CLK_IS_CRITICAL

> 


I did find CLK_IS_CRITICAL and CLK_IGNORE_UNUSED while trying to figure
out how to handle this. However, while CLK_IS_CRITICAL is in principle
settable via DT, the comment above of_clk_detect_critical() seems to
make it clear that adding a call of that from the RTC driver is a total
no-no.

CLK_IGNORE_UNUSED can't be set at all from DT, and wouldn't solve the
problem fully - while we can and do make sure the bootloader sets the
appropriate bit in the RTCs registers, it's more robust if we also
ensure the kernel explicitly enables the clock.

But if there is some way to do this within the clk framework/existing
bindings, I'm all ears - that's the reason I cc'ed the clk list.

Rasmus
diff mbox series

Patch

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index f532c59bb59b..44b0b6ce42df 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -445,6 +445,13 @@  config HISI_HIKEY_USB
 	  switching between the dual-role USB-C port and the USB-A host ports
 	  using only one USB controller.
 
+config RIPPLE_CTR
+	tristate "Trivial ripple counter driver"
+	help
+	  This provides a stub driver for a ripple counter, whose
+	  only purpose is to request and enable the clock source
+	  driving the counter.
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 99b6f15a3c70..d560163068a9 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -56,3 +56,4 @@  obj-$(CONFIG_HABANA_AI)		+= habanalabs/
 obj-$(CONFIG_UACCE)		+= uacce/
 obj-$(CONFIG_XILINX_SDFEC)	+= xilinx_sdfec.o
 obj-$(CONFIG_HISI_HIKEY_USB)	+= hisi_hikey_usb.o
+obj-$(CONFIG_RIPPLE_CTR)	+= ripple-ctr.o
diff --git a/drivers/misc/ripple-ctr.c b/drivers/misc/ripple-ctr.c
new file mode 100644
index 000000000000..f086eaf335df
--- /dev/null
+++ b/drivers/misc/ripple-ctr.c
@@ -0,0 +1,31 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+static int ripple_ctr_probe(struct platform_device *pdev)
+{
+	struct clk *clk;
+
+	clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+	return clk_prepare_enable(clk);
+}
+
+static const struct of_device_id ripple_ctr_ids[] = {
+	{ .compatible = "linux,ripple-counter", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ripple_ctr_ids);
+
+static struct platform_driver ripple_ctr_driver = {
+	.driver	= {
+		.name		= "ripple-counter",
+		.of_match_table	= ripple_ctr_ids,
+	},
+	.probe	= ripple_ctr_probe,
+};
+module_platform_driver(ripple_ctr_driver);