Thread (13 messages) 13 messages, 4 authors, 2017-05-25

Re: [PATCH 1/1] xilinx ps uart: Adding a kernel parameter for the number of xilinx ps uarts

From: Michal Simek <hidden>
Date: 2017-05-24 13:07:38
Also in: linux-arm-kernel, lkml

On 23.5.2017 22:07, Alan Cox wrote:
quoted
yep hardcoded max 4 where in probe first free space is found and used
(range 0-3) but still max3100s statically allocated.
Shouldn't be this also dynamically allocated?
The code to do the dynamic allocation would be larger than the array of
pointers for the sane worst case.
quoted
I am not quite sure how exactly you want to do this via DT.
Count the number of DT entries for this kind of port and allocate that
many ?
:-) I think there is no doubt how to calculate that number for
static/fixed system. But where do you want to put it? And who should
read it?

A lot of drivers are calling uart_register_driver in module_init.
Others are calling it in probe(pl011 for example).

In module init you probably don't have a pointer to DT to read this and
this should be in generic location not in node itself.
If all drivers should call it from probe then we should change that -
then reading property is easy and only location should be cleared.

That's why I am curious how exactly you would do it because I haven't
seen this before.
quoted
Also what do you think is a safe maximum number? This is fpga - hundreds
of pins which can do just uart.
quoted
There are lots of options better than breaking the "one kernel many
platforms" model.  
Another options is also module parameter and dynamically allocated array
in cdns_uart_init.
For a given platform the number is constant and they need to be
described, so it seems to make no sense to put it anywhere other than the
DT for that platform.
Also I don't think this is really constant in all cases. Especially in
connection to fpga where you can put others IPs to PL. You never know
what it is present in partial region and how many of these are there.
It means having truly dynamic behavior would be welcome.

Can you call uart_register_driver() from probe for every instance? It
means nr is 1 all the time.
Why should users have to pass magic config options not use DT as
intended ?
I am not saying that config option is perfect solution. It is at least
aligned with 5 others serial drivers in the tree.

Thanks,
Michal
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help