mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Ahmad Fatoum <a.fatoum@pengutronix.de>
To: Antony Pavlov <antonynpavlov@gmail.com>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Marek Czerski <m.czerski@ap-tech.pl>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH v3 05/10] litex serial: add setbrg callback
Date: Tue, 25 May 2021 14:48:52 +0200	[thread overview]
Message-ID: <2cea0c51-5979-a0b6-0f93-2b19eec1af4e@pengutronix.de> (raw)
In-Reply-To: <20210525103647.b4c08389183128e7abea97a0@gmail.com>

Hello Antony,

On 25.05.21 09:36, Antony Pavlov wrote:
> On Tue, 25 May 2021 10:19:47 +0300
> Antony Pavlov <antonynpavlov@gmail.com> wrote:
> 
> Hi all!
> 
> 
>> From: Marek Czerski <m.czerski@ap-tech.pl>
>>
>> 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@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


  reply	other threads:[~2021-05-25 12:51 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-25  7:19 [PATCH v3 00/10] RISC-V: add LiteX SoC support; resurrect nmon Antony Pavlov
2021-05-25  7:19 ` [PATCH v3 01/10] clocksource: timer-riscv: select CSR from device tree Antony Pavlov
2021-05-25 12:54   ` Ahmad Fatoum
2021-05-25  7:19 ` [PATCH v3 02/10] RISC-V: make it possible to run nmon from PBL C code Antony Pavlov
2021-05-25  7:19 ` [PATCH v3 03/10] RISC-V: boards: erizo: make it possible to use nmon Antony Pavlov
2021-05-25  7:19 ` [PATCH v3 04/10] serial: add litex UART driver Antony Pavlov
2021-05-25  7:19 ` [PATCH v3 05/10] litex serial: add setbrg callback Antony Pavlov
2021-05-25  7:36   ` Antony Pavlov
2021-05-25 12:48     ` Ahmad Fatoum [this message]
2021-05-25  7:19 ` [PATCH v3 06/10] gpio: add driver for 74xx-ICs with MMIO access Antony Pavlov
2021-05-25  7:19 ` [PATCH v3 07/10] spi: add litex spiflash driver Antony Pavlov
2021-05-25  7:19 ` [PATCH v3 08/10] net: add LiteEth driver Antony Pavlov
2021-05-25  7:19 ` [PATCH v3 09/10] RISC-V: add LiteX SoC and linux-on-litex-vexriscv support Antony Pavlov
2021-05-25  7:47   ` Jan Lübbe
2021-05-25  8:52     ` Antony Pavlov
2021-05-25  7:19 ` [PATCH v3 10/10] RISC-V: add litex_linux_defconfig Antony Pavlov
2021-08-11  8:52 ` [PATCH v3 00/10] RISC-V: add LiteX SoC support; resurrect nmon Ahmad Fatoum
2021-08-17 10:16   ` Antony Pavlov
2021-08-17 10:20     ` Ahmad Fatoum

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2cea0c51-5979-a0b6-0f93-2b19eec1af4e@pengutronix.de \
    --to=a.fatoum@pengutronix.de \
    --cc=antonynpavlov@gmail.com \
    --cc=barebox@lists.infradead.org \
    --cc=m.czerski@ap-tech.pl \
    --cc=s.hauer@pengutronix.de \
    --subject='Re: [PATCH v3 05/10] litex serial: add setbrg callback' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox