[PATCH] uartlite: Update the default for the parameter

[PATCH] uartlite: Update the default for the parameter

[Shubhrajyoti Datta]

Update the default number of uarts.
The commit b44b96a060f3 changed the default from 16 to 1 change it back to 16.

Fixes: b44b96a060f3 (uartlite: Adding a kernel parameter for the number of uartlites)

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
 drivers/tty/serial/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 131a6a587acd..1254b523ca9a 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -531,7 +531,7 @@ config SERIAL_UARTLITE_NR_UARTS
 	int "Maximum number of uartlite serial ports"
 	depends on SERIAL_UARTLITE
 	range 1 256
-	default 1
+	default 16
 	help
 	  Set this to the number of uartlites in your system, or the number
 	  you think you might implement.
-- 
2.25.1

Re: [PATCH] uartlite: Update the default for the parameter

[Michal Simek]

On 10/18/21 10:20, Shubhrajyoti Datta wrote:
> Update the default number of uarts.
> The commit b44b96a060f3 changed the default from 16 to 1 change it back to 16.
> 
> Fixes: b44b96a060f3 (uartlite: Adding a kernel parameter for the number of uartlites)
> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
>   drivers/tty/serial/Kconfig | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 131a6a587acd..1254b523ca9a 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -531,7 +531,7 @@ config SERIAL_UARTLITE_NR_UARTS
>   	int "Maximum number of uartlite serial ports"
>   	depends on SERIAL_UARTLITE
>   	range 1 256
> -	default 1
> +	default 16
>   	help
>   	  Set this to the number of uartlites in your system, or the number
>   	  you think you might implement.
> 

Reviewed-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal

Re: [PATCH] uartlite: Update the default for the parameter

[Greg KH]

On Mon, Oct 18, 2021 at 01:50:00PM +0530, Shubhrajyoti Datta wrote:
> Update the default number of uarts.
> The commit b44b96a060f3 changed the default from 16 to 1 change it back to 16.
> 
> Fixes: b44b96a060f3 (uartlite: Adding a kernel parameter for the number of uartlites)
> 

No extra line here please.

Also, your subject is odd, please make it more obvious what is
happening.

But nost importantly, _WHY_ are you making this change?  What happened
since that commit was made that now we should have the default be 16?
Previously that commit said the default should NOT be 16, and here you
are changing it back.  What changed to require this?

thanks,

greg k-h

Re: [PATCH] uartlite: Update the default for the parameter

[Greg KH]

On Mon, Oct 18, 2021 at 10:42:13AM +0200, Greg KH wrote:
> On Mon, Oct 18, 2021 at 01:50:00PM +0530, Shubhrajyoti Datta wrote:
> > Update the default number of uarts.
> > The commit b44b96a060f3 changed the default from 16 to 1 change it back to 16.
> > 
> > Fixes: b44b96a060f3 (uartlite: Adding a kernel parameter for the number of uartlites)
> > 
> 
> No extra line here please.
> 
> Also, your subject is odd, please make it more obvious what is
> happening.
> 
> But nost importantly, _WHY_ are you making this change?  What happened
> since that commit was made that now we should have the default be 16?
> Previously that commit said the default should NOT be 16, and here you
> are changing it back.  What changed to require this?

Also, you failed to cc: the author of that commit, which isn't the most
kind thing to do.  Please do so when you resubmit this with the
requested changes.

thanks,

greg k-h

Re: [PATCH] uartlite: Update the default for the parameter

[Michal Simek]

+Sam,

On 10/18/21 10:42, Greg KH wrote:
> On Mon, Oct 18, 2021 at 01:50:00PM +0530, Shubhrajyoti Datta wrote:
>> Update the default number of uarts.
>> The commit b44b96a060f3 changed the default from 16 to 1 change it back to 16.
>>
>> Fixes: b44b96a060f3 (uartlite: Adding a kernel parameter for the number of uartlites)
>>
> 
> No extra line here please.
> 
> Also, your subject is odd, please make it more obvious what is
> happening.
> 
> But nost importantly, _WHY_ are you making this change?  What happened
> since that commit was made that now we should have the default be 16?
> Previously that commit said the default should NOT be 16, and here you
> are changing it back.  What changed to require this?

"The uartlite is used by FPGAs that support a basically unlimited number
of uarts so limiting it at 16 dosn't make sense as users might need more
than that."

He said that doesn't make sense to limit it to 16 and limit it to 256 by 
Kconfig range and set default to 1. Where it is also said that number 
should be unlimited.
And also no information why he setup default to 1 where it should be 
unlimited. I would say high number should be fine for the most of cases.
I have never seen any design with 256 uarts on it.

I think we could potentially find real maximum number for looking at the 
biggest fpga and setup this number but from my perspective 256 is big 
enough for a range. But default 1 sounds weird. And origin 16 written in 
driver should be quite reasonable default to cover 99% cases.

Thanks,
Michal