* Re: [PATCH v3] uart: fix race between uart_put_char() and uart_shutdown()
From: Andy Shevchenko @ 2018-07-06 16:49 UTC (permalink / raw)
To: Tycho Andersen
Cc: Greg Kroah-Hartman, Jiri Slaby, open list:SERIAL DRIVERS,
Linux Kernel Mailing List, Serge E . Hallyn
In-Reply-To: <20180706162457.20489-1-tycho@tycho.ws>
On Fri, Jul 6, 2018 at 7:24 PM, Tycho Andersen <tycho@tycho.ws> wrote:
> Looking in uart_port_startup(), it seems that circ->buf (state->xmit.buf)
> protected by the "per-port mutex", which based on uart_port_check() is
> state->port.mutex. Indeed, the lock acquired in uart_put_char() is
> uport->lock, i.e. not the same lock.
>
> Anyway, since the lock is not acquired, if uart_shutdown() is called, the
> last chunk of that function may release state->xmit.buf before its assigned
> to null, and cause the race above.
>
> To fix it, let's lock uport->lock when allocating/deallocating
> state->xmit.buf in addition to the per-port mutex.
Thanks for fixing this!
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Some nitpicks though.
> + unsigned long page, flags = 0;
I would rather put on separate lines and btw assignment is not needed.
It all goes through macros.
> - if (!state->xmit.buf) {
> - /* This is protected by the per port mutex */
> - page = get_zeroed_page(GFP_KERNEL);
> - if (!page)
> - return -ENOMEM;
> + page = get_zeroed_page(GFP_KERNEL);
> + if (!page)
> + return -ENOMEM;
> + if (!state->xmit.buf) {
> state->xmit.buf = (unsigned char *) page;
> uart_circ_clear(&state->xmit);
> + } else {
> + free_page(page);
> }
I see original code, but since you are adding else, does it make sense
to switch to positive condition?
> + unsigned long flags = 0;
Ditto about assignment.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 3/3] serial: 8250_dw: add fractional divisor support
From: Andy Shevchenko @ 2018-07-06 17:37 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Greg Kroah-Hartman, Jiri Slaby, linux-arm-kernel, linux-kernel,
linux-serial
In-Reply-To: <20180705143921.6a8aeb50@xhacker.debian>
On Thu, 2018-07-05 at 14:39 +0800, Jisheng Zhang wrote:
> > On Wed, 2018-07-04 at 17:03 +0800, Jisheng Zhang wrote:
My comments below.
> > > For Synopsys DesignWare 8250 uart which version >= 4.00a, there's
> > > a
> > > valid divisor latch fraction register. The fractional divisor
> > > width is
> > > 4bits ~ 6bits.
> >
> > I have read 4.00a spec a bit and didn't find this limitation.
> > The fractional divider can fit up to 32 bits.
>
> the limitation isn't put in the register description, but in the
> description
> about fractional divisor width configure parameters. Searching
> DLF_SIZE will
> find it.
Found, thanks.
> From another side, 32bits fractional div is a waste, for example,
> let's
> assume the fract divisor = 0.9,
> if fractional width is 4 bits, DLF reg will be set to UP(0.9*2^4) =
> 15, the
> real frac divisor = 15/2^4 = 0.93;
> if fractional width is 32 bits, DLF reg will be set to UP(0.9*2^32) =
> 3865470567
> the real frac divisor = 3865470567/2^32 = 0.90;
So, your example shows that 32-bit gives better value. Where is
contradiction?
> > I would test this a bit later.
It reads back 0 on our hardware with 3.xx version of IP.
> > > + unsigned int dlf_size:3;
> >
> > These bit fields (besides the fact you need 5) more or less for
> > software
> > quirk flags. In your case I would rather keep a u32 value of DFL
> > mask as
> > done for msr slightly above in this structure.
>
> OK. When setting the frac div, we use DLF size rather than mask, so
> could
> I only calculate the DLF size once then use an u8 value to hold the
> calculated DLF size rather than calculating every time?
Let's see below.
> > > + quot = DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4),
> > > baud);
> >
> > If we have clock rate like 100MHz and 10 bits of fractional divider
> > it
> > would give an integer overflow.
>
> This depends on the fraction divider width. If it's only 4~6 bits,
> then we are fine.
True.
> >
> > 4 here is a magic. Needs to be commented / described better.
>
> Yes. divisor = clk/(16*baud) = BRD(I) + BRD(F)
> "I" means integer, "F" means fractional
>
> 2^dlf_size*clk/(16*baud) = 2^dlf_size*(BRD(I) + BRD(F))
>
> clk*2^(dlf_size - 4)/baud = 2^dlf_size*((BRD(I) + BRD(F))
>
> the left part is "DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4),
> baud)",
> let's assume it equals quot.
>
> then, BRD(I) = quot / 2^dlf_size = quot >> dlf_size, this is where
> "quot >> d->dlf_size" below from.
>
> BRD(F) = quot & dlf_mask = quot & (~0U >> (32 - dlf_size))
Yes, I understand from where it comes. It's a hard coded value of PS all
over the serial code.
And better use the same idiom as in other code, i.e. * 16 or / 16
depends on context.
> > > + *frac = quot & (~0U >> (32 - d->dlf_size));
> > > +
> >
> > Operating on dfl_mask is simple as
> >
> > u64 quot = p->uartclk * (p->dfl_mask + 1);
>
> Since the dlf_mask is always 2^n - 1,
> clk * (p->dlf_mask + 1) = clk << p->dlf_size, but the later
> is simpler
>
> >
> > *frac = div64_u64(quot, baud * 16 * (p->dfl_mask + 1);
> > return quot;
>
> quot = DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4), baud)
> *frac = quot & (~0U >> (32 - d->dlf_size))
> return quot >> d->dlf_size;
>
> vs.
>
> quot = p->uartclk * (p->dfl_mask + 1);
> *frac = div64_u64(quot, baud * 16 * (p->dfl_mask + 1);
> return quot;
>
> shift vs mul.
>
> If the dlf width is only 4~6 bits, the first calculation can avoid
> 64bit div. I prefer the first calculation.
OK, taking that into consideration and looking at the code snippets
again I would to
- keep two values
// mask we get for free because it's needed in intermediate calculus
//
and it doesn't overflow u8 (4-6 bits by spec)
u8 dlf_mask;
u8 dlf_size;
- implement function like below
unsigned int quot;
/* Consider maximum possible DLF_SIZE, i.e. 6 bits */
quot = DIV_ROUND_CLOSEST(p->uartclk * 4, baud);
*frac = (quot >> (6 - dlf_size)) & dlf_mask;
return quot >> dlf_size;
Would you agree it looks slightly less complicated?
> > > + serial_port_out(p, UART_LCR, up->lcr | UART_LCR_DLAB);
> > > + serial_dl_write(up, quot);
(1)
> >
> > At some point it would be a helper, I think. We can call
> > serial8250_do_set_divisor() here. So, perhaps we might export it.
>
> serial8250_do_set_divisor will drop the frac, that's not we want ;)
Can you check again? What I see is that for DW 8250 the
_do_set_divisor() would be an equivalent to the two lines, i.e. (1).
And basically at the end it should become just those two lines when the
rest will implement their custom set_divisor().
> > This should use some helpers. I'll prepare a patch soon and send it
> > here, you may include it in your series.
>
> Nice. Thanks.
Sent.
> > > + d->dlf_size = fls(reg);
> >
> > Just save value itself as dfl_mask.
>
> we use the dlf size during calculation, so it's better to hold the
> dlf_size
> instead.
See above.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCH v2 3/3] serial: 8250_dw: add fractional divisor support
From: Andy Shevchenko @ 2018-07-06 17:39 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Greg Kroah-Hartman, Jiri Slaby, linux-arm-kernel, linux-kernel,
linux-serial
In-Reply-To: <20180705145458.7e7d9b9f@xhacker.debian>
On Thu, 2018-07-05 at 14:54 +0800, Jisheng Zhang wrote:
> On Thu, 5 Jul 2018 14:39:21 +0800 Jisheng Zhang wrote:
>
> <snip>
>
> > >
> > > > + serial_port_out(p, UART_LCR, up->lcr | UART_LCR_DLAB);
> > > > + serial_dl_write(up, quot);
> > >
> > > At some point it would be a helper, I think. We can call
> > > serial8250_do_set_divisor() here. So, perhaps we might export
> > > it.
> >
> > serial8250_do_set_divisor will drop the frac, that's not we want ;)
> >
>
> And most importantly, serial8250_do_set_divisor() will set a wrong
> BRD(I)
> for fractional capable DW uarts. For example, clk = 25MHZ, baud =
> 115200.
>
> In fractional capable DW uarts, we should set BRD(I) as
> 25000000/(16*115200) = 13
>
> but serial8250_do_set_divisor() will set BRD(I) as
> DIV_ROUND_CLOSEST(25*1000000, 16*115200)) = 14
How come? It doesn't do any calculus (for DW 8250), it just writes few
registers based on input.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCH v3] uart: fix race between uart_put_char() and uart_shutdown()
From: Tycho Andersen @ 2018-07-06 18:39 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Greg Kroah-Hartman, Jiri Slaby, open list:SERIAL DRIVERS,
Linux Kernel Mailing List, Serge E . Hallyn
In-Reply-To: <CAHp75Vfbh5SzpqP=2LfeXteEGrBG46dPfeHsU0ac5SiJSjZOXw@mail.gmail.com>
On Fri, Jul 06, 2018 at 07:49:09PM +0300, Andy Shevchenko wrote:
> On Fri, Jul 6, 2018 at 7:24 PM, Tycho Andersen <tycho@tycho.ws> wrote:
>
> > Looking in uart_port_startup(), it seems that circ->buf (state->xmit.buf)
> > protected by the "per-port mutex", which based on uart_port_check() is
> > state->port.mutex. Indeed, the lock acquired in uart_put_char() is
> > uport->lock, i.e. not the same lock.
> >
> > Anyway, since the lock is not acquired, if uart_shutdown() is called, the
> > last chunk of that function may release state->xmit.buf before its assigned
> > to null, and cause the race above.
> >
> > To fix it, let's lock uport->lock when allocating/deallocating
> > state->xmit.buf in addition to the per-port mutex.
>
> Thanks for fixing this!
>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> Some nitpicks though.
>
> > + unsigned long page, flags = 0;
>
> I would rather put on separate lines and btw assignment is not needed.
> It all goes through macros.
Sure, I can split it up, but without the initialization I get,
CC drivers/tty/serial/serial_core.o
In file included from ./include/linux/seqlock.h:36:0,
from ./include/linux/time.h:6,
from ./include/linux/stat.h:19,
from ./include/linux/module.h:10,
from drivers/tty/serial/serial_core.c:10:
drivers/tty/serial/serial_core.c: In function ‘uart_startup.part.20’:
./include/linux/spinlock.h:260:3: warning: ‘flags’ may be used uninitialized in this function -Wmaybe-uninitialized]
_raw_spin_unlock_irqrestore(lock, flags); \
^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/tty/serial/serial_core.c:184:22: note: ‘flags’ was declared here
unsigned long page, flags;
^~~~~
> > - if (!state->xmit.buf) {
> > - /* This is protected by the per port mutex */
> > - page = get_zeroed_page(GFP_KERNEL);
> > - if (!page)
> > - return -ENOMEM;
> > + page = get_zeroed_page(GFP_KERNEL);
> > + if (!page)
> > + return -ENOMEM;
> > + if (!state->xmit.buf) {
> > state->xmit.buf = (unsigned char *) page;
> > uart_circ_clear(&state->xmit);
> > + } else {
> > + free_page(page);
> > }
>
> I see original code, but since you are adding else, does it make sense
> to switch to positive condition?
Sure, I'll switch it.
> > + unsigned long flags = 0;
>
> Ditto about assignment.
And in this case too,
drivers/tty/serial/serial_core.c:184:22: note: ‘flags’ was declared here
unsigned long page, flags;
^~~~~
In file included from ./include/linux/seqlock.h:36:0,
from ./include/linux/time.h:6,
from ./include/linux/stat.h:19,
from ./include/linux/module.h:10,
from drivers/tty/serial/serial_core.c:10:
drivers/tty/serial/serial_core.c: In function ‘uart_shutdown’:
./include/linux/spinlock.h:260:3: warning: ‘flags’ may be used uninitialized in this function -Wmaybe-uninitialized]
_raw_spin_unlock_irqrestore(lock, flags); \
^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/tty/serial/serial_core.c:269:16: note: ‘flags’ was declared here
unsigned long flags;
^~~~~
Tycho
^ permalink raw reply
* Re: [PATCH v3] uart: fix race between uart_put_char() and uart_shutdown()
From: Andy Shevchenko @ 2018-07-06 20:48 UTC (permalink / raw)
To: Tycho Andersen
Cc: Greg Kroah-Hartman, Jiri Slaby, open list:SERIAL DRIVERS,
Linux Kernel Mailing List, Serge E . Hallyn
In-Reply-To: <20180706183928.GA3583@cisco.lan>
On Fri, Jul 6, 2018 at 9:39 PM, Tycho Andersen <tycho@tycho.ws> wrote:
> On Fri, Jul 06, 2018 at 07:49:09PM +0300, Andy Shevchenko wrote:
>> On Fri, Jul 6, 2018 at 7:24 PM, Tycho Andersen <tycho@tycho.ws> wrote:
> but without the initialization I get,
>
> CC drivers/tty/serial/serial_core.o
> In file included from ./include/linux/seqlock.h:36:0,
> from ./include/linux/time.h:6,
> from ./include/linux/stat.h:19,
> from ./include/linux/module.h:10,
> from drivers/tty/serial/serial_core.c:10:
> drivers/tty/serial/serial_core.c: In function ‘uart_startup.part.20’:
> ./include/linux/spinlock.h:260:3: warning: ‘flags’ may be used uninitialized in this function -Wmaybe-uninitialized]
> _raw_spin_unlock_irqrestore(lock, flags); \
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/tty/serial/serial_core.c:184:22: note: ‘flags’ was declared here
> unsigned long page, flags;
> ^~~~~
Hmm... I didn't see such warning. How you run make?
Btw, you adding the only places with such assignments in this file.
So, I would not do in your case, until entire file would be fixed.
(But warning looks bogus, or you have some patches on top of current
vanilla / next)
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3] uart: fix race between uart_put_char() and uart_shutdown()
From: Tycho Andersen @ 2018-07-06 21:22 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Greg Kroah-Hartman, Jiri Slaby, open list:SERIAL DRIVERS,
Linux Kernel Mailing List, Serge E . Hallyn
In-Reply-To: <CAHp75VeAoL9aN4pX4G9WGPS4Oxi=G6PyF7qCCyZ1fNH=gKHYiw@mail.gmail.com>
On Fri, Jul 06, 2018 at 11:48:58PM +0300, Andy Shevchenko wrote:
> On Fri, Jul 6, 2018 at 9:39 PM, Tycho Andersen <tycho@tycho.ws> wrote:
> > On Fri, Jul 06, 2018 at 07:49:09PM +0300, Andy Shevchenko wrote:
> >> On Fri, Jul 6, 2018 at 7:24 PM, Tycho Andersen <tycho@tycho.ws> wrote:
>
> > but without the initialization I get,
> >
> > CC drivers/tty/serial/serial_core.o
> > In file included from ./include/linux/seqlock.h:36:0,
> > from ./include/linux/time.h:6,
> > from ./include/linux/stat.h:19,
> > from ./include/linux/module.h:10,
> > from drivers/tty/serial/serial_core.c:10:
> > drivers/tty/serial/serial_core.c: In function ‘uart_startup.part.20’:
> > ./include/linux/spinlock.h:260:3: warning: ‘flags’ may be used uninitialized in this function -Wmaybe-uninitialized]
> > _raw_spin_unlock_irqrestore(lock, flags); \
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> > drivers/tty/serial/serial_core.c:184:22: note: ‘flags’ was declared here
> > unsigned long page, flags;
> > ^~~~~
>
> Hmm... I didn't see such warning. How you run make?
Just with `make`, although using the specific object file works too.
Perhaps it's gcc versions?
~/packages/linux uart-fix-v4 make drivers/tty/serial/serial_core.o
CALL scripts/checksyscalls.sh
DESCEND objtool
CC drivers/tty/serial/serial_core.o
In file included from ./include/linux/seqlock.h:36:0,
from ./include/linux/time.h:6,
from ./include/linux/stat.h:19,
from ./include/linux/module.h:10,
from drivers/tty/serial/serial_core.c:10:
drivers/tty/serial/serial_core.c: In function ‘uart_startup.part.20’:
./include/linux/spinlock.h:260:3: warning: ‘flags’ may be used uninitialized in this function -Wmaybe-uninitialized]
_raw_spin_unlock_irqrestore(lock, flags); \
^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/tty/serial/serial_core.c:184:22: note: ‘flags’ was declared here
unsigned long page, flags;
^~~~~
In file included from ./include/linux/seqlock.h:36:0,
from ./include/linux/time.h:6,
from ./include/linux/stat.h:19,
from ./include/linux/module.h:10,
from drivers/tty/serial/serial_core.c:10:
drivers/tty/serial/serial_core.c: In function ‘uart_shutdown’:
./include/linux/spinlock.h:260:3: warning: ‘flags’ may be used uninitialized in this function -Wmaybe-uninitialized]
_raw_spin_unlock_irqrestore(lock, flags); \
^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/tty/serial/serial_core.c:269:16: note: ‘flags’ was declared here
unsigned long flags;
^~~~~
~/packages/linux uart-fix-v4 gcc --version
gcc (Ubuntu 7.3.0-16ubuntu3) 7.3.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> Btw, you adding the only places with such assignments in this file.
> So, I would not do in your case, until entire file would be fixed.
>
> (But warning looks bogus, or you have some patches on top of current
> vanilla / next)
I don't have any patches, but I do admit to not thinking about it very hard and
adding initializations. I'll see if I can figure out what's going on.
Thanks,
Tycho
^ permalink raw reply
* Re: [PATCH v2 3/3] serial: 8250_dw: add fractional divisor support
From: Jisheng Zhang @ 2018-07-09 6:04 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Greg Kroah-Hartman, Jiri Slaby, linux-arm-kernel, linux-kernel,
linux-serial
In-Reply-To: <66eff871bc97f24e7fbcc2494df1bb6fdd98d8da.camel@linux.intel.com>
On Fri, 6 Jul 2018 20:37:09 +0300 Andy Shevchenko wrote:
> On Thu, 2018-07-05 at 14:39 +0800, Jisheng Zhang wrote:
> > > On Wed, 2018-07-04 at 17:03 +0800, Jisheng Zhang wrote:
>
> My comments below.
>
> > > > For Synopsys DesignWare 8250 uart which version >= 4.00a, there's
> > > > a
> > > > valid divisor latch fraction register. The fractional divisor
> > > > width is
> > > > 4bits ~ 6bits.
> > >
> > > I have read 4.00a spec a bit and didn't find this limitation.
> > > The fractional divider can fit up to 32 bits.
> >
> > the limitation isn't put in the register description, but in the
> > description
> > about fractional divisor width configure parameters. Searching
> > DLF_SIZE will
> > find it.
>
> Found, thanks.
>
> > From another side, 32bits fractional div is a waste, for example,
> > let's
> > assume the fract divisor = 0.9,
> > if fractional width is 4 bits, DLF reg will be set to UP(0.9*2^4) =
> > 15, the
> > real frac divisor = 15/2^4 = 0.93;
> > if fractional width is 32 bits, DLF reg will be set to UP(0.9*2^32) =
> > 3865470567
> > the real frac divisor = 3865470567/2^32 = 0.90;
>
> So, your example shows that 32-bit gives better value. Where is
> contradiction?
The gain -- 0.03 is small, the pay is expensive ;)
>
> > > I would test this a bit later.
>
> It reads back 0 on our hardware with 3.xx version of IP.
Thanks. So the ver check could be removed.
>
> > > > + unsigned int dlf_size:3;
> > >
> > > These bit fields (besides the fact you need 5) more or less for
> > > software
> > > quirk flags. In your case I would rather keep a u32 value of DFL
> > > mask as
> > > done for msr slightly above in this structure.
> >
> > OK. When setting the frac div, we use DLF size rather than mask, so
> > could
> > I only calculate the DLF size once then use an u8 value to hold the
> > calculated DLF size rather than calculating every time?
>
> Let's see below.
>
> > > > + quot = DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4),
> > > > baud);
> > >
> > > If we have clock rate like 100MHz and 10 bits of fractional divider
> > > it
> > > would give an integer overflow.
> >
> > This depends on the fraction divider width. If it's only 4~6 bits,
> > then we are fine.
>
> True.
>
> > >
> > > 4 here is a magic. Needs to be commented / described better.
> >
> > Yes. divisor = clk/(16*baud) = BRD(I) + BRD(F)
> > "I" means integer, "F" means fractional
> >
> > 2^dlf_size*clk/(16*baud) = 2^dlf_size*(BRD(I) + BRD(F))
> >
> > clk*2^(dlf_size - 4)/baud = 2^dlf_size*((BRD(I) + BRD(F))
> >
> > the left part is "DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4),
> > baud)",
> > let's assume it equals quot.
> >
> > then, BRD(I) = quot / 2^dlf_size = quot >> dlf_size, this is where
> > "quot >> d->dlf_size" below from.
> >
> > BRD(F) = quot & dlf_mask = quot & (~0U >> (32 - dlf_size))
>
> Yes, I understand from where it comes. It's a hard coded value of PS all
> over the serial code.
>
> And better use the same idiom as in other code, i.e. * 16 or / 16
> depends on context.
>
> > > > + *frac = quot & (~0U >> (32 - d->dlf_size));
> > > > +
> > >
> > > Operating on dfl_mask is simple as
> > >
> > > u64 quot = p->uartclk * (p->dfl_mask + 1);
> >
> > Since the dlf_mask is always 2^n - 1,
> > clk * (p->dlf_mask + 1) = clk << p->dlf_size, but the later
> > is simpler
> >
> > >
> > > *frac = div64_u64(quot, baud * 16 * (p->dfl_mask + 1);
> > > return quot;
> >
> > quot = DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4), baud)
> > *frac = quot & (~0U >> (32 - d->dlf_size))
> > return quot >> d->dlf_size;
> >
> > vs.
> >
> > quot = p->uartclk * (p->dfl_mask + 1);
> > *frac = div64_u64(quot, baud * 16 * (p->dfl_mask + 1);
> > return quot;
> >
> > shift vs mul.
> >
> > If the dlf width is only 4~6 bits, the first calculation can avoid
> > 64bit div. I prefer the first calculation.
>
> OK, taking that into consideration and looking at the code snippets
> again I would to
> - keep two values
>
> // mask we get for free because it's needed in intermediate calculus
> //
> and it doesn't overflow u8 (4-6 bits by spec)
> u8 dlf_mask;
> u8 dlf_size;
>
> - implement function like below
>
> unsigned int quot;
>
> /* Consider maximum possible DLF_SIZE, i.e. 6 bits */
> quot = DIV_ROUND_CLOSEST(p->uartclk * 4, baud);
>
> *frac = (quot >> (6 - dlf_size)) & dlf_mask;
> return quot >> dlf_size;
>
> Would you agree it looks slightly less complicated?
Nice. I will follow this solution.
>
> > > > + serial_port_out(p, UART_LCR, up->lcr | UART_LCR_DLAB);
> > > > + serial_dl_write(up, quot);
>
> (1)
>
> > >
> > > At some point it would be a helper, I think. We can call
> > > serial8250_do_set_divisor() here. So, perhaps we might export it.
> >
> > serial8250_do_set_divisor will drop the frac, that's not we want ;)
>
> Can you check again? What I see is that for DW 8250 the
> _do_set_divisor() would be an equivalent to the two lines, i.e. (1).
My bad, I mixed it with get_divisor.
>
> And basically at the end it should become just those two lines when the
> rest will implement their custom set_divisor().
yes, makes sense. Will send a new version soon.
^ permalink raw reply
* [PATCH v4 0/1] Add basic SoC support for mt6765
From: Mars Cheng @ 2018-07-09 6:05 UTC (permalink / raw)
To: Matthias Brugger, Rob Herring, Marc Zyngier
Cc: CC Hwang, Loda Chou, linux-kernel, linux-mediatek, devicetree,
wsd_upstream, linux-serial, linux-arm-kernel
This patch adds basic SoC support for Mediatek's new 8-core SoC,
MT6765, which is mainly for smartphone application.
Changes in V4:
1. add gic's settings in reg properties
2. remove some patches about dt-bindings since GKH already took them
Changes in V3:
1. split dt-binding document patchs
2. fix mt6765.dtsi warnings with W=12
3. remove uncessary PPI affinity for timer
4. add gicc base for gic dt node
Changes in V2:
1. fix clk properties in uart dts node
2. fix typo in submit title
3. add simple-bus in mt6765.dtsi
4. use correct SPDX license format
Mars Cheng (1):
arm64: dts: mediatek: add mt6765 support
arch/arm64/boot/dts/mediatek/Makefile | 1 +
arch/arm64/boot/dts/mediatek/mt6765-evb.dts | 33 ++++++
arch/arm64/boot/dts/mediatek/mt6765.dtsi | 157 +++++++++++++++++++++++++++
3 files changed, 191 insertions(+)
create mode 100644 arch/arm64/boot/dts/mediatek/mt6765-evb.dts
create mode 100644 arch/arm64/boot/dts/mediatek/mt6765.dtsi
--
1.7.9.5
^ permalink raw reply
* [PATCH v4 1/1] arm64: dts: mediatek: add mt6765 support
From: Mars Cheng @ 2018-07-09 6:05 UTC (permalink / raw)
To: Matthias Brugger, Rob Herring, Marc Zyngier
Cc: CC Hwang, Loda Chou, linux-kernel, linux-mediatek, devicetree,
wsd_upstream, linux-serial, linux-arm-kernel, Mars Cheng
In-Reply-To: <1531116302-29921-1-git-send-email-mars.cheng@mediatek.com>
This adds basic chip support for MT6765 SoC.
Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
---
arch/arm64/boot/dts/mediatek/Makefile | 1 +
arch/arm64/boot/dts/mediatek/mt6765-evb.dts | 33 ++++++
arch/arm64/boot/dts/mediatek/mt6765.dtsi | 156 +++++++++++++++++++++++++++
3 files changed, 190 insertions(+)
create mode 100644 arch/arm64/boot/dts/mediatek/mt6765-evb.dts
create mode 100644 arch/arm64/boot/dts/mediatek/mt6765.dtsi
diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
index ac17f60..7506b0d 100644
--- a/arch/arm64/boot/dts/mediatek/Makefile
+++ b/arch/arm64/boot/dts/mediatek/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
dtb-$(CONFIG_ARCH_MEDIATEK) += mt2712-evb.dtb
dtb-$(CONFIG_ARCH_MEDIATEK) += mt6755-evb.dtb
+dtb-$(CONFIG_ARCH_MEDIATEK) += mt6765-evb.dtb
dtb-$(CONFIG_ARCH_MEDIATEK) += mt6795-evb.dtb
dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb
dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
diff --git a/arch/arm64/boot/dts/mediatek/mt6765-evb.dts b/arch/arm64/boot/dts/mediatek/mt6765-evb.dts
new file mode 100644
index 0000000..36dddff2
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt6765-evb.dts
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * dts file for Mediatek MT6765
+ *
+ * (C) Copyright 2018. Mediatek, Inc.
+ *
+ * Mars Cheng <mars.cheng@mediatek.com>
+ */
+
+/dts-v1/;
+#include "mt6765.dtsi"
+
+/ {
+ model = "MediaTek MT6765 EVB";
+ compatible = "mediatek,mt6765-evb", "mediatek,mt6765";
+
+ aliases {
+ serial0 = &uart0;
+ };
+
+ memory@40000000 {
+ device_type = "memory";
+ reg = <0 0x40000000 0 0x1e800000>;
+ };
+
+ chosen {
+ stdout-path = "serial0:921600n8";
+ };
+};
+
+&uart0 {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/mediatek/mt6765.dtsi b/arch/arm64/boot/dts/mediatek/mt6765.dtsi
new file mode 100644
index 0000000..cc365b1
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt6765.dtsi
@@ -0,0 +1,156 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * dts file for Mediatek MT6765
+ *
+ * (C) Copyright 2018. Mediatek, Inc.
+ *
+ * Mars Cheng <mars.cheng@mediatek.com>
+ */
+
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+/ {
+ compatible = "mediatek,mt6765";
+ interrupt-parent = <&sysirq>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ psci {
+ compatible = "arm,psci-0.2";
+ method = "smc";
+ };
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cpu@0 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ enable-method = "psci";
+ reg = <0x000>;
+ };
+
+ cpu@1 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ enable-method = "psci";
+ reg = <0x001>;
+ };
+
+ cpu@2 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ enable-method = "psci";
+ reg = <0x002>;
+ };
+
+ cpu@3 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ enable-method = "psci";
+ reg = <0x003>;
+ };
+
+ cpu@100 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ enable-method = "psci";
+ reg = <0x100>;
+ };
+
+ cpu@101 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ enable-method = "psci";
+ reg = <0x101>;
+ };
+
+ cpu@102 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ enable-method = "psci";
+ reg = <0x102>;
+ };
+
+ cpu@103 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ enable-method = "psci";
+ reg = <0x103>;
+ };
+ };
+
+ baud_clk: dummy26m {
+ compatible = "fixed-clock";
+ clock-frequency = <26000000>;
+ #clock-cells = <0>;
+ };
+
+ sys_clk: dummyclk {
+ compatible = "fixed-clock";
+ clock-frequency = <26000000>;
+ #clock-cells = <0>;
+ };
+
+ timer {
+ compatible = "arm,armv8-timer";
+ interrupt-parent = <&gic>;
+ interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
+ };
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ compatible = "simple-bus";
+ ranges;
+
+ gic: interrupt-controller@c000000 {
+ compatible = "arm,gic-v3";
+ #interrupt-cells = <3>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+ interrupt-parent = <&gic>;
+ interrupt-controller;
+ reg = <0 0x0c000000 0 0x40000>, /* GICD */
+ <0 0x0c100000 0 0x200000>, /* GICR */
+ <0 0x0c400000 0 0x2000>, /* GICC */
+ <0 0x0c410000 0 0x2000>, /* GICH */
+ <0 0x0c420000 0 0x20000>; /* GICV */
+ interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
+ sysirq: interrupt-controller@10200a80 {
+ compatible = "mediatek,mt6765-sysirq",
+ "mediatek,mt6577-sysirq";
+ interrupt-controller;
+ #interrupt-cells = <3>;
+ interrupt-parent = <&gic>;
+ reg = <0 0x10200a80 0 0x50>;
+ };
+
+ uart0: serial@11002000 {
+ compatible = "mediatek,mt6765-uart",
+ "mediatek,mt6577-uart";
+ reg = <0 0x11002000 0 0x400>;
+ interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&baud_clk>, <&sys_clk>;
+ clock-names = "baud", "bus";
+ status = "disabled";
+ };
+
+ uart1: serial@11003000 {
+ compatible = "mediatek,mt6765-uart",
+ "mediatek,mt6577-uart";
+ reg = <0 0x11003000 0 0x400>;
+ interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&baud_clk>, <&sys_clk>;
+ clock-names = "baud", "bus";
+ status = "disabled";
+ };
+ }; /* end of soc */
+};
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v2 3/3] serial: 8250_dw: add fractional divisor support
From: Andy Shevchenko @ 2018-07-09 6:15 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby,
linux-arm Mailing List, Linux Kernel Mailing List,
open list:SERIAL DRIVERS
In-Reply-To: <20180709140435.3996588b@xhacker.debian>
On Mon, Jul 9, 2018 at 9:04 AM, Jisheng Zhang
<Jisheng.Zhang@synaptics.com> wrote:
> On Fri, 6 Jul 2018 20:37:09 +0300 Andy Shevchenko wrote:
> yes, makes sense. Will send a new version soon.
Please, be sure you base it on top of tty-next and there is no
warnings added (makes sense to run make W=1 as well),
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v4 1/1] arm64: dts: mediatek: add mt6765 support
From: Marc Zyngier @ 2018-07-09 7:27 UTC (permalink / raw)
To: Mars Cheng
Cc: Matthias Brugger, Rob Herring, CC Hwang, Loda Chou, linux-kernel,
linux-mediatek, devicetree, wsd_upstream, linux-serial,
linux-arm-kernel
In-Reply-To: <1531116302-29921-2-git-send-email-mars.cheng@mediatek.com>
On Mon, 9 Jul 2018 14:05:02 +0800
Mars Cheng <mars.cheng@mediatek.com> wrote:
> This adds basic chip support for MT6765 SoC.
>
> Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
* [PATCH v3 0/3] serial: 8250_dw: add fractional divisor support
From: Jisheng Zhang @ 2018-07-09 8:18 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-arm-kernel, linux-kernel, linux-serial
For Synopsys DesignWare 8250 uart which version >= 4.00a, there's a
valid divisor latch fraction register. The fractional divisor width is
4bits ~ 6bits.
patch1 introduces necessary hooks to 8250 core.
patch2 exports serial8250_do_set_divisor()
patch3 implements the fractional divisor support for Synopsys DW 8250.
Since v2:
- rebase to tty-next branch, since I need one patch from Andy which
is in tty-next
- drop the patch "serial: 8250: let serial8250_get_divisor() get
uart_port * as param" since it's in tty-next now.
- add a new patch to export serial8250_do_set_divisor(), and reuse it
to complete dw8250_set_divisor().
- remove DW 8250 version check, since the DLF register always exists
and if fractional divisor isn't supported, the register read as 0
- add comments to explain how dw8250_get_divisor() get quot and frac.
- the frac calcuation is simplified with well implemented GENMASK
- Add Andy's Reviewed-by tag to patch1.
Since v1:
- add an extra patch to let serial8250_get_divisor() get uart_port *
as param
- take Andy's suggestions to "integrates hooks in the same way like
it's done for the rest of 8250 ones". Many thanks to Andy.
Jisheng Zhang (3):
serial: 8250: introduce get_divisor() and set_divisor() hook
serial: 8250: export serial8250_do_set_divisor()
serial: 8250_dw: add fractional divisor support
drivers/tty/serial/8250/8250_core.c | 4 +++
drivers/tty/serial/8250/8250_dw.c | 45 +++++++++++++++++++++++++++++
drivers/tty/serial/8250/8250_port.c | 30 +++++++++++++++----
include/linux/serial_8250.h | 3 ++
include/linux/serial_core.h | 7 +++++
5 files changed, 84 insertions(+), 5 deletions(-)
--
2.18.0
^ permalink raw reply
* serial: 8250: introduce get_divisor() and set_divisor() hook
From: Jisheng Zhang @ 2018-07-09 8:19 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-arm-kernel, linux-kernel, linux-serial
In-Reply-To: <20180709160942.11a74f7a@xhacker.debian>
Add these two hooks so that they can be overridden with driver specific
implementations.
Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/tty/serial/8250/8250_core.c | 4 ++++
drivers/tty/serial/8250/8250_port.c | 27 +++++++++++++++++++++++----
include/linux/serial_core.h | 7 +++++++
3 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 9342fc2ee7df..a0bb77290747 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1023,6 +1023,10 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
uart->port.get_mctrl = up->port.get_mctrl;
if (up->port.set_mctrl)
uart->port.set_mctrl = up->port.set_mctrl;
+ if (up->port.get_divisor)
+ uart->port.get_divisor = up->port.get_divisor;
+ if (up->port.set_divisor)
+ uart->port.set_divisor = up->port.set_divisor;
if (up->port.startup)
uart->port.startup = up->port.startup;
if (up->port.shutdown)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 709fe6b4265c..ce0dc17f18ee 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2498,9 +2498,9 @@ static unsigned int npcm_get_divisor(struct uart_8250_port *up,
return DIV_ROUND_CLOSEST(port->uartclk, 16 * baud + 2) - 2;
}
-static unsigned int serial8250_get_divisor(struct uart_port *port,
- unsigned int baud,
- unsigned int *frac)
+static unsigned int serial8250_do_get_divisor(struct uart_port *port,
+ unsigned int baud,
+ unsigned int *frac)
{
struct uart_8250_port *up = up_to_u8250p(port);
unsigned int quot;
@@ -2532,6 +2532,16 @@ static unsigned int serial8250_get_divisor(struct uart_port *port,
return quot;
}
+static unsigned int serial8250_get_divisor(struct uart_port *port,
+ unsigned int baud,
+ unsigned int *frac)
+{
+ if (port->get_divisor)
+ return port->get_divisor(port, baud, frac);
+
+ return serial8250_do_get_divisor(port, baud, frac);
+}
+
static unsigned char serial8250_compute_lcr(struct uart_8250_port *up,
tcflag_t c_cflag)
{
@@ -2570,7 +2580,7 @@ static unsigned char serial8250_compute_lcr(struct uart_8250_port *up,
return cval;
}
-static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
+static void serial8250_do_set_divisor(struct uart_port *port, unsigned int baud,
unsigned int quot, unsigned int quot_frac)
{
struct uart_8250_port *up = up_to_u8250p(port);
@@ -2603,6 +2613,15 @@ static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
}
}
+static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
+ unsigned int quot, unsigned int quot_frac)
+{
+ if (port->set_divisor)
+ port->set_divisor(port, baud, quot, quot_frac);
+ else
+ serial8250_do_set_divisor(port, baud, quot, quot_frac);
+}
+
static unsigned int serial8250_get_baud_rate(struct uart_port *port,
struct ktermios *termios,
struct ktermios *old)
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 06ea4eeb09ab..406edae44ca3 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -127,6 +127,13 @@ struct uart_port {
struct ktermios *);
unsigned int (*get_mctrl)(struct uart_port *);
void (*set_mctrl)(struct uart_port *, unsigned int);
+ unsigned int (*get_divisor)(struct uart_port *,
+ unsigned int baud,
+ unsigned int *frac);
+ void (*set_divisor)(struct uart_port *,
+ unsigned int baud,
+ unsigned int quot,
+ unsigned int quot_frac);
int (*startup)(struct uart_port *port);
void (*shutdown)(struct uart_port *port);
void (*throttle)(struct uart_port *port);
--
2.18.0
^ permalink raw reply related
* [PATCH v3 1/3] serial: 8250: introduce get_divisor() and set_divisor() hook
From: Jisheng Zhang @ 2018-07-09 8:20 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-arm-kernel, linux-kernel, linux-serial
In-Reply-To: <20180709160942.11a74f7a@xhacker.debian>
Add these two hooks so that they can be overridden with driver specific
implementations.
Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/tty/serial/8250/8250_core.c | 4 ++++
drivers/tty/serial/8250/8250_port.c | 27 +++++++++++++++++++++++----
include/linux/serial_core.h | 7 +++++++
3 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 9342fc2ee7df..a0bb77290747 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1023,6 +1023,10 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
uart->port.get_mctrl = up->port.get_mctrl;
if (up->port.set_mctrl)
uart->port.set_mctrl = up->port.set_mctrl;
+ if (up->port.get_divisor)
+ uart->port.get_divisor = up->port.get_divisor;
+ if (up->port.set_divisor)
+ uart->port.set_divisor = up->port.set_divisor;
if (up->port.startup)
uart->port.startup = up->port.startup;
if (up->port.shutdown)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 709fe6b4265c..ce0dc17f18ee 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2498,9 +2498,9 @@ static unsigned int npcm_get_divisor(struct uart_8250_port *up,
return DIV_ROUND_CLOSEST(port->uartclk, 16 * baud + 2) - 2;
}
-static unsigned int serial8250_get_divisor(struct uart_port *port,
- unsigned int baud,
- unsigned int *frac)
+static unsigned int serial8250_do_get_divisor(struct uart_port *port,
+ unsigned int baud,
+ unsigned int *frac)
{
struct uart_8250_port *up = up_to_u8250p(port);
unsigned int quot;
@@ -2532,6 +2532,16 @@ static unsigned int serial8250_get_divisor(struct uart_port *port,
return quot;
}
+static unsigned int serial8250_get_divisor(struct uart_port *port,
+ unsigned int baud,
+ unsigned int *frac)
+{
+ if (port->get_divisor)
+ return port->get_divisor(port, baud, frac);
+
+ return serial8250_do_get_divisor(port, baud, frac);
+}
+
static unsigned char serial8250_compute_lcr(struct uart_8250_port *up,
tcflag_t c_cflag)
{
@@ -2570,7 +2580,7 @@ static unsigned char serial8250_compute_lcr(struct uart_8250_port *up,
return cval;
}
-static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
+static void serial8250_do_set_divisor(struct uart_port *port, unsigned int baud,
unsigned int quot, unsigned int quot_frac)
{
struct uart_8250_port *up = up_to_u8250p(port);
@@ -2603,6 +2613,15 @@ static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
}
}
+static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
+ unsigned int quot, unsigned int quot_frac)
+{
+ if (port->set_divisor)
+ port->set_divisor(port, baud, quot, quot_frac);
+ else
+ serial8250_do_set_divisor(port, baud, quot, quot_frac);
+}
+
static unsigned int serial8250_get_baud_rate(struct uart_port *port,
struct ktermios *termios,
struct ktermios *old)
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 06ea4eeb09ab..406edae44ca3 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -127,6 +127,13 @@ struct uart_port {
struct ktermios *);
unsigned int (*get_mctrl)(struct uart_port *);
void (*set_mctrl)(struct uart_port *, unsigned int);
+ unsigned int (*get_divisor)(struct uart_port *,
+ unsigned int baud,
+ unsigned int *frac);
+ void (*set_divisor)(struct uart_port *,
+ unsigned int baud,
+ unsigned int quot,
+ unsigned int quot_frac);
int (*startup)(struct uart_port *port);
void (*shutdown)(struct uart_port *port);
void (*throttle)(struct uart_port *port);
--
2.18.0
^ permalink raw reply related
* Re: serial: 8250: introduce get_divisor() and set_divisor() hook
From: Jisheng Zhang @ 2018-07-09 8:21 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-arm-kernel, linux-kernel, linux-serial
In-Reply-To: <20180709161922.3c43f599@xhacker.debian>
oops, sorry, please ignore this patch.
On Mon, 9 Jul 2018 16:19:22 +0800
Jisheng Zhang <Jisheng.Zhang@synaptics.com> wrote:
> Add these two hooks so that they can be overridden with driver specific
> implementations.
>
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/tty/serial/8250/8250_core.c | 4 ++++
> drivers/tty/serial/8250/8250_port.c | 27 +++++++++++++++++++++++----
> include/linux/serial_core.h | 7 +++++++
> 3 files changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index 9342fc2ee7df..a0bb77290747 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -1023,6 +1023,10 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
> uart->port.get_mctrl = up->port.get_mctrl;
> if (up->port.set_mctrl)
> uart->port.set_mctrl = up->port.set_mctrl;
> + if (up->port.get_divisor)
> + uart->port.get_divisor = up->port.get_divisor;
> + if (up->port.set_divisor)
> + uart->port.set_divisor = up->port.set_divisor;
> if (up->port.startup)
> uart->port.startup = up->port.startup;
> if (up->port.shutdown)
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 709fe6b4265c..ce0dc17f18ee 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -2498,9 +2498,9 @@ static unsigned int npcm_get_divisor(struct uart_8250_port *up,
> return DIV_ROUND_CLOSEST(port->uartclk, 16 * baud + 2) - 2;
> }
>
> -static unsigned int serial8250_get_divisor(struct uart_port *port,
> - unsigned int baud,
> - unsigned int *frac)
> +static unsigned int serial8250_do_get_divisor(struct uart_port *port,
> + unsigned int baud,
> + unsigned int *frac)
> {
> struct uart_8250_port *up = up_to_u8250p(port);
> unsigned int quot;
> @@ -2532,6 +2532,16 @@ static unsigned int serial8250_get_divisor(struct uart_port *port,
> return quot;
> }
>
> +static unsigned int serial8250_get_divisor(struct uart_port *port,
> + unsigned int baud,
> + unsigned int *frac)
> +{
> + if (port->get_divisor)
> + return port->get_divisor(port, baud, frac);
> +
> + return serial8250_do_get_divisor(port, baud, frac);
> +}
> +
> static unsigned char serial8250_compute_lcr(struct uart_8250_port *up,
> tcflag_t c_cflag)
> {
> @@ -2570,7 +2580,7 @@ static unsigned char serial8250_compute_lcr(struct uart_8250_port *up,
> return cval;
> }
>
> -static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
> +static void serial8250_do_set_divisor(struct uart_port *port, unsigned int baud,
> unsigned int quot, unsigned int quot_frac)
> {
> struct uart_8250_port *up = up_to_u8250p(port);
> @@ -2603,6 +2613,15 @@ static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
> }
> }
>
> +static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
> + unsigned int quot, unsigned int quot_frac)
> +{
> + if (port->set_divisor)
> + port->set_divisor(port, baud, quot, quot_frac);
> + else
> + serial8250_do_set_divisor(port, baud, quot, quot_frac);
> +}
> +
> static unsigned int serial8250_get_baud_rate(struct uart_port *port,
> struct ktermios *termios,
> struct ktermios *old)
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 06ea4eeb09ab..406edae44ca3 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -127,6 +127,13 @@ struct uart_port {
> struct ktermios *);
> unsigned int (*get_mctrl)(struct uart_port *);
> void (*set_mctrl)(struct uart_port *, unsigned int);
> + unsigned int (*get_divisor)(struct uart_port *,
> + unsigned int baud,
> + unsigned int *frac);
> + void (*set_divisor)(struct uart_port *,
> + unsigned int baud,
> + unsigned int quot,
> + unsigned int quot_frac);
> int (*startup)(struct uart_port *port);
> void (*shutdown)(struct uart_port *port);
> void (*throttle)(struct uart_port *port);
^ permalink raw reply
* [PATCH v3 2/3] serial: 8250: export serial8250_do_set_divisor()
From: Jisheng Zhang @ 2018-07-09 8:22 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-arm-kernel, linux-kernel, linux-serial
In-Reply-To: <20180709160942.11a74f7a@xhacker.debian>
Some drivers could call serial8250_do_set_divisor() to complete its
own set_divisor routine. Export this symbol for code reusing.
Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
drivers/tty/serial/8250/8250_port.c | 5 +++--
include/linux/serial_8250.h | 3 +++
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index ce0dc17f18ee..945f8dc2d50f 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2580,8 +2580,8 @@ static unsigned char serial8250_compute_lcr(struct uart_8250_port *up,
return cval;
}
-static void serial8250_do_set_divisor(struct uart_port *port, unsigned int baud,
- unsigned int quot, unsigned int quot_frac)
+void serial8250_do_set_divisor(struct uart_port *port, unsigned int baud,
+ unsigned int quot, unsigned int quot_frac)
{
struct uart_8250_port *up = up_to_u8250p(port);
@@ -2612,6 +2612,7 @@ static void serial8250_do_set_divisor(struct uart_port *port, unsigned int baud,
serial_port_out(port, 0x2, quot_frac);
}
}
+EXPORT_SYMBOL_GPL(serial8250_do_set_divisor);
static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
unsigned int quot, unsigned int quot_frac)
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index 76b9db71e489..18e21427bce4 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -160,6 +160,9 @@ extern void serial8250_do_shutdown(struct uart_port *port);
extern void serial8250_do_pm(struct uart_port *port, unsigned int state,
unsigned int oldstate);
extern void serial8250_do_set_mctrl(struct uart_port *port, unsigned int mctrl);
+extern void serial8250_do_set_divisor(struct uart_port *port, unsigned int baud,
+ unsigned int quot,
+ unsigned int quot_frac);
extern int fsl8250_handle_irq(struct uart_port *port);
int serial8250_handle_irq(struct uart_port *port, unsigned int iir);
unsigned char serial8250_rx_chars(struct uart_8250_port *up, unsigned char lsr);
--
2.18.0
^ permalink raw reply related
* [PATCH v3 3/3] serial: 8250_dw: add fractional divisor support
From: Jisheng Zhang @ 2018-07-09 8:23 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-arm-kernel, linux-kernel, linux-serial
In-Reply-To: <20180709160942.11a74f7a@xhacker.debian>
For Synopsys DesignWare 8250 uart which version >= 4.00a, there's a
valid divisor latch fraction register. The fractional divisor width is
4bits ~ 6bits.
Now the preparation is done, it's easy to add the feature support.
This patch firstly tries to get the fractional divisor width during
probe, then setups dw specific get_divisor() and set_divisor() hook.
Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
drivers/tty/serial/8250/8250_dw.c | 45 +++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index fa8a00e8c9c6..e90c3d229f00 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -31,6 +31,7 @@
/* Offsets for the DesignWare specific registers */
#define DW_UART_USR 0x1f /* UART Status Register */
+#define DW_UART_DLF 0xc0 /* Divisor Latch Fraction Register */
#define DW_UART_CPR 0xf4 /* Component Parameter Register */
#define DW_UART_UCV 0xf8 /* UART Component Version */
@@ -55,6 +56,7 @@
struct dw8250_data {
u8 usr_reg;
+ u8 dlf_size;
int line;
int msr_mask_on;
int msr_mask_off;
@@ -366,6 +368,37 @@ static bool dw8250_idma_filter(struct dma_chan *chan, void *param)
return param == chan->device->dev->parent;
}
+/*
+ * divisor = clk / (16 * baud) = div(I) + div(F)
+ * "I" means integer, "F" means fractional
+ *
+ * 2^dlf_size * clk / (16 * baud) = 2^dlf_size * (div(I) + div(F))
+ * so, (clk << (dlf_siz - 4)) / baud = (div(I) + div(F)) << dlf_size
+ *
+ * let quot = DIV_ROUND_CLOSEST(clk << (dlf_size - 4), baud), we get
+ * div(I) = quot >> dlf_size
+ * div(F) = quot & dlf_mask, where dlf_mask = GENMASK(dlf_size - 1, 0)
+ */
+static unsigned int dw8250_get_divisor(struct uart_port *p,
+ unsigned int baud,
+ unsigned int *frac)
+{
+ unsigned int quot;
+ struct dw8250_data *d = p->private_data;
+
+ quot = DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4), baud);
+ *frac = quot & GENMASK(d->dlf_size - 1, 0);
+
+ return quot >> d->dlf_size;
+}
+
+static void dw8250_set_divisor(struct uart_port *p, unsigned int baud,
+ unsigned int quot, unsigned int quot_frac)
+{
+ dw8250_writel_ext(p, DW_UART_DLF, quot_frac);
+ serial8250_do_set_divisor(p, baud, quot, quot_frac);
+}
+
static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
{
if (p->dev->of_node) {
@@ -426,6 +459,18 @@ static void dw8250_setup_port(struct uart_port *p)
dev_dbg(p->dev, "Designware UART version %c.%c%c\n",
(reg >> 24) & 0xff, (reg >> 16) & 0xff, (reg >> 8) & 0xff);
+ dw8250_writel_ext(p, DW_UART_DLF, ~0U);
+ reg = dw8250_readl_ext(p, DW_UART_DLF);
+ dw8250_writel_ext(p, DW_UART_DLF, 0);
+
+ if (reg) {
+ struct dw8250_data *d = p->private_data;
+
+ d->dlf_size = fls(reg);
+ p->get_divisor = dw8250_get_divisor;
+ p->set_divisor = dw8250_set_divisor;
+ }
+
reg = dw8250_readl_ext(p, DW_UART_CPR);
if (!reg)
return;
--
2.18.0
^ permalink raw reply related
* Re: [PATCH v3 3/3] serial: 8250_dw: add fractional divisor support
From: Jisheng Zhang @ 2018-07-09 8:32 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-arm-kernel, linux-kernel, linux-serial
In-Reply-To: <20180709162315.7f53eb9a@xhacker.debian>
Hi Andy,
On Mon, 9 Jul 2018 16:23:15 +0800 Jisheng Zhang wrote:
> For Synopsys DesignWare 8250 uart which version >= 4.00a, there's a
> valid divisor latch fraction register. The fractional divisor width is
> 4bits ~ 6bits.
>
> Now the preparation is done, it's easy to add the feature support.
> This patch firstly tries to get the fractional divisor width during
> probe, then setups dw specific get_divisor() and set_divisor() hook.
>
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> ---
> drivers/tty/serial/8250/8250_dw.c | 45 +++++++++++++++++++++++++++++++
> 1 file changed, 45 insertions(+)
>
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index fa8a00e8c9c6..e90c3d229f00 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -31,6 +31,7 @@
>
> /* Offsets for the DesignWare specific registers */
> #define DW_UART_USR 0x1f /* UART Status Register */
> +#define DW_UART_DLF 0xc0 /* Divisor Latch Fraction Register */
> #define DW_UART_CPR 0xf4 /* Component Parameter Register */
> #define DW_UART_UCV 0xf8 /* UART Component Version */
>
> @@ -55,6 +56,7 @@
>
> struct dw8250_data {
> u8 usr_reg;
> + u8 dlf_size;
> int line;
> int msr_mask_on;
> int msr_mask_off;
> @@ -366,6 +368,37 @@ static bool dw8250_idma_filter(struct dma_chan *chan, void *param)
> return param == chan->device->dev->parent;
> }
>
> +/*
> + * divisor = clk / (16 * baud) = div(I) + div(F)
> + * "I" means integer, "F" means fractional
> + *
> + * 2^dlf_size * clk / (16 * baud) = 2^dlf_size * (div(I) + div(F))
> + * so, (clk << (dlf_siz - 4)) / baud = (div(I) + div(F)) << dlf_size
> + *
> + * let quot = DIV_ROUND_CLOSEST(clk << (dlf_size - 4), baud), we get
> + * div(I) = quot >> dlf_size
> + * div(F) = quot & dlf_mask, where dlf_mask = GENMASK(dlf_size - 1, 0)
> + */
> +static unsigned int dw8250_get_divisor(struct uart_port *p,
> + unsigned int baud,
> + unsigned int *frac)
> +{
> + unsigned int quot;
> + struct dw8250_data *d = p->private_data;
> +
> + quot = DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4), baud);
> + *frac = quot & GENMASK(d->dlf_size - 1, 0);
> +
> + return quot >> d->dlf_size;
After more consideration, I sent out this version for the following two
points:
1. the max frac divisor width is 6bits now, but we dunno whether future IP
extends it or not. This patch version can still support > 6bits width except
the overflow concern. But fixing overflow(if there is) is as simple as using\
the DIV_ROUND_CLOSEST_ULL macro.
2. this version makes use of well implemented GENMASK to get the dlf_mask,
the micro is well understood, I think. so the code is simplified as well.
3. the magic "4" is explained in the comments, I hope it could help the
code.
what do you think?
Thanks in advance,
Jisheng
> +}
> +
> +static void dw8250_set_divisor(struct uart_port *p, unsigned int baud,
> + unsigned int quot, unsigned int quot_frac)
> +{
> + dw8250_writel_ext(p, DW_UART_DLF, quot_frac);
> + serial8250_do_set_divisor(p, baud, quot, quot_frac);
> +}
> +
> static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
> {
> if (p->dev->of_node) {
> @@ -426,6 +459,18 @@ static void dw8250_setup_port(struct uart_port *p)
> dev_dbg(p->dev, "Designware UART version %c.%c%c\n",
> (reg >> 24) & 0xff, (reg >> 16) & 0xff, (reg >> 8) & 0xff);
>
> + dw8250_writel_ext(p, DW_UART_DLF, ~0U);
> + reg = dw8250_readl_ext(p, DW_UART_DLF);
> + dw8250_writel_ext(p, DW_UART_DLF, 0);
> +
> + if (reg) {
> + struct dw8250_data *d = p->private_data;
> +
> + d->dlf_size = fls(reg);
> + p->get_divisor = dw8250_get_divisor;
> + p->set_divisor = dw8250_set_divisor;
> + }
> +
> reg = dw8250_readl_ext(p, DW_UART_CPR);
> if (!reg)
> return;
^ permalink raw reply
* Re: [PATCH v4 1/1] arm64: dts: mediatek: add mt6765 support
From: Matthias Brugger @ 2018-07-09 10:20 UTC (permalink / raw)
To: Mars Cheng, Rob Herring, Marc Zyngier
Cc: CC Hwang, Loda Chou, linux-kernel, linux-mediatek, devicetree,
wsd_upstream, linux-serial, linux-arm-kernel
In-Reply-To: <1531116302-29921-2-git-send-email-mars.cheng@mediatek.com>
On 09/07/18 08:05, Mars Cheng wrote:
> This adds basic chip support for MT6765 SoC.
>
> Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
> ---
> arch/arm64/boot/dts/mediatek/Makefile | 1 +
> arch/arm64/boot/dts/mediatek/mt6765-evb.dts | 33 ++++++
> arch/arm64/boot/dts/mediatek/mt6765.dtsi | 156 +++++++++++++++++++++++++++
> 3 files changed, 190 insertions(+)
> create mode 100644 arch/arm64/boot/dts/mediatek/mt6765-evb.dts
> create mode 100644 arch/arm64/boot/dts/mediatek/mt6765.dtsi
>
> diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
> index ac17f60..7506b0d 100644
> --- a/arch/arm64/boot/dts/mediatek/Makefile
> +++ b/arch/arm64/boot/dts/mediatek/Makefile
> @@ -1,6 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0
> dtb-$(CONFIG_ARCH_MEDIATEK) += mt2712-evb.dtb
> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6755-evb.dtb
> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt6765-evb.dtb
> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6795-evb.dtb
> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb
> dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
As you can see, we have a long list of SoCs which are poorly supported.
I'm not very keen to just add another SoC which supports booting into a ramdisk
using the serial console. Do you have a roadmap adding mainline support for this
SoC?
Regards,
Matthias
> diff --git a/arch/arm64/boot/dts/mediatek/mt6765-evb.dts b/arch/arm64/boot/dts/mediatek/mt6765-evb.dts
> new file mode 100644
> index 0000000..36dddff2
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt6765-evb.dts
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * dts file for Mediatek MT6765
> + *
> + * (C) Copyright 2018. Mediatek, Inc.
> + *
> + * Mars Cheng <mars.cheng@mediatek.com>
> + */
> +
> +/dts-v1/;
> +#include "mt6765.dtsi"
> +
> +/ {
> + model = "MediaTek MT6765 EVB";
> + compatible = "mediatek,mt6765-evb", "mediatek,mt6765";
> +
> + aliases {
> + serial0 = &uart0;
> + };
> +
> + memory@40000000 {
> + device_type = "memory";
> + reg = <0 0x40000000 0 0x1e800000>;
> + };
> +
> + chosen {
> + stdout-path = "serial0:921600n8";
> + };
> +};
> +
> +&uart0 {
> + status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/mediatek/mt6765.dtsi b/arch/arm64/boot/dts/mediatek/mt6765.dtsi
> new file mode 100644
> index 0000000..cc365b1
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt6765.dtsi
> @@ -0,0 +1,156 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * dts file for Mediatek MT6765
> + *
> + * (C) Copyright 2018. Mediatek, Inc.
> + *
> + * Mars Cheng <mars.cheng@mediatek.com>
> + */
> +
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +/ {
> + compatible = "mediatek,mt6765";
> + interrupt-parent = <&sysirq>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + psci {
> + compatible = "arm,psci-0.2";
> + method = "smc";
> + };
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cpu@0 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + enable-method = "psci";
> + reg = <0x000>;
> + };
> +
> + cpu@1 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + enable-method = "psci";
> + reg = <0x001>;
> + };
> +
> + cpu@2 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + enable-method = "psci";
> + reg = <0x002>;
> + };
> +
> + cpu@3 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + enable-method = "psci";
> + reg = <0x003>;
> + };
> +
> + cpu@100 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + enable-method = "psci";
> + reg = <0x100>;
> + };
> +
> + cpu@101 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + enable-method = "psci";
> + reg = <0x101>;
> + };
> +
> + cpu@102 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + enable-method = "psci";
> + reg = <0x102>;
> + };
> +
> + cpu@103 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + enable-method = "psci";
> + reg = <0x103>;
> + };
> + };
> +
> + baud_clk: dummy26m {
> + compatible = "fixed-clock";
> + clock-frequency = <26000000>;
> + #clock-cells = <0>;
> + };
> +
> + sys_clk: dummyclk {
> + compatible = "fixed-clock";
> + clock-frequency = <26000000>;
> + #clock-cells = <0>;
> + };
> +
> + timer {
> + compatible = "arm,armv8-timer";
> + interrupt-parent = <&gic>;
> + interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
> + };
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + compatible = "simple-bus";
> + ranges;
> +
> + gic: interrupt-controller@c000000 {
> + compatible = "arm,gic-v3";
> + #interrupt-cells = <3>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> + interrupt-parent = <&gic>;
> + interrupt-controller;
> + reg = <0 0x0c000000 0 0x40000>, /* GICD */
> + <0 0x0c100000 0 0x200000>, /* GICR */
> + <0 0x0c400000 0 0x2000>, /* GICC */
> + <0 0x0c410000 0 0x2000>, /* GICH */
> + <0 0x0c420000 0 0x20000>; /* GICV */
> + interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> + };
> +
> + sysirq: interrupt-controller@10200a80 {
> + compatible = "mediatek,mt6765-sysirq",
> + "mediatek,mt6577-sysirq";
> + interrupt-controller;
> + #interrupt-cells = <3>;
> + interrupt-parent = <&gic>;
> + reg = <0 0x10200a80 0 0x50>;
> + };
> +
> + uart0: serial@11002000 {
> + compatible = "mediatek,mt6765-uart",
> + "mediatek,mt6577-uart";
> + reg = <0 0x11002000 0 0x400>;
> + interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&baud_clk>, <&sys_clk>;
> + clock-names = "baud", "bus";
> + status = "disabled";
> + };
> +
> + uart1: serial@11003000 {
> + compatible = "mediatek,mt6765-uart",
> + "mediatek,mt6577-uart";
> + reg = <0 0x11003000 0 0x400>;
> + interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&baud_clk>, <&sys_clk>;
> + clock-names = "baud", "bus";
> + status = "disabled";
> + };
> + }; /* end of soc */
> +};
>
^ permalink raw reply
* [PATCH v5 2/7] serdev: add dev_pm_domain_attach|detach()
From: sean.wang @ 2018-07-09 15:56 UTC (permalink / raw)
To: robh+dt, mark.rutland, marcel, johan.hedberg
Cc: devicetree, linux-bluetooth, linux-arm-kernel, linux-mediatek,
linux-kernel, Sean Wang, Rob Herring, Ulf Hansson,
Greg Kroah-Hartman, Jiri Slaby, linux-serial
In-Reply-To: <cover.1531150733.git.sean.wang@mediatek.com>
From: Sean Wang <sean.wang@mediatek.com>
In order to open up the required power gate before any operation can be
effectively performed over the serial bus between CPU and serdev, it's
clearly essential to add common attach functions for PM domains to serdev
at the probe phase.
Similarly, the relevant dettach function for the PM domains should be
properly and reversely added at the remove phase.
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: linux-serial@vger.kernel.org
---
drivers/tty/serdev/core.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index bd47c46..9db93f5 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -13,6 +13,7 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_device.h>
+#include <linux/pm_domain.h>
#include <linux/pm_runtime.h>
#include <linux/serdev.h>
#include <linux/slab.h>
@@ -350,8 +351,17 @@ EXPORT_SYMBOL_GPL(serdev_device_set_tiocm);
static int serdev_drv_probe(struct device *dev)
{
const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
+ int ret;
- return sdrv->probe(to_serdev_device(dev));
+ ret = dev_pm_domain_attach(dev, true);
+ if (ret)
+ return ret;
+
+ ret = sdrv->probe(to_serdev_device(dev));
+ if (ret)
+ dev_pm_domain_detach(dev, true);
+
+ return ret;
}
static int serdev_drv_remove(struct device *dev)
@@ -359,6 +369,9 @@ static int serdev_drv_remove(struct device *dev)
const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
if (sdrv->remove)
sdrv->remove(to_serdev_device(dev));
+
+ dev_pm_domain_detach(dev, true);
+
return 0;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v4 1/1] arm64: dts: mediatek: add mt6765 support
From: Marc Zyngier @ 2018-07-09 16:43 UTC (permalink / raw)
To: Matthias Brugger, Mars Cheng, Rob Herring
Cc: CC Hwang, Loda Chou, linux-kernel, linux-mediatek, devicetree,
wsd_upstream, linux-serial, linux-arm-kernel
In-Reply-To: <7fb13f45-4e33-6efc-467b-c7256a40cbc9@gmail.com>
On 09/07/18 11:20, Matthias Brugger wrote:
>
>
> On 09/07/18 08:05, Mars Cheng wrote:
>> This adds basic chip support for MT6765 SoC.
>>
>> Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
>> ---
>> arch/arm64/boot/dts/mediatek/Makefile | 1 +
>> arch/arm64/boot/dts/mediatek/mt6765-evb.dts | 33 ++++++
>> arch/arm64/boot/dts/mediatek/mt6765.dtsi | 156 +++++++++++++++++++++++++++
>> 3 files changed, 190 insertions(+)
>> create mode 100644 arch/arm64/boot/dts/mediatek/mt6765-evb.dts
>> create mode 100644 arch/arm64/boot/dts/mediatek/mt6765.dtsi
>>
>> diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
>> index ac17f60..7506b0d 100644
>> --- a/arch/arm64/boot/dts/mediatek/Makefile
>> +++ b/arch/arm64/boot/dts/mediatek/Makefile
>> @@ -1,6 +1,7 @@
>> # SPDX-License-Identifier: GPL-2.0
>> dtb-$(CONFIG_ARCH_MEDIATEK) += mt2712-evb.dtb
>> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6755-evb.dtb
>> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt6765-evb.dtb
>> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6795-evb.dtb
>> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb
>> dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
>
> As you can see, we have a long list of SoCs which are poorly supported.
> I'm not very keen to just add another SoC which supports booting into a ramdisk
> using the serial console. Do you have a roadmap adding mainline support for this
> SoC?
Yes, that's a valid concern.
mt6755 and mt6795 are in a similar state, the latter after three years.
I'm all for supporting new SoCs, but this feels looks a box-ticking
exercise ("hey, look, our SoC is supported in mainline") which doesn't
help anyone.
My Ack still stands, but I'd definitely like to see some more complete
support before this patch goes in.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* Re: [PATCH v4 1/1] arm64: dts: mediatek: add mt6765 support
From: Mars Cheng @ 2018-07-09 23:04 UTC (permalink / raw)
To: Marc Zyngier
Cc: Matthias Brugger, Rob Herring, CC Hwang, Loda Chou, linux-kernel,
linux-mediatek, devicetree, wsd_upstream, linux-serial,
linux-arm-kernel
In-Reply-To: <d439eb7d-6365-7a0a-2969-1ddecc51defc@arm.com>
Hi Matthias/Marc
On Mon, 2018-07-09 at 17:43 +0100, Marc Zyngier wrote:
> On 09/07/18 11:20, Matthias Brugger wrote:
> >
> >
> > On 09/07/18 08:05, Mars Cheng wrote:
> >> This adds basic chip support for MT6765 SoC.
> >>
> >> Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
> >> ---
> >> arch/arm64/boot/dts/mediatek/Makefile | 1 +
> >> arch/arm64/boot/dts/mediatek/mt6765-evb.dts | 33 ++++++
> >> arch/arm64/boot/dts/mediatek/mt6765.dtsi | 156 +++++++++++++++++++++++++++
> >> 3 files changed, 190 insertions(+)
> >> create mode 100644 arch/arm64/boot/dts/mediatek/mt6765-evb.dts
> >> create mode 100644 arch/arm64/boot/dts/mediatek/mt6765.dtsi
> >>
> >> diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
> >> index ac17f60..7506b0d 100644
> >> --- a/arch/arm64/boot/dts/mediatek/Makefile
> >> +++ b/arch/arm64/boot/dts/mediatek/Makefile
> >> @@ -1,6 +1,7 @@
> >> # SPDX-License-Identifier: GPL-2.0
> >> dtb-$(CONFIG_ARCH_MEDIATEK) += mt2712-evb.dtb
> >> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6755-evb.dtb
> >> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt6765-evb.dtb
> >> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6795-evb.dtb
> >> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb
> >> dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
> >
> > As you can see, we have a long list of SoCs which are poorly supported.
> > I'm not very keen to just add another SoC which supports booting into a ramdisk
> > using the serial console. Do you have a roadmap adding mainline support for this
> > SoC?
>
> Yes, that's a valid concern.
>
> mt6755 and mt6795 are in a similar state, the latter after three years.
> I'm all for supporting new SoCs, but this feels looks a box-ticking
> exercise ("hey, look, our SoC is supported in mainline") which doesn't
> help anyone.
>
> My Ack still stands, but I'd definitely like to see some more complete
> support before this patch goes in.
>
> Thanks,
>
> M.
Yes, we do arrange more resources to do upstream task for mt6765,
clk/pinctrl drivers are almost ready to submit. systimer is under
reviewing (v9).
http://lists.infradead.org/pipermail/linux-mediatek/2018-July/013989.html
other drivers including
pmic/pwrap/i2c/rtc/kpd/spi/wdt/cqdma/auxadc/pwm/cmdq/disp. We have
dedicated owners to handle them and will cowork tightly with members to
make sure things happen in the following weeks.
For previous chips, we did have no enough support after shell. It is due
to fast pace of smartphone SoC and other resource issues. We also know
that is no excuse so that we already confirmed owners and their
schedules for mt6765.
If there is any suggestion, please let us know.
Thanks.
^ permalink raw reply
* Re: [PATCH v3 3/3] serial: 8250_dw: add fractional divisor support
From: Jisheng Zhang @ 2018-07-10 2:07 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-arm-kernel, linux-kernel, linux-serial
In-Reply-To: <20180709163241.4d4ee343@xhacker.debian>
Hi Andy,
On Mon, 9 Jul 2018 16:32:41 +0800 Jisheng Zhang wrote:
> Hi Andy,
>
> On Mon, 9 Jul 2018 16:23:15 +0800 Jisheng Zhang wrote:
>
> > For Synopsys DesignWare 8250 uart which version >= 4.00a, there's a
> > valid divisor latch fraction register. The fractional divisor width is
> > 4bits ~ 6bits.
> >
> > Now the preparation is done, it's easy to add the feature support.
> > This patch firstly tries to get the fractional divisor width during
> > probe, then setups dw specific get_divisor() and set_divisor() hook.
> >
> > Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> > ---
> > drivers/tty/serial/8250/8250_dw.c | 45 +++++++++++++++++++++++++++++++
> > 1 file changed, 45 insertions(+)
> >
> > diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> > index fa8a00e8c9c6..e90c3d229f00 100644
> > --- a/drivers/tty/serial/8250/8250_dw.c
> > +++ b/drivers/tty/serial/8250/8250_dw.c
> > @@ -31,6 +31,7 @@
> >
> > /* Offsets for the DesignWare specific registers */
> > #define DW_UART_USR 0x1f /* UART Status Register */
> > +#define DW_UART_DLF 0xc0 /* Divisor Latch Fraction Register */
> > #define DW_UART_CPR 0xf4 /* Component Parameter Register */
> > #define DW_UART_UCV 0xf8 /* UART Component Version */
> >
> > @@ -55,6 +56,7 @@
> >
> > struct dw8250_data {
> > u8 usr_reg;
> > + u8 dlf_size;
> > int line;
> > int msr_mask_on;
> > int msr_mask_off;
> > @@ -366,6 +368,37 @@ static bool dw8250_idma_filter(struct dma_chan *chan, void *param)
> > return param == chan->device->dev->parent;
> > }
> >
> > +/*
> > + * divisor = clk / (16 * baud) = div(I) + div(F)
> > + * "I" means integer, "F" means fractional
> > + *
> > + * 2^dlf_size * clk / (16 * baud) = 2^dlf_size * (div(I) + div(F))
> > + * so, (clk << (dlf_siz - 4)) / baud = (div(I) + div(F)) << dlf_size
> > + *
> > + * let quot = DIV_ROUND_CLOSEST(clk << (dlf_size - 4), baud), we get
> > + * div(I) = quot >> dlf_size
> > + * div(F) = quot & dlf_mask, where dlf_mask = GENMASK(dlf_size - 1, 0)
> > + */
> > +static unsigned int dw8250_get_divisor(struct uart_port *p,
> > + unsigned int baud,
> > + unsigned int *frac)
> > +{
> > + unsigned int quot;
> > + struct dw8250_data *d = p->private_data;
> > +
> > + quot = DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4), baud);
> > + *frac = quot & GENMASK(d->dlf_size - 1, 0);
> > +
> > + return quot >> d->dlf_size;
>
> After more consideration, I sent out this version for the following two
> points:
>
> 1. the max frac divisor width is 6bits now, but we dunno whether future IP
> extends it or not. This patch version can still support > 6bits width except
> the overflow concern. But fixing overflow(if there is) is as simple as using\
> the DIV_ROUND_CLOSEST_ULL macro.
>
> 2. this version makes use of well implemented GENMASK to get the dlf_mask,
> the micro is well understood, I think. so the code is simplified as well.
>
> 3. the magic "4" is explained in the comments, I hope it could help the
> code.
I agree with your "making the code simple" concern. So I have a consideration
again. I think I made the code complex unnecessarily, the simplest get_divisor code
could be:
get_divisor()
{
unsigned int quot, rem;
quot = clk / (16 * baud);
rem = clk % (16 *baud);
*frac = DIV_ROUND_CLOSEST(rem << dlf_size, 16*baud);
return quot;
}
Compared with previous version, this one adds one more div instruction,
but remove several "shift, and" instructions, the performance isn't that bad.
From another side, even this version gets a trivial performance regression,
the get_divisor() doesn't sit on the hot code path. Making the code simpler is
more important.
I will send a new version.
Thanks
^ permalink raw reply
* [PATCH v4 0/3] serial: 8250_dw: add fractional divisor support
From: Jisheng Zhang @ 2018-07-10 3:09 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-kernel, linux-arm-kernel, linux-serial
For Synopsys DesignWare 8250 uart which version >= 4.00a, there's a
valid divisor latch fraction register. The fractional divisor width is
4bits ~ 6bits.
patch1 introduces necessary hooks to 8250 core.
patch2 exports serial8250_do_set_divisor()
patch3 implements the fractional divisor support for Synopsys DW 8250.
Since v3:
- simplify the dw8250_get_divisor() implementation again.
Since v2:
- rebase to tty-next branch, since I need one patch from Andy which
is in tty-next
- drop the patch "serial: 8250: let serial8250_get_divisor() get
uart_port * as param" since it's in tty-next now.
- add a new patch to export serial8250_do_set_divisor(), and reuse it
to complete dw8250_set_divisor().
- remove DW 8250 version check, since the DLF register always exists
and if fractional divisor isn't supported, the register read as 0
- add comments to explain how dw8250_get_divisor() get quot and frac.
- the frac calcuation is simplified with well implemented GENMASK
- Add Andy's Reviewed-by tag to patch1.
Since v1:
- add an extra patch to let serial8250_get_divisor() get uart_port *
as param
- take Andy's suggestions to "integrates hooks in the same way like
it's done for the rest of 8250 ones". Many thanks to Andy.
Jisheng Zhang (3):
serial: 8250: introduce get_divisor() and set_divisor() hook
serial: 8250: export serial8250_do_set_divisor()
serial: 8250_dw: add fractional divisor support
drivers/tty/serial/8250/8250_core.c | 4 +++
drivers/tty/serial/8250/8250_dw.c | 45 +++++++++++++++++++++++++++++
drivers/tty/serial/8250/8250_port.c | 30 +++++++++++++++----
include/linux/serial_8250.h | 3 ++
include/linux/serial_core.h | 7 +++++
5 files changed, 84 insertions(+), 5 deletions(-)
--
2.18.0
^ permalink raw reply
* [PATCH v4 1/3] serial: 8250: introduce get_divisor() and set_divisor() hook
From: Jisheng Zhang @ 2018-07-10 3:12 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-arm-kernel, linux-kernel, linux-serial
In-Reply-To: <20180710110942.5b0a016e@xhacker.debian>
Add these two hooks so that they can be overridden with driver specific
implementations.
Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/tty/serial/8250/8250_core.c | 4 ++++
drivers/tty/serial/8250/8250_port.c | 27 +++++++++++++++++++++++----
include/linux/serial_core.h | 7 +++++++
3 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 9342fc2ee7df..a0bb77290747 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1023,6 +1023,10 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
uart->port.get_mctrl = up->port.get_mctrl;
if (up->port.set_mctrl)
uart->port.set_mctrl = up->port.set_mctrl;
+ if (up->port.get_divisor)
+ uart->port.get_divisor = up->port.get_divisor;
+ if (up->port.set_divisor)
+ uart->port.set_divisor = up->port.set_divisor;
if (up->port.startup)
uart->port.startup = up->port.startup;
if (up->port.shutdown)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 709fe6b4265c..ce0dc17f18ee 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2498,9 +2498,9 @@ static unsigned int npcm_get_divisor(struct uart_8250_port *up,
return DIV_ROUND_CLOSEST(port->uartclk, 16 * baud + 2) - 2;
}
-static unsigned int serial8250_get_divisor(struct uart_port *port,
- unsigned int baud,
- unsigned int *frac)
+static unsigned int serial8250_do_get_divisor(struct uart_port *port,
+ unsigned int baud,
+ unsigned int *frac)
{
struct uart_8250_port *up = up_to_u8250p(port);
unsigned int quot;
@@ -2532,6 +2532,16 @@ static unsigned int serial8250_get_divisor(struct uart_port *port,
return quot;
}
+static unsigned int serial8250_get_divisor(struct uart_port *port,
+ unsigned int baud,
+ unsigned int *frac)
+{
+ if (port->get_divisor)
+ return port->get_divisor(port, baud, frac);
+
+ return serial8250_do_get_divisor(port, baud, frac);
+}
+
static unsigned char serial8250_compute_lcr(struct uart_8250_port *up,
tcflag_t c_cflag)
{
@@ -2570,7 +2580,7 @@ static unsigned char serial8250_compute_lcr(struct uart_8250_port *up,
return cval;
}
-static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
+static void serial8250_do_set_divisor(struct uart_port *port, unsigned int baud,
unsigned int quot, unsigned int quot_frac)
{
struct uart_8250_port *up = up_to_u8250p(port);
@@ -2603,6 +2613,15 @@ static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
}
}
+static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
+ unsigned int quot, unsigned int quot_frac)
+{
+ if (port->set_divisor)
+ port->set_divisor(port, baud, quot, quot_frac);
+ else
+ serial8250_do_set_divisor(port, baud, quot, quot_frac);
+}
+
static unsigned int serial8250_get_baud_rate(struct uart_port *port,
struct ktermios *termios,
struct ktermios *old)
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 06ea4eeb09ab..406edae44ca3 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -127,6 +127,13 @@ struct uart_port {
struct ktermios *);
unsigned int (*get_mctrl)(struct uart_port *);
void (*set_mctrl)(struct uart_port *, unsigned int);
+ unsigned int (*get_divisor)(struct uart_port *,
+ unsigned int baud,
+ unsigned int *frac);
+ void (*set_divisor)(struct uart_port *,
+ unsigned int baud,
+ unsigned int quot,
+ unsigned int quot_frac);
int (*startup)(struct uart_port *port);
void (*shutdown)(struct uart_port *port);
void (*throttle)(struct uart_port *port);
--
2.18.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox