Re: [PATCH v2 10/14] serial: pic32_uart: Add PIC32 UART driver

[One Thousand Gnomes]

On Wed, 6 Jan 2016 22:00:43 +
 wrote:

> On 01/05/2016 03:50 PM, One Thousand Gnomes wrote:
> >
> >> +#define PIC32_SDEV_NAME   "ttyS"
> >> +#define PIC32_SDEV_MAJOR  TTY_MAJOR
> >> +#define PIC32_SDEV_MINOR  64
> >
> > No. Same goes for you as every one of the forty other people a year who
> > try and claim their console is ttyS. If it's not an 8250 it isn't.
> >
> > ttyS is the 8250, use dynamic major and minor and a different name.
> 
> Ok. Is there a naming convention documented anywhere? How about ttyPIC?

We used to document it but the document was always stale. ttyPIC sounds
fine providing nobody else is using it (and I don't think they are but
grep is your friend). We enforce the rule because in the early days lots
of people re-used ttyS for their chip. Then their chip grew an external
bus or turned into a SoC and a 16x50 got added and it all broke.

ttyPIC ought to be fine because even if you get new PIC devices with a
different uart you aren't likely to have both of the PIC cores on the
same device.

Alan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 10/14] serial: pic32_uart: Add PIC32 UART driver

[Paul.Thacker]

On 01/05/2016 03:50 PM, One Thousand Gnomes wrote:
>
>> +#define PIC32_SDEV_NAME "ttyS"
>> +#define PIC32_SDEV_MAJORTTY_MAJOR
>> +#define PIC32_SDEV_MINOR64
>
> No. Same goes for you as every one of the forty other people a year who
> try and claim their console is ttyS. If it's not an 8250 it isn't.
>
> ttyS is the 8250, use dynamic major and minor and a different name.

Ok. Is there a naming convention documented anywhere? How about ttyPIC?

>
>
>> +/* serial core request to change current uart setting */
>> +static void pic32_uart_set_termios(struct uart_port *port,
>> +   struct ktermios *new,
>> +   struct ktermios *old)
>> +{
>
> You need to clear any termios features requested but not supported. In
> your case that appears to be CMSPAR, as you don't seem to support
> mark/space parity.

Ack.

>
> Similarly if you only support 8N1 or 7E1/7O1 you need to force the CSIZE
> bits to match what you ended up setting the UART to do.

Ack.

>
>> +/* update baud */
>> +baud = uart_get_baud_rate(port, new, old, 0, port->uartclk / 16);
>> +quot = uart_get_divisor(port, baud) - 1;
>> +pic32_uart_write(quot, sport, PIC32_UART_BRG);
>> +uart_update_timeout(port, new->c_cflag, baud);
>
> See the 8250 driver for an example: you probably need to write back the
> actual rate you got.

Ack.

>
>> +/* serial core request to release uart iomem */
>> +static void pic32_uart_release_port(struct uart_port *port)
>> +{
>> +struct platform_device *pdev = to_platform_device(port->dev);
>> +struct resource *res_mem;
>> +unsigned int res_size;
>
> resource_size_t for resources. Or you could just avoid the pointless
> variable in the first place 8)

Pointless variable removed.

>
> Other oddments - things like kasprintf() returns should be checked

Ack.

>
>
> Alan

Thanks,
Paul

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 10/14] serial: pic32_uart: Add PIC32 UART driver

[Paul.Thacker]

On 01/05/2016 03:50 PM, One Thousand Gnomes wrote:
>
>> +#define PIC32_SDEV_NAME "ttyS"
>> +#define PIC32_SDEV_MAJORTTY_MAJOR
>> +#define PIC32_SDEV_MINOR64
>
> No. Same goes for you as every one of the forty other people a year who
> try and claim their console is ttyS. If it's not an 8250 it isn't.
>
> ttyS is the 8250, use dynamic major and minor and a different name.

Ok. Is there a naming convention documented anywhere? How about ttyPIC?

>
>
>> +/* serial core request to change current uart setting */
>> +static void pic32_uart_set_termios(struct uart_port *port,
>> +   struct ktermios *new,
>> +   struct ktermios *old)
>> +{
>
> You need to clear any termios features requested but not supported. In
> your case that appears to be CMSPAR, as you don't seem to support
> mark/space parity.

Ack.

>
> Similarly if you only support 8N1 or 7E1/7O1 you need to force the CSIZE
> bits to match what you ended up setting the UART to do.

Ack.

>
>> +/* update baud */
>> +baud = uart_get_baud_rate(port, new, old, 0, port->uartclk / 16);
>> +quot = uart_get_divisor(port, baud) - 1;
>> +pic32_uart_write(quot, sport, PIC32_UART_BRG);
>> +uart_update_timeout(port, new->c_cflag, baud);
>
> See the 8250 driver for an example: you probably need to write back the
> actual rate you got.

Ack.

>
>> +/* serial core request to release uart iomem */
>> +static void pic32_uart_release_port(struct uart_port *port)
>> +{
>> +struct platform_device *pdev = to_platform_device(port->dev);
>> +struct resource *res_mem;
>> +unsigned int res_size;
>
> resource_size_t for resources. Or you could just avoid the pointless
> variable in the first place 8)

Pointless variable removed.

>
> Other oddments - things like kasprintf() returns should be checked

Ack.

>
>
> Alan

Thanks,
Paul

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 10/14] serial: pic32_uart: Add PIC32 UART driver

[One Thousand Gnomes]

On Wed, 6 Jan 2016 22:00:43 +
 wrote:

> On 01/05/2016 03:50 PM, One Thousand Gnomes wrote:
> >
> >> +#define PIC32_SDEV_NAME   "ttyS"
> >> +#define PIC32_SDEV_MAJOR  TTY_MAJOR
> >> +#define PIC32_SDEV_MINOR  64
> >
> > No. Same goes for you as every one of the forty other people a year who
> > try and claim their console is ttyS. If it's not an 8250 it isn't.
> >
> > ttyS is the 8250, use dynamic major and minor and a different name.
> 
> Ok. Is there a naming convention documented anywhere? How about ttyPIC?

We used to document it but the document was always stale. ttyPIC sounds
fine providing nobody else is using it (and I don't think they are but
grep is your friend). We enforce the rule because in the early days lots
of people re-used ttyS for their chip. Then their chip grew an external
bus or turned into a SoC and a 16x50 got added and it all broke.

ttyPIC ought to be fine because even if you get new PIC devices with a
different uart you aren't likely to have both of the PIC cores on the
same device.

Alan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 10/14] serial: pic32_uart: Add PIC32 UART driver

[One Thousand Gnomes]

> +#define PIC32_SDEV_NAME  "ttyS"
> +#define PIC32_SDEV_MAJOR TTY_MAJOR
> +#define PIC32_SDEV_MINOR 64

No. Same goes for you as every one of the forty other people a year who
try and claim their console is ttyS. If it's not an 8250 it isn't.

ttyS is the 8250, use dynamic major and minor and a different name.


> +/* serial core request to change current uart setting */
> +static void pic32_uart_set_termios(struct uart_port *port,
> +struct ktermios *new,
> +struct ktermios *old)
> +{

You need to clear any termios features requested but not supported. In
your case that appears to be CMSPAR, as you don't seem to support
mark/space parity.

Similarly if you only support 8N1 or 7E1/7O1 you need to force the CSIZE
bits to match what you ended up setting the UART to do.

> + /* update baud */
> + baud = uart_get_baud_rate(port, new, old, 0, port->uartclk / 16);
> + quot = uart_get_divisor(port, baud) - 1;
> + pic32_uart_write(quot, sport, PIC32_UART_BRG);
> + uart_update_timeout(port, new->c_cflag, baud);

See the 8250 driver for an example: you probably need to write back the
actual rate you got.

> +/* serial core request to release uart iomem */
> +static void pic32_uart_release_port(struct uart_port *port)
> +{
> + struct platform_device *pdev = to_platform_device(port->dev);
> + struct resource *res_mem;
> + unsigned int res_size;

resource_size_t for resources. Or you could just avoid the pointless
variable in the first place 8)

Other oddments - things like kasprintf() returns should be checked


Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 10/14] serial: pic32_uart: Add PIC32 UART driver

[Paul.Thacker]

