Linux Serial subsystem development
 help / color / mirror / Atom feed
* Re: [PATCH 000/102] Convert drivers to explicit reset API
From: Philipp Zabel @ 2017-07-24  8:33 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Dmitry Torokhov, Thomas Petazzoni, lkml, Andrew Lunn,
	Prashant Gaikwad, Heiko Stuebner, Peter Chen, DRI, Marc Dietrich,
	Rakesh Iyer, Peter Meerwald-Stadler, linux-clk, Wim Van Sebroeck,
	Wolfram Sang, Xinliang Liu, Chanwoo Choi, Alan Stern, Jiri Slaby,
	Michael Turquette
In-Reply-To: <CACRpkdaXOv7mX+b-q1K34CB5w0SWPXCKa21wHaxL7qjf91PJXQ@mail.gmail.com>

On Sun, 2017-07-23 at 20:41 +0200, Linus Walleij wrote:
> On Thu, Jul 20, 2017 at 10:46 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Thu, Jul 20, 2017 at 5:55 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> 
> >>> What about reset_control_get(struct device *, const char *, int flags)
> >>> to replace all those variants ?
> >>
> >> While I like how this looks, unfortunately (devm_)reset_control_get
> >> already exists without the flags, so we can't change to that with a
> >> gentle transition.
> >
> > This was done for gpiod_get() and its flags argument with horrifying
> > #define-ry, which thankfully was completely hidden from users.
> 
> For your reference:
> 
> commit bae48da237fcedd7ad09569025483b988635efb7
> "gpiolib: add gpiod_get() and gpiod_put() functions"
> 
> commit 39b2bbe3d715cf5013b5c48695ccdd25bd3bf120
> "gpio: add flags argument to gpiod_get*() functions"
> 
> commit 0dbc8b7afef6e4fddcfebcbacbeb269a0a3b06d5
> "gpio: move varargs hack outside #ifdef GPIOLIB"
> 
> commit b17d1bf16cc72a374a48d748940f700009d40ff4
> "gpio: make flags mandatory for gpiod_get functions"
> 
> Retrospectively ... was that really a good idea... it was a LOT
> of trouble to add a flag, maybe it had been better to try and
> just slam all users in a single go.
> 
> But it worked.

Thanks for the hint and the references. It seems this turned out okay,
but I wouldn't dare to introduce such macro horror^Wmagic.
I'd rather have all users converted to the _exclusive/_shared function
calls and maybe then replace the internal __reset_control_get with
Thomas' suggestion.

regards
Philipp

^ permalink raw reply

* Re: [PATCH] powerpc/asm/cacheflush: Cleanup cacheflush function params
From: Matt Brown @ 2017-07-24  4:31 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Geert Uytterhoeven,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	linux-ide-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	DRI Development,
	adi-buildroot-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org,
	Linux MMC List, MTD Maling List,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-spi, linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	USB list
In-Reply-To: <87h8y7gt2z.fsf-W0DJWXSxmBNbyGPkN3NxC2scP1bn1w/D@public.gmane.org>

I've realised that changing the arguments for the cacheflush functions
is much more work than its worth, due to other archs using these
functions.
The next patch will just translate the asm cacheflush functions to c,
keeping the existing parameters.
So this won't have any effect on the drivers.

Thanks,
Matt Brown

On Thu, Jul 20, 2017 at 11:01 PM, Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org> wrote:
> Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> writes:
>
>> On Thu, Jul 20, 2017 at 1:43 PM, Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org> wrote:
>>> Matt Brown <matthew.brown.dev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
>>>> The cacheflush prototypes currently use start and stop values and each
>>>> call requires typecasting the address to an unsigned long.
>>>> This patch changes the cacheflush prototypes to follow the x86 style of
>>>> using a base and size values, with base being a void pointer.
>>>>
>>>> All callers of the cacheflush functions, including drivers, have been
>>>> modified to conform to the new prototypes.
>>>>
>>>> The 64 bit cacheflush functions which were implemented in assembly code
>>>> (flush_dcache_range, flush_inval_dcache_range) have been translated into
>>>> C for readability and coherence.
>>
>>>> --- a/arch/powerpc/include/asm/cacheflush.h
>>>> +++ b/arch/powerpc/include/asm/cacheflush.h
>>>> @@ -51,13 +51,13 @@ static inline void __flush_dcache_icache_phys(unsigned long physaddr)
>>>>   * Write any modified data cache blocks out to memory and invalidate them.
>>>>   * Does not invalidate the corresponding instruction cache blocks.
>>>>   */
>>>> -static inline void flush_dcache_range(unsigned long start, unsigned long stop)
>>>> +static inline void flush_dcache_range(void *start, unsigned long size)
>>>>  {
>>>> -     void *addr = (void *)(start & ~(L1_CACHE_BYTES - 1));
>>>> -     unsigned long size = stop - (unsigned long)addr + (L1_CACHE_BYTES - 1);
>>>> +     void *addr = (void *)((u32)start & ~(L1_CACHE_BYTES - 1));
>>>
>>> unsigned long would be nicer than u32.
>>
>> Indeed. That would make this work on ppc64, too.
>> After which ppc64 has an identical copy (u64 = unsigned long on ppc64) below?
>
> That was Matt's homework to notice that ;)
>
> cheers
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" 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 2/2] printk/console: Enhance the check for consoles using init memory
From: Sergey Senozhatsky @ 2017-07-24  2:03 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Sergey Senozhatsky, Steven Rostedt, Andrew Morton, Peter Zijlstra,
	Matt Redfearn, Greg Kroah-Hartman, Jiri Slaby, David S. Miller,
	Alan Cox, Fabio M. Di Nitto, linux-serial, linux-kernel,
	Sergey Senozhatsky
In-Reply-To: <20170721143253.GA26877@pathway.suse.cz>

Hello,

On (07/21/17 16:32), Petr Mladek wrote:
[..]
> > sort of a problem here is that the next time anyone adds a new ->foo()
> > callback to struct console, that person also needs to remember to update
> > printk_late_init().
> 
> I am not super happy with this as well. Any hint how to do it better
> or more secure is welcome. But I do not see a beter solution at the moment.
> 
> Note that there are only 3 commits in the git history that change this
> structure. Neither of them invalidates this check!

well, the console output is far from perfect, so I can imagine future
changes ;)

> > a completely crazy idea,
> > can we have a dedicated "console init" section which we will not offload
> > if we see keep_bootcon?
> 
> I though about this as well. But this will not avoid the above
> problem. We still would need to make sure that the consoles
> use the special section. Or do I miss anything?

you don't miss anything.

to fix the rootcause of the problem, and not its aftershock, we still
need to either:
	a) move consoles to normal section
or
	b) move consoles to a special section

I don't mind that warning, but I think we also need to tweak the
affected consoles. otherwise, upon maintainer's request to keep
bootcon, a user can just report back "uses init memory and must
be disabled even before the real one is ready" warning, yet the
kernel still would crash (a theoretical case, but for some reason
someone wanted to keep bootcon after all).

[..]
> It means that less than 25% of early consoles are located in the init
> code. I am not sure if it is worth introducing a new section.

ok, good.

> Instead it would make sense to move all these consoles into the normal
> section. But it is not strictly needed if the normal console is
> registered using an init call (always in time). In this case, it is "enough"
> to mention the real console as the last one on the command line.

let's move. to normal section, or to special section. depending on how
much space we can saved unloading the consoles.

> > or... even crazier... disable bootmem offloading (do not offload init
> > section) at all if we see keep_bootcon? keep_bootcon is a purely debugging
> > option which people enable when things are bad and unclear, no one should
> > be using it otherwise, so may be that idea can be a way to go.
> 
> I have talked about this with my colleagues. They told me that it
> would be pity. The keep_bootcon option might be useful to debug
> problems related to freeing the init memory.

agree.

	-ss

^ permalink raw reply

* Re: [PATCH 000/102] Convert drivers to explicit reset API
From: Linus Walleij @ 2017-07-23 18:41 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Philipp Zabel, Thomas Petazzoni, lkml, Andrew Lunn,
	Prashant Gaikwad, Heiko Stuebner, Peter Chen, DRI, Marc Dietrich,
	Rakesh Iyer, Peter Meerwald-Stadler, linux-clk, Wim Van Sebroeck,
	Wolfram Sang, Xinliang Liu, Chanwoo Choi, Alan Stern, Jiri Slaby,
	Michael Turquette, Guenter Roeck
In-Reply-To: <CAKdAkRTx8jd8UToz5_EgMmdW3V47i2uo++YvVB_yzytqSA=P1Q@mail.gmail.com>

On Thu, Jul 20, 2017 at 10:46 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Thu, Jul 20, 2017 at 5:55 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:

>>> What about reset_control_get(struct device *, const char *, int flags)
>>> to replace all those variants ?
>>
>> While I like how this looks, unfortunately (devm_)reset_control_get
>> already exists without the flags, so we can't change to that with a
>> gentle transition.
>
> This was done for gpiod_get() and its flags argument with horrifying
> #define-ry, which thankfully was completely hidden from users.

For your reference:

commit bae48da237fcedd7ad09569025483b988635efb7
"gpiolib: add gpiod_get() and gpiod_put() functions"

commit 39b2bbe3d715cf5013b5c48695ccdd25bd3bf120
"gpio: add flags argument to gpiod_get*() functions"

commit 0dbc8b7afef6e4fddcfebcbacbeb269a0a3b06d5
"gpio: move varargs hack outside #ifdef GPIOLIB"

commit b17d1bf16cc72a374a48d748940f700009d40ff4
"gpio: make flags mandatory for gpiod_get functions"

Retrospectively ... was that really a good idea... it was a LOT
of trouble to add a flag, maybe it had been better to try and
just slam all users in a single go.

But it worked.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH 9/9] tty: synclink_gt: constify pci_device_id.
From: Arvind Yadav @ 2017-07-23 10:01 UTC (permalink / raw)
  To: gregkh, jirislaby, jslaby, gpiccoli; +Cc: linux-kernel, linux-serial
In-Reply-To: <1500804069-11043-1-git-send-email-arvind.yadav.cs@gmail.com>

pci_device_id are not supposed to change at runtime. All functions
working with pci_device_id provided by <linux/pci.h> work with
const pci_device_id. So mark the non-const structs as const.

File size before:
   text	   data	    bss	    dec	    hex	filename
  41180	    480	    185	  41845	   a375	drivers/tty/synclink_gt.o

File size After adding 'const':
   text	   data	    bss	    dec	    hex	filename
  41340	    320	    185	  41845	   a375	drivers/tty/synclink_gt.o

Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
 drivers/tty/synclink_gt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/synclink_gt.c b/drivers/tty/synclink_gt.c
index 529c6e3..636b8ae 100644
--- a/drivers/tty/synclink_gt.c
+++ b/drivers/tty/synclink_gt.c
@@ -95,7 +95,7 @@ MODULE_LICENSE("GPL");
 #define MGSL_MAGIC 0x5401
 #define MAX_DEVICES 32
 
-static struct pci_device_id pci_table[] = {
+static const struct pci_device_id pci_table[] = {
 	{PCI_VENDOR_ID_MICROGATE, SYNCLINK_GT_DEVICE_ID, PCI_ANY_ID, PCI_ANY_ID,},
 	{PCI_VENDOR_ID_MICROGATE, SYNCLINK_GT2_DEVICE_ID, PCI_ANY_ID, PCI_ANY_ID,},
 	{PCI_VENDOR_ID_MICROGATE, SYNCLINK_GT4_DEVICE_ID, PCI_ANY_ID, PCI_ANY_ID,},
-- 
2.7.4

^ permalink raw reply related

* [PATCH 8/9] tty: serial: jsm: constify pci_device_id.
From: Arvind Yadav @ 2017-07-23 10:01 UTC (permalink / raw)
  To: gregkh, jirislaby, jslaby, gpiccoli; +Cc: linux-kernel, linux-serial
In-Reply-To: <1500804069-11043-1-git-send-email-arvind.yadav.cs@gmail.com>

pci_device_id are not supposed to change at runtime. All functions
working with pci_device_id provided by <linux/pci.h> work with
const pci_device_id. So mark the non-const structs as const.

File size before:
   text	   data	    bss	    dec	    hex	filename
   2442	   1088	      8	   3538	    dd2	tty/serial/jsm/jsm_driver.o

File size After adding 'const':
   text	   data	    bss	    dec	    hex	filename
   3082	    448	      8	   3538	    dd2 tty/serial/jsm/jsm_driver.o

Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
 drivers/tty/serial/jsm/jsm_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/jsm/jsm_driver.c b/drivers/tty/serial/jsm/jsm_driver.c
index a119f11..102d499 100644
--- a/drivers/tty/serial/jsm/jsm_driver.c
+++ b/drivers/tty/serial/jsm/jsm_driver.c
@@ -304,7 +304,7 @@ static void jsm_remove_one(struct pci_dev *pdev)
 	kfree(brd);
 }
 
-static struct pci_device_id jsm_pci_tbl[] = {
+static const struct pci_device_id jsm_pci_tbl[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2DB9), 0, 0, 0 },
 	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2DB9PRI), 0, 0, 1 },
 	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2RJ45), 0, 0, 2 },
-- 
2.7.4

^ permalink raw reply related

* [PATCH 7/9] tty: serial: exar: constify pci_device_id.
From: Arvind Yadav @ 2017-07-23 10:01 UTC (permalink / raw)
  To: gregkh, jirislaby, jslaby, gpiccoli; +Cc: linux-kernel, linux-serial
In-Reply-To: <1500804069-11043-1-git-send-email-arvind.yadav.cs@gmail.com>

pci_device_id are not supposed to change at runtime. All functions
working with pci_device_id provided by <linux/pci.h> work with
const pci_device_id. So mark the non-const structs as const.

File size before:
   text	   data	    bss	    dec	    hex	filename
   4030	   1280	      0	   5310	   14be	tty/serial/8250/8250_exar.o

File size After adding 'const':
   text	   data	    bss	    dec	    hex	filename
   4958	    352	      0	   5310	   14be	tty/serial/8250/8250_exar.o

Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
 drivers/tty/serial/8250/8250_exar.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index b5c98e5..6f031fc 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -601,7 +601,7 @@ static const struct exar8250_board pbn_exar_XR17V8358 = {
 		(kernel_ulong_t)&bd			\
 	}
 
-static struct pci_device_id exar_pci_tbl[] = {
+static const struct pci_device_id exar_pci_tbl[] = {
 	CONNECT_DEVICE(XR17C152, UART_2_232, pbn_connect),
 	CONNECT_DEVICE(XR17C154, UART_4_232, pbn_connect),
 	CONNECT_DEVICE(XR17C158, UART_8_232, pbn_connect),
-- 
2.7.4

^ permalink raw reply related

* [PATCH 6/9] tty: serial: pci: constify pci_device_id.
From: Arvind Yadav @ 2017-07-23 10:01 UTC (permalink / raw)
  To: gregkh, jirislaby, jslaby, gpiccoli; +Cc: linux-kernel, linux-serial
In-Reply-To: <1500804069-11043-1-git-send-email-arvind.yadav.cs@gmail.com>

pci_device_id are not supposed to change at runtime. All functions
working with pci_device_id provided by <linux/pci.h> work with
const pci_device_id. So mark the non-const structs as const.

File size before:
   text	   data	    bss	    dec	    hex	filename
  12626	  18128	      0	  30754	   7822 tty/serial/8250/8250_pci.o

File size After adding 'const':
   text	   data	    bss	    dec	    hex	filename
  23986	   6768	      0	  30754	   7822 tty/serial/8250/8250_pci.o

Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
 drivers/tty/serial/8250/8250_pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 00e51a0..553823c 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -3723,7 +3723,7 @@ static int pciserial_resume_one(struct device *dev)
 static SIMPLE_DEV_PM_OPS(pciserial_pm_ops, pciserial_suspend_one,
 			 pciserial_resume_one);
 
-static struct pci_device_id serial_pci_tbl[] = {
+static const struct pci_device_id serial_pci_tbl[] = {
 	/* Advantech use PCI_DEVICE_ID_ADVANTECH_PCI3620 (0x3620) as 'PCI_SUBVENDOR_ID' */
 	{	PCI_VENDOR_ID_ADVANTECH, PCI_DEVICE_ID_ADVANTECH_PCI3620,
 		PCI_DEVICE_ID_ADVANTECH_PCI3620, 0x0001, 0, 0,
-- 
2.7.4

^ permalink raw reply related

* [PATCH 5/9] tty: moxa: constify pci_device_id.
From: Arvind Yadav @ 2017-07-23 10:01 UTC (permalink / raw)
  To: gregkh, jirislaby, jslaby, gpiccoli; +Cc: linux-kernel, linux-serial
In-Reply-To: <1500804069-11043-1-git-send-email-arvind.yadav.cs@gmail.com>

pci_device_id are not supposed to change at runtime. All functions
working with pci_device_id provided by <linux/pci.h> work with
const pci_device_id. So mark the non-const structs as const.

File size before:
   text	   data	    bss	    dec	    hex	filename
  14201	    656	   1760	  16617	   40e9	drivers/tty/moxa.o

File size After adding 'const':
   text	   data	    bss	    dec	    hex	filename
  14329	    528	   1760	  16617	   40e9	drivers/tty/moxa.o

Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
 drivers/tty/moxa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/moxa.c b/drivers/tty/moxa.c
index 3b251f4e..7f3d4cb 100644
--- a/drivers/tty/moxa.c
+++ b/drivers/tty/moxa.c
@@ -88,7 +88,7 @@ static char *moxa_brdname[] =
 };
 
 #ifdef CONFIG_PCI
-static struct pci_device_id moxa_pcibrds[] = {
+static const struct pci_device_id moxa_pcibrds[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_C218),
 		.driver_data = MOXA_BOARD_C218_PCI },
 	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_C320),
-- 
2.7.4

^ permalink raw reply related

* [PATCH 4/9] tty: synclink: constify pci_device_id.
From: Arvind Yadav @ 2017-07-23 10:01 UTC (permalink / raw)
  To: gregkh, jirislaby, jslaby, gpiccoli; +Cc: linux-kernel, linux-serial
In-Reply-To: <1500804069-11043-1-git-send-email-arvind.yadav.cs@gmail.com>

pci_device_id are not supposed to change at runtime. All functions
working with pci_device_id provided by <linux/pci.h> work with
const pci_device_id. So mark the non-const structs as const.

File size before:
   text	   data	    bss	    dec	    hex	filename
  51755	    400	    513	  52668	   cdbc	drivers/tty/synclink.o

File size After adding 'const':
   text	   data	    bss	    dec	    hex	filename
  51883	    304	    513	  52700	   cddc	drivers/tty/synclink.o

Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
 drivers/tty/synclink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/synclink.c b/drivers/tty/synclink.c
index 3fafc5a..3be9811 100644
--- a/drivers/tty/synclink.c
+++ b/drivers/tty/synclink.c
@@ -884,7 +884,7 @@ static int synclink_init_one (struct pci_dev *dev,
 				     const struct pci_device_id *ent);
 static void synclink_remove_one (struct pci_dev *dev);
 
-static struct pci_device_id synclink_pci_tbl[] = {
+static const struct pci_device_id synclink_pci_tbl[] = {
 	{ PCI_VENDOR_ID_MICROGATE, PCI_DEVICE_ID_MICROGATE_USC, PCI_ANY_ID, PCI_ANY_ID, },
 	{ PCI_VENDOR_ID_MICROGATE, 0x0210, PCI_ANY_ID, PCI_ANY_ID, },
 	{ 0, }, /* terminate list */
-- 
2.7.4

^ permalink raw reply related

* [PATCH 3/9] tty: synclinkmp: constify pci_device_id.
From: Arvind Yadav @ 2017-07-23 10:01 UTC (permalink / raw)
  To: gregkh, jirislaby, jslaby, gpiccoli; +Cc: linux-kernel, linux-serial
In-Reply-To: <1500804069-11043-1-git-send-email-arvind.yadav.cs@gmail.com>

pci_device_id are not supposed to change at runtime. All functions
working with pci_device_id provided by <linux/pci.h> work with
const pci_device_id. So mark the non-const structs as const.

File size before:
   text	   data	    bss	    dec	    hex	filename
  44660	    432	    104	  45196	   b08c	drivers/tty/synclinkmp.o

File size After adding 'const':
   text	   data	    bss	    dec	    hex	filename
  44756	    336	    104	  45196	   b08c	drivers/tty/synclinkmp.o

Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
 drivers/tty/synclinkmp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/synclinkmp.c b/drivers/tty/synclinkmp.c
index 9b4fb02..4fed9e7 100644
--- a/drivers/tty/synclinkmp.c
+++ b/drivers/tty/synclinkmp.c
@@ -479,7 +479,7 @@ static char *driver_version = "$Revision: 4.38 $";
 static int synclinkmp_init_one(struct pci_dev *dev,const struct pci_device_id *ent);
 static void synclinkmp_remove_one(struct pci_dev *dev);
 
-static struct pci_device_id synclinkmp_pci_tbl[] = {
+static const struct pci_device_id synclinkmp_pci_tbl[] = {
 	{ PCI_VENDOR_ID_MICROGATE, PCI_DEVICE_ID_MICROGATE_SCA, PCI_ANY_ID, PCI_ANY_ID, },
 	{ 0, }, /* terminate list */
 };
-- 
2.7.4

^ permalink raw reply related

* [PATCH 2/9] tty: isicom: constify pci_device_id.
From: Arvind Yadav @ 2017-07-23 10:01 UTC (permalink / raw)
  To: gregkh, jirislaby, jslaby, gpiccoli; +Cc: linux-kernel, linux-serial
In-Reply-To: <1500804069-11043-1-git-send-email-arvind.yadav.cs@gmail.com>

pci_device_id are not supposed to change at runtime. All functions
working with pci_device_id provided by <linux/pci.h> work with
const pci_device_id. So mark the non-const structs as const.

File size before:
   text	   data	    bss	    dec	    hex	filename
  12176	   1520	  25864	  39560	   9a88	drivers/tty/isicom.o

File size After adding 'const':
   text	   data	    bss	    dec	    hex	filename
  12528	   1168	  25864	  39560	   9a88	drivers/tty/isicom.o

Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
 drivers/tty/isicom.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/isicom.c b/drivers/tty/isicom.c
index b70187b..61ecdd6 100644
--- a/drivers/tty/isicom.c
+++ b/drivers/tty/isicom.c
@@ -150,7 +150,7 @@
 static int isicom_probe(struct pci_dev *, const struct pci_device_id *);
 static void isicom_remove(struct pci_dev *);
 
-static struct pci_device_id isicom_pci_tbl[] = {
+static const struct pci_device_id isicom_pci_tbl[] = {
 	{ PCI_DEVICE(VENDOR_ID, 0x2028) },
 	{ PCI_DEVICE(VENDOR_ID, 0x2051) },
 	{ PCI_DEVICE(VENDOR_ID, 0x2052) },
-- 
2.7.4

^ permalink raw reply related

* [PATCH 1/9] tty: mxser: constify pci_device_id.
From: Arvind Yadav @ 2017-07-23 10:01 UTC (permalink / raw)
  To: gregkh, jirislaby, jslaby, gpiccoli; +Cc: linux-kernel, linux-serial
In-Reply-To: <1500804069-11043-1-git-send-email-arvind.yadav.cs@gmail.com>

pci_device_id are not supposed to change at runtime. All functions
working with pci_device_id provided by <linux/pci.h> work with
const pci_device_id. So mark the non-const structs as const.

File size before:
   text	   data	    bss	    dec	    hex	filename
  20253	   1184	  19904	  41341	   a17d	drivers/tty/mxser.o

File size After adding 'const':
   text	   data	    bss	    dec	    hex	filename
  21117	    300	  19904	  41341	   a17d	drivers/tty/mxser.o

Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
 drivers/tty/mxser.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/mxser.c b/drivers/tty/mxser.c
index 8bd6fb6..1c0c955 100644
--- a/drivers/tty/mxser.c
+++ b/drivers/tty/mxser.c
@@ -145,7 +145,7 @@ static const struct mxser_cardinfo mxser_cards[] = {
 
 /* driver_data correspond to the lines in the structure above
    see also ISA probe function before you change something */
-static struct pci_device_id mxser_pcibrds[] = {
+static const struct pci_device_id mxser_pcibrds[] = {
 	{ PCI_VDEVICE(MOXA, PCI_DEVICE_ID_MOXA_C168),	.driver_data = 3 },
 	{ PCI_VDEVICE(MOXA, PCI_DEVICE_ID_MOXA_C104),	.driver_data = 4 },
 	{ PCI_VDEVICE(MOXA, PCI_DEVICE_ID_MOXA_CP132),	.driver_data = 8 },
-- 
2.7.4

^ permalink raw reply related

* [PATCH 0/9] constify tty pci_device_id.
From: Arvind Yadav @ 2017-07-23 10:01 UTC (permalink / raw)
  To: gregkh, jirislaby, jslaby, gpiccoli; +Cc: linux-kernel, linux-serial

pci_device_id are not supposed to change at runtime. All functions
working with pci_device_id provided by <linux/pci.h> work with
const pci_device_id. So mark the non-const structs as const.

Arvind Yadav (9):
  [PATCH 1/9] tty: mxser: constify pci_device_id.
  [PATCH 2/9] tty: isicom: constify pci_device_id.
  [PATCH 3/9] tty: synclinkmp: constify pci_device_id.
  [PATCH 4/9] tty: synclink: constify pci_device_id.
  [PATCH 5/9] tty: moxa: constify pci_device_id.
  [PATCH 6/9] tty: serial: pci: constify pci_device_id.
  [PATCH 7/9] tty: serial: exar: constify pci_device_id.
  [PATCH 8/9] tty: serial: jsm: constify pci_device_id.
  [PATCH 9/9] tty: synclink_gt: constify pci_device_id.

 drivers/tty/isicom.c                | 2 +-
 drivers/tty/moxa.c                  | 2 +-
 drivers/tty/mxser.c                 | 2 +-
 drivers/tty/serial/8250/8250_exar.c | 2 +-
 drivers/tty/serial/8250/8250_pci.c  | 2 +-
 drivers/tty/serial/jsm/jsm_driver.c | 2 +-
 drivers/tty/synclink.c              | 2 +-
 drivers/tty/synclink_gt.c           | 2 +-
 drivers/tty/synclinkmp.c            | 2 +-
 9 files changed, 9 insertions(+), 9 deletions(-)

-- 
2.7.4

^ permalink raw reply

* [GIT PULL] TTY/Serial fixes for 4.13-rc2
From: Greg KH @ 2017-07-22  9:54 UTC (permalink / raw)
  To: Linus Torvalds, Jiri Slaby
  Cc: Stephen Rothwell, Andrew Morton, linux-kernel, linux-serial

The following changes since commit 5771a8c08880cdca3bfb4a3fc6d309d6bba20877:

  Linux v4.13-rc1 (2017-07-15 15:22:10 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/ tags/tty-4.13-rc2

for you to fetch changes up to c6325179238f1d4683edbec53d8322575d76d7e2:

  tty: Fix TIOCGPTPEER ioctl definition (2017-07-17 17:04:41 +0200)

----------------------------------------------------------------
TTY/Serial fixes for 4.13-rc2

Here are some small tty and serial driver fixes for 4.13-rc2.  Nothing
huge at all, a revert of a patch that turned out to break things, a fix
up for a new tty ioctl we added in 4.13-rc1 to get the uapi definition
correct, and a few minor serial driver fixes for reported issues.

All of these have been in linux-next for a while with no reported
issues.

Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

----------------------------------------------------------------
Arnd Bergmann (1):
      tty: hide unused pty_get_peer function

Dan Carpenter (2):
      serial: st-asc: Potential error pointer dereference
      serial: sh-sci: Uninitialized variables in sysfs files

Fabio Estevam (1):
      tty: serial: lpuart: Fix the logic for detecting the 32-bit type UART

Gleb Fotengauer-Malinovskiy (1):
      tty: Fix TIOCGPTPEER ioctl definition

Greg Kroah-Hartman (1):
      Revert "serial: imx-serial - move DMA buffer configuration to DT"

Ian Jamison (1):
      serial: imx: Prevent TX buffer PIO write when a DMA has been started

 .../devicetree/bindings/serial/fsl-imx-uart.txt    |  2 -
 arch/alpha/include/uapi/asm/ioctls.h               |  2 +-
 arch/mips/include/uapi/asm/ioctls.h                |  2 +-
 arch/parisc/include/uapi/asm/ioctls.h              |  2 +-
 arch/powerpc/include/uapi/asm/ioctls.h             |  2 +-
 arch/sh/include/uapi/asm/ioctls.h                  |  2 +-
 arch/sparc/include/uapi/asm/ioctls.h               |  2 +-
 arch/xtensa/include/uapi/asm/ioctls.h              |  2 +-
 drivers/tty/pty.c                                  | 85 +++++++++++-----------
 drivers/tty/serial/fsl_lpuart.c                    | 24 +++---
 drivers/tty/serial/imx.c                           | 27 +++----
 drivers/tty/serial/sh-sci.c                        | 12 ++-
 drivers/tty/serial/st-asc.c                        |  1 +
 include/uapi/asm-generic/ioctls.h                  |  2 +-
 14 files changed, 83 insertions(+), 84 deletions(-)

^ permalink raw reply

* Re: [RFC PATCH 1/4] serial: uartps: Remove console_initcall from the driver
From: Sören Brinkmann @ 2017-07-21 15:47 UTC (permalink / raw)
  To: Michal Simek
  Cc: linux-kernel, monstr, Alan Cox, Jiri Slaby, linux-serial,
	Greg Kroah-Hartman, linux-arm-kernel
In-Reply-To: <bd75a2ab5838ae1cf44af667e24be2f02c2e9f3c.1500629544.git.michal.simek@xilinx.com>

On Fri, 2017-07-21 at 11:32:24 +0200, Michal Simek wrote:
> register_console() is called from
> uart_add_one_port()->uart_configure_port()
> that's why register_console() is called twice.
> 
> This patch remove console_initcall to call register_console() only from
> one location.
> 
> Also based on my tests cdns_uart_console_setup() is not called
> from the first register_console() call.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> 
> I am not quite sure about this because console_initcall is called
> early but I can see any difference in usage.
> pl011 is not calling this but others are doing it.

Doesn't this break early console/printk? I would expect that the UART
initialization may happen later than console init.

	Sören

^ permalink raw reply

* Re: [PATCH 2/2] printk/console: Enhance the check for consoles using init memory
From: Petr Mladek @ 2017-07-21 14:32 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Steven Rostedt, Andrew Morton, Peter Zijlstra, Matt Redfearn,
	Greg Kroah-Hartman, Jiri Slaby, David S. Miller, Alan Cox,
	Fabio M. Di Nitto, linux-serial, linux-kernel, Sergey Senozhatsky
In-Reply-To: <20170714220626.GB10437@tigerII.localdomain>

On Sat 2017-07-15 07:06:26, Sergey Senozhatsky wrote:
> On (07/14/17 14:51), Petr Mladek wrote:
> [..]
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index f35d3ac3b8c7..1ebe1525ef64 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -2659,8 +2659,16 @@ static int __init printk_late_init(void)
> >  	int ret;
> >  
> >  	for_each_console(con) {
> > -		if ((con->flags & CON_BOOT) &&
> > -		    init_section_intersects(con, sizeof(*con))) {
> > +		if (!(con->flags & CON_BOOT))
> > +			continue;
> > +
> > +		/* Check addresses that might be used for enabled consoles. */
> > +		if (init_section_intersects(con, sizeof(*con)) ||
> > +		    init_section_contains(con->write, 0) ||
> > +		    init_section_contains(con->read, 0) ||
> > +		    init_section_contains(con->device, 0) ||
> > +		    init_section_contains(con->unblank, 0) ||
> > +		    init_section_contains(con->data, 0)) {
> 
> sort of a problem here is that the next time anyone adds a new ->foo()
> callback to struct console, that person also needs to remember to update
> printk_late_init().

I am not super happy with this as well. Any hint how to do it better
or more secure is welcome. But I do not see a beter solution at the moment.

Note that there are only 3 commits in the git history that change this
structure. Neither of them invalidates this check!

Just for record. The commits are:

  + c7cef0a84912cab3 ("console: Add extensible console matching")
      + Mar 9 2015
      + replaced .early_setup with .match

  + 18a8bd949d6adb31 ("serial: convert early_uart to earlycon
    for 8250")
	+ Jul 15 2007 (10 years ago)!
	+ added .early_setup

  + 6ae9200f2cab7b32 ("enlarge console.name")
	+ May 8 2007
	+ resized .name[8] -> .name[16]


> a completely crazy idea,
> can we have a dedicated "console init" section which we will not offload
> if we see keep_bootcon?

I though about this as well. But this will not avoid the above
problem. We still would need to make sure that the consoles
use the special section. Or do I miss anything?

Also note that only few early consoles actually use the init section
and are affected by this problem. Well, I did only a rough grepping.
It is not perfect but it might give a picture:

# struct console location

$> git grep "struct console.*{" | grep early | wc -l
23
$> git grep "struct console.*{" | grep early | grep init | wc -l
5

# write() function location of statically defined struct consoles

$> for func in `git grep -A 6 "struct console.*{" | grep write |
        cut -d= -f2 | cut -d',' -f1 | grep -v void` ;do 
    git grep "void.*$func(struct " ; done | grep early | wc -l
83

$> for func in `git grep -A 6 "struct console.*{" | grep write | 
        cut -d= -f2 | cut -d',' -f1 | grep -v void` ;
   do git grep "void.*$func(struct " ; done | grep early | grep init | wc -l
12

# write method location of struct consoles defined by OF_EARLYCON_DECLARE()

$> for func in `git grep 'con->write =' | cut -d= -f2 | cut -d';' -f1`
     ; do git grep "void.*$func(struct " ; done | grep early wc -l
26

$> for func in `git grep 'con->write =' | cut -d= -f2 | cut -d';' -f1`
     ; do git grep "void.*$func(struct " ; done | grep early | 
       grep init | wc -l
7


It means that less than 25% of early consoles are located in the init
code. I am not sure if it is worth introducing a new section.

Instead it would make sense to move all these consoles into the normal
section. But it is not strictly needed if the normal console is
registered using an init call (always in time). In this case, it is "enough"
to mention the real console as the last one on the command line.


> or... even crazier... disable bootmem offloading (do not offload init
> section) at all if we see keep_bootcon? keep_bootcon is a purely debugging
> option which people enable when things are bad and unclear, no one should
> be using it otherwise, so may be that idea can be a way to go.

I have talked about this with my colleagues. They told me that it
would be pity. The keep_bootcon option might be useful to debug
problems related to freeing the init memory.

In addition, it will not help to avoid the controversal check above.
We need to remove early icons also when the preferred console is
registered too late because of a deferred probe. Or when the preferred
console is not registred at all from some reasons. I mean that problematic
early consoles might need to be forcibly removed even without
keep_bootcon option.

I still vote for the proposed patch even though it is not perfect.
IMHO, "The perfect is the enemy of the good" fits here.

Best Regards,
Petr

^ permalink raw reply

* Re: [PATCH v4 2/2] arm64: dts: Add Mediatek SoC MT2712 and evaluation board dts and Makefile
From: Matthias Brugger @ 2017-07-21 11:52 UTC (permalink / raw)
  To: YT Shen
  Cc: Rob Herring, Mark Rutland, Thomas Gleixner, Jason Cooper,
	Marc Zyngier, Greg Kroah-Hartman, Catalin Marinas, Will Deacon,
	Mars Cheng, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	srv_heupstream-NuS5LvNUpcJWk0Htik3J/w
In-Reply-To: <1500618746.14868.13.camel@mtksdaap41>



On 07/21/2017 08:32 AM, YT Shen wrote:
> On Wed, 2017-07-19 at 11:26 +0200, Matthias Brugger wrote:
>>
>> On 07/19/2017 08:48 AM, YT Shen wrote:
>>> On Tue, 2017-07-18 at 18:29 +0200, Matthias Brugger wrote:
>>>>
>>>> On 06/22/2017 11:32 AM, YT Shen wrote:
>>>>> This adds basic chip support for Mediatek 2712
>>
>> [...]
>>
>>>>> +
>>>>> +	uart_clk: dummy26m {
>>>>> +		compatible = "fixed-clock";
>>>>> +		clock-frequency = <26000000>;
>>>>> +		#clock-cells = <0>;
>>>>> +	};
>>>>> +
>>
>> [...]
>>
>>>>> +
>>>>> +	soc {
>>>>> +		#address-cells = <2>;
>>>>> +		#size-cells = <2>;
>>>>> +		compatible = "simple-bus";
>>>>> +		ranges;
>>>>> +
>>>>> +		uart5: serial@1000f000 {
>>>>> +			compatible = "mediatek,mt2712-uart",
>>>>> +				     "mediatek,mt6577-uart";
>>>>> +			reg = <0 0x1000f000 0 0x400>;
>>>>> +			interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_LOW>;
>>>>> +			clocks = <&uart_clk>, <&uart_clk>;
>>>>> +			clock-names = "baud", "bus";
>>>>> +			status = "disabled";
>>>>> +		};
>>>>
>>>> So baud and bus clock are both 26 MHz?
>>> We didn't have CCF clock support in this series.
>>> After we have clock source support, we could use the correct clocks to
>>> the UARTs and drop the 26MHz fixed rate UART clock.
>>>
>>> The bus clock is 26MHz.  The baud clock could be from another clock
>>> source, using the same 26MHz fixed clock works also.
>>>
>>>
>>> [1] https://patchwork.kernel.org/patch/9670877/
>>> [2] https://patchwork.kernel.org/patch/6436021/
>>>
>>
>> Yes, just using one 26 MHz clock works, but it uses an deprecated
>> binding, so we should not do this, as through copying from the source of
>> other SoCs we will keep it alive forever. Anyway that's not your case,
>> as you defined the two clocks.
>>
>> The device tree should reflect the HW, that's why I asked for the clock
>> frequency of both clocks. I searched the git history and it was never
>> done right before. So you could be the first :)
>>
>> Thanks,
>> Matthias
> Ok, I want to make it clear.  The following example
> 
>          baud_clk: dummy26m {
>                  compatible = "fixed-clock";
>                  clock-frequency = <26000000>;
>                  #clock-cells = <0>;
>          };
> 
>          sys_clk: dummyclk {
>                  compatible = "fixed-clock";
>                  clock-frequency = <26000000>;
>                  #clock-cells = <0>;
>          };
> 
> 	uart0: serial@11002000 {
> [...]
>                  clocks = <&baud_clk>, <&sys_clk>;
> [...]
> 	}
> 
> Do you think it is clear to reflect the HW that the baud clock and sys
> clock can be different source or we need to choose another frequency?
> Thanks.
> 

Best would be to choose the frequency you expect from both clocks. E.g. 
the frequency set by the bootloader. If the clock is the same, then we 
can leave the patch as-is.

Thanks,
Matthias
--
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

* [RFC PATCH 4/4] serial: uartps: Remove static port array
From: Michal Simek @ 2017-07-21  9:32 UTC (permalink / raw)
  To: linux-kernel, monstr, Alan Cox
  Cc: Sören Brinkmann, Jiri Slaby, linux-serial,
	Greg Kroah-Hartman, linux-arm-kernel
In-Reply-To: <cover.1500629544.git.michal.simek@xilinx.com>

Allocate uart port structure dynamically.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 drivers/tty/serial/xilinx_uartps.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 4fb74baeae35..1c9ec8c4c2b6 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1102,8 +1102,6 @@ static void cdns_uart_pm(struct uart_port *port, unsigned int state,
 #endif
 };
 
-static struct uart_port cdns_uart_port[CDNS_UART_NR_PORTS];
-
 #ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE
 /**
  * cdns_uart_console_wait_tx - Wait for the TX to be full
@@ -1443,6 +1441,9 @@ static int cdns_uart_probe(struct platform_device *pdev)
 			GFP_KERNEL);
 	if (!cdns_uart_data)
 		return -ENOMEM;
+	port = devm_kzalloc(&pdev->dev, sizeof(*port), GFP_KERNEL);
+	if (!port)
+		return -ENOMEM;
 
 	match = of_match_node(cdns_uart_of_match, pdev->dev.of_node);
 	if (match && match->data) {
@@ -1508,16 +1509,7 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	if (id < 0)
 		id = 0;
 
-	/* Try the given port id if failed use default method */
-	if (cdns_uart_port[id].mapbase != 0) {
-		/* Find the next unused port */
-		for (id = 0; id < CDNS_UART_NR_PORTS; id++)
-			if (cdns_uart_port[id].mapbase == 0)
-				break;
-	}
-
-	port = &cdns_uart_port[id];
-	if (!port || id >= CDNS_UART_NR_PORTS) {
+	if (id >= CDNS_UART_NR_PORTS) {
 		dev_err(&pdev->dev, "Cannot get uart_port structure\n");
 		rc = -ENODEV;
 		goto err_out_notif_unreg;
-- 
1.9.1

^ permalink raw reply related

* [RFC PATCH 3/4] serial: uartps: Move cnds_uart_get_port to probe
From: Michal Simek @ 2017-07-21  9:32 UTC (permalink / raw)
  To: linux-kernel, monstr, Alan Cox
  Cc: Sören Brinkmann, Jiri Slaby, linux-serial,
	Greg Kroah-Hartman, linux-arm-kernel
In-Reply-To: <cover.1500629544.git.michal.simek@xilinx.com>

c&p this function to probe as preparation for removing
cdns_uart_port[] static array.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 drivers/tty/serial/xilinx_uartps.c | 61 +++++++++++++-------------------------
 1 file changed, 21 insertions(+), 40 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index e6470a3111ce..4fb74baeae35 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1104,43 +1104,6 @@ static void cdns_uart_pm(struct uart_port *port, unsigned int state,
 
 static struct uart_port cdns_uart_port[CDNS_UART_NR_PORTS];
 
-/**
- * cdns_uart_get_port - Configure the port from platform device resource info
- * @id: Port id
- *
- * Return: a pointer to a uart_port or NULL for failure
- */
-static struct uart_port *cdns_uart_get_port(int id)
-{
-	struct uart_port *port;
-
-	/* Try the given port id if failed use default method */
-	if (cdns_uart_port[id].mapbase != 0) {
-		/* Find the next unused port */
-		for (id = 0; id < CDNS_UART_NR_PORTS; id++)
-			if (cdns_uart_port[id].mapbase == 0)
-				break;
-	}
-
-	if (id >= CDNS_UART_NR_PORTS)
-		return NULL;
-
-	port = &cdns_uart_port[id];
-
-	/* At this point, we've got an empty uart_port struct, initialize it */
-	spin_lock_init(&port->lock);
-	port->membase	= NULL;
-	port->irq	= 0;
-	port->type	= PORT_UNKNOWN;
-	port->iotype	= UPIO_MEM32;
-	port->flags	= UPF_BOOT_AUTOCONF;
-	port->ops	= &cdns_uart_ops;
-	port->fifosize	= CDNS_UART_FIFO_SIZE;
-	port->line	= id;
-	port->dev	= NULL;
-	return port;
-}
-
 #ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE
 /**
  * cdns_uart_console_wait_tx - Wait for the TX to be full
@@ -1545,15 +1508,33 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	if (id < 0)
 		id = 0;
 
-	/* Initialize the port structure */
-	port = cdns_uart_get_port(id);
+	/* Try the given port id if failed use default method */
+	if (cdns_uart_port[id].mapbase != 0) {
+		/* Find the next unused port */
+		for (id = 0; id < CDNS_UART_NR_PORTS; id++)
+			if (cdns_uart_port[id].mapbase == 0)
+				break;
+	}
 
-	if (!port) {
+	port = &cdns_uart_port[id];
+	if (!port || id >= CDNS_UART_NR_PORTS) {
 		dev_err(&pdev->dev, "Cannot get uart_port structure\n");
 		rc = -ENODEV;
 		goto err_out_notif_unreg;
 	}
 
+	/* At this point, we've got an empty uart_port struct, initialize it */
+	spin_lock_init(&port->lock);
+	port->membase	= NULL;
+	port->irq	= 0;
+	port->type	= PORT_UNKNOWN;
+	port->iotype	= UPIO_MEM32;
+	port->flags	= UPF_BOOT_AUTOCONF;
+	port->ops	= &cdns_uart_ops;
+	port->fifosize	= CDNS_UART_FIFO_SIZE;
+	port->line	= id;
+	port->dev	= NULL;
+
 	/*
 	 * Register the port.
 	 * This function also registers this device with the tty layer
-- 
1.9.1

^ permalink raw reply related

* [RFC PATCH 2/4] serial: uartps: Use dynamic array for console port
From: Michal Simek @ 2017-07-21  9:32 UTC (permalink / raw)
  To: linux-kernel, monstr, Alan Cox
  Cc: Sören Brinkmann, Jiri Slaby, linux-serial,
	Greg Kroah-Hartman, linux-arm-kernel
In-Reply-To: <cover.1500629544.git.michal.simek@xilinx.com>

Driver console functions are using pointer to static array with fixed
size. There can be only one serial console at the time which is found
by register_console(). register_console() is filling cons->index to
port->line value.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 drivers/tty/serial/xilinx_uartps.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 4614349403c1..e6470a3111ce 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1211,6 +1211,10 @@ static int __init cdns_early_console_setup(struct earlycon_device *device,
 OF_EARLYCON_DECLARE(cdns, "cdns,uart-r1p12", cdns_early_console_setup);
 OF_EARLYCON_DECLARE(cdns, "xlnx,zynqmp-uart", cdns_early_console_setup);
 
+
+/* Static pointer to console port */
+static struct uart_port *console_port;
+
 /**
  * cdns_uart_console_write - perform write operation
  * @co: Console handle
@@ -1220,7 +1224,7 @@ static int __init cdns_early_console_setup(struct earlycon_device *device,
 static void cdns_uart_console_write(struct console *co, const char *s,
 				unsigned int count)
 {
-	struct uart_port *port = &cdns_uart_port[co->index];
+	struct uart_port *port = console_port;
 	unsigned long flags;
 	unsigned int imr, ctrl;
 	int locked = 1;
@@ -1266,15 +1270,13 @@ static void cdns_uart_console_write(struct console *co, const char *s,
  */
 static int __init cdns_uart_console_setup(struct console *co, char *options)
 {
-	struct uart_port *port = &cdns_uart_port[co->index];
+	struct uart_port *port = console_port;
+
 	int baud = 9600;
 	int bits = 8;
 	int parity = 'n';
 	int flow = 'n';
 
-	if (co->index < 0 || co->index >= CDNS_UART_NR_PORTS)
-		return -EINVAL;
-
 	if (!port->membase) {
 		pr_debug("console on " CDNS_UART_TTY_NAME "%i not present\n",
 			 co->index);
@@ -1570,6 +1572,17 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
 
+#ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE
+	/*
+	 * If console hasn't been found yet try to assign this port
+	 * because it is required to be assigned for console setup function.
+	 * If register_console() don't assign value, then console_port pointer
+	 * is cleanup.
+	 */
+	if (cdns_uart_uart_driver.cons->index == -1)
+		console_port = port;
+#endif
+
 	rc = uart_add_one_port(&cdns_uart_uart_driver, port);
 	if (rc) {
 		dev_err(&pdev->dev,
@@ -1577,6 +1590,12 @@ static int cdns_uart_probe(struct platform_device *pdev)
 		goto err_out_pm_disable;
 	}
 
+#ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE
+	/* This is not port which is used for console that's why clean it up */
+	if (cdns_uart_uart_driver.cons->index == -1)
+		console_port = NULL;
+#endif
+
 	return 0;
 
 err_out_pm_disable:
-- 
1.9.1

^ permalink raw reply related

* [RFC PATCH 1/4] serial: uartps: Remove console_initcall from the driver
From: Michal Simek @ 2017-07-21  9:32 UTC (permalink / raw)
  To: linux-kernel, monstr, Alan Cox
  Cc: Sören Brinkmann, Jiri Slaby, linux-serial,
	Greg Kroah-Hartman, linux-arm-kernel
In-Reply-To: <cover.1500629544.git.michal.simek@xilinx.com>

register_console() is called from
uart_add_one_port()->uart_configure_port()
that's why register_console() is called twice.

This patch remove console_initcall to call register_console() only from
one location.

Also based on my tests cdns_uart_console_setup() is not called
from the first register_console() call.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

I am not quite sure about this because console_initcall is called
early but I can see any difference in usage.
pl011 is not calling this but others are doing it.

---
 drivers/tty/serial/xilinx_uartps.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index fde55dcdea5a..4614349403c1 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1298,20 +1298,6 @@ static int __init cdns_uart_console_setup(struct console *co, char *options)
 	.index	= -1, /* Specified on the cmdline (e.g. console=ttyPS ) */
 	.data	= &cdns_uart_uart_driver,
 };
-
-/**
- * cdns_uart_console_init - Initialization call
- *
- * Return: 0 on success, negative errno otherwise
- */
-static int __init cdns_uart_console_init(void)
-{
-	register_console(&cdns_uart_console);
-	return 0;
-}
-
-console_initcall(cdns_uart_console_init);
-
 #endif /* CONFIG_SERIAL_XILINX_PS_UART_CONSOLE */
 
 static struct uart_driver cdns_uart_uart_driver = {
-- 
1.9.1

^ permalink raw reply related

* [RFC PATCH 0/4] serial: uartps: Dynamic allocation
From: Michal Simek @ 2017-07-21  9:32 UTC (permalink / raw)
  To: linux-kernel, monstr, Alan Cox
  Cc: Sören Brinkmann, Jiri Slaby, linux-serial,
	Greg Kroah-Hartman, linux-arm-kernel

Hi Alan,

this is the initial version before next step which is move
uart_register_driver to probe function.
I was able to get rid of static array with uart_port structures.
It was wired with console which is also fixed.
And the next step is the most complicated one handle .nr in uart_driver
structure in more generic way.

Thanks,
Michal


Michal Simek (4):
  serial: uartps: Remove console_initcall from the driver
  serial: uartps: Use dynamic array for console port
  serial: uartps: Move cnds_uart_get_port to probe
  serial: uartps: Remove static port array

 drivers/tty/serial/xilinx_uartps.c | 102 +++++++++++++++----------------------
 1 file changed, 40 insertions(+), 62 deletions(-)

-- 
1.9.1

^ permalink raw reply

* Re: [PATCH v4 2/2] arm64: dts: Add Mediatek SoC MT2712 and evaluation board dts and Makefile
From: YT Shen @ 2017-07-21  6:32 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Rob Herring, Mark Rutland, Thomas Gleixner, Jason Cooper,
	Marc Zyngier, Greg Kroah-Hartman, Catalin Marinas, Will Deacon,
	Mars Cheng, devicetree, linux-kernel, linux-serial,
	linux-arm-kernel, linux-mediatek, srv_heupstream
In-Reply-To: <a470dfc9-71a4-6574-f342-fbd8d60aafce@gmail.com>

On Wed, 2017-07-19 at 11:26 +0200, Matthias Brugger wrote:
> 
> On 07/19/2017 08:48 AM, YT Shen wrote:
> > On Tue, 2017-07-18 at 18:29 +0200, Matthias Brugger wrote:
> >>
> >> On 06/22/2017 11:32 AM, YT Shen wrote:
> >>> This adds basic chip support for Mediatek 2712
> 
> [...]
> 
> >>> +
> >>> +	uart_clk: dummy26m {
> >>> +		compatible = "fixed-clock";
> >>> +		clock-frequency = <26000000>;
> >>> +		#clock-cells = <0>;
> >>> +	};
> >>> +
> 
> [...]
> 
> >>> +
> >>> +	soc {
> >>> +		#address-cells = <2>;
> >>> +		#size-cells = <2>;
> >>> +		compatible = "simple-bus";
> >>> +		ranges;
> >>> +
> >>> +		uart5: serial@1000f000 {
> >>> +			compatible = "mediatek,mt2712-uart",
> >>> +				     "mediatek,mt6577-uart";
> >>> +			reg = <0 0x1000f000 0 0x400>;
> >>> +			interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_LOW>;
> >>> +			clocks = <&uart_clk>, <&uart_clk>;
> >>> +			clock-names = "baud", "bus";
> >>> +			status = "disabled";
> >>> +		};
> >>
> >> So baud and bus clock are both 26 MHz?
> > We didn't have CCF clock support in this series.
> > After we have clock source support, we could use the correct clocks to
> > the UARTs and drop the 26MHz fixed rate UART clock.
> > 
> > The bus clock is 26MHz.  The baud clock could be from another clock
> > source, using the same 26MHz fixed clock works also.
> > 
> > 
> > [1] https://patchwork.kernel.org/patch/9670877/
> > [2] https://patchwork.kernel.org/patch/6436021/
> > 
> 
> Yes, just using one 26 MHz clock works, but it uses an deprecated 
> binding, so we should not do this, as through copying from the source of 
> other SoCs we will keep it alive forever. Anyway that's not your case, 
> as you defined the two clocks.
> 
> The device tree should reflect the HW, that's why I asked for the clock 
> frequency of both clocks. I searched the git history and it was never 
> done right before. So you could be the first :)
> 
> Thanks,
> Matthias
Ok, I want to make it clear.  The following example

        baud_clk: dummy26m {
                compatible = "fixed-clock";
                clock-frequency = <26000000>;
                #clock-cells = <0>;
        };

        sys_clk: dummyclk {
                compatible = "fixed-clock";
                clock-frequency = <26000000>;
                #clock-cells = <0>;
        };

	uart0: serial@11002000 {
[...]
                clocks = <&baud_clk>, <&sys_clk>;
[...]
	}

Do you think it is clear to reflect the HW that the baud clock and sys
clock can be different source or we need to choose another frequency?
Thanks.

yt.shen

^ permalink raw reply

* Re: [PATCH 000/102] Convert drivers to explicit reset API
From: Dmitry Torokhov @ 2017-07-20 20:46 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Thomas Petazzoni, lkml, Andrew Lunn, Prashant Gaikwad,
	Heiko Stuebner, Peter Chen, Linus Walleij, DRI, Marc Dietrich,
	Rakesh Iyer, Peter Meerwald-Stadler, linux-clk, Wim Van Sebroeck,
	Wolfram Sang, Xinliang Liu, Chanwoo Choi, Alan Stern, Jiri Slaby,
	Michael Turquette, Guenter Roeck
In-Reply-To: <1500555312.2354.75.camel@pengutronix.de>

On Thu, Jul 20, 2017 at 5:55 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Hi Thomas,
>
> On Thu, 2017-07-20 at 12:36 +0200, Thomas Petazzoni wrote:
>> Hello,
>>
>> On Thu, 20 Jul 2017 11:36:55 +0200, Philipp Zabel wrote:
>>
>> > > I don't know if it has been discussed in the past, so forgive me if it
>> > > has been. Have you considered adding a "int flags" argument to the
>> > > existing reset_control_get_*() functions, rather than introducing
>> > > separate exclusive variants ?
>> > >
>> > > Indeed, with a "int flags" argument you could in the future add more
>> > > variants/behaviors without actually multiplying the number of
>> > > functions. Something like the "flags" argument for request_irq() for
>> > > example.
>> >
>> > I can't find the discussion right now, but I remember we had talked
>> > about this in the past.
>> > Behind the scenes, all the inline API functions already call common
>> > entry points with flags (well, currently separate bool parameters for
>> > shared and optional).
>> > One reason against exposing those as an int flags in the user facing API
>> > is the possibility to accidentally provide a wrong value.
>>
>> This is a quite strange argument. You could also accidentally use the
>> wrong variant of the function, just like you could use the wrong flag.
>
> You can't accidentally use no flag at all or a completely bogus value
> with the "plethora of inline functions" variant.
>
>> Once again, the next time you have another parameter for those reset
>> functions, beyond the exclusive/shared variant, you will multiply again
>> by two the number of functions ? You already have the  exclusive/shared
>> and optional/mandatory variants, so 4 variants. When you'll add a new
>> parameter, you'll have 8 variants. Doesn't seem really good.
>
> I'd rather avoid adding more variants, if possible. The complexity
> increases regardless of whether the API is expressed as a bunch of
> functions or as a single function with a bunch of flags.
>
>> What about reset_control_get(struct device *, const char *, int flags)
>> to replace all those variants ?
>
> While I like how this looks, unfortunately (devm_)reset_control_get
> already exists without the flags, so we can't change to that with a
> gentle transition.

This was done for gpiod_get() and its flags argument with horrifying
#define-ry, which thankfully was completely hidden from users.

-- 
Dmitry

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox