* Re: [PATCH] serial: 8250_dw: Minor code cleanup
From: Olliver Schinagl @ 2017-03-29 17:10 UTC (permalink / raw)
To: Doug Anderson
Cc: Greg Kroah-Hartman, Jiri Slaby, Kefeng Wang, Andy Shevchenko,
Heikki Krogerus, Jason Uy, Heiko Stuebner, Ed Blake,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
dev-3kdeTeqwOZ9EV1b7eY7vFQ
In-Reply-To: <CAD=FV=XZFuZ-Zqv75wF7wj6AY35o9n4-zrbsVjNcFZ+DxS=fbQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hey Doug,
On 29-03-17 17:50, Doug Anderson wrote:
> Hi,
>
> On Wed, Mar 29, 2017 at 3:04 AM, Olliver Schinagl <oliver-dxLnbx3+1qmEVqv0pETR8A@public.gmane.org> wrote:
>> Commit 424d79183af0 ("serial: 8250_dw: Avoid "too much work" from bogus rx timeout interrupt")
>> added a bit check with quite a wide mask. To be concise with the other
>> similar calls in this driver, change it to mask against the flag we want to
>> check only. This thus removes a magic value/mask.
>
> How certain are you that your patch is correct? You are now basically
> checking to see if the bits "0xc" are set in the IIR. Previously the
> patch ensured that the bits 0x33 were clear.
You raise a good point. And after writing two replies that made perfect
sense, I just realized looking at the table you wisely posted below, I
should have spotted that the interrupts are not bits! So very good catch
and my bad indeed.
>
> Have you tried looking through the kernel for other places where
> UART_IIR_RX_TIMEOUT is referenced? In 8250_omap.c and 8250_port.c I
> believe you'll find it masking against 0x3f. In omap-serial.c you'll
> see a mask against 0x3e.
>
> Looking at the TRM for rk3399, I see that bits 4 and 5 (bitmask 0x30)
> as "reserved". I see the following definitions for bits 3:0:
>
> 0000 = modem status
> 0001 = no interrupt pending
> 0010 = THR empty
> 0100 = received data available
> 0110 = receiver line status
> 0111 = busy detect
> 1100 = character timeout
>
> ...so while your patch will probably function OK, it would also
> function equally well to simply test bit 3 (0x80) and ignore
> everything else. ...but IMHO it is more correct to at least mask the
> IIR with 0x0F and confirm that bits 2 and 3 are set and bits 0 and 1
> are zero. ...and since the main 8250 code uses 0x3f, that seems even
> better to me (despite the fact that it seems to be relying on the fact
> that the "reserved" bits come back as 0).
I strongly agree with you here, I did it wrong, but 0x3f really is wrong
too imo. The bits to look at are 3:0, bits 4:5 are reserved and we
should never look at those, as as you rightfully put it are being relied
on to be 0 (which may always be the case) but imo is still wrong and
thus the mask should be 0x0f.
Going to the horse's mouth [0] which is the documentation of the IP
block used in all these designs, they also say the same thing. 4 bits
and while I don't have any of the other datasheets of other 8250 cores,
I bet they are the same?
And then, the following is actually wrong on the same grounds, from the
8250_dw.c
if ((iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
This just happens to work as it is the only way this can match, but
clearly it is wrong then, right?
I'll check against the same mask of 0x0f here as well.
>
>
> If you want to make a fix, I'd suggest adding a #define for 0x3f and
> using it in various places.
Yeah I'll do that, add the define next to the others for 0x0f, unless
someone says it is a good idea (or needed idea) to mask against the
reserved bits as well.
>
>
>> Some very minor code cleanups, such as including the bitops header for
>> DW_UART_MCR_SIRE, use the BIT() macro as suggested by checkpatc and
>> removed a whitespace to match other invocations.
>
> Maybe it's just me, but it seems like a bad idea to combine these
> cleanups in the same patch with a functional change..
Well with the oversight of the bit check above, it becomes obvious that
it is a bigger change indeed. I will change it!
Olliver
>
[0] http://linux-sunxi.org/images/d/d2/Dw_apb_uart_db.pdf
^ permalink raw reply
* Re: [PATCH 1/1] tty: serial: st-asc: Make the locking RT aware
From: Sebastian Andrzej Siewior @ 2017-03-29 16:10 UTC (permalink / raw)
To: Lionel Debieve
Cc: Patrice Chotard, Greg Kroah-Hartman, Jiri Slaby, linux-serial,
linux-kernel, linux-arm-kernel, linux-rt-users
In-Reply-To: <1490202751-16581-1-git-send-email-lionel.debieve@st.com>
On 2017-03-22 18:12:31 [+0100], Lionel Debieve wrote:
> The lock is a sleeping lock and local_irq_save() is not the
> standard implementation now. Working for both -RT and non
> RT.
>
> Signed-off-by: Lionel Debieve <lionel.debieve@st.com>
Acked-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
This is how serial8250_console_write() looks upstream, too.
Sebastian
^ permalink raw reply
* Re: [PATCH] serial: 8250_dw: Minor code cleanup
From: Doug Anderson @ 2017-03-29 15:50 UTC (permalink / raw)
To: Olliver Schinagl
Cc: Greg Kroah-Hartman, Jiri Slaby, Kefeng Wang, Andy Shevchenko,
Heikki Krogerus, Jason Uy, Heiko Stuebner, Ed Blake,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
dev-3kdeTeqwOZ9EV1b7eY7vFQ
In-Reply-To: <20170329100424.7439-1-oliver-dxLnbx3+1qmEVqv0pETR8A@public.gmane.org>
Hi,
On Wed, Mar 29, 2017 at 3:04 AM, Olliver Schinagl <oliver-dxLnbx3+1qmEVqv0pETR8A@public.gmane.org> wrote:
> Commit 424d79183af0 ("serial: 8250_dw: Avoid "too much work" from bogus rx timeout interrupt")
> added a bit check with quite a wide mask. To be concise with the other
> similar calls in this driver, change it to mask against the flag we want to
> check only. This thus removes a magic value/mask.
How certain are you that your patch is correct? You are now basically
checking to see if the bits "0xc" are set in the IIR. Previously the
patch ensured that the bits 0x33 were clear.
Have you tried looking through the kernel for other places where
UART_IIR_RX_TIMEOUT is referenced? In 8250_omap.c and 8250_port.c I
believe you'll find it masking against 0x3f. In omap-serial.c you'll
see a mask against 0x3e.
Looking at the TRM for rk3399, I see that bits 4 and 5 (bitmask 0x30)
as "reserved". I see the following definitions for bits 3:0:
0000 = modem status
0001 = no interrupt pending
0010 = THR empty
0100 = received data available
0110 = receiver line status
0111 = busy detect
1100 = character timeout
...so while your patch will probably function OK, it would also
function equally well to simply test bit 3 (0x80) and ignore
everything else. ...but IMHO it is more correct to at least mask the
IIR with 0x0F and confirm that bits 2 and 3 are set and bits 0 and 1
are zero. ...and since the main 8250 code uses 0x3f, that seems even
better to me (despite the fact that it seems to be relying on the fact
that the "reserved" bits come back as 0).
If you want to make a fix, I'd suggest adding a #define for 0x3f and
using it in various places.
> Some very minor code cleanups, such as including the bitops header for
> DW_UART_MCR_SIRE, use the BIT() macro as suggested by checkpatc and
> removed a whitespace to match other invocations.
Maybe it's just me, but it seems like a bad idea to combine these
cleanups in the same patch with a functional change..
^ permalink raw reply
* Re: [PATCH v2] serdev: Replace serdev_device_write_buf with serdev_device_write
From: Andy Shevchenko @ 2017-03-29 14:43 UTC (permalink / raw)
To: Andrey Smirnov
Cc: Rob Herring, Chris Healy, Guenter Roeck,
linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CAHQ1cqG8jD-aaoD2=jqierhx5+WfgqGQiO9b8qz2uzJyWxTPdQ@mail.gmail.com>
On Wed, Mar 29, 2017 at 5:16 PM, Andrey Smirnov
<andrew.smirnov@gmail.com> wrote:
> On Tue, Mar 28, 2017 at 10:07 AM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Tue, Mar 28, 2017 at 7:01 PM, Andrey Smirnov
>> <andrew.smirnov@gmail.com> wrote:
>> So, what I would see if no one objects is patch series of two:
>> 1) introduction of new API
>> 2) removing old one.
>>
>> It will benefit for easier review and any potential code anthropologist.
>>
>
> Second version of the patch preserves the old API an just
> re-implements it in terms of a new one. I am not sure I see the
> benefit in splitting it into two patches, but I'll leave it up to Rob
> to decide.
Sure. At least I posted benefits I see from splitting.
+ bisectability (in case we have to revert your new API by some reason
it will be easier, hope will be not the case, though...)
>>> + } while (count &&
>>> + (timeout = wait_for_completion_timeout(&serdev->write_comp,
>>> + timeout)));
>>
>> So, would it be better to support interrupts here and return a
>> corresponding error code to the user?
>>
>
> I don't have a use-case for that and as far as I can tell, neither SPI
> nor I2C slave device API offer such functionality universally, so I am
> inclined to say no. Since the change from wait_for_completion to
> wait_for_completion_timeout was made per Rob's request, I'd leave it
> up to him to decided about this change as well.
OK.
>> Besides that question, readability might be better if you use
>> temporary variable and pack above on one line:
>>
>> unsigned long to = timeout;
>>
>> } while (count && (to = ...(to)));
>>
>
> Even if you shorten 'timeout' to 'to', formatted as a single line,
> it'd still exceed line length limitations.
How many? If we are talking about 2-3 characters, that's okay to leave
them on one line.
>>> + * @write_lock Mutext used to esure exclusive access to the bus when
>>> + * writing data with serdev_device_write()
>>
>> checkpatch.pl has integrated spellchecker AFAIU.
>
> My bad, forgot to enable it as a git hook, will fix.
>
>> Moreover, can you try harder to make that description shorter?
>>
>
> I am all ears for suggestions alternative phrasing, otherwise, no,
> that's about as hard as I try.
First of all, "used to" is (closer) equivalent to was.
Second, Mutex is one letter longer than Lock (here is important that
is just a kind of lock).
Third, "exclusive" is implied by Mutex / Lock word.
Fourth, "access to the bus when writing data" too verbose.
So, my suggestion is (two variants):
a) "Lock to serialize bus access when writing data."
b) "Lock to serialize access when writing data with serdev_device_write()."
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2] serdev: Replace serdev_device_write_buf with serdev_device_write
From: Andrey Smirnov @ 2017-03-29 14:16 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Rob Herring, Chris Healy, Guenter Roeck,
linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CAHp75VdHWE6jK12Q=2wGV-J5eCYWiAWnLFwz2r8AcJJQvAaztA@mail.gmail.com>
On Tue, Mar 28, 2017 at 10:07 AM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Tue, Mar 28, 2017 at 7:01 PM, Andrey Smirnov
> <andrew.smirnov@gmail.com> wrote:
>> Convert serdev_device_write_buf's code to be able to transfer amount of
>> data potentially exceeding "write room" at the moment of invocation.
>>
>> To support that, also add serdev_device_write_wakeup.
>>
>> Drivers wanting to use full extent of serdev_device_write
>> functionality are expected to provide serdev_device_write_wakeup as a
>> sole handler of .write_wakeup event or call it as a part of driver's
>> custom .write_wakeup code.
>>
>> Drivers wanting to retain old serdev_device_write_buf behaviour can
>
>> replace those call to calls to serdev_device_write with timeout of
>> 0. Providing .write_wakeup handler in such case is optional.
>
> Some indentation would be better if, for example, 0 will be kept on
> previous line.
>
OK, sure.
> So, what I would see if no one objects is patch series of two:
> 1) introduction of new API
> 2) removing old one.
>
> It will benefit for easier review and any potential code anthropologist.
>
Second version of the patch preserves the old API an just
re-implements it in terms of a new one. I am not sure I see the
benefit in splitting it into two patches, but I'll leave it up to Rob
to decide.
>> --- a/drivers/tty/serdev/core.c
>> +++ b/drivers/tty/serdev/core.c
>> @@ -116,17 +116,41 @@ void serdev_device_close(struct serdev_device *serdev)
>> }
>> EXPORT_SYMBOL_GPL(serdev_device_close);
>>
>> -int serdev_device_write_buf(struct serdev_device *serdev,
>> - const unsigned char *buf, size_t count)
>> +void serdev_device_write_wakeup(struct serdev_device *serdev)
>> +{
>> + complete(&serdev->write_comp);
>> +}
>> +EXPORT_SYMBOL_GPL(serdev_device_write_wakeup);
>> +
>> +int serdev_device_write(struct serdev_device *serdev,
>> + const unsigned char *buf, size_t count,
>> + unsigned long timeout)
>> {
>> struct serdev_controller *ctrl = serdev->ctrl;
>> + int ret;
>>
>> - if (!ctrl || !ctrl->ops->write_buf)
>> + if (!ctrl || !ctrl->ops->write_buf ||
>> + (timeout && !serdev->ops->write_wakeup))
>> return -EINVAL;
>>
>> - return ctrl->ops->write_buf(ctrl, buf, count);
>> + mutex_lock(&serdev->write_lock);
>> + do {
>> + reinit_completion(&serdev->write_comp);
>> +
>> + ret = ctrl->ops->write_buf(ctrl, buf, count);
>> + if (ret < 0)
>> + break;
>> +
>
>> + buf += ret;
>
> Extra white spaces.
Which is there on purpose to re-align "+=" with "-=" on the next line.
I'll remove it.
>
>> + count -= ret;
>> +
>
>> + } while (count &&
>> + (timeout = wait_for_completion_timeout(&serdev->write_comp,
>> + timeout)));
>
> So, would it be better to support interrupts here and return a
> corresponding error code to the user?
>
I don't have a use-case for that and as far as I can tell, neither SPI
nor I2C slave device API offer such functionality universally, so I am
inclined to say no. Since the change from wait_for_completion to
wait_for_completion_timeout was made per Rob's request, I'd leave it
up to him to decided about this change as well.
> Besides that question, readability might be better if you use
> temporary variable and pack above on one line:
>
> unsigned long to = timeout;
>
> } while (count && (to = ...(to)));
>
Even if you shorten 'timeout' to 'to', formatted as a single line,
it'd still exceed line length limitations.
>
>
>> + mutex_unlock(&serdev->write_lock);
>> + return ret < 0 ? ret : (count ? -ETIMEDOUT : 0);
>> }
>> -EXPORT_SYMBOL_GPL(serdev_device_write_buf);
>> +EXPORT_SYMBOL_GPL(serdev_device_write);
>
>> + * @write_comp Completion used by serdev_device_write internally
>
> Links to the functions are func()-like. Please check kernel doc howto:s.
OK, will do.
>
>> + * @write_lock Mutext used to esure exclusive access to the bus when
>> + * writing data with serdev_device_write()
>
> checkpatch.pl has integrated spellchecker AFAIU.
My bad, forgot to enable it as a git hook, will fix.
> Moreover, can you try harder to make that description shorter?
>
I am all ears for suggestions alternative phrasing, otherwise, no,
that's about as hard as I try.
>> void serdev_device_write_flush(struct serdev_device *);
>> int serdev_device_write_room(struct serdev_device *);
>>
>> +
>> /*
>> * serdev device driver functions
>> */
>
> This doesn't belong to the change.
Oops, didn't notice this. Will remove.
Thanks,
Andrey Smirnov
^ permalink raw reply
* [PATCH] serial: 8250_dw: Minor code cleanup
From: Olliver Schinagl @ 2017-03-29 10:04 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Kefeng Wang, Andy Shevchenko, Heikki Krogerus, Jason Uy,
Heiko Stuebner, Ed Blake, Douglas Anderson,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, dev-3kdeTeqwOZ9EV1b7eY7vFQ,
Olliver Schinagl
Commit 424d79183af0 ("serial: 8250_dw: Avoid "too much work" from bogus rx timeout interrupt")
added a bit check with quite a wide mask. To be concise with the other
similar calls in this driver, change it to mask against the flag we want to
check only. This thus removes a magic value/mask.
Some very minor code cleanups, such as including the bitops header for
DW_UART_MCR_SIRE, use the BIT() macro as suggested by checkpatc and
removed a whitespace to match other invocations.
Signed-off-by: Olliver Schinagl <oliver-dxLnbx3+1qmEVqv0pETR8A@public.gmane.org>
---
drivers/tty/serial/8250/8250_dw.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index e65808c482f1..49117bdc7b6a 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -13,6 +13,7 @@
* LCR is written whilst busy. If it is, then a busy detect interrupt is
* raised, the LCR needs to be rewritten and the uart status register read.
*/
+#include <linux/bitops.h>
#include <linux/device.h>
#include <linux/io.h>
#include <linux/module.h>
@@ -39,16 +40,16 @@
/* Component Parameter Register bits */
#define DW_UART_CPR_ABP_DATA_WIDTH (3 << 0)
-#define DW_UART_CPR_AFCE_MODE (1 << 4)
-#define DW_UART_CPR_THRE_MODE (1 << 5)
-#define DW_UART_CPR_SIR_MODE (1 << 6)
-#define DW_UART_CPR_SIR_LP_MODE (1 << 7)
-#define DW_UART_CPR_ADDITIONAL_FEATURES (1 << 8)
-#define DW_UART_CPR_FIFO_ACCESS (1 << 9)
-#define DW_UART_CPR_FIFO_STAT (1 << 10)
-#define DW_UART_CPR_SHADOW (1 << 11)
-#define DW_UART_CPR_ENCODED_PARMS (1 << 12)
-#define DW_UART_CPR_DMA_EXTRA (1 << 13)
+#define DW_UART_CPR_AFCE_MODE BIT(4)
+#define DW_UART_CPR_THRE_MODE BIT(5)
+#define DW_UART_CPR_SIR_MODE BIT(6)
+#define DW_UART_CPR_SIR_LP_MODE BIT(7)
+#define DW_UART_CPR_ADDITIONAL_FEATURES BIT(8)
+#define DW_UART_CPR_FIFO_ACCESS BIT(9)
+#define DW_UART_CPR_FIFO_STAT BIT(10)
+#define DW_UART_CPR_SHADOW BIT(11)
+#define DW_UART_CPR_ENCODED_PARMS BIT(12)
+#define DW_UART_CPR_DMA_EXTRA BIT(13)
#define DW_UART_CPR_FIFO_MODE (0xff << 16)
/* Helper for fifo size calculation */
#define DW_UART_CPR_FIFO_SIZE(a) (((a >> 16) & 0xff) * 16)
@@ -217,12 +218,12 @@ static int dw8250_handle_irq(struct uart_port *p)
* This problem has only been observed so far when not in DMA mode
* so we limit the workaround only to non-DMA mode.
*/
- if (!up->dma && ((iir & 0x3f) == UART_IIR_RX_TIMEOUT)) {
+ if (!up->dma && ((iir & UART_IIR_RX_TIMEOUT) == UART_IIR_RX_TIMEOUT)) {
spin_lock_irqsave(&p->lock, flags);
status = p->serial_in(p, UART_LSR);
if (!(status & (UART_LSR_DR | UART_LSR_BI)))
- (void) p->serial_in(p, UART_RX);
+ (void)p->serial_in(p, UART_RX);
spin_unlock_irqrestore(&p->lock, flags);
}
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v2, 3/3] tty/serial: meson_uart: add the core clock handling to the driver
From: Helmut Klein @ 2017-03-29 9:47 UTC (permalink / raw)
To: Jerome Brunet, gregkh, carlo, khilman
Cc: linux-amlogic, linux-arm-kernel, linux-serial, linux-kernel
In-Reply-To: <1490728562.20764.9.camel@baylibre.com>
On 28.03.2017 21:16, Jerome Brunet wrote:
> On Tue, 2017-03-28 at 11:25 +0200, Helmut Klein wrote:
>> This patch gets the core clock as provided by the DT and enables it.
>> The code was taken from Amlogic's serial driver, and was tested on my
>> board.
>>
>> Signed-off-by: Helmut Klein <hgkr.klein@gmail.com>
>> ---
>> drivers/tty/serial/meson_uart.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
>> index 60f16795d16b..cb99112288eb 100644
>> --- a/drivers/tty/serial/meson_uart.c
>> +++ b/drivers/tty/serial/meson_uart.c
>> @@ -600,6 +600,7 @@ static int meson_uart_probe(struct platform_device *pdev)
>> struct resource *res_mem, *res_irq;
>> struct uart_port *port;
>> struct clk *clk;
>> + struct clk *core_clk;
>> int ret = 0;
>>
>> if (pdev->dev.of_node)
>> @@ -625,6 +626,15 @@ static int meson_uart_probe(struct platform_device *pdev)
>> if (!port)
>> return -ENOMEM;
>>
>> + core_clk = devm_clk_get(&pdev->dev, "core");
>> + if (!IS_ERR(core_clk)) {
>> + ret = clk_prepare_enable(core_clk);
>> + if (ret) {
>> + dev_err(&pdev->dev, "couldn't enable clkc\n");
>> + return ret;
>> + }
>> + }
>> +
>> clk = clk_get(&pdev->dev, NULL);
> Now that you have 2 clocks, shouldn't this be named as well ?
>
if i change the call to "clk = clk_get(&pdev->dev, "xtal");", then IMHO
i must add the
"clock-names" parameter (clock-names = "xtal") to the sections of the 2
AO-UARTs
(uart_AO & uart_AO_B).
Is this ok? (or is my assumption wrong?)
I'm going to add a patch meson-gx.dtsi anyway, so 2 lines more to add,
isn't a big deal.
Helmut
>> if (IS_ERR(clk))
>> return PTR_ERR(clk);
>> --
>> 2.11.0
>>
>>
>> _______________________________________________
>> linux-amlogic mailing list
>> linux-amlogic@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-amlogic
>
^ permalink raw reply
* Re: [PATCH v2] serial: 8250_dw: Avoid "too much work" from bogus rx timeout interrupt
From: Olliver Schinagl @ 2017-03-29 9:45 UTC (permalink / raw)
To: Andy Shevchenko, Douglas Anderson, Cal Sullivan
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
guennadi.liakhovetski-ral2JQCrhuEAvxtiuMwx3w, jslaby-IBi9RG/b67k,
Jeffy Chen, eric.gao-TNX95d0MmH7DzftRWevZcw,
briannorris-F7+t8E8rja9g9hUCZPvPmw, dev-3kdeTeqwOZ9EV1b7eY7vFQ
In-Reply-To: <CAHp75VeND-85ze-zPqz3=8qfSQasK1LmLxcfC=_R1KvN-S7C+A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hey Andy,
On 29-03-17 11:11, Andy Shevchenko wrote:
> On Wed, Mar 29, 2017 at 10:58 AM, Olliver Schinagl <oliver-dxLnbx3+1qmEVqv0pETR8A@public.gmane.org> wrote:
>> On 07-02-17 00:30, Douglas Anderson wrote:
>
> First of all I didn't get why people from Cc list are suddenly
> disappeared. Check your mail client settings.
> Returning back some of them.
Appologies, I replied via gmane's news feed to Douglas's initial post as
I did not have the original post and I failed to check the other
recipients. My fault. Sorry. I've added the original others as well.
>
>>> It appears that somehow we have a RX Timeout interrupt but there is no
>>> actual data present to receive. When we're in this state the UART
>>> driver claims that it handled the interrupt but it actually doesn't
>>> really do anything. This means that we keep getting the interrupt
>>> over and over again.
>
>> I may be running into the same thing on an A20 SoC, but still in the stage
>> of figuring out what is going on, as we get this error very occasionally. Do
>> you have a way to externally induce this behavior other then suspend/resume?
>> As we get it during uart-use and do not have (or I have never tried)
>> suspend/resume on our platform.
>
> On Intel platforms with this IP I can see similar when run loopback
> test on high speeds.
> California may correct me since he did a lot of investigation of the
> issue on x86.
>
>>> static int dw8250_handle_irq(struct uart_port *p)
>>> {
>>> + struct uart_8250_port *up = up_to_u8250p(p);
>>> struct dw8250_data *d = p->private_data;
>>> unsigned int iir = p->serial_in(p, UART_IIR);
>>> + unsigned int status;
>>> + unsigned long flags;
>>> +
>>> + /*
>>> + * There are ways to get Designware-based UARTs into a state where
>>> + * they are asserting UART_IIR_RX_TIMEOUT but there is no actual
>>> + * data available. If we see such a case then we'll do a bogus
>>> + * read. If we don't do this then the "RX TIMEOUT" interrupt will
>>> + * fire forever.
>>
>> I think what you are saying is 'do a bogus read as that is the only way to
>> clear the interrupt, otherwise it will keep firing forever.'?
>
> No, we don't know if this _the only way_. It looks like no one from us
> can tell you a root cause, except may be Synopsys guys.
Has anybody tried to contact synopsis/dw about this issue at all?
true, it is not the only way (maybe only as far as we know for now) but
it is 'the' way currently.
>
>>> + spin_lock_irqsave(&p->lock, flags);
>>
>> this is a bit above my knowledge of driver etc, but I don't any spinlocks in
>> the 8250 handle_irq glue drivers, except in the OMAP's case where they are
>> handeling a DMA IRQ. So I ask, because I don't know, why is it needed here?
>
> They serialize IO accessors.
>
> Regarding to the rest comments, the patch is already in upstream, if
> you feel that something should be changed, send an incremental fix.
Ah, I thought I checked, but thought I didn't see it. I'll probably
forgot to fetch. I'll send a patch for the small mask fix.
>
>> Once I found a way to reproduce the problem (without suspend) I will test
>> this to see if it fixes it for us too.
>
> It would be appreciated, but better to get know the root cause and
> what _hardware_ guys think about solutions.
>
I read over the docs of the IP block (I know a little FPGA programming)
(dw_apb_uart of 2006) but found nothing yet that would warn for this
behavior. I suppose hardware/fgpa guys can give more background here
potentially, but it may also be simply an IP bug?
Olliver
^ permalink raw reply
* Re: [PATCH v2] serial: 8250_dw: Avoid "too much work" from bogus rx timeout interrupt
From: Andy Shevchenko @ 2017-03-29 9:11 UTC (permalink / raw)
To: Olliver Schinagl, Douglas Anderson, Cal Sullivan
Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
guennadi.liakhovetski-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <obfpfg$h6e$1@blaine.gmane.org>
On Wed, Mar 29, 2017 at 10:58 AM, Olliver Schinagl <oliver-dxLnbx3+1qmEVqv0pETR8A@public.gmane.org> wrote:
> On 07-02-17 00:30, Douglas Anderson wrote:
First of all I didn't get why people from Cc list are suddenly
disappeared. Check your mail client settings.
Returning back some of them.
>> It appears that somehow we have a RX Timeout interrupt but there is no
>> actual data present to receive. When we're in this state the UART
>> driver claims that it handled the interrupt but it actually doesn't
>> really do anything. This means that we keep getting the interrupt
>> over and over again.
> I may be running into the same thing on an A20 SoC, but still in the stage
> of figuring out what is going on, as we get this error very occasionally. Do
> you have a way to externally induce this behavior other then suspend/resume?
> As we get it during uart-use and do not have (or I have never tried)
> suspend/resume on our platform.
On Intel platforms with this IP I can see similar when run loopback
test on high speeds.
California may correct me since he did a lot of investigation of the
issue on x86.
>> static int dw8250_handle_irq(struct uart_port *p)
>> {
>> + struct uart_8250_port *up = up_to_u8250p(p);
>> struct dw8250_data *d = p->private_data;
>> unsigned int iir = p->serial_in(p, UART_IIR);
>> + unsigned int status;
>> + unsigned long flags;
>> +
>> + /*
>> + * There are ways to get Designware-based UARTs into a state where
>> + * they are asserting UART_IIR_RX_TIMEOUT but there is no actual
>> + * data available. If we see such a case then we'll do a bogus
>> + * read. If we don't do this then the "RX TIMEOUT" interrupt will
>> + * fire forever.
>
> I think what you are saying is 'do a bogus read as that is the only way to
> clear the interrupt, otherwise it will keep firing forever.'?
No, we don't know if this _the only way_. It looks like no one from us
can tell you a root cause, except may be Synopsys guys.
>> + spin_lock_irqsave(&p->lock, flags);
>
> this is a bit above my knowledge of driver etc, but I don't any spinlocks in
> the 8250 handle_irq glue drivers, except in the OMAP's case where they are
> handeling a DMA IRQ. So I ask, because I don't know, why is it needed here?
They serialize IO accessors.
Regarding to the rest comments, the patch is already in upstream, if
you feel that something should be changed, send an incremental fix.
> Once I found a way to reproduce the problem (without suspend) I will test
> this to see if it fixes it for us too.
It would be appreciated, but better to get know the root cause and
what _hardware_ guys think about solutions.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2] serial: 8250_dw: Avoid "too much work" from bogus rx timeout interrupt
From: Olliver Schinagl @ 2017-03-29 7:58 UTC (permalink / raw)
To: linux-kernel; +Cc: linux-rockchip, linux-serial
In-Reply-To: <20170206233000.3021-1-dianders@chromium.org>
Hey Douglas,
On 07-02-17 00:30, Douglas Anderson wrote:
> On a Rockchip rk3399-based board during suspend/resume testing, we
> found that we could get the console UART into a state where it would
> print this to the console a lot:
> serial8250: too much work for irq42
>
> Followed eventually by:
> NMI watchdog: BUG: soft lockup - CPU#0 stuck for 11s!
>
> Upon debugging I found that we're in this state:
> iir = 0x000000cc
> lsr = 0x00000060
>
> It appears that somehow we have a RX Timeout interrupt but there is no
> actual data present to receive. When we're in this state the UART
> driver claims that it handled the interrupt but it actually doesn't
> really do anything. This means that we keep getting the interrupt
> over and over again.
I may be running into the same thing on an A20 SoC, but still in the
stage of figuring out what is going on, as we get this error very
occasionally. Do you have a way to externally induce this behavior other
then suspend/resume? As we get it during uart-use and do not have (or I
have never tried) suspend/resume on our platform.
>
> Normally we don't actually need to do anything special to handle a RX
> Timeout interrupt. We'll notice that there is some data ready and
> we'll read it, which will end up clearing the RX Timeout. In this
> case we have a problem specifically because we got the RX TImeout
> without any data. Reading a bogus byte is confirmed to get us out of
> this state.
>
> It's unclear how exactly the UART got into this state, but it is known
> that the UART lines are essentially undriven and unpowered during
> suspend, so possibly during resume some garbage / half transmitted
> bits are seen on the line and put the UART into this state.
>
> The UART on the rk3399 is a DesignWare based 8250 UART. From mailing
> list posts, it appears that other people have run into similar
> problems with DesignWare based IP. Presumably this problem is unique
> to that IP, so I have placed the workaround there to avoid possibly of
> accidentally triggering bad behavior on other IP. Also note the RX
> Timeout behaves very differently in the DMA case, for for now the
> workaround is only applied to the non-DMA case.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> Testing and development done on a kernel-4.4 based tree, then picked
> to ToT, where the code applied cleanly.
>
> Changes in v2:
> - Only apply to 8250_dw, not all 8250
> - Only apply to the non-DMA case
>
> drivers/tty/serial/8250/8250_dw.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index c89ae4581378..6ee55a2d47bb 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -201,8 +201,31 @@ static unsigned int dw8250_serial_in32be(struct uart_port *p, int offset)
>
> static int dw8250_handle_irq(struct uart_port *p)
> {
> + struct uart_8250_port *up = up_to_u8250p(p);
> struct dw8250_data *d = p->private_data;
> unsigned int iir = p->serial_in(p, UART_IIR);
> + unsigned int status;
> + unsigned long flags;
> +
> + /*
> + * There are ways to get Designware-based UARTs into a state where
> + * they are asserting UART_IIR_RX_TIMEOUT but there is no actual
> + * data available. If we see such a case then we'll do a bogus
> + * read. If we don't do this then the "RX TIMEOUT" interrupt will
> + * fire forever.
I think what you are saying is 'do a bogus read as that is the only way
to clear the interrupt, otherwise it will keep firing forever.'?
> + *
> + * This problem has only been observed so far when not in DMA mode
> + * so we limit the workaround only to non-DMA mode.
> + */
> + if (!up->dma && ((iir & 0x3f) == UART_IIR_RX_TIMEOUT)) {
why not
if (!up->dma && ((iir & UART_IIR_RX_TIMEOUT) == UART_IIR_RX_TIMEOUT)) {
it follows the flow as other conditionals in the 8250 source and you
really only need to mask the specific interrupt anyway.
> + spin_lock_irqsave(&p->lock, flags);
this is a bit above my knowledge of driver etc, but I don't any
spinlocks in the 8250 handle_irq glue drivers, except in the OMAP's case
where they are handeling a DMA IRQ. So I ask, because I don't know, why
is it needed here?
> + status = p->serial_in(p, UART_LSR);
> +
> + if (!(status & (UART_LSR_DR | UART_LSR_BI)))
> + (void) p->serial_in(p, UART_RX);
I think there should be no space between (void) and p->serial_in
> +
> + spin_unlock_irqrestore(&p->lock, flags);
> + }
>
> if (serial8250_handle_irq(p, iir))
> return 1;
>
Once I found a way to reproduce the problem (without suspend) I will
test this to see if it fixes it for us too.
Olliver
^ permalink raw reply
* Re: [PATCH v2, 3/3] tty/serial: meson_uart: add the core clock handling to the driver
From: Jerome Brunet @ 2017-03-28 19:16 UTC (permalink / raw)
To: Helmut Klein, gregkh, carlo, khilman
Cc: linux-amlogic, linux-serial, linux-arm-kernel, linux-kernel
In-Reply-To: <20170328092545.4644-4-hgkr.klein@gmail.com>
On Tue, 2017-03-28 at 11:25 +0200, Helmut Klein wrote:
> This patch gets the core clock as provided by the DT and enables it.
> The code was taken from Amlogic's serial driver, and was tested on my
> board.
>
> Signed-off-by: Helmut Klein <hgkr.klein@gmail.com>
> ---
> drivers/tty/serial/meson_uart.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index 60f16795d16b..cb99112288eb 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -600,6 +600,7 @@ static int meson_uart_probe(struct platform_device *pdev)
> struct resource *res_mem, *res_irq;
> struct uart_port *port;
> struct clk *clk;
> + struct clk *core_clk;
> int ret = 0;
>
> if (pdev->dev.of_node)
> @@ -625,6 +626,15 @@ static int meson_uart_probe(struct platform_device *pdev)
> if (!port)
> return -ENOMEM;
>
> + core_clk = devm_clk_get(&pdev->dev, "core");
> + if (!IS_ERR(core_clk)) {
> + ret = clk_prepare_enable(core_clk);
> + if (ret) {
> + dev_err(&pdev->dev, "couldn't enable clkc\n");
> + return ret;
> + }
> + }
> +
> clk = clk_get(&pdev->dev, NULL);
Now that you have 2 clocks, shouldn't this be named as well ?
> if (IS_ERR(clk))
> return PTR_ERR(clk);
> --
> 2.11.0
>
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH RFC v4 06/10] net: qualcomm: make qca_common a separate kernel module
From: Dan Williams @ 2017-03-28 17:18 UTC (permalink / raw)
To: Stefan Wahren, Rob Herring, Mark Rutland, David S. Miller
Cc: Greg Kroah-Hartman, Jiri Slaby, Marcel Holtmann,
Sebastian Reichel, netdev, devicetree, linux-serial, linux-kernel
In-Reply-To: <d90f8e33-8b86-2cf7-145b-b15997e152a7@i2se.com>
On Tue, 2017-03-28 at 18:24 +0200, Stefan Wahren wrote:
> Am 27.03.2017 um 17:44 schrieb Dan Williams:
> > On Mon, 2017-03-27 at 15:37 +0200, Stefan Wahren wrote:
> > > In order to share common functions between QCA7000 SPI and UART
> > > protocol
> > > driver the qca_common needs to be a separate kernel module.
> >
> > Maybe "qca_eth_common"? There are many things Qualcomm, slightly
> > fewer
> > things Qualcomm Atheros, and "qca_common" isn't very descriptive.
>
> Since it is a Homeplug powerline chip which doesn't have any physical
> connection to Ethernet only on protocol level, i wouldn't use eth in
> the
> module name.
>
> The code is very specific to the QCA7000, so how about
> "qca_7k_common"?
Sure; I just saw it was under drivers/net/ethernet, which kinda means
it's ethernet-related... But 7k common is fine.
Dan
> >
> > Dan
> >
> > > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> > > ---
> > > drivers/net/ethernet/qualcomm/Kconfig | 8 +++++++-
> > > drivers/net/ethernet/qualcomm/Makefile | 5 +++--
> > > drivers/net/ethernet/qualcomm/qca_common.c | 10 ++++++++++
> > > 3 files changed, 20 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/qualcomm/Kconfig
> > > b/drivers/net/ethernet/qualcomm/Kconfig
> > > index d7720bf..b4c369d 100644
> > > --- a/drivers/net/ethernet/qualcomm/Kconfig
> > > +++ b/drivers/net/ethernet/qualcomm/Kconfig
> > > @@ -16,7 +16,13 @@ config NET_VENDOR_QUALCOMM
> > > if NET_VENDOR_QUALCOMM
> > >
> > > config QCA7000
> > > - tristate "Qualcomm Atheros QCA7000 support"
> > > + tristate
> > > + help
> > > + This enables support for the Qualcomm Atheros QCA7000.
> > > +
> > > +config QCA7000_SPI
> > > + tristate "Qualcomm Atheros QCA7000 SPI support"
> > > + select QCA7000
> > > depends on SPI_MASTER && OF
> > > ---help---
> > > This SPI protocol driver supports the Qualcomm Atheros
> > > QCA7000.
> > > diff --git a/drivers/net/ethernet/qualcomm/Makefile
> > > b/drivers/net/ethernet/qualcomm/Makefile
> > > index 8080570..00d8729 100644
> > > --- a/drivers/net/ethernet/qualcomm/Makefile
> > > +++ b/drivers/net/ethernet/qualcomm/Makefile
> > > @@ -2,7 +2,8 @@
> > > # Makefile for the Qualcomm network device drivers.
> > > #
> > >
> > > -obj-$(CONFIG_QCA7000) += qcaspi.o
> > > -qcaspi-objs := qca_spi.o qca_common.o qca_7k.o qca_debug.o
> > > +obj-$(CONFIG_QCA7000) += qca_common.o
> > > +obj-$(CONFIG_QCA7000_SPI) += qcaspi.o
> > > +qcaspi-objs := qca_7k.o qca_debug.o qca_spi.o
> > >
> > > obj-y += emac/
> > > diff --git a/drivers/net/ethernet/qualcomm/qca_common.c
> > > b/drivers/net/ethernet/qualcomm/qca_common.c
> > > index d930524..f2c9e76 100644
> > > --- a/drivers/net/ethernet/qualcomm/qca_common.c
> > > +++ b/drivers/net/ethernet/qualcomm/qca_common.c
> > > @@ -21,7 +21,9 @@
> > > * by an atheros frame while transmitted over a serial
> > > channel;
> > > */
> > >
> > > +#include <linux/init.h>
> > > #include <linux/kernel.h>
> > > +#include <linux/module.h>
> > >
> > > #include "qca_common.h"
> > >
> > > @@ -46,6 +48,7 @@ qcafrm_create_header(u8 *buf, u16 length)
> > >
> > > return QCAFRM_HEADER_LEN;
> > > }
> > > +EXPORT_SYMBOL_GPL(qcafrm_create_header);
> > >
> > > u16
> > > qcafrm_create_footer(u8 *buf)
> > > @@ -57,6 +60,7 @@ qcafrm_create_footer(u8 *buf)
> > > buf[1] = 0x55;
> > > return QCAFRM_FOOTER_LEN;
> > > }
> > > +EXPORT_SYMBOL_GPL(qcafrm_create_footer);
> > >
> > > /* Gather received bytes and try to extract a full ethernet
> > > frame
> > > by
> > > * following a simple state machine.
> > > @@ -154,3 +158,9 @@ qcafrm_fsm_decode(struct qcafrm_handle
> > > *handle,
> > > u8 *buf, u16 buf_len, u8 recv_by
> > >
> > > return ret;
> > > }
> > > +EXPORT_SYMBOL_GPL(qcafrm_fsm_decode);
> > > +
> > > +MODULE_DESCRIPTION("Qualcomm Atheros Common");
> > > +MODULE_AUTHOR("Qualcomm Atheros Communications");
> > > +MODULE_AUTHOR("Stefan Wahren <stefan.wahren@i2se.com>");
> > > +MODULE_LICENSE("Dual BSD/GPL");
>
>
> *** Diese E-Mail ist allein für den bezeichneten Adressaten bestimmt.
> Sie kann rechtlich vertrauliche Informationen enthalten. Wenn Sie
> diese E-Mail irrtümlich erhalten haben, informieren Sie bitte
> unverzüglich den Absender per E-Mail und löschen Sie diese E-Mail von
> Ihrem Computer, ohne Kopien anzufertigen.
> Vielen Dank. ***
>
> *** This email is for the exclusive use of the addressee. It may
> contain legally privileged information. If you have received this
> message in error, please notify the sender by email immediately and
> delete the message from your computer without making any copies.
> Thank you. ***
>
^ permalink raw reply
* Re: [PATCH v2] serdev: Replace serdev_device_write_buf with serdev_device_write
From: Andy Shevchenko @ 2017-03-28 17:07 UTC (permalink / raw)
To: Andrey Smirnov
Cc: Rob Herring, Chris Healy, Guenter Roeck,
linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20170328160139.7739-1-andrew.smirnov@gmail.com>
On Tue, Mar 28, 2017 at 7:01 PM, Andrey Smirnov
<andrew.smirnov@gmail.com> wrote:
> Convert serdev_device_write_buf's code to be able to transfer amount of
> data potentially exceeding "write room" at the moment of invocation.
>
> To support that, also add serdev_device_write_wakeup.
>
> Drivers wanting to use full extent of serdev_device_write
> functionality are expected to provide serdev_device_write_wakeup as a
> sole handler of .write_wakeup event or call it as a part of driver's
> custom .write_wakeup code.
>
> Drivers wanting to retain old serdev_device_write_buf behaviour can
> replace those call to calls to serdev_device_write with timeout of
> 0. Providing .write_wakeup handler in such case is optional.
Some indentation would be better if, for example, 0 will be kept on
previous line.
So, what I would see if no one objects is patch series of two:
1) introduction of new API
2) removing old one.
It will benefit for easier review and any potential code anthropologist.
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -116,17 +116,41 @@ void serdev_device_close(struct serdev_device *serdev)
> }
> EXPORT_SYMBOL_GPL(serdev_device_close);
>
> -int serdev_device_write_buf(struct serdev_device *serdev,
> - const unsigned char *buf, size_t count)
> +void serdev_device_write_wakeup(struct serdev_device *serdev)
> +{
> + complete(&serdev->write_comp);
> +}
> +EXPORT_SYMBOL_GPL(serdev_device_write_wakeup);
> +
> +int serdev_device_write(struct serdev_device *serdev,
> + const unsigned char *buf, size_t count,
> + unsigned long timeout)
> {
> struct serdev_controller *ctrl = serdev->ctrl;
> + int ret;
>
> - if (!ctrl || !ctrl->ops->write_buf)
> + if (!ctrl || !ctrl->ops->write_buf ||
> + (timeout && !serdev->ops->write_wakeup))
> return -EINVAL;
>
> - return ctrl->ops->write_buf(ctrl, buf, count);
> + mutex_lock(&serdev->write_lock);
> + do {
> + reinit_completion(&serdev->write_comp);
> +
> + ret = ctrl->ops->write_buf(ctrl, buf, count);
> + if (ret < 0)
> + break;
> +
> + buf += ret;
Extra white spaces.
> + count -= ret;
> +
> + } while (count &&
> + (timeout = wait_for_completion_timeout(&serdev->write_comp,
> + timeout)));
So, would it be better to support interrupts here and return a
corresponding error code to the user?
Besides that question, readability might be better if you use
temporary variable and pack above on one line:
unsigned long to = timeout;
} while (count && (to = ...(to)));
> + mutex_unlock(&serdev->write_lock);
> + return ret < 0 ? ret : (count ? -ETIMEDOUT : 0);
> }
> -EXPORT_SYMBOL_GPL(serdev_device_write_buf);
> +EXPORT_SYMBOL_GPL(serdev_device_write);
> + * @write_comp Completion used by serdev_device_write internally
Links to the functions are func()-like. Please check kernel doc howto:s.
> + * @write_lock Mutext used to esure exclusive access to the bus when
> + * writing data with serdev_device_write()
checkpatch.pl has integrated spellchecker AFAIU.
Moreover, can you try harder to make that description shorter?
> void serdev_device_write_flush(struct serdev_device *);
> int serdev_device_write_room(struct serdev_device *);
>
> +
> /*
> * serdev device driver functions
> */
This doesn't belong to the change.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH RFC v4 10/10] tty: serdev: add functions to retrieve common UART settings
From: Stefan Wahren @ 2017-03-28 16:33 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, David S. Miller, Greg Kroah-Hartman, Jiri Slaby,
Marcel Holtmann, Sebastian Reichel, netdev,
devicetree@vger.kernel.org, linux-serial@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <CAL_JsqJ3b2zTGNHNVtt6Y+k23iw+2G2z-Jm2786a8CEzZBS7=g@mail.gmail.com>
Am 27.03.2017 um 22:00 schrieb Rob Herring:
> On Mon, Mar 27, 2017 at 8:37 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote:
>> Currently serdev core doesn't provide functions to retrieve common
>> UART settings like data bits, stop bits or parity. This patch adds
>> the interface to the core and the necessary implementation for
>> serdev-ttyport.
> It doesn't provide them because why do you need to know? The attached
> device should request the settings it needs and be done with it. Maybe
> some devices can support a number of settings and you could want
> negotiate the settings with the UART, though surely 8N1 is in that
> list. It's rare to see something that's not 8N1 from what I've seen.
During development it's very helpful to check the current UART settings
and error counter. Currently i can't see a replacement for
/sys/class/tty/ttyXYZ .
Are there any plans about it?
>
> Rob
^ permalink raw reply
* Re: [PATCH RFC v4 06/10] net: qualcomm: make qca_common a separate kernel module
From: Stefan Wahren @ 2017-03-28 16:24 UTC (permalink / raw)
To: Dan Williams, Rob Herring, Mark Rutland, David S. Miller
Cc: Greg Kroah-Hartman, Jiri Slaby, Marcel Holtmann,
Sebastian Reichel, netdev, devicetree, linux-serial, linux-kernel
In-Reply-To: <1490629444.2719.3.camel@redhat.com>
Am 27.03.2017 um 17:44 schrieb Dan Williams:
> On Mon, 2017-03-27 at 15:37 +0200, Stefan Wahren wrote:
>> In order to share common functions between QCA7000 SPI and UART
>> protocol
>> driver the qca_common needs to be a separate kernel module.
> Maybe "qca_eth_common"? There are many things Qualcomm, slightly fewer
> things Qualcomm Atheros, and "qca_common" isn't very descriptive.
Since it is a Homeplug powerline chip which doesn't have any physical
connection to Ethernet only on protocol level, i wouldn't use eth in the
module name.
The code is very specific to the QCA7000, so how about "qca_7k_common"?
>
> Dan
>
>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
>> ---
>> drivers/net/ethernet/qualcomm/Kconfig | 8 +++++++-
>> drivers/net/ethernet/qualcomm/Makefile | 5 +++--
>> drivers/net/ethernet/qualcomm/qca_common.c | 10 ++++++++++
>> 3 files changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/qualcomm/Kconfig
>> b/drivers/net/ethernet/qualcomm/Kconfig
>> index d7720bf..b4c369d 100644
>> --- a/drivers/net/ethernet/qualcomm/Kconfig
>> +++ b/drivers/net/ethernet/qualcomm/Kconfig
>> @@ -16,7 +16,13 @@ config NET_VENDOR_QUALCOMM
>> if NET_VENDOR_QUALCOMM
>>
>> config QCA7000
>> - tristate "Qualcomm Atheros QCA7000 support"
>> + tristate
>> + help
>> + This enables support for the Qualcomm Atheros QCA7000.
>> +
>> +config QCA7000_SPI
>> + tristate "Qualcomm Atheros QCA7000 SPI support"
>> + select QCA7000
>> depends on SPI_MASTER && OF
>> ---help---
>> This SPI protocol driver supports the Qualcomm Atheros
>> QCA7000.
>> diff --git a/drivers/net/ethernet/qualcomm/Makefile
>> b/drivers/net/ethernet/qualcomm/Makefile
>> index 8080570..00d8729 100644
>> --- a/drivers/net/ethernet/qualcomm/Makefile
>> +++ b/drivers/net/ethernet/qualcomm/Makefile
>> @@ -2,7 +2,8 @@
>> # Makefile for the Qualcomm network device drivers.
>> #
>>
>> -obj-$(CONFIG_QCA7000) += qcaspi.o
>> -qcaspi-objs := qca_spi.o qca_common.o qca_7k.o qca_debug.o
>> +obj-$(CONFIG_QCA7000) += qca_common.o
>> +obj-$(CONFIG_QCA7000_SPI) += qcaspi.o
>> +qcaspi-objs := qca_7k.o qca_debug.o qca_spi.o
>>
>> obj-y += emac/
>> diff --git a/drivers/net/ethernet/qualcomm/qca_common.c
>> b/drivers/net/ethernet/qualcomm/qca_common.c
>> index d930524..f2c9e76 100644
>> --- a/drivers/net/ethernet/qualcomm/qca_common.c
>> +++ b/drivers/net/ethernet/qualcomm/qca_common.c
>> @@ -21,7 +21,9 @@
>> * by an atheros frame while transmitted over a serial channel;
>> */
>>
>> +#include <linux/init.h>
>> #include <linux/kernel.h>
>> +#include <linux/module.h>
>>
>> #include "qca_common.h"
>>
>> @@ -46,6 +48,7 @@ qcafrm_create_header(u8 *buf, u16 length)
>>
>> return QCAFRM_HEADER_LEN;
>> }
>> +EXPORT_SYMBOL_GPL(qcafrm_create_header);
>>
>> u16
>> qcafrm_create_footer(u8 *buf)
>> @@ -57,6 +60,7 @@ qcafrm_create_footer(u8 *buf)
>> buf[1] = 0x55;
>> return QCAFRM_FOOTER_LEN;
>> }
>> +EXPORT_SYMBOL_GPL(qcafrm_create_footer);
>>
>> /* Gather received bytes and try to extract a full ethernet frame
>> by
>> * following a simple state machine.
>> @@ -154,3 +158,9 @@ qcafrm_fsm_decode(struct qcafrm_handle *handle,
>> u8 *buf, u16 buf_len, u8 recv_by
>>
>> return ret;
>> }
>> +EXPORT_SYMBOL_GPL(qcafrm_fsm_decode);
>> +
>> +MODULE_DESCRIPTION("Qualcomm Atheros Common");
>> +MODULE_AUTHOR("Qualcomm Atheros Communications");
>> +MODULE_AUTHOR("Stefan Wahren <stefan.wahren@i2se.com>");
>> +MODULE_LICENSE("Dual BSD/GPL");
*** Diese E-Mail ist allein für den bezeichneten Adressaten bestimmt. Sie kann rechtlich vertrauliche Informationen enthalten. Wenn Sie diese E-Mail irrtümlich erhalten haben, informieren Sie bitte unverzüglich den Absender per E-Mail und löschen Sie diese E-Mail von Ihrem Computer, ohne Kopien anzufertigen.
Vielen Dank. ***
*** This email is for the exclusive use of the addressee. It may contain legally privileged information. If you have received this message in error, please notify the sender by email immediately and delete the message from your computer without making any copies.
Thank you. ***
^ permalink raw reply
* Re: [PATCH RFC v4 07/10] dt-bindings: net: add binding for QCA7000 UART
From: Stefan Wahren @ 2017-03-28 16:18 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, David S. Miller, Greg Kroah-Hartman, Jiri Slaby,
Marcel Holtmann, Sebastian Reichel, netdev,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_Jsq+gxS4u+b+C_eWeguFR0AFKuq8Hhe9Xo_5B_5w1hxvzEg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Am 27.03.2017 um 22:30 schrieb Rob Herring:
> On Mon, Mar 27, 2017 at 8:37 AM, Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> wrote:
>> This is the serdev binding for the QCA7000 UART driver (Ethernet over UART).
>>
>> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
>> ---
>>
>> According to this binding are still some questions:
>>
>> Where should be the optional hardware flow control defined (at master or slave side)?
> Probably should be in the slave side. We already have uart-has-rtscts
> and rts/cts-gpios for the UART. Those mean we have RTS/CTS, but not
> necessarily that we want to enable them.
>
> In many cases, the driver may know what it needs.
Like all the other UART settings the hardware flow control can be
configured in the QCA7000 firmware and the driver can't detect it.
Property suggestion for the slave side:
use-rtscts
>
>> Is it okay to have two bindings (qca-qca7000-spi and qca-qca7000-uart) or should they be merged?
> Are they mutually-exclusive or both are used at the same time?
They are mutually-exclusive because they use the same pins.
> What
> are the dependencies between the interfaces?
Except they uses the same pins of the QCA7000, i can't see any dependency.
>
>>
>> .../devicetree/bindings/net/qca-qca7000-uart.txt | 31 ++++++++++++++++++++++
>> 1 file changed, 31 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/net/qca-qca7000-uart.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/qca-qca7000-uart.txt b/Documentation/devicetree/bindings/net/qca-qca7000-uart.txt
>> new file mode 100644
>> index 0000000..f2e0450
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/qca-qca7000-uart.txt
>> @@ -0,0 +1,31 @@
>> +* Qualcomm QCA7000 (Ethernet over UART protocol)
>> +
>> +Note: This binding applies in case the QCA7000 is configured as a
>> +UART slave device. It is possible to preconfigure the UART settings
>> +of the QCA7000 firmware, which can't be changed during runtime.
>> +
>> +Required properties:
>> +- compatible : Should be "qca,qca7000-uart"
>> +
>> +Optional properties:
>> +- local-mac-address : 6 bytes, Specifies MAC address
> The description can be "see ./ethernet.txt"
>
>> +- current-speed : Specifies the serial device speed in
>> + bits per second (default = 115200), which is
>> + predefined by the QCA7000 firmware configuration
> Add this to the slave binding doc with some caveats as to when this
> should or should not be used as we discussed.
>
> Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v2] serdev: Replace serdev_device_write_buf with serdev_device_write
From: Andrey Smirnov @ 2017-03-28 16:01 UTC (permalink / raw)
To: Rob Herring
Cc: Andrey Smirnov, cphealy, Guenter Roeck, Andy Shevchenko,
linux-serial, linux-kernel
Convert serdev_device_write_buf's code to be able to transfer amount of
data potentially exceeding "write room" at the moment of invocation.
To support that, also add serdev_device_write_wakeup.
Drivers wanting to use full extent of serdev_device_write
functionality are expected to provide serdev_device_write_wakeup as a
sole handler of .write_wakeup event or call it as a part of driver's
custom .write_wakeup code.
Drivers wanting to retain old serdev_device_write_buf behaviour can
replace those call to calls to serdev_device_write with timeout of
0. Providing .write_wakeup handler in such case is optional.
Cc: cphealy@gmail.com
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: linux-serial@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
Changes since v1 (see [v1]):
- Make timeout to be a total(as opposed to per-iteration)
timeout
- Keep serdev_device_write_buf() as a wrapper arount
serdev_device_write() for compatibility
[v1] https://lkml.org/lkml/2017/3/20/650
drivers/tty/serdev/core.c | 36 +++++++++++++++++++++++++++++++-----
include/linux/serdev.h | 18 ++++++++++++++++--
2 files changed, 47 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index f4c6c90..c47c421 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -116,17 +116,41 @@ void serdev_device_close(struct serdev_device *serdev)
}
EXPORT_SYMBOL_GPL(serdev_device_close);
-int serdev_device_write_buf(struct serdev_device *serdev,
- const unsigned char *buf, size_t count)
+void serdev_device_write_wakeup(struct serdev_device *serdev)
+{
+ complete(&serdev->write_comp);
+}
+EXPORT_SYMBOL_GPL(serdev_device_write_wakeup);
+
+int serdev_device_write(struct serdev_device *serdev,
+ const unsigned char *buf, size_t count,
+ unsigned long timeout)
{
struct serdev_controller *ctrl = serdev->ctrl;
+ int ret;
- if (!ctrl || !ctrl->ops->write_buf)
+ if (!ctrl || !ctrl->ops->write_buf ||
+ (timeout && !serdev->ops->write_wakeup))
return -EINVAL;
- return ctrl->ops->write_buf(ctrl, buf, count);
+ mutex_lock(&serdev->write_lock);
+ do {
+ reinit_completion(&serdev->write_comp);
+
+ ret = ctrl->ops->write_buf(ctrl, buf, count);
+ if (ret < 0)
+ break;
+
+ buf += ret;
+ count -= ret;
+
+ } while (count &&
+ (timeout = wait_for_completion_timeout(&serdev->write_comp,
+ timeout)));
+ mutex_unlock(&serdev->write_lock);
+ return ret < 0 ? ret : (count ? -ETIMEDOUT : 0);
}
-EXPORT_SYMBOL_GPL(serdev_device_write_buf);
+EXPORT_SYMBOL_GPL(serdev_device_write);
void serdev_device_write_flush(struct serdev_device *serdev)
{
@@ -232,6 +256,8 @@ struct serdev_device *serdev_device_alloc(struct serdev_controller *ctrl)
serdev->dev.parent = &ctrl->dev;
serdev->dev.bus = &serdev_bus_type;
serdev->dev.type = &serdev_device_type;
+ init_completion(&serdev->write_comp);
+ mutex_init(&serdev->write_lock);
return serdev;
}
EXPORT_SYMBOL_GPL(serdev_device_alloc);
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index 5176cdc..535566d 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -39,12 +39,17 @@ struct serdev_device_ops {
* @nr: Device number on serdev bus.
* @ctrl: serdev controller managing this device.
* @ops: Device operations.
+ * @write_comp Completion used by serdev_device_write internally
+ * @write_lock Mutext used to esure exclusive access to the bus when
+ * writing data with serdev_device_write()
*/
struct serdev_device {
struct device dev;
int nr;
struct serdev_controller *ctrl;
const struct serdev_device_ops *ops;
+ struct completion write_comp;
+ struct mutex write_lock;
};
static inline struct serdev_device *to_serdev_device(struct device *d)
@@ -186,10 +191,12 @@ int serdev_device_open(struct serdev_device *);
void serdev_device_close(struct serdev_device *);
unsigned int serdev_device_set_baudrate(struct serdev_device *, unsigned int);
void serdev_device_set_flow_control(struct serdev_device *, bool);
-int serdev_device_write_buf(struct serdev_device *, const unsigned char *, size_t);
+void serdev_device_write_wakeup(struct serdev_device *);
+int serdev_device_write(struct serdev_device *, const unsigned char *, size_t, unsigned long);
void serdev_device_write_flush(struct serdev_device *);
int serdev_device_write_room(struct serdev_device *);
+
/*
* serdev device driver functions
*/
@@ -223,7 +230,8 @@ static inline unsigned int serdev_device_set_baudrate(struct serdev_device *sdev
return 0;
}
static inline void serdev_device_set_flow_control(struct serdev_device *sdev, bool enable) {}
-static inline int serdev_device_write_buf(struct serdev_device *sdev, const unsigned char *buf, size_t count)
+static inline int serdev_device_write(struct serdev_device *sdev, const unsigned char *buf,
+ size_t count, unsigned long timeout)
{
return -ENODEV;
}
@@ -259,4 +267,10 @@ static inline struct device *serdev_tty_port_register(struct tty_port *port,
static inline void serdev_tty_port_unregister(struct tty_port *port) {}
#endif /* CONFIG_SERIAL_DEV_CTRL_TTYPORT */
+static inline int serdev_device_write_buf(struct serdev_device *serdev,
+ const unsigned char *data, size_t count)
+{
+ return serdev_device_write(serdev, data, count, 0);
+}
+
#endif /*_LINUX_SERDEV_H */
--
2.9.3
^ permalink raw reply related
* [PATCHv3 10/10] Bluetooth: add nokia driver
From: Sebastian Reichel @ 2017-03-28 15:59 UTC (permalink / raw)
To: Sebastian Reichel, Marcel Holtmann, Gustavo Padovan,
Johan Hedberg, Rob Herring
Cc: Samuel Thibault, Pavel Machek, Tony Lindgren, Greg Kroah-Hartman,
Jiri Slaby, Mark Rutland, linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170328155939.31566-1-sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
This adds a driver for the Nokia H4+ protocol, which is used
at least on the Nokia N9, N900 & N950.
Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
Changes since PATCHv1:
* replace __u8 and uint8_t with u8
* replace __u16 and uint16_t with u16
* drop BT_BAUDRATE_DIVIDER and use btdev->sysclk_speed * 10 instead
* fix wording of a sentence
* fix error path of negotation & alive package receive functions
* replaced nokia_wait_for_cts with newly introduced serdev function
* use "nokia,h4p-bluetooth" as compatible string
---
drivers/bluetooth/Kconfig | 12 +
drivers/bluetooth/Makefile | 2 +
drivers/bluetooth/hci_nokia.c | 819 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 833 insertions(+)
create mode 100644 drivers/bluetooth/hci_nokia.c
diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index c2c14a12713b..2e3e4d3547ad 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -86,6 +86,18 @@ config BT_HCIUART_H4
Say Y here to compile support for HCI UART (H4) protocol.
+config BT_HCIUART_NOKIA
+ tristate "UART Nokia H4+ protocol support"
+ depends on BT_HCIUART
+ depends on SERIAL_DEV_BUS
+ depends on PM
+ help
+ Nokia H4+ is serial protocol for communication between Bluetooth
+ device and host. This protocol is required for Bluetooth devices
+ with UART interface in Nokia devices.
+
+ Say Y here to compile support for Nokia's H4+ protocol.
+
config BT_HCIUART_BCSP
bool "BCSP protocol support"
depends on BT_HCIUART
diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
index fd571689eed6..a7f237320f4b 100644
--- a/drivers/bluetooth/Makefile
+++ b/drivers/bluetooth/Makefile
@@ -25,6 +25,8 @@ obj-$(CONFIG_BT_BCM) += btbcm.o
obj-$(CONFIG_BT_RTL) += btrtl.o
obj-$(CONFIG_BT_QCA) += btqca.o
+obj-$(CONFIG_BT_HCIUART_NOKIA) += hci_nokia.o
+
btmrvl-y := btmrvl_main.o
btmrvl-$(CONFIG_DEBUG_FS) += btmrvl_debugfs.o
diff --git a/drivers/bluetooth/hci_nokia.c b/drivers/bluetooth/hci_nokia.c
new file mode 100644
index 000000000000..c77f04af01bf
--- /dev/null
+++ b/drivers/bluetooth/hci_nokia.c
@@ -0,0 +1,819 @@
+/*
+ * Bluetooth HCI UART H4 driver with Nokia Extensions AKA Nokia H4+
+ *
+ * Copyright (C) 2015 Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
+ * Copyright (C) 2015-2017 Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/clk.h>
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/interrupt.h>
+#include <linux/pm_runtime.h>
+#include <linux/firmware.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/errno.h>
+#include <linux/skbuff.h>
+#include <linux/gpio/consumer.h>
+#include <linux/unaligned/le_struct.h>
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci_core.h>
+#include <linux/serdev.h>
+
+#include "hci_uart.h"
+#include "btbcm.h"
+
+#define NOKIA_ID_BCM2048 0x04
+#define NOKIA_ID_TI1271 0x31
+
+#define FIRMWARE_BCM2048 "nokia/bcmfw.bin"
+#define FIRMWARE_TI1271 "nokia/ti1273.bin"
+
+#define HCI_NOKIA_NEG_PKT 0x06
+#define HCI_NOKIA_ALIVE_PKT 0x07
+#define HCI_NOKIA_RADIO_PKT 0x08
+
+#define HCI_NOKIA_NEG_HDR_SIZE 1
+#define HCI_NOKIA_MAX_NEG_SIZE 255
+#define HCI_NOKIA_ALIVE_HDR_SIZE 1
+#define HCI_NOKIA_MAX_ALIVE_SIZE 255
+#define HCI_NOKIA_RADIO_HDR_SIZE 2
+#define HCI_NOKIA_MAX_RADIO_SIZE 255
+
+#define NOKIA_PROTO_PKT 0x44
+#define NOKIA_PROTO_BYTE 0x4c
+
+#define NOKIA_NEG_REQ 0x00
+#define NOKIA_NEG_ACK 0x20
+#define NOKIA_NEG_NAK 0x40
+
+#define H4_TYPE_SIZE 1
+
+#define NOKIA_RECV_ALIVE \
+ .type = HCI_NOKIA_ALIVE_PKT, \
+ .hlen = HCI_NOKIA_ALIVE_HDR_SIZE, \
+ .loff = 0, \
+ .lsize = 1, \
+ .maxlen = HCI_NOKIA_MAX_ALIVE_SIZE \
+
+#define NOKIA_RECV_NEG \
+ .type = HCI_NOKIA_NEG_PKT, \
+ .hlen = HCI_NOKIA_NEG_HDR_SIZE, \
+ .loff = 0, \
+ .lsize = 1, \
+ .maxlen = HCI_NOKIA_MAX_NEG_SIZE \
+
+#define NOKIA_RECV_RADIO \
+ .type = HCI_NOKIA_RADIO_PKT, \
+ .hlen = HCI_NOKIA_RADIO_HDR_SIZE, \
+ .loff = 1, \
+ .lsize = 1, \
+ .maxlen = HCI_NOKIA_MAX_RADIO_SIZE \
+
+struct hci_nokia_neg_hdr {
+ u8 dlen;
+} __packed;
+
+struct hci_nokia_neg_cmd {
+ u8 ack;
+ u16 baud;
+ u16 unused1;
+ u8 proto;
+ u16 sys_clk;
+ u16 unused2;
+} __packed;
+
+#define NOKIA_ALIVE_REQ 0x55
+#define NOKIA_ALIVE_RESP 0xcc
+
+struct hci_nokia_alive_hdr {
+ u8 dlen;
+} __packed;
+
+struct hci_nokia_alive_pkt {
+ u8 mid;
+ u8 unused;
+} __packed;
+
+struct hci_nokia_neg_evt {
+ u8 ack;
+ u16 baud;
+ u16 unused1;
+ u8 proto;
+ u16 sys_clk;
+ u16 unused2;
+ u8 man_id;
+ u8 ver_id;
+} __packed;
+
+#define MAX_BAUD_RATE 3692300
+#define SETUP_BAUD_RATE 921600
+#define INIT_BAUD_RATE 120000
+
+struct hci_nokia_radio_hdr {
+ u8 evt;
+ u8 dlen;
+} __packed;
+
+struct nokia_bt_dev {
+ struct hci_uart hu;
+ struct serdev_device *serdev;
+
+ struct gpio_desc *reset;
+ struct gpio_desc *wakeup_host;
+ struct gpio_desc *wakeup_bt;
+ unsigned long sysclk_speed;
+
+ int wake_irq;
+ struct sk_buff *rx_skb;
+ struct sk_buff_head txq;
+ bdaddr_t bdaddr;
+
+ int init_error;
+ struct completion init_completion;
+
+ u8 man_id;
+ u8 ver_id;
+
+ bool initialized;
+ bool tx_enabled;
+ bool rx_enabled;
+};
+
+static int nokia_enqueue(struct hci_uart *hu, struct sk_buff *skb);
+
+static void nokia_flow_control(struct serdev_device *serdev, bool enable)
+{
+ if (enable) {
+ serdev_device_set_rts(serdev, true);
+ serdev_device_set_flow_control(serdev, true);
+ } else {
+ serdev_device_set_flow_control(serdev, false);
+ serdev_device_set_rts(serdev, false);
+ }
+}
+
+static irqreturn_t wakeup_handler(int irq, void *data)
+{
+ struct nokia_bt_dev *btdev = data;
+ struct device *dev = &btdev->serdev->dev;
+ int wake_state = gpiod_get_value(btdev->wakeup_host);
+
+ if (btdev->rx_enabled == wake_state)
+ return IRQ_HANDLED;
+
+ if (wake_state)
+ pm_runtime_get(dev);
+ else
+ pm_runtime_put(dev);
+
+ btdev->rx_enabled = wake_state;
+
+ return IRQ_HANDLED;
+}
+
+static int nokia_reset(struct hci_uart *hu)
+{
+ struct nokia_bt_dev *btdev = hu->priv;
+ struct device *dev = &btdev->serdev->dev;
+ int err;
+
+ /* reset routine */
+ gpiod_set_value_cansleep(btdev->reset, 1);
+ gpiod_set_value_cansleep(btdev->wakeup_bt, 1);
+
+ msleep(100);
+
+ /* safety check */
+ err = gpiod_get_value_cansleep(btdev->wakeup_host);
+ if (err == 1) {
+ dev_err(dev, "reset: host wakeup not low!");
+ return -EPROTO;
+ }
+
+ /* flush queue */
+ serdev_device_write_flush(btdev->serdev);
+
+ /* init uart */
+ nokia_flow_control(btdev->serdev, false);
+ serdev_device_set_baudrate(btdev->serdev, INIT_BAUD_RATE);
+
+ gpiod_set_value_cansleep(btdev->reset, 0);
+
+ /* wait for cts */
+ err = serdev_device_wait_for_cts(btdev->serdev, true, 200);
+ if (err < 0) {
+ dev_err(dev, "CTS not received: %d", err);
+ return err;
+ }
+
+ nokia_flow_control(btdev->serdev, true);
+
+ return 0;
+}
+
+static int nokia_send_alive_packet(struct hci_uart *hu)
+{
+ struct nokia_bt_dev *btdev = hu->priv;
+ struct device *dev = &btdev->serdev->dev;
+ struct hci_nokia_alive_hdr *hdr;
+ struct hci_nokia_alive_pkt *pkt;
+ struct sk_buff *skb;
+ int len;
+
+ init_completion(&btdev->init_completion);
+
+ len = H4_TYPE_SIZE + sizeof(*hdr) + sizeof(*pkt);
+ skb = bt_skb_alloc(len, GFP_KERNEL);
+ if (!skb)
+ return -ENOMEM;
+
+ hci_skb_pkt_type(skb) = HCI_NOKIA_ALIVE_PKT;
+ memset(skb->data, 0x00, len);
+
+ hdr = (struct hci_nokia_alive_hdr *)skb_put(skb, sizeof(*hdr));
+ hdr->dlen = sizeof(*pkt);
+ pkt = (struct hci_nokia_alive_pkt *)skb_put(skb, sizeof(*pkt));
+ pkt->mid = NOKIA_ALIVE_REQ;
+
+ nokia_enqueue(hu, skb);
+ hci_uart_tx_wakeup(hu);
+
+ dev_dbg(dev, "Alive sent");
+
+ if (!wait_for_completion_interruptible_timeout(&btdev->init_completion,
+ msecs_to_jiffies(1000))) {
+ return -ETIMEDOUT;
+ }
+
+ if (btdev->init_error < 0)
+ return btdev->init_error;
+
+ return 0;
+}
+
+static int nokia_send_negotiation(struct hci_uart *hu)
+{
+ struct nokia_bt_dev *btdev = hu->priv;
+ struct device *dev = &btdev->serdev->dev;
+ struct hci_nokia_neg_cmd *neg_cmd;
+ struct hci_nokia_neg_hdr *neg_hdr;
+ struct sk_buff *skb;
+ int len, err;
+ u16 baud = DIV_ROUND_CLOSEST(btdev->sysclk_speed * 10, SETUP_BAUD_RATE);
+ int sysclk = btdev->sysclk_speed / 1000;
+
+ len = H4_TYPE_SIZE + sizeof(*neg_hdr) + sizeof(*neg_cmd);
+ skb = bt_skb_alloc(len, GFP_KERNEL);
+ if (!skb)
+ return -ENOMEM;
+
+ hci_skb_pkt_type(skb) = HCI_NOKIA_NEG_PKT;
+
+ neg_hdr = (struct hci_nokia_neg_hdr *)skb_put(skb, sizeof(*neg_hdr));
+ neg_hdr->dlen = sizeof(*neg_cmd);
+
+ neg_cmd = (struct hci_nokia_neg_cmd *)skb_put(skb, sizeof(*neg_cmd));
+ neg_cmd->ack = NOKIA_NEG_REQ;
+ neg_cmd->baud = cpu_to_le16(baud);
+ neg_cmd->unused1 = 0x0000;
+ neg_cmd->proto = NOKIA_PROTO_BYTE;
+ neg_cmd->sys_clk = cpu_to_le16(sysclk);
+ neg_cmd->unused2 = 0x0000;
+
+ btdev->init_error = 0;
+ init_completion(&btdev->init_completion);
+
+ nokia_enqueue(hu, skb);
+ hci_uart_tx_wakeup(hu);
+
+ dev_dbg(dev, "Negotiation sent");
+
+ if (!wait_for_completion_interruptible_timeout(&btdev->init_completion,
+ msecs_to_jiffies(10000))) {
+ return -ETIMEDOUT;
+ }
+
+ if (btdev->init_error < 0)
+ return btdev->init_error;
+
+ /* Change to previously negotiated speed. Flow Control
+ * is disabled until bluetooth adapter is ready to avoid
+ * broken bytes being received.
+ */
+ nokia_flow_control(btdev->serdev, false);
+ serdev_device_set_baudrate(btdev->serdev, SETUP_BAUD_RATE);
+ err = serdev_device_wait_for_cts(btdev->serdev, true, 200);
+ if (err < 0) {
+ dev_err(dev, "CTS not received: %d", err);
+ return err;
+ }
+ nokia_flow_control(btdev->serdev, true);
+
+ dev_dbg(dev, "Negotiation successful");
+
+ return 0;
+}
+
+static int nokia_setup_fw(struct hci_uart *hu)
+{
+ struct nokia_bt_dev *btdev = hu->priv;
+ struct device *dev = &btdev->serdev->dev;
+ const char *fwname;
+ const struct firmware *fw;
+ const u8 *fw_ptr;
+ size_t fw_size;
+ int err;
+
+ dev_dbg(dev, "setup firmware");
+
+ if (btdev->man_id == NOKIA_ID_BCM2048) {
+ fwname = FIRMWARE_BCM2048;
+ } else if (btdev->man_id == NOKIA_ID_TI1271) {
+ fwname = FIRMWARE_TI1271;
+ } else {
+ dev_err(dev, "Unsupported bluetooth device!");
+ return -ENODEV;
+ }
+
+ err = request_firmware(&fw, fwname, dev);
+ if (err < 0) {
+ dev_err(dev, "%s: Failed to load Nokia firmware file (%d)",
+ hu->hdev->name, err);
+ return err;
+ }
+
+ fw_ptr = fw->data;
+ fw_size = fw->size;
+
+ while (fw_size >= 4) {
+ u16 pkt_size = get_unaligned_le16(fw_ptr);
+ u8 pkt_type = fw_ptr[2];
+ const struct hci_command_hdr *cmd;
+ u16 opcode;
+ struct sk_buff *skb;
+
+ switch (pkt_type) {
+ case HCI_COMMAND_PKT:
+ cmd = (struct hci_command_hdr *)(fw_ptr + 3);
+ opcode = le16_to_cpu(cmd->opcode);
+
+ skb = __hci_cmd_sync(hu->hdev, opcode, cmd->plen,
+ fw_ptr + 3 + HCI_COMMAND_HDR_SIZE,
+ HCI_INIT_TIMEOUT);
+ if (IS_ERR(skb)) {
+ err = PTR_ERR(skb);
+ dev_err(dev, "%s: FW command %04x failed (%d)",
+ hu->hdev->name, opcode, err);
+ goto done;
+ }
+ kfree_skb(skb);
+ break;
+ case HCI_NOKIA_RADIO_PKT:
+ case HCI_NOKIA_NEG_PKT:
+ case HCI_NOKIA_ALIVE_PKT:
+ break;
+ }
+
+ fw_ptr += pkt_size + 2;
+ fw_size -= pkt_size + 2;
+ }
+
+done:
+ release_firmware(fw);
+ return err;
+}
+
+static int nokia_setup(struct hci_uart *hu)
+{
+ struct nokia_bt_dev *btdev = hu->priv;
+ struct device *dev = &btdev->serdev->dev;
+ int err;
+
+ btdev->initialized = false;
+
+ nokia_flow_control(btdev->serdev, false);
+
+ pm_runtime_get_sync(dev);
+
+ if (btdev->tx_enabled) {
+ gpiod_set_value_cansleep(btdev->wakeup_bt, 0);
+ pm_runtime_put(&btdev->serdev->dev);
+ btdev->tx_enabled = false;
+ }
+
+ dev_dbg(dev, "protocol setup");
+
+ /* 0. reset connection */
+ err = nokia_reset(hu);
+ if (err < 0) {
+ dev_err(dev, "Reset failed: %d", err);
+ goto out;
+ }
+
+ /* 1. negotiate speed etc */
+ err = nokia_send_negotiation(hu);
+ if (err < 0) {
+ dev_err(dev, "Negotiation failed: %d", err);
+ goto out;
+ }
+
+ /* 2. verify correct setup using alive packet */
+ err = nokia_send_alive_packet(hu);
+ if (err < 0) {
+ dev_err(dev, "Alive check failed: %d", err);
+ goto out;
+ }
+
+ /* 3. send firmware */
+ err = nokia_setup_fw(hu);
+ if (err < 0) {
+ dev_err(dev, "Could not setup FW: %d", err);
+ goto out;
+ }
+
+ nokia_flow_control(btdev->serdev, false);
+ serdev_device_set_baudrate(btdev->serdev, MAX_BAUD_RATE);
+ nokia_flow_control(btdev->serdev, true);
+
+ if (btdev->man_id == NOKIA_ID_BCM2048) {
+ hu->hdev->set_bdaddr = btbcm_set_bdaddr;
+ set_bit(HCI_QUIRK_INVALID_BDADDR, &hu->hdev->quirks);
+ dev_dbg(dev, "bcm2048 has invalid bluetooth address!");
+ }
+
+ dev_dbg(dev, "protocol setup done!");
+
+ gpiod_set_value_cansleep(btdev->wakeup_bt, 0);
+ pm_runtime_put(dev);
+ btdev->tx_enabled = false;
+ btdev->initialized = true;
+
+ return 0;
+out:
+ pm_runtime_put(dev);
+
+ return err;
+}
+
+static int nokia_open(struct hci_uart *hu)
+{
+ struct device *dev = &hu->serdev->dev;
+
+ dev_dbg(dev, "protocol open");
+
+ serdev_device_open(hu->serdev);
+
+ pm_runtime_enable(dev);
+
+ return 0;
+}
+
+static int nokia_flush(struct hci_uart *hu)
+{
+ struct nokia_bt_dev *btdev = hu->priv;
+
+ dev_dbg(&btdev->serdev->dev, "flush device");
+
+ skb_queue_purge(&btdev->txq);
+
+ return 0;
+}
+
+static int nokia_close(struct hci_uart *hu)
+{
+ struct nokia_bt_dev *btdev = hu->priv;
+ struct device *dev = &btdev->serdev->dev;
+
+ dev_dbg(dev, "close device");
+
+ btdev->initialized = false;
+
+ skb_queue_purge(&btdev->txq);
+
+ kfree_skb(btdev->rx_skb);
+
+ /* disable module */
+ gpiod_set_value(btdev->reset, 1);
+ gpiod_set_value(btdev->wakeup_bt, 0);
+
+ pm_runtime_disable(&btdev->serdev->dev);
+ serdev_device_close(btdev->serdev);
+
+ return 0;
+}
+
+/* Enqueue frame for transmittion (padding, crc, etc) */
+static int nokia_enqueue(struct hci_uart *hu, struct sk_buff *skb)
+{
+ struct nokia_bt_dev *btdev = hu->priv;
+ int err;
+
+ /* Prepend skb with frame type */
+ memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
+
+ /* Packets must be word aligned */
+ if (skb->len % 2) {
+ err = skb_pad(skb, 1);
+ if (err)
+ return err;
+ *skb_put(skb, 1) = 0x00;
+ }
+
+ skb_queue_tail(&btdev->txq, skb);
+
+ return 0;
+}
+
+static int nokia_recv_negotiation_packet(struct hci_dev *hdev,
+ struct sk_buff *skb)
+{
+ struct hci_uart *hu = hci_get_drvdata(hdev);
+ struct nokia_bt_dev *btdev = hu->priv;
+ struct device *dev = &btdev->serdev->dev;
+ struct hci_nokia_neg_hdr *hdr;
+ struct hci_nokia_neg_evt *evt;
+ int ret = 0;
+
+ hdr = (struct hci_nokia_neg_hdr *)skb->data;
+ if (hdr->dlen != sizeof(*evt)) {
+ btdev->init_error = -EIO;
+ ret = -EIO;
+ goto finish_neg;
+ }
+
+ evt = (struct hci_nokia_neg_evt *)skb_pull(skb, sizeof(*hdr));
+
+ if (evt->ack != NOKIA_NEG_ACK) {
+ dev_err(dev, "Negotiation received: wrong reply");
+ btdev->init_error = -EINVAL;
+ ret = -EINVAL;
+ goto finish_neg;
+ }
+
+ btdev->man_id = evt->man_id;
+ btdev->ver_id = evt->ver_id;
+
+ dev_dbg(dev, "Negotiation received: baud=%u:clk=%u:manu=%u:vers=%u",
+ evt->baud, evt->sys_clk, evt->man_id, evt->ver_id);
+
+finish_neg:
+ complete(&btdev->init_completion);
+ kfree_skb(skb);
+ return ret;
+}
+
+static int nokia_recv_alive_packet(struct hci_dev *hdev, struct sk_buff *skb)
+{
+ struct hci_uart *hu = hci_get_drvdata(hdev);
+ struct nokia_bt_dev *btdev = hu->priv;
+ struct device *dev = &btdev->serdev->dev;
+ struct hci_nokia_alive_hdr *hdr;
+ struct hci_nokia_alive_pkt *pkt;
+ int ret = 0;
+
+ hdr = (struct hci_nokia_alive_hdr *)skb->data;
+ if (hdr->dlen != sizeof(*pkt)) {
+ dev_err(dev, "Corrupted alive message");
+ btdev->init_error = -EIO;
+ ret = -EIO;
+ goto finish_alive;
+ }
+
+ pkt = (struct hci_nokia_alive_pkt *)skb_pull(skb, sizeof(*hdr));
+
+ if (pkt->mid != NOKIA_ALIVE_RESP) {
+ dev_err(dev, "Alive received: invalid response: 0x%02x!",
+ pkt->mid);
+ btdev->init_error = -EINVAL;
+ ret = -EINVAL;
+ goto finish_alive;
+ }
+
+ dev_dbg(dev, "Alive received");
+
+finish_alive:
+ complete(&btdev->init_completion);
+ kfree_skb(skb);
+ return ret;
+}
+
+static int nokia_recv_radio(struct hci_dev *hdev, struct sk_buff *skb)
+{
+ /* Packets received on the dedicated radio channel are
+ * HCI events and so feed them back into the core.
+ */
+ hci_skb_pkt_type(skb) = HCI_EVENT_PKT;
+ return hci_recv_frame(hdev, skb);
+}
+
+/* Recv data */
+static const struct h4_recv_pkt nokia_recv_pkts[] = {
+ { H4_RECV_ACL, .recv = hci_recv_frame },
+ { H4_RECV_SCO, .recv = hci_recv_frame },
+ { H4_RECV_EVENT, .recv = hci_recv_frame },
+ { NOKIA_RECV_ALIVE, .recv = nokia_recv_alive_packet },
+ { NOKIA_RECV_NEG, .recv = nokia_recv_negotiation_packet },
+ { NOKIA_RECV_RADIO, .recv = nokia_recv_radio },
+};
+
+static int nokia_recv(struct hci_uart *hu, const void *data, int count)
+{
+ struct nokia_bt_dev *btdev = hu->priv;
+ struct device *dev = &btdev->serdev->dev;
+ int err;
+
+ if (!test_bit(HCI_UART_REGISTERED, &hu->flags))
+ return -EUNATCH;
+
+ btdev->rx_skb = h4_recv_buf(hu->hdev, btdev->rx_skb, data, count,
+ nokia_recv_pkts, ARRAY_SIZE(nokia_recv_pkts));
+ if (IS_ERR(btdev->rx_skb)) {
+ err = PTR_ERR(btdev->rx_skb);
+ dev_err(dev, "Frame reassembly failed (%d)", err);
+ btdev->rx_skb = NULL;
+ return err;
+ }
+
+ return count;
+}
+
+static struct sk_buff *nokia_dequeue(struct hci_uart *hu)
+{
+ struct nokia_bt_dev *btdev = hu->priv;
+ struct device *dev = &btdev->serdev->dev;
+ struct sk_buff *result = skb_dequeue(&btdev->txq);
+
+ if (!btdev->initialized)
+ return result;
+
+ if (btdev->tx_enabled == !!result)
+ return result;
+
+ if (result) {
+ pm_runtime_get_sync(dev);
+ gpiod_set_value_cansleep(btdev->wakeup_bt, 1);
+ } else {
+ serdev_device_wait_until_sent(btdev->serdev, 0);
+ gpiod_set_value_cansleep(btdev->wakeup_bt, 0);
+ pm_runtime_put(dev);
+ }
+
+ btdev->tx_enabled = !!result;
+
+ return result;
+}
+
+static const struct hci_uart_proto nokia_proto = {
+ .id = HCI_UART_NOKIA,
+ .name = "Nokia",
+ .open = nokia_open,
+ .close = nokia_close,
+ .recv = nokia_recv,
+ .enqueue = nokia_enqueue,
+ .dequeue = nokia_dequeue,
+ .flush = nokia_flush,
+ .setup = nokia_setup,
+ .manufacturer = 1,
+};
+
+static int nokia_bluetooth_serdev_probe(struct serdev_device *serdev)
+{
+ struct device *dev = &serdev->dev;
+ struct nokia_bt_dev *btdev;
+ struct clk *sysclk;
+ int err = 0;
+
+ btdev = devm_kzalloc(dev, sizeof(*btdev), GFP_KERNEL);
+ if (!btdev)
+ return -ENOMEM;
+
+ btdev->hu.serdev = btdev->serdev = serdev;
+ serdev_device_set_drvdata(serdev, btdev);
+
+ btdev->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(btdev->reset)) {
+ err = PTR_ERR(btdev->reset);
+ dev_err(dev, "could not get reset gpio: %d", err);
+ return err;
+ }
+
+ btdev->wakeup_host = devm_gpiod_get(dev, "host-wakeup", GPIOD_IN);
+ if (IS_ERR(btdev->wakeup_host)) {
+ err = PTR_ERR(btdev->wakeup_host);
+ dev_err(dev, "could not get host wakeup gpio: %d", err);
+ return err;
+ }
+
+ btdev->wake_irq = gpiod_to_irq(btdev->wakeup_host);
+
+ err = devm_request_threaded_irq(dev, btdev->wake_irq, NULL,
+ wakeup_handler,
+ IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+ "wakeup", btdev);
+ if (err) {
+ dev_err(dev, "could request wakeup irq: %d", err);
+ return err;
+ }
+
+ btdev->wakeup_bt = devm_gpiod_get(dev, "bluetooth-wakeup",
+ GPIOD_OUT_LOW);
+ if (IS_ERR(btdev->wakeup_bt)) {
+ err = PTR_ERR(btdev->wakeup_bt);
+ dev_err(dev, "could not get BT wakeup gpio: %d", err);
+ return err;
+ }
+
+ sysclk = devm_clk_get(dev, "sysclk");
+ if (IS_ERR(sysclk)) {
+ err = PTR_ERR(sysclk);
+ dev_err(dev, "could not get sysclk: %d", err);
+ return err;
+ }
+
+ clk_prepare_enable(sysclk);
+ btdev->sysclk_speed = clk_get_rate(sysclk);
+ clk_disable_unprepare(sysclk);
+
+ skb_queue_head_init(&btdev->txq);
+
+ btdev->hu.priv = btdev;
+ btdev->hu.alignment = 2; /* Nokia H4+ is word aligned */
+
+ err = hci_uart_register_device(&btdev->hu, &nokia_proto);
+ if (err) {
+ dev_err(dev, "could not register bluetooth uart: %d", err);
+ return err;
+ }
+
+ return 0;
+}
+
+static void nokia_bluetooth_serdev_remove(struct serdev_device *serdev)
+{
+ struct nokia_bt_dev *btdev = serdev_device_get_drvdata(serdev);
+ struct hci_uart *hu = &btdev->hu;
+ struct hci_dev *hdev = hu->hdev;
+
+ cancel_work_sync(&hu->write_work);
+
+ hci_unregister_dev(hdev);
+ hci_free_dev(hdev);
+ hu->proto->close(hu);
+
+ pm_runtime_disable(&btdev->serdev->dev);
+}
+
+static int nokia_bluetooth_runtime_suspend(struct device *dev)
+{
+ struct serdev_device *serdev = to_serdev_device(dev);
+
+ nokia_flow_control(serdev, false);
+ return 0;
+}
+
+static int nokia_bluetooth_runtime_resume(struct device *dev)
+{
+ struct serdev_device *serdev = to_serdev_device(dev);
+
+ nokia_flow_control(serdev, true);
+ return 0;
+}
+
+static const struct dev_pm_ops nokia_bluetooth_pm_ops = {
+ SET_RUNTIME_PM_OPS(nokia_bluetooth_runtime_suspend,
+ nokia_bluetooth_runtime_resume,
+ NULL)
+};
+
+#ifdef CONFIG_OF
+static const struct of_device_id nokia_bluetooth_of_match[] = {
+ { .compatible = "nokia,h4p-bluetooth", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, nokia_bluetooth_of_match);
+#endif
+
+static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
+ .probe = nokia_bluetooth_serdev_probe,
+ .remove = nokia_bluetooth_serdev_remove,
+ .driver = {
+ .name = "nokia-bluetooth",
+ .pm = &nokia_bluetooth_pm_ops,
+ .of_match_table = of_match_ptr(nokia_bluetooth_of_match),
+ },
+};
+
+module_serdev_device_driver(nokia_bluetooth_serdev_driver);
--
2.11.0
^ permalink raw reply related
* [PATCHv3 09/10] dt-bindings: net: bluetooth: Add nokia-bluetooth
From: Sebastian Reichel @ 2017-03-28 15:59 UTC (permalink / raw)
To: Sebastian Reichel, Marcel Holtmann, Gustavo Padovan,
Johan Hedberg, Rob Herring
Cc: Samuel Thibault, Pavel Machek, Tony Lindgren, Greg Kroah-Hartman,
Jiri Slaby, Mark Rutland, linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170328155939.31566-1-sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Add binding document for serial bluetooth chips using
Nokia H4+ protocol.
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Changes since PATCHv1:
* change compatible strings
* mention active high/low state for GPIOs
---
.../devicetree/bindings/net/nokia-bluetooth.txt | 51 ++++++++++++++++++++++
1 file changed, 51 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/nokia-bluetooth.txt
diff --git a/Documentation/devicetree/bindings/net/nokia-bluetooth.txt b/Documentation/devicetree/bindings/net/nokia-bluetooth.txt
new file mode 100644
index 000000000000..42be7dc9a70b
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/nokia-bluetooth.txt
@@ -0,0 +1,51 @@
+Nokia Bluetooth Chips
+---------------------
+
+Nokia phones often come with UART connected bluetooth chips from different
+vendors and modified device API. Those devices speak a protocol named H4+
+(also known as h4p) by Nokia, which is similar to the H4 protocol from the
+Bluetooth standard. In addition to the H4 protocol it specifies two more
+UART status lines for wakeup of UART transceivers to improve power management
+and a few new packet types used to negotiate uart speed.
+
+Required properties:
+
+ - compatible: should contain "nokia,h4p-bluetooth" as well as one of the following:
+ * "brcm,bcm2048-nokia"
+ * "ti,wl1271-bluetooth-nokia"
+ - reset-gpios: GPIO specifier, used to reset the BT module (active low)
+ - bluetooth-wakeup-gpios: GPIO specifier, used to wakeup the BT module (active high)
+ - host-wakeup-gpios: GPIO specifier, used to wakeup the host processor (active high)
+ - clock-names: should be "sysclk"
+ - clocks: should contain a clock specifier for every name in clock-names
+
+Optional properties:
+
+ - None
+
+Example:
+
+/ {
+ /* controlled (enabled/disabled) directly by BT module */
+ bluetooth_clk: vctcxo {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <38400000>;
+ };
+};
+
+&uart2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart2_pins>;
+
+ bluetooth {
+ compatible = "ti,wl1271-bluetooth-nokia", "nokia,h4p-bluetooth";
+
+ reset-gpios = <&gpio1 26 GPIO_ACTIVE_LOW>; /* gpio26 */
+ host-wakeup-gpios = <&gpio4 5 GPIO_ACTIVE_HIGH>; /* gpio101 */
+ bluetooth-wakeup-gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>; /* gpio37 */
+
+ clocks = <&bluetooth_clk>;
+ clock-names = "sysclk";
+ };
+};
--
2.11.0
^ permalink raw reply related
* [PATCHv3 08/10] Bluetooth: hci_serdev: allow modular drivers
From: Sebastian Reichel @ 2017-03-28 15:59 UTC (permalink / raw)
To: Sebastian Reichel, Marcel Holtmann, Gustavo Padovan,
Johan Hedberg, Rob Herring
Cc: Samuel Thibault, Pavel Machek, Tony Lindgren, Greg Kroah-Hartman,
Jiri Slaby, Mark Rutland, linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170328155939.31566-1-sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
For bluetooth protocol driver only supporting serdev it makes
sense to follow common practice and built them into their own
module.
Such modules need access to hci_uart_register_device and
hci_uart_tx_wakeup for using the common protocol helpers.
Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
New patch since patchset v2.
---
drivers/bluetooth/hci_ldisc.c | 1 +
drivers/bluetooth/hci_serdev.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 0ec8a94bd712..17bcbc13623f 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -134,6 +134,7 @@ int hci_uart_tx_wakeup(struct hci_uart *hu)
return 0;
}
+EXPORT_SYMBOL_GPL(hci_uart_tx_wakeup);
static void hci_uart_write_work(struct work_struct *work)
{
diff --git a/drivers/bluetooth/hci_serdev.c b/drivers/bluetooth/hci_serdev.c
index 3b8ac0ece3fb..7de0edc0ff8c 100644
--- a/drivers/bluetooth/hci_serdev.c
+++ b/drivers/bluetooth/hci_serdev.c
@@ -353,3 +353,4 @@ int hci_uart_register_device(struct hci_uart *hu,
p->close(hu);
return err;
}
+EXPORT_SYMBOL_GPL(hci_uart_register_device);
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCHv3 07/10] Bluetooth: hci_serdev: do not open device in hci open
From: Sebastian Reichel @ 2017-03-28 15:59 UTC (permalink / raw)
To: Sebastian Reichel, Marcel Holtmann, Gustavo Padovan,
Johan Hedberg, Rob Herring
Cc: Samuel Thibault, Pavel Machek, Tony Lindgren, Greg Kroah-Hartman,
Jiri Slaby, Mark Rutland, linux-bluetooth, linux-serial,
devicetree, linux-kernel
In-Reply-To: <20170328155939.31566-1-sre@kernel.org>
The device driver may need to communicate with the UART
device while the Bluetooth device is closed (e.g. due
to interrupts).
Acked-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
drivers/bluetooth/hci_serdev.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/bluetooth/hci_serdev.c b/drivers/bluetooth/hci_serdev.c
index f5ccb2c7ef92..3b8ac0ece3fb 100644
--- a/drivers/bluetooth/hci_serdev.c
+++ b/drivers/bluetooth/hci_serdev.c
@@ -104,13 +104,9 @@ static void hci_uart_write_work(struct work_struct *work)
/* Initialize device */
static int hci_uart_open(struct hci_dev *hdev)
{
- struct hci_uart *hu = hci_get_drvdata(hdev);
-
BT_DBG("%s %p", hdev->name, hdev);
- serdev_device_set_client_ops(hu->serdev, &hci_serdev_client_ops);
-
- return serdev_device_open(hu->serdev);
+ return 0;
}
/* Reset device */
@@ -136,15 +132,11 @@ static int hci_uart_flush(struct hci_dev *hdev)
/* Close device */
static int hci_uart_close(struct hci_dev *hdev)
{
- struct hci_uart *hu = hci_get_drvdata(hdev);
-
BT_DBG("hdev %p", hdev);
hci_uart_flush(hdev);
hdev->flush = NULL;
- serdev_device_close(hu->serdev);
-
return 0;
}
@@ -289,6 +281,8 @@ int hci_uart_register_device(struct hci_uart *hu,
BT_DBG("");
+ serdev_device_set_client_ops(hu->serdev, &hci_serdev_client_ops);
+
err = p->open(hu);
if (err)
return err;
--
2.11.0
^ permalink raw reply related
* [PATCHv3 06/10] Bluetooth: hci_uart: add serdev driver support library
From: Sebastian Reichel @ 2017-03-28 15:59 UTC (permalink / raw)
To: Sebastian Reichel, Marcel Holtmann, Gustavo Padovan,
Johan Hedberg, Rob Herring
Cc: Samuel Thibault, Pavel Machek, Tony Lindgren, Greg Kroah-Hartman,
Jiri Slaby, Mark Rutland, linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring
In-Reply-To: <20170328155939.31566-1-sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
From: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
This adds library functions for serdev based BT drivers. This is largely
copied from hci_ldisc.c and modified to use serdev calls. There's a little
bit of duplication, but I avoided intermixing this as the ldisc code should
eventually go away.
Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
Cc: Gustavo Padovan <gustavo-THi1TnShQwVAfugRpC6u6w@public.gmane.org>
Cc: Johan Hedberg <johan.hedberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: linux-bluetooth-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
[Fix style issues reported by Pavel]
Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
Changes since PATCHv1:
* Drop FSF address from license section
* Add error message if hu->proto->set_baudrate fails
* Remove useless label/goto in hci_uart_setup
* Use proper kerneldoc for hci_uart_write_wakeup
* Use proper kerneldoc for hci_uart_receive_buf
* Wrap hci_uart_register_device define to get < 80 chars
* Use do..while instead of goto restart in hci_uart_write_work
* The file checkpatch clean now
---
drivers/bluetooth/Makefile | 1 +
drivers/bluetooth/hci_serdev.c | 361 +++++++++++++++++++++++++++++++++++++++++
drivers/bluetooth/hci_uart.h | 4 +
3 files changed, 366 insertions(+)
create mode 100644 drivers/bluetooth/hci_serdev.c
diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
index 80627187c8b6..fd571689eed6 100644
--- a/drivers/bluetooth/Makefile
+++ b/drivers/bluetooth/Makefile
@@ -29,6 +29,7 @@ btmrvl-y := btmrvl_main.o
btmrvl-$(CONFIG_DEBUG_FS) += btmrvl_debugfs.o
hci_uart-y := hci_ldisc.o
+hci_uart-$(CONFIG_SERIAL_DEV_BUS) += hci_serdev.o
hci_uart-$(CONFIG_BT_HCIUART_H4) += hci_h4.o
hci_uart-$(CONFIG_BT_HCIUART_BCSP) += hci_bcsp.o
hci_uart-$(CONFIG_BT_HCIUART_LL) += hci_ll.o
diff --git a/drivers/bluetooth/hci_serdev.c b/drivers/bluetooth/hci_serdev.c
new file mode 100644
index 000000000000..f5ccb2c7ef92
--- /dev/null
+++ b/drivers/bluetooth/hci_serdev.c
@@ -0,0 +1,361 @@
+/*
+ * Bluetooth HCI serdev driver lib
+ *
+ * Copyright (C) 2017 Linaro, Ltd., Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
+ *
+ * Based on hci_ldisc.c:
+ *
+ * Copyright (C) 2000-2001 Qualcomm Incorporated
+ * Copyright (C) 2002-2003 Maxim Krasnyansky <maxk-zC7DfRvBq/JWk0Htik3J/w@public.gmane.org>
+ * Copyright (C) 2004-2005 Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/serdev.h>
+#include <linux/skbuff.h>
+
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci_core.h>
+
+#include "hci_uart.h"
+
+struct serdev_device_ops hci_serdev_client_ops;
+
+static inline void hci_uart_tx_complete(struct hci_uart *hu, int pkt_type)
+{
+ struct hci_dev *hdev = hu->hdev;
+
+ /* Update HCI stat counters */
+ switch (pkt_type) {
+ case HCI_COMMAND_PKT:
+ hdev->stat.cmd_tx++;
+ break;
+
+ case HCI_ACLDATA_PKT:
+ hdev->stat.acl_tx++;
+ break;
+
+ case HCI_SCODATA_PKT:
+ hdev->stat.sco_tx++;
+ break;
+ }
+}
+
+static inline struct sk_buff *hci_uart_dequeue(struct hci_uart *hu)
+{
+ struct sk_buff *skb = hu->tx_skb;
+
+ if (!skb)
+ skb = hu->proto->dequeue(hu);
+ else
+ hu->tx_skb = NULL;
+
+ return skb;
+}
+
+static void hci_uart_write_work(struct work_struct *work)
+{
+ struct hci_uart *hu = container_of(work, struct hci_uart, write_work);
+ struct serdev_device *serdev = hu->serdev;
+ struct hci_dev *hdev = hu->hdev;
+ struct sk_buff *skb;
+
+ /* REVISIT:
+ * should we cope with bad skbs or ->write() returning an error value?
+ */
+ do {
+ clear_bit(HCI_UART_TX_WAKEUP, &hu->tx_state);
+
+ while ((skb = hci_uart_dequeue(hu))) {
+ int len;
+
+ len = serdev_device_write_buf(serdev,
+ skb->data, skb->len);
+ hdev->stat.byte_tx += len;
+
+ skb_pull(skb, len);
+ if (skb->len) {
+ hu->tx_skb = skb;
+ break;
+ }
+
+ hci_uart_tx_complete(hu, hci_skb_pkt_type(skb));
+ kfree_skb(skb);
+ }
+ } while(test_bit(HCI_UART_TX_WAKEUP, &hu->tx_state));
+
+ clear_bit(HCI_UART_SENDING, &hu->tx_state);
+}
+
+/* ------- Interface to HCI layer ------ */
+
+/* Initialize device */
+static int hci_uart_open(struct hci_dev *hdev)
+{
+ struct hci_uart *hu = hci_get_drvdata(hdev);
+
+ BT_DBG("%s %p", hdev->name, hdev);
+
+ serdev_device_set_client_ops(hu->serdev, &hci_serdev_client_ops);
+
+ return serdev_device_open(hu->serdev);
+}
+
+/* Reset device */
+static int hci_uart_flush(struct hci_dev *hdev)
+{
+ struct hci_uart *hu = hci_get_drvdata(hdev);
+
+ BT_DBG("hdev %p serdev %p", hdev, hu->serdev);
+
+ if (hu->tx_skb) {
+ kfree_skb(hu->tx_skb); hu->tx_skb = NULL;
+ }
+
+ /* Flush any pending characters in the driver and discipline. */
+ serdev_device_write_flush(hu->serdev);
+
+ if (test_bit(HCI_UART_PROTO_READY, &hu->flags))
+ hu->proto->flush(hu);
+
+ return 0;
+}
+
+/* Close device */
+static int hci_uart_close(struct hci_dev *hdev)
+{
+ struct hci_uart *hu = hci_get_drvdata(hdev);
+
+ BT_DBG("hdev %p", hdev);
+
+ hci_uart_flush(hdev);
+ hdev->flush = NULL;
+
+ serdev_device_close(hu->serdev);
+
+ return 0;
+}
+
+/* Send frames from HCI layer */
+static int hci_uart_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
+{
+ struct hci_uart *hu = hci_get_drvdata(hdev);
+
+ BT_DBG("%s: type %d len %d", hdev->name, hci_skb_pkt_type(skb),
+ skb->len);
+
+ hu->proto->enqueue(hu, skb);
+
+ hci_uart_tx_wakeup(hu);
+
+ return 0;
+}
+
+static int hci_uart_setup(struct hci_dev *hdev)
+{
+ struct hci_uart *hu = hci_get_drvdata(hdev);
+ struct hci_rp_read_local_version *ver;
+ struct sk_buff *skb;
+ unsigned int speed;
+ int err;
+
+ /* Init speed if any */
+ if (hu->init_speed)
+ speed = hu->init_speed;
+ else if (hu->proto->init_speed)
+ speed = hu->proto->init_speed;
+ else
+ speed = 0;
+
+ if (speed)
+ serdev_device_set_baudrate(hu->serdev, speed);
+
+ /* Operational speed if any */
+ if (hu->oper_speed)
+ speed = hu->oper_speed;
+ else if (hu->proto->oper_speed)
+ speed = hu->proto->oper_speed;
+ else
+ speed = 0;
+
+ if (hu->proto->set_baudrate && speed) {
+ err = hu->proto->set_baudrate(hu, speed);
+ if (err)
+ BT_ERR("%s: failed to set baudrate", hdev->name);
+ else
+ serdev_device_set_baudrate(hu->serdev, speed);
+ }
+
+ if (hu->proto->setup)
+ return hu->proto->setup(hu);
+
+ if (!test_bit(HCI_UART_VND_DETECT, &hu->hdev_flags))
+ return 0;
+
+ skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_VERSION, 0, NULL,
+ HCI_INIT_TIMEOUT);
+ if (IS_ERR(skb)) {
+ BT_ERR("%s: Reading local version information failed (%ld)",
+ hdev->name, PTR_ERR(skb));
+ return 0;
+ }
+
+ if (skb->len != sizeof(*ver)) {
+ BT_ERR("%s: Event length mismatch for version information",
+ hdev->name);
+ }
+
+ kfree_skb(skb);
+ return 0;
+}
+
+/** hci_uart_write_wakeup - transmit buffer wakeup
+ * @serdev: serial device
+ *
+ * This function is called by the serdev framework when it accepts
+ * more data being sent.
+ */
+static void hci_uart_write_wakeup(struct serdev_device *serdev)
+{
+ struct hci_uart *hu = serdev_device_get_drvdata(serdev);
+
+ BT_DBG("");
+
+ if (!hu || serdev != hu->serdev) {
+ WARN_ON(1);
+ return;
+ }
+
+ if (test_bit(HCI_UART_PROTO_READY, &hu->flags))
+ hci_uart_tx_wakeup(hu);
+}
+
+/** hci_uart_receive_buf - receive buffer wakeup
+ * @serdev: serial device
+ * @data: pointer to received data
+ * @count: count of received data in bytes
+ *
+ * This function is called by the serdev framework when it received data
+ * in the RX buffer.
+ *
+ * Return: number of processed bytes
+ */
+static int hci_uart_receive_buf(struct serdev_device *serdev, const u8 *data,
+ size_t count)
+{
+ struct hci_uart *hu = serdev_device_get_drvdata(serdev);
+
+ if (!hu || serdev != hu->serdev) {
+ WARN_ON(1);
+ return 0;
+ }
+
+ if (!test_bit(HCI_UART_PROTO_READY, &hu->flags))
+ return 0;
+
+ /* It does not need a lock here as it is already protected by a mutex in
+ * tty caller
+ */
+ hu->proto->recv(hu, data, count);
+
+ if (hu->hdev)
+ hu->hdev->stat.byte_rx += count;
+
+ return count;
+}
+
+struct serdev_device_ops hci_serdev_client_ops = {
+ .receive_buf = hci_uart_receive_buf,
+ .write_wakeup = hci_uart_write_wakeup,
+};
+
+int hci_uart_register_device(struct hci_uart *hu,
+ const struct hci_uart_proto *p)
+{
+ int err;
+ struct hci_dev *hdev;
+
+ BT_DBG("");
+
+ err = p->open(hu);
+ if (err)
+ return err;
+
+ hu->proto = p;
+ set_bit(HCI_UART_PROTO_READY, &hu->flags);
+
+ /* Initialize and register HCI device */
+ hdev = hci_alloc_dev();
+ if (!hdev) {
+ BT_ERR("Can't allocate HCI device");
+ err = -ENOMEM;
+ goto err_alloc;
+ }
+
+ hu->hdev = hdev;
+
+ hdev->bus = HCI_UART;
+ hci_set_drvdata(hdev, hu);
+
+ INIT_WORK(&hu->write_work, hci_uart_write_work);
+
+ /* Only when vendor specific setup callback is provided, consider
+ * the manufacturer information valid. This avoids filling in the
+ * value for Ericsson when nothing is specified.
+ */
+ if (hu->proto->setup)
+ hdev->manufacturer = hu->proto->manufacturer;
+
+ hdev->open = hci_uart_open;
+ hdev->close = hci_uart_close;
+ hdev->flush = hci_uart_flush;
+ hdev->send = hci_uart_send_frame;
+ hdev->setup = hci_uart_setup;
+ SET_HCIDEV_DEV(hdev, &hu->serdev->dev);
+
+ if (test_bit(HCI_UART_RAW_DEVICE, &hu->hdev_flags))
+ set_bit(HCI_QUIRK_RAW_DEVICE, &hdev->quirks);
+
+ if (test_bit(HCI_UART_EXT_CONFIG, &hu->hdev_flags))
+ set_bit(HCI_QUIRK_EXTERNAL_CONFIG, &hdev->quirks);
+
+ if (!test_bit(HCI_UART_RESET_ON_INIT, &hu->hdev_flags))
+ set_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks);
+
+ if (test_bit(HCI_UART_CREATE_AMP, &hu->hdev_flags))
+ hdev->dev_type = HCI_AMP;
+ else
+ hdev->dev_type = HCI_PRIMARY;
+
+ if (test_bit(HCI_UART_INIT_PENDING, &hu->hdev_flags))
+ return 0;
+
+ if (hci_register_dev(hdev) < 0) {
+ BT_ERR("Can't register HCI device");
+ err = -ENODEV;
+ goto err_register;
+ }
+
+ set_bit(HCI_UART_REGISTERED, &hu->flags);
+
+ return 0;
+
+err_register:
+ hci_free_dev(hdev);
+err_alloc:
+ clear_bit(HCI_UART_PROTO_READY, &hu->flags);
+ p->close(hu);
+ return err;
+}
diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
index 4aff50960cac..1b41c661bbb8 100644
--- a/drivers/bluetooth/hci_uart.h
+++ b/drivers/bluetooth/hci_uart.h
@@ -58,6 +58,7 @@
#define HCI_UART_VND_DETECT 5
struct hci_uart;
+struct serdev_device;
struct hci_uart_proto {
unsigned int id;
@@ -77,6 +78,7 @@ struct hci_uart_proto {
struct hci_uart {
struct tty_struct *tty;
+ struct serdev_device *serdev;
struct hci_dev *hdev;
unsigned long flags;
unsigned long hdev_flags;
@@ -108,6 +110,8 @@ struct hci_uart {
int hci_uart_register_proto(const struct hci_uart_proto *p);
int hci_uart_unregister_proto(const struct hci_uart_proto *p);
+int hci_uart_register_device(struct hci_uart *hu, const struct hci_uart_proto *p);
+
int hci_uart_tx_wakeup(struct hci_uart *hu);
int hci_uart_init_ready(struct hci_uart *hu);
void hci_uart_init_tty(struct hci_uart *hu);
--
2.11.0
^ permalink raw reply related
* [PATCHv3 05/10] Bluetooth: hci_uart: add support for word alignment
From: Sebastian Reichel @ 2017-03-28 15:59 UTC (permalink / raw)
To: Sebastian Reichel, Marcel Holtmann, Gustavo Padovan,
Johan Hedberg, Rob Herring
Cc: Samuel Thibault, Pavel Machek, Tony Lindgren, Greg Kroah-Hartman,
Jiri Slaby, Mark Rutland, linux-bluetooth, linux-serial,
devicetree, linux-kernel
In-Reply-To: <20170328155939.31566-1-sre@kernel.org>
This will be used by Nokia's H4+ protocol, which
uses 2-byte aligned packets.
Acked-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
Changes since PATCHv1:
* use u8 instead of uint8_t
---
drivers/bluetooth/hci_h4.c | 17 +++++++++++++++++
drivers/bluetooth/hci_ldisc.c | 4 ++++
drivers/bluetooth/hci_uart.h | 3 +++
3 files changed, 24 insertions(+)
diff --git a/drivers/bluetooth/hci_h4.c b/drivers/bluetooth/hci_h4.c
index 635597b6e168..82e5a32b87a4 100644
--- a/drivers/bluetooth/hci_h4.c
+++ b/drivers/bluetooth/hci_h4.c
@@ -171,9 +171,20 @@ struct sk_buff *h4_recv_buf(struct hci_dev *hdev, struct sk_buff *skb,
const unsigned char *buffer, int count,
const struct h4_recv_pkt *pkts, int pkts_count)
{
+ struct hci_uart *hu = hci_get_drvdata(hdev);
+ u8 alignment = hu->alignment;
+
while (count) {
int i, len;
+ /* remove padding bytes from buffer */
+ for (; hu->padding && count > 0; hu->padding--) {
+ count--;
+ buffer++;
+ }
+ if (!count)
+ break;
+
if (!skb) {
for (i = 0; i < pkts_count; i++) {
if (buffer[0] != (&pkts[i])->type)
@@ -253,11 +264,17 @@ struct sk_buff *h4_recv_buf(struct hci_dev *hdev, struct sk_buff *skb,
}
if (!dlen) {
+ hu->padding = (skb->len - 1) % alignment;
+ hu->padding = (alignment - hu->padding) % alignment;
+
/* No more data, complete frame */
(&pkts[i])->recv(hdev, skb);
skb = NULL;
}
} else {
+ hu->padding = (skb->len - 1) % alignment;
+ hu->padding = (alignment - hu->padding) % alignment;
+
/* Complete frame */
(&pkts[i])->recv(hdev, skb);
skb = NULL;
diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 9497c469efd2..0ec8a94bd712 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -459,6 +459,10 @@ static int hci_uart_tty_open(struct tty_struct *tty)
hu->tty = tty;
tty->receive_room = 65536;
+ /* disable alignment support by default */
+ hu->alignment = 1;
+ hu->padding = 0;
+
INIT_WORK(&hu->init_ready, hci_uart_init_work);
INIT_WORK(&hu->write_work, hci_uart_write_work);
diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
index 070139513e65..4aff50960cac 100644
--- a/drivers/bluetooth/hci_uart.h
+++ b/drivers/bluetooth/hci_uart.h
@@ -92,6 +92,9 @@ struct hci_uart {
unsigned int init_speed;
unsigned int oper_speed;
+
+ u8 alignment;
+ u8 padding;
};
/* HCI_UART proto flag bits */
--
2.11.0
^ permalink raw reply related
* [PATCHv3 04/10] serdev: add helpers for cts and rts handling
From: Sebastian Reichel @ 2017-03-28 15:59 UTC (permalink / raw)
To: Sebastian Reichel, Marcel Holtmann, Gustavo Padovan,
Johan Hedberg, Rob Herring
Cc: Samuel Thibault, Pavel Machek, Tony Lindgren, Greg Kroah-Hartman,
Jiri Slaby, Mark Rutland, linux-bluetooth, linux-serial,
devicetree, linux-kernel
In-Reply-To: <20170328155939.31566-1-sre@kernel.org>
Add serdev helper functions for handling of cts and rts
lines using the serdev's tiocm functions.
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
Changes since PATCHv2:
* use time_is_after_jiffies(xyz) instead of !time_after(jiffies, xyz)
* drop OUT2 handling in rts function
---
include/linux/serdev.h | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index e29a270f603c..37395b8eb8f1 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -16,6 +16,7 @@
#include <linux/types.h>
#include <linux/device.h>
#include <linux/termios.h>
+#include <linux/delay.h>
struct serdev_controller;
struct serdev_device;
@@ -254,6 +255,36 @@ static inline int serdev_device_write_room(struct serdev_device *sdev)
#endif /* CONFIG_SERIAL_DEV_BUS */
+static inline bool serdev_device_get_cts(struct serdev_device *serdev)
+{
+ int status = serdev_device_get_tiocm(serdev);
+ return !!(status & TIOCM_CTS);
+}
+
+static inline int serdev_device_wait_for_cts(struct serdev_device *serdev, bool state, int timeout_ms)
+{
+ unsigned long timeout;
+ bool signal;
+
+ timeout = jiffies + msecs_to_jiffies(timeout_ms);
+ while (time_is_after_jiffies(timeout)) {
+ signal = serdev_device_get_cts(serdev);
+ if (signal == state)
+ return 0;
+ usleep_range(1000, 2000);
+ }
+
+ return -ETIMEDOUT;
+}
+
+static inline int serdev_device_set_rts(struct serdev_device *serdev, bool enable)
+{
+ if (enable)
+ return serdev_device_set_tiocm(serdev, TIOCM_RTS, 0);
+ else
+ return serdev_device_set_tiocm(serdev, 0, TIOCM_RTS);
+}
+
/*
* serdev hooks into TTY core
*/
--
2.11.0
^ permalink raw reply related
* [PATCHv3 03/10] serdev: implement get/set tiocm
From: Sebastian Reichel @ 2017-03-28 15:59 UTC (permalink / raw)
To: Sebastian Reichel, Marcel Holtmann, Gustavo Padovan,
Johan Hedberg, Rob Herring
Cc: Samuel Thibault, Pavel Machek, Tony Lindgren, Greg Kroah-Hartman,
Jiri Slaby, Mark Rutland, linux-bluetooth, linux-serial,
devicetree, linux-kernel
In-Reply-To: <20170328155939.31566-1-sre@kernel.org>
Add method for getting and setting tiocm.
Acked-by: Pavel Machek <pavel@ucw.cz>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
drivers/tty/serdev/core.c | 22 ++++++++++++++++++++++
drivers/tty/serdev/serdev-ttyport.c | 24 ++++++++++++++++++++++++
include/linux/serdev.h | 13 +++++++++++++
3 files changed, 59 insertions(+)
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index a63b74031e22..1e1cbae3a0ea 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -184,6 +184,28 @@ void serdev_device_wait_until_sent(struct serdev_device *serdev, long timeout)
}
EXPORT_SYMBOL_GPL(serdev_device_wait_until_sent);
+int serdev_device_get_tiocm(struct serdev_device *serdev)
+{
+ struct serdev_controller *ctrl = serdev->ctrl;
+
+ if (!ctrl || !ctrl->ops->get_tiocm)
+ return -ENOTSUPP;
+
+ return ctrl->ops->get_tiocm(ctrl);
+}
+EXPORT_SYMBOL_GPL(serdev_device_get_tiocm);
+
+int serdev_device_set_tiocm(struct serdev_device *serdev, int set, int clear)
+{
+ struct serdev_controller *ctrl = serdev->ctrl;
+
+ if (!ctrl || !ctrl->ops->set_tiocm)
+ return -ENOTSUPP;
+
+ return ctrl->ops->set_tiocm(ctrl, set, clear);
+}
+EXPORT_SYMBOL_GPL(serdev_device_set_tiocm);
+
static int serdev_drv_probe(struct device *dev)
{
const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
index 50dc75c4d204..487c88f6aa0e 100644
--- a/drivers/tty/serdev/serdev-ttyport.c
+++ b/drivers/tty/serdev/serdev-ttyport.c
@@ -176,6 +176,28 @@ static void ttyport_wait_until_sent(struct serdev_controller *ctrl, long timeout
tty_wait_until_sent(tty, timeout);
}
+static int ttyport_get_tiocm(struct serdev_controller *ctrl)
+{
+ struct serport *serport = serdev_controller_get_drvdata(ctrl);
+ struct tty_struct *tty = serport->tty;
+
+ if (!tty->ops->tiocmget)
+ return -ENOTSUPP;
+
+ return tty->driver->ops->tiocmget(tty);
+}
+
+static int ttyport_set_tiocm(struct serdev_controller *ctrl, unsigned int set, unsigned int clear)
+{
+ struct serport *serport = serdev_controller_get_drvdata(ctrl);
+ struct tty_struct *tty = serport->tty;
+
+ if (!tty->ops->tiocmset)
+ return -ENOTSUPP;
+
+ return tty->driver->ops->tiocmset(tty, set, clear);
+}
+
static const struct serdev_controller_ops ctrl_ops = {
.write_buf = ttyport_write_buf,
.write_flush = ttyport_write_flush,
@@ -185,6 +207,8 @@ static const struct serdev_controller_ops ctrl_ops = {
.set_flow_control = ttyport_set_flow_control,
.set_baudrate = ttyport_set_baudrate,
.wait_until_sent = ttyport_wait_until_sent,
+ .get_tiocm = ttyport_get_tiocm,
+ .set_tiocm = ttyport_set_tiocm,
};
struct device *serdev_tty_port_register(struct tty_port *port,
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index a308b206d204..e29a270f603c 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -15,6 +15,7 @@
#include <linux/types.h>
#include <linux/device.h>
+#include <linux/termios.h>
struct serdev_controller;
struct serdev_device;
@@ -82,6 +83,8 @@ struct serdev_controller_ops {
void (*set_flow_control)(struct serdev_controller *, bool);
unsigned int (*set_baudrate)(struct serdev_controller *, unsigned int);
void (*wait_until_sent)(struct serdev_controller *, long);
+ int (*get_tiocm)(struct serdev_controller *);
+ int (*set_tiocm)(struct serdev_controller *, unsigned int, unsigned int);
};
/**
@@ -188,6 +191,8 @@ void serdev_device_close(struct serdev_device *);
unsigned int serdev_device_set_baudrate(struct serdev_device *, unsigned int);
void serdev_device_set_flow_control(struct serdev_device *, bool);
void serdev_device_wait_until_sent(struct serdev_device *, long);
+int serdev_device_get_tiocm(struct serdev_device *);
+int serdev_device_set_tiocm(struct serdev_device *, int, int);
int serdev_device_write_buf(struct serdev_device *, const unsigned char *, size_t);
void serdev_device_write_flush(struct serdev_device *);
int serdev_device_write_room(struct serdev_device *);
@@ -226,6 +231,14 @@ static inline unsigned int serdev_device_set_baudrate(struct serdev_device *sdev
}
static inline void serdev_device_set_flow_control(struct serdev_device *sdev, bool enable) {}
static inline void serdev_device_wait_until_sent(struct serdev_device *sdev, long timeout) {}
+static inline int serdev_device_get_tiocm(struct serdev_device *serdev)
+{
+ return -ENOTSUPP;
+}
+static inline int serdev_device_set_tiocm(struct serdev_device *serdev, int set, int clear)
+{
+ return -ENOTSUPP;
+}
static inline int serdev_device_write_buf(struct serdev_device *sdev, const unsigned char *buf, size_t count)
{
return -ENODEV;
--
2.11.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox