All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] serial:Add SUNIX SDC serial port driver
@ 2021-05-28  9:23 Moriis Ku
  2021-05-28  9:27 ` Greg KH
  2021-06-02 13:36 ` Enrico Weigelt, metux IT consult
  0 siblings, 2 replies; 3+ messages in thread
From: Moriis Ku @ 2021-05-28  9:23 UTC (permalink / raw)
  To: gregkh; +Cc: linux-serial, jason_lee, taian.chen, Morris Ku

From: Morris Ku <saumah@gmail.com>

SUNIX SDC serial port driver.

Cc: Jason Lee <jason_lee@sunix.com>
Cc: Taian Chen <taian.chen@sunix.com>
Signed-off-by: Morris Ku <saumah@gmail.com>
---
 8250_sdc.c | 519 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 519 insertions(+)
 create mode 100644 8250_sdc.c

diff --git a/drivers/tty/serial/8250_sdc.c b/8250_sdc.c
new file mode 100644
index 0000000..85ff351
--- /dev/null
+++ b/drivers/tty/serial//8250_sdc.c
@@ -0,0 +1,519 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * SUNIX SDC serial port driver.
+ *
+ * Copyright (C) 2021, SUNIX Co., Ltd.
+ *
+ * Based on Synopsys DesignWare 8250 driver written by
+ * - Copyright 2011 Picochip, Jamie Iles.
+ * - Copyright 2013 Intel Corporation
+ */
+
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/of_platform.h>
+#include <linux/debugfs.h>
+#include <linux/serial_8250.h>
+
+struct sdc8250_port_data {
+	int bus_number;
+	int device_number;
+	int irq;
+
+	unsigned char number;
+	unsigned char version;
+	unsigned char resource_cap;
+	unsigned char event_type;
+
+	unsigned short tx_fifo_size;
+	unsigned short rx_fifo_size;
+	unsigned int significand;
+	unsigned char exponent;
+	unsigned char rs232_cap;
+	unsigned char rs422_cap;
+	unsigned char rs485_cap;
+	unsigned char ahdc_cap;
+	unsigned char cs_cap;
+	unsigned char rs422_end_cap;
+	unsigned char rs485_end_cap;
+
+	int line;
+};
+
+#define UART_SDC_V0_UMER				0x0e
+
+struct sdc8250_data {
+	struct sdc8250_port_data data;
+	struct uart_8250_port uart;
+	struct device *device;
+	char device_name[50];
+	struct dentry *debugfs;
+	struct debugfs_blob_wrapper debugfs_blob_device_name;
+
+	struct resource *regs;
+	int fifosize;
+	unsigned long long clk_rate;
+	unsigned char umr;
+};
+
+static struct dentry *sdc_serial_debugfs;
+
+static inline struct sdc8250_data *to_sdc8250_data(
+				struct sdc8250_port_data *data)
+{
+	return container_of(data, struct sdc8250_data, data);
+}
+
+static void sdc8250_do_pm(struct uart_port *p, unsigned int state,
+				unsigned int old)
+{
+	if (!state)
+		pm_runtime_get_sync(p->dev);
+
+	serial8250_do_pm(p, state, old);
+
+	if (state)
+		pm_runtime_put_sync_suspend(p->dev);
+}
+
+static int sdc8250_rs485_config(struct uart_port *p,
+				struct serial_rs485 *rs485)
+{
+	struct sdc8250_data *data = to_sdc8250_data(p->private_data);
+	int ret = 0;
+	unsigned char umr = 0;
+	unsigned char umr_check = 0;
+
+	if (rs485->flags & SER_RS485_ENABLED) {
+		if (rs485->flags & SER_RS485_RX_DURING_TX) {
+			// rs422
+			if (data->data.rs422_cap == 0x01) {
+				umr = 0x02;
+
+				if (rs485->flags & SER_RS485_TERMINATE_BUS) {
+					if (data->data.rs422_end_cap == 0x01) {
+						umr |= 0x40;
+					} else {
+						dev_err(p->dev, "no RS422 termination cap.\n");
+						ret = -EINVAL;
+					}
+				}
+			} else {
+				dev_err(p->dev, "no RS422 cap.\n");
+				ret = -EINVAL;
+			}
+		} else {
+			// rs485
+			if (data->data.rs485_cap == 0x01) {
+				umr = 0x04;
+
+				if (data->data.ahdc_cap == 0x01)
+					umr |= 0x08;
+
+				if (data->data.cs_cap == 0x01)
+					umr |= 0x10;
+
+				if (rs485->flags & SER_RS485_TERMINATE_BUS) {
+					if (data->data.rs485_end_cap == 0x01) {
+						umr |= 0x80;
+					} else {
+						dev_err(p->dev, "no RS485 termination cap.\n");
+						ret = -EINVAL;
+					}
+				}
+			} else {
+				dev_err(p->dev, "no RS485 cap.\n");
+				ret = -EINVAL;
+			}
+		}
+	} else {
+		// rs232
+		if (data->data.rs232_cap == 0x01) {
+			umr = 0x01;
+		} else {
+			dev_err(p->dev, "no rs232 cap.\n");
+			ret = -EINVAL;
+		}
+	}
+
+	if (ret == 0) {
+		do {
+			outb(umr, p->iobase + UART_SDC_V0_UMER);
+			umr_check = inb(p->iobase + UART_SDC_V0_UMER);
+			if (umr != umr_check) {
+				ret = -EIO;
+				break;
+			}
+
+			data->umr = umr;
+			p->rs485 = *rs485;
+
+		} while (false);
+	}
+
+	return ret;
+}
+
+static int sdc8250_debugfs_add(struct sdc8250_data *data)
+{
+	struct dentry *root_dir;
+	char root_name[20];
+
+	memset(root_name, 0, sizeof(root_name));
+	sprintf(root_name, "ttyS%d", data->data.line);
+	root_dir = debugfs_create_dir(root_name, sdc_serial_debugfs);
+	if (IS_ERR(root_dir))
+		return PTR_ERR(root_dir);
+
+	debugfs_create_u32("bus_number", 0644, root_dir,
+		&data->data.bus_number);
+	debugfs_create_u32("device_number", 0644, root_dir,
+		&data->data.device_number);
+	debugfs_create_u32("irq", 0644, root_dir, &data->data.irq);
+	debugfs_create_u8("number", 0644, root_dir, &data->data.number);
+	debugfs_create_u8("version", 0644, root_dir, &data->data.version);
+	debugfs_create_u8("resource_cap", 0644, root_dir,
+		&data->data.resource_cap);
+	debugfs_create_u8("event_type", 0644, root_dir,
+		&data->data.event_type);
+	debugfs_create_u16("tx_fifo_size", 0644, root_dir,
+		&data->data.tx_fifo_size);
+	debugfs_create_u16("rx_fifo_size", 0644, root_dir,
+		&data->data.rx_fifo_size);
+	debugfs_create_u32("significand", 0644, root_dir,
+		&data->data.significand);
+	debugfs_create_u8("exponent", 0644, root_dir,
+		&data->data.exponent);
+	debugfs_create_u8("rs232_cap", 0644, root_dir, &data->data.rs232_cap);
+	debugfs_create_u8("rs422_cap", 0644, root_dir, &data->data.rs422_cap);
+	debugfs_create_u8("rs485_cap", 0644, root_dir, &data->data.rs485_cap);
+	debugfs_create_u8("ahdc_cap", 0644, root_dir, &data->data.ahdc_cap);
+	debugfs_create_u8("cs_cap", 0644, root_dir, &data->data.cs_cap);
+	debugfs_create_u8("rs422_end_cap", 0644, root_dir,
+		&data->data.rs422_end_cap);
+	debugfs_create_u8("rs485_end_cap", 0644, root_dir,
+		&data->data.rs485_end_cap);
+	debugfs_create_u32("line", 0644, root_dir, &data->data.line);
+
+	memset(data->device_name, 0, sizeof(data->device_name));
+	sprintf(data->device_name, "%s", dev_name(data->device));
+	data->device_name[strlen(data->device_name)] = 0x0a;
+	data->debugfs_blob_device_name.data = data->device_name;
+	data->debugfs_blob_device_name.size = strlen(data->device_name) + 1;
+	debugfs_create_blob("device_name", 0644, root_dir,
+		&data->debugfs_blob_device_name);
+
+	data->debugfs = root_dir;
+	return 0;
+}
+
+static void sdc8250_debugfs_remove(struct sdc8250_data *data)
+{
+	debugfs_remove_recursive(data->debugfs);
+}
+
+static int sdc8250_read_property(struct sdc8250_data *data,
+				struct device *dev)
+{
+	int ret;
+
+	ret = device_property_read_u32(dev, "bus_number",
+				&data->data.bus_number);
+	if (ret < 0)
+		return -EINVAL;
+	ret = device_property_read_u32(dev, "device_number",
+				&data->data.device_number);
+	if (ret < 0)
+		return -EINVAL;
+	ret = device_property_read_u32(dev, "irq", &data->data.irq);
+	if (ret < 0)
+		return -EINVAL;
+	ret = device_property_read_u8(dev, "number", &data->data.number);
+	if (ret < 0)
+		return -EINVAL;
+	ret = device_property_read_u8(dev, "version", &data->data.version);
+	if (ret < 0)
+		return -EINVAL;
+	ret = device_property_read_u8(dev, "resource_cap",
+				&data->data.resource_cap);
+	if (ret < 0)
+		return -EINVAL;
+	ret = device_property_read_u8(dev, "event_type",
+				&data->data.event_type);
+	if (ret < 0)
+		return -EINVAL;
+	ret = device_property_read_u16(dev, "tx_fifo_size",
+				&data->data.tx_fifo_size);
+	if (ret < 0)
+		return -EINVAL;
+	ret = device_property_read_u16(dev, "rx_fifo_size",
+				&data->data.rx_fifo_size);
+	if (ret < 0)
+		return -EINVAL;
+	ret = device_property_read_u32(dev, "significand",
+				&data->data.significand);
+	if (ret < 0)
+		return -EINVAL;
+	ret = device_property_read_u8(dev, "exponent",
+				&data->data.exponent);
+	if (ret < 0)
+		return -EINVAL;
+	ret = device_property_read_u8(dev, "rs232_cap", &data->data.rs232_cap);
+	if (ret < 0)
+		return -EINVAL;
+	ret = device_property_read_u8(dev, "rs422_cap", &data->data.rs422_cap);
+	if (ret < 0)
+		return -EINVAL;
+	ret = device_property_read_u8(dev, "rs485_cap", &data->data.rs485_cap);
+	if (ret < 0)
+		return -EINVAL;
+	ret = device_property_read_u8(dev, "ahdc_cap", &data->data.ahdc_cap);
+	if (ret < 0)
+		return -EINVAL;
+	ret = device_property_read_u8(dev, "cs_cap", &data->data.cs_cap);
+	if (ret < 0)
+		return -EINVAL;
+	ret = device_property_read_u8(dev, "rs422_end_cap",
+				&data->data.rs422_end_cap);
+	if (ret < 0)
+		return -EINVAL;
+	ret = device_property_read_u8(dev, "rs485_end_cap",
+				&data->data.rs485_end_cap);
+	if (ret < 0)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int config_rs422485(struct sdc8250_data *data,
+				struct serial_rs485 *rs485)
+{
+	int ret = 0;
+	unsigned char umr_check = 0;
+
+	memset(rs485, 0, sizeof(struct serial_rs485));
+	if (data->data.version == 0x00) {
+		data->umr = inb(data->regs->start + UART_SDC_V0_UMER);
+		if ((data->umr & 0x07) == 0x01) {
+			// rs232 mode
+
+		} else if ((data->umr & 0x07) == 0x02) {
+			// rs422 mode
+			rs485->flags =
+				SER_RS485_ENABLED | SER_RS485_RX_DURING_TX;
+
+			if (data->umr & 0x40)
+				rs485->flags |= SER_RS485_TERMINATE_BUS;
+
+		} else if ((data->umr & 0x07) == 0x04) {
+			// rs485 mode
+			rs485->flags = SER_RS485_ENABLED;
+
+			if (data->umr & 0x80)
+				rs485->flags |= SER_RS485_TERMINATE_BUS;
+
+			if (data->data.ahdc_cap == 0x01)
+				data->umr |= 0x08;
+
+			if (data->data.cs_cap == 0x01)
+				data->umr |= 0x10;
+
+			outb(data->umr, data->regs->start + UART_SDC_V0_UMER);
+			umr_check = inb(data->regs->start + UART_SDC_V0_UMER);
+
+			if (data->umr != umr_check)
+				ret = -EIO;
+
+		} else {
+			ret = -EINVAL;
+		}
+	} else {
+		ret = -EINVAL;
+	}
+	return ret;
+}
+
+static int sdc8250_probe(struct platform_device *pdev)
+{
+	int ret;
+	int i;
+	struct uart_8250_port *up = NULL;
+	struct uart_port *p = NULL;
+	struct sdc8250_data *data;
+	struct device *dev = &pdev->dev;
+	struct resource *regs = platform_get_resource(pdev, IORESOURCE_IO, 0);
+	int irq;
+	unsigned long long exponent;
+	struct serial_rs485 rs485;
+
+	if (!regs) {
+		dev_err(dev, "no registers defined\n");
+		return -EINVAL;
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	ret = sdc8250_read_property(data, dev);
+	if (ret < 0) {
+		dev_err(dev, "failed to read property\n");
+		return ret;
+	}
+	data->device = dev;
+	data->regs = regs;
+	up = &data->uart;
+	p = &up->port;
+
+	data->fifosize = data->data.rx_fifo_size;
+	if (data->fifosize > data->data.tx_fifo_size)
+		data->fifosize = data->data.tx_fifo_size;
+
+	exponent = 1;
+	for (i = 0; i < data->data.exponent; i++)
+		exponent *= 10;
+	data->clk_rate = data->data.significand * exponent;
+
+	ret = config_rs422485(data, &rs485);
+	if (ret < 0)
+		goto err_do_nothing;
+
+	spin_lock_init(&p->lock);
+	p->iotype       = UPIO_PORT;
+	p->iobase       = regs->start;
+	p->mapbase      = 0;
+	p->membase      = NULL;
+	p->regshift     = 0;
+
+	p->irq          = irq;
+	p->type         = PORT_SUNIX;
+	p->flags        = UPF_SHARE_IRQ | UPF_FIXED_PORT | UPF_FIXED_TYPE;
+	p->fifosize     = data->fifosize;
+	p->dev          = dev;
+	p->uartclk      = data->clk_rate;
+	p->private_data = &data->data;
+
+	p->pm           = sdc8250_do_pm;
+	p->rs485_config = sdc8250_rs485_config;
+	memcpy(&p->rs485, &rs485, sizeof(rs485));
+
+	data->data.line = serial8250_register_8250_port(up);
+	if (data->data.line < 0) {
+		ret = data->data.line;
+		goto err_do_nothing;
+	}
+
+	ret = sdc8250_debugfs_add(data);
+	if (ret)
+		dev_warn(dev, "failed to create debugfs entries\n");
+
+	platform_set_drvdata(pdev, data);
+
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+	return 0;
+
+err_do_nothing:
+	dev_err(dev, "failed to register, error:%d\n", ret);
+	return ret;
+}
+
+static int sdc8250_remove(struct platform_device *pdev)
+{
+	struct sdc8250_data *data = platform_get_drvdata(pdev);
+	struct device *dev = &pdev->dev;
+
+	pm_runtime_get_sync(dev);
+
+	sdc8250_debugfs_remove(data);
+	serial8250_unregister_port(data->data.line);
+
+	pm_runtime_disable(dev);
+	pm_runtime_put_noidle(dev);
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int sdc8250_suspend(struct device *dev)
+{
+	struct sdc8250_data *data = dev_get_drvdata(dev);
+
+	// save umr
+	data->umr = inb(data->uart.port.iobase + UART_SDC_V0_UMER);
+
+	serial8250_suspend_port(data->data.line);
+	return 0;
+}
+
+static int sdc8250_resume(struct device *dev)
+{
+	struct sdc8250_data *data = dev_get_drvdata(dev);
+
+	// restore umr
+	outb(data->umr, data->uart.port.iobase + UART_SDC_V0_UMER);
+
+	serial8250_resume_port(data->data.line);
+	return 0;
+}
+#endif
+
+#ifdef CONFIG_PM
+static int sdc8250_runtime_suspend(struct device *dev)
+{
+	return 0;
+}
+
+static int sdc8250_runtime_resume(struct device *dev)
+{
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops sdc8250_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(sdc8250_suspend, sdc8250_resume)
+	SET_RUNTIME_PM_OPS(sdc8250_runtime_suspend,
+		sdc8250_runtime_resume, NULL)
+};
+
+static const struct of_device_id sdc8250_of_match[] = {
+	{ .compatible = "sunix,sdc_serial" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, sdc8250_of_match);
+
+static struct platform_driver sdc8250_platform_driver = {
+	.driver = {
+		.name = "8250_sdc",
+		.pm = &sdc8250_pm_ops,
+		.of_match_table = sdc8250_of_match,
+	},
+	.probe = sdc8250_probe,
+	.remove = sdc8250_remove,
+};
+
+static int __init sdc8250_init(void)
+{
+	sdc_serial_debugfs = debugfs_create_dir("sdc_serial", NULL);
+	platform_driver_register(&sdc8250_platform_driver);
+	return 0;
+}
+module_init(sdc8250_init);
+
+static void __exit sdc8250_exit(void)
+{
+	platform_driver_unregister(&sdc8250_platform_driver);
+	debugfs_remove(sdc_serial_debugfs);
+}
+module_exit(sdc8250_exit);
+
+MODULE_AUTHOR("Jason Lee <jason_lee@sunix.com>");
+MODULE_DESCRIPTION("SUNIX SDC serial port driver");
+MODULE_LICENSE("GPL");
+
+MODULE_ALIAS("platform:8250_sdc");
+
+
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] serial:Add SUNIX SDC serial port driver
  2021-05-28  9:23 [PATCH] serial:Add SUNIX SDC serial port driver Moriis Ku
@ 2021-05-28  9:27 ` Greg KH
  2021-06-02 13:36 ` Enrico Weigelt, metux IT consult
  1 sibling, 0 replies; 3+ messages in thread
From: Greg KH @ 2021-05-28  9:27 UTC (permalink / raw)
  To: Moriis Ku; +Cc: linux-serial, jason_lee, taian.chen

On Fri, May 28, 2021 at 05:23:07PM +0800, Moriis Ku wrote:
> From: Morris Ku <saumah@gmail.com>
> 
> SUNIX SDC serial port driver.
> 
> Cc: Jason Lee <jason_lee@sunix.com>
> Cc: Taian Chen <taian.chen@sunix.com>
> Signed-off-by: Morris Ku <saumah@gmail.com>
> ---
>  8250_sdc.c | 519 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 519 insertions(+)
>  create mode 100644 8250_sdc.c

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- You sent multiple patches, yet no indication of which ones should be
  applied in which order.  Greg could just guess, but if you are
  receiving this email, he guessed wrong and the patches didn't apply.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/SubmittingPatches for a description of how
  to do this so that Greg has a chance to apply these correctly.

- You did not specify a description of why the patch is needed, or
  possibly, any description at all, in the email body.  Please read the
  section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what is needed in order to
  properly describe the change.

- You did not write a descriptive Subject: for the patch, allowing Greg,
  and everyone else, to know what this patch is all about.  Please read
  the section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what a proper Subject: line should
  look like.

- This looks like a new version of a previously submitted patch, but you
  did not list below the --- line any changes from the previous version.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/SubmittingPatches for what needs to be done
  here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] serial:Add SUNIX SDC serial port driver
  2021-05-28  9:23 [PATCH] serial:Add SUNIX SDC serial port driver Moriis Ku
  2021-05-28  9:27 ` Greg KH
@ 2021-06-02 13:36 ` Enrico Weigelt, metux IT consult
  1 sibling, 0 replies; 3+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2021-06-02 13:36 UTC (permalink / raw)
  To: Moriis Ku, gregkh; +Cc: linux-serial, jason_lee, taian.chen

On 28.05.21 11:23, Moriis Ku wrote:

<snip>

> +static struct dentry *sdc_serial_debugfs;

Dead code if CONFIG_DEBUG_FS is disabled

> +static void sdc8250_do_pm(struct uart_port *p, unsigned int state,
> +				unsigned int old)
> +{
> +	if (!state)
> +		pm_runtime_get_sync(p->dev);
> +
> +	serial8250_do_pm(p, state, old);
> +
> +	if (state)
> +		pm_runtime_put_sync_suspend(p->dev);
> +}

Dead code if CONFIG_PM_SLEEP is disabled ?

> +static int sdc8250_debugfs_add(struct sdc8250_data *data)
> +{
> +	struct dentry *root_dir;
> +	char root_name[20];
> +
> +	memset(root_name, 0, sizeof(root_name));
> +	sprintf(root_name, "ttyS%d", data->data.line);
> +	root_dir = debugfs_create_dir(root_name, sdc_serial_debugfs);
> +	if (IS_ERR(root_dir))
> +		return PTR_ERR(root_dir);
> +
> +	debugfs_create_u32("bus_number", 0644, root_dir,
> +		&data->data.bus_number);
> +	debugfs_create_u32("device_number", 0644, root_dir,
> +		&data->data.device_number);
> +	debugfs_create_u32("irq", 0644, root_dir, &data->data.irq);
> +	debugfs_create_u8("number", 0644, root_dir, &data->data.number);
> +	debugfs_create_u8("version", 0644, root_dir, &data->data.version);
> +	debugfs_create_u8("resource_cap", 0644, root_dir,
> +		&data->data.resource_cap);
> +	debugfs_create_u8("event_type", 0644, root_dir,
> +		&data->data.event_type);
> +	debugfs_create_u16("tx_fifo_size", 0644, root_dir,
> +		&data->data.tx_fifo_size);
> +	debugfs_create_u16("rx_fifo_size", 0644, root_dir,
> +		&data->data.rx_fifo_size);
> +	debugfs_create_u32("significand", 0644, root_dir,
> +		&data->data.significand);
> +	debugfs_create_u8("exponent", 0644, root_dir,
> +		&data->data.exponent);
> +	debugfs_create_u8("rs232_cap", 0644, root_dir, &data->data.rs232_cap);
> +	debugfs_create_u8("rs422_cap", 0644, root_dir, &data->data.rs422_cap);
> +	debugfs_create_u8("rs485_cap", 0644, root_dir, &data->data.rs485_cap);
> +	debugfs_create_u8("ahdc_cap", 0644, root_dir, &data->data.ahdc_cap);
> +	debugfs_create_u8("cs_cap", 0644, root_dir, &data->data.cs_cap);
> +	debugfs_create_u8("rs422_end_cap", 0644, root_dir,
> +		&data->data.rs422_end_cap);
> +	debugfs_create_u8("rs485_end_cap", 0644, root_dir,
> +		&data->data.rs485_end_cap);
> +	debugfs_create_u32("line", 0644, root_dir, &data->data.line);
> +
> +	memset(data->device_name, 0, sizeof(data->device_name));
> +	sprintf(data->device_name, "%s", dev_name(data->device));
> +	data->device_name[strlen(data->device_name)] = 0x0a;

what exactly is this line feed for ?

> +	data->debugfs_blob_device_name.data = data->device_name;
> +	data->debugfs_blob_device_name.size = strlen(data->device_name) + 1;
> +	debugfs_create_blob("device_name", 0644, root_dir,
> +		&data->debugfs_blob_device_name);
> +
> +	data->debugfs = root_dir;
> +	return 0;
> +}

Can we make that optional (only if CONFIG_DEBUG_FS enabled) ?
Can we use devm_() functions ?

And why not using device attributes for that ?

> +static void sdc8250_debugfs_remove(struct sdc8250_data *data)
> +{
> +	debugfs_remove_recursive(data->debugfs);
> +}

See above: dead code if CONFIG_DEBUG_FS is disabled.

> +static int sdc8250_read_property(struct sdc8250_data *data,
> +				struct device *dev)
> +{
> +	int ret;
> +
> +	ret = device_property_read_u32(dev, "bus_number",
> +				&data->data.bus_number);
> +	if (ret < 0)
> +		return -EINVAL;
> +	ret = device_property_read_u32(dev, "device_number",
> +				&data->data.device_number);
> +	if (ret < 0)
> +		return -EINVAL;
> +	ret = device_property_read_u32(dev, "irq", &data->data.irq);
> +	if (ret < 0)
> +		return -EINVAL;
> +	ret = device_property_read_u8(dev, "number", &data->data.number);
> +	if (ret < 0)
> +		return -EINVAL;
> +	ret = device_property_read_u8(dev, "version", &data->data.version);
> +	if (ret < 0)
> +		return -EINVAL;
> +	ret = device_property_read_u8(dev, "resource_cap",
> +				&data->data.resource_cap);
> +	if (ret < 0)
> +		return -EINVAL;
> +	ret = device_property_read_u8(dev, "event_type",
> +				&data->data.event_type);
> +	if (ret < 0)
> +		return -EINVAL;
> +	ret = device_property_read_u16(dev, "tx_fifo_size",
> +				&data->data.tx_fifo_size);
> +	if (ret < 0)
> +		return -EINVAL;
> +	ret = device_property_read_u16(dev, "rx_fifo_size",
> +				&data->data.rx_fifo_size);
> +	if (ret < 0)
> +		return -EINVAL;
> +	ret = device_property_read_u32(dev, "significand",
> +				&data->data.significand);
> +	if (ret < 0)
> +		return -EINVAL;
> +	ret = device_property_read_u8(dev, "exponent",
> +				&data->data.exponent);
> +	if (ret < 0)
> +		return -EINVAL;
> +	ret = device_property_read_u8(dev, "rs232_cap", &data->data.rs232_cap);
> +	if (ret < 0)
> +		return -EINVAL;
> +	ret = device_property_read_u8(dev, "rs422_cap", &data->data.rs422_cap);
> +	if (ret < 0)
> +		return -EINVAL;
> +	ret = device_property_read_u8(dev, "rs485_cap", &data->data.rs485_cap);
> +	if (ret < 0)
> +		return -EINVAL;
> +	ret = device_property_read_u8(dev, "ahdc_cap", &data->data.ahdc_cap);
> +	if (ret < 0)
> +		return -EINVAL;
> +	ret = device_property_read_u8(dev, "cs_cap", &data->data.cs_cap);
> +	if (ret < 0)
> +		return -EINVAL;
> +	ret = device_property_read_u8(dev, "rs422_end_cap",
> +				&data->data.rs422_end_cap);
> +	if (ret < 0)
> +		return -EINVAL;
> +	ret = device_property_read_u8(dev, "rs485_end_cap",
> +				&data->data.rs485_end_cap);
> +	if (ret < 0)
> +		return -EINVAL;
> +
> +	return 0;
> +}

Is there some device tree specification for that ?

> +static struct platform_driver sdc8250_platform_driver = {
> +	.driver = {
> +		.name = "8250_sdc",
> +		.pm = &sdc8250_pm_ops,
> +		.of_match_table = sdc8250_of_match,

use of_match_ptr()

> +	},
> +	.probe = sdc8250_probe,
> +	.remove = sdc8250_remove,
> +};
> +
> +static int __init sdc8250_init(void)
> +{
> +	sdc_serial_debugfs = debugfs_create_dir("sdc_serial", NULL);
> +	platform_driver_register(&sdc8250_platform_driver);
> +	return 0;
> +}
> +module_init(sdc8250_init);
> +
> +static void __exit sdc8250_exit(void)
> +{
> +	platform_driver_unregister(&sdc8250_platform_driver);
> +	debugfs_remove(sdc_serial_debugfs);
> +}
> +module_exit(sdc8250_exit);
> +
> +MODULE_AUTHOR("Jason Lee <jason_lee@sunix.com>");
> +MODULE_DESCRIPTION("SUNIX SDC serial port driver");
> +MODULE_LICENSE("GPL");
> +
> +MODULE_ALIAS("platform:8250_sdc");
> +
> +
> 

-- 
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-06-02 13:37 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-28  9:23 [PATCH] serial:Add SUNIX SDC serial port driver Moriis Ku
2021-05-28  9:27 ` Greg KH
2021-06-02 13:36 ` Enrico Weigelt, metux IT consult

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.