Re: [PATCH v2] serial: uartlite: Move out-of-range port-numbers into ULITE_NR_UARTS range

[Michal Simek <michal.simek@xilinx.com>]

On 11/22/21 23:08, Ruediger Willenberg wrote:
> Am 19.11.2021 um 13:21 schrieb Michal Simek:
>> On 11/18/21 22:17, Ruediger Willenberg wrote:
>>> Find free uart_port struct in range 0 <= id < ULITE_NR_UARTS when
>>> the device tree port-number property is outside that range. This
>>> happens when there are other UART types in the system because the
>>> Xilinx device tree generator numbers all UARTs consecutively;
>>> as a result, not as many Uartlites as specified by the
>>> SERIAL_UARTLITE_NR_UARTS parameter could be successfully added.
>>>
>>> Signed-off-by: Ruediger Willenberg <r.willenberg@hs-mannheim.de>
>>> ---
>>> Changes in v2:
>>>   - give KERN_NOTICE when changing the id,
>>>     with reference to the requested port-number
>>>
>>> -    if (id < 0 || id >= ULITE_NR_UARTS) {
>>> -        dev_err(dev, "%s%i too large\n", ULITE_NAME, id);
>>> +    if (id == ULITE_NR_UARTS) {
>>> +        dev_err(dev, "maximum number of %s assigned\n", ULITE_NAME);
>>>           return -EINVAL;
>>>       }
>>> @@ -676,7 +678,11 @@ static int ulite_assign(struct device *dev, int 
>>> id, u32 base, int irq,
>>>           dev_set_drvdata(dev, NULL);
>>>           return rc;
>>>       }
>>
>> nit: please keep this newline here.
>>
>>> -
>>> +    if (oor_id >= 0)
>>> +        dev_notice(dev,
>>> +            "assigned uartlite with device tree port-number=<%i> to 
>>> %s%i\n",
>>> +            oor_id, ULITE_NAME, id);
>>> +
>>
>>
>>
>> [linux](master)$ ./scripts/checkpatch.pl --strict 
>> 0001-serial-uartlite-Move-out-of-range-port-numbers-into-.patch
>> CHECK: Alignment should match open parenthesis
> 
> Thanks, I apparently didn't do "--strict" checking, sorry. Will fix both 
> in PATCH v3
> 
> 
>> And there is one more issue with this. If you start to mix serial IPs 
>> which are partially recorded in aliases. For example like this.
>>
>> aliases {
>>      serial0 = &uart1;
>> };
>>
>> uart0 {
>> ...
>> };
>> Uart0 is probed first. It is without alias and id 0 will be assigned 
>> to it. Then uart1 is probed. It looks at aliases you get id 0 but it 
>> is already taken and you end up with "cannot assign to %s%i; it is 
>> already in use"
> 
> As I point out in the other thread
>   "[PATCH v2] uartlite: Update the default for the 
> SERIAL_UARTLITE_NR_UARTS"
> (I don't feel crossposting all of it here would be good practice)
> 
> a) the Xilinx DTG gives "serial" aliases to _all_ Xilinx UARTs
> b) the Xilinx DTG gives "port-number" properties to _all_ Xilinx UARTs
> c) these two are not assigned in the same order by the Xilinx DTG, which 
> is problematic

DTG/MSS logic should be solved in different mailing list that's why 
please move it there.

> 
> uartlite.c currently looks at port-number properties, not aliases. So 
> unassigned aliases are not the issue. A similar-looking but more 
> complicated issue that COULD happen, to be transparent, is this:

port-number is optional and as I said in the second thread I think that 
this should be deprecated and removed. Most of DT drivers take care 
about serial alias that's why we should go that route too.

And I also remember that drivers shouldn't really use different names as 
ttyUL, ttyPS, ttyAMA, etc but all of them should use the same name as 
ttyUSB is doing.

Thanks,
Michal