Hello Antony,
On 25.05.21 09:36, Antony Pavlov wrote:
> On Tue, 25 May 2021 10:19:47 +0300
> Antony Pavlov <antonynpavlov@xxxxxxxxx> wrote:
>
> Hi all!
>
>
>> From: Marek Czerski <m.czerski@xxxxxxxxxx>
>>
>> setbrg callback (set baudrate) is needed by the loadx/loady commands.
>> Because litex serial has fixed baudrate the callback only checks if
>> the requested baudrate is the same as the CONFIG_BAUDRATE.
>> ---
>> drivers/serial/serial_litex.c | 9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/serial/serial_litex.c b/drivers/serial/serial_litex.c
>> index 8562a45ecc..9d35a6e44a 100644
>> --- a/drivers/serial/serial_litex.c
>> +++ b/drivers/serial/serial_litex.c
>> @@ -58,6 +58,13 @@ static int litex_serial_tstc(struct console_device *cdev)
>> return !litex_serial_readb(cdev, UART_RXEMPTY);
>> }
>>
>> +static int litex_setial_setbaudrate(struct console_device *cdev, int baudrate)
>> +{
>> + if (baudrate != CONFIG_BAUDRATE)
>> + return -EINVAL;
>> + return 0;
>> +}
>> +
>
> I have sent this patch separately because it need special attention.
>
> LiteX serial port hardware has fixed baudrate and setbaudrate has no sence.
> On the other hand absent setnbaudrate() litex serial makes impossible
> to use Y-modem data trasfer.
>
> I don't like CONFIG_BAUDRATE here. Can we use
>
> if (baudrate != cdev->baudrate)
> return -EINVAL;
> return 0;
>
> instead?
>
> Please comment!
There are other consoles (efi, linux, virtio) that don't support baud rate
setting either, so I think it makes sense to solve this outside of the
individual console drivers.
One way to go about it is move
if (cdev->baudrate == baudrate)
return 0;
in common/console.c up. Then setting baud rate to the preconfigured value
would succeed everywhere.
> To: Ahmad
> It looks like CONFIG_BAUDRATE is a one more global defconfig parameter
> that complicates "one defconfig for all RISC-V boards" approach.
It's not a deal breaker, users can still use the environment for fine-grained
configuration of the baud rate on a per board basis if they indeed want to
ship the same barebox configuration for LiteX and something else.
The idea of one-defconfig-for-all is that we don't introduce unneeded mutually-
exclusive configurations. This is not the case here.
(I know I still owe you some patches, I will try to get those sent out soon-ish)
Cheers,
Ahmad
>
> P.S. There is typo in litex_seTial_setbaudrate name. I have noted just now.
> It should be fixed of cause.
>
>
>> static int litex_serial_probe(struct device_d *dev)
>> {
>> struct resource *iores;
>> @@ -73,7 +80,7 @@ static int litex_serial_probe(struct device_d *dev)
>> cdev->tstc = &litex_serial_tstc;
>> cdev->putc = &litex_serial_putc;
>> cdev->getc = &litex_serial_getc;
>> - cdev->setbrg = NULL;
>> + cdev->setbrg = &litex_setial_setbaudrate;
>>
>> console_register(cdev);
>>
>> --
>> 2.31.1
>>
>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