On 12/20/2015 09:14 AM, Andy Shevchenko wrote:
> On Tue, Dec 15, 2015 at 12:42 AM, Joshua Henderson
>  wrote:
>> From: Andrei Pistirica 
>>
>> This adds UART and a serial console driver for Microchip PIC32 class
>> devices.
>>
>> Signed-off-by: Andrei Pistirica 
>> Signed-off-by: Joshua Henderson 
>> Cc: Ralf Baechle 
>> ---
>>   drivers/tty/serial/Kconfig   |   21 +
>>   drivers/tty/serial/Makefile  |1 +
>>   drivers/tty/serial/pic32_uart.c  |  927 
>> ++
>>   drivers/tty/serial/pic32_uart.h  |  198 
>>   include/uapi/linux/serial_core.h |3 +
>>   5 files changed, 1150 insertions(+)
>>   create mode 100644 drivers/tty/serial/pic32_uart.c
>>   create mode 100644 drivers/tty/serial/pic32_uart.h
>>
>> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
>> index f38beb2..8853b1e 100644
>> --- a/drivers/tty/serial/Kconfig
>> +++ b/drivers/tty/serial/Kconfig
>> @@ -901,6 +901,27 @@ config SERIAL_SGI_L1_CONSOLE
>>  controller serial port as your console (you want this!),
>>  say Y.  Otherwise, say N.
>>
>> +config SERIAL_PIC32
>> +   tristate "Microchip PIC32 serial support"
>> +   depends on MACH_PIC32
>> +   select SERIAL_CORE
>> +   help
>> + If you have a PIC32, this driver supports the serial ports.
>> +
>> + Say Y or M to use PIC32 serial ports, otherwise say N. Note that
>> + to use a serial port as a console, this must be included in kernel 
>> and
>> + not as a module.
>> +
>> +config SERIAL_PIC32_CONSOLE
>> +   bool "PIC32 serial console support"
>> +   depends on SERIAL_PIC32
>> +   select SERIAL_CORE_CONSOLE
>> +   help
>> + If you have a PIC32, this driver supports the putting a console on 
>> one
>> + of the serial ports.
>> +
>> + Say Y to use the PIC32 console, otherwise say N.
>> +
>>   config SERIAL_MPC52xx
>>  tristate "Freescale MPC52xx/MPC512x family PSC serial support"
>>  depends on PPC_MPC52xx || PPC_MPC512x
>> diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
>> index 5ab4111..bc5e354 100644
>> --- a/drivers/tty/serial/Makefile
>> +++ b/drivers/tty/serial/Makefile
>> @@ -93,6 +93,7 @@ obj-$(CONFIG_SERIAL_CONEXANT_DIGICOLOR)   += 
>> digicolor-usart.o
>>   obj-$(CONFIG_SERIAL_MEN_Z135)  += men_z135_uart.o
>>   obj-$(CONFIG_SERIAL_SPRD) += sprd_serial.o
>>   obj-$(CONFIG_SERIAL_STM32) += stm32-usart.o
>> +obj-$(CONFIG_SERIAL_PIC32) += pic32_uart.o
>>
>>   # GPIOLIB helpers for modem control lines
>>   obj-$(CONFIG_SERIAL_MCTRL_GPIO)+= serial_mctrl_gpio.o
>> diff --git a/drivers/tty/serial/pic32_uart.c 
>> b/drivers/tty/serial/pic32_uart.c
>> new file mode 100644
>> index 000..5c05c11
>> --- /dev/null
>> +++ b/drivers/tty/serial/pic32_uart.c
>> @@ -0,0 +1,927 @@
>> +/*
>> + * PIC32 Integrated Serial Driver.
>> + *
>> + * Copyright (C) 2015 Microchip Technology, Inc.
>> + *
>> + * Authors:
>> + *   Steve Scott ,
>> + *   Sorin-Andrei Pistirica 
>> + *
>> + * Licensed under GPLv2 or later.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>
> Didn't notice clock provider here.

Not needed. Will be removed.

>
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>
> Revisit this list and leave exactly what is used.

Done. Updated patch will remove linux/clkdev.h, linux/sysrq.h, 
linux/serial.h, uapi/linux/serial_core.h.

>
>> +
>> +#include "pic32_uart.h"
>> +
>> +/* UART name and device definitions */
>> +#define PIC32_DEV_NAME "pic32-uart"
>> +#define PIC32_MAX_UARTS6
>> +
>> +#define PIC32_SDEV_NAME"ttyS"
>> +#define PIC32_SDEV_MAJOR   TTY_MAJOR
>> +#define PIC32_SDEV_MINOR   64
>> +
>> +/* pic32_sport pointer for console use */
>> +static struct pic32_sport *pic32_sports[PIC32_MAX_UARTS];
>> +
>> +static inline int pic32_enable_clock(struct pic32_sport *sport)
>> +{
>> +   sport->ref_clk++;
>> +
>
> Useless empty line (do in one style).

Ack.

>
>> +   return clk_prepare_enable(sport->clk);
>> +}
>> +
>> +static inline void pic32_disable_clock(struct pic32_sport *sport)
>> +{
>> +   sport->ref_clk--;
>> +   clk_disable_unprepare(sport->clk);
>> +}
>> +
>> +/* serial core request to check if uart tx buffer is empty */
>> +static unsigned int pic32_uart_tx_empty(struct uart_port *port)
>> +{
>> +   struct pic32_sport *sport = to_pic32_sport(port);
>> +   u32 val = pic32_uart_read(sport, PIC32_UART_STA);
>> +
>> +   return (val & PIC32_UART_STA_TRMT) ? 1 : 0;
>> +}
>> +
>> +/* serial core request to set UART outputs */
>> +static void pic32_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
>> +{
>> +   struct pic32_sport *sport = to_pic32_sport(port);
>> +
>> + 

Re: [PATCH v2 10/14] serial: pic32_uart: Add PIC32 UART driver

[Paul.Thacker]

On 12/20/2015 09:14 AM, Andy Shevchenko wrote:
> On Tue, Dec 15, 2015 at 12:42 AM, Joshua Henderson
>  wrote:
>> From: Andrei Pistirica 
>>
>> This adds UART and a serial console driver for Microchip PIC32 class
>> devices.
>>
>> Signed-off-by: Andrei Pistirica 
>> Signed-off-by: Joshua Henderson 
>> Cc: Ralf Baechle 
>> ---
>>   drivers/tty/serial/Kconfig   |   21 +
>>   drivers/tty/serial/Makefile  |1 +
>>   drivers/tty/serial/pic32_uart.c  |  927 
>> ++
>>   drivers/tty/serial/pic32_uart.h  |  198 
>>   include/uapi/linux/serial_core.h |3 +
>>   5 files changed, 1150 insertions(+)
>>   create mode 100644 drivers/tty/serial/pic32_uart.c
>>   create mode 100644 drivers/tty/serial/pic32_uart.h
>>
>> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
>> index f38beb2..8853b1e 100644
>> --- a/drivers/tty/serial/Kconfig
>> +++ b/drivers/tty/serial/Kconfig
>> @@ -901,6 +901,27 @@ config SERIAL_SGI_L1_CONSOLE
>>  controller serial port as your console (you want this!),
>>  say Y.  Otherwise, say N.
>>
>> +config SERIAL_PIC32
>> +   tristate "Microchip PIC32 serial support"
>> +   depends on MACH_PIC32
>> +   select SERIAL_CORE
>> +   help
>> + If you have a PIC32, this driver supports the serial ports.
>> +
>> + Say Y or M to use PIC32 serial ports, otherwise say N. Note that
>> + to use a serial port as a console, this must be included in kernel 
>> and
>> + not as a module.
>> +
>> +config SERIAL_PIC32_CONSOLE
>> +   bool "PIC32 serial console support"
>> +   depends on SERIAL_PIC32
>> +   select SERIAL_CORE_CONSOLE
>> +   help
>> + If you have a PIC32, this driver supports the putting a console on 
>> one
>> + of the serial ports.
>> +
>> + Say Y to use the PIC32 console, otherwise say N.
>> +
>>   config SERIAL_MPC52xx
>>  tristate "Freescale MPC52xx/MPC512x family PSC serial support"
>>  depends on PPC_MPC52xx || PPC_MPC512x
>> diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
>> index 5ab4111..bc5e354 100644
>> --- a/drivers/tty/serial/Makefile
>> +++ b/drivers/tty/serial/Makefile
>> @@ -93,6 +93,7 @@ obj-$(CONFIG_SERIAL_CONEXANT_DIGICOLOR)   += 
>> digicolor-usart.o
>>   obj-$(CONFIG_SERIAL_MEN_Z135)  += men_z135_uart.o
>>   obj-$(CONFIG_SERIAL_SPRD) += sprd_serial.o
>>   obj-$(CONFIG_SERIAL_STM32) += stm32-usart.o
>> +obj-$(CONFIG_SERIAL_PIC32) += pic32_uart.o
>>
>>   # GPIOLIB helpers for modem control lines
>>   obj-$(CONFIG_SERIAL_MCTRL_GPIO)+= serial_mctrl_gpio.o
>> diff --git a/drivers/tty/serial/pic32_uart.c 
>> b/drivers/tty/serial/pic32_uart.c
>> new file mode 100644
>> index 000..5c05c11
>> --- /dev/null
>> +++ b/drivers/tty/serial/pic32_uart.c
>> @@ -0,0 +1,927 @@
>> +/*
>> + * PIC32 Integrated Serial Driver.
>> + *
>> + * Copyright (C) 2015 Microchip Technology, Inc.
>> + *
>> + * Authors:
>> + *   Steve Scott ,
>> + *   Sorin-Andrei Pistirica 
>> + *
>> + * Licensed under GPLv2 or later.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>
> Didn't notice clock provider here.

Not needed. Will be removed.

>
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>
> Revisit this list and leave exactly what is used.

Done. Updated patch will remove linux/clkdev.h, linux/sysrq.h, 
linux/serial.h, uapi/linux/serial_core.h.

>
>> +
>> +#include "pic32_uart.h"
>> +
>> +/* UART name and device definitions */
>> +#define PIC32_DEV_NAME "pic32-uart"
>> +#define PIC32_MAX_UARTS6
>> +
>> +#define PIC32_SDEV_NAME"ttyS"
>> +#define PIC32_SDEV_MAJOR   TTY_MAJOR
>> +#define PIC32_SDEV_MINOR   64
>> +
>> +/* pic32_sport pointer for console use */
>> +static struct pic32_sport *pic32_sports[PIC32_MAX_UARTS];
>> +
>> +static inline int pic32_enable_clock(struct pic32_sport *sport)
>> +{
>> +   sport->ref_clk++;
>> +
>
> Useless empty line (do in one style).

Ack.

>
>> +   return clk_prepare_enable(sport->clk);
>> +}
>> +
>> +static inline void pic32_disable_clock(struct pic32_sport *sport)
>> +{
>> +   sport->ref_clk--;
>> +   clk_disable_unprepare(sport->clk);
>> +}
>> +
>> +/* serial core request to check if uart tx buffer is empty */
>> +static unsigned int pic32_uart_tx_empty(struct uart_port *port)
>> +{
>> +   struct pic32_sport *sport = to_pic32_sport(port);
>> +   u32 val = pic32_uart_read(sport, PIC32_UART_STA);
>> +
>> +   return (val & PIC32_UART_STA_TRMT) ? 1 : 0;
>> +}
>> +
>> 

Re: [PATCH v2 10/14] serial: pic32_uart: Add PIC32 UART driver

[One Thousand Gnomes]

> +#define PIC32_SDEV_NAME  "ttyS"
> +#define PIC32_SDEV_MAJOR TTY_MAJOR
> +#define PIC32_SDEV_MINOR 64

No. Same goes for you as every one of the forty other people a year who
try and claim their console is ttyS. If it's not an 8250 it isn't.

ttyS is the 8250, use dynamic major and minor and a different name.


> +/* serial core request to change current uart setting */
> +static void pic32_uart_set_termios(struct uart_port *port,
> +struct ktermios *new,
> +struct ktermios *old)
> +{

You need to clear any termios features requested but not supported. In
your case that appears to be CMSPAR, as you don't seem to support
mark/space parity.

Similarly if you only support 8N1 or 7E1/7O1 you need to force the CSIZE
bits to match what you ended up setting the UART to do.

> + /* update baud */
> + baud = uart_get_baud_rate(port, new, old, 0, port->uartclk / 16);
> + quot = uart_get_divisor(port, baud) - 1;
> + pic32_uart_write(quot, sport, PIC32_UART_BRG);
> + uart_update_timeout(port, new->c_cflag, baud);

See the 8250 driver for an example: you probably need to write back the
actual rate you got.

> +/* serial core request to release uart iomem */
> +static void pic32_uart_release_port(struct uart_port *port)
> +{
> + struct platform_device *pdev = to_platform_device(port->dev);
> + struct resource *res_mem;
> + unsigned int res_size;

resource_size_t for resources. Or you could just avoid the pointless
variable in the first place 8)

Other oddments - things like kasprintf() returns should be checked


Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 10/14] serial: pic32_uart: Add PIC32 UART driver

[Andy Shevchenko]

On Tue, Dec 15, 2015 at 12:42 AM, Joshua Henderson
 wrote:
> From: Andrei Pistirica 
>
> This adds UART and a serial console driver for Microchip PIC32 class
> devices.
>
> Signed-off-by: Andrei Pistirica 
> Signed-off-by: Joshua Henderson 
> Cc: Ralf Baechle 
> ---
>  drivers/tty/serial/Kconfig   |   21 +
>  drivers/tty/serial/Makefile  |1 +
>  drivers/tty/serial/pic32_uart.c  |  927 
> ++
>  drivers/tty/serial/pic32_uart.h  |  198 
>  include/uapi/linux/serial_core.h |3 +
>  5 files changed, 1150 insertions(+)
>  create mode 100644 drivers/tty/serial/pic32_uart.c
>  create mode 100644 drivers/tty/serial/pic32_uart.h
>
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index f38beb2..8853b1e 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -901,6 +901,27 @@ config SERIAL_SGI_L1_CONSOLE
> controller serial port as your console (you want this!),
> say Y.  Otherwise, say N.
>
> +config SERIAL_PIC32
> +   tristate "Microchip PIC32 serial support"
> +   depends on MACH_PIC32
> +   select SERIAL_CORE
> +   help
> + If you have a PIC32, this driver supports the serial ports.
> +
> + Say Y or M to use PIC32 serial ports, otherwise say N. Note that
> + to use a serial port as a console, this must be included in kernel 
> and
> + not as a module.
> +
> +config SERIAL_PIC32_CONSOLE
> +   bool "PIC32 serial console support"
> +   depends on SERIAL_PIC32
> +   select SERIAL_CORE_CONSOLE
> +   help
> + If you have a PIC32, this driver supports the putting a console on 
> one
> + of the serial ports.
> +
> + Say Y to use the PIC32 console, otherwise say N.
> +
>  config SERIAL_MPC52xx
> tristate "Freescale MPC52xx/MPC512x family PSC serial support"
> depends on PPC_MPC52xx || PPC_MPC512x
> diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
> index 5ab4111..bc5e354 100644
> --- a/drivers/tty/serial/Makefile
> +++ b/drivers/tty/serial/Makefile
> @@ -93,6 +93,7 @@ obj-$(CONFIG_SERIAL_CONEXANT_DIGICOLOR)   += 
> digicolor-usart.o
>  obj-$(CONFIG_SERIAL_MEN_Z135)  += men_z135_uart.o
>  obj-$(CONFIG_SERIAL_SPRD) += sprd_serial.o
>  obj-$(CONFIG_SERIAL_STM32) += stm32-usart.o
> +obj-$(CONFIG_SERIAL_PIC32) += pic32_uart.o
>
>  # GPIOLIB helpers for modem control lines
>  obj-$(CONFIG_SERIAL_MCTRL_GPIO)+= serial_mctrl_gpio.o
> diff --git a/drivers/tty/serial/pic32_uart.c b/drivers/tty/serial/pic32_uart.c
> new file mode 100644
> index 000..5c05c11
> --- /dev/null
> +++ b/drivers/tty/serial/pic32_uart.c
> @@ -0,0 +1,927 @@
> +/*
> + * PIC32 Integrated Serial Driver.
> + *
> + * Copyright (C) 2015 Microchip Technology, Inc.
> + *
> + * Authors:
> + *   Steve Scott ,
> + *   Sorin-Andrei Pistirica 
> + *
> + * Licensed under GPLv2 or later.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Didn't notice clock provider here.

> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Revisit this list and leave exactly what is used.

> +
> +#include "pic32_uart.h"
> +
> +/* UART name and device definitions */
> +#define PIC32_DEV_NAME "pic32-uart"
> +#define PIC32_MAX_UARTS6
> +
> +#define PIC32_SDEV_NAME"ttyS"
> +#define PIC32_SDEV_MAJOR   TTY_MAJOR
> +#define PIC32_SDEV_MINOR   64
> +
> +/* pic32_sport pointer for console use */
> +static struct pic32_sport *pic32_sports[PIC32_MAX_UARTS];
> +
> +static inline int pic32_enable_clock(struct pic32_sport *sport)
> +{
> +   sport->ref_clk++;
> +

Useless empty line (do in one style).

> +   return clk_prepare_enable(sport->clk);
> +}
> +
> +static inline void pic32_disable_clock(struct pic32_sport *sport)
> +{
> +   sport->ref_clk--;
> +   clk_disable_unprepare(sport->clk);
> +}
> +
> +/* serial core request to check if uart tx buffer is empty */
> +static unsigned int pic32_uart_tx_empty(struct uart_port *port)
> +{
> +   struct pic32_sport *sport = to_pic32_sport(port);
> +   u32 val = pic32_uart_read(sport, PIC32_UART_STA);
> +
> +   return (val & PIC32_UART_STA_TRMT) ? 1 : 0;
> +}
> +
> +/* serial core request to set UART outputs */
> +static void pic32_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
> +{
> +   struct pic32_sport *sport = to_pic32_sport(port);
> +
> +   /* set loopback mode */
> +   if (mctrl & TIOCM_LOOP)
> +   pic32_uart_rset(PIC32_UART_MODE_LPBK, sport, PIC32_UART_MODE);
> +   else
> +   pic32_uart_rclr(PIC32_UART_MODE_LPBK, sport, PIC32_UART_MODE);
> +}
> +
> +/* get the state of CTS input pin for this port */
> +static unsigned int get_cts_state(struct pic32_sport *sport)
> +{
> + 

Re: [PATCH v2 10/14] serial: pic32_uart: Add PIC32 UART driver

[Andy Shevchenko]

On Tue, Dec 15, 2015 at 12:42 AM, Joshua Henderson
 wrote:
> From: Andrei Pistirica 
>
> This adds UART and a serial console driver for Microchip PIC32 class
> devices.
>
> Signed-off-by: Andrei Pistirica 
> Signed-off-by: Joshua Henderson 
> Cc: Ralf Baechle 
> ---
>  drivers/tty/serial/Kconfig   |   21 +
>  drivers/tty/serial/Makefile  |1 +
>  drivers/tty/serial/pic32_uart.c  |  927 
> ++
>  drivers/tty/serial/pic32_uart.h  |  198 
>  include/uapi/linux/serial_core.h |3 +
>  5 files changed, 1150 insertions(+)
>  create mode 100644 drivers/tty/serial/pic32_uart.c
>  create mode 100644 drivers/tty/serial/pic32_uart.h
>
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index f38beb2..8853b1e 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -901,6 +901,27 @@ config SERIAL_SGI_L1_CONSOLE
> controller serial port as your console (you want this!),
> say Y.  Otherwise, say N.
>
> +config SERIAL_PIC32
> +   tristate "Microchip PIC32 serial support"
> +   depends on MACH_PIC32
> +   select SERIAL_CORE
> +   help
> + If you have a PIC32, this driver supports the serial ports.
> +
> + Say Y or M to use PIC32 serial ports, otherwise say N. Note that
> + to use a serial port as a console, this must be included in kernel 
> and
> + not as a module.
> +
> +config SERIAL_PIC32_CONSOLE
> +   bool "PIC32 serial console support"
> +   depends on SERIAL_PIC32
> +   select SERIAL_CORE_CONSOLE
> +   help
> + If you have a PIC32, this driver supports the putting a console on 
> one
> + of the serial ports.
> +
> + Say Y to use the PIC32 console, otherwise say N.
> +
>  config SERIAL_MPC52xx
> tristate "Freescale MPC52xx/MPC512x family PSC serial support"
> depends on PPC_MPC52xx || PPC_MPC512x
> diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
> index 5ab4111..bc5e354 100644
> --- a/drivers/tty/serial/Makefile
> +++ b/drivers/tty/serial/Makefile
> @@ -93,6 +93,7 @@ obj-$(CONFIG_SERIAL_CONEXANT_DIGICOLOR)   += 
> digicolor-usart.o
>  obj-$(CONFIG_SERIAL_MEN_Z135)  += men_z135_uart.o
>  obj-$(CONFIG_SERIAL_SPRD) += sprd_serial.o
>  obj-$(CONFIG_SERIAL_STM32) += stm32-usart.o
> +obj-$(CONFIG_SERIAL_PIC32) += pic32_uart.o
>
>  # GPIOLIB helpers for modem control lines
>  obj-$(CONFIG_SERIAL_MCTRL_GPIO)+= serial_mctrl_gpio.o
> diff --git a/drivers/tty/serial/pic32_uart.c b/drivers/tty/serial/pic32_uart.c
> new file mode 100644
> index 000..5c05c11
> --- /dev/null
> +++ b/drivers/tty/serial/pic32_uart.c
> @@ -0,0 +1,927 @@
> +/*
> + * PIC32 Integrated Serial Driver.
> + *
> + * Copyright (C) 2015 Microchip Technology, Inc.
> + *
> + * Authors:
> + *   Steve Scott ,
> + *   Sorin-Andrei Pistirica 
> + *
> + * Licensed under GPLv2 or later.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Didn't notice clock provider here.

> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Revisit this list and leave exactly what is used.

> +
> +#include "pic32_uart.h"
> +
> +/* UART name and device definitions */
> +#define PIC32_DEV_NAME "pic32-uart"
> +#define PIC32_MAX_UARTS6
> +
> +#define PIC32_SDEV_NAME"ttyS"
> +#define PIC32_SDEV_MAJOR   TTY_MAJOR
> +#define PIC32_SDEV_MINOR   64
> +
> +/* pic32_sport pointer for console use */
> +static struct pic32_sport *pic32_sports[PIC32_MAX_UARTS];
> +
> +static inline int pic32_enable_clock(struct pic32_sport *sport)
> +{
> +   sport->ref_clk++;
> +

Useless empty line (do in one style).

> +   return clk_prepare_enable(sport->clk);
> +}
> +
> +static inline void pic32_disable_clock(struct pic32_sport *sport)
> +{
> +   sport->ref_clk--;
> +   clk_disable_unprepare(sport->clk);
> +}
> +
> +/* serial core request to check if uart tx buffer is empty */
> +static unsigned int pic32_uart_tx_empty(struct uart_port *port)
> +{
> +   struct pic32_sport *sport = to_pic32_sport(port);
> +   u32 val = pic32_uart_read(sport, PIC32_UART_STA);
> +
> +   return (val & PIC32_UART_STA_TRMT) ? 1 : 0;
> +}
> +
> +/* serial core request to set UART outputs */
> +static void pic32_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
> +{
> +   struct pic32_sport *sport = to_pic32_sport(port);
> +
> +   /* set loopback mode */
> +   if (mctrl & TIOCM_LOOP)
> +   pic32_uart_rset(PIC32_UART_MODE_LPBK, sport, PIC32_UART_MODE);
> +   else
> +  

[PATCH v2 10/14] serial: pic32_uart: Add PIC32 UART driver

[Joshua Henderson]

From: Andrei Pistirica 

This adds UART and a serial console driver for Microchip PIC32 class
devices.

Signed-off-by: Andrei Pistirica 
Signed-off-by: Joshua Henderson 
Cc: Ralf Baechle 
---
 drivers/tty/serial/Kconfig   |   21 +
 drivers/tty/serial/Makefile  |1 +
 drivers/tty/serial/pic32_uart.c  |  927 ++
 drivers/tty/serial/pic32_uart.h  |  198 
 include/uapi/linux/serial_core.h |3 +
 5 files changed, 1150 insertions(+)
 create mode 100644 drivers/tty/serial/pic32_uart.c
 create mode 100644 drivers/tty/serial/pic32_uart.h

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index f38beb2..8853b1e 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -901,6 +901,27 @@ config SERIAL_SGI_L1_CONSOLE
controller serial port as your console (you want this!),
say Y.  Otherwise, say N.
 
+config SERIAL_PIC32
+   tristate "Microchip PIC32 serial support"
+   depends on MACH_PIC32
+   select SERIAL_CORE
+   help
+ If you have a PIC32, this driver supports the serial ports.
+
+ Say Y or M to use PIC32 serial ports, otherwise say N. Note that
+ to use a serial port as a console, this must be included in kernel and
+ not as a module.
+
+config SERIAL_PIC32_CONSOLE
+   bool "PIC32 serial console support"
+   depends on SERIAL_PIC32
+   select SERIAL_CORE_CONSOLE
+   help
+ If you have a PIC32, this driver supports the putting a console on one
+ of the serial ports.
+
+ Say Y to use the PIC32 console, otherwise say N.
+
 config SERIAL_MPC52xx
tristate "Freescale MPC52xx/MPC512x family PSC serial support"
depends on PPC_MPC52xx || PPC_MPC512x
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index 5ab4111..bc5e354 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -93,6 +93,7 @@ obj-$(CONFIG_SERIAL_CONEXANT_DIGICOLOR)   += 
digicolor-usart.o
 obj-$(CONFIG_SERIAL_MEN_Z135)  += men_z135_uart.o
 obj-$(CONFIG_SERIAL_SPRD) += sprd_serial.o
 obj-$(CONFIG_SERIAL_STM32) += stm32-usart.o
+obj-$(CONFIG_SERIAL_PIC32) += pic32_uart.o
 
 # GPIOLIB helpers for modem control lines
 obj-$(CONFIG_SERIAL_MCTRL_GPIO)+= serial_mctrl_gpio.o
diff --git a/drivers/tty/serial/pic32_uart.c b/drivers/tty/serial/pic32_uart.c
new file mode 100644
index 000..5c05c11
--- /dev/null
+++ b/drivers/tty/serial/pic32_uart.c
@@ -0,0 +1,927 @@
+/*
+ * PIC32 Integrated Serial Driver.
+ *
+ * Copyright (C) 2015 Microchip Technology, Inc.
+ *
+ * Authors:
+ *   Steve Scott ,
+ *   Sorin-Andrei Pistirica 
+ *
+ * Licensed under GPLv2 or later.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "pic32_uart.h"
+
+/* UART name and device definitions */
+#define PIC32_DEV_NAME "pic32-uart"
+#define PIC32_MAX_UARTS6
+
+#define PIC32_SDEV_NAME"ttyS"
+#define PIC32_SDEV_MAJOR   TTY_MAJOR
+#define PIC32_SDEV_MINOR   64
+
+/* pic32_sport pointer for console use */
+static struct pic32_sport *pic32_sports[PIC32_MAX_UARTS];
+
+static inline int pic32_enable_clock(struct pic32_sport *sport)
+{
+   sport->ref_clk++;
+
+   return clk_prepare_enable(sport->clk);
+}
+
+static inline void pic32_disable_clock(struct pic32_sport *sport)
+{
+   sport->ref_clk--;
+   clk_disable_unprepare(sport->clk);
+}
+
+/* serial core request to check if uart tx buffer is empty */
+static unsigned int pic32_uart_tx_empty(struct uart_port *port)
+{
+   struct pic32_sport *sport = to_pic32_sport(port);
+   u32 val = pic32_uart_read(sport, PIC32_UART_STA);
+
+   return (val & PIC32_UART_STA_TRMT) ? 1 : 0;
+}
+
+/* serial core request to set UART outputs */
+static void pic32_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+   struct pic32_sport *sport = to_pic32_sport(port);
+
+   /* set loopback mode */
+   if (mctrl & TIOCM_LOOP)
+   pic32_uart_rset(PIC32_UART_MODE_LPBK, sport, PIC32_UART_MODE);
+   else
+   pic32_uart_rclr(PIC32_UART_MODE_LPBK, sport, PIC32_UART_MODE);
+}
+
+/* get the state of CTS input pin for this port */
+static unsigned int get_cts_state(struct pic32_sport *sport)
+{
+   /* default state must be asserted */
+   int val = 1;
+
+   /* read and invert UxCTS */
+   if (gpio_is_valid(sport->cts_gpio))
+   val = !gpio_get_value(sport->cts_gpio);
+
+   return val;
+}
+
+/* serial core request to return the state of misc UART input pins */
+static unsigned int pic32_uart_get_mctrl(struct uart_port *port)
+{
+   struct pic32_sport *sport = to_pic32_sport(port);
+   unsigned int mctrl = 0;
+
+   if (!sport->hw_flow_ctrl) {
+  

[PATCH v2 10/14] serial: pic32_uart: Add PIC32 UART driver

[Joshua Henderson]

From: Andrei Pistirica 

This adds UART and a serial console driver for Microchip PIC32 class
devices.

Signed-off-by: Andrei Pistirica 
Signed-off-by: Joshua Henderson 
Cc: Ralf Baechle 
---
 drivers/tty/serial/Kconfig   |   21 +
 drivers/tty/serial/Makefile  |1 +
 drivers/tty/serial/pic32_uart.c  |  927 ++
 drivers/tty/serial/pic32_uart.h  |  198 
 include/uapi/linux/serial_core.h |3 +
 5 files changed, 1150 insertions(+)
 create mode 100644 drivers/tty/serial/pic32_uart.c
 create mode 100644 drivers/tty/serial/pic32_uart.h

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index f38beb2..8853b1e 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -901,6 +901,27 @@ config SERIAL_SGI_L1_CONSOLE
controller serial port as your console (you want this!),
say Y.  Otherwise, say N.
 
+config SERIAL_PIC32
+   tristate "Microchip PIC32 serial support"
+   depends on MACH_PIC32
+   select SERIAL_CORE
+   help
+ If you have a PIC32, this driver supports the serial ports.
+
+ Say Y or M to use PIC32 serial ports, otherwise say N. Note that
+ to use a serial port as a console, this must be included in kernel and
+ not as a module.
+
+config SERIAL_PIC32_CONSOLE
+   bool "PIC32 serial console support"
+   depends on SERIAL_PIC32
+   select SERIAL_CORE_CONSOLE
+   help
+ If you have a PIC32, this driver supports the putting a console on one
+ of the serial ports.
+
+ Say Y to use the PIC32 console, otherwise say N.
+
 config SERIAL_MPC52xx
tristate "Freescale MPC52xx/MPC512x family PSC serial support"
depends on PPC_MPC52xx || PPC_MPC512x
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index 5ab4111..bc5e354 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -93,6 +93,7 @@ obj-$(CONFIG_SERIAL_CONEXANT_DIGICOLOR)   += 
digicolor-usart.o
 obj-$(CONFIG_SERIAL_MEN_Z135)  += men_z135_uart.o
 obj-$(CONFIG_SERIAL_SPRD) += sprd_serial.o
 obj-$(CONFIG_SERIAL_STM32) += stm32-usart.o
+obj-$(CONFIG_SERIAL_PIC32) += pic32_uart.o
 
 # GPIOLIB helpers for modem control lines
 obj-$(CONFIG_SERIAL_MCTRL_GPIO)+= serial_mctrl_gpio.o
diff --git a/drivers/tty/serial/pic32_uart.c b/drivers/tty/serial/pic32_uart.c
new file mode 100644
index 000..5c05c11
--- /dev/null
+++ b/drivers/tty/serial/pic32_uart.c
@@ -0,0 +1,927 @@
+/*
+ * PIC32 Integrated Serial Driver.
+ *
+ * Copyright (C) 2015 Microchip Technology, Inc.
+ *
+ * Authors:
+ *   Steve Scott ,
+ *   Sorin-Andrei Pistirica 
+ *
+ * Licensed under GPLv2 or later.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "pic32_uart.h"
+
+/* UART name and device definitions */
+#define PIC32_DEV_NAME "pic32-uart"
+#define PIC32_MAX_UARTS6
+
+#define PIC32_SDEV_NAME"ttyS"
+#define PIC32_SDEV_MAJOR   TTY_MAJOR
+#define PIC32_SDEV_MINOR   64
+
+/* pic32_sport pointer for console use */
+static struct pic32_sport *pic32_sports[PIC32_MAX_UARTS];
+
+static inline int pic32_enable_clock(struct pic32_sport *sport)
+{
+   sport->ref_clk++;
+
+   return clk_prepare_enable(sport->clk);
+}
+
+static inline void pic32_disable_clock(struct pic32_sport *sport)
+{
+   sport->ref_clk--;
+   clk_disable_unprepare(sport->clk);
+}
+
+/* serial core request to check if uart tx buffer is empty */
+static unsigned int pic32_uart_tx_empty(struct uart_port *port)
+{
+   struct pic32_sport *sport = to_pic32_sport(port);
+   u32 val = pic32_uart_read(sport, PIC32_UART_STA);
+
+   return (val & PIC32_UART_STA_TRMT) ? 1 : 0;
+}
+
+/* serial core request to set UART outputs */
+static void pic32_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+   struct pic32_sport *sport = to_pic32_sport(port);
+
+   /* set loopback mode */
+   if (mctrl & TIOCM_LOOP)
+   pic32_uart_rset(PIC32_UART_MODE_LPBK, sport, PIC32_UART_MODE);
+   else
+   pic32_uart_rclr(PIC32_UART_MODE_LPBK, sport, PIC32_UART_MODE);
+}
+
+/* get the state of CTS input pin for this port */
+static unsigned int get_cts_state(struct pic32_sport *sport)
+{
+   /* default state must be asserted */
+   int val = 1;
+
+   /* read and invert UxCTS */
+   if (gpio_is_valid(sport->cts_gpio))
+   val = !gpio_get_value(sport->cts_gpio);
+
+   return val;
+}
+
+/* serial core request to return the state of misc UART input pins */
+static unsigned int