* Re: [PATCH v5 16/17] tty: serdev-ttyport: return actual baudrate from ttyport_set_baudrate
From: Stefan Wahren @ 2017-05-17 11:58 UTC (permalink / raw)
To: Rob Herring, David S. Miller, Greg Kroah-Hartman
Cc: Mark Rutland, Jiri Slaby, linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
Lino Sanfilippo, Jakub Kicinski
In-Reply-To: <1494406408-31760-17-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org>
Hi Greg,
Am 10.05.2017 um 10:53 schrieb Stefan Wahren:
> Instead of returning the requested baudrate, we better return the
> actual one because it isn't always the same.
>
> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
> drivers/tty/serdev/serdev-ttyport.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
> index 487c88f..2cfdf34 100644
> --- a/drivers/tty/serdev/serdev-ttyport.c
> +++ b/drivers/tty/serdev/serdev-ttyport.c
> @@ -151,7 +151,7 @@ static unsigned int ttyport_set_baudrate(struct serdev_controller *ctrl, unsigne
>
> /* tty_set_termios() return not checked as it is always 0 */
> tty_set_termios(tty, &ktermios);
> - return speed;
> + return ktermios.c_ospeed;
> }
>
> static void ttyport_set_flow_control(struct serdev_controller *ctrl, bool enable)
there is no dependency to this series. I would be happy if this could be
applied, so i don't have to carry it for the next version.
Thanks
Stefan
--
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] serdev: Restore serdev_device_write_buf for atomic context
From: Stefan Wahren @ 2017-05-17 11:38 UTC (permalink / raw)
To: Johan Hovold, Rob Herring, Greg Kroah-Hartman
Cc: Jiri Slaby, Sebastian Reichel, Guenter Roeck, Andy Shevchenko,
Andrey Smirnov, linux-serial@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20170508151858.GB30445@localhost>
Hi Greg,
Am 08.05.2017 um 17:18 schrieb Johan Hovold:
> On Thu, May 04, 2017 at 03:32:53PM -0500, Rob Herring wrote:
>> On Thu, May 4, 2017 at 11:22 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote:
>>> Am 02.05.2017 um 15:18 schrieb Johan Hovold:
>>>> 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.
>>> So it's unwanted to have 2 write functions (non-atomic, atomic)?
>> No, it's unwanted to have more than we need.
>>
>> Looking closer, we'd also have to ensure the wait for completion is
>> not called also. So probably better to just leave it as you have done
>> it.
> Indeed. Sorry if my reply above was unclear on that point (i.e. that
> Stefan's patch is still needed regardless of whether we keep the mutex
> or not).
>
> Thanks,
> Johan
are you okay with this patch and can you please apply it?
Stefan
^ permalink raw reply
* Re: [PATCH 0/4] serdev: fix broken lifetime assumptions
From: Johan Hovold @ 2017-05-17 10:39 UTC (permalink / raw)
To: Rob Herring, Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-kernel, Johan Hovold
In-Reply-To: <20170411170731.4085-1-johan@kernel.org>
On Tue, Apr 11, 2017 at 07:07:27PM +0200, Johan Hovold wrote:
> This series fixes a number of issues with the new serdev code, which was
> based on incorrect tty-port lifetime assumptions.
>
> The first patch disables serdev support by reverting the patch which
> hooked into the tty layer in a broken way that leads to crashes and
> leaks when deregistering devices. This one should probably go into 4.11.
>
> The second patch fixes a specific bug in the tty-port client
> registration code, while the third patch adds a new interface for
> registering serdev devices. The final patch ultimately enables serdev
> again for the serial drivers.
>
> More details can be found in the individual commit messages.
>
> Johan
>
>
> Johan Hovold (4):
> Revert "tty_port: register tty ports with serdev bus"
> serdev: fix tty-port client deregistration
> tty/serdev: add serdev registration interface
> serial: enable serdev support
Rob, did you have any further comments to this series?
Greg, I think these should go into 4.12-rc2, and the first patch should
probably also have a stable tag now as serdev resource-management is
clearly broken in 4.11.
Johan
^ permalink raw reply
* Re: [PATCH] imx-serial: RX DMA startup latency
From: Sascha Hauer @ 2017-05-17 10:04 UTC (permalink / raw)
To: Peter Senna Tschudin
Cc: Greg Kroah-Hartman, Jiri Slaby, open list:SERIAL DRIVERS,
open list
In-Reply-To: <20170514123515.15794-1-peter.senna@collabora.com>
On Sun, May 14, 2017 at 02:35:15PM +0200, Peter Senna Tschudin wrote:
> 18a4208 introduced a change to reduce the RX DMA latency on the first reception
> when the serial port was opened for reading. However it was claiming a hardirq
> unsafe lock after a hardirq safe lock which is not allowed and causes lockdep
> to complain verbosely.
>
> This patch changes the code to always start RX DMA earlier, instead of
> relying on the flags used to open the serial port removing the code that
> was looking for the serial file flags.
>
> CC: Sascha Hauer <s.hauer@pengutronix.de>
> Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
> ---
> Tested on iMX53.
Tested-by: Sascha Hauer <s.hauer@pengutronix.de>
(on an i.MX6)
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support
From: Andy Shevchenko @ 2017-05-17 9:55 UTC (permalink / raw)
To: Dong Aisheng
Cc: Nikita Yushchenko, Dong Aisheng, linux-serial@vger.kernel.org,
fugang.duan, Greg Kroah-Hartman, yangbo.lu,
linux-kernel@vger.kernel.org, Stefan Agner, Mingkai.Hu,
Jiri Slaby, linux-arm Mailing List
In-Reply-To: <20170517054356.GE9913@b29396-OptiPlex-7040>
On Wed, May 17, 2017 at 8:43 AM, Dong Aisheng <dongas86@gmail.com> wrote:
> On Wed, May 17, 2017 at 08:37:41AM +0300, Nikita Yushchenko wrote:
>>
>>
>> 17.05.2017 06:39, Dong Aisheng wrote:
>> > On Tue, May 16, 2017 at 02:15:08PM +0300, Nikita Yushchenko wrote:
>> >>> static u32 lpuart32_read(void __iomem *addr)
>> >>> {
>> >>> - return ioread32be(addr);
>> >>> + return lpuart_is_be ? ioread32be(addr) : readl(addr);
>> >>> }
>> >>>
>> >>> static void lpuart32_write(u32 val, void __iomem *addr)
>> >>> {
>> >>> - iowrite32be(val, addr);
>> >>> + if (lpuart_is_be)
>> >>> + iowrite32be(val, addr);
>> >>> + else
>> >>> + writel(val, addr);
>> >>> }
>> >>
>> >> What if this is ever executed on big endian system?
>> >>
>> >
>> > Sorry, not catching the point...
>> >
>> > What issues will meet?
>>
>> Isn't writel() in host endian?
>
> On big endian systems, it is supposed to run iowrite32be.
It looks like you substituting *bus* side with CPU *side* of communication.
If you are talking about CPU side
__raw_readl() / __raw_writel() will do the trick.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support
From: Andy Shevchenko @ 2017-05-17 9:53 UTC (permalink / raw)
To: Nikita Yushchenko
Cc: Dong Aisheng, linux-serial@vger.kernel.org, fugang.duan, dongas86,
Greg Kroah-Hartman, yangbo.lu, linux-kernel@vger.kernel.org,
Stefan Agner, Mingkai.Hu, Jiri Slaby, linux-arm Mailing List
In-Reply-To: <fb9fe0cd-5ca7-74e6-33dc-7638e205b2e3@cogentembedded.com>
On Tue, May 16, 2017 at 2:15 PM, Nikita Yushchenko
<nikita.yoush@cogentembedded.com> wrote:
>> static u32 lpuart32_read(void __iomem *addr)
>> {
>> - return ioread32be(addr);
>> + return lpuart_is_be ? ioread32be(addr) : readl(addr);
>> }
>>
>> static void lpuart32_write(u32 val, void __iomem *addr)
>> {
>> - iowrite32be(val, addr);
>> + if (lpuart_is_be)
>> + iowrite32be(val, addr);
>> + else
>> + writel(val, addr);
>> }
>
> What if this is ever executed on big endian system?
Above is apparently about bus side of communication and it looks like
it needs to be CPU side. Is this what you are trying to discuss?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support
From: Nikita Yushchenko @ 2017-05-17 8:04 UTC (permalink / raw)
To: Dong Aisheng
Cc: A.S. Dong, linux-serial@vger.kernel.org, Andy Duan,
gregkh@linuxfoundation.org, Y.B. Lu, linux-kernel@vger.kernel.org,
stefan@agner.ch, Mingkai Hu, jslaby@suse.com,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20170517070019.GG9913@b29396-OptiPlex-7040>
Hi
My view of your statement is:
- you currently assume only a few cases for this driver - builtin UART
in vf610, ls1012a and imx7,
- in each of these cases, all lpuart instances share same endian, thus
having that in global var works for these cases,
- having that in global var makes it possible for you to write less
lines of code
My complain is:
- in Linux, we are trying to keep drivers generic,
- in Linux, having less lines of code has never been sufficient to break
basic data structure consistency,
- having driver to keep per-device capability in global var is a clear
case of breaking consistency.
>>> That's for special case, normally we wouldn't do that.
>>
>> For me this "special case" looks like "let's break data structure
>> consistency to reuse several lines of code".
>>
>> With code snippets you show, it looks even worse: you assign same global
>> variable in several places for different uses.
>
> If you mean lpuart_is_be, it's not for different uses.
> The purpose is the same to align the correct endian but in two places.
_probe() routine called for device X alters state already in use for
device Y.
>
>> implicitly assuming that
>> it is for same device. Which can be true in your current system, but not
>> elsewhere (e.g. why not having lpuart programmed into fpga)?
>>
>
> Sorry, What issues for fpga?
Connect FPGA to IMX7 based system and program LS1012a version of lpuart
core into it. Have your console on system UART broken at time when
driver gets registered.
>
>> Alternative solution could be - have separate write path for earlycon.
>
> It looks to me having the same issue with a separate write patch
> for earlycon as we still need distinguish Little or Big endian
> for Layerscape and IMX.
>
>> At a glance, it is dozen lines of code.
>
> Would you please show some sample code?
Do not reuse lpuart32_console_putchar() in earlycon code.
Have two sets of early_setup/early_write/putchar - for BE and
defaut-endian earlycon. And in these putchar's do not use
lpuart_(read|write).
As far as I can see, fsl_lpuart.c already has two drivers in one -
there is separate set of routines for 8bit and 32bit cases.
And those routines that are common, have if blocks that separate cases.
I think these drivers will be cleaner if separated.
However that's completely different story.
^ permalink raw reply
* Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support
From: Dong Aisheng @ 2017-05-17 7:00 UTC (permalink / raw)
To: Nikita Yushchenko
Cc: A.S. Dong, linux-serial@vger.kernel.org, Andy Duan,
gregkh@linuxfoundation.org, Y.B. Lu, linux-kernel@vger.kernel.org,
stefan@agner.ch, Mingkai Hu, jslaby@suse.com,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <55bae8d2-0e23-b5e1-f304-63d9130ccb08@cogentembedded.com>
On Wed, May 17, 2017 at 09:25:51AM +0300, Nikita Yushchenko wrote:
> >> Code should be consistent.
> >>
> >
> > Yes.
> >
> >> There is no good reason to have sport->lpuart32 inside sport, but
> >> lpuart_is_be outside of it. Both these values describe properties of
> >> particular device, and thus should be in per-device structure.
> >>
> >
> > That's for special case, normally we wouldn't do that.
>
> For me this "special case" looks like "let's break data structure
> consistency to reuse several lines of code".
>
> With code snippets you show, it looks even worse: you assign same global
> variable in several places for different uses.
If you mean lpuart_is_be, it's not for different uses.
The purpose is the same to align the correct endian but in two places.
> implicitly assuming that
> it is for same device. Which can be true in your current system, but not
> elsewhere (e.g. why not having lpuart programmed into fpga)?
>
Sorry, What issues for fpga?
> Alternative solution could be - have separate write path for earlycon.
It looks to me having the same issue with a separate write patch
for earlycon as we still need distinguish Little or Big endian
for Layerscape and IMX.
> At a glance, it is dozen lines of code.
Would you please show some sample code?
Then we probably may understand better with each other.
Anyway, thanks for detailed review.
Regards
Dong Aisheng
^ permalink raw reply
* Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support
From: Nikita Yushchenko @ 2017-05-17 6:25 UTC (permalink / raw)
To: A.S. Dong, Dong Aisheng
Cc: linux-serial@vger.kernel.org, Andy Duan,
gregkh@linuxfoundation.org, Y.B. Lu, linux-kernel@vger.kernel.org,
stefan@agner.ch, Mingkai Hu, jslaby@suse.com,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <AM3PR04MB306D5C86AED120D15C4D46A80E70@AM3PR04MB306.eurprd04.prod.outlook.com>
>> Code should be consistent.
>>
>
> Yes.
>
>> There is no good reason to have sport->lpuart32 inside sport, but
>> lpuart_is_be outside of it. Both these values describe properties of
>> particular device, and thus should be in per-device structure.
>>
>
> That's for special case, normally we wouldn't do that.
For me this "special case" looks like "let's break data structure
consistency to reuse several lines of code".
With code snippets you show, it looks even worse: you assign same global
variable in several places for different uses. implicitly assuming that
it is for same device. Which can be true in your current system, but not
elsewhere (e.g. why not having lpuart programmed into fpga)?
Alternative solution could be - have separate write path for earlycon.
At a glance, it is dozen lines of code.
^ permalink raw reply
* Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support
From: Dong Aisheng @ 2017-05-17 6:09 UTC (permalink / raw)
To: Nikita Yushchenko
Cc: Dong Aisheng, linux-serial, fugang.duan, gregkh, yangbo.lu,
linux-kernel, stefan, Mingkai.Hu, jslaby, linux-arm-kernel
In-Reply-To: <f096009a-9bcd-e575-2c81-49d08f99fc44@cogentembedded.com>
On Wed, May 17, 2017 at 08:50:39AM +0300, Nikita Yushchenko wrote:
> >>>>> static u32 lpuart32_read(void __iomem *addr)
> >>>>> {
> >>>>> - return ioread32be(addr);
> >>>>> + return lpuart_is_be ? ioread32be(addr) : readl(addr);
> >>>>> }
> >>>>>
> >>>>> static void lpuart32_write(u32 val, void __iomem *addr)
> >>>>> {
> >>>>> - iowrite32be(val, addr);
> >>>>> + if (lpuart_is_be)
> >>>>> + iowrite32be(val, addr);
> >>>>> + else
> >>>>> + writel(val, addr);
> >>>>> }
> >>>>
> >>>> What if this is ever executed on big endian system?
> >>>>
> >>>
> >>> Sorry, not catching the point...
> >>>
> >>> What issues will meet?
> >>
> >> Isn't writel() in host endian?
> >
> > On big endian systems, it is supposed to run iowrite32be.
>
> Your code states, "force BE if lpuart_is_be, don't care otherwise".
> This semantics looks questionable for code reviewer.
> If driver handles endian, should't it be explicit in both cases?
> And if indeed driver means handling BE explicitly, but don't caring
> otherwise, maybe variable name should suggest that (i.e. "force_be")?
>
For lpuart32, only two cases that LS platforms is Big endian while
IMX is little endian.
It's SoC IP native property, i don't think force_be is better.
Regards
Dong Aisheng
> Although driver maintainer could think differently. I won't insist on this.
>
> Nikita
^ permalink raw reply
* RE: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support
From: A.S. Dong @ 2017-05-17 6:01 UTC (permalink / raw)
To: Nikita Yushchenko, Dong Aisheng
Cc: linux-serial@vger.kernel.org, Andy Duan,
gregkh@linuxfoundation.org, Y.B. Lu, linux-kernel@vger.kernel.org,
stefan@agner.ch, Mingkai Hu, jslaby@suse.com,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <48819894-4943-7d14-ccf8-83cfd2195c9a@cogentembedded.com>
> -----Original Message-----
> From: Nikita Yushchenko [mailto:nikita.yoush@cogentembedded.com]
> Sent: Wednesday, May 17, 2017 1:44 PM
> To: Dong Aisheng
> Cc: A.S. Dong; linux-serial@vger.kernel.org; Andy Duan;
> gregkh@linuxfoundation.org; Y.B. Lu; linux-kernel@vger.kernel.org;
> stefan@agner.ch; Mingkai Hu; jslaby@suse.com; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit
> register support
>
> >>> @@ -2000,6 +2007,7 @@ static int lpuart_probe(struct platform_device
> *pdev)
> >>> }
> >>> sport->port.line = ret;
> >>> sport->lpuart32 = sdata->is_32;
> >>> + lpuart_is_be = sdata->is_be;
> >>
> >> Setting a global variable in per-device routine is quite bad design.
> >>
> >
> > There is a reason for that we don't want to change the exist
> > lpuart32_read[write] API which is widely used in driver.
> > Making a global lpuart_is_be is the simplest way to do it.
> >
> > Any strong blocking reason?
>
> Code should be consistent.
>
Yes.
> There is no good reason to have sport->lpuart32 inside sport, but
> lpuart_is_be outside of it. Both these values describe properties of
> particular device, and thus should be in per-device structure.
>
That's for special case, normally we wouldn't do that.
> If that implies adding sport arg to lpuart32_(read|write), just do that.
There's another reason that we have to deal with earlycon which is
executed much early before driver probe.
And I need specificly align the endian data.
e.g.
static int __init lpuart32_early_console_setup(struct earlycon_device *device,
const char *opt)
{
if (!device->port.membase)
return -ENODEV;
lpuart_is_be = true;
device->con->write = lpuart32_early_write;
return 0;
}
static int __init lpuart32_imx_early_console_setup(struct earlycon_device *device,
const char *opt)
{
if (!device->port.membase)
return -ENODEV;
lpuart_is_be = false;
device->port.membase += IMX_REG_OFF;
device->con->write = lpuart32_early_write;
return 0;
}
Regards
Dong Aisheng
^ permalink raw reply
* Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support
From: Nikita Yushchenko @ 2017-05-17 5:50 UTC (permalink / raw)
To: Dong Aisheng
Cc: Dong Aisheng, linux-serial, fugang.duan, gregkh, yangbo.lu,
linux-kernel, stefan, Mingkai.Hu, jslaby, linux-arm-kernel
In-Reply-To: <20170517054356.GE9913@b29396-OptiPlex-7040>
>>>>> static u32 lpuart32_read(void __iomem *addr)
>>>>> {
>>>>> - return ioread32be(addr);
>>>>> + return lpuart_is_be ? ioread32be(addr) : readl(addr);
>>>>> }
>>>>>
>>>>> static void lpuart32_write(u32 val, void __iomem *addr)
>>>>> {
>>>>> - iowrite32be(val, addr);
>>>>> + if (lpuart_is_be)
>>>>> + iowrite32be(val, addr);
>>>>> + else
>>>>> + writel(val, addr);
>>>>> }
>>>>
>>>> What if this is ever executed on big endian system?
>>>>
>>>
>>> Sorry, not catching the point...
>>>
>>> What issues will meet?
>>
>> Isn't writel() in host endian?
>
> On big endian systems, it is supposed to run iowrite32be.
Your code states, "force BE if lpuart_is_be, don't care otherwise".
This semantics looks questionable for code reviewer.
If driver handles endian, should't it be explicit in both cases?
And if indeed driver means handling BE explicitly, but don't caring
otherwise, maybe variable name should suggest that (i.e. "force_be")?
Although driver maintainer could think differently. I won't insist on this.
Nikita
^ permalink raw reply
* Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support
From: Dong Aisheng @ 2017-05-17 5:43 UTC (permalink / raw)
To: Nikita Yushchenko
Cc: Dong Aisheng, linux-serial, fugang.duan, gregkh, yangbo.lu,
linux-kernel, stefan, Mingkai.Hu, jslaby, linux-arm-kernel
In-Reply-To: <8f4cf45e-45ab-e6e6-d95c-f3d4243697f4@cogentembedded.com>
On Wed, May 17, 2017 at 08:37:41AM +0300, Nikita Yushchenko wrote:
>
>
> 17.05.2017 06:39, Dong Aisheng wrote:
> > On Tue, May 16, 2017 at 02:15:08PM +0300, Nikita Yushchenko wrote:
> >>> static u32 lpuart32_read(void __iomem *addr)
> >>> {
> >>> - return ioread32be(addr);
> >>> + return lpuart_is_be ? ioread32be(addr) : readl(addr);
> >>> }
> >>>
> >>> static void lpuart32_write(u32 val, void __iomem *addr)
> >>> {
> >>> - iowrite32be(val, addr);
> >>> + if (lpuart_is_be)
> >>> + iowrite32be(val, addr);
> >>> + else
> >>> + writel(val, addr);
> >>> }
> >>
> >> What if this is ever executed on big endian system?
> >>
> >
> > Sorry, not catching the point...
> >
> > What issues will meet?
>
> Isn't writel() in host endian?
On big endian systems, it is supposed to run iowrite32be.
Regards
Dong Aisheng
^ permalink raw reply
* Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support
From: Nikita Yushchenko @ 2017-05-17 5:43 UTC (permalink / raw)
To: Dong Aisheng
Cc: Dong Aisheng, linux-serial, fugang.duan, gregkh, yangbo.lu,
linux-kernel, stefan, Mingkai.Hu, jslaby, linux-arm-kernel
In-Reply-To: <20170517033139.GB9913@b29396-OptiPlex-7040>
>>> @@ -2000,6 +2007,7 @@ static int lpuart_probe(struct platform_device *pdev)
>>> }
>>> sport->port.line = ret;
>>> sport->lpuart32 = sdata->is_32;
>>> + lpuart_is_be = sdata->is_be;
>>
>> Setting a global variable in per-device routine is quite bad design.
>>
>
> There is a reason for that we don't want to change the exist
> lpuart32_read[write] API which is widely used in driver.
> Making a global lpuart_is_be is the simplest way to do it.
>
> Any strong blocking reason?
Code should be consistent.
There is no good reason to have sport->lpuart32 inside sport, but
lpuart_is_be outside of it. Both these values describe properties of
particular device, and thus should be in per-device structure.
If that implies adding sport arg to lpuart32_(read|write), just do that.
^ permalink raw reply
* Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support
From: Nikita Yushchenko @ 2017-05-17 5:37 UTC (permalink / raw)
To: Dong Aisheng
Cc: Dong Aisheng, linux-serial, fugang.duan, gregkh, yangbo.lu,
linux-kernel, stefan, Mingkai.Hu, jslaby, linux-arm-kernel
In-Reply-To: <20170517033927.GC9913@b29396-OptiPlex-7040>
17.05.2017 06:39, Dong Aisheng wrote:
> On Tue, May 16, 2017 at 02:15:08PM +0300, Nikita Yushchenko wrote:
>>> static u32 lpuart32_read(void __iomem *addr)
>>> {
>>> - return ioread32be(addr);
>>> + return lpuart_is_be ? ioread32be(addr) : readl(addr);
>>> }
>>>
>>> static void lpuart32_write(u32 val, void __iomem *addr)
>>> {
>>> - iowrite32be(val, addr);
>>> + if (lpuart_is_be)
>>> + iowrite32be(val, addr);
>>> + else
>>> + writel(val, addr);
>>> }
>>
>> What if this is ever executed on big endian system?
>>
>
> Sorry, not catching the point...
>
> What issues will meet?
Isn't writel() in host endian?
^ permalink raw reply
* Re: [PATCH V2 6/6] tty: serial: lpuart: add a more accurate baud rate calculation method
From: Dong Aisheng @ 2017-05-17 3:47 UTC (permalink / raw)
To: Stefan Agner
Cc: Dong Aisheng, linux-serial, linux-kernel, linux-arm-kernel,
gregkh, jslaby, fugang.duan, Mingkai.Hu, yangbo.lu
In-Reply-To: <73a70b856adf28b34c793f03c4db43a9@agner.ch>
On Mon, May 15, 2017 at 10:06:41AM -0700, Stefan Agner wrote:
> On 2017-05-15 00:48, Dong Aisheng wrote:
> > On new LPUART versions, the oversampling ratio for the receiver can be
> > changed from 4x (00011) to 32x (11111) which could help us get a more
> > accurate baud rate divider.
> >
> > The idea is to use the best OSR (over-sampling rate) possible.
> > Note, OSR is typically hard-set to 16 in other LPUART instantiations.
> > Loop to find the best OSR value possible, one that generates minimum
> > baud diff iterate through the rest of the supported values of OSR.
> >
> > Currently only i.MX7ULP is using it.
> >
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Jiri Slaby <jslaby@suse.com>
> > Cc: Stefan Agner <stefan@agner.ch>
> > Cc: Mingkai Hu <Mingkai.Hu@nxp.com>
> > Cc: Yangbo Lu <yangbo.lu@nxp.com>
> > Acked-by: Fugang Duan <fugang.duan@nxp.com>
> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> > ---
> > drivers/tty/serial/fsl_lpuart.c | 85 ++++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 79 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> > index 107d0911..bda4b0c 100644
> > --- a/drivers/tty/serial/fsl_lpuart.c
> > +++ b/drivers/tty/serial/fsl_lpuart.c
> > @@ -140,6 +140,8 @@
> > #define UARTBAUD_SBNS 0x00002000
> > #define UARTBAUD_SBR 0x00000000
> > #define UARTBAUD_SBR_MASK 0x1fff
> > +#define UARTBAUD_OSR_MASK 0x1f
> > +#define UARTBAUD_OSR_SHIFT 24
> >
> > #define UARTSTAT_LBKDIF 0x80000000
> > #define UARTSTAT_RXEDGIF 0x40000000
> > @@ -1506,6 +1508,72 @@ lpuart_set_termios(struct uart_port *port,
> > struct ktermios *termios,
> > }
> >
> > static void
> > +lpuart32_serial_setbrg(struct lpuart_port *sport, unsigned int baudrate)
> > +{
> > + u32 sbr, osr, baud_diff, tmp_osr, tmp_sbr, tmp_diff, tmp;
> > + u32 clk = sport->port.uartclk;
> > +
> > + /*
> > + * The idea is to use the best OSR (over-sampling rate) possible.
> > + * Note, OSR is typically hard-set to 16 in other LPUART instantiations.
> > + * Loop to find the best OSR value possible, one that generates minimum
> > + * baud_diff iterate through the rest of the supported values of OSR.
> > + *
> > + * Calculation Formula:
> > + * Baud Rate = baud clock / ((OSR+1) × SBR)
> > + */
> > + baud_diff = baudrate;
> > + osr = 0;
> > + sbr = 0;
> > +
> > + for (tmp_osr = 4; tmp_osr <= 32; tmp_osr++) {
> > + /* calculate the temporary sbr value */
> > + tmp_sbr = (clk / (baudrate * tmp_osr));
> > + if (tmp_sbr == 0)
> > + tmp_sbr = 1;
> > +
> > + /*
> > + * calculate the baud rate difference based on the temporary
> > + * osr and sbr values
> > + */
> > + tmp_diff = clk / (tmp_osr * tmp_sbr) - baudrate;
> > +
> > + /* select best values between sbr and sbr+1 */
> > + tmp = clk / (tmp_osr * (tmp_sbr + 1));
> > + if (tmp_diff > (baudrate - tmp)) {
> > + tmp_diff = baudrate - tmp;
> > + tmp_sbr++;
> > + }
> > +
> > + if (tmp_diff <= baud_diff) {
> > + baud_diff = tmp_diff;
> > + osr = tmp_osr;
> > + sbr = tmp_sbr;
> > + }
> > + }
> > +
> > + /* handle buadrate outside acceptable rate */
> > + if (baud_diff > ((baudrate / 100) * 3))
> > + dev_warn(sport->port.dev,
> > + "unacceptable baud rate difference of more than 3%%\n");
> > +
> > + tmp = lpuart32_read(sport->port.membase + UARTBAUD);
> > +
> > + if ((osr > 3) && (osr < 8))
> > + tmp |= UARTBAUD_BOTHEDGE;
> > +
> > + tmp &= ~(UARTBAUD_OSR_MASK << UARTBAUD_OSR_SHIFT);
> > + tmp |= (((osr-1) & UARTBAUD_OSR_MASK) << UARTBAUD_OSR_SHIFT);
> > +
> > + tmp &= ~UARTBAUD_SBR_MASK;
> > + tmp |= sbr & UARTBAUD_SBR_MASK;
> > +
> > + tmp &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
> > +
> > + lpuart32_write(tmp, sport->port.membase + UARTBAUD);
> > +}
> > +
> > +static void
> > lpuart32_set_termios(struct uart_port *port, struct ktermios *termios,
> > struct ktermios *old)
> > {
> > @@ -1611,12 +1679,17 @@ lpuart32_set_termios(struct uart_port *port,
> > struct ktermios *termios,
> > lpuart32_write(old_ctrl & ~(UARTCTRL_TE | UARTCTRL_RE),
> > sport->port.membase + UARTCTRL);
> >
> > - sbr = sport->port.uartclk / (16 * baud);
> > - bd &= ~UARTBAUD_SBR_MASK;
> > - bd |= sbr & UARTBAUD_SBR_MASK;
> > - bd |= UARTBAUD_BOTHEDGE;
> > - bd &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
> > - lpuart32_write(bd, sport->port.membase + UARTBAUD);
> > + if (of_device_is_compatible(port->dev->of_node, "fsl,imx7ulp-lpuart")) {
>
> Shouldn't we be consequent here and also use a flag in the soc data
> instead of of_device_is_compatible...?
>
The original purpose is that this is a temporary code and supposed will
be deleted later once LS platforms confirmed the new baud setting API
works for them as well.
That's why i did not make it a property, as i stated in the cover letter.
> Btw, instead of using 3 bools, I would prefer using a single flags like
> your patchset is proposing for the GPIO driver, what do you think?
>
Yes, good suggestion.
Probably we could convert the below two.
.is_32 = true,
.is_be = true,
But reg_off seems better to be kept.
Regards
Dong Aisheng
> --
> Stefan
>
>
> > + lpuart32_serial_setbrg(sport, baud);
> > + } else {
> > + sbr = sport->port.uartclk / (16 * baud);
> > + bd &= ~UARTBAUD_SBR_MASK;
> > + bd |= sbr & UARTBAUD_SBR_MASK;
> > + bd |= UARTBAUD_BOTHEDGE;
> > + bd &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
> > + lpuart32_write(bd, sport->port.membase + UARTBAUD);
> > + }
> > +
> > lpuart32_write(modem, sport->port.membase + UARTMODIR);
> > lpuart32_write(ctrl, sport->port.membase + UARTCTRL);
> > /* restore control register */
^ permalink raw reply
* Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support
From: Dong Aisheng @ 2017-05-17 3:39 UTC (permalink / raw)
To: Nikita Yushchenko
Cc: Dong Aisheng, linux-serial, fugang.duan, gregkh, yangbo.lu,
linux-kernel, stefan, Mingkai.Hu, jslaby, linux-arm-kernel
In-Reply-To: <fb9fe0cd-5ca7-74e6-33dc-7638e205b2e3@cogentembedded.com>
On Tue, May 16, 2017 at 02:15:08PM +0300, Nikita Yushchenko wrote:
> > static u32 lpuart32_read(void __iomem *addr)
> > {
> > - return ioread32be(addr);
> > + return lpuart_is_be ? ioread32be(addr) : readl(addr);
> > }
> >
> > static void lpuart32_write(u32 val, void __iomem *addr)
> > {
> > - iowrite32be(val, addr);
> > + if (lpuart_is_be)
> > + iowrite32be(val, addr);
> > + else
> > + writel(val, addr);
> > }
>
> What if this is ever executed on big endian system?
>
Sorry, not catching the point...
What issues will meet?
Regards
Dong Aisheng
^ permalink raw reply
* Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support
From: Dong Aisheng @ 2017-05-17 3:31 UTC (permalink / raw)
To: Nikita Yushchenko
Cc: Dong Aisheng, linux-serial, fugang.duan, gregkh, yangbo.lu,
linux-kernel, stefan, Mingkai.Hu, jslaby, linux-arm-kernel
In-Reply-To: <b4a2ed13-c71f-c973-3b09-b632e083a88b@cogentembedded.com>
On Tue, May 16, 2017 at 02:08:12PM +0300, Nikita Yushchenko wrote:
> > @@ -2000,6 +2007,7 @@ static int lpuart_probe(struct platform_device *pdev)
> > }
> > sport->port.line = ret;
> > sport->lpuart32 = sdata->is_32;
> > + lpuart_is_be = sdata->is_be;
>
> Setting a global variable in per-device routine is quite bad design.
>
There is a reason for that we don't want to change the exist
lpuart32_read[write] API which is widely used in driver.
Making a global lpuart_is_be is the simplest way to do it.
Any strong blocking reason?
Regards
Dong Aisheng
^ permalink raw reply
* Re: [PATCH V2 2/6] tty: serial: lpuart: add little endian 32 bit register support
From: Dong Aisheng @ 2017-05-17 3:26 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Dong Aisheng, linux-serial@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-arm Mailing List,
Greg Kroah-Hartman, Jiri Slaby, fugang.duan, Stefan Agner,
Mingkai.Hu, yangbo.lu
In-Reply-To: <CAHp75VeGKzDfwi1zMz+u5WZWmqnJkE41n8gw+uWcce8ngXrm2w@mail.gmail.com>
On Mon, May 15, 2017 at 04:36:25PM +0300, Andy Shevchenko wrote:
> On Mon, May 15, 2017 at 10:48 AM, Dong Aisheng <aisheng.dong@nxp.com> wrote:
> > It's based on the exist lpuart32 read/write implementation.
>
> Same as per previous comment (perhaps not in other UART driver, but
> some might have been using device property for that.
>
The reason why we do that is: a) we don't want to make a churn on the
exist users. b) it's not really necessary to use dt property
when we already have a soc_data as it has no much benefit but introduce
overhead.
Regards
Dong Aisheng
>
> --
> With Best Regards,
> Andy Shevchenko
^ permalink raw reply
* Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support
From: Nikita Yushchenko @ 2017-05-16 11:15 UTC (permalink / raw)
To: Dong Aisheng, linux-serial
Cc: fugang.duan, dongas86, gregkh, yangbo.lu, linux-kernel, stefan,
Mingkai.Hu, jslaby, linux-arm-kernel
In-Reply-To: <1494834539-17523-3-git-send-email-aisheng.dong@nxp.com>
> static u32 lpuart32_read(void __iomem *addr)
> {
> - return ioread32be(addr);
> + return lpuart_is_be ? ioread32be(addr) : readl(addr);
> }
>
> static void lpuart32_write(u32 val, void __iomem *addr)
> {
> - iowrite32be(val, addr);
> + if (lpuart_is_be)
> + iowrite32be(val, addr);
> + else
> + writel(val, addr);
> }
What if this is ever executed on big endian system?
^ permalink raw reply
* Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support
From: Nikita Yushchenko @ 2017-05-16 11:08 UTC (permalink / raw)
To: Dong Aisheng, linux-serial
Cc: fugang.duan, dongas86, gregkh, yangbo.lu, linux-kernel, stefan,
Mingkai.Hu, jslaby, linux-arm-kernel
In-Reply-To: <1494834539-17523-3-git-send-email-aisheng.dong@nxp.com>
> @@ -2000,6 +2007,7 @@ static int lpuart_probe(struct platform_device *pdev)
> }
> sport->port.line = ret;
> sport->lpuart32 = sdata->is_32;
> + lpuart_is_be = sdata->is_be;
Setting a global variable in per-device routine is quite bad design.
^ permalink raw reply
* Re: [PATCH] serial: meson: add Magic SysRq support
From: Greg Kroah-Hartman @ 2017-05-16 8:00 UTC (permalink / raw)
To: Yixun Lan
Cc: Kevin Hilman, Ben Dooks, linux-serial, linux-amlogic,
linux-arm-kernel, linux-kernel
In-Reply-To: <20170516074529.10086-1-dlan@gentoo.org>
On Tue, May 16, 2017 at 03:45:29PM +0800, Yixun Lan wrote:
> Signed-off-by: Yixun Lan <dlan@gentoo.org>
I don't accept patches without any changelog text, sorry.
Please fix.
thanks,
greg k-h
^ permalink raw reply
* [PATCH] serial: meson: add Magic SysRq support
From: Yixun Lan @ 2017-05-16 7:45 UTC (permalink / raw)
To: Kevin Hilman, Greg Kroah-Hartman
Cc: Yixun Lan, Ben Dooks, linux-serial, linux-amlogic,
linux-arm-kernel, linux-kernel
Signed-off-by: Yixun Lan <dlan@gentoo.org>
---
drivers/tty/serial/meson_uart.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 60f16795d16b..f0d9222db82f 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -14,6 +14,10 @@
*
*/
+#if defined(CONFIG_SERIAL_MESON_CONSOLE) && defined(CONFIG_MAGIC_SYSRQ)
+#define SUPPORT_SYSRQ
+#endif
+
#include <linux/clk.h>
#include <linux/console.h>
#include <linux/delay.h>
@@ -183,12 +187,12 @@ static void meson_receive_chars(struct uart_port *port)
{
struct tty_port *tport = &port->state->port;
char flag;
- u32 status, ch, mode;
+ u32 ostatus, status, ch, mode;
do {
flag = TTY_NORMAL;
port->icount.rx++;
- status = readl(port->membase + AML_UART_STATUS);
+ ostatus = status = readl(port->membase + AML_UART_STATUS);
if (status & AML_UART_ERR) {
if (status & AML_UART_TX_FIFO_WERR)
@@ -216,6 +220,16 @@ static void meson_receive_chars(struct uart_port *port)
ch = readl(port->membase + AML_UART_RFIFO);
ch &= 0xff;
+ if ((ostatus & AML_UART_FRAME_ERR) && (ch == 0)) {
+ port->icount.brk++;
+ flag = TTY_BREAK;
+ if (uart_handle_break(port))
+ continue;
+ }
+
+ if (uart_handle_sysrq_char(port, ch))
+ continue;
+
if ((status & port->ignore_status_mask) == 0)
tty_insert_flip_char(tport, ch, flag);
--
2.12.2
^ permalink raw reply related
* Re: [PATCH V2 6/6] tty: serial: lpuart: add a more accurate baud rate calculation method
From: Stefan Agner @ 2017-05-15 17:06 UTC (permalink / raw)
To: Dong Aisheng
Cc: linux-serial, linux-kernel, linux-arm-kernel, gregkh, jslaby,
fugang.duan, dongas86, Mingkai.Hu, yangbo.lu
In-Reply-To: <1494834539-17523-7-git-send-email-aisheng.dong@nxp.com>
On 2017-05-15 00:48, Dong Aisheng wrote:
> On new LPUART versions, the oversampling ratio for the receiver can be
> changed from 4x (00011) to 32x (11111) which could help us get a more
> accurate baud rate divider.
>
> The idea is to use the best OSR (over-sampling rate) possible.
> Note, OSR is typically hard-set to 16 in other LPUART instantiations.
> Loop to find the best OSR value possible, one that generates minimum
> baud diff iterate through the rest of the supported values of OSR.
>
> Currently only i.MX7ULP is using it.
>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jiri Slaby <jslaby@suse.com>
> Cc: Stefan Agner <stefan@agner.ch>
> Cc: Mingkai Hu <Mingkai.Hu@nxp.com>
> Cc: Yangbo Lu <yangbo.lu@nxp.com>
> Acked-by: Fugang Duan <fugang.duan@nxp.com>
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---
> drivers/tty/serial/fsl_lpuart.c | 85 ++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 79 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> index 107d0911..bda4b0c 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -140,6 +140,8 @@
> #define UARTBAUD_SBNS 0x00002000
> #define UARTBAUD_SBR 0x00000000
> #define UARTBAUD_SBR_MASK 0x1fff
> +#define UARTBAUD_OSR_MASK 0x1f
> +#define UARTBAUD_OSR_SHIFT 24
>
> #define UARTSTAT_LBKDIF 0x80000000
> #define UARTSTAT_RXEDGIF 0x40000000
> @@ -1506,6 +1508,72 @@ lpuart_set_termios(struct uart_port *port,
> struct ktermios *termios,
> }
>
> static void
> +lpuart32_serial_setbrg(struct lpuart_port *sport, unsigned int baudrate)
> +{
> + u32 sbr, osr, baud_diff, tmp_osr, tmp_sbr, tmp_diff, tmp;
> + u32 clk = sport->port.uartclk;
> +
> + /*
> + * The idea is to use the best OSR (over-sampling rate) possible.
> + * Note, OSR is typically hard-set to 16 in other LPUART instantiations.
> + * Loop to find the best OSR value possible, one that generates minimum
> + * baud_diff iterate through the rest of the supported values of OSR.
> + *
> + * Calculation Formula:
> + * Baud Rate = baud clock / ((OSR+1) × SBR)
> + */
> + baud_diff = baudrate;
> + osr = 0;
> + sbr = 0;
> +
> + for (tmp_osr = 4; tmp_osr <= 32; tmp_osr++) {
> + /* calculate the temporary sbr value */
> + tmp_sbr = (clk / (baudrate * tmp_osr));
> + if (tmp_sbr == 0)
> + tmp_sbr = 1;
> +
> + /*
> + * calculate the baud rate difference based on the temporary
> + * osr and sbr values
> + */
> + tmp_diff = clk / (tmp_osr * tmp_sbr) - baudrate;
> +
> + /* select best values between sbr and sbr+1 */
> + tmp = clk / (tmp_osr * (tmp_sbr + 1));
> + if (tmp_diff > (baudrate - tmp)) {
> + tmp_diff = baudrate - tmp;
> + tmp_sbr++;
> + }
> +
> + if (tmp_diff <= baud_diff) {
> + baud_diff = tmp_diff;
> + osr = tmp_osr;
> + sbr = tmp_sbr;
> + }
> + }
> +
> + /* handle buadrate outside acceptable rate */
> + if (baud_diff > ((baudrate / 100) * 3))
> + dev_warn(sport->port.dev,
> + "unacceptable baud rate difference of more than 3%%\n");
> +
> + tmp = lpuart32_read(sport->port.membase + UARTBAUD);
> +
> + if ((osr > 3) && (osr < 8))
> + tmp |= UARTBAUD_BOTHEDGE;
> +
> + tmp &= ~(UARTBAUD_OSR_MASK << UARTBAUD_OSR_SHIFT);
> + tmp |= (((osr-1) & UARTBAUD_OSR_MASK) << UARTBAUD_OSR_SHIFT);
> +
> + tmp &= ~UARTBAUD_SBR_MASK;
> + tmp |= sbr & UARTBAUD_SBR_MASK;
> +
> + tmp &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
> +
> + lpuart32_write(tmp, sport->port.membase + UARTBAUD);
> +}
> +
> +static void
> lpuart32_set_termios(struct uart_port *port, struct ktermios *termios,
> struct ktermios *old)
> {
> @@ -1611,12 +1679,17 @@ lpuart32_set_termios(struct uart_port *port,
> struct ktermios *termios,
> lpuart32_write(old_ctrl & ~(UARTCTRL_TE | UARTCTRL_RE),
> sport->port.membase + UARTCTRL);
>
> - sbr = sport->port.uartclk / (16 * baud);
> - bd &= ~UARTBAUD_SBR_MASK;
> - bd |= sbr & UARTBAUD_SBR_MASK;
> - bd |= UARTBAUD_BOTHEDGE;
> - bd &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
> - lpuart32_write(bd, sport->port.membase + UARTBAUD);
> + if (of_device_is_compatible(port->dev->of_node, "fsl,imx7ulp-lpuart")) {
Shouldn't we be consequent here and also use a flag in the soc data
instead of of_device_is_compatible...?
Btw, instead of using 3 bools, I would prefer using a single flags like
your patchset is proposing for the GPIO driver, what do you think?
--
Stefan
> + lpuart32_serial_setbrg(sport, baud);
> + } else {
> + sbr = sport->port.uartclk / (16 * baud);
> + bd &= ~UARTBAUD_SBR_MASK;
> + bd |= sbr & UARTBAUD_SBR_MASK;
> + bd |= UARTBAUD_BOTHEDGE;
> + bd &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
> + lpuart32_write(bd, sport->port.membase + UARTBAUD);
> + }
> +
> lpuart32_write(modem, sport->port.membase + UARTMODIR);
> lpuart32_write(ctrl, sport->port.membase + UARTCTRL);
> /* restore control register */
^ permalink raw reply
* Re: [PATCH V2 2/6] tty: serial: lpuart: add little endian 32 bit register support
From: Andy Shevchenko @ 2017-05-15 13:36 UTC (permalink / raw)
To: Dong Aisheng
Cc: fugang.duan, dongas86, Greg Kroah-Hartman, yangbo.lu,
linux-kernel@vger.kernel.org, Stefan Agner, Mingkai.Hu,
linux-serial@vger.kernel.org, Jiri Slaby, linux-arm Mailing List
In-Reply-To: <1494834539-17523-3-git-send-email-aisheng.dong@nxp.com>
On Mon, May 15, 2017 at 10:48 AM, Dong Aisheng <aisheng.dong@nxp.com> wrote:
> It's based on the exist lpuart32 read/write implementation.
Same as per previous comment (perhaps not in other UART driver, but
some might have been using device property for that.
--
With Best Regards,
Andy Shevchenko
^ 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