public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Lukasz Majewski <lukma@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] dm: serial: Add .pre_probe() function to to wait for previous transmission end
Date: Fri, 27 Oct 2017 00:51:03 +0200	[thread overview]
Message-ID: <20171027005103.17556ab7@jawa> (raw)
In-Reply-To: <D6494961-F7B9-4BB8-A7E0-38D54D1E110F@theobroma-systems.com>

Hi Philipp,

> 
> > On 27 Oct 2017, at 00:13, Lukasz Majewski <lukma@denx.de> wrote:
> > 
> > It may happen that the serial IP block is performing some ongoing
> > transmission (started at e.g. board_init()) when the serial "probe"
> > is called.
> > 
> > As a result the serial port IP block is reset, so transmitted data
> > is corrupted:
> > 
> >    I2C:   ready
> >    DRAM:  1 GiB
> >    jSS('HH��SL_SDHC: 04 rev 0x0
> > 
> > This patch prevents from this situation, by defining pre_probe()
> > callback in which we wait till the TX buffer is empty (from
> > previous transmission):
> > 
> >    I2C:   ready
> >    DRAM:  1 GiB
> >    ID:    unit type 0x4 rev 0x0
> > 
> > All defined ->pending callbacks at ./drivers/serial are non blocking
> > - just simple reading from registers and testing flags. Hence, it
> > should be enough to not use any timeout from timer.
> > One shall also note that we enable console very early - not all
> > timers may be ready for work - adding timeout here would impose
> > implicit dependency that timers are setup before serial.
> 
> Given that this is effectively a busy polling loop, why can’t this be
> done from the probe-function of serial drivers that require this
> functionality?

That would be one of the options. 

Originally, this code was placed at iMX specific function - namely
_mxc_serial_init() [1]. 

However, Simon suggested to solve this problem globally via DM's
serial-uclass, which offers pre_probe() callback for such purpose.

The problem here is the polling loop. I've checked and ->pending on
real SoCs is just a read from the register - which should not block (a
similar approach is used in Linux kernel).

Having timeout from timer would impose dependency on timer init'ed
first - before serial.

[1] - https://patchwork.ozlabs.org/patch/820824/

> 
> > 
> > 
> > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> > ---
> > 
> > drivers/serial/serial-uclass.c | 20 ++++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> > 
> > diff --git a/drivers/serial/serial-uclass.c
> > b/drivers/serial/serial-uclass.c index 2e5116f..5e6964d 100644
> > --- a/drivers/serial/serial-uclass.c
> > +++ b/drivers/serial/serial-uclass.c
> > @@ -420,10 +420,30 @@ static int serial_pre_remove(struct udevice
> > *dev) return 0;
> > }
> > 
> > +static int serial_pre_probe(struct udevice *dev)
> > +{
> > +	struct dm_serial_ops *ops = serial_get_ops(dev);
> > +	int ret = 0;
> > +
> > +	/*
> > +	 * Wait for any ongoing transmission to finish - for
> > example
> > +	 * from pre-relocation enabled UART
> > +	 */
> > +	if (ops && ops->pending)
> > +		do {
> > +			ret = ops->pending(dev, false);
> > +			if (ret < 0)
> > +				break;
> > +		} while (ret > 0);
> > +
> > +	return ret;
> > +}
> > +
> > UCLASS_DRIVER(serial) = {
> > 	.id		= UCLASS_SERIAL,
> > 	.name		= "serial",
> > 	.flags		= DM_UC_FLAG_SEQ_ALIAS,
> > +	.pre_probe	= serial_pre_probe,
> > 	.post_probe	= serial_post_probe,
> > 	.pre_remove	= serial_pre_remove,
> > 	.per_device_auto_alloc_size = sizeof(struct
> > serial_dev_priv), -- 
> > 2.1.4
> > 
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

  reply	other threads:[~2017-10-26 22:51 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-26 22:13 [U-Boot] [PATCH] dm: serial: Add .pre_probe() function to to wait for previous transmission end Lukasz Majewski
2017-10-26 22:19 ` Dr. Philipp Tomsich
2017-10-26 22:51   ` Lukasz Majewski [this message]
2017-10-27 13:34     ` Dr. Philipp Tomsich
2017-10-27 13:57       ` Lukasz Majewski
2017-11-04 21:42         ` Lukasz Majewski
2017-11-05  2:45           ` Simon Glass
2017-11-05 22:16             ` Lukasz Majewski

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=20171027005103.17556ab7@jawa \
    --to=lukma@denx.de \
    --cc=u-boot@lists.denx.de \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox