From: Matthias Kaehlcke <mka@chromium.org>
To: Balakrishna Godavarthi <bgodavar@codeaurora.org>
Cc: Johan Hovold <johan@kernel.org>,
marcel@holtmann.org, johan.hedberg@gmail.com,
linux-kernel@vger.kernel.org, linux-bluetooth@vger.kernel.org,
hemantg@codeaurora.org, linux-arm-msm@vger.kernel.org,
Johan Hovold <jhovold@gmail.com>
Subject: Re: [PATCH v5 2/5] Bluetooth: hci_qca: Deassert RTS while baudrate change command
Date: Thu, 10 Jan 2019 17:37:07 -0800 [thread overview]
Message-ID: <20190111013707.GD261387@google.com> (raw)
In-Reply-To: <61301df80bd6a8ee0265b31b7f6a3aa1@codeaurora.org>
On Thu, Jan 10, 2019 at 08:22:12PM +0530, Balakrishna Godavarthi wrote:
> Hi Johan,
>
> On 2019-01-10 20:09, Johan Hovold wrote:
> > On Thu, Jan 10, 2019 at 08:04:12PM +0530, Balakrishna Godavarthi wrote:
> > > Hi Johan,
> > >
> > > On 2019-01-09 20:22, Johan Hovold wrote:
> > > > On Thu, Dec 20, 2018 at 08:16:36PM +0530, Balakrishna Godavarthi wrote:
> > > >> This patch will help to stop frame reassembly errors while changing
> > > >> the baudrate. This is because host send a change baudrate request
> > > >> command to the chip with 115200 bps, Whereas chip will change their
> > > >> UART clocks to the enable for new baudrate and sends the response
> > > >> for the change request command with newer baudrate, On host side
> > > >> we are still operating in 115200 bps which results of reading garbage
> > > >> data. Here we are pulling RTS line, so that chip we will wait to send
> > > >> data
> > > >> to host until host change its baudrate.
> >
> > > >> + /* Deassert RTS while changing the baudrate of chip and host.
> > > >> + * This will prevent chip from transmitting its response with
> > > >> + * the new baudrate while the host port is still operating at
> > > >> + * the old speed.
> > > >> + */
> > > >> + qcadev = serdev_device_get_drvdata(hu->serdev);
> > > >> + if (qcadev->btsoc_type == QCA_WCN3990)
> > > >> + serdev_device_set_rts(hu->serdev, false);
> > > >> +
> > > >
> > > > This may not do what you want unless you also disable hardware flow
> > > > control.
> >
> > > Here my requirement here is to block the chip to send its data before
> > > HOST changes it is baudrate. So if i disable flow control lines of
> > > HOST which will be in low state. so that the chip will send it data
> > > before HOST change the baudrate of HOST. which results in frame
> > > reassembly error.
> >
> > Not sure I understand what you're trying to say above. My point is that
> > you cannot reliable control RTS when you have automatic flow control
> > enabled (i.e. it is managed by hardware and it's state reflects whether
> > there's room in the UART receive FIFO).
> >
> > Johan
>
> [Bala]: Yes i got your point, but our driver
I suppose with "our driver" you refer to a Qualcomm UART driver like
qcom_geni_serial.c. Unless the Bluetooth controller is really tied to
some specific SoC (e.g. because it is on-chip) you shouldn't make
assumptions about the UART driver or hardware beyond standard
behavior.
But even if we assume that the driver you mention is used, I think you
are rather confirming Johan's concern than dispersing it:
> will not support automatic flow control (based on the FIFO status)
> unless we explicitly enabled via software. i.e. if we enable the
> flow, hardware will look for it else it will not looks for CTS or
> RTS Line.
So we agree that the UART hardware may change RTS if hardware flow
control is enabled?
static int qca_send_power_pulse(struct hci_uart *hu, u8 cmd)
{
...
hci_uart_set_flow_control(hu, false);
...
}
I still find it utterly confusing that set_flow_control(false) enables
flow control, but that's what it does, hence after
qca_send_power_pulse() flow control is (re-)enabled.
So far I haven't seen problems with qcom_geni_serial.c overriding the
level set with serdev_device_set_rts(), but I tend to agree with Johan
that this could be a problem (if not with this UART (driver) then with
another). I'm not keen about adding more flow control on/off clutter,
but if that is needed for the driver to operate reliably across
platforms so be it.
Cheers
Matthias
next prev parent reply other threads:[~2019-01-11 1:37 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-20 14:46 [PATCH v5 0/5] Bug fixes for Qualcomm BT chip wcn3990 Balakrishna Godavarthi
2018-12-20 14:46 ` [PATCH v5 1/5] Bluetooth: hci_qca: use wait_until_sent() for power pulses Balakrishna Godavarthi
2018-12-22 1:59 ` Matthias Kaehlcke
2018-12-26 6:31 ` Balakrishna Godavarthi
2018-12-26 22:21 ` Matthias Kaehlcke
2018-12-27 3:23 ` Balakrishna Godavarthi
2019-01-09 14:38 ` Johan Hovold
2019-01-10 14:48 ` Balakrishna Godavarthi
2019-01-11 0:55 ` Matthias Kaehlcke
2019-01-11 14:32 ` Balakrishna Godavarthi
2019-01-11 23:38 ` Matthias Kaehlcke
2019-01-14 10:25 ` Balakrishna Godavarthi
2018-12-20 14:46 ` [PATCH v5 2/5] Bluetooth: hci_qca: Deassert RTS while baudrate change command Balakrishna Godavarthi
2018-12-22 0:31 ` Matthias Kaehlcke
2018-12-26 5:45 ` Balakrishna Godavarthi
2018-12-26 20:25 ` Matthias Kaehlcke
2018-12-27 3:20 ` Balakrishna Godavarthi
2019-01-09 14:52 ` Johan Hovold
2019-01-10 14:34 ` Balakrishna Godavarthi
2019-01-10 14:39 ` Johan Hovold
2019-01-10 14:52 ` Balakrishna Godavarthi
2019-01-11 1:37 ` Matthias Kaehlcke [this message]
2019-01-11 15:07 ` Balakrishna Godavarthi
2019-01-11 23:56 ` Matthias Kaehlcke
2019-01-14 14:52 ` Balakrishna Godavarthi
2019-01-15 23:46 ` Matthias Kaehlcke
2019-01-17 16:09 ` Johan Hovold
2019-01-17 17:21 ` Matthias Kaehlcke
2019-01-18 9:44 ` Johan Hovold
2019-01-19 0:31 ` Matthias Kaehlcke
2019-01-21 8:56 ` Johan Hovold
2019-01-22 21:39 ` Matthias Kaehlcke
2019-01-21 14:41 ` Balakrishna Godavarthi
2019-01-22 21:53 ` Matthias Kaehlcke
2019-01-23 12:57 ` Balakrishna Godavarthi
2018-12-20 14:46 ` [PATCH v5 3/5] Bluetooth: hci_qca: Fix frame reassembly errors for wcn3990 Balakrishna Godavarthi
2018-12-20 14:46 ` [PATCH v5 4/5] Bluetooth: hci_qca: Disable IBS state machine and flush Tx buffer Balakrishna Godavarthi
2018-12-20 14:46 ` [PATCH v5 5/5] Bluetooth: btqca: inject command complete event during fw download Balakrishna Godavarthi
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=20190111013707.GD261387@google.com \
--to=mka@chromium.org \
--cc=bgodavar@codeaurora.org \
--cc=hemantg@codeaurora.org \
--cc=jhovold@gmail.com \
--cc=johan.hedberg@gmail.com \
--cc=johan@kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-bluetooth@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marcel@holtmann.org \
/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;
as well as URLs for NNTP newsgroup(s).