[PATCH 1/4] tty: serial: uartlite: Add structure for private data

[PATCH 1/4] tty: serial: uartlite: Add structure for private data

[Shubhrajyoti Datta]

Add struct uartlite_data, to store the private data of the Uartlite
driver.

Signed-off-by: Tanvi Desai <tanvi.desai@xilinx.com>
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
 drivers/tty/serial/uartlite.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
index c47db78..060ac12 100644
--- a/drivers/tty/serial/uartlite.c
+++ b/drivers/tty/serial/uartlite.c
@@ -54,6 +54,10 @@
 #define ULITE_CONTROL_RST_RX   0x02
 #define ULITE_CONTROL_IE       0x10

+struct uartlite_data {
+       const struct uartlite_reg_ops *reg_ops;
+};
+
 struct uartlite_reg_ops {
        u32 (*in)(void __iomem *addr);
        void (*out)(u32 val, void __iomem *addr);
@@ -91,16 +95,16 @@ static const struct uartlite_reg_ops uartlite_le = {

 static inline u32 uart_in32(u32 offset, struct uart_port *port)
 {
-       const struct uartlite_reg_ops *reg_ops = port->private_data;
+       struct uartlite_data *pdata = port->private_data;

-       return reg_ops->in(port->membase + offset);
+       return pdata->reg_ops->in(port->membase + offset);
 }

 static inline void uart_out32(u32 val, u32 offset, struct uart_port *port)
 {
-       const struct uartlite_reg_ops *reg_ops = port->private_data;
+       struct uartlite_data *pdata = port->private_data;

-       reg_ops->out(val, port->membase + offset);
+       pdata->reg_ops->out(val, port->membase + offset);
 }

 static struct uart_port ulite_ports[ULITE_NR_UARTS];
@@ -325,6 +329,7 @@ static void ulite_release_port(struct uart_port *port)

 static int ulite_request_port(struct uart_port *port)
 {
+       struct uartlite_data *pdata = port->private_data;
        int ret;

        pr_debug("ulite console: port=%p; port->mapbase=%llx\n",
@@ -342,13 +347,13 @@ static int ulite_request_port(struct uart_port *port)
                return -EBUSY;
        }

-       port->private_data = (void *)&uartlite_be;
+       pdata->reg_ops = &uartlite_be;
        ret = uart_in32(ULITE_CONTROL, port);
        uart_out32(ULITE_CONTROL_RST_TX, ULITE_CONTROL, port);
        ret = uart_in32(ULITE_STATUS, port);
        /* Endianess detection */
        if ((ret & ULITE_STATUS_TXEMPTY) != ULITE_STATUS_TXEMPTY)
-               port->private_data = (void *)&uartlite_le;
+               pdata->reg_ops = &uartlite_le;

        return 0;
 }
@@ -585,10 +590,12 @@ static struct uart_driver ulite_uart_driver = {
  * @id: requested id number.  Pass -1 for automatic port assignment
  * @base: base address of uartlite registers
  * @irq: irq number for uartlite
+ * @pdata: private data for uartlite
  *
  * Returns: 0 on success, <0 otherwise
  */
-static int ulite_assign(struct device *dev, int id, u32 base, int irq)
+static int ulite_assign(struct device *dev, int id, u32 base, int irq,
+               struct uartlite_data *pdata)
 {
        struct uart_port *port;
        int rc;
@@ -625,6 +632,7 @@ static int ulite_assign(struct device *dev, int id, u32 base, int irq)
        port->dev = dev;
        port->type = PORT_UNKNOWN;
        port->line = id;
+       port->private_data = pdata;

        dev_set_drvdata(dev, port);

@@ -675,6 +683,7 @@ MODULE_DEVICE_TABLE(of, ulite_of_match);
 static int ulite_probe(struct platform_device *pdev)
 {
        struct resource *res;
+       struct uartlite_data *pdata;
        int irq;
        int id = pdev->id;
 #ifdef CONFIG_OF
@@ -684,6 +693,10 @@ static int ulite_probe(struct platform_device *pdev)
        if (prop)
                id = be32_to_cpup(prop);
 #endif
+       pdata = devm_kzalloc(&pdev->dev, sizeof(struct uartlite_data),
+                       GFP_KERNEL);
+       if (!pdata)
+               return -ENOMEM;

        res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
        if (!res)
@@ -693,7 +706,7 @@ static int ulite_probe(struct platform_device *pdev)
        if (irq <= 0)
                return -ENXIO;

-       return ulite_assign(&pdev->dev, id, res->start, irq);
+       return ulite_assign(&pdev->dev, id, res->start, irq, pdata);
 }

 static int ulite_remove(struct platform_device *pdev)
--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

[PATCH 2/4] tty: serial: uartlite: Add clock adaptation

[Shubhrajyoti Datta]

 - Add support of Common Clock Framework for Uartlite driver
 - Add support for suspend and resume operations

Signed-off-by: Tanvi Desai <tanvi.desai@xilinx.com>
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
 drivers/tty/serial/uartlite.c | 83 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 82 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
index 060ac12..eb8afd9 100644
--- a/drivers/tty/serial/uartlite.c
+++ b/drivers/tty/serial/uartlite.c
@@ -21,6 +21,7 @@
 #include <linux/of_address.h>
 #include <linux/of_device.h>
 #include <linux/of_platform.h>
+#include <linux/clk.h>

 #define ULITE_NAME             "ttyUL"
 #define ULITE_MAJOR            204
@@ -56,6 +57,7 @@

 struct uartlite_data {
        const struct uartlite_reg_ops *reg_ops;
+       struct clk *clk;
 };

 struct uartlite_reg_ops {
@@ -261,8 +263,15 @@ static void ulite_break_ctl(struct uart_port *port, int ctl)

 static int ulite_startup(struct uart_port *port)
 {
+       struct uartlite_data *pdata = port->private_data;
        int ret;

+       ret = clk_enable(pdata->clk);
+       if (ret) {
+               dev_err(port->dev, "Failed to enable clock\n");
+               return ret;
+       }
+
        ret = request_irq(port->irq, ulite_isr, IRQF_SHARED | IRQF_TRIGGER_RISING,
                          "uartlite", port);
        if (ret)
@@ -277,9 +286,12 @@ static int ulite_startup(struct uart_port *port)

 static void ulite_shutdown(struct uart_port *port)
 {
+       struct uartlite_data *pdata = port->private_data;
+
        uart_out32(0, ULITE_CONTROL, port);
        uart_in32(ULITE_CONTROL, port); /* dummy */
        free_irq(port->irq, port);
+       clk_disable(pdata->clk);
 }

 static void ulite_set_termios(struct uart_port *port, struct ktermios *termios,
@@ -370,6 +382,17 @@ static int ulite_verify_port(struct uart_port *port, struct serial_struct *ser)
        return -EINVAL;
 }

+static void ulite_pm(struct uart_port *port, unsigned int state,
+             unsigned int oldstate)
+{
+       struct uartlite_data *pdata = port->private_data;
+
+       if (!state)
+               clk_enable(pdata->clk);
+       else
+               clk_disable(pdata->clk);
+}
+
 #ifdef CONFIG_CONSOLE_POLL
 static int ulite_get_poll_char(struct uart_port *port)
 {
@@ -405,6 +428,7 @@ static const struct uart_ops ulite_ops = {
        .request_port   = ulite_request_port,
        .config_port    = ulite_config_port,
        .verify_port    = ulite_verify_port,
+       .pm             = ulite_pm,
 #ifdef CONFIG_CONSOLE_POLL
        .poll_get_char  = ulite_get_poll_char,
        .poll_put_char  = ulite_put_poll_char,
@@ -666,10 +690,44 @@ static int ulite_release(struct device *dev)
        return rc;
 }

+/**
+ * ulite_suspend - Stop the device.
+ *
+ * @dev: handle to the device structure.
+ * Return: 0 always.
+ */
+static int __maybe_unused ulite_suspend(struct device *dev)
+{
+       struct uart_port *port = dev_get_drvdata(dev);
+
+       if (port)
+               uart_suspend_port(&ulite_uart_driver, port);
+
+       return 0;
+}
+
+/**
+ * ulite_resume - Resume the device.
+ *
+ * @dev: handle to the device structure.
+ * Return: 0 on success, errno otherwise.
+ */
+static int __maybe_unused ulite_resume(struct device *dev)
+{
+       struct uart_port *port = dev_get_drvdata(dev);
+
+       if (port)
+               uart_resume_port(&ulite_uart_driver, port);
+
+       return 0;
+}
+
 /* ---------------------------------------------------------------------
  * Platform bus binding
  */

+static SIMPLE_DEV_PM_OPS(ulite_pm_ops, ulite_suspend, ulite_resume);
+
 #if defined(CONFIG_OF)
 /* Match table for of_platform binding */
 static const struct of_device_id ulite_of_match[] = {
@@ -684,7 +742,7 @@ static int ulite_probe(struct platform_device *pdev)
 {
        struct resource *res;
        struct uartlite_data *pdata;
-       int irq;
+       int irq, ret;
        int id = pdev->id;
 #ifdef CONFIG_OF
        const __be32 *prop;
@@ -706,11 +764,33 @@ static int ulite_probe(struct platform_device *pdev)
        if (irq <= 0)
                return -ENXIO;

+       pdata->clk = devm_clk_get(&pdev->dev, "s_axi_aclk");
+       if (IS_ERR(pdata->clk)) {
+               if (PTR_ERR(pdata->clk) != -ENOENT)
+                       return PTR_ERR(pdata->clk);
+
+               /*
+                * Clock framework support is optional, continue on
+                * anyways if we don't find a matching clock.
+                */
+               pdata->clk = NULL;
+       }
+
+       ret = clk_prepare(pdata->clk);
+       if (ret) {
+               dev_err(&pdev->dev, "Failed to prepare clock\n");
+               return ret;
+       }
+
        return ulite_assign(&pdev->dev, id, res->start, irq, pdata);
 }

 static int ulite_remove(struct platform_device *pdev)
 {
+       struct uart_port *port = dev_get_drvdata(&pdev->dev);
+       struct uartlite_data *pdata = port->private_data;
+
+       clk_disable_unprepare(pdata->clk);
        return ulite_release(&pdev->dev);
 }

@@ -723,6 +803,7 @@ static struct platform_driver ulite_platform_driver = {
        .driver = {
                .name  = "uartlite",
                .of_match_table = of_match_ptr(ulite_of_match),
+               .pm = &ulite_pm_ops,
        },
 };

--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

[PATCH 3/4] doc: uartlite: Add binding for the driver

[Shubhrajyoti Datta]

Add uartlite binding for the driver

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
 .../bindings/serial/xlnx,opb-uartlite.txt          | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serial/xlnx,opb-uartlite.txt

diff --git a/Documentation/devicetree/bindings/serial/xlnx,opb-uartlite.txt b/Documentation/devicetree/bindings/serial/xlnx,opb-uartlite.txt
new file mode 100644
index 0000000..df8aba0
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/xlnx,opb-uartlite.txt
@@ -0,0 +1,24 @@
+Xilinx Axi Uartlite controller Device Tree Bindings
+---------------------------------------------------------
+
+Required properties:
+- compatible           : Can be either of
+                               "xlnx,xps-uartlite-1.00.a"
+                               "xlnx,opb-uartlite-1.00.b"
+- reg                  : Physical base address and size of the Axi Uartlite
+                         registers map.
+- interrupts           : Property with a value describing the interrupt
+                         number.
+- interrupt-parent     : Must be core interrupt controller.
+
+Optional properties:
+- port-number          : Set Uart port number
+
+Example:
+serial@800C0000 {
+       compatible = "xlnx,xps-uartlite-1.00.a";
+       reg = <0x0 0x800c0000 0x10000>;
+       interrupt-parent = <&gic>;
+       interrupts = <0x0 0x6e 0x1>;
+       port-number = <0>;
+};
--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

[PATCH 4/4] Documentation: uartlite: Add clock to the binding

[Shubhrajyoti Datta]

Add clock to the binding

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
 Documentation/devicetree/bindings/serial/xlnx,opb-uartlite.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/xlnx,opb-uartlite.txt b/Documentation/devicetree/bindings/serial/xlnx,opb-uartlite.txt
index df8aba0..7ae9008 100644
--- a/Documentation/devicetree/bindings/serial/xlnx,opb-uartlite.txt
+++ b/Documentation/devicetree/bindings/serial/xlnx,opb-uartlite.txt
@@ -13,6 +13,8 @@ Required properties:

 Optional properties:
 - port-number          : Set Uart port number
+- clock-names          : Should be "s_axi_aclk"
+- clocks               : Input clock specifier. Refer to common clock bindings.

 Example:
 serial@800C0000 {
--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

Re: [PATCH 2/4] tty: serial: uartlite: Add clock adaptation

[Greg KH]

On Wed, Jun 27, 2018 at 05:53:43PM +0530, Shubhrajyoti Datta wrote:
>  - Add support of Common Clock Framework for Uartlite driver
>  - Add support for suspend and resume operations
> 

When you have to list multiple things that a single patch does, that's a
huge hint that it should be broken up into smaller pieces.  Can you
split this up into two patches please?

thanks,

greg k-h

Re: [PATCH 1/4] tty: serial: uartlite: Add structure for private data

[Greg KH]

On Wed, Jun 27, 2018 at 05:53:42PM +0530, Shubhrajyoti Datta wrote:
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

I am supposed to just delete emails with this type of footer, but I'll
tell you that this prevents any such patch from being applied to the
kernel, sorry.

Please remove it is not compatible with kernel development processes.

greg k-h

Re: [PATCH 1/4] tty: serial: uartlite: Add structure for private data

[Shubhrajyoti Datta]

On Thu, Jun 28, 2018 at 5:44 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Wed, Jun 27, 2018 at 05:53:42PM +0530, Shubhrajyoti Datta wrote:
>> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
>
> I am supposed to just delete emails with this type of footer, but I'll
> tell you that this prevents any such patch from being applied to the
> kernel, sorry.
>
> Please remove it is not compatible with kernel development processes.

My Apologies will fix .
>
> greg k-h

Re: [PATCH 3/4] doc: uartlite: Add binding for the driver

[Rob Herring]

On Wed, Jun 27, 2018 at 05:53:44PM +0530, Shubhrajyoti Datta wrote:
> Add uartlite binding for the driver
> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
>  .../bindings/serial/xlnx,opb-uartlite.txt          | 24 ++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/serial/xlnx,opb-uartlite.txt
> 
> diff --git a/Documentation/devicetree/bindings/serial/xlnx,opb-uartlite.txt b/Documentation/devicetree/bindings/serial/xlnx,opb-uartlite.txt
> new file mode 100644
> index 0000000..df8aba0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/xlnx,opb-uartlite.txt
> @@ -0,0 +1,24 @@
> +Xilinx Axi Uartlite controller Device Tree Bindings
> +---------------------------------------------------------
> +
> +Required properties:
> +- compatible           : Can be either of
> +                               "xlnx,xps-uartlite-1.00.a"
> +                               "xlnx,opb-uartlite-1.00.b"
> +- reg                  : Physical base address and size of the Axi Uartlite
> +                         registers map.
> +- interrupts           : Property with a value describing the interrupt
> +                         number.

Just describe how many interrupts, what they are and order.

> +- interrupt-parent     : Must be core interrupt controller.

Doesn't match the example already in bindings/xilinx.txt...

> +
> +Optional properties:
> +- port-number          : Set Uart port number

Nope. Use aliases.

> +
> +Example:
> +serial@800C0000 {

Lower case hex please.

> +       compatible = "xlnx,xps-uartlite-1.00.a";
> +       reg = <0x0 0x800c0000 0x10000>;
> +       interrupt-parent = <&gic>;
> +       interrupts = <0x0 0x6e 0x1>;
> +       port-number = <0>;
> +};
> --
> 2.7.4
> 
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] doc: uartlite: Add binding for the driver

[Rob Herring]

On Wed, Jun 27, 2018 at 05:53:44PM +0530, Shubhrajyoti Datta wrote:
> Add uartlite binding for the driver

Also, bindings are for h/w, not drivers.

For the subject: "dt-bindings: serial: ..."

> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>

Re: [PATCH 4/4] Documentation: uartlite: Add clock to the binding

[Rob Herring]

On Wed, Jun 27, 2018 at 05:53:45PM +0530, Shubhrajyoti Datta wrote:
> Add clock to the binding

Just squash this into the previous patch. Bindings should be complete, 
not evolving...

> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
>  Documentation/devicetree/bindings/serial/xlnx,opb-uartlite.txt | 2 ++
>  1 file changed, 2 insertions(+)

Re: [PATCH 3/4] doc: uartlite: Add binding for the driver

[Maarten Brock]

On 2018-07-04 00:42, Rob Herring wrote:
> On Wed, Jun 27, 2018 at 05:53:44PM +0530, Shubhrajyoti Datta wrote:
>> +
>> +Optional properties:
>> +- port-number          : Set Uart port number
> 
> Nope. Use aliases.

You may not like the current implementation, but that is no reason to 
deny
this documentation of the current state.

Personally I don't get this preference for aliases. It is a one way 
mapping
in the wrong direction if you ask me. If e.g. you have serial0 pointing 
to
a 16550 it will give /dev/ttyS0. This prevents you to create 
/dev/ttyUL0,
because serial0 is already taken. Note that ttySx and ttyULx are in
different namespaces. It would only make sense if ttyULx was an illegal
name for a UART.

Maarten

Re: [PATCH 3/4] doc: uartlite: Add binding for the driver

[Rob Herring]

On Mon, Jul 9, 2018 at 8:15 AM Maarten Brock <m.brock@vanmierlo.com> wrote:
>
> On 2018-07-04 00:42, Rob Herring wrote:
> > On Wed, Jun 27, 2018 at 05:53:44PM +0530, Shubhrajyoti Datta wrote:
> >> +
> >> +Optional properties:
> >> +- port-number          : Set Uart port number
> >
> > Nope. Use aliases.
>
> You may not like the current implementation, but that is no reason to
> deny
> this documentation of the current state.

Okay, didn't realize it was already in use.

> Personally I don't get this preference for aliases. It is a one way
> mapping
> in the wrong direction if you ask me. If e.g. you have serial0 pointing
> to
> a 16550 it will give /dev/ttyS0. This prevents you to create
> /dev/ttyUL0,
> because serial0 is already taken. Note that ttySx and ttyULx are in
> different namespaces. It would only make sense if ttyULx was an illegal
> name for a UART.

The problem here is that each UART defines its own namespace. We don't
do that for any other type of device. The problem hasn't been fixed
because doing so without breaking userspace is hard.

Rob