* [PATCH] mfd:Add SUNIX mfd & PCIe driver
@ 2021-05-28 9:58 Moriis Ku
2021-05-28 12:26 ` Andy Shevchenko
2021-05-28 18:26 ` Lee Jones
0 siblings, 2 replies; 3+ messages in thread
From: Moriis Ku @ 2021-05-28 9:58 UTC (permalink / raw)
To: lee.jones; +Cc: linux-kernel, jason_lee, taian.chen, Morris Ku
From: Morris Ku <saumah@gmail.com>
Add SUNIX mfd & PCIe driver
Cc: Jason Lee <jason_lee@sunix.com>
Cc: Taian Chen <taian.chen@sunix.com>
Signed-off-by: Morris Ku <saumah@gmail.com>
---
sdc_mfd.c | 513 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
sdc_mfd.h | 61 +++++++
sdc_pci.c | 92 ++++++++++
3 files changed, 666 insertions(+)
create mode 100644 sdc_mfd.c
create mode 100644 sdc_mfd.h
create mode 100644 sdc_pci.c
diff --git a/driver/mfd/sdc_mfd.c b/driver/mfd/sdc_mfd.c
new file mode 100644
index 0000000..d0acd54
--- /dev/null
+++ b/driver/mfd/sdc_mfd.c
@@ -0,0 +1,513 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * SUNIX SDC mfd driver.
+ *
+ * Copyright (C) 2021, SUNIX Co., Ltd.
+ *
+ * Based on Intel Sunrisepoint LPSS core driver written by
+ * - Andy Shevchenko <andriy.shevchenko@linux.intel.com>
+ * - Mika Westerberg <mika.westerberg@linux.intel.com>
+ * - Heikki Krogerus <heikki.krogerus@linux.intel.com>
+ * - Jarkko Nikula <jarkko.nikula@linux.intel.com>
+ * Copyright (C) 2015, Intel Corporation
+ */
+
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/pci.h>
+#include <linux/property.h>
+#include <linux/debugfs.h>
+#include <linux/idr.h>
+#include <linux/mfd/core.h>
+#include "sdc_mfd.h"
+
+struct cib_config {
+ unsigned int mem_offset;
+ unsigned int mem_size;
+ unsigned char ic_brand;
+ unsigned char ic_model;
+};
+
+struct cib_uart {
+ unsigned int io_offset;
+ unsigned char io_size;
+ unsigned int mem_offset;
+ unsigned int mem_size;
+ 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;
+};
+
+struct cib_info {
+ unsigned char number;
+ unsigned char type;
+ unsigned char version;
+ unsigned char total_length;
+ unsigned char resource_cap;
+ unsigned char event_type;
+
+ struct cib_config *config;
+ struct cib_uart *uart;
+};
+
+struct sdc_channel {
+ struct cib_info info;
+
+ struct property_entry *property;
+ struct resource *resource;
+ struct mfd_cell *cell;
+};
+
+struct sdc_mfd {
+ struct sdc_platform_info info;
+
+ unsigned char major_version;
+ unsigned char minor_version;
+ unsigned char available_chls;
+ unsigned char total_length;
+ char model_name[18];
+
+ struct sdc_channel *channels;
+
+ struct device *dev;
+ int devid;
+
+ struct dentry *debugfs;
+ struct debugfs_blob_wrapper debugfs_blob_model_name;
+};
+
+static DEFINE_IDA(sdc_devid_ida);
+static struct dentry *sdc_mfd_debugfs;
+static int sdc_serial_id = 1;
+
+static void sdc_get_config_info(struct cib_info *info, void __iomem *membase,
+ unsigned short ptr)
+{
+ unsigned int temp;
+ unsigned int offsetDW = 0;
+
+ if (!info || !info->config)
+ return;
+
+ offsetDW = 2;
+ info->config->mem_offset = readl(membase + ptr + (offsetDW * 4));
+ offsetDW = 3;
+ info->config->mem_size = readl(membase + ptr + (offsetDW * 4));
+ offsetDW = 4;
+ temp = readl(membase + ptr + (offsetDW * 4));
+ info->config->ic_brand = (unsigned char)((temp & 0x0000ff00) >> 8);
+ info->config->ic_model = (unsigned char)((temp & 0x00ff0000) >> 16);
+}
+
+static void sdc_get_uart_info(struct cib_info *info, void __iomem *membase,
+ unsigned short ptr)
+{
+ unsigned int temp;
+ unsigned int offsetDW = 0;
+
+ if (!info || !info->uart)
+ return;
+
+ offsetDW = 2;
+ temp = readl(membase + ptr + (offsetDW * 4));
+ info->uart->io_offset = (unsigned int)((temp & 0x00ffffff));
+ info->uart->io_size = (unsigned char)((temp & 0xff000000) >> 24);
+ offsetDW = 3;
+ info->uart->mem_offset = readl(membase + ptr + (offsetDW * 4));
+ offsetDW = 4;
+ info->uart->mem_size = readl(membase + ptr + (offsetDW * 4));
+ offsetDW = 5;
+ temp = readl(membase + ptr + (offsetDW * 4));
+ info->uart->tx_fifo_size = (unsigned short)((temp & 0x0000ffff));
+ info->uart->rx_fifo_size = (unsigned short)((temp & 0xff0000) >> 16);
+ offsetDW = 6;
+ temp = readl(membase + ptr + (offsetDW * 4));
+ info->uart->significand = temp & 0x00ffffff;
+ info->uart->exponent =
+ (unsigned char)((temp & 0xff000000) >> 24);
+ offsetDW = 7;
+ temp = readl(membase + ptr + (offsetDW * 4));
+ info->uart->rs232_cap = (temp & 0x00000001) ? 0x01 : 0x00;
+ info->uart->rs422_cap = (temp & 0x00000002) ? 0x01 : 0x00;
+ info->uart->rs485_cap = (temp & 0x00000004) ? 0x01 : 0x00;
+ info->uart->ahdc_cap = (temp & 0x00000008) ? 0x01 : 0x00;
+ info->uart->cs_cap = (temp & 0x00000010) ? 0x01 : 0x00;
+ info->uart->rs422_end_cap = (temp & 0x00000040) ? 0x01 : 0x00;
+ info->uart->rs485_end_cap = (temp & 0x00000080) ? 0x01 : 0x00;
+}
+
+static int sdc_debugfs_add(struct sdc_mfd *sdc)
+{
+ struct dentry *root_dir;
+ struct dentry *chl_dir;
+ char chl_name[20];
+ struct sdc_channel *chl = NULL;
+ int i;
+
+ root_dir = debugfs_create_dir(dev_name(sdc->dev), sdc_mfd_debugfs);
+ if (IS_ERR(root_dir))
+ return PTR_ERR(root_dir);
+
+ debugfs_create_u32("devid", 0644, root_dir, &sdc->devid);
+ debugfs_create_u32("bus_number", 0644, root_dir, &sdc->info.bus_number);
+ debugfs_create_u32("device_number", 0644, root_dir,
+ &sdc->info.device_number);
+ debugfs_create_u32("irq", 0644, root_dir, &sdc->info.irq);
+ debugfs_create_u8("major_version", 0644, root_dir, &sdc->major_version);
+ debugfs_create_u8("minor_version", 0644, root_dir,
+ &sdc->minor_version);
+ debugfs_create_u8("available_chls", 0644, root_dir,
+ &sdc->available_chls);
+ sdc->debugfs_blob_model_name.data = sdc->model_name;
+ sdc->debugfs_blob_model_name.size = strlen(sdc->model_name) + 1;
+ debugfs_create_blob("model_name", 0644, root_dir,
+ &sdc->debugfs_blob_model_name);
+
+ for (i = 0; i < sdc->available_chls; i++) {
+ chl = &sdc->channels[i];
+ memset(chl_name, 0, sizeof(char) * 20);
+ sprintf(chl_name, "chl%d", i);
+ chl_dir = debugfs_create_dir(chl_name, root_dir);
+
+ if (!chl_dir) {
+ dev_warn(sdc->dev, "create chl %d debugfs fail\n", i);
+ continue;
+ }
+ debugfs_create_x8("number", 0644, chl_dir, &chl->info.number);
+ debugfs_create_x8("type", 0644, chl_dir, &chl->info.type);
+ debugfs_create_x8("version", 0644, chl_dir, &chl->info.version);
+ debugfs_create_x8("total_length", 0644, chl_dir,
+ &chl->info.total_length);
+ debugfs_create_x8("resource_cap", 0644, chl_dir,
+ &chl->info.resource_cap);
+ debugfs_create_x8("event_type", 0644, chl_dir,
+ &chl->info.event_type);
+
+ switch (chl->info.type) {
+ case 0x00:
+ if (!chl->info.config)
+ break;
+ debugfs_create_x32("mem_offset", 0644, chl_dir,
+ &chl->info.config->mem_offset);
+ debugfs_create_x32("mem_size", 0644, chl_dir,
+ &chl->info.config->mem_size);
+ debugfs_create_x8("ic_brand", 0644, chl_dir,
+ &chl->info.config->ic_brand);
+ debugfs_create_x8("ic_model", 0644, chl_dir,
+ &chl->info.config->ic_model);
+ break;
+
+ case 0x01:
+ if (!chl->info.uart)
+ break;
+ debugfs_create_x32("io_offset", 0644, chl_dir,
+ &chl->info.uart->io_offset);
+ debugfs_create_x8("io_size", 0644, chl_dir,
+ &chl->info.uart->io_size);
+ debugfs_create_x32("mem_offset", 0644, chl_dir,
+ &chl->info.uart->mem_offset);
+ debugfs_create_x32("mem_size", 0644, chl_dir,
+ &chl->info.uart->mem_size);
+ debugfs_create_x16("tx_fifo_size", 0644, chl_dir,
+ &chl->info.uart->tx_fifo_size);
+ debugfs_create_x16("rx_fifo_size", 0644, chl_dir,
+ &chl->info.uart->rx_fifo_size);
+ debugfs_create_x32("significand", 0644, chl_dir,
+ &chl->info.uart->significand);
+ debugfs_create_x8("exponent", 0644, chl_dir,
+ &chl->info.uart->exponent);
+ debugfs_create_x8("rs232_cap", 0644, chl_dir,
+ &chl->info.uart->rs232_cap);
+ debugfs_create_x8("rs422_cap", 0644, chl_dir,
+ &chl->info.uart->rs422_cap);
+ debugfs_create_x8("rs485_cap", 0644, chl_dir,
+ &chl->info.uart->rs485_cap);
+ debugfs_create_x8("ahdc_cap", 0644, chl_dir,
+ &chl->info.uart->ahdc_cap);
+ debugfs_create_x8("cs_cap", 0644, chl_dir,
+ &chl->info.uart->cs_cap);
+ debugfs_create_x8("rs422_end_cap", 0644, chl_dir,
+ &chl->info.uart->rs422_end_cap);
+ debugfs_create_x8("rs485_end_cap", 0644, chl_dir,
+ &chl->info.uart->rs485_end_cap);
+ break;
+ }
+ }
+
+ sdc->debugfs = root_dir;
+ return 0;
+}
+
+static void sdc_debugfs_remove(struct sdc_mfd *sdc)
+{
+ debugfs_remove_recursive(sdc->debugfs);
+}
+
+int sdc_probe(struct device *dev, struct sdc_platform_info *info)
+{
+ int ret;
+ int i;
+ int j;
+ int prop_index;
+ struct sdc_mfd *sdc = NULL;
+ unsigned int temp;
+ struct sdc_channel *chl = NULL;
+ unsigned short next_cib_ptr = 0;
+ unsigned short next_cib_ptr_backup = 0;
+ unsigned long bar1_io;
+ void __iomem *bar2_mem;
+ unsigned long bar2_length;
+
+ if (!info || !info->pdev || info->irq <= 0)
+ return -EINVAL;
+
+ sdc = devm_kzalloc(dev, sizeof(*sdc), GFP_KERNEL);
+ if (!sdc)
+ return -ENOMEM;
+
+ sdc->info.pdev = info->pdev;
+ sdc->info.bus_number = info->bus_number;
+ sdc->info.device_number = info->device_number;
+ sdc->info.irq = info->irq;
+
+ bar1_io = pci_resource_start(info->pdev, 1);
+
+ bar2_length = pci_resource_len(info->pdev, 2);
+ bar2_mem = devm_ioremap(dev, pci_resource_start(info->pdev, 2),
+ bar2_length);
+ if (!bar2_mem)
+ return -ENOMEM;
+
+ temp = readl(bar2_mem);
+ sdc->major_version = (unsigned char)((temp & 0x000000ff));
+ sdc->minor_version = (unsigned char)((temp & 0x0000ff00) >> 8);
+ sdc->available_chls = (unsigned char)((temp & 0x00ff0000) >> 16);
+ sdc->total_length = (unsigned char)((temp & 0xff000000) >> 24);
+
+ temp = readl(bar2_mem + 4);
+ next_cib_ptr = next_cib_ptr_backup =
+ (unsigned short)((temp & 0x0000ffff));
+
+ j = 0;
+ for (i = 0; i < 4; i++) {
+ temp = readl(bar2_mem + 8 + (i * 4));
+ sdc->model_name[j++] = (char)((temp & 0x000000ff));
+ sdc->model_name[j++] = (char)((temp & 0x0000ff00) >> 8);
+ sdc->model_name[j++] = (char)((temp & 0x00ff0000) >> 16);
+ sdc->model_name[j++] = (char)((temp & 0xff000000) >> 24);
+ }
+ sdc->model_name[strlen(sdc->model_name)] = 0x0a;
+
+ sdc->channels = devm_kzalloc(dev, sizeof(struct sdc_channel) *
+ sdc->available_chls, GFP_KERNEL);
+ if (!sdc->channels)
+ return -ENOMEM;
+
+ for (i = 0; i < sdc->available_chls; i++) {
+ chl = &sdc->channels[i];
+
+ next_cib_ptr_backup = next_cib_ptr;
+
+ temp = readl(bar2_mem + next_cib_ptr);
+ chl->info.number = (unsigned char)((temp & 0x000000ff));
+ chl->info.type = (unsigned char)((temp & 0x0000ff00) >> 8);
+ chl->info.version = (unsigned char)((temp & 0x00ff0000) >> 16);
+ chl->info.total_length =
+ (unsigned char)((temp & 0xff000000) >> 24);
+
+ temp = readl(bar2_mem + next_cib_ptr + 4);
+ next_cib_ptr = temp & 0x0000ffff;
+ chl->info.resource_cap =
+ (unsigned char)((temp & 0x00ff0000) >> 16);
+ chl->info.event_type =
+ (unsigned char)((temp & 0xff000000) >> 24);
+
+ switch (chl->info.type) {
+ case 0x00:
+ chl->info.config = devm_kzalloc(dev,
+ sizeof(struct cib_config), GFP_KERNEL);
+ if (!chl->info.config)
+ return -ENOMEM;
+ sdc_get_config_info(&chl->info, bar2_mem,
+ next_cib_ptr_backup);
+ break;
+
+ case 0x01:
+ chl->info.uart = devm_kzalloc(dev,
+ sizeof(struct cib_uart), GFP_KERNEL);
+ if (!chl->info.uart)
+ return -ENOMEM;
+ sdc_get_uart_info(&chl->info, bar2_mem,
+ next_cib_ptr_backup);
+
+ chl->property = devm_kzalloc(dev,
+ sizeof(struct property_entry) * 18, GFP_KERNEL);
+ if (!chl->property)
+ return -ENOMEM;
+ prop_index = 0;
+ chl->property[prop_index++] = PROPERTY_ENTRY_U32(
+ "bus_number", sdc->info.bus_number);
+ chl->property[prop_index++] = PROPERTY_ENTRY_U32(
+ "device_number", sdc->info.device_number);
+ chl->property[prop_index++] = PROPERTY_ENTRY_U32(
+ "irq", sdc->info.irq);
+ chl->property[prop_index++] = PROPERTY_ENTRY_U8(
+ "number", chl->info.number);
+ chl->property[prop_index++] = PROPERTY_ENTRY_U8(
+ "version", chl->info.version);
+ chl->property[prop_index++] = PROPERTY_ENTRY_U8(
+ "resource_cap", chl->info.resource_cap);
+ chl->property[prop_index++] = PROPERTY_ENTRY_U8(
+ "event_type", chl->info.event_type);
+ chl->property[prop_index++] = PROPERTY_ENTRY_U16(
+ "tx_fifo_size", chl->info.uart->tx_fifo_size);
+ chl->property[prop_index++] = PROPERTY_ENTRY_U16(
+ "rx_fifo_size", chl->info.uart->rx_fifo_size);
+ chl->property[prop_index++] = PROPERTY_ENTRY_U32(
+ "significand", chl->info.uart->significand);
+ chl->property[prop_index++] = PROPERTY_ENTRY_U8(
+ "exponent", chl->info.uart->exponent);
+ chl->property[prop_index++] = PROPERTY_ENTRY_U8(
+ "rs232_cap", chl->info.uart->rs232_cap);
+ chl->property[prop_index++] = PROPERTY_ENTRY_U8(
+ "rs422_cap", chl->info.uart->rs422_cap);
+ chl->property[prop_index++] = PROPERTY_ENTRY_U8(
+ "rs485_cap", chl->info.uart->rs485_cap);
+ chl->property[prop_index++] = PROPERTY_ENTRY_U8(
+ "ahdc_cap", chl->info.uart->ahdc_cap);
+ chl->property[prop_index++] = PROPERTY_ENTRY_U8(
+ "cs_cap", chl->info.uart->cs_cap);
+ chl->property[prop_index++] = PROPERTY_ENTRY_U8(
+ "rs422_end_cap", chl->info.uart->rs422_end_cap);
+ chl->property[prop_index++] = PROPERTY_ENTRY_U8(
+ "rs485_end_cap", chl->info.uart->rs485_end_cap);
+
+ chl->resource = devm_kzalloc(dev,
+ sizeof(struct resource) * 2, GFP_KERNEL);
+ if (!chl->resource)
+ return -ENOMEM;
+ chl->resource[0].start =
+ bar1_io + chl->info.uart->io_offset;
+ chl->resource[0].end =
+ bar1_io + chl->info.uart->io_offset +
+ chl->info.uart->io_size - 1;
+ chl->resource[0].name = "iobase";
+ chl->resource[0].flags = IORESOURCE_IO;
+ chl->resource[0].desc = IORES_DESC_NONE;
+ chl->resource[1].start = 0;
+ chl->resource[1].end = 0;
+ chl->resource[1].name = "irq";
+ chl->resource[1].flags = IORESOURCE_IRQ;
+ chl->resource[1].desc = IORES_DESC_NONE;
+
+ chl->cell = devm_kzalloc(dev,
+ sizeof(struct mfd_cell), GFP_KERNEL);
+ if (!chl->cell)
+ return -ENOMEM;
+ chl->cell->name = "8250_sdc";
+ chl->cell->id = sdc_serial_id++;
+ chl->cell->properties = chl->property;
+ chl->cell->num_resources = 2;
+ chl->cell->resources = chl->resource;
+ break;
+ }
+ }
+
+ sdc->dev = dev;
+ dev_set_drvdata(dev, sdc);
+
+ ret = ida_simple_get(&sdc_devid_ida, 0, 0, GFP_KERNEL);
+ if (ret < 0)
+ return ret;
+ sdc->devid = ret;
+
+ ret = sdc_debugfs_add(sdc);
+ if (ret)
+ dev_warn(dev, "failed to create debugfs entries\n");
+
+ for (i = 0; i < sdc->available_chls; i++) {
+ chl = &sdc->channels[i];
+
+ if (chl->cell) {
+ ret = mfd_add_devices(dev, sdc->devid, chl->cell, 1,
+ NULL, sdc->info.irq, NULL);
+ if (ret)
+ goto err_remove_sdc;
+ }
+ }
+
+ dev_pm_set_driver_flags(dev, DPM_FLAG_SMART_SUSPEND);
+ return 0;
+
+err_remove_sdc:
+ sdc_debugfs_remove(sdc);
+ ida_simple_remove(&sdc_devid_ida, sdc->devid);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(sdc_probe);
+
+void sdc_remove(struct device *dev)
+{
+ struct sdc_mfd *sdc = dev_get_drvdata(dev);
+
+ mfd_remove_devices(dev);
+ sdc_debugfs_remove(sdc);
+ ida_simple_remove(&sdc_devid_ida, sdc->devid);
+}
+EXPORT_SYMBOL_GPL(sdc_remove);
+
+static int resume_sdc_device(struct device *dev, void *data)
+{
+ if (!dev_pm_test_driver_flags(dev, DPM_FLAG_SMART_SUSPEND))
+ pm_runtime_resume(dev);
+
+ return 0;
+}
+
+int sdc_prepare(struct device *dev)
+{
+ device_for_each_child_reverse(dev, NULL, resume_sdc_device);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(sdc_prepare);
+
+int sdc_suspend(struct device *dev)
+{
+ // save context
+ return 0;
+}
+EXPORT_SYMBOL_GPL(sdc_suspend);
+
+int sdc_resume(struct device *dev)
+{
+ // restore context
+ return 0;
+}
+EXPORT_SYMBOL_GPL(sdc_resume);
+
+static int __init sdc_init(void)
+{
+ sdc_mfd_debugfs = debugfs_create_dir("sdc_mfd", NULL);
+ return 0;
+}
+module_init(sdc_init);
+
+static void __exit sdc_exit(void)
+{
+ ida_destroy(&sdc_devid_ida);
+ debugfs_remove(sdc_mfd_debugfs);
+}
+module_exit(sdc_exit);
+
+MODULE_AUTHOR("Jason Lee <jason_lee@sunix.com>");
+MODULE_DESCRIPTION("SUNIX SDC mfd driver");
+MODULE_LICENSE("GPL v2");
+MODULE_SOFTDEP("pre: 8250_sdc");
diff --git a/sdc_mfd.h b/sdc_mfd.h
new file mode 100644
index 0000000..099912f
--- /dev/null
+++ b/sdc_mfd.h
@@ -0,0 +1,61 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * SUNIX SDC mfd driver.
+ *
+ * Copyright (C) 2021, SUNIX Co., Ltd.
+ *
+ * Based on Intel Sunrisepoint LPSS core driver written by
+ * - Andy Shevchenko <andriy.shevchenko@linux.intel.com>
+ * - Mika Westerberg <mika.westerberg@linux.intel.com>
+ * - Heikki Krogerus <heikki.krogerus@linux.intel.com>
+ * - Jarkko Nikula <jarkko.nikula@linux.intel.com>
+ * Copyright (C) 2015, Intel Corporation
+ */
+
+#ifndef __SDC_MFD_H
+#define __SDC_MFD_H
+
+#include <linux/pm.h>
+
+struct device;
+struct resource;
+struct property_entry;
+
+struct sdc_platform_info {
+ struct pci_dev *pdev;
+ int bus_number;
+ int device_number;
+ int irq;
+};
+
+int sdc_probe(struct device *dev, struct sdc_platform_info *info);
+void sdc_remove(struct device *dev);
+
+#ifdef CONFIG_PM
+int sdc_prepare(struct device *dev);
+int sdc_suspend(struct device *dev);
+int sdc_resume(struct device *dev);
+
+#ifdef CONFIG_PM_SLEEP
+#define SDC_SLEEP_PM_OPS \
+ .prepare = sdc_prepare, \
+ SET_LATE_SYSTEM_SLEEP_PM_OPS(sdc_suspend, sdc_resume)
+#else
+#define SDC_SLEEP_PM_OPS
+#endif // CONFIG_PM_SLEEP
+
+#define SDC_RUNTIME_PM_OPS \
+ .runtime_suspend = sdc_suspend, \
+ .runtime_resume = sdc_resume,
+#else // !CONFIG_PM
+#define SDC_SLEEP_PM_OPS
+#define SDC_RUNTIME_PM_OPS
+#endif // CONFIG_PM
+
+#define SDC_PM_OPS(name) \
+const struct dev_pm_ops name = { \
+ SDC_SLEEP_PM_OPS \
+ SDC_RUNTIME_PM_OPS \
+}
+
+#endif // __SDC_MFD_H
diff --git a/sdc_pci.c b/sdc_pci.c
new file mode 100644
index 0000000..6366d0e
--- /dev/null
+++ b/sdc_pci.c
@@ -0,0 +1,92 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * SUNIX SDC PCIe driver.
+ *
+ * Copyright (C) 2021, SUNIX Co., Ltd.
+ *
+ * Based on Intel Sunrisepoint LPSS PCI driver written by
+ * - Andy Shevchenko <andriy.shevchenko@linux.intel.com>
+ * - Mika Westerberg <mika.westerberg@linux.intel.com>
+ * Copyright (C) 2015, Intel Corporation
+ */
+
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/pci.h>
+#include "sdc_mfd.h"
+
+static int sdc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+ int ret;
+ struct sdc_platform_info info;
+ unsigned long flags;
+
+ ret = pcim_enable_device(pdev);
+ if (ret)
+ return ret;
+
+ flags = pci_resource_flags(pdev, 0);
+ if (!(flags & IORESOURCE_MEM)) {
+ pr_err("bar0 resource flag x%lx invalid\n", flags);
+ return -ENODEV;
+ }
+
+ flags = pci_resource_flags(pdev, 1);
+ if (!(flags & IORESOURCE_IO)) {
+ pr_err("bar1 resource flag x%lx invalid\n", flags);
+ return -ENODEV;
+ }
+
+ flags = pci_resource_flags(pdev, 2);
+ if (!(flags & IORESOURCE_MEM)) {
+ pr_err("bar2 resource flag x%lx invalid\n", flags);
+ return -ENODEV;
+ }
+
+ memset(&info, 0, sizeof(info));
+ info.pdev = pdev;
+ info.bus_number = pdev->bus->number;
+ info.device_number = PCI_SLOT(pdev->devfn);
+ info.irq = pdev->irq;
+
+ ret = sdc_probe(&pdev->dev, &info);
+ if (ret)
+ return ret;
+
+ pm_runtime_put(&pdev->dev);
+ pm_runtime_allow(&pdev->dev);
+
+ return 0;
+}
+
+static void sdc_pci_remove(struct pci_dev *pdev)
+{
+ pm_runtime_forbid(&pdev->dev);
+ pm_runtime_get_sync(&pdev->dev);
+
+ sdc_remove(&pdev->dev);
+}
+
+static SDC_PM_OPS(sdc_pci_pm_ops);
+
+static const struct pci_device_id sdc_pci_ids[] = {
+ { PCI_VDEVICE(SUNIX, 0x2000) },
+ { },
+};
+MODULE_DEVICE_TABLE(pci, sdc_pci_ids);
+
+static struct pci_driver sdc_pci_driver = {
+ .name = "sdc_pci",
+ .id_table = sdc_pci_ids,
+ .probe = sdc_pci_probe,
+ .remove = sdc_pci_remove,
+ .driver = {
+ .pm = &sdc_pci_pm_ops,
+ },
+};
+module_pci_driver(sdc_pci_driver);
+
+MODULE_AUTHOR("Jason Lee <jason_lee@sunix.com>");
+MODULE_DESCRIPTION("SUNIX SDC PCIe driver");
+MODULE_LICENSE("GPL v2");
+MODULE_SOFTDEP("pre: sdc_mfd");
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] mfd:Add SUNIX mfd & PCIe driver
2021-05-28 9:58 [PATCH] mfd:Add SUNIX mfd & PCIe driver Moriis Ku
@ 2021-05-28 12:26 ` Andy Shevchenko
2021-05-28 18:26 ` Lee Jones
1 sibling, 0 replies; 3+ messages in thread
From: Andy Shevchenko @ 2021-05-28 12:26 UTC (permalink / raw)
To: Moriis Ku; +Cc: Lee Jones, Linux Kernel Mailing List, jason_lee, taian.chen
On Fri, May 28, 2021 at 2:44 PM Moriis Ku <saumah@gmail.com> wrote:
>
> From: Morris Ku <saumah@gmail.com>
Not sure why we have it here.
> Add SUNIX mfd & PCIe driver
Needs a bit more information.
> Signed-off-by: Morris Ku <saumah@gmail.com>
...
> + * Based on Intel Sunrisepoint LPSS core driver written by
Looking into the code I'm not sure how it's based on that.
> + * - Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> + * - Mika Westerberg <mika.westerberg@linux.intel.com>
> + * - Heikki Krogerus <heikki.krogerus@linux.intel.com>
> + * - Jarkko Nikula <jarkko.nikula@linux.intel.com>
> + * Copyright (C) 2015, Intel Corporation
...
> +struct cib_config {
> + unsigned int mem_offset;
> + unsigned int mem_size;
The above is probably better when they are type of resource_size_t.
If it's something that goes directly to the hardware or from it, use
fixed-width types.
> + unsigned char ic_brand;
> + unsigned char ic_model;
u8 ?
> +};
> +
> +struct cib_uart {
> + unsigned int io_offset;
> + unsigned char io_size;
> + unsigned int mem_offset;
> + unsigned int mem_size;
> + 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;
> +};
As per above
> +struct cib_info {
> + unsigned char number;
> + unsigned char type;
> + unsigned char version;
> + unsigned char total_length;
> + unsigned char resource_cap;
> + unsigned char event_type;
> +
> + struct cib_config *config;
> + struct cib_uart *uart;
> +};
As per above.
...
> + char model_name[18];
Strange size, but okay.
...
> + offsetDW = 2;
> + info->config->mem_offset = readl(membase + ptr + (offsetDW * 4));
> + offsetDW = 3;
> + info->config->mem_size = readl(membase + ptr + (offsetDW * 4));
> + offsetDW = 4;
> + temp = readl(membase + ptr + (offsetDW * 4));
You will benefit if you create the IO accessor helper functions, these
become like
a = my_ioread(base, ptr, offset);
b = ...
...
> + info->config->ic_brand = (unsigned char)((temp & 0x0000ff00) >> 8);
> + info->config->ic_model = (unsigned char)((temp & 0x00ff0000) >> 16);
Use GENMASK().
Castings are not needed.
> +}
...
> +static void sdc_get_uart_info(struct cib_info *info, void __iomem *membase,
> + unsigned short ptr)
> +{
Same as above.
> + info->uart->rs232_cap = (temp & 0x00000001) ? 0x01 : 0x00;
> + info->uart->rs422_cap = (temp & 0x00000002) ? 0x01 : 0x00;
> + info->uart->rs485_cap = (temp & 0x00000004) ? 0x01 : 0x00;
> + info->uart->ahdc_cap = (temp & 0x00000008) ? 0x01 : 0x00;
> + info->uart->cs_cap = (temp & 0x00000010) ? 0x01 : 0x00;
> + info->uart->rs422_end_cap = (temp & 0x00000040) ? 0x01 : 0x00;
> + info->uart->rs485_end_cap = (temp & 0x00000080) ? 0x01 : 0x00;
It seems you are using char type for boolean variables.
Also consider BIT() to be in use.
> +}
...
> + root_dir = debugfs_create_dir(dev_name(sdc->dev), sdc_mfd_debugfs);
> + if (IS_ERR(root_dir))
> + return PTR_ERR(root_dir);
Nope, we don't check for error codes for debugfs.
...
> + debugfs_create_u32("device_number", 0644, root_dir,
> + &sdc->info.device_number);
> + debugfs_create_u8("minor_version", 0644, root_dir,
> + &sdc->minor_version);
> + debugfs_create_u8("available_chls", 0644, root_dir,
> + &sdc->available_chls);
Above can sit on a single line each.
...
> + for (i = 0; i < sdc->available_chls; i++) {
> + chl = &sdc->channels[i];
> + memset(chl_name, 0, sizeof(char) * 20);
Why?
> + sprintf(chl_name, "chl%d", i);
> + chl_dir = debugfs_create_dir(chl_name, root_dir);
> +
Redundant empty line.
> + if (!chl_dir) {
> + dev_warn(sdc->dev, "create chl %d debugfs fail\n", i);
Message with no value.
> + continue;
> + }
...
> + switch (chl->info.type) {
The default case is missing.
> + }
> + }
...
> + sdc->debugfs = root_dir;
Do we really need this? Debufs API can lookup for the folders with
predefined names.
...
> + int ret;
> + int i;
> + int j;
> + int prop_index;
> + struct sdc_mfd *sdc = NULL;
> + unsigned int temp;
> + struct sdc_channel *chl = NULL;
> + unsigned short next_cib_ptr = 0;
> + unsigned short next_cib_ptr_backup = 0;
> + unsigned long bar1_io;
> + void __iomem *bar2_mem;
> + unsigned long bar2_length;
Reverse xmas tree order, please.
...
> + bar2_length = pci_resource_len(info->pdev, 2);
> + bar2_mem = devm_ioremap(dev, pci_resource_start(info->pdev, 2),
> + bar2_length);
> + if (!bar2_mem)
> + return -ENOMEM;
Why can't you use pcim_iomap_regions()?
...
> + sdc->major_version = (unsigned char)((temp & 0x000000ff));
> + sdc->minor_version = (unsigned char)((temp & 0x0000ff00) >> 8);
> + sdc->available_chls = (unsigned char)((temp & 0x00ff0000) >> 16);
> + sdc->total_length = (unsigned char)((temp & 0xff000000) >> 24);
> + next_cib_ptr = next_cib_ptr_backup =
> + (unsigned short)((temp & 0x0000ffff));
GENMASK(), no castings.
Same for other similar places.
...
> + sdc->model_name[strlen(sdc->model_name)] = 0x0a;
Use '\n'
...
> + sdc->channels = devm_kzalloc(dev, sizeof(struct sdc_channel) *
> + sdc->available_chls, GFP_KERNEL);
devm_kcalloc()
> + if (!sdc->channels)
> + return -ENOMEM;
...
> + chl->info.config = devm_kzalloc(dev,
> + sizeof(struct cib_config), GFP_KERNEL);
Safer pattern is sizeof(*chl->info.config).
> + if (!chl->info.config)
> + return -ENOMEM;
...
> + chl->property = devm_kzalloc(dev,
> + sizeof(struct property_entry) * 18, GFP_KERNEL);
devm_kcalloc()
> + if (!chl->property)
> + return -ENOMEM;
...
> + chl->property[prop_index++] = PROPERTY_ENTRY_U32(
> + "bus_number", sdc->info.bus_number);
> + chl->property[prop_index++] = PROPERTY_ENTRY_U32(
> + "device_number", sdc->info.device_number);
> + chl->property[prop_index++] = PROPERTY_ENTRY_U32(
> + "irq", sdc->info.irq);
> + chl->property[prop_index++] = PROPERTY_ENTRY_U8(
> + "number", chl->info.number);
> + chl->property[prop_index++] = PROPERTY_ENTRY_U8(
> + "version", chl->info.version);
> + chl->property[prop_index++] = PROPERTY_ENTRY_U8(
> + "resource_cap", chl->info.resource_cap);
> + chl->property[prop_index++] = PROPERTY_ENTRY_U8(
> + "event_type", chl->info.event_type);
> + chl->property[prop_index++] = PROPERTY_ENTRY_U16(
> + "tx_fifo_size", chl->info.uart->tx_fifo_size);
> + chl->property[prop_index++] = PROPERTY_ENTRY_U16(
> + "rx_fifo_size", chl->info.uart->rx_fifo_size);
> + chl->property[prop_index++] = PROPERTY_ENTRY_U32(
> + "significand", chl->info.uart->significand);
> + chl->property[prop_index++] = PROPERTY_ENTRY_U8(
> + "exponent", chl->info.uart->exponent);
> + chl->property[prop_index++] = PROPERTY_ENTRY_U8(
> + "rs232_cap", chl->info.uart->rs232_cap);
> + chl->property[prop_index++] = PROPERTY_ENTRY_U8(
> + "rs422_cap", chl->info.uart->rs422_cap);
> + chl->property[prop_index++] = PROPERTY_ENTRY_U8(
> + "rs485_cap", chl->info.uart->rs485_cap);
> + chl->property[prop_index++] = PROPERTY_ENTRY_U8(
> + "ahdc_cap", chl->info.uart->ahdc_cap);
> + chl->property[prop_index++] = PROPERTY_ENTRY_U8(
> + "cs_cap", chl->info.uart->cs_cap);
> + chl->property[prop_index++] = PROPERTY_ENTRY_U8(
> + "rs422_end_cap", chl->info.uart->rs422_end_cap);
> + chl->property[prop_index++] = PROPERTY_ENTRY_U8(
> + "rs485_end_cap", chl->info.uart->rs485_end_cap);
I'm not sure what's going on here. Can you add documentation somewhere
to explain all these?
...
> +static void __exit sdc_exit(void)
> +{
> + ida_destroy(&sdc_devid_ida);
If you do it on non-freeid IDA it means you have a bug in resource
management (leakage) somewhere. I guess it was mistakenly used by the
drivers.
> + debugfs_remove(sdc_mfd_debugfs);
> +}
> +module_exit(sdc_exit);
...
> +static int sdc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> + flags = pci_resource_flags(pdev, 0);
> + if (!(flags & IORESOURCE_MEM)) {
> + pr_err("bar0 resource flag x%lx invalid\n", flags);
So what? Besides the fact that dev_err() should be used.
> + return -ENODEV;
> + }
> +
> + flags = pci_resource_flags(pdev, 1);
> + if (!(flags & IORESOURCE_IO)) {
> + pr_err("bar1 resource flag x%lx invalid\n", flags);
> + return -ENODEV;
> + }
> +
> + flags = pci_resource_flags(pdev, 2);
> + if (!(flags & IORESOURCE_MEM)) {
> + pr_err("bar2 resource flag x%lx invalid\n", flags);
> + return -ENODEV;
> + }
All above checks seem redundant.
> + ret = sdc_probe(&pdev->dev, &info);
> + if (ret)
> + return ret;
...
> +static const struct pci_device_id sdc_pci_ids[] = {
> + { PCI_VDEVICE(SUNIX, 0x2000) },
> + { },
Comma is not needed for terminator lines.
> +};
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mfd:Add SUNIX mfd & PCIe driver
2021-05-28 9:58 [PATCH] mfd:Add SUNIX mfd & PCIe driver Moriis Ku
2021-05-28 12:26 ` Andy Shevchenko
@ 2021-05-28 18:26 ` Lee Jones
1 sibling, 0 replies; 3+ messages in thread
From: Lee Jones @ 2021-05-28 18:26 UTC (permalink / raw)
To: Moriis Ku; +Cc: linux-kernel, jason_lee, taian.chen
On Fri, 28 May 2021, Moriis Ku wrote:
> From: Morris Ku <saumah@gmail.com>
>
> Add SUNIX mfd & PCIe driver
>
> Cc: Jason Lee <jason_lee@sunix.com>
> Cc: Taian Chen <taian.chen@sunix.com>
> Signed-off-by: Morris Ku <saumah@gmail.com>
> ---
> sdc_mfd.c | 513 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
To be frank, this is a bit of a mess.
Let's take one step back for a moment.
Please tell me how this compares to any existing driver currently
present in drivers/mfd. I'm 100% sure this H/W is not different to
anything we've seen before. Thus, my advice is to base it off of a
piece of code that has been previously accepted and take it from
there.
> sdc_mfd.h | 61 +++++++
> sdc_pci.c | 92 ++++++++++
> 3 files changed, 666 insertions(+)
> create mode 100644 sdc_mfd.c
> create mode 100644 sdc_mfd.h
> create mode 100644 sdc_pci.c
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-05-28 18:26 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:58 [PATCH] mfd:Add SUNIX mfd & PCIe driver Moriis Ku
2021-05-28 12:26 ` Andy Shevchenko
2021-05-28 18:26 ` Lee Jones
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.