* Re: [GIT PULL] TTY/Serial driver fixes for 4.11-rc4
From: Greg KH @ 2017-05-03 12:01 UTC (permalink / raw)
To: Vegard Nossum
Cc: Dmitry Vyukov, Linus Torvalds, Jiri Slaby, Andrew Morton, LKML,
linux-serial
In-Reply-To: <CAOMGZ=GJah0GNXzscGGkYgScbZaeSM_TpxmrLZuxhrqWmBX9BQ@mail.gmail.com>
On Tue, May 02, 2017 at 11:52:40PM +0200, Vegard Nossum wrote:
> On 2 May 2017 at 18:35, Dmitry Vyukov <dvyukov@google.com> wrote:
> > On Fri, Apr 14, 2017 at 2:30 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> >> On Fri, Apr 14, 2017 at 11:41:26AM +0200, Vegard Nossum wrote:
> >>> On 13 April 2017 at 20:34, Greg KH <gregkh@linuxfoundation.org> wrote:
> >>> > On Thu, Apr 13, 2017 at 09:07:40AM -0700, Linus Torvalds wrote:
> >>> >> On Thu, Apr 13, 2017 at 3:50 AM, Vegard Nossum <vegard.nossum@gmail.com> wrote:
> >>> So the original problem is that the vmalloc() in n_tty_open() can
> >>> fail, and that will panic in tty_set_ldisc()/tty_ldisc_restore()
> >>> because of its unwillingness to proceed if the tty doesn't have an
> >>> ldisc.
> >>>
> >>> Dmitry fixed this by allowing tty->ldisc == NULL in the case of memory
> >>> allocation failure as we can see from the comment in tty_set_ldisc().
> >>>
> >>> Unfortunately, it would appear that some other bits of code do not
> >>> like tty->ldisc == NULL (other than the crash in this thread, I saw
> >>> 2-3 similar crashes in other functions, e.g. poll()). I see two
> >>> possibilities:
> >>>
> >>> 1) make other code handle tty->ldisc == NULL.
> >>>
> >>> 2) don't close/free the old ldisc until the new one has been
> >>> successfully created/initialised/opened/attached to the tty, and
> >>> return an error to userspace if changing it failed.
> >>>
> >>> I'm leaning towards #2 as the more obviously correct fix, it makes
> >>> tty_set_ldisc() transactional, the fix seems limited in scope to
> >>> tty_set_ldisc() itself, and we don't need to make every other bit of
> >>> code that uses tty->ldisc handle the NULL case.
> >>
> >> That sounds reasonable to me, care to work on a patch for this?
> >
> > Vegard, do you know how to do this?
> > That was first thing that I tried, but I did not manage to make it
> > work. disc is tied to tty, so it's not that one can create a fully
> > initialized disc on the side and then simply swap pointers. Looking at
> > the code now, there is at least TTY_LDISC_OPEN bit in tty. But as far
> > as I remember there were more fundamental problems. Or maybe I just
> > did not try too hard.
>
> I had a look at it but like you said, the tty/ldisc relationship is
> complicated :-/
>
> Maybe we can split up ldisc initialisation into two methods so that
> the first one (e.g. ->alloc) does all the allocation and is allowed to
> fail and the second one (e.g. ->open) is not allowed to fail. Then you
> can allocate a new ldisc without freeing the old one and only swap
> them over if the allocation succeeded.
>
> That would require fixing up ->open for all the ldisc drivers though,
> I'm not sure how easy/feasible it is.
We don't have that many ldisc drivers, so it shouldn't be that hard to
change to use this. It makes a lot more sense to fix this the correct
way like this.
> I'll think about possible solutions, but I have no prior experience
> with the tty code. In the meantime syzkaller also hit a couple of
> other fun tty/pty bugs including a write/ioctl race that results in
> buffer overflow :-/
Ugh, let me know what they are and we'll work to fix them.
Thanks for all of the work you all are doing in finding these issues, I
might grumble about having to fix this and what a pain it is, but it's
just me being grumpy about the tty code, not your effort. Your effort
is much appreciated.
thanks,
greg k-h
^ permalink raw reply
* Re: [GIT PULL] TTY/Serial driver fixes for 4.11-rc4
From: Dmitry Vyukov @ 2017-05-03 11:25 UTC (permalink / raw)
To: Vegard Nossum
Cc: Greg KH, Linus Torvalds, Jiri Slaby, Andrew Morton, LKML,
linux-serial
In-Reply-To: <CAOMGZ=GJah0GNXzscGGkYgScbZaeSM_TpxmrLZuxhrqWmBX9BQ@mail.gmail.com>
On Tue, May 2, 2017 at 11:52 PM, Vegard Nossum <vegard.nossum@gmail.com> wrote:
>>>> So the original problem is that the vmalloc() in n_tty_open() can
>>>> fail, and that will panic in tty_set_ldisc()/tty_ldisc_restore()
>>>> because of its unwillingness to proceed if the tty doesn't have an
>>>> ldisc.
>>>>
>>>> Dmitry fixed this by allowing tty->ldisc == NULL in the case of memory
>>>> allocation failure as we can see from the comment in tty_set_ldisc().
>>>>
>>>> Unfortunately, it would appear that some other bits of code do not
>>>> like tty->ldisc == NULL (other than the crash in this thread, I saw
>>>> 2-3 similar crashes in other functions, e.g. poll()). I see two
>>>> possibilities:
>>>>
>>>> 1) make other code handle tty->ldisc == NULL.
>>>>
>>>> 2) don't close/free the old ldisc until the new one has been
>>>> successfully created/initialised/opened/attached to the tty, and
>>>> return an error to userspace if changing it failed.
>>>>
>>>> I'm leaning towards #2 as the more obviously correct fix, it makes
>>>> tty_set_ldisc() transactional, the fix seems limited in scope to
>>>> tty_set_ldisc() itself, and we don't need to make every other bit of
>>>> code that uses tty->ldisc handle the NULL case.
>>>
>>> That sounds reasonable to me, care to work on a patch for this?
>>
>> Vegard, do you know how to do this?
>> That was first thing that I tried, but I did not manage to make it
>> work. disc is tied to tty, so it's not that one can create a fully
>> initialized disc on the side and then simply swap pointers. Looking at
>> the code now, there is at least TTY_LDISC_OPEN bit in tty. But as far
>> as I remember there were more fundamental problems. Or maybe I just
>> did not try too hard.
>
> I had a look at it but like you said, the tty/ldisc relationship is
> complicated :-/
>
> Maybe we can split up ldisc initialisation into two methods so that
> the first one (e.g. ->alloc) does all the allocation and is allowed to
> fail and the second one (e.g. ->open) is not allowed to fail. Then you
> can allocate a new ldisc without freeing the old one and only swap
> them over if the allocation succeeded.
>
> That would require fixing up ->open for all the ldisc drivers though,
> I'm not sure how easy/feasible it is.
What do you think about making all tty code deal with NULL disc?
It seems that most of code is already prepared for this.
> I'll think about possible solutions, but I have no prior experience
> with the tty code. In the meantime syzkaller also hit a couple of
> other fun tty/pty bugs including a write/ioctl race that results in
> buffer overflow :-/
>
>
> Vegard
^ permalink raw reply
* Re: [GIT PULL] TTY/Serial driver fixes for 4.11-rc4
From: Vegard Nossum @ 2017-05-02 21:52 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: Greg KH, Linus Torvalds, Jiri Slaby, Andrew Morton, LKML,
linux-serial
In-Reply-To: <CACT4Y+acU1xTZEo+oeBvZL__c12bzcSPa0CBfDiw4j7+CUjfDg@mail.gmail.com>
On 2 May 2017 at 18:35, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Fri, Apr 14, 2017 at 2:30 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> On Fri, Apr 14, 2017 at 11:41:26AM +0200, Vegard Nossum wrote:
>>> On 13 April 2017 at 20:34, Greg KH <gregkh@linuxfoundation.org> wrote:
>>> > On Thu, Apr 13, 2017 at 09:07:40AM -0700, Linus Torvalds wrote:
>>> >> On Thu, Apr 13, 2017 at 3:50 AM, Vegard Nossum <vegard.nossum@gmail.com> wrote:
>>> So the original problem is that the vmalloc() in n_tty_open() can
>>> fail, and that will panic in tty_set_ldisc()/tty_ldisc_restore()
>>> because of its unwillingness to proceed if the tty doesn't have an
>>> ldisc.
>>>
>>> Dmitry fixed this by allowing tty->ldisc == NULL in the case of memory
>>> allocation failure as we can see from the comment in tty_set_ldisc().
>>>
>>> Unfortunately, it would appear that some other bits of code do not
>>> like tty->ldisc == NULL (other than the crash in this thread, I saw
>>> 2-3 similar crashes in other functions, e.g. poll()). I see two
>>> possibilities:
>>>
>>> 1) make other code handle tty->ldisc == NULL.
>>>
>>> 2) don't close/free the old ldisc until the new one has been
>>> successfully created/initialised/opened/attached to the tty, and
>>> return an error to userspace if changing it failed.
>>>
>>> I'm leaning towards #2 as the more obviously correct fix, it makes
>>> tty_set_ldisc() transactional, the fix seems limited in scope to
>>> tty_set_ldisc() itself, and we don't need to make every other bit of
>>> code that uses tty->ldisc handle the NULL case.
>>
>> That sounds reasonable to me, care to work on a patch for this?
>
> Vegard, do you know how to do this?
> That was first thing that I tried, but I did not manage to make it
> work. disc is tied to tty, so it's not that one can create a fully
> initialized disc on the side and then simply swap pointers. Looking at
> the code now, there is at least TTY_LDISC_OPEN bit in tty. But as far
> as I remember there were more fundamental problems. Or maybe I just
> did not try too hard.
I had a look at it but like you said, the tty/ldisc relationship is
complicated :-/
Maybe we can split up ldisc initialisation into two methods so that
the first one (e.g. ->alloc) does all the allocation and is allowed to
fail and the second one (e.g. ->open) is not allowed to fail. Then you
can allocate a new ldisc without freeing the old one and only swap
them over if the allocation succeeded.
That would require fixing up ->open for all the ldisc drivers though,
I'm not sure how easy/feasible it is.
I'll think about possible solutions, but I have no prior experience
with the tty code. In the meantime syzkaller also hit a couple of
other fun tty/pty bugs including a write/ioctl race that results in
buffer overflow :-/
Vegard
^ permalink raw reply
* Re: [GIT PULL] TTY/Serial driver fixes for 4.11-rc4
From: Dmitry Vyukov @ 2017-05-02 16:35 UTC (permalink / raw)
To: Greg KH
Cc: Vegard Nossum, Linus Torvalds, Jiri Slaby, Andrew Morton, LKML,
linux-serial
In-Reply-To: <20170414123029.GA17217@kroah.com>
On Fri, Apr 14, 2017 at 2:30 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Apr 14, 2017 at 11:41:26AM +0200, Vegard Nossum wrote:
>> On 13 April 2017 at 20:34, Greg KH <gregkh@linuxfoundation.org> wrote:
>> > On Thu, Apr 13, 2017 at 09:07:40AM -0700, Linus Torvalds wrote:
>> >> On Thu, Apr 13, 2017 at 3:50 AM, Vegard Nossum <vegard.nossum@gmail.com> wrote:
>> >> >
>> >> > I've bisected a syzkaller crash down to this commit
>> >> > (5362544bebe85071188dd9e479b5a5040841c895). The crash is:
>> >> >
>> >> > [ 25.137552] BUG: unable to handle kernel paging request at 0000000000002280
>> >> > [ 25.137579] IP: mutex_lock_interruptible+0xb/0x30
>> >>
>> >> It would seem to be the
>> >>
>> >> if (mutex_lock_interruptible(&ldata->atomic_read_lock))
>> >>
>> >> call in n_tty_read(), the offset is about right for a NULL 'ldata'
>> >> pointer (it's a big structure, it has a couple of character buffers of
>> >> size N_TTY_BUF_SIZE).
>> >>
>> >> I don't see the obvious fix, so I suspect at this point we should just
>> >> revert, as that commit seems to introduce worse problems that it is
>> >> supposed to fix. Greg?
>> >
>> > Unless Dmitry has a better idea, I will just revert it and send you the
>> > pull request in a day or so.
>>
>> I don't think we need to rush a revert, I'd hope there's a way to fix
>> it properly.
>
> For this late in the release cycle, for something as complex as tty
> ldisc handling, for an issue that has been present for over a decade,
> the safest thing right now is to go back to the old well-known code by
> applying a revert :)
>
>> So the original problem is that the vmalloc() in n_tty_open() can
>> fail, and that will panic in tty_set_ldisc()/tty_ldisc_restore()
>> because of its unwillingness to proceed if the tty doesn't have an
>> ldisc.
>>
>> Dmitry fixed this by allowing tty->ldisc == NULL in the case of memory
>> allocation failure as we can see from the comment in tty_set_ldisc().
>>
>> Unfortunately, it would appear that some other bits of code do not
>> like tty->ldisc == NULL (other than the crash in this thread, I saw
>> 2-3 similar crashes in other functions, e.g. poll()). I see two
>> possibilities:
>>
>> 1) make other code handle tty->ldisc == NULL.
>>
>> 2) don't close/free the old ldisc until the new one has been
>> successfully created/initialised/opened/attached to the tty, and
>> return an error to userspace if changing it failed.
>>
>> I'm leaning towards #2 as the more obviously correct fix, it makes
>> tty_set_ldisc() transactional, the fix seems limited in scope to
>> tty_set_ldisc() itself, and we don't need to make every other bit of
>> code that uses tty->ldisc handle the NULL case.
>
> That sounds reasonable to me, care to work on a patch for this?
Vegard, do you know how to do this?
That was first thing that I tried, but I did not manage to make it
work. disc is tied to tty, so it's not that one can create a fully
initialized disc on the side and then simply swap pointers. Looking at
the code now, there is at least TTY_LDISC_OPEN bit in tty. But as far
as I remember there were more fundamental problems. Or maybe I just
did not try too hard.
^ permalink raw reply
* Re: [PATCH] serdev: Restore serdev_device_write_buf for atomic context
From: Johan Hovold @ 2017-05-02 13:18 UTC (permalink / raw)
To: Rob Herring
Cc: Johan Hovold, Stefan Wahren, Greg Kroah-Hartman, Jiri Slaby,
Sebastian Reichel, Guenter Roeck, Andy Shevchenko, Andrey Smirnov,
linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CAL_Jsq+QOv6G2EFX8RpE1gk2S9M5VrndwQNNCjM0GqZ+2PXz3A@mail.gmail.com>
On Tue, May 02, 2017 at 07:41:34AM -0500, Rob Herring wrote:
> On Tue, May 2, 2017 at 4:06 AM, Johan Hovold <johan@kernel.org> wrote:
> > On Fri, Apr 28, 2017 at 01:47:21PM +0200, Stefan Wahren wrote:
> >> Starting with commit 6fe729c4bdae ("serdev: Add serdev_device_write
> >> subroutine") the function serdev_device_write_buf cannot be used in
> >> atomic context anymore (mutex_lock is sleeping). So restore the old
> >> behavior.
> >
> > Yeah, preventing use in atomic context seems unnecessary, although any
> > clients writing must now deal with serialisation themselves (as before,
> > and as they should).
>
> We could just remove the mutex for serdev_device_write and always make
> the client responsible for serialization.
That sounds reasonable.
> > Calling wait_for_completion in the non-blocking case was also needlessly
> > inefficient.
>
> It won't be called because count should be 0.
That's not guaranteed; count would be nonzero whenever the tty
driver does not accept the full buffer and then we'd currently end up
calling wait_for_completion_timeout() with a zero-timeout instead of
just returning immediately.
Johan
^ permalink raw reply
* Re: [PATCH] serdev: Restore serdev_device_write_buf for atomic context
From: Rob Herring @ 2017-05-02 12:41 UTC (permalink / raw)
To: Johan Hovold
Cc: Stefan Wahren, Greg Kroah-Hartman, Jiri Slaby, Sebastian Reichel,
Guenter Roeck, Andy Shevchenko, Andrey Smirnov,
linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20170502090638.GB2973@localhost>
On Tue, May 2, 2017 at 4:06 AM, Johan Hovold <johan@kernel.org> wrote:
> On Fri, Apr 28, 2017 at 01:47:21PM +0200, Stefan Wahren wrote:
>> Starting with commit 6fe729c4bdae ("serdev: Add serdev_device_write
>> subroutine") the function serdev_device_write_buf cannot be used in
>> atomic context anymore (mutex_lock is sleeping). So restore the old
>> behavior.
>
> Yeah, preventing use in atomic context seems unnecessary, although any
> clients writing must now deal with serialisation themselves (as before,
> and as they should).
We could just remove the mutex for serdev_device_write and always make
the client responsible for serialization.
> Calling wait_for_completion in the non-blocking case was also needlessly
> inefficient.
It won't be called because count should be 0.
Rob
^ permalink raw reply
* Re: [PATCH] tty: serdev: fix serdev_device_write return value
From: Rob Herring @ 2017-05-02 12:30 UTC (permalink / raw)
To: Johan Hovold
Cc: Greg Kroah-Hartman, linux-serial@vger.kernel.org,
linux-kernel@vger.kernel.org, Andrey Smirnov
In-Reply-To: <20170502092527.GC2973@localhost>
On Tue, May 2, 2017 at 4:25 AM, Johan Hovold <johan@kernel.org> wrote:
> On Mon, May 01, 2017 at 07:17:14PM -0500, Rob Herring wrote:
>> Commit 6fe729c4bdae ("serdev: Add serdev_device_write subroutine")
>> provides a compatibility wrapper for the existing
>> serdev_device_write_buf, but it fails to return the number of bytes
>> written causing users to timeout.
>
> So this would also be fixed for serdev_device_write_buf() by Stefan
> Wahren's patch restoring that function implementation, but returning the
> amount written is perhaps desirable also for blocking writes for
> consistency reasons.
Yes, I saw it after I wrote this. We should apply both IMO.
>> Fixes: 6fe729c4bdae ("serdev: Add serdev_device_write subroutine")
>> Cc: Andrey Smirnov <andrew.smirnov@gmail.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> ---
>> drivers/tty/serdev/core.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
>> index 433de5ea9b02..ccfe56355c4f 100644
>> --- a/drivers/tty/serdev/core.c
>> +++ b/drivers/tty/serdev/core.c
>> @@ -127,7 +127,7 @@ int serdev_device_write(struct serdev_device *serdev,
>> unsigned long timeout)
>> {
>> struct serdev_controller *ctrl = serdev->ctrl;
>> - int ret;
>> + int ret, wr_cnt = 0;
>>
>> if (!ctrl || !ctrl->ops->write_buf ||
>> (timeout && !serdev->ops->write_wakeup))
>> @@ -143,12 +143,13 @@ int serdev_device_write(struct serdev_device *serdev,
>>
>> buf += ret;
>> count -= ret;
>> + wr_cnt += ret;
>>
>> } while (count &&
>> (timeout = wait_for_completion_timeout(&serdev->write_comp,
>> timeout)));
>>
>> mutex_unlock(&serdev->write_lock);
>> - return ret < 0 ? ret : (count ? -ETIMEDOUT : 0);
>> + return ret < 0 ? ret : (count ? -ETIMEDOUT : wr_cnt);
>
> That's some nasty use of the ternary operator. Ditching it completely
> would be more readable.
>
> if (ret < 0)
> return ret;
>
> if (count)
> return -ETIMEDOUT;
>
> return wr_count;
>
> and here wr_count is the value of count passed to the function (and
> could just be stored on entry instead).
Okay.
I'll wait for Greg to apply Stefan's patch and respin on top of it.
Rob
^ permalink raw reply
* Re: [PATCH] tty: serdev: fix serdev_device_write return value
From: Johan Hovold @ 2017-05-02 9:25 UTC (permalink / raw)
To: Rob Herring
Cc: Greg Kroah-Hartman, linux-serial, linux-kernel, Andrey Smirnov
In-Reply-To: <20170502001714.11576-1-robh@kernel.org>
On Mon, May 01, 2017 at 07:17:14PM -0500, Rob Herring wrote:
> Commit 6fe729c4bdae ("serdev: Add serdev_device_write subroutine")
> provides a compatibility wrapper for the existing
> serdev_device_write_buf, but it fails to return the number of bytes
> written causing users to timeout.
So this would also be fixed for serdev_device_write_buf() by Stefan
Wahren's patch restoring that function implementation, but returning the
amount written is perhaps desirable also for blocking writes for
consistency reasons.
> Fixes: 6fe729c4bdae ("serdev: Add serdev_device_write subroutine")
> Cc: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> drivers/tty/serdev/core.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index 433de5ea9b02..ccfe56355c4f 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -127,7 +127,7 @@ int serdev_device_write(struct serdev_device *serdev,
> unsigned long timeout)
> {
> struct serdev_controller *ctrl = serdev->ctrl;
> - int ret;
> + int ret, wr_cnt = 0;
>
> if (!ctrl || !ctrl->ops->write_buf ||
> (timeout && !serdev->ops->write_wakeup))
> @@ -143,12 +143,13 @@ int serdev_device_write(struct serdev_device *serdev,
>
> buf += ret;
> count -= ret;
> + wr_cnt += ret;
>
> } while (count &&
> (timeout = wait_for_completion_timeout(&serdev->write_comp,
> timeout)));
>
> mutex_unlock(&serdev->write_lock);
> - return ret < 0 ? ret : (count ? -ETIMEDOUT : 0);
> + return ret < 0 ? ret : (count ? -ETIMEDOUT : wr_cnt);
That's some nasty use of the ternary operator. Ditching it completely
would be more readable.
if (ret < 0)
return ret;
if (count)
return -ETIMEDOUT;
return wr_count;
and here wr_count is the value of count passed to the function (and
could just be stored on entry instead).
> }
> EXPORT_SYMBOL_GPL(serdev_device_write);
Thanks,
Johan
^ permalink raw reply
* Re: [PATCH] serdev: Restore serdev_device_write_buf for atomic context
From: Johan Hovold @ 2017-05-02 9:06 UTC (permalink / raw)
To: Stefan Wahren
Cc: Rob Herring, Greg Kroah-Hartman, Jiri Slaby, Sebastian Reichel,
Guenter Roeck, Andy Shevchenko, Andrey Smirnov, linux-serial,
linux-kernel
In-Reply-To: <1493380041-14710-1-git-send-email-stefan.wahren@i2se.com>
On Fri, Apr 28, 2017 at 01:47:21PM +0200, Stefan Wahren wrote:
> Starting with commit 6fe729c4bdae ("serdev: Add serdev_device_write
> subroutine") the function serdev_device_write_buf cannot be used in
> atomic context anymore (mutex_lock is sleeping). So restore the old
> behavior.
Yeah, preventing use in atomic context seems unnecessary, although any
clients writing must now deal with serialisation themselves (as before,
and as they should).
Calling wait_for_completion in the non-blocking case was also needlessly
inefficient.
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> Fixes: 6fe729c4bdae ("serdev: Add serdev_device_write subroutine")
Reviewed-by: Johan Hovold <johan@kernel.org>
Thanks,
Johan
^ permalink raw reply
* Re: [PATCH v2] tty/serial: atmel: use offset_in_page() macro
From: Richard Genoud @ 2017-05-02 8:47 UTC (permalink / raw)
To: Geliang Tang, Greg Kroah-Hartman, Jiri Slaby; +Cc: linux-serial, linux-kernel
In-Reply-To: <4467961523eb25226f3dc7b570c98208f31a9750.1493187215.git.geliangtang@gmail.com>
On 29/04/2017 03:39, Geliang Tang wrote:
> Use offset_in_page() macro instead of open-coding.
>
> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
Acked-by: Richard Genoud <richard.genoud@gmail.com>
> ---
> Changes in v2:
> - include mm.h
> ---
> drivers/tty/serial/atmel_serial.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index c355ac9..d25f044 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -46,6 +46,7 @@
> #include <linux/err.h>
> #include <linux/irq.h>
> #include <linux/suspend.h>
> +#include <linux/mm.h>
>
> #include <asm/io.h>
> #include <asm/ioctls.h>
> @@ -959,7 +960,7 @@ static int atmel_prepare_tx_dma(struct uart_port *port)
> sg_set_page(&atmel_port->sg_tx,
> virt_to_page(port->state->xmit.buf),
> UART_XMIT_SIZE,
> - (unsigned long)port->state->xmit.buf & ~PAGE_MASK);
> + offset_in_page(port->state->xmit.buf));
> nent = dma_map_sg(port->dev,
> &atmel_port->sg_tx,
> 1,
> @@ -1141,7 +1142,7 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
> sg_set_page(&atmel_port->sg_rx,
> virt_to_page(ring->buf),
> sizeof(struct atmel_uart_char) * ATMEL_SERIAL_RINGSIZE,
> - (unsigned long)ring->buf & ~PAGE_MASK);
> + offset_in_page(ring->buf));
> nent = dma_map_sg(port->dev,
> &atmel_port->sg_rx,
> 1,
>
Thanks !
^ permalink raw reply
* Re: [PATCH v4 2/2] drivers/serial: Add driver for Aspeed virtual UART
From: Benjamin Herrenschmidt @ 2017-05-02 7:59 UTC (permalink / raw)
To: Joel Stanley, Greg Kroah-Hartman, Jiri Slaby
Cc: Jeremy Kerr, linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Andy Shevchenko, Mark Rutland,
Rob Herring, openbmc-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <20170502074543.1380-3-joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
On Tue, 2017-05-02 at 17:15 +0930, Joel Stanley wrote:
> From: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
>
> This change adds a driver for the 16550-based Aspeed virtual UART
> device. We use a similar process to the of_serial driver for device
> probe, but expose some VUART-specific functions through sysfs too.
>
> The VUART is two UART 'front ends' connected by their FIFO (no actual
> serial line in between). One is on the BMC side (management controller)
> and one is on the host CPU side.
>
> This driver is for the BMC side. The sysfs files allow the BMC
> userspace, which owns the system configuration policy, to specify at
> what IO port and interrupt number the host side will appear to the host
> on the Host <-> BMC LPC bus. It could be different on a different system
> (though most of them use 3f8/4).
>
> OpenPOWER host firmware doesn't like it when the host-side of the
> VUART's FIFO is not drained. This driver only disables host TX discard
> mode when the port is in use. We set the VUART enabled bit when we bind
> to the device, and clear it on unbind.
>
> We don't want to do this on open/release, as the host may be using this
> bit to configure serial output modes, which is independent of whether
> the devices has been opened by BMC userspace.
>
> Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Reviewed-by: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
> ---
> v4:
> - Reorder if statements
> - Remove uncessary comment
>
> v3:
> - remove whitespace in header
> - reformat comment
> - don't check for reg-io-width property; we don't need any special
> accessors for reading/writing bytes
> - move file to 8250_aspeed_vuart.c
>
> v2:
> - Use attribute groups and DEVICE_ATTR_RW
> - Use platform_get_resource/devm_ioremap_resource
> - of_find_property -> of_property_read_bool
> - Drop unncessary 0xff mask
> - Fix comment style
> - Use BIT and GENMASK where pssible
> - Move to 8250 directory
> - Rename ast -> aspeed to match other Aspeed drivers
> - Add documentation of sysfs file
> - Add detail to the commit message
>
> Documentation/ABI/stable/sysfs-driver-aspeed-vuart | 15 +
> Documentation/devicetree/bindings/serial/8250.txt | 2 +
> drivers/tty/serial/8250/8250_aspeed_vuart.c | 323 +++++++++++++++++++++
> drivers/tty/serial/8250/Kconfig | 10 +
> drivers/tty/serial/8250/Makefile | 1 +
> 5 files changed, 351 insertions(+)
> create mode 100644 Documentation/ABI/stable/sysfs-driver-aspeed-vuart
> create mode 100644 drivers/tty/serial/8250/8250_aspeed_vuart.c
>
> diff --git a/Documentation/ABI/stable/sysfs-driver-aspeed-vuart b/Documentation/ABI/stable/sysfs-driver-aspeed-vuart
> new file mode 100644
> index 000000000000..8062953ce77b
> --- /dev/null
> +++ b/Documentation/ABI/stable/sysfs-driver-aspeed-vuart
> @@ -0,0 +1,15 @@
> +What: /sys/bus/platform/drivers/aspeed-vuart/*/lpc_address
> +Date: April 2017
> +Contact: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> +Description: Configures which IO port the host side of the UART
> + will appear on the host <-> BMC LPC bus.
> +Users: OpenBMC. Proposed changes should be mailed to
> + openbmc-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> +
> +What: /sys/bus/platform/drivers/aspeed-vuart*/sirq
> +Date: April 2017
> +Contact: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> +Description: Configures which interrupt number the host side of
> + the UART will appear on the host <-> BMC LPC bus.
> +Users: OpenBMC. Proposed changes should be mailed to
> + openbmc-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
> index 10276a46ecef..656733949309 100644
> --- a/Documentation/devicetree/bindings/serial/8250.txt
> +++ b/Documentation/devicetree/bindings/serial/8250.txt
> @@ -20,6 +20,8 @@ Required properties:
> - "fsl,16550-FIFO64"
> - "fsl,ns16550"
> - "ti,da830-uart"
> + - "aspeed,ast2400-vuart"
> + - "aspeed,ast2500-vuart"
> - "serial" if the port type is unknown.
> - reg : offset and length of the register set for the device.
> - interrupts : should contain uart interrupt.
> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> new file mode 100644
> index 000000000000..822be4906763
> --- /dev/null
> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> @@ -0,0 +1,323 @@
> +/*
> + * Serial Port driver for Aspeed VUART device
> + *
> + * Copyright (C) 2016 Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>, IBM Corp.
> + * Copyright (C) 2006 Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>, IBM Corp.
> + *
> + * 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.
> + */
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/clk.h>
> +
> +#include "8250.h"
> +
> +#define ASPEED_VUART_GCRA 0x20
> +#define ASPEED_VUART_GCRA_VUART_EN BIT(0)
> +#define ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD BIT(5)
> +#define ASPEED_VUART_GCRB 0x24
> +#define ASPEED_VUART_GCRB_HOST_SIRQ_MASK GENMASK(7, 4)
> +#define ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT 4
> +#define ASPEED_VUART_ADDRL 0x28
> +#define ASPEED_VUART_ADDRH 0x2c
> +
> +struct aspeed_vuart {
> + struct device *dev;
> + void __iomem *regs;
> + struct clk *clk;
> + int line;
> +};
> +
> +/*
> + * The VUART is basically two UART 'front ends' connected by their FIFO
> + * (no actual serial line in between). One is on the BMC side (management
> + * controller) and one is on the host CPU side.
> + *
> + * It allows the BMC to provide to the host a "UART" that pipes into
> + * the BMC itself and can then be turned by the BMC into a network console
> + * of some sort for example.
> + *
> + * This driver is for the BMC side. The sysfs files allow the BMC
> + * userspace which owns the system configuration policy, to specify
> + * at what IO port and interrupt number the host side will appear
> + * to the host on the Host <-> BMC LPC bus. It could be different on a
> + * different system (though most of them use 3f8/4).
> + */
> +
> +static ssize_t lpc_address_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> + u16 addr;
> +
> + addr = (readb(vuart->regs + ASPEED_VUART_ADDRH) << 8) |
> + (readb(vuart->regs + ASPEED_VUART_ADDRL));
> +
> + return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);
> +}
> +
> +static ssize_t lpc_address_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> + unsigned long val;
> + int err;
> +
> + err = kstrtoul(buf, 0, &val);
> + if (err)
> + return err;
> +
> + writeb(val >> 8, vuart->regs + ASPEED_VUART_ADDRH);
> + writeb(val >> 0, vuart->regs + ASPEED_VUART_ADDRL);
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR_RW(lpc_address);
> +
> +static ssize_t sirq_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> + u8 reg;
> +
> + reg = readb(vuart->regs + ASPEED_VUART_GCRB);
> + reg &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> + reg >>= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
> +
> + return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg);
> +}
> +
> +static ssize_t sirq_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> + unsigned long val;
> + int err;
> + u8 reg;
> +
> + err = kstrtoul(buf, 0, &val);
> + if (err)
> + return err;
> +
> + val <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
> + val &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> +
> + reg = readb(vuart->regs + ASPEED_VUART_GCRB);
> + reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> + reg |= val;
> + writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR_RW(sirq);
> +
> +static struct attribute *aspeed_vuart_attrs[] = {
> + &dev_attr_sirq.attr,
> + &dev_attr_lpc_address.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group aspeed_vuart_attr_group = {
> + .attrs = aspeed_vuart_attrs,
> +};
> +
> +static void aspeed_vuart_set_enabled(struct aspeed_vuart *vuart, bool enabled)
> +{
> + u8 reg = readb(vuart->regs + ASPEED_VUART_GCRA);
> +
> + if (enabled)
> + reg |= ASPEED_VUART_GCRA_VUART_EN;
> + else
> + reg &= ~ASPEED_VUART_GCRA_VUART_EN;
> +
> + writeb(reg, vuart->regs + ASPEED_VUART_GCRA);
> +}
> +
> +static void aspeed_vuart_set_host_tx_discard(struct aspeed_vuart *vuart,
> + bool discard)
> +{
> + u8 reg;
> +
> + reg = readb(vuart->regs + ASPEED_VUART_GCRA);
> +
> + /* If the DISABLE_HOST_TX_DISCARD bit is set, discard is disabled */
> + if (!discard)
> + reg |= ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD;
> + else
> + reg &= ~ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD;
> +
> + writeb(reg, vuart->regs + ASPEED_VUART_GCRA);
> +}
> +
> +static int aspeed_vuart_startup(struct uart_port *uart_port)
> +{
> + struct uart_8250_port *uart_8250_port = up_to_u8250p(uart_port);
> + struct aspeed_vuart *vuart = uart_8250_port->port.private_data;
> + int rc;
> +
> + rc = serial8250_do_startup(uart_port);
> + if (rc)
> + return rc;
> +
> + aspeed_vuart_set_host_tx_discard(vuart, false);
> +
> + return 0;
> +}
> +
> +static void aspeed_vuart_shutdown(struct uart_port *uart_port)
> +{
> + struct uart_8250_port *uart_8250_port = up_to_u8250p(uart_port);
> + struct aspeed_vuart *vuart = uart_8250_port->port.private_data;
> +
> + aspeed_vuart_set_host_tx_discard(vuart, true);
> +
> + serial8250_do_shutdown(uart_port);
> +}
> +
> +static int aspeed_vuart_probe(struct platform_device *pdev)
> +{
> + struct uart_8250_port port;
> + struct aspeed_vuart *vuart;
> + struct device_node *np;
> + struct resource *res;
> + u32 clk, prop;
> + int rc;
> +
> + np = pdev->dev.of_node;
> +
> + vuart = devm_kzalloc(&pdev->dev, sizeof(*vuart), GFP_KERNEL);
> + if (!vuart)
> + return -ENOMEM;
> +
> + vuart->dev = &pdev->dev;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + vuart->regs = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(vuart->regs))
> + return PTR_ERR(vuart->regs);
> +
> + memset(&port, 0, sizeof(port));
> + port.port.private_data = vuart;
> + port.port.membase = vuart->regs;
> + port.port.mapbase = res->start;
> + port.port.mapsize = resource_size(res);
> + port.port.startup = aspeed_vuart_startup;
> + port.port.shutdown = aspeed_vuart_shutdown;
> + port.port.dev = &pdev->dev;
> +
> + rc = sysfs_create_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
> + if (rc < 0)
> + return rc;
> +
> + if (of_property_read_u32(np, "clock-frequency", &clk)) {
> + vuart->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(vuart->clk)) {
> + dev_warn(&pdev->dev,
> + "clk or clock-frequency not defined\n");
> + return PTR_ERR(vuart->clk);
> + }
> +
> + rc = clk_prepare_enable(vuart->clk);
> + if (rc < 0)
> + return rc;
> +
> + clk = clk_get_rate(vuart->clk);
> + }
> +
> + /* If current-speed was set, then try not to change it. */
> + if (of_property_read_u32(np, "current-speed", &prop) == 0)
> + port.port.custom_divisor = clk / (16 * prop);
> +
> + /* Check for shifted address mapping */
> + if (of_property_read_u32(np, "reg-offset", &prop) == 0)
> + port.port.mapbase += prop;
> +
> + /* Check for registers offset within the devices address range */
> + if (of_property_read_u32(np, "reg-shift", &prop) == 0)
> + port.port.regshift = prop;
> +
> + /* Check for fifo size */
> + if (of_property_read_u32(np, "fifo-size", &prop) == 0)
> + port.port.fifosize = prop;
> +
> + /* Check for a fixed line number */
> + rc = of_alias_get_id(np, "serial");
> + if (rc >= 0)
> + port.port.line = rc;
> +
> + port.port.irq = irq_of_parse_and_map(np, 0);
> + port.port.irqflags = IRQF_SHARED;
> + port.port.iotype = UPIO_MEM;
> + port.port.type = PORT_16550A;
> + port.port.uartclk = clk;
> + port.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF
> + | UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_NO_THRE_TEST;
> +
> + if (of_property_read_bool(np, "no-loopback-test"))
> + port.port.flags |= UPF_SKIP_TEST;
> +
> + if (port.port.fifosize)
> + port.capabilities = UART_CAP_FIFO;
> +
> + if (of_property_read_bool(np, "auto-flow-control"))
> + port.capabilities |= UART_CAP_AFE;
> +
> + rc = serial8250_register_8250_port(&port);
> + if (rc < 0)
> + goto err_clk_disable;
> +
> + vuart->line = rc;
> +
> + aspeed_vuart_set_enabled(vuart, true);
> + aspeed_vuart_set_host_tx_discard(vuart, true);
> + platform_set_drvdata(pdev, vuart);
> +
> + return 0;
> +
> +err_clk_disable:
> + clk_disable_unprepare(vuart->clk);
> + irq_dispose_mapping(port.port.irq);
> + return rc;
> +}
> +
> +static int aspeed_vuart_remove(struct platform_device *pdev)
> +{
> + struct aspeed_vuart *vuart = platform_get_drvdata(pdev);
> +
> + aspeed_vuart_set_enabled(vuart, false);
> + serial8250_unregister_port(vuart->line);
> + sysfs_remove_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
> + clk_disable_unprepare(vuart->clk);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id aspeed_vuart_table[] = {
> + { .compatible = "aspeed,ast2400-vuart" },
> + { .compatible = "aspeed,ast2500-vuart" },
> + { },
> +};
> +
> +static struct platform_driver aspeed_vuart_driver = {
> + .driver = {
> + .name = "aspeed-vuart",
> + .of_match_table = aspeed_vuart_table,
> + },
> + .probe = aspeed_vuart_probe,
> + .remove = aspeed_vuart_remove,
> +};
> +
> +module_platform_driver(aspeed_vuart_driver);
> +
> +MODULE_AUTHOR("Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Driver for Aspeed VUART device");
> diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> index 0e3f529d50e9..a1161ec0256f 100644
> --- a/drivers/tty/serial/8250/Kconfig
> +++ b/drivers/tty/serial/8250/Kconfig
> @@ -224,6 +224,16 @@ config SERIAL_8250_ACCENT
> To compile this driver as a module, choose M here: the module
> will be called 8250_accent.
>
> +config SERIAL_8250_ASPEED_VUART
> + tristate "Aspeed Virtual UART"
> + depends on SERIAL_8250
> + depends on OF
> + help
> + If you want to use the virtual UART (VUART) device on Aspeed
> + BMC platforms, enable this option. This enables the 16550A-
> + compatible device on the local LPC bus, giving a UART device
> + with no physical RS232 connections.
> +
> config SERIAL_8250_BOCA
> tristate "Support Boca cards"
> depends on SERIAL_8250 != n && ISA && SERIAL_8250_MANY_PORTS
> diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
> index 2f30f9ecdb1b..a44a99a3e623 100644
> --- a/drivers/tty/serial/8250/Makefile
> +++ b/drivers/tty/serial/8250/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_SERIAL_8250_EXAR) += 8250_exar.o
> obj-$(CONFIG_SERIAL_8250_HP300) += 8250_hp300.o
> obj-$(CONFIG_SERIAL_8250_CS) += serial_cs.o
> obj-$(CONFIG_SERIAL_8250_ACORN) += 8250_acorn.o
> +obj-$(CONFIG_SERIAL_8250_ASPEED_VUART) += 8250_aspeed_vuart.o
> obj-$(CONFIG_SERIAL_8250_BCM2835AUX) += 8250_bcm2835aux.o
> obj-$(CONFIG_SERIAL_8250_CONSOLE) += 8250_early.o
> obj-$(CONFIG_SERIAL_8250_FOURPORT) += 8250_fourport.o
--
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
* Re: [PATCH v4 2/2] drivers/serial: Add driver for Aspeed virtual UART
From: Andy Shevchenko @ 2017-05-02 7:51 UTC (permalink / raw)
To: Joel Stanley
Cc: Greg Kroah-Hartman, Jiri Slaby, Jeremy Kerr,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree,
Benjamin Herrenschmidt, Mark Rutland, Rob Herring,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <20170502074543.1380-3-joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
On Tue, May 2, 2017 at 10:45 AM, Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org> wrote:
> From: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
>
> This change adds a driver for the 16550-based Aspeed virtual UART
> device. We use a similar process to the of_serial driver for device
> probe, but expose some VUART-specific functions through sysfs too.
>
> The VUART is two UART 'front ends' connected by their FIFO (no actual
> serial line in between). One is on the BMC side (management controller)
> and one is on the host CPU side.
>
> This driver is for the BMC side. The sysfs files allow the BMC
> userspace, which owns the system configuration policy, to specify at
> what IO port and interrupt number the host side will appear to the host
> on the Host <-> BMC LPC bus. It could be different on a different system
> (though most of them use 3f8/4).
>
> OpenPOWER host firmware doesn't like it when the host-side of the
> VUART's FIFO is not drained. This driver only disables host TX discard
> mode when the port is in use. We set the VUART enabled bit when we bind
> to the device, and clear it on unbind.
>
> We don't want to do this on open/release, as the host may be using this
> bit to configure serial output modes, which is independent of whether
> the devices has been opened by BMC userspace.
>
> Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>
I have reviewed the driver and found it's quality good, though one
minor about IRQ getting is left untouched. Let's Greg or others to
decide, from my side FWIW:
Reviewed-by: Andy Shevchenko <andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> v4:
> - Reorder if statements
> - Remove uncessary comment
>
> v3:
> - remove whitespace in header
> - reformat comment
> - don't check for reg-io-width property; we don't need any special
> accessors for reading/writing bytes
> - move file to 8250_aspeed_vuart.c
>
> v2:
> - Use attribute groups and DEVICE_ATTR_RW
> - Use platform_get_resource/devm_ioremap_resource
> - of_find_property -> of_property_read_bool
> - Drop unncessary 0xff mask
> - Fix comment style
> - Use BIT and GENMASK where pssible
> - Move to 8250 directory
> - Rename ast -> aspeed to match other Aspeed drivers
> - Add documentation of sysfs file
> - Add detail to the commit message
>
> Documentation/ABI/stable/sysfs-driver-aspeed-vuart | 15 +
> Documentation/devicetree/bindings/serial/8250.txt | 2 +
> drivers/tty/serial/8250/8250_aspeed_vuart.c | 323 +++++++++++++++++++++
> drivers/tty/serial/8250/Kconfig | 10 +
> drivers/tty/serial/8250/Makefile | 1 +
> 5 files changed, 351 insertions(+)
> create mode 100644 Documentation/ABI/stable/sysfs-driver-aspeed-vuart
> create mode 100644 drivers/tty/serial/8250/8250_aspeed_vuart.c
>
> diff --git a/Documentation/ABI/stable/sysfs-driver-aspeed-vuart b/Documentation/ABI/stable/sysfs-driver-aspeed-vuart
> new file mode 100644
> index 000000000000..8062953ce77b
> --- /dev/null
> +++ b/Documentation/ABI/stable/sysfs-driver-aspeed-vuart
> @@ -0,0 +1,15 @@
> +What: /sys/bus/platform/drivers/aspeed-vuart/*/lpc_address
> +Date: April 2017
> +Contact: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> +Description: Configures which IO port the host side of the UART
> + will appear on the host <-> BMC LPC bus.
> +Users: OpenBMC. Proposed changes should be mailed to
> + openbmc-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> +
> +What: /sys/bus/platform/drivers/aspeed-vuart*/sirq
> +Date: April 2017
> +Contact: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> +Description: Configures which interrupt number the host side of
> + the UART will appear on the host <-> BMC LPC bus.
> +Users: OpenBMC. Proposed changes should be mailed to
> + openbmc-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
> index 10276a46ecef..656733949309 100644
> --- a/Documentation/devicetree/bindings/serial/8250.txt
> +++ b/Documentation/devicetree/bindings/serial/8250.txt
> @@ -20,6 +20,8 @@ Required properties:
> - "fsl,16550-FIFO64"
> - "fsl,ns16550"
> - "ti,da830-uart"
> + - "aspeed,ast2400-vuart"
> + - "aspeed,ast2500-vuart"
> - "serial" if the port type is unknown.
> - reg : offset and length of the register set for the device.
> - interrupts : should contain uart interrupt.
> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> new file mode 100644
> index 000000000000..822be4906763
> --- /dev/null
> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> @@ -0,0 +1,323 @@
> +/*
> + * Serial Port driver for Aspeed VUART device
> + *
> + * Copyright (C) 2016 Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>, IBM Corp.
> + * Copyright (C) 2006 Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>, IBM Corp.
> + *
> + * 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.
> + */
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/clk.h>
> +
> +#include "8250.h"
> +
> +#define ASPEED_VUART_GCRA 0x20
> +#define ASPEED_VUART_GCRA_VUART_EN BIT(0)
> +#define ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD BIT(5)
> +#define ASPEED_VUART_GCRB 0x24
> +#define ASPEED_VUART_GCRB_HOST_SIRQ_MASK GENMASK(7, 4)
> +#define ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT 4
> +#define ASPEED_VUART_ADDRL 0x28
> +#define ASPEED_VUART_ADDRH 0x2c
> +
> +struct aspeed_vuart {
> + struct device *dev;
> + void __iomem *regs;
> + struct clk *clk;
> + int line;
> +};
> +
> +/*
> + * The VUART is basically two UART 'front ends' connected by their FIFO
> + * (no actual serial line in between). One is on the BMC side (management
> + * controller) and one is on the host CPU side.
> + *
> + * It allows the BMC to provide to the host a "UART" that pipes into
> + * the BMC itself and can then be turned by the BMC into a network console
> + * of some sort for example.
> + *
> + * This driver is for the BMC side. The sysfs files allow the BMC
> + * userspace which owns the system configuration policy, to specify
> + * at what IO port and interrupt number the host side will appear
> + * to the host on the Host <-> BMC LPC bus. It could be different on a
> + * different system (though most of them use 3f8/4).
> + */
> +
> +static ssize_t lpc_address_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> + u16 addr;
> +
> + addr = (readb(vuart->regs + ASPEED_VUART_ADDRH) << 8) |
> + (readb(vuart->regs + ASPEED_VUART_ADDRL));
> +
> + return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);
> +}
> +
> +static ssize_t lpc_address_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> + unsigned long val;
> + int err;
> +
> + err = kstrtoul(buf, 0, &val);
> + if (err)
> + return err;
> +
> + writeb(val >> 8, vuart->regs + ASPEED_VUART_ADDRH);
> + writeb(val >> 0, vuart->regs + ASPEED_VUART_ADDRL);
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR_RW(lpc_address);
> +
> +static ssize_t sirq_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> + u8 reg;
> +
> + reg = readb(vuart->regs + ASPEED_VUART_GCRB);
> + reg &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> + reg >>= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
> +
> + return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg);
> +}
> +
> +static ssize_t sirq_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> + unsigned long val;
> + int err;
> + u8 reg;
> +
> + err = kstrtoul(buf, 0, &val);
> + if (err)
> + return err;
> +
> + val <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
> + val &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> +
> + reg = readb(vuart->regs + ASPEED_VUART_GCRB);
> + reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> + reg |= val;
> + writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR_RW(sirq);
> +
> +static struct attribute *aspeed_vuart_attrs[] = {
> + &dev_attr_sirq.attr,
> + &dev_attr_lpc_address.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group aspeed_vuart_attr_group = {
> + .attrs = aspeed_vuart_attrs,
> +};
> +
> +static void aspeed_vuart_set_enabled(struct aspeed_vuart *vuart, bool enabled)
> +{
> + u8 reg = readb(vuart->regs + ASPEED_VUART_GCRA);
> +
> + if (enabled)
> + reg |= ASPEED_VUART_GCRA_VUART_EN;
> + else
> + reg &= ~ASPEED_VUART_GCRA_VUART_EN;
> +
> + writeb(reg, vuart->regs + ASPEED_VUART_GCRA);
> +}
> +
> +static void aspeed_vuart_set_host_tx_discard(struct aspeed_vuart *vuart,
> + bool discard)
> +{
> + u8 reg;
> +
> + reg = readb(vuart->regs + ASPEED_VUART_GCRA);
> +
> + /* If the DISABLE_HOST_TX_DISCARD bit is set, discard is disabled */
> + if (!discard)
> + reg |= ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD;
> + else
> + reg &= ~ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD;
> +
> + writeb(reg, vuart->regs + ASPEED_VUART_GCRA);
> +}
> +
> +static int aspeed_vuart_startup(struct uart_port *uart_port)
> +{
> + struct uart_8250_port *uart_8250_port = up_to_u8250p(uart_port);
> + struct aspeed_vuart *vuart = uart_8250_port->port.private_data;
> + int rc;
> +
> + rc = serial8250_do_startup(uart_port);
> + if (rc)
> + return rc;
> +
> + aspeed_vuart_set_host_tx_discard(vuart, false);
> +
> + return 0;
> +}
> +
> +static void aspeed_vuart_shutdown(struct uart_port *uart_port)
> +{
> + struct uart_8250_port *uart_8250_port = up_to_u8250p(uart_port);
> + struct aspeed_vuart *vuart = uart_8250_port->port.private_data;
> +
> + aspeed_vuart_set_host_tx_discard(vuart, true);
> +
> + serial8250_do_shutdown(uart_port);
> +}
> +
> +static int aspeed_vuart_probe(struct platform_device *pdev)
> +{
> + struct uart_8250_port port;
> + struct aspeed_vuart *vuart;
> + struct device_node *np;
> + struct resource *res;
> + u32 clk, prop;
> + int rc;
> +
> + np = pdev->dev.of_node;
> +
> + vuart = devm_kzalloc(&pdev->dev, sizeof(*vuart), GFP_KERNEL);
> + if (!vuart)
> + return -ENOMEM;
> +
> + vuart->dev = &pdev->dev;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + vuart->regs = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(vuart->regs))
> + return PTR_ERR(vuart->regs);
> +
> + memset(&port, 0, sizeof(port));
> + port.port.private_data = vuart;
> + port.port.membase = vuart->regs;
> + port.port.mapbase = res->start;
> + port.port.mapsize = resource_size(res);
> + port.port.startup = aspeed_vuart_startup;
> + port.port.shutdown = aspeed_vuart_shutdown;
> + port.port.dev = &pdev->dev;
> +
> + rc = sysfs_create_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
> + if (rc < 0)
> + return rc;
> +
> + if (of_property_read_u32(np, "clock-frequency", &clk)) {
> + vuart->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(vuart->clk)) {
> + dev_warn(&pdev->dev,
> + "clk or clock-frequency not defined\n");
> + return PTR_ERR(vuart->clk);
> + }
> +
> + rc = clk_prepare_enable(vuart->clk);
> + if (rc < 0)
> + return rc;
> +
> + clk = clk_get_rate(vuart->clk);
> + }
> +
> + /* If current-speed was set, then try not to change it. */
> + if (of_property_read_u32(np, "current-speed", &prop) == 0)
> + port.port.custom_divisor = clk / (16 * prop);
> +
> + /* Check for shifted address mapping */
> + if (of_property_read_u32(np, "reg-offset", &prop) == 0)
> + port.port.mapbase += prop;
> +
> + /* Check for registers offset within the devices address range */
> + if (of_property_read_u32(np, "reg-shift", &prop) == 0)
> + port.port.regshift = prop;
> +
> + /* Check for fifo size */
> + if (of_property_read_u32(np, "fifo-size", &prop) == 0)
> + port.port.fifosize = prop;
> +
> + /* Check for a fixed line number */
> + rc = of_alias_get_id(np, "serial");
> + if (rc >= 0)
> + port.port.line = rc;
> +
> + port.port.irq = irq_of_parse_and_map(np, 0);
> + port.port.irqflags = IRQF_SHARED;
> + port.port.iotype = UPIO_MEM;
> + port.port.type = PORT_16550A;
> + port.port.uartclk = clk;
> + port.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF
> + | UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_NO_THRE_TEST;
> +
> + if (of_property_read_bool(np, "no-loopback-test"))
> + port.port.flags |= UPF_SKIP_TEST;
> +
> + if (port.port.fifosize)
> + port.capabilities = UART_CAP_FIFO;
> +
> + if (of_property_read_bool(np, "auto-flow-control"))
> + port.capabilities |= UART_CAP_AFE;
> +
> + rc = serial8250_register_8250_port(&port);
> + if (rc < 0)
> + goto err_clk_disable;
> +
> + vuart->line = rc;
> +
> + aspeed_vuart_set_enabled(vuart, true);
> + aspeed_vuart_set_host_tx_discard(vuart, true);
> + platform_set_drvdata(pdev, vuart);
> +
> + return 0;
> +
> +err_clk_disable:
> + clk_disable_unprepare(vuart->clk);
> + irq_dispose_mapping(port.port.irq);
> + return rc;
> +}
> +
> +static int aspeed_vuart_remove(struct platform_device *pdev)
> +{
> + struct aspeed_vuart *vuart = platform_get_drvdata(pdev);
> +
> + aspeed_vuart_set_enabled(vuart, false);
> + serial8250_unregister_port(vuart->line);
> + sysfs_remove_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
> + clk_disable_unprepare(vuart->clk);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id aspeed_vuart_table[] = {
> + { .compatible = "aspeed,ast2400-vuart" },
> + { .compatible = "aspeed,ast2500-vuart" },
> + { },
> +};
> +
> +static struct platform_driver aspeed_vuart_driver = {
> + .driver = {
> + .name = "aspeed-vuart",
> + .of_match_table = aspeed_vuart_table,
> + },
> + .probe = aspeed_vuart_probe,
> + .remove = aspeed_vuart_remove,
> +};
> +
> +module_platform_driver(aspeed_vuart_driver);
> +
> +MODULE_AUTHOR("Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Driver for Aspeed VUART device");
> diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> index 0e3f529d50e9..a1161ec0256f 100644
> --- a/drivers/tty/serial/8250/Kconfig
> +++ b/drivers/tty/serial/8250/Kconfig
> @@ -224,6 +224,16 @@ config SERIAL_8250_ACCENT
> To compile this driver as a module, choose M here: the module
> will be called 8250_accent.
>
> +config SERIAL_8250_ASPEED_VUART
> + tristate "Aspeed Virtual UART"
> + depends on SERIAL_8250
> + depends on OF
> + help
> + If you want to use the virtual UART (VUART) device on Aspeed
> + BMC platforms, enable this option. This enables the 16550A-
> + compatible device on the local LPC bus, giving a UART device
> + with no physical RS232 connections.
> +
> config SERIAL_8250_BOCA
> tristate "Support Boca cards"
> depends on SERIAL_8250 != n && ISA && SERIAL_8250_MANY_PORTS
> diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
> index 2f30f9ecdb1b..a44a99a3e623 100644
> --- a/drivers/tty/serial/8250/Makefile
> +++ b/drivers/tty/serial/8250/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_SERIAL_8250_EXAR) += 8250_exar.o
> obj-$(CONFIG_SERIAL_8250_HP300) += 8250_hp300.o
> obj-$(CONFIG_SERIAL_8250_CS) += serial_cs.o
> obj-$(CONFIG_SERIAL_8250_ACORN) += 8250_acorn.o
> +obj-$(CONFIG_SERIAL_8250_ASPEED_VUART) += 8250_aspeed_vuart.o
> obj-$(CONFIG_SERIAL_8250_BCM2835AUX) += 8250_bcm2835aux.o
> obj-$(CONFIG_SERIAL_8250_CONSOLE) += 8250_early.o
> obj-$(CONFIG_SERIAL_8250_FOURPORT) += 8250_fourport.o
> --
> 2.11.0
>
--
With Best Regards,
Andy Shevchenko
--
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 v4 2/2] drivers/serial: Add driver for Aspeed virtual UART
From: Joel Stanley @ 2017-05-02 7:45 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Jeremy Kerr, linux-serial, linux-kernel, devicetree,
Andy Shevchenko, Benjamin Herrenschmidt, Mark Rutland,
Rob Herring, openbmc
In-Reply-To: <20170502074543.1380-1-joel@jms.id.au>
From: Jeremy Kerr <jk@ozlabs.org>
This change adds a driver for the 16550-based Aspeed virtual UART
device. We use a similar process to the of_serial driver for device
probe, but expose some VUART-specific functions through sysfs too.
The VUART is two UART 'front ends' connected by their FIFO (no actual
serial line in between). One is on the BMC side (management controller)
and one is on the host CPU side.
This driver is for the BMC side. The sysfs files allow the BMC
userspace, which owns the system configuration policy, to specify at
what IO port and interrupt number the host side will appear to the host
on the Host <-> BMC LPC bus. It could be different on a different system
(though most of them use 3f8/4).
OpenPOWER host firmware doesn't like it when the host-side of the
VUART's FIFO is not drained. This driver only disables host TX discard
mode when the port is in use. We set the VUART enabled bit when we bind
to the device, and clear it on unbind.
We don't want to do this on open/release, as the host may be using this
bit to configure serial output modes, which is independent of whether
the devices has been opened by BMC userspace.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Joel Stanley <joel@jms.id.au>
Acked-by: Rob Herring <robh@kernel.org>
---
v4:
- Reorder if statements
- Remove uncessary comment
v3:
- remove whitespace in header
- reformat comment
- don't check for reg-io-width property; we don't need any special
accessors for reading/writing bytes
- move file to 8250_aspeed_vuart.c
v2:
- Use attribute groups and DEVICE_ATTR_RW
- Use platform_get_resource/devm_ioremap_resource
- of_find_property -> of_property_read_bool
- Drop unncessary 0xff mask
- Fix comment style
- Use BIT and GENMASK where pssible
- Move to 8250 directory
- Rename ast -> aspeed to match other Aspeed drivers
- Add documentation of sysfs file
- Add detail to the commit message
Documentation/ABI/stable/sysfs-driver-aspeed-vuart | 15 +
Documentation/devicetree/bindings/serial/8250.txt | 2 +
drivers/tty/serial/8250/8250_aspeed_vuart.c | 323 +++++++++++++++++++++
drivers/tty/serial/8250/Kconfig | 10 +
drivers/tty/serial/8250/Makefile | 1 +
5 files changed, 351 insertions(+)
create mode 100644 Documentation/ABI/stable/sysfs-driver-aspeed-vuart
create mode 100644 drivers/tty/serial/8250/8250_aspeed_vuart.c
diff --git a/Documentation/ABI/stable/sysfs-driver-aspeed-vuart b/Documentation/ABI/stable/sysfs-driver-aspeed-vuart
new file mode 100644
index 000000000000..8062953ce77b
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-driver-aspeed-vuart
@@ -0,0 +1,15 @@
+What: /sys/bus/platform/drivers/aspeed-vuart/*/lpc_address
+Date: April 2017
+Contact: Jeremy Kerr <jk@ozlabs.org>
+Description: Configures which IO port the host side of the UART
+ will appear on the host <-> BMC LPC bus.
+Users: OpenBMC. Proposed changes should be mailed to
+ openbmc@lists.ozlabs.org
+
+What: /sys/bus/platform/drivers/aspeed-vuart*/sirq
+Date: April 2017
+Contact: Jeremy Kerr <jk@ozlabs.org>
+Description: Configures which interrupt number the host side of
+ the UART will appear on the host <-> BMC LPC bus.
+Users: OpenBMC. Proposed changes should be mailed to
+ openbmc@lists.ozlabs.org
diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
index 10276a46ecef..656733949309 100644
--- a/Documentation/devicetree/bindings/serial/8250.txt
+++ b/Documentation/devicetree/bindings/serial/8250.txt
@@ -20,6 +20,8 @@ Required properties:
- "fsl,16550-FIFO64"
- "fsl,ns16550"
- "ti,da830-uart"
+ - "aspeed,ast2400-vuart"
+ - "aspeed,ast2500-vuart"
- "serial" if the port type is unknown.
- reg : offset and length of the register set for the device.
- interrupts : should contain uart interrupt.
diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
new file mode 100644
index 000000000000..822be4906763
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
@@ -0,0 +1,323 @@
+/*
+ * Serial Port driver for Aspeed VUART device
+ *
+ * Copyright (C) 2016 Jeremy Kerr <jk@ozlabs.org>, IBM Corp.
+ * Copyright (C) 2006 Arnd Bergmann <arnd@arndb.de>, IBM Corp.
+ *
+ * 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.
+ */
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/clk.h>
+
+#include "8250.h"
+
+#define ASPEED_VUART_GCRA 0x20
+#define ASPEED_VUART_GCRA_VUART_EN BIT(0)
+#define ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD BIT(5)
+#define ASPEED_VUART_GCRB 0x24
+#define ASPEED_VUART_GCRB_HOST_SIRQ_MASK GENMASK(7, 4)
+#define ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT 4
+#define ASPEED_VUART_ADDRL 0x28
+#define ASPEED_VUART_ADDRH 0x2c
+
+struct aspeed_vuart {
+ struct device *dev;
+ void __iomem *regs;
+ struct clk *clk;
+ int line;
+};
+
+/*
+ * The VUART is basically two UART 'front ends' connected by their FIFO
+ * (no actual serial line in between). One is on the BMC side (management
+ * controller) and one is on the host CPU side.
+ *
+ * It allows the BMC to provide to the host a "UART" that pipes into
+ * the BMC itself and can then be turned by the BMC into a network console
+ * of some sort for example.
+ *
+ * This driver is for the BMC side. The sysfs files allow the BMC
+ * userspace which owns the system configuration policy, to specify
+ * at what IO port and interrupt number the host side will appear
+ * to the host on the Host <-> BMC LPC bus. It could be different on a
+ * different system (though most of them use 3f8/4).
+ */
+
+static ssize_t lpc_address_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct aspeed_vuart *vuart = dev_get_drvdata(dev);
+ u16 addr;
+
+ addr = (readb(vuart->regs + ASPEED_VUART_ADDRH) << 8) |
+ (readb(vuart->regs + ASPEED_VUART_ADDRL));
+
+ return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);
+}
+
+static ssize_t lpc_address_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct aspeed_vuart *vuart = dev_get_drvdata(dev);
+ unsigned long val;
+ int err;
+
+ err = kstrtoul(buf, 0, &val);
+ if (err)
+ return err;
+
+ writeb(val >> 8, vuart->regs + ASPEED_VUART_ADDRH);
+ writeb(val >> 0, vuart->regs + ASPEED_VUART_ADDRL);
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(lpc_address);
+
+static ssize_t sirq_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct aspeed_vuart *vuart = dev_get_drvdata(dev);
+ u8 reg;
+
+ reg = readb(vuart->regs + ASPEED_VUART_GCRB);
+ reg &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
+ reg >>= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
+
+ return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg);
+}
+
+static ssize_t sirq_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct aspeed_vuart *vuart = dev_get_drvdata(dev);
+ unsigned long val;
+ int err;
+ u8 reg;
+
+ err = kstrtoul(buf, 0, &val);
+ if (err)
+ return err;
+
+ val <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
+ val &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
+
+ reg = readb(vuart->regs + ASPEED_VUART_GCRB);
+ reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
+ reg |= val;
+ writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(sirq);
+
+static struct attribute *aspeed_vuart_attrs[] = {
+ &dev_attr_sirq.attr,
+ &dev_attr_lpc_address.attr,
+ NULL,
+};
+
+static const struct attribute_group aspeed_vuart_attr_group = {
+ .attrs = aspeed_vuart_attrs,
+};
+
+static void aspeed_vuart_set_enabled(struct aspeed_vuart *vuart, bool enabled)
+{
+ u8 reg = readb(vuart->regs + ASPEED_VUART_GCRA);
+
+ if (enabled)
+ reg |= ASPEED_VUART_GCRA_VUART_EN;
+ else
+ reg &= ~ASPEED_VUART_GCRA_VUART_EN;
+
+ writeb(reg, vuart->regs + ASPEED_VUART_GCRA);
+}
+
+static void aspeed_vuart_set_host_tx_discard(struct aspeed_vuart *vuart,
+ bool discard)
+{
+ u8 reg;
+
+ reg = readb(vuart->regs + ASPEED_VUART_GCRA);
+
+ /* If the DISABLE_HOST_TX_DISCARD bit is set, discard is disabled */
+ if (!discard)
+ reg |= ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD;
+ else
+ reg &= ~ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD;
+
+ writeb(reg, vuart->regs + ASPEED_VUART_GCRA);
+}
+
+static int aspeed_vuart_startup(struct uart_port *uart_port)
+{
+ struct uart_8250_port *uart_8250_port = up_to_u8250p(uart_port);
+ struct aspeed_vuart *vuart = uart_8250_port->port.private_data;
+ int rc;
+
+ rc = serial8250_do_startup(uart_port);
+ if (rc)
+ return rc;
+
+ aspeed_vuart_set_host_tx_discard(vuart, false);
+
+ return 0;
+}
+
+static void aspeed_vuart_shutdown(struct uart_port *uart_port)
+{
+ struct uart_8250_port *uart_8250_port = up_to_u8250p(uart_port);
+ struct aspeed_vuart *vuart = uart_8250_port->port.private_data;
+
+ aspeed_vuart_set_host_tx_discard(vuart, true);
+
+ serial8250_do_shutdown(uart_port);
+}
+
+static int aspeed_vuart_probe(struct platform_device *pdev)
+{
+ struct uart_8250_port port;
+ struct aspeed_vuart *vuart;
+ struct device_node *np;
+ struct resource *res;
+ u32 clk, prop;
+ int rc;
+
+ np = pdev->dev.of_node;
+
+ vuart = devm_kzalloc(&pdev->dev, sizeof(*vuart), GFP_KERNEL);
+ if (!vuart)
+ return -ENOMEM;
+
+ vuart->dev = &pdev->dev;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ vuart->regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(vuart->regs))
+ return PTR_ERR(vuart->regs);
+
+ memset(&port, 0, sizeof(port));
+ port.port.private_data = vuart;
+ port.port.membase = vuart->regs;
+ port.port.mapbase = res->start;
+ port.port.mapsize = resource_size(res);
+ port.port.startup = aspeed_vuart_startup;
+ port.port.shutdown = aspeed_vuart_shutdown;
+ port.port.dev = &pdev->dev;
+
+ rc = sysfs_create_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
+ if (rc < 0)
+ return rc;
+
+ if (of_property_read_u32(np, "clock-frequency", &clk)) {
+ vuart->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(vuart->clk)) {
+ dev_warn(&pdev->dev,
+ "clk or clock-frequency not defined\n");
+ return PTR_ERR(vuart->clk);
+ }
+
+ rc = clk_prepare_enable(vuart->clk);
+ if (rc < 0)
+ return rc;
+
+ clk = clk_get_rate(vuart->clk);
+ }
+
+ /* If current-speed was set, then try not to change it. */
+ if (of_property_read_u32(np, "current-speed", &prop) == 0)
+ port.port.custom_divisor = clk / (16 * prop);
+
+ /* Check for shifted address mapping */
+ if (of_property_read_u32(np, "reg-offset", &prop) == 0)
+ port.port.mapbase += prop;
+
+ /* Check for registers offset within the devices address range */
+ if (of_property_read_u32(np, "reg-shift", &prop) == 0)
+ port.port.regshift = prop;
+
+ /* Check for fifo size */
+ if (of_property_read_u32(np, "fifo-size", &prop) == 0)
+ port.port.fifosize = prop;
+
+ /* Check for a fixed line number */
+ rc = of_alias_get_id(np, "serial");
+ if (rc >= 0)
+ port.port.line = rc;
+
+ port.port.irq = irq_of_parse_and_map(np, 0);
+ port.port.irqflags = IRQF_SHARED;
+ port.port.iotype = UPIO_MEM;
+ port.port.type = PORT_16550A;
+ port.port.uartclk = clk;
+ port.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF
+ | UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_NO_THRE_TEST;
+
+ if (of_property_read_bool(np, "no-loopback-test"))
+ port.port.flags |= UPF_SKIP_TEST;
+
+ if (port.port.fifosize)
+ port.capabilities = UART_CAP_FIFO;
+
+ if (of_property_read_bool(np, "auto-flow-control"))
+ port.capabilities |= UART_CAP_AFE;
+
+ rc = serial8250_register_8250_port(&port);
+ if (rc < 0)
+ goto err_clk_disable;
+
+ vuart->line = rc;
+
+ aspeed_vuart_set_enabled(vuart, true);
+ aspeed_vuart_set_host_tx_discard(vuart, true);
+ platform_set_drvdata(pdev, vuart);
+
+ return 0;
+
+err_clk_disable:
+ clk_disable_unprepare(vuart->clk);
+ irq_dispose_mapping(port.port.irq);
+ return rc;
+}
+
+static int aspeed_vuart_remove(struct platform_device *pdev)
+{
+ struct aspeed_vuart *vuart = platform_get_drvdata(pdev);
+
+ aspeed_vuart_set_enabled(vuart, false);
+ serial8250_unregister_port(vuart->line);
+ sysfs_remove_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
+ clk_disable_unprepare(vuart->clk);
+
+ return 0;
+}
+
+static const struct of_device_id aspeed_vuart_table[] = {
+ { .compatible = "aspeed,ast2400-vuart" },
+ { .compatible = "aspeed,ast2500-vuart" },
+ { },
+};
+
+static struct platform_driver aspeed_vuart_driver = {
+ .driver = {
+ .name = "aspeed-vuart",
+ .of_match_table = aspeed_vuart_table,
+ },
+ .probe = aspeed_vuart_probe,
+ .remove = aspeed_vuart_remove,
+};
+
+module_platform_driver(aspeed_vuart_driver);
+
+MODULE_AUTHOR("Jeremy Kerr <jk@ozlabs.org>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Driver for Aspeed VUART device");
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index 0e3f529d50e9..a1161ec0256f 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -224,6 +224,16 @@ config SERIAL_8250_ACCENT
To compile this driver as a module, choose M here: the module
will be called 8250_accent.
+config SERIAL_8250_ASPEED_VUART
+ tristate "Aspeed Virtual UART"
+ depends on SERIAL_8250
+ depends on OF
+ help
+ If you want to use the virtual UART (VUART) device on Aspeed
+ BMC platforms, enable this option. This enables the 16550A-
+ compatible device on the local LPC bus, giving a UART device
+ with no physical RS232 connections.
+
config SERIAL_8250_BOCA
tristate "Support Boca cards"
depends on SERIAL_8250 != n && ISA && SERIAL_8250_MANY_PORTS
diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
index 2f30f9ecdb1b..a44a99a3e623 100644
--- a/drivers/tty/serial/8250/Makefile
+++ b/drivers/tty/serial/8250/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_SERIAL_8250_EXAR) += 8250_exar.o
obj-$(CONFIG_SERIAL_8250_HP300) += 8250_hp300.o
obj-$(CONFIG_SERIAL_8250_CS) += serial_cs.o
obj-$(CONFIG_SERIAL_8250_ACORN) += 8250_acorn.o
+obj-$(CONFIG_SERIAL_8250_ASPEED_VUART) += 8250_aspeed_vuart.o
obj-$(CONFIG_SERIAL_8250_BCM2835AUX) += 8250_bcm2835aux.o
obj-$(CONFIG_SERIAL_8250_CONSOLE) += 8250_early.o
obj-$(CONFIG_SERIAL_8250_FOURPORT) += 8250_fourport.o
--
2.11.0
^ permalink raw reply related
* [PATCH v4 1/2] serial: 8250: Add flag so drivers can avoid THRE probe
From: Joel Stanley @ 2017-05-02 7:45 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: linux-serial, linux-kernel, devicetree, Andy Shevchenko,
Benjamin Herrenschmidt, Jeremy Kerr, Mark Rutland, Rob Herring,
openbmc
In-Reply-To: <20170502074543.1380-1-joel@jms.id.au>
The probing of THRE irq behaviour assumes the other end will be reading
bytes out of the buffer in order to probe the port at driver init. In
some cases the other end cannot be relied upon to read these bytes, so
provide a flag for them to skip this step.
Bit 19 was chosen as the flags are a int and the top bits are taken.
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
v3:
- No change
v3:
- Correct the bit number in the changelog
v2:
- No change
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
drivers/tty/serial/8250/8250_port.c | 2 +-
include/linux/serial_core.h | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 6119516ef5fc..60a6c247340f 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2229,7 +2229,7 @@ int serial8250_do_startup(struct uart_port *port)
}
}
- if (port->irq) {
+ if (port->irq && !(up->port.flags & UPF_NO_THRE_TEST)) {
unsigned char iir1;
/*
* Test for UARTs that do not reassert THRE when the
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 58484fb35cc8..260245deec94 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -195,6 +195,7 @@ struct uart_port {
#define UPF_NO_TXEN_TEST ((__force upf_t) (1 << 15))
#define UPF_MAGIC_MULTIPLIER ((__force upf_t) ASYNC_MAGIC_MULTIPLIER /* 16 */ )
+#define UPF_NO_THRE_TEST ((__force upf_t) (1 << 19))
/* Port has hardware-assisted h/w flow control */
#define UPF_AUTO_CTS ((__force upf_t) (1 << 20))
#define UPF_AUTO_RTS ((__force upf_t) (1 << 21))
--
2.11.0
^ permalink raw reply related
* [PATCH v4 0/2] drivers: serial: Aspeed VUART driver
From: Joel Stanley @ 2017-05-02 7:45 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Andy Shevchenko,
Benjamin Herrenschmidt, Jeremy Kerr, Mark Rutland, Rob Herring,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ
This is v4 of a driver for the Aspeed VUART. This version addresses feedback
from Andy and Greg, and includes Rob's ack for the bindings change.
The VUART is a serial device on the BMC side of the LPC bus that connects a BMC
to it's host processor.
We add a flag to the serial core to allow the driver to skip probing of the
THRE irq behaviour, which could hang due to the host not reading bytes out of
the buffer.
We've been using this on systems for over a year, so it has seen a good amount
of testing.
Cheers,
Joel
Jeremy Kerr (1):
drivers/serial: Add driver for Aspeed virtual UART
Joel Stanley (1):
serial: 8250: Add flag so drivers can avoid THRE probe
Documentation/ABI/stable/sysfs-driver-aspeed-vuart | 15 +
Documentation/devicetree/bindings/serial/8250.txt | 2 +
drivers/tty/serial/8250/8250_aspeed_vuart.c | 323 +++++++++++++++++++++
drivers/tty/serial/8250/8250_port.c | 2 +-
drivers/tty/serial/8250/Kconfig | 10 +
drivers/tty/serial/8250/Makefile | 1 +
include/linux/serial_core.h | 1 +
7 files changed, 353 insertions(+), 1 deletion(-)
create mode 100644 Documentation/ABI/stable/sysfs-driver-aspeed-vuart
create mode 100644 drivers/tty/serial/8250/8250_aspeed_vuart.c
--
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
* Re: [PATCH v3 2/2] drivers/serial: Add driver for Aspeed virtual UART
From: Joel Stanley @ 2017-05-02 7:45 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Greg Kroah-Hartman, Jiri Slaby, Mark Rutland, Rob Herring,
Jeremy Kerr, linux-serial@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree, Benjamin Herrenschmidt,
OpenBMC Maillist
In-Reply-To: <CAHp75Vd=oONhzF34Pe8WwYKBWWxcsaQtgJy66aFD68iNKTxEfg@mail.gmail.com>
On Tue, Apr 11, 2017 at 9:45 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Apr 10, 2017 at 7:04 AM, Joel Stanley <joel@jms.id.au> wrote:
>> From: Jeremy Kerr <jk@ozlabs.org>
>>
>> This change adds a driver for the 16550-based Aspeed virtual UART
>> device. We use a similar process to the of_serial driver for device
>> probe, but expose some VUART-specific functions through sysfs too.
>>
>> The VUART is two UART 'front ends' connected by their FIFO (no actual
>> serial line in between). One is on the BMC side (management controller)
>> and one is on the host CPU side.
>>
>> This driver is for the BMC side. The sysfs files allow the BMC
>> userspace, which owns the system configuration policy, to specify at
>> what IO port and interrupt number the host side will appear to the host
>> on the Host <-> BMC LPC bus. It could be different on a different system
>> (though most of them use 3f8/4).
>>
>> OpenPOWER host firmware doesn't like it when the host-side of the
>> VUART's FIFO is not drained. This driver only disables host TX discard
>> mode when the port is in use. We set the VUART enabled bit when we bind
>> to the device, and clear it on unbind.
>>
>> We don't want to do this on open/release, as the host may be using this
>> bit to configure serial output modes, which is independent of whether
>> the devices has been opened by BMC userspace.
>
>> +static void aspeed_vuart_set_enabled(struct aspeed_vuart *vuart, bool enabled)
>> +{
>> + u8 reg;
>> +
>> + reg = readb(vuart->regs + ASPEED_VUART_GCRA);
>
>> + reg &= ~ASPEED_VUART_GCRA_VUART_EN;
>> + if (enabled)
>> + reg |= ASPEED_VUART_GCRA_VUART_EN;
>
> Usually the pattern is
> if (something)
> set x bit;
> else
> clear x bit;
>
> It would make it one operation in any case and a bit more understandable.
I have made these ordering changes you requested.
>
>> + port.port.irq = irq_of_parse_and_map(np, 0);
>
> The benefit of use platform_get_irq() is to get rid of some OF specific headers.
It's an of driver, and this function does exactly what we require. I
have left it in for v4.
Cheers,
Joel
^ permalink raw reply
* [PATCH] tty: serdev: fix serdev_device_write return value
From: Rob Herring @ 2017-05-02 0:17 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-serial, linux-kernel, Andrey Smirnov
Commit 6fe729c4bdae ("serdev: Add serdev_device_write subroutine")
provides a compatibility wrapper for the existing
serdev_device_write_buf, but it fails to return the number of bytes
written causing users to timeout.
Fixes: 6fe729c4bdae ("serdev: Add serdev_device_write subroutine")
Cc: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Rob Herring <robh@kernel.org>
---
drivers/tty/serdev/core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index 433de5ea9b02..ccfe56355c4f 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -127,7 +127,7 @@ int serdev_device_write(struct serdev_device *serdev,
unsigned long timeout)
{
struct serdev_controller *ctrl = serdev->ctrl;
- int ret;
+ int ret, wr_cnt = 0;
if (!ctrl || !ctrl->ops->write_buf ||
(timeout && !serdev->ops->write_wakeup))
@@ -143,12 +143,13 @@ int serdev_device_write(struct serdev_device *serdev,
buf += ret;
count -= ret;
+ wr_cnt += ret;
} while (count &&
(timeout = wait_for_completion_timeout(&serdev->write_comp,
timeout)));
mutex_unlock(&serdev->write_lock);
- return ret < 0 ? ret : (count ? -ETIMEDOUT : 0);
+ return ret < 0 ? ret : (count ? -ETIMEDOUT : wr_cnt);
}
EXPORT_SYMBOL_GPL(serdev_device_write);
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] SPCR: check bit width for the 16550 UART
From: Jon Masters @ 2017-04-30 21:39 UTC (permalink / raw)
To: Mark Salter, Duc Dang
Cc: Aleksey Makarov, Rafael J . Wysocki, linux-acpi, linux-serial,
Linux Kernel Mailing List, Greg Kroah-Hartman, Russell King,
Peter Hurley, Graeme Gregory, Len Brown
In-Reply-To: <3a27e455-dc5f-4fee-aa72-4eab752e91d0@redhat.com>
On 12/13/2016 01:20 AM, Jon Masters wrote:
> On 12/07/2016 10:23 AM, Mark Salter wrote:
>> If you specify a baudrate with earlycon=, the driver tries to set that
>> baudrate and if you have an 8250 with some non-standard baud clock, then
>> it will fail. Perhaps SPCR shouldn't pass baud option to setup_earlycon().
>
> Yet they seem to explicitly want to do this...in my conversations with some
> others we agree that, in many cases, you really want to say "leave the baud
> whatever the firmware set it", which would work in this case, but might
> break some others. Then again, nobody on x86 Linux is really using the
> SPCR today due to it not having been something they used until now and
> due to the location of the COM ports being fairly well known ;)
>
> So who knows what folks will prefer, but we should at least get the spec
> to cover both situations by explicitly calling out Applied as special.
<snip>
> So I've been discussing some changes to the SPCR and the current proposal
> is that we have two new subtypes - one for 16550s that are non-standard
> register width/stride but use the typical base clock, and a specific
> additional type for SBSA level 0 compatible 16550 UARTs (Applied). I
> will followup when the specification document has been revised.
As an update. I've been speaking with friends at Microsoft on and off
about this for quite some time. They've been very helpful (as usual) and
we should get an SPCR update soon covering Applied's case. I have pinged
the APM team to make sure that they're ready to post a patch.
I would like this small fix squeezed in 4.12, but before 4.13.
Jon.
^ permalink raw reply
* [PATCH v2] tty/serial: atmel: use offset_in_page() macro
From: Geliang Tang @ 2017-04-29 1:39 UTC (permalink / raw)
To: Richard Genoud, Greg Kroah-Hartman, Jiri Slaby
Cc: Geliang Tang, linux-serial, linux-kernel
In-Reply-To: <2c5f1602-c474-d3d3-01d3-03e7bc22f91f@gmail.com>
Use offset_in_page() macro instead of open-coding.
Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
Changes in v2:
- include mm.h
---
drivers/tty/serial/atmel_serial.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index c355ac9..d25f044 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -46,6 +46,7 @@
#include <linux/err.h>
#include <linux/irq.h>
#include <linux/suspend.h>
+#include <linux/mm.h>
#include <asm/io.h>
#include <asm/ioctls.h>
@@ -959,7 +960,7 @@ static int atmel_prepare_tx_dma(struct uart_port *port)
sg_set_page(&atmel_port->sg_tx,
virt_to_page(port->state->xmit.buf),
UART_XMIT_SIZE,
- (unsigned long)port->state->xmit.buf & ~PAGE_MASK);
+ offset_in_page(port->state->xmit.buf));
nent = dma_map_sg(port->dev,
&atmel_port->sg_tx,
1,
@@ -1141,7 +1142,7 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
sg_set_page(&atmel_port->sg_rx,
virt_to_page(ring->buf),
sizeof(struct atmel_uart_char) * ATMEL_SERIAL_RINGSIZE,
- (unsigned long)ring->buf & ~PAGE_MASK);
+ offset_in_page(ring->buf));
nent = dma_map_sg(port->dev,
&atmel_port->sg_rx,
1,
--
2.9.3
^ permalink raw reply related
* [PATCH] serdev: Restore serdev_device_write_buf for atomic context
From: Stefan Wahren @ 2017-04-28 11:47 UTC (permalink / raw)
To: Rob Herring
Cc: Greg Kroah-Hartman, Jiri Slaby, Sebastian Reichel, Guenter Roeck,
Andy Shevchenko, Andrey Smirnov, linux-serial, linux-kernel,
Stefan Wahren
Starting with commit 6fe729c4bdae ("serdev: Add serdev_device_write
subroutine") the function serdev_device_write_buf cannot be used in
atomic context anymore (mutex_lock is sleeping). So restore the old
behavior.
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
Fixes: 6fe729c4bdae ("serdev: Add serdev_device_write subroutine")
---
drivers/tty/serdev/core.c | 12 ++++++++++++
include/linux/serdev.h | 14 +++++++-------
2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index 433de5e..f71b473 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -122,6 +122,18 @@ void serdev_device_write_wakeup(struct serdev_device *serdev)
}
EXPORT_SYMBOL_GPL(serdev_device_write_wakeup);
+int serdev_device_write_buf(struct serdev_device *serdev,
+ const unsigned char *buf, size_t count)
+{
+ struct serdev_controller *ctrl = serdev->ctrl;
+
+ if (!ctrl || !ctrl->ops->write_buf)
+ return -EINVAL;
+
+ return ctrl->ops->write_buf(ctrl, buf, count);
+}
+EXPORT_SYMBOL_GPL(serdev_device_write_buf);
+
int serdev_device_write(struct serdev_device *serdev,
const unsigned char *buf, size_t count,
unsigned long timeout)
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index cda76c6..e2a225b 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -195,6 +195,7 @@ 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_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);
@@ -236,6 +237,12 @@ 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 *serdev,
+ const unsigned char *buf,
+ size_t count)
+{
+ return -ENODEV;
+}
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)
{
@@ -312,11 +319,4 @@ 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.1.4
^ permalink raw reply related
* [PATCH] tty: ehv_bytechan: clean up init error handling
From: Johan Hovold @ 2017-04-26 10:30 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-kernel, Timur Tabi, Johan Hovold
Straighten out the initcall error handling to avoid deregistering a
never-registered tty driver (something which would lead to a
NULL-pointer dereference) in the most unlikely event that driver
registration fails (e.g. we've run out of major numbers).
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/tty/ehv_bytechan.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/ehv_bytechan.c b/drivers/tty/ehv_bytechan.c
index 7ac9bcdf1e61..61fe8d6fd24e 100644
--- a/drivers/tty/ehv_bytechan.c
+++ b/drivers/tty/ehv_bytechan.c
@@ -764,7 +764,7 @@ static int __init ehv_bc_init(void)
ehv_bc_driver = alloc_tty_driver(count);
if (!ehv_bc_driver) {
ret = -ENOMEM;
- goto error;
+ goto err_free_bcs;
}
ehv_bc_driver->driver_name = "ehv-bc";
@@ -778,24 +778,23 @@ static int __init ehv_bc_init(void)
ret = tty_register_driver(ehv_bc_driver);
if (ret) {
pr_err("ehv-bc: could not register tty driver (ret=%i)\n", ret);
- goto error;
+ goto err_put_tty_driver;
}
ret = platform_driver_register(&ehv_bc_tty_driver);
if (ret) {
pr_err("ehv-bc: could not register platform driver (ret=%i)\n",
ret);
- goto error;
+ goto err_deregister_tty_driver;
}
return 0;
-error:
- if (ehv_bc_driver) {
- tty_unregister_driver(ehv_bc_driver);
- put_tty_driver(ehv_bc_driver);
- }
-
+err_deregister_tty_driver:
+ tty_unregister_driver(ehv_bc_driver);
+err_put_tty_driver:
+ put_tty_driver(ehv_bc_driver);
+err_free_bcs:
kfree(bcs);
return ret;
--
2.12.2
^ permalink raw reply related
* [PATCH] serial: ifx6x60: fix use-after-free on module unload
From: Johan Hovold @ 2017-04-26 10:24 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-kernel, Johan Hovold, stable,
Jun Chen
Make sure to deregister the SPI driver before releasing the tty driver
to avoid use-after-free in the SPI remove callback where the tty
devices are deregistered.
Fixes: 72d4724ea54c ("serial: ifx6x60: Add modem power off function in the platform reboot process")
Cc: stable <stable@vger.kernel.org> # 3.8
Cc: Jun Chen <jun.d.chen@intel.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/tty/serial/ifx6x60.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/ifx6x60.c b/drivers/tty/serial/ifx6x60.c
index 157883653256..f190a84a0246 100644
--- a/drivers/tty/serial/ifx6x60.c
+++ b/drivers/tty/serial/ifx6x60.c
@@ -1382,9 +1382,9 @@ static struct spi_driver ifx_spi_driver = {
static void __exit ifx_spi_exit(void)
{
/* unregister */
+ spi_unregister_driver(&ifx_spi_driver);
tty_unregister_driver(tty_drv);
put_tty_driver(tty_drv);
- spi_unregister_driver(&ifx_spi_driver);
unregister_reboot_notifier(&ifx_modem_reboot_notifier_block);
}
--
2.12.2
^ permalink raw reply related
* Re: [PATCH] serial: sh-sci: Fix race condition causing garbage during shutdown
From: Richard Genoud @ 2017-04-26 10:08 UTC (permalink / raw)
To: Geert Uytterhoeven, Greg Kroah-Hartman
Cc: Jiri Slaby, Yoshihiro Shimoda, linux-serial, linux-renesas-soc,
linux-kernel
In-Reply-To: <1493144135-898-1-git-send-email-geert+renesas@glider.be>
On 25/04/2017 20:15, Geert Uytterhoeven wrote:
> If DMA is enabled and used, a burst of old data may be seen on the
> serial console during "poweroff" or "reboot". uart_flush_buffer()
> clears the circular buffer, but sci_port.tx_dma_len is not reset.
> This leads to a circular buffer overflow, dumping (UART_XMIT_SIZE -
> sci_port.tx_dma_len) bytes.
>
> To fix this, add a .flush_buffer() callback that resets
> sci_port.tx_dma_len.
>
> Inspired by commit 31ca2c63fdc0aee7 ("tty/serial: atmel: fix race
> condition (TX+DMA)").
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> A big thanks to Richard!
> I had no idea what was happening here, until I saw his patch passing by
> in a stable backport.
:)
you're welcome !
> In v4.3 and older, the field is called sg_len_tx.
> ---
> drivers/tty/serial/sh-sci.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index 9a47cc4f16a2798f..a15739202d6c75f2 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -1545,7 +1545,16 @@ static void sci_free_dma(struct uart_port *port)
> if (s->chan_rx)
> sci_rx_dma_release(s, false);
> }
> -#else
> +
> +static void sci_flush_buffer(struct uart_port *port)
> +{
> + /*
> + * In uart_flush_buffer(), the xmit circular buffer has just been
> + * cleared, so we have to reset tx_dma_len accordingly.
> + */
> + to_sci_port(port)->tx_dma_len = 0;
> +}
> +#else /* !CONFIG_SERIAL_SH_SCI_DMA */
> static inline void sci_request_dma(struct uart_port *port)
> {
> }
> @@ -1553,7 +1562,9 @@ static inline void sci_request_dma(struct uart_port *port)
> static inline void sci_free_dma(struct uart_port *port)
> {
> }
> -#endif
> +
> +#define sci_flush_buffer NULL
> +#endif /* !CONFIG_SERIAL_SH_SCI_DMA */
>
> static irqreturn_t sci_rx_interrupt(int irq, void *ptr)
> {
> @@ -2566,6 +2577,7 @@ static const struct uart_ops sci_uart_ops = {
> .break_ctl = sci_break_ctl,
> .startup = sci_startup,
> .shutdown = sci_shutdown,
> + .flush_buffer = sci_flush_buffer,
> .set_termios = sci_set_termios,
> .pm = sci_pm,
> .type = sci_type,
>
^ permalink raw reply
* Re: [PATCH] serial: imx: Enable RTSD only when needed
From: Romain Perier @ 2017-04-26 7:17 UTC (permalink / raw)
To: Fabio Estevam
Cc: Greg Kroah-Hartman, linux-serial@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Nandor Han
In-Reply-To: <CAOMZO5Anz_C-pxPke_mqCtD_D9LBxTRWSk5fAZrr6KaMeMZGqQ@mail.gmail.com>
Hello all,
Le 12/04/2017 à 15:38, Fabio Estevam a écrit :
> On Wed, Apr 12, 2017 at 10:30 AM, Romain Perier
> <romain.perier@collabora.com> wrote:
>> From: Nandor Han <nandor.han@ge.com>
>>
>> Currently, this IRQ is always enabled. Some devices might mux these pins
>> to other I/Os, like I2C. This could lead to spurious interrupts.
>>
>> This commit makes this IRQ optional, by using the field have_rtscts.
>>
>> Signed-off-by: Nandor Han <nandor.han@ge.com>
>> Signed-off-by: Romain Perier <romain.perier@collabora.com>
> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
No more feedback on this patch ?
Regards,
Romain
_______________________________________________
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: serial: core: Dynamic minor support
From: Alan Cox @ 2017-04-25 19:43 UTC (permalink / raw)
To: Sjoerd Simons
Cc: Greg Kroah-Hartman, linux-serial, Geert Uytterhoeven,
linux-kernel, Jiri Slaby
In-Reply-To: <20170420120357.18317-1-sjoerd.simons@collabora.co.uk>
> Furthermore some other drivers rely upon the usage of a dynamic major
> e.g. the tegra serial port driver. However as there is only a block of
> 20 major numbers reserved for dynamic assignment that isn't going to
> scale either for multiplatform kernels (I can already easily build 18
> different serial drivers on an arm64 kernel today).
The patch makes sense to me - we really don't need the static minors any
more.
> This does not try to address device naming by making all serial ports be
> ttyS<value> as that would first need a better way of deterministically
> specifying serial console devices. While it would be an option to move
> to consistent serial device naming while keeping the current console
> names, having both not aligned would probably confuse everyone
> massively.
The existing ttyS being 8250 is ABI and there are lots of things that
know far too much about what each type of ttyFOO is to ever fix that.
What could certainly be done is to do what some other classes of device
do and start also registering
/dev/ttyport/bus-information/0 /1 /2 .. etc
in parallel.
Alan
^ permalink raw reply
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