On 11/18/21 22:21, Ruediger Willenberg wrote:
Am 18.11.2021 um 09:45 schrieb Michal Simek:
On 11/17/21 10:53, Ruediger Willenberg wrote:
Am 17.11.2021 um 06:16 schrieb Shubhrajyoti Datta:
"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."
the commit also said that number should be unlimited. However it set
the
default to 1 instead of 16.The original 16 written in driver should be
quite reasonable default to cover most of the cases.
So change the default number of uarts back to 16.
The DTG should number devices for each driver separately from 0;
serial_core.c checks for (0 <= uart_port->line < NR_UART_PORTS). As a
consequence, when a Zynq system has both a PS-UART and a Uartlite,
setting SERIAL_UARTLITE_NR_UARTS explicitly to 1 in Kconfig means
probing the uartlite fails, which is confusing to the unsuspecting
KConfig user. Setting the default to 16 just kicks the can down the
road because it will fail for more than 15 Uartlites (or less, if there
are more PS-UARTs or AXI 16550A UARTs).
I have no problem with your patch and it is correct (I would prefer a
message to show for this option). But it doesn't mean that default 1
was correct value to setup especially when range was setup by Sam from
1-256. And originally number 16 was used.
That's why I think this patch is still valid. If you don't like
description it can be changed because definitely people are using
systems with more then one uartlite and 1 is not reasonable default.
We can also take a look at default for others serial drivers
and none of them is using 1 as default. IIRC discussion about UARTPS
it was said if number is not
Clarification: I support changing the default up to 16. It will avoid
problems for most cases where
a) there are only a few Uartlites in the system
AND
b) the Kconfig user doesn't explictly specify SERIAL_UARTLITE_NR_UARTS
I just wanted to point out that trouble ensues when
a) somebody wants 16 Uartlites in a system when there is also a PS UART
(basically any Zynq)
OR
b) somebody explicitly sets the SERIAL_UARTLITE_NR_UARTS to the number
she built into a Zynq system
(when the Uartlite driver is activated in menuconfig, the NR parameters
pops right up, so it's mighty tempting to actually set it to the
accurate number)
Moving the default up should just not be a band-aid for the cases where
a single Uartlite can not be assigned, when really there is a
fundamental inconsistency between how serial_core.c has limited line
numbering since Kernel 2.6 (0 <= line_id < NR_UARTs) and what Xilinx DTG
does with UARTs.
> And using aliases for serial id identification is around for a while
and Xilinx DTG is aligned with
> it because via current dt you can't describe cases where you will
have stable numbers for different
> tty names like
> ttyS0, ttyUL0, ttyPS0, ttyAMA0, etc.
> That's why DTG generates it per all serial IDs.
Sincere apologies, I do not quite understand this explanation/argument.
Would you be so kind to rephrase?
(As I'm a newbie to this list, I'm not sure how much back-and-forth is
appropriate about a topic. Since my issue here is about the Xilinx DTG,
not Linux kernel code per se, I'd also be fine to continue emailing
off-list. There does not seem to be a similar list for device-tree-xlnx)
A lot of drivers are calling of_alias_get_id() which returns you ID from
aliases list. If you don't use it or ID is more then your amount of
allocates ports you are choosing different logic how to find proper number.
In for example arch/arm/boot/dts/at91-kizbox3_common.dtsi
you can see
24 aliases {
25 serial0 = &uart0;
26 serial1 = &uart1;
27 serial2 = &uart2;
28 serial3 = &uart3;
29 serial4 = &uart4;
30 serial5 = &uart5;
31 serial6 = &uart8;
32 };
It means uart0 should become ttyX0 uart1 should become ttyX1, etc
If you have all uarts the same type then everything is fine.
But on systems like FPGAs you have different types of uart and via DT
you can't really describe things like this
aliases {
serial0 = &ps_uart; // to become ttyPS0
serial0 = &uartlite; // to become ttyUL0
serial0 = &uart16550; // to become ttyS0
serial0 = &uartAMA; // to become ttyAMA0
...
}
This is not allowed in DT. You can have only one alias with the same ID.
And if you don't use aliases order depends on probe order which doesn't
need to be stable.
If you have only one instance of IP then you will get just id 0 but if
you have multiple instances of the same IP type then order is
unpredictable. And you really don't want to assigned console to data
uart channel and vice-versa or mix channels, etc.
That's why Xilinx DTG assigns number to every IP to be stable and
predictable. A lot of people think that what you get from DTG doesn't
need to be changed but that's not intention of DTG at all. DTG just does
the best to describe the system. It is up to user to do whatever changes
are required for their system. If you don't like it feel free to
change/fix it.
And in past port-number property was in spec but it not supported by all
drivers.
mpc52xx_uart dt binding used it (mpc5200-psc-uart v2.6.20)
ucc_uart and uartlite.
I think we have also generated it for 16550 xilinx uart in past.
Thanks,
Michal
