lkml.org 
[lkml]   [2023]   [Aug]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] serial: 8250_bcm7271: improve bcm7271 8250 port
From
On 8/14/2023 9:28 AM, Justin Chen wrote:
>
>
> On 8/14/23 8:12 AM, Andy Shevchenko wrote:
>> On Sat, Aug 12, 2023 at 09:24:21PM -0700, Justin Chen wrote:
>>> On Sat, Aug 12, 2023 at 3:50 AM Greg Kroah-Hartman
>>> <gregkh@linuxfoundation.org> wrote:
>>>> On Fri, Aug 11, 2023 at 03:14:01PM -0700, Justin Chen wrote:
>>
>>>>> +     [PORT_BCM7271] = {
>>>>> +             .name           = "bcm7271_uart",
>>
>> This is badly named port type.
>>
This may be true, but it does mirror the PORT_BCM63XX naming and I do
value consistency so it is acceptable to me. However, I will happily
yield to a better name if one can be determined by popular consensus.

>
> Would "Brcmstb 7271 UART" suffice?
>
Perhaps, "Broadcom BCM7271 UART" but it seems excessively "chatty" to
me, so as I said I am OK with the original submission.

>>>>> +             .fifo_size      = 32,
>>>>> +             .tx_loadsz      = 32,
>>>>> +             .fcr            = UART_FCR_ENABLE_FIFO |
>>>>> UART_FCR_R_TRIG_01,
>>>>> +             .rxtrig_bytes   = {1, 8, 16, 30},
>>>>> +             .flags          = UART_CAP_FIFO | UART_CAP_AFE
>>>>> +     },
>>>>>   };
>>
>> This is almost a dup of PORT_ALTR_16550_F32. Use it if you wish.
>> You can always rename it if it feels the right thing to do.
>>
>
> There is some other PORT_ALTR logic that I would like to avoid. I would
> also like to avoid future changes to PORT_ALTR that wouldn't be
> applicable to us.
I too am reluctant to introduce yet another port type, but Justin is
correct in pointing out that the PORT_ALTR_16550_* port types include Tx
FIFO threshold programming that is incompatible with the BCM7271 UART
hardware. This port type does appear necessary to address fundamental
differences in the hardware unless we are willing to scrap the
uart_config[] array and have the individual drivers manage these
differences (which I would also be OK with, but I am just a tail on this
dog).

The BCM7271 UART IP does support programmable Tx FIFO thresholds in a
different way, so if I (or someone else) decided to enable support for
that it would appear that this new port type would be necessary at that
time as well.

>
>> But why 8 and not 16 is the default rxtrig?
>>
>
> We were seeing some latency issues on our chips where 16 would cause
> overflows. Trying to kill 2 birds with one stone. If creating another
> port type is avoidable then alternatively I can change the default in
> userspace.
>
> Thanks,
> Justin

Regards,
Doug

\
 
 \ /
  Last update: 2023-08-14 20:12    [W:0.143 / U:0.328 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site