pps: make number of PPS devices configurable

[RFC PATCH] pps: make number of PPS devices configurable

Posted by Vadim Fedorenko 4 weeks, 1 day ago

Modern systems may have more than 16 PPS sources and current hard-coded
limit breaks registration of some devices. Make the limit configurable
via Kconfig, keep default value of 16 devices. UAPI has to be changed to
support maximum possible number.

Signed-off-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
---
 drivers/pps/Kconfig      | 9 +++++++++
 drivers/pps/pps.c        | 6 +++---
 include/uapi/linux/pps.h | 2 +-
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/pps/Kconfig b/drivers/pps/Kconfig
index e1651d51cfc9..166c157505fe 100644
--- a/drivers/pps/Kconfig
+++ b/drivers/pps/Kconfig
@@ -22,6 +22,15 @@ menuconfig PPS
 
 if PPS
 
+config PPS_NR_SOURCES
+	int "Maximum number of PPS devices"
+	depends on PPS
+	range 16 256
+	default "16"
+	help
+	  Set this to the number of PPS devices you want the driver
+	  to support.
+
 config PPS_DEBUG
 	bool "PPS debugging messages"
 	help
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index de1122bb69ea..d3eac77b4cb5 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -370,7 +370,7 @@ int pps_register_cdev(struct pps_device *pps)
 	 * Get new ID for the new PPS source.  After idr_alloc() calling
 	 * the new source will be freely available into the kernel.
 	 */
-	err = idr_alloc(&pps_idr, pps, 0, PPS_MAX_SOURCES, GFP_KERNEL);
+	err = idr_alloc(&pps_idr, pps, 0, CONFIG_PPS_NR_SOURCES, GFP_KERNEL);
 	if (err < 0) {
 		if (err == -ENOSPC) {
 			pr_err("%s: too many PPS sources in the system\n",
@@ -464,7 +464,7 @@ EXPORT_SYMBOL(pps_lookup_dev);
 static void __exit pps_exit(void)
 {
 	class_unregister(&pps_class);
-	__unregister_chrdev(pps_major, 0, PPS_MAX_SOURCES, "pps");
+	__unregister_chrdev(pps_major, 0, CONFIG_PPS_NR_SOURCES, "pps");
 }
 
 static int __init pps_init(void)
@@ -477,7 +477,7 @@ static int __init pps_init(void)
 		return err;
 	}
 
-	pps_major = __register_chrdev(0, 0, PPS_MAX_SOURCES, "pps",
+	pps_major = __register_chrdev(0, 0, CONFIG_PPS_NR_SOURCES, "pps",
 				      &pps_cdev_fops);
 	if (pps_major < 0) {
 		pr_err("failed to allocate char device region\n");
diff --git a/include/uapi/linux/pps.h b/include/uapi/linux/pps.h
index 009ebcd8ced5..1088dea65e12 100644
--- a/include/uapi/linux/pps.h
+++ b/include/uapi/linux/pps.h
@@ -26,7 +26,7 @@
 #include <linux/types.h>
 
 #define PPS_VERSION		"5.3.6"
-#define PPS_MAX_SOURCES		16		/* should be enough... */
+#define PPS_MAX_SOURCES		256		/* should be enough... */
 
 /* Implementation note: the logical states ``assert'' and ``clear''
  * are implemented in terms of the chip register, i.e. ``assert''
-- 
2.47.3

Re: [RFC PATCH] pps: make number of PPS devices configurable

Posted by Rodolfo Giometti 4 weeks ago

On 5/14/26 13:26, Vadim Fedorenko wrote:
> Modern systems may have more than 16 PPS sources and current hard-coded
> limit breaks registration of some devices. Make the limit configurable
> via Kconfig, keep default value of 16 devices. UAPI has to be changed to
> support maximum possible number.
> 
> Signed-off-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
> ---
>   drivers/pps/Kconfig      | 9 +++++++++
>   drivers/pps/pps.c        | 6 +++---
>   include/uapi/linux/pps.h | 2 +-
>   3 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pps/Kconfig b/drivers/pps/Kconfig
> index e1651d51cfc9..166c157505fe 100644
> --- a/drivers/pps/Kconfig
> +++ b/drivers/pps/Kconfig
> @@ -22,6 +22,15 @@ menuconfig PPS
>   
>   if PPS
>   
> +config PPS_NR_SOURCES
> +	int "Maximum number of PPS devices"
> +	depends on PPS
> +	range 16 256
> +	default "16"
> +	help
> +	  Set this to the number of PPS devices you want the driver
> +	  to support.
> +
>   config PPS_DEBUG
>   	bool "PPS debugging messages"
>   	help
> diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
> index de1122bb69ea..d3eac77b4cb5 100644
> --- a/drivers/pps/pps.c
> +++ b/drivers/pps/pps.c
> @@ -370,7 +370,7 @@ int pps_register_cdev(struct pps_device *pps)
>   	 * Get new ID for the new PPS source.  After idr_alloc() calling
>   	 * the new source will be freely available into the kernel.
>   	 */
> -	err = idr_alloc(&pps_idr, pps, 0, PPS_MAX_SOURCES, GFP_KERNEL);
> +	err = idr_alloc(&pps_idr, pps, 0, CONFIG_PPS_NR_SOURCES, GFP_KERNEL);
>   	if (err < 0) {
>   		if (err == -ENOSPC) {
>   			pr_err("%s: too many PPS sources in the system\n",
> @@ -464,7 +464,7 @@ EXPORT_SYMBOL(pps_lookup_dev);
>   static void __exit pps_exit(void)
>   {
>   	class_unregister(&pps_class);
> -	__unregister_chrdev(pps_major, 0, PPS_MAX_SOURCES, "pps");
> +	__unregister_chrdev(pps_major, 0, CONFIG_PPS_NR_SOURCES, "pps");
>   }
>   
>   static int __init pps_init(void)
> @@ -477,7 +477,7 @@ static int __init pps_init(void)
>   		return err;
>   	}
>   
> -	pps_major = __register_chrdev(0, 0, PPS_MAX_SOURCES, "pps",
> +	pps_major = __register_chrdev(0, 0, CONFIG_PPS_NR_SOURCES, "pps",
>   				      &pps_cdev_fops);
>   	if (pps_major < 0) {
>   		pr_err("failed to allocate char device region\n");
> diff --git a/include/uapi/linux/pps.h b/include/uapi/linux/pps.h
> index 009ebcd8ced5..1088dea65e12 100644
> --- a/include/uapi/linux/pps.h
> +++ b/include/uapi/linux/pps.h
> @@ -26,7 +26,7 @@
>   #include <linux/types.h>
>   
>   #define PPS_VERSION		"5.3.6"
> -#define PPS_MAX_SOURCES		16		/* should be enough... */
> +#define PPS_MAX_SOURCES		256		/* should be enough... */

Nak. Let's make everything configurable, or let's leave it constant. 256 is OK.

Ciao,

Rodolfo

>   /* Implementation note: the logical states ``assert'' and ``clear''
>    * are implemented in terms of the chip register, i.e. ``assert''


-- 
GNU/Linux Solutions                  e-mail: giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming

Re: [RFC PATCH] pps: make number of PPS devices configurable

Posted by Jakub Kicinski 4 weeks ago

On Thu, 14 May 2026 11:26:14 +0000 Vadim Fedorenko wrote:
> Modern systems may have more than 16 PPS sources and current hard-coded
> limit breaks registration of some devices. Make the limit configurable
> via Kconfig, keep default value of 16 devices. UAPI has to be changed to
> support maximum possible number.

Hm, since this is effectively just sizing an IDR maybe we should just
bump it to 64? It appears to cost nothing if not used.

Re: [RFC PATCH] pps: make number of PPS devices configurable

Posted by Vadim Fedorenko 4 weeks ago

On 14/05/2026 14:55, Jakub Kicinski wrote:
> On Thu, 14 May 2026 11:26:14 +0000 Vadim Fedorenko wrote:
>> Modern systems may have more than 16 PPS sources and current hard-coded
>> limit breaks registration of some devices. Make the limit configurable
>> via Kconfig, keep default value of 16 devices. UAPI has to be changed to
>> support maximum possible number.
> 
> Hm, since this is effectively just sizing an IDR maybe we should just
> bump it to 64? It appears to cost nothing if not used.

Just bump to 64, but keep it constant? In this case maybe 256 just to
align with the max minor value for chardev (in compat mode)?

Re: [RFC PATCH] pps: make number of PPS devices configurable

Posted by Jakub Kicinski 4 weeks ago

On Thu, 14 May 2026 15:03:03 +0100 Vadim Fedorenko wrote:
> On 14/05/2026 14:55, Jakub Kicinski wrote:
> > On Thu, 14 May 2026 11:26:14 +0000 Vadim Fedorenko wrote:  
> >> Modern systems may have more than 16 PPS sources and current hard-coded
> >> limit breaks registration of some devices. Make the limit configurable
> >> via Kconfig, keep default value of 16 devices. UAPI has to be changed to
> >> support maximum possible number.  
> > 
> > Hm, since this is effectively just sizing an IDR maybe we should just
> > bump it to 64? It appears to cost nothing if not used.  
> 
> Just bump to 64, but keep it constant? In this case maybe 256 just to
> align with the max minor value for chardev (in compat mode)?

That's fine too (the value is the count right? It's not the max id?
Cause 256 would spill out of u8)

Re: [RFC PATCH] pps: make number of PPS devices configurable

Posted by Vadim Fedorenko 4 weeks ago

On 14/05/2026 15:08, Jakub Kicinski wrote:
> On Thu, 14 May 2026 15:03:03 +0100 Vadim Fedorenko wrote:
>> On 14/05/2026 14:55, Jakub Kicinski wrote:
>>> On Thu, 14 May 2026 11:26:14 +0000 Vadim Fedorenko wrote:
>>>> Modern systems may have more than 16 PPS sources and current hard-coded
>>>> limit breaks registration of some devices. Make the limit configurable
>>>> via Kconfig, keep default value of 16 devices. UAPI has to be changed to
>>>> support maximum possible number.
>>>
>>> Hm, since this is effectively just sizing an IDR maybe we should just
>>> bump it to 64? It appears to cost nothing if not used.
>>
>> Just bump to 64, but keep it constant? In this case maybe 256 just to
>> align with the max minor value for chardev (in compat mode)?
> 
> That's fine too (the value is the count right? It's not the max id?
> Cause 256 would spill out of u8)

It's the count. Max id will be 255.