Linux-MIPS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v3 0/7] Use MFD framework for SGI IOC3 drivers
@ 2019-06-13 17:06 Thomas Bogendoerfer
  2019-06-13 17:06 ` [PATCH v3 1/7] nvmem: core: add nvmem_device_find Thomas Bogendoerfer
                   ` (6 more replies)
  0 siblings, 7 replies; 13+ messages in thread
From: Thomas Bogendoerfer @ 2019-06-13 17:06 UTC (permalink / raw)
  To: Ralf Baechle, Paul Burton, James Hogan, Dmitry Torokhov,
	Lee Jones, David S. Miller, Srinivas Kandagatla,
	Alessandro Zummo, Alexandre Belloni, Greg Kroah-Hartman,
	Jiri Slaby, linux-mips, linux-kernel, linux-input, netdev,
	linux-rtc, linux-serial

SGI IOC3 ASIC includes support for ethernet, PS2 keyboard/mouse,
NIC (number in a can), GPIO and a byte  bus. By attaching a
SuperIO chip to it, it also supports serial lines and a parallel
port. The chip is used on a variety of SGI systems with different
configurations. This patchset moves code out of the network driver,
which doesn't belong there, into its new place a MFD driver and
specific platform drivers for the different subfunctions.

Changes in v3:
 - use 1-wire subsystem for handling proms
 - pci-xtalk driver uses prom information to create PCI subsystem
   ids for use in MFD driver
 - changed MFD driver to only use static declared mfd_cells
 - added IP30 system board setup to MFD driver
 - mac address is now read from ioc3-eth driver with nvmem framework 

Changes in v2:
 - fixed issue in ioc3kbd.c reported by Dmitry Torokhov
 - merged IP27 RTC removal and 8250 serial driver addition into
   main MFD patch to keep patches bisectable

Thomas Bogendoerfer (7):
  nvmem: core: add nvmem_device_find
  MIPS: PCI: refactor ioc3 special handling
  MIPS: PCI: use information from 1-wire PROM for IOC3 detection
  MIPS: SGI-IP27: remove ioc3 ethernet init
  mfd: ioc3: Add driver for SGI IOC3 chip
  MIPS: SGI-IP27: fix readb/writeb addressing
  Input: add IOC3 serio driver

 arch/mips/include/asm/mach-ip27/mangle-port.h |    4 +-
 arch/mips/include/asm/pci/bridge.h            |    1 +
 arch/mips/include/asm/sn/ioc3.h               |  356 ++---
 arch/mips/pci/pci-xtalk-bridge.c              |  296 ++--
 arch/mips/sgi-ip27/ip27-console.c             |    5 +-
 arch/mips/sgi-ip27/ip27-init.c                |   13 -
 arch/mips/sgi-ip27/ip27-timer.c               |   20 -
 arch/mips/sgi-ip27/ip27-xtalk.c               |   38 +-
 drivers/input/serio/Kconfig                   |   10 +
 drivers/input/serio/Makefile                  |    1 +
 drivers/input/serio/ioc3kbd.c                 |  158 ++
 drivers/mfd/Kconfig                           |   13 +
 drivers/mfd/Makefile                          |    1 +
 drivers/mfd/ioc3.c                            |  683 +++++++++
 drivers/net/ethernet/sgi/Kconfig              |    4 +-
 drivers/net/ethernet/sgi/ioc3-eth.c           | 1932 ++++++++++---------------
 drivers/nvmem/core.c                          |   62 +-
 drivers/rtc/rtc-m48t35.c                      |   11 +
 drivers/tty/serial/8250/8250_ioc3.c           |   98 ++
 drivers/tty/serial/8250/Kconfig               |   11 +
 drivers/tty/serial/8250/Makefile              |    1 +
 include/linux/nvmem-consumer.h                |    9 +
 22 files changed, 2152 insertions(+), 1575 deletions(-)
 create mode 100644 drivers/input/serio/ioc3kbd.c
 create mode 100644 drivers/mfd/ioc3.c
 create mode 100644 drivers/tty/serial/8250/8250_ioc3.c

-- 
2.13.7


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

* [PATCH v3 1/7] nvmem: core: add nvmem_device_find
  2019-06-13 17:06 [PATCH v3 0/7] Use MFD framework for SGI IOC3 drivers Thomas Bogendoerfer
@ 2019-06-13 17:06 ` Thomas Bogendoerfer
  2019-06-13 17:06 ` [PATCH v3 2/7] MIPS: PCI: refactor ioc3 special handling Thomas Bogendoerfer
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Thomas Bogendoerfer @ 2019-06-13 17:06 UTC (permalink / raw)
  To: Ralf Baechle, Paul Burton, James Hogan, Dmitry Torokhov,
	Lee Jones, David S. Miller, Srinivas Kandagatla,
	Alessandro Zummo, Alexandre Belloni, Greg Kroah-Hartman,
	Jiri Slaby, linux-mips, linux-kernel, linux-input, netdev,
	linux-rtc, linux-serial

nvmem_device_find provides a way to search for nvmem devices with
the help of a match function simlair to bus_find_device.

Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
---
 drivers/nvmem/core.c           | 62 ++++++++++++++++++++++--------------------
 include/linux/nvmem-consumer.h |  9 ++++++
 2 files changed, 41 insertions(+), 30 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index c7892c3da91f..29a1c937b290 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -76,36 +76,18 @@ static struct bus_type nvmem_bus_type = {
 	.name		= "nvmem",
 };
 
+#if IS_ENABLED(CONFIG_OF)
 static int of_nvmem_match(struct device *dev, void *nvmem_np)
 {
 	return dev->of_node == nvmem_np;
 }
+#endif
 
-static struct nvmem_device *of_nvmem_find(struct device_node *nvmem_np)
+static int nvmem_match_name(struct device *dev, void *data)
 {
-	struct device *d;
-
-	if (!nvmem_np)
-		return NULL;
-
-	d = bus_find_device(&nvmem_bus_type, NULL, nvmem_np, of_nvmem_match);
-
-	if (!d)
-		return NULL;
+	const char *name = data;
 
-	return to_nvmem_device(d);
-}
-
-static struct nvmem_device *nvmem_find(const char *name)
-{
-	struct device *d;
-
-	d = bus_find_device_by_name(&nvmem_bus_type, NULL, name);
-
-	if (!d)
-		return NULL;
-
-	return to_nvmem_device(d);
+	return sysfs_streq(name, dev_name(dev));
 }
 
 static void nvmem_cell_drop(struct nvmem_cell *cell)
@@ -537,13 +519,16 @@ int devm_nvmem_unregister(struct device *dev, struct nvmem_device *nvmem)
 }
 EXPORT_SYMBOL(devm_nvmem_unregister);
 
-static struct nvmem_device *__nvmem_device_get(struct device_node *np,
-					       const char *nvmem_name)
+static struct nvmem_device *__nvmem_device_get(void *data,
+				int (*match)(struct device *dev, void *data))
 {
 	struct nvmem_device *nvmem = NULL;
+	struct device *dev;
 
 	mutex_lock(&nvmem_mutex);
-	nvmem = np ? of_nvmem_find(np) : nvmem_find(nvmem_name);
+	dev = bus_find_device(&nvmem_bus_type, NULL, data, match);
+	if (dev)
+		nvmem = to_nvmem_device(dev);
 	mutex_unlock(&nvmem_mutex);
 	if (!nvmem)
 		return ERR_PTR(-EPROBE_DEFER);
@@ -592,7 +577,7 @@ struct nvmem_device *of_nvmem_device_get(struct device_node *np, const char *id)
 	if (!nvmem_np)
 		return ERR_PTR(-ENOENT);
 
-	return __nvmem_device_get(nvmem_np, NULL);
+	return __nvmem_device_get(nvmem_np, of_nvmem_match);
 }
 EXPORT_SYMBOL_GPL(of_nvmem_device_get);
 #endif
@@ -618,10 +603,26 @@ struct nvmem_device *nvmem_device_get(struct device *dev, const char *dev_name)
 
 	}
 
-	return __nvmem_device_get(NULL, dev_name);
+	return __nvmem_device_get((void *)dev_name, nvmem_match_name);
 }
 EXPORT_SYMBOL_GPL(nvmem_device_get);
 
+/**
+ * nvmem_device_find() - Find nvmem device with matching function
+ *
+ * @data: Data to pass to match function
+ * @match: Callback function to check device
+ *
+ * Return: ERR_PTR() on error or a valid pointer to a struct nvmem_device
+ * on success.
+ */
+struct nvmem_device *nvmem_device_find(void *data,
+				int (*match)(struct device *dev, void *data))
+{
+	return __nvmem_device_get(data, match);
+}
+EXPORT_SYMBOL_GPL(nvmem_device_find);
+
 static int devm_nvmem_device_match(struct device *dev, void *res, void *data)
 {
 	struct nvmem_device **nvmem = res;
@@ -715,7 +716,8 @@ nvmem_cell_get_from_lookup(struct device *dev, const char *con_id)
 		if ((strcmp(lookup->dev_id, dev_id) == 0) &&
 		    (strcmp(lookup->con_id, con_id) == 0)) {
 			/* This is the right entry. */
-			nvmem = __nvmem_device_get(NULL, lookup->nvmem_name);
+			nvmem = __nvmem_device_get((void *)lookup->nvmem_name,
+						   nvmem_match_name);
 			if (IS_ERR(nvmem)) {
 				/* Provider may not be registered yet. */
 				cell = ERR_CAST(nvmem);
@@ -785,7 +787,7 @@ struct nvmem_cell *of_nvmem_cell_get(struct device_node *np, const char *id)
 	if (!nvmem_np)
 		return ERR_PTR(-EINVAL);
 
-	nvmem = __nvmem_device_get(nvmem_np, NULL);
+	nvmem = __nvmem_device_get(nvmem_np, of_nvmem_match);
 	of_node_put(nvmem_np);
 	if (IS_ERR(nvmem))
 		return ERR_CAST(nvmem);
diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
index 8f8be5b00060..a8249b2dcaf4 100644
--- a/include/linux/nvmem-consumer.h
+++ b/include/linux/nvmem-consumer.h
@@ -89,6 +89,9 @@ void nvmem_del_cell_lookups(struct nvmem_cell_lookup *entries,
 int nvmem_register_notifier(struct notifier_block *nb);
 int nvmem_unregister_notifier(struct notifier_block *nb);
 
+struct nvmem_device *nvmem_device_find(void *data,
+				int (*match)(struct device *dev, void *data));
+
 #else
 
 static inline struct nvmem_cell *nvmem_cell_get(struct device *dev,
@@ -204,6 +207,12 @@ static inline int nvmem_unregister_notifier(struct notifier_block *nb)
 	return -EOPNOTSUPP;
 }
 
+static inline struct nvmem_device *nvmem_device_find(void *data,
+				int (*match)(struct device *dev, void *data))
+{
+	return NULL;
+}
+
 #endif /* CONFIG_NVMEM */
 
 #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
-- 
2.13.7


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

* [PATCH v3 2/7] MIPS: PCI: refactor ioc3 special handling
  2019-06-13 17:06 [PATCH v3 0/7] Use MFD framework for SGI IOC3 drivers Thomas Bogendoerfer
  2019-06-13 17:06 ` [PATCH v3 1/7] nvmem: core: add nvmem_device_find Thomas Bogendoerfer
@ 2019-06-13 17:06 ` Thomas Bogendoerfer
  2019-06-13 17:06 ` [PATCH v3 3/7] MIPS: PCI: use information from 1-wire PROM for IOC3 detection Thomas Bogendoerfer
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Thomas Bogendoerfer @ 2019-06-13 17:06 UTC (permalink / raw)
  To: Ralf Baechle, Paul Burton, James Hogan, Dmitry Torokhov,
	Lee Jones, David S. Miller, Srinivas Kandagatla,
	Alessandro Zummo, Alexandre Belloni, Greg Kroah-Hartman,
	Jiri Slaby, linux-mips, linux-kernel, linux-input, netdev,
	linux-rtc, linux-serial

Refactored code to only have one ioc3 special handling for read
access and one for write access.

Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
---
 arch/mips/pci/pci-xtalk-bridge.c | 167 +++++++++++++++------------------------
 1 file changed, 62 insertions(+), 105 deletions(-)

diff --git a/arch/mips/pci/pci-xtalk-bridge.c b/arch/mips/pci/pci-xtalk-bridge.c
index bcf7f559789a..7b4d40354ee7 100644
--- a/arch/mips/pci/pci-xtalk-bridge.c
+++ b/arch/mips/pci/pci-xtalk-bridge.c
@@ -20,16 +20,50 @@
  * Most of the IOC3 PCI config register aren't present
  * we emulate what is needed for a normal PCI enumeration
  */
-static u32 emulate_ioc3_cfg(int where, int size)
+static int ioc3_cfg_rd(void *addr, int where, int size, u32 *value)
 {
-	if (size == 1 && where == 0x3d)
-		return 0x01;
-	else if (size == 2 && where == 0x3c)
-		return 0x0100;
-	else if (size == 4 && where == 0x3c)
-		return 0x00000100;
+	u32 cf, shift, mask;
 
-	return 0;
+	switch (where & ~3) {
+	case 0x00 ... 0x10:
+	case 0x40 ... 0x44:
+		if (get_dbe(cf, (u32 *)addr))
+			return PCIBIOS_DEVICE_NOT_FOUND;
+		break;
+	case 0x3c:
+		/* emulate sane interrupt pin value */
+		cf = 0x00000100;
+		break;
+	default:
+		cf = 0;
+		break;
+	}
+	shift = (where & 3) << 3;
+	mask = 0xffffffffU >> ((4 - size) << 3);
+	*value = (cf >> shift) & mask;
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static int ioc3_cfg_wr(void *addr, int where, int size, u32 value)
+{
+	u32 cf, shift, mask, smask;
+
+	if ((where >= 0x14 && where < 0x40) || (where >= 0x48))
+		return PCIBIOS_SUCCESSFUL;
+
+	if (get_dbe(cf, (u32 *)addr))
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	shift = ((where & 3) << 3);
+	mask = (0xffffffffU >> ((4 - size) << 3));
+	smask = mask << shift;
+
+	cf = (cf & ~smask) | ((value & mask) << shift);
+	if (put_dbe(cf, (u32 *)addr))
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	return PCIBIOS_SUCCESSFUL;
 }
 
 static void bridge_disable_swapping(struct pci_dev *dev)
@@ -64,7 +98,7 @@ static int pci_conf0_read_config(struct pci_bus *bus, unsigned int devfn,
 	int slot = PCI_SLOT(devfn);
 	int fn = PCI_FUNC(devfn);
 	void *addr;
-	u32 cf, shift, mask;
+	u32 cf;
 	int res;
 
 	addr = &bridge->b_type0_cfg_dev[slot].f[fn].c[PCI_VENDOR_ID];
@@ -75,8 +109,10 @@ static int pci_conf0_read_config(struct pci_bus *bus, unsigned int devfn,
 	 * IOC3 is broken beyond belief ...  Don't even give the
 	 * generic PCI code a chance to look at it for real ...
 	 */
-	if (cf == (PCI_VENDOR_ID_SGI | (PCI_DEVICE_ID_SGI_IOC3 << 16)))
-		goto is_ioc3;
+	if (cf == (PCI_VENDOR_ID_SGI | (PCI_DEVICE_ID_SGI_IOC3 << 16))) {
+		addr = &bridge->b_type0_cfg_dev[slot].f[fn].l[where >> 2];
+		return ioc3_cfg_rd(addr, where, size, value);
+	}
 
 	addr = &bridge->b_type0_cfg_dev[slot].f[fn].c[where ^ (4 - size)];
 
@@ -88,26 +124,6 @@ static int pci_conf0_read_config(struct pci_bus *bus, unsigned int devfn,
 		res = get_dbe(*value, (u32 *)addr);
 
 	return res ? PCIBIOS_DEVICE_NOT_FOUND : PCIBIOS_SUCCESSFUL;
-
-is_ioc3:
-
-	/*
-	 * IOC3 special handling
-	 */
-	if ((where >= 0x14 && where < 0x40) || (where >= 0x48)) {
-		*value = emulate_ioc3_cfg(where, size);
-		return PCIBIOS_SUCCESSFUL;
-	}
-
-	addr = &bridge->b_type0_cfg_dev[slot].f[fn].l[where >> 2];
-	if (get_dbe(cf, (u32 *)addr))
-		return PCIBIOS_DEVICE_NOT_FOUND;
-
-	shift = ((where & 3) << 3);
-	mask = (0xffffffffU >> ((4 - size) << 3));
-	*value = (cf >> shift) & mask;
-
-	return PCIBIOS_SUCCESSFUL;
 }
 
 static int pci_conf1_read_config(struct pci_bus *bus, unsigned int devfn,
@@ -119,7 +135,7 @@ static int pci_conf1_read_config(struct pci_bus *bus, unsigned int devfn,
 	int slot = PCI_SLOT(devfn);
 	int fn = PCI_FUNC(devfn);
 	void *addr;
-	u32 cf, shift, mask;
+	u32 cf;
 	int res;
 
 	bridge_write(bc, b_pci_cfg, (busno << 16) | (slot << 11));
@@ -131,8 +147,10 @@ static int pci_conf1_read_config(struct pci_bus *bus, unsigned int devfn,
 	 * IOC3 is broken beyond belief ...  Don't even give the
 	 * generic PCI code a chance to look at it for real ...
 	 */
-	if (cf == (PCI_VENDOR_ID_SGI | (PCI_DEVICE_ID_SGI_IOC3 << 16)))
-		goto is_ioc3;
+	if (cf == (PCI_VENDOR_ID_SGI | (PCI_DEVICE_ID_SGI_IOC3 << 16))) {
+		addr = &bridge->b_type1_cfg.c[(fn << 8) | (where & ~3)];
+		return ioc3_cfg_rd(addr, where, size, value);
+	}
 
 	addr = &bridge->b_type1_cfg.c[(fn << 8) | (where ^ (4 - size))];
 
@@ -144,26 +162,6 @@ static int pci_conf1_read_config(struct pci_bus *bus, unsigned int devfn,
 		res = get_dbe(*value, (u32 *)addr);
 
 	return res ? PCIBIOS_DEVICE_NOT_FOUND : PCIBIOS_SUCCESSFUL;
-
-is_ioc3:
-
-	/*
-	 * IOC3 special handling
-	 */
-	if ((where >= 0x14 && where < 0x40) || (where >= 0x48)) {
-		*value = emulate_ioc3_cfg(where, size);
-		return PCIBIOS_SUCCESSFUL;
-	}
-
-	addr = &bridge->b_type1_cfg.c[(fn << 8) | where];
-	if (get_dbe(cf, (u32 *)addr))
-		return PCIBIOS_DEVICE_NOT_FOUND;
-
-	shift = ((where & 3) << 3);
-	mask = (0xffffffffU >> ((4 - size) << 3));
-	*value = (cf >> shift) & mask;
-
-	return PCIBIOS_SUCCESSFUL;
 }
 
 static int pci_read_config(struct pci_bus *bus, unsigned int devfn,
@@ -183,7 +181,7 @@ static int pci_conf0_write_config(struct pci_bus *bus, unsigned int devfn,
 	int slot = PCI_SLOT(devfn);
 	int fn = PCI_FUNC(devfn);
 	void *addr;
-	u32 cf, shift, mask, smask;
+	u32 cf;
 	int res;
 
 	addr = &bridge->b_type0_cfg_dev[slot].f[fn].c[PCI_VENDOR_ID];
@@ -194,8 +192,10 @@ static int pci_conf0_write_config(struct pci_bus *bus, unsigned int devfn,
 	 * IOC3 is broken beyond belief ...  Don't even give the
 	 * generic PCI code a chance to look at it for real ...
 	 */
-	if (cf == (PCI_VENDOR_ID_SGI | (PCI_DEVICE_ID_SGI_IOC3 << 16)))
-		goto is_ioc3;
+	if (cf == (PCI_VENDOR_ID_SGI | (PCI_DEVICE_ID_SGI_IOC3 << 16))) {
+		addr = &bridge->b_type0_cfg_dev[slot].f[fn].l[where >> 2];
+		return ioc3_cfg_wr(addr, where, size, value);
+	}
 
 	addr = &bridge->b_type0_cfg_dev[slot].f[fn].c[where ^ (4 - size)];
 
@@ -210,29 +210,6 @@ static int pci_conf0_write_config(struct pci_bus *bus, unsigned int devfn,
 		return PCIBIOS_DEVICE_NOT_FOUND;
 
 	return PCIBIOS_SUCCESSFUL;
-
-is_ioc3:
-
-	/*
-	 * IOC3 special handling
-	 */
-	if ((where >= 0x14 && where < 0x40) || (where >= 0x48))
-		return PCIBIOS_SUCCESSFUL;
-
-	addr = &bridge->b_type0_cfg_dev[slot].f[fn].l[where >> 2];
-
-	if (get_dbe(cf, (u32 *)addr))
-		return PCIBIOS_DEVICE_NOT_FOUND;
-
-	shift = ((where & 3) << 3);
-	mask = (0xffffffffU >> ((4 - size) << 3));
-	smask = mask << shift;
-
-	cf = (cf & ~smask) | ((value & mask) << shift);
-	if (put_dbe(cf, (u32 *)addr))
-		return PCIBIOS_DEVICE_NOT_FOUND;
-
-	return PCIBIOS_SUCCESSFUL;
 }
 
 static int pci_conf1_write_config(struct pci_bus *bus, unsigned int devfn,
@@ -244,7 +221,7 @@ static int pci_conf1_write_config(struct pci_bus *bus, unsigned int devfn,
 	int fn = PCI_FUNC(devfn);
 	int busno = bus->number;
 	void *addr;
-	u32 cf, shift, mask, smask;
+	u32 cf;
 	int res;
 
 	bridge_write(bc, b_pci_cfg, (busno << 16) | (slot << 11));
@@ -256,8 +233,10 @@ static int pci_conf1_write_config(struct pci_bus *bus, unsigned int devfn,
 	 * IOC3 is broken beyond belief ...  Don't even give the
 	 * generic PCI code a chance to look at it for real ...
 	 */
-	if (cf == (PCI_VENDOR_ID_SGI | (PCI_DEVICE_ID_SGI_IOC3 << 16)))
-		goto is_ioc3;
+	if (cf == (PCI_VENDOR_ID_SGI | (PCI_DEVICE_ID_SGI_IOC3 << 16))) {
+		addr = &bridge->b_type0_cfg_dev[slot].f[fn].l[where >> 2];
+		return ioc3_cfg_wr(addr, where, size, value);
+	}
 
 	addr = &bridge->b_type1_cfg.c[(fn << 8) | (where ^ (4 - size))];
 
@@ -272,28 +251,6 @@ static int pci_conf1_write_config(struct pci_bus *bus, unsigned int devfn,
 		return PCIBIOS_DEVICE_NOT_FOUND;
 
 	return PCIBIOS_SUCCESSFUL;
-
-is_ioc3:
-
-	/*
-	 * IOC3 special handling
-	 */
-	if ((where >= 0x14 && where < 0x40) || (where >= 0x48))
-		return PCIBIOS_SUCCESSFUL;
-
-	addr = &bridge->b_type0_cfg_dev[slot].f[fn].l[where >> 2];
-	if (get_dbe(cf, (u32 *)addr))
-		return PCIBIOS_DEVICE_NOT_FOUND;
-
-	shift = ((where & 3) << 3);
-	mask = (0xffffffffU >> ((4 - size) << 3));
-	smask = mask << shift;
-
-	cf = (cf & ~smask) | ((value & mask) << shift);
-	if (put_dbe(cf, (u32 *)addr))
-		return PCIBIOS_DEVICE_NOT_FOUND;
-
-	return PCIBIOS_SUCCESSFUL;
 }
 
 static int pci_write_config(struct pci_bus *bus, unsigned int devfn,
-- 
2.13.7


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

* [PATCH v3 3/7] MIPS: PCI: use information from 1-wire PROM for IOC3 detection
  2019-06-13 17:06 [PATCH v3 0/7] Use MFD framework for SGI IOC3 drivers Thomas Bogendoerfer
  2019-06-13 17:06 ` [PATCH v3 1/7] nvmem: core: add nvmem_device_find Thomas Bogendoerfer
  2019-06-13 17:06 ` [PATCH v3 2/7] MIPS: PCI: refactor ioc3 special handling Thomas Bogendoerfer
@ 2019-06-13 17:06 ` Thomas Bogendoerfer
  2019-06-13 17:06 ` [PATCH v3 4/7] MIPS: SGI-IP27: remove ioc3 ethernet init Thomas Bogendoerfer
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Thomas Bogendoerfer @ 2019-06-13 17:06 UTC (permalink / raw)
  To: Ralf Baechle, Paul Burton, James Hogan, Dmitry Torokhov,
	Lee Jones, David S. Miller, Srinivas Kandagatla,
	Alessandro Zummo, Alexandre Belloni, Greg Kroah-Hartman,
	Jiri Slaby, linux-mips, linux-kernel, linux-input, netdev,
	linux-rtc, linux-serial

IOC3 chips in SGI system are conntected to a bridge ASIC, which has
a 1-wire prom attached with part number information. This changeset
uses this information to create PCI subsystem information, which
the MFD driver uses for further platform device setup.

Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
---
 arch/mips/include/asm/pci/bridge.h |   1 +
 arch/mips/include/asm/sn/ioc3.h    |   9 +++
 arch/mips/pci/pci-xtalk-bridge.c   | 135 ++++++++++++++++++++++++++++++++++++-
 arch/mips/sgi-ip27/ip27-xtalk.c    |  38 +++++++++--
 4 files changed, 175 insertions(+), 8 deletions(-)

diff --git a/arch/mips/include/asm/pci/bridge.h b/arch/mips/include/asm/pci/bridge.h
index a92cd30b48c9..3bc630ff9ad4 100644
--- a/arch/mips/include/asm/pci/bridge.h
+++ b/arch/mips/include/asm/pci/bridge.h
@@ -807,6 +807,7 @@ struct bridge_controller {
 	unsigned long		intr_addr;
 	struct irq_domain	*domain;
 	unsigned int		pci_int[8];
+	u32			ioc3_sid[8];
 	nasid_t			nasid;
 };
 
diff --git a/arch/mips/include/asm/sn/ioc3.h b/arch/mips/include/asm/sn/ioc3.h
index 25c8dccab51f..5022b0ab2074 100644
--- a/arch/mips/include/asm/sn/ioc3.h
+++ b/arch/mips/include/asm/sn/ioc3.h
@@ -661,4 +661,13 @@ typedef enum ioc3_subdevs_e {
 #define IOC3_INTA_SUBDEVS	IOC3_SDB_ETHER
 #define IOC3_INTB_SUBDEVS	(IOC3_SDB_GENERIC|IOC3_SDB_KBMS|IOC3_SDB_SERIAL|IOC3_SDB_ECPP|IOC3_SDB_RT)
 
+/* subsystem IDs supplied by card detection in pci-xtalk-bridge */
+#define	IOC3_SUBSYS_IP27_BASEIO6G	0xc300
+#define	IOC3_SUBSYS_IP27_MIO		0xc301
+#define	IOC3_SUBSYS_IP27_BASEIO		0xc302
+#define	IOC3_SUBSYS_IP29_SYSBOARD	0xc303
+#define	IOC3_SUBSYS_IP30_SYSBOARD	0xc304
+#define	IOC3_SUBSYS_MENET		0xc305
+#define	IOC3_SUBSYS_MENET4		0xc306
+
 #endif /* _IOC3_H */
diff --git a/arch/mips/pci/pci-xtalk-bridge.c b/arch/mips/pci/pci-xtalk-bridge.c
index 7b4d40354ee7..df87c686b308 100644
--- a/arch/mips/pci/pci-xtalk-bridge.c
+++ b/arch/mips/pci/pci-xtalk-bridge.c
@@ -11,16 +11,22 @@
 #include <linux/dma-direct.h>
 #include <linux/platform_device.h>
 #include <linux/platform_data/xtalk-bridge.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/crc16.h>
 
 #include <asm/pci/bridge.h>
 #include <asm/paccess.h>
 #include <asm/sn/irq_alloc.h>
+#include <asm/sn/ioc3.h>
+
+#define CRC16_INIT	0
+#define CRC16_VALID	0xb001
 
 /*
  * Most of the IOC3 PCI config register aren't present
  * we emulate what is needed for a normal PCI enumeration
  */
-static int ioc3_cfg_rd(void *addr, int where, int size, u32 *value)
+static int ioc3_cfg_rd(void *addr, int where, int size, u32 *value, u32 sid)
 {
 	u32 cf, shift, mask;
 
@@ -30,6 +36,9 @@ static int ioc3_cfg_rd(void *addr, int where, int size, u32 *value)
 		if (get_dbe(cf, (u32 *)addr))
 			return PCIBIOS_DEVICE_NOT_FOUND;
 		break;
+	case 0x2c:
+		cf = sid;
+		break;
 	case 0x3c:
 		/* emulate sane interrupt pin value */
 		cf = 0x00000100;
@@ -111,7 +120,8 @@ static int pci_conf0_read_config(struct pci_bus *bus, unsigned int devfn,
 	 */
 	if (cf == (PCI_VENDOR_ID_SGI | (PCI_DEVICE_ID_SGI_IOC3 << 16))) {
 		addr = &bridge->b_type0_cfg_dev[slot].f[fn].l[where >> 2];
-		return ioc3_cfg_rd(addr, where, size, value);
+		return ioc3_cfg_rd(addr, where, size, value,
+				   bc->ioc3_sid[slot]);
 	}
 
 	addr = &bridge->b_type0_cfg_dev[slot].f[fn].c[where ^ (4 - size)];
@@ -149,7 +159,8 @@ static int pci_conf1_read_config(struct pci_bus *bus, unsigned int devfn,
 	 */
 	if (cf == (PCI_VENDOR_ID_SGI | (PCI_DEVICE_ID_SGI_IOC3 << 16))) {
 		addr = &bridge->b_type1_cfg.c[(fn << 8) | (where & ~3)];
-		return ioc3_cfg_rd(addr, where, size, value);
+		return ioc3_cfg_rd(addr, where, size, value,
+				   bc->ioc3_sid[slot]);
 	}
 
 	addr = &bridge->b_type1_cfg.c[(fn << 8) | (where ^ (4 - size))];
@@ -426,6 +437,117 @@ static int bridge_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
 	return irq;
 }
 
+#define IOC3_SID(sid)	(PCI_VENDOR_ID_SGI << 16 | (sid))
+
+static void bridge_setup_ip27_baseio6g(struct bridge_controller *bc)
+{
+	bc->ioc3_sid[2] = IOC3_SID(IOC3_SUBSYS_IP27_BASEIO6G);
+	bc->ioc3_sid[6] = IOC3_SID(IOC3_SUBSYS_IP27_MIO);
+}
+
+static void bridge_setup_ip27_baseio(struct bridge_controller *bc)
+{
+	bc->ioc3_sid[2] = IOC3_SID(IOC3_SUBSYS_IP27_BASEIO);
+}
+
+static void bridge_setup_ip29_baseio(struct bridge_controller *bc)
+{
+	bc->ioc3_sid[2] = IOC3_SID(IOC3_SUBSYS_IP29_SYSBOARD);
+}
+
+static void bridge_setup_ip30_sysboard(struct bridge_controller *bc)
+{
+	bc->ioc3_sid[2] = IOC3_SID(IOC3_SUBSYS_IP30_SYSBOARD);
+}
+
+static void bridge_setup_menet(struct bridge_controller *bc)
+{
+	bc->ioc3_sid[0] = IOC3_SID(IOC3_SUBSYS_MENET);
+	bc->ioc3_sid[1] = IOC3_SID(IOC3_SUBSYS_MENET);
+	bc->ioc3_sid[2] = IOC3_SID(IOC3_SUBSYS_MENET);
+	bc->ioc3_sid[3] = IOC3_SID(IOC3_SUBSYS_MENET4);
+}
+
+#define BRIDGE_BOARD_SETUP(_partno, _setup)	\
+	{ .match = _partno, .setup = _setup }
+
+static const struct {
+	char *match;
+	void (*setup)(struct bridge_controller *bc);
+} bridge_ioc3_devid[] = {
+	BRIDGE_BOARD_SETUP("030-0734-", bridge_setup_ip27_baseio6g),
+	BRIDGE_BOARD_SETUP("030-0880-", bridge_setup_ip27_baseio6g),
+	BRIDGE_BOARD_SETUP("030-1023-", bridge_setup_ip27_baseio),
+	BRIDGE_BOARD_SETUP("030-1124-", bridge_setup_ip27_baseio),
+	BRIDGE_BOARD_SETUP("030-1025-", bridge_setup_ip29_baseio),
+	BRIDGE_BOARD_SETUP("030-1244-", bridge_setup_ip29_baseio),
+	BRIDGE_BOARD_SETUP("030-1389-", bridge_setup_ip29_baseio),
+	BRIDGE_BOARD_SETUP("030-0887-", bridge_setup_ip30_sysboard),
+	BRIDGE_BOARD_SETUP("030-1467-", bridge_setup_ip30_sysboard),
+	BRIDGE_BOARD_SETUP("030-0873-", bridge_setup_menet),
+};
+
+static void bridge_setup_board(struct bridge_controller *bc, char *partnum)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(bridge_ioc3_devid); i++)
+		if (!strncmp(partnum, bridge_ioc3_devid[i].match,
+			     strlen(bridge_ioc3_devid[i].match))) {
+			bridge_ioc3_devid[i].setup(bc);
+		}
+}
+
+static int bridge_nvmem_match(struct device *dev, void *data)
+{
+	const char *name = dev_name(dev);
+	const char *prefix = data;
+
+	if (strlen(name) < strlen(prefix))
+		return 0;
+
+	return memcmp(prefix, dev_name(dev), strlen(prefix)) == 0;
+}
+
+static int bridge_get_partnum(u64 baddr, char *partnum)
+{
+	struct nvmem_device *nvmem;
+	char prefix[24];
+	u8 prom[64];
+	int i, j;
+	int ret;
+
+	snprintf(prefix, sizeof(prefix), "bridge-%012llx-0b-", baddr);
+
+	nvmem = nvmem_device_find(prefix, bridge_nvmem_match);
+	if (IS_ERR(nvmem))
+		return PTR_ERR(nvmem);
+
+	ret = nvmem_device_read(nvmem, 0, 64, prom);
+	nvmem_device_put(nvmem);
+
+	if (ret != 64)
+		return ret;
+
+	if (crc16(CRC16_INIT, prom, 32) != CRC16_VALID ||
+	    crc16(CRC16_INIT, prom + 32, 32) != CRC16_VALID)
+		return -EINVAL;
+
+	/* Assemble part number */
+	j = 0;
+	for (i = 0; i < 19; i++)
+		if (prom[i + 11] != ' ')
+			partnum[j++] = prom[i + 11];
+
+	for (i = 0; i < 6; i++)
+		if (prom[i + 32] != ' ')
+			partnum[j++] = prom[i + 32];
+
+	partnum[j] = 0;
+
+	return 0;
+}
+
 static int bridge_probe(struct platform_device *pdev)
 {
 	struct xtalk_bridge_platform_data *bd = dev_get_platdata(&pdev->dev);
@@ -434,9 +556,14 @@ static int bridge_probe(struct platform_device *pdev)
 	struct pci_host_bridge *host;
 	struct irq_domain *domain, *parent;
 	struct fwnode_handle *fn;
+	char partnum[26];
 	int slot;
 	int err;
 
+	/* get part number from one wire prom */
+	if (bridge_get_partnum(virt_to_phys((void *)bd->bridge_addr), partnum))
+		return -EPROBE_DEFER; /* not available yet */
+
 	parent = irq_get_default_host();
 	if (!parent)
 		return -ENODEV;
@@ -517,6 +644,8 @@ static int bridge_probe(struct platform_device *pdev)
 	}
 	bridge_read(bc, b_wid_tflush);	  /* wait until Bridge PIO complete */
 
+	bridge_setup_board(bc, partnum);
+
 	host->dev.parent = dev;
 	host->sysdata = bc;
 	host->busnr = 0;
diff --git a/arch/mips/sgi-ip27/ip27-xtalk.c b/arch/mips/sgi-ip27/ip27-xtalk.c
index 4a1f0b0c29e2..9b7524362a11 100644
--- a/arch/mips/sgi-ip27/ip27-xtalk.c
+++ b/arch/mips/sgi-ip27/ip27-xtalk.c
@@ -10,6 +10,7 @@
 #include <linux/kernel.h>
 #include <linux/smp.h>
 #include <linux/platform_device.h>
+#include <linux/platform_data/sgi-w1.h>
 #include <linux/platform_data/xtalk-bridge.h>
 #include <asm/sn/addrs.h>
 #include <asm/sn/types.h>
@@ -26,9 +27,35 @@
 static void bridge_platform_create(nasid_t nasid, int widget, int masterwid)
 {
 	struct xtalk_bridge_platform_data *bd;
+	struct sgi_w1_platform_data *wd;
 	struct platform_device *pdev;
+	struct resource w1_res;
 	unsigned long offset;
 
+	offset = NODE_OFFSET(nasid);
+
+	wd = kzalloc(sizeof(*wd), GFP_KERNEL);
+	if (!wd)
+		goto no_mem;
+
+	snprintf(wd->dev_id, sizeof(wd->dev_id), "bridge-%012lx",
+		 offset + (widget << SWIN_SIZE_BITS));
+
+	memset(&w1_res, 0, sizeof(w1_res));
+	w1_res.start = offset + (widget << SWIN_SIZE_BITS) +
+				offsetof(struct bridge_regs, b_nic);
+	w1_res.end = w1_res.start + 3;
+	w1_res.flags = IORESOURCE_MEM;
+
+	pdev = platform_device_alloc("sgi_w1", PLATFORM_DEVID_AUTO);
+	if (!pdev) {
+		kfree(wd);
+		goto no_mem;
+	}
+	platform_device_add_resources(pdev, &w1_res, 1);
+	platform_device_add_data(pdev, wd, sizeof(*wd));
+	platform_device_add(pdev);
+
 	bd = kzalloc(sizeof(*bd), GFP_KERNEL);
 	if (!bd)
 		goto no_mem;
@@ -38,7 +65,6 @@ static void bridge_platform_create(nasid_t nasid, int widget, int masterwid)
 		goto no_mem;
 	}
 
-	offset = NODE_OFFSET(nasid);
 
 	bd->bridge_addr = RAW_NODE_SWIN_BASE(nasid, widget);
 	bd->intr_addr	= BIT_ULL(47) + 0x01800000 + PI_INT_PEND_MOD;
@@ -46,14 +72,14 @@ static void bridge_platform_create(nasid_t nasid, int widget, int masterwid)
 	bd->masterwid	= masterwid;
 
 	bd->mem.name	= "Bridge PCI MEM";
-	bd->mem.start	= offset + (widget << SWIN_SIZE_BITS);
-	bd->mem.end	= bd->mem.start + SWIN_SIZE - 1;
+	bd->mem.start	= offset + (widget << SWIN_SIZE_BITS) + BRIDGE_DEVIO0;
+	bd->mem.end	= offset + (widget << SWIN_SIZE_BITS) + SWIN_SIZE - 1;
 	bd->mem.flags	= IORESOURCE_MEM;
 	bd->mem_offset	= offset;
 
 	bd->io.name	= "Bridge PCI IO";
-	bd->io.start	= offset + (widget << SWIN_SIZE_BITS);
-	bd->io.end	= bd->io.start + SWIN_SIZE - 1;
+	bd->io.start	= offset + (widget << SWIN_SIZE_BITS) + BRIDGE_DEVIO0;
+	bd->io.end	= offset + (widget << SWIN_SIZE_BITS) + SWIN_SIZE - 1;
 	bd->io.flags	= IORESOURCE_IO;
 	bd->io_offset	= offset;
 
@@ -81,6 +107,8 @@ static int probe_one_port(nasid_t nasid, int widget, int masterwid)
 		bridge_platform_create(nasid, widget, masterwid);
 		break;
 	default:
+		pr_info("xtalk:n%d/%d unknown widget (0x%x)\n",
+			nasid, widget, partnum);
 		break;
 	}
 
-- 
2.13.7


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

* [PATCH v3 4/7] MIPS: SGI-IP27: remove ioc3 ethernet init
  2019-06-13 17:06 [PATCH v3 0/7] Use MFD framework for SGI IOC3 drivers Thomas Bogendoerfer
                   ` (2 preceding siblings ...)
  2019-06-13 17:06 ` [PATCH v3 3/7] MIPS: PCI: use information from 1-wire PROM for IOC3 detection Thomas Bogendoerfer
@ 2019-06-13 17:06 ` Thomas Bogendoerfer
  2019-06-13 17:06 ` [PATCH v3 6/7] MIPS: SGI-IP27: fix readb/writeb addressing Thomas Bogendoerfer
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Thomas Bogendoerfer @ 2019-06-13 17:06 UTC (permalink / raw)
  To: Ralf Baechle, Paul Burton, James Hogan, Dmitry Torokhov,
	Lee Jones, David S. Miller, Srinivas Kandagatla,
	Alessandro Zummo, Alexandre Belloni, Greg Kroah-Hartman,
	Jiri Slaby, linux-mips, linux-kernel, linux-input, netdev,
	linux-rtc, linux-serial

Removed not needed disabling of ethernet interrupts in IP27 platform code.

Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
---
 arch/mips/sgi-ip27/ip27-init.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/arch/mips/sgi-ip27/ip27-init.c b/arch/mips/sgi-ip27/ip27-init.c
index 066b33f50bcc..59d5375c9021 100644
--- a/arch/mips/sgi-ip27/ip27-init.c
+++ b/arch/mips/sgi-ip27/ip27-init.c
@@ -130,17 +130,6 @@ cnodeid_t get_compact_nodeid(void)
 	return NASID_TO_COMPACT_NODEID(get_nasid());
 }
 
-static inline void ioc3_eth_init(void)
-{
-	struct ioc3 *ioc3;
-	nasid_t nid;
-
-	nid = get_nasid();
-	ioc3 = (struct ioc3 *) KL_CONFIG_CH_CONS_INFO(nid)->memory_base;
-
-	ioc3->eier = 0;
-}
-
 extern void ip27_reboot_setup(void);
 
 void __init plat_mem_setup(void)
@@ -182,8 +171,6 @@ void __init plat_mem_setup(void)
 		panic("Kernel compiled for N mode.");
 #endif
 
-	ioc3_eth_init();
-
 	ioport_resource.start = 0;
 	ioport_resource.end = ~0UL;
 	set_io_port_base(IO_BASE);
-- 
2.13.7


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

* [PATCH v3 6/7] MIPS: SGI-IP27: fix readb/writeb addressing
  2019-06-13 17:06 [PATCH v3 0/7] Use MFD framework for SGI IOC3 drivers Thomas Bogendoerfer
                   ` (3 preceding siblings ...)
  2019-06-13 17:06 ` [PATCH v3 4/7] MIPS: SGI-IP27: remove ioc3 ethernet init Thomas Bogendoerfer
@ 2019-06-13 17:06 ` Thomas Bogendoerfer
  2019-06-13 17:06 ` [PATCH v3 7/7] Input: add IOC3 serio driver Thomas Bogendoerfer
       [not found] ` <20190613170636.6647-6-tbogendoerfer@suse.de>
  6 siblings, 0 replies; 13+ messages in thread
From: Thomas Bogendoerfer @ 2019-06-13 17:06 UTC (permalink / raw)
  To: Ralf Baechle, Paul Burton, James Hogan, Dmitry Torokhov,
	Lee Jones, David S. Miller, Srinivas Kandagatla,
	Alessandro Zummo, Alexandre Belloni, Greg Kroah-Hartman,
	Jiri Slaby, linux-mips, linux-kernel, linux-input, netdev,
	linux-rtc, linux-serial

Our chosen byte swapping, which is what firmware already uses, is to
do readl/writel by normal lw/sw intructions (data invariance). This
also means we need to mangle addresses for u8 and u16 accesses. The
mangling for 16bit has been done aready, but 8bit one was missing.
Correcting this causes different addresses for accesses to the
SuperIO and local bus of the IOC3 chip. This is fixed by changing
byte order in ioc3 and m48rtc_rtc structs.

Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
---
 arch/mips/include/asm/mach-ip27/mangle-port.h |   4 +-
 arch/mips/include/asm/sn/ioc3.h               | 198 +++++++++++++-------------
 arch/mips/sgi-ip27/ip27-console.c             |   5 +-
 drivers/rtc/rtc-m48t35.c                      |  11 ++
 drivers/tty/serial/8250/8250_ioc3.c           |   4 +-
 5 files changed, 115 insertions(+), 107 deletions(-)

diff --git a/arch/mips/include/asm/mach-ip27/mangle-port.h b/arch/mips/include/asm/mach-ip27/mangle-port.h
index f6e4912ea062..27c56efa519f 100644
--- a/arch/mips/include/asm/mach-ip27/mangle-port.h
+++ b/arch/mips/include/asm/mach-ip27/mangle-port.h
@@ -8,7 +8,7 @@
 #ifndef __ASM_MACH_IP27_MANGLE_PORT_H
 #define __ASM_MACH_IP27_MANGLE_PORT_H
 
-#define __swizzle_addr_b(port)	(port)
+#define __swizzle_addr_b(port)	((port) ^ 3)
 #define __swizzle_addr_w(port)	((port) ^ 2)
 #define __swizzle_addr_l(port)	(port)
 #define __swizzle_addr_q(port)	(port)
@@ -20,6 +20,6 @@
 # define ioswabl(a, x)		(x)
 # define __mem_ioswabl(a, x)	cpu_to_le32(x)
 # define ioswabq(a, x)		(x)
-# define __mem_ioswabq(a, x)	cpu_to_le32(x)
+# define __mem_ioswabq(a, x)	cpu_to_le64(x)
 
 #endif /* __ASM_MACH_IP27_MANGLE_PORT_H */
diff --git a/arch/mips/include/asm/sn/ioc3.h b/arch/mips/include/asm/sn/ioc3.h
index 89938fdce8ef..ac6d7e6e31e0 100644
--- a/arch/mips/include/asm/sn/ioc3.h
+++ b/arch/mips/include/asm/sn/ioc3.h
@@ -10,35 +10,35 @@
 
 /* serial port register map */
 struct ioc3_serialregs {
-	uint32_t	sscr;
-	uint32_t	stpir;
-	uint32_t	stcir;
-	uint32_t	srpir;
-	uint32_t	srcir;
-	uint32_t	srtr;
-	uint32_t	shadow;
+	u32	sscr;
+	u32	stpir;
+	u32	stcir;
+	u32	srpir;
+	u32	srcir;
+	u32	srtr;
+	u32	shadow;
 };
 
 /* SUPERIO uart register map */
 struct ioc3_uartregs {
+	u8	iu_lcr;
 	union {
-		char	rbr;	/* read only, DLAB == 0 */
-		char	thr;	/* write only, DLAB == 0 */
-		char	dll;	/* DLAB == 1 */
-	} u1;
+		u8	iir;	/* read only */
+		u8	fcr;	/* write only */
+	};
 	union {
-		char	ier;	/* DLAB == 0 */
-		char	dlm;	/* DLAB == 1 */
-	} u2;
+		u8	ier;	/* DLAB == 0 */
+		u8	dlm;	/* DLAB == 1 */
+	};
 	union {
-		char	iir;	/* read only */
-		char	fcr;	/* write only */
-	} u3;
-	char	iu_lcr;
-	char	iu_mcr;
-	char	iu_lsr;
-	char	iu_msr;
-	char	iu_scr;
+		u8	rbr;	/* read only, DLAB == 0 */
+		u8	thr;	/* write only, DLAB == 0 */
+		u8	dll;	/* DLAB == 1 */
+	} u1;
+	u8	iu_scr;
+	u8	iu_msr;
+	u8	iu_lsr;
+	u8	iu_mcr;
 };
 
 #define iu_rbr u1.rbr
@@ -50,122 +50,122 @@ struct ioc3_uartregs {
 #define iu_fcr u3.fcr
 
 struct ioc3_sioregs {
-	char	fill[0x141];	/* starts at 0x141 */
+	u8	fill[0x141];	/* starts at 0x141 */
 
-	char	uartc;
-	char	kbdcg;
+	u8	kbdcg;
+	u8	uartc;
 
-	char	fill0[0x150 - 0x142 - 1];
+	u8	fill0[0x151 - 0x142 - 1];
 
-	char	pp_data;
-	char	pp_dsr;
-	char	pp_dcr;
+	u8	pp_dcr;
+	u8	pp_dsr;
+	u8	pp_data;
 
-	char	fill1[0x158 - 0x152 - 1];
+	u8	fill1[0x159 - 0x153 - 1];
 
-	char	pp_fifa;
-	char	pp_cfgb;
-	char	pp_ecr;
+	u8	pp_ecr;
+	u8	pp_cfgb;
+	u8	pp_fifa;
 
-	char	fill2[0x168 - 0x15a - 1];
+	u8	fill2[0x16a - 0x15b - 1];
 
-	char	rtcad;
-	char	rtcdat;
+	u8	rtcdat;
+	u8	rtcad;
 
-	char	fill3[0x170 - 0x169 - 1];
+	u8	fill3[0x170 - 0x16b - 1];
 
 	struct ioc3_uartregs	uartb;	/* 0x20170  */
 	struct ioc3_uartregs	uarta;	/* 0x20178  */
 };
 
 struct ioc3_ethregs {
-	uint32_t	emcr;		/* 0x000f0  */
-	uint32_t	eisr;		/* 0x000f4  */
-	uint32_t	eier;		/* 0x000f8  */
-	uint32_t	ercsr;		/* 0x000fc  */
-	uint32_t	erbr_h;		/* 0x00100  */
-	uint32_t	erbr_l;		/* 0x00104  */
-	uint32_t	erbar;		/* 0x00108  */
-	uint32_t	ercir;		/* 0x0010c  */
-	uint32_t	erpir;		/* 0x00110  */
-	uint32_t	ertr;		/* 0x00114  */
-	uint32_t	etcsr;		/* 0x00118  */
-	uint32_t	ersr;		/* 0x0011c  */
-	uint32_t	etcdc;		/* 0x00120  */
-	uint32_t	ebir;		/* 0x00124  */
-	uint32_t	etbr_h;		/* 0x00128  */
-	uint32_t	etbr_l;		/* 0x0012c  */
-	uint32_t	etcir;		/* 0x00130  */
-	uint32_t	etpir;		/* 0x00134  */
-	uint32_t	emar_h;		/* 0x00138  */
-	uint32_t	emar_l;		/* 0x0013c  */
-	uint32_t	ehar_h;		/* 0x00140  */
-	uint32_t	ehar_l;		/* 0x00144  */
-	uint32_t	micr;		/* 0x00148  */
-	uint32_t	midr_r;		/* 0x0014c  */
-	uint32_t	midr_w;		/* 0x00150  */
+	u32	emcr;		/* 0x000f0  */
+	u32	eisr;		/* 0x000f4  */
+	u32	eier;		/* 0x000f8  */
+	u32	ercsr;		/* 0x000fc  */
+	u32	erbr_h;		/* 0x00100  */
+	u32	erbr_l;		/* 0x00104  */
+	u32	erbar;		/* 0x00108  */
+	u32	ercir;		/* 0x0010c  */
+	u32	erpir;		/* 0x00110  */
+	u32	ertr;		/* 0x00114  */
+	u32	etcsr;		/* 0x00118  */
+	u32	ersr;		/* 0x0011c  */
+	u32	etcdc;		/* 0x00120  */
+	u32	ebir;		/* 0x00124  */
+	u32	etbr_h;		/* 0x00128  */
+	u32	etbr_l;		/* 0x0012c  */
+	u32	etcir;		/* 0x00130  */
+	u32	etpir;		/* 0x00134  */
+	u32	emar_h;		/* 0x00138  */
+	u32	emar_l;		/* 0x0013c  */
+	u32	ehar_h;		/* 0x00140  */
+	u32	ehar_l;		/* 0x00144  */
+	u32	micr;		/* 0x00148  */
+	u32	midr_r;		/* 0x0014c  */
+	u32	midr_w;		/* 0x00150  */
 };
 
 struct ioc3_serioregs {
-	uint32_t	km_csr;		/* 0x0009c  */
-	uint32_t	k_rd;		/* 0x000a0  */
-	uint32_t	m_rd;		/* 0x000a4  */
-	uint32_t	k_wd;		/* 0x000a8  */
-	uint32_t	m_wd;		/* 0x000ac  */
+	u32	km_csr;		/* 0x0009c  */
+	u32	k_rd;		/* 0x000a0  */
+	u32	m_rd;		/* 0x000a4  */
+	u32	k_wd;		/* 0x000a8  */
+	u32	m_wd;		/* 0x000ac  */
 };
 
 /* Register layout of IOC3 in configuration space.  */
 struct ioc3 {
 	/* PCI Config Space registers  */
-	uint32_t	pci_id;		/* 0x00000  */
-	uint32_t	pci_scr;	/* 0x00004  */
-	uint32_t	pci_rev;	/* 0x00008  */
-	uint32_t	pci_lat;	/* 0x0000c  */
-	uint32_t	pci_addr;	/* 0x00010  */
-	uint32_t	pci_err_addr_l;	/* 0x00014  */
-	uint32_t	pci_err_addr_h;	/* 0x00018  */
-
-	uint32_t	sio_ir;		/* 0x0001c  */
-	uint32_t	sio_ies;	/* 0x00020  */
-	uint32_t	sio_iec;	/* 0x00024  */
-	uint32_t	sio_cr;		/* 0x00028  */
-	uint32_t	int_out;	/* 0x0002c  */
-	uint32_t	mcr;		/* 0x00030  */
+	u32	pci_id;		/* 0x00000  */
+	u32	pci_scr;	/* 0x00004  */
+	u32	pci_rev;	/* 0x00008  */
+	u32	pci_lat;	/* 0x0000c  */
+	u32	pci_addr;	/* 0x00010  */
+	u32	pci_err_addr_l;	/* 0x00014  */
+	u32	pci_err_addr_h;	/* 0x00018  */
+
+	u32	sio_ir;		/* 0x0001c  */
+	u32	sio_ies;	/* 0x00020  */
+	u32	sio_iec;	/* 0x00024  */
+	u32	sio_cr;		/* 0x00028  */
+	u32	int_out;	/* 0x0002c  */
+	u32	mcr;		/* 0x00030  */
 
 	/* General Purpose I/O registers  */
-	uint32_t	gpcr_s;		/* 0x00034  */
-	uint32_t	gpcr_c;		/* 0x00038  */
-	uint32_t	gpdr;		/* 0x0003c  */
-	uint32_t	gppr[16];	/* 0x00040  */
+	u32	gpcr_s;		/* 0x00034  */
+	u32	gpcr_c;		/* 0x00038  */
+	u32	gpdr;		/* 0x0003c  */
+	u32	gppr[16];	/* 0x00040  */
 
 	/* Parallel Port Registers  */
-	uint32_t	ppbr_h_a;	/* 0x00080  */
-	uint32_t	ppbr_l_a;	/* 0x00084  */
-	uint32_t	ppcr_a;		/* 0x00088  */
-	uint32_t	ppcr;		/* 0x0008c  */
-	uint32_t	ppbr_h_b;	/* 0x00090  */
-	uint32_t	ppbr_l_b;	/* 0x00094  */
-	uint32_t	ppcr_b;		/* 0x00098  */
+	u32	ppbr_h_a;	/* 0x00080  */
+	u32	ppbr_l_a;	/* 0x00084  */
+	u32	ppcr_a;		/* 0x00088  */
+	u32	ppcr;		/* 0x0008c  */
+	u32	ppbr_h_b;	/* 0x00090  */
+	u32	ppbr_l_b;	/* 0x00094  */
+	u32	ppcr_b;		/* 0x00098  */
 
 	/* Keyboard and Mouse Registers	 */
 	struct ioc3_serioregs	serio;
 
 	/* Serial Port Registers  */
-	uint32_t	sbbr_h;		/* 0x000b0  */
-	uint32_t	sbbr_l;		/* 0x000b4  */
+	u32	sbbr_h;		/* 0x000b0  */
+	u32	sbbr_l;		/* 0x000b4  */
 	struct ioc3_serialregs	port_a;
 	struct ioc3_serialregs	port_b;
 
 	/* Ethernet Registers */
 	struct ioc3_ethregs	eth;
-	uint32_t	pad1[(0x20000 - 0x00154) / 4];
+	u32	pad1[(0x20000 - 0x00154) / 4];
 
 	/* SuperIO Registers  XXX */
 	struct ioc3_sioregs	sregs;	/* 0x20000 */
-	uint32_t	pad2[(0x40000 - 0x20180) / 4];
+	u32	pad2[(0x40000 - 0x20180) / 4];
 
 	/* SSRAM Diagnostic Access */
-	uint32_t	ssram[(0x80000 - 0x40000) / 4];
+	u32	ssram[(0x80000 - 0x40000) / 4];
 
 	/* Bytebus device offsets
 	   0x80000 -   Access to the generic devices selected with   DEV0
@@ -598,10 +598,6 @@ struct ioc3_etxd {
 
 #define MIDR_DATA_MASK		0x0000ffff
 
-#if defined(CONFIG_SGI_IP27) || defined(CONFIG_SGI_IP30)
-extern int bridge_alloc_irq(struct pci_dev *dev);
-#endif
-
 /* subsystem IDs supplied by card detection in pci-xtalk-bridge */
 #define	IOC3_SUBSYS_IP27_BASEIO6G	0xc300
 #define	IOC3_SUBSYS_IP27_MIO		0xc301
diff --git a/arch/mips/sgi-ip27/ip27-console.c b/arch/mips/sgi-ip27/ip27-console.c
index 6bdb48d41276..5886bee89d06 100644
--- a/arch/mips/sgi-ip27/ip27-console.c
+++ b/arch/mips/sgi-ip27/ip27-console.c
@@ -35,6 +35,7 @@ void prom_putchar(char c)
 {
 	struct ioc3_uartregs *uart = console_uart();
 
-	while ((uart->iu_lsr & 0x20) == 0);
-	uart->iu_thr = c;
+	while ((readb(&uart->iu_lsr) & 0x20) == 0)
+		;
+	writeb(c, &uart->iu_thr);
 }
diff --git a/drivers/rtc/rtc-m48t35.c b/drivers/rtc/rtc-m48t35.c
index d3a75d447fce..e8194f1f01a8 100644
--- a/drivers/rtc/rtc-m48t35.c
+++ b/drivers/rtc/rtc-m48t35.c
@@ -20,6 +20,16 @@
 
 struct m48t35_rtc {
 	u8	pad[0x7ff8];    /* starts at 0x7ff8 */
+#ifdef CONFIG_SGI_IP27
+	u8	hour;
+	u8	min;
+	u8	sec;
+	u8	control;
+	u8	year;
+	u8	month;
+	u8	date;
+	u8	day;
+#else
 	u8	control;
 	u8	sec;
 	u8	min;
@@ -28,6 +38,7 @@ struct m48t35_rtc {
 	u8	date;
 	u8	month;
 	u8	year;
+#endif
 };
 
 #define M48T35_RTC_SET		0x80
diff --git a/drivers/tty/serial/8250/8250_ioc3.c b/drivers/tty/serial/8250/8250_ioc3.c
index 2be6ed2967e0..4c405f1b9c67 100644
--- a/drivers/tty/serial/8250/8250_ioc3.c
+++ b/drivers/tty/serial/8250/8250_ioc3.c
@@ -23,12 +23,12 @@ struct ioc3_8250_data {
 
 static unsigned int ioc3_serial_in(struct uart_port *p, int offset)
 {
-	return readb(p->membase + offset);
+	return readb(p->membase + (offset ^ 3));
 }
 
 static void ioc3_serial_out(struct uart_port *p, int offset, int value)
 {
-	writeb(value, p->membase + offset);
+	writeb(value, p->membase + (offset ^ 3));
 }
 
 static int serial8250_ioc3_probe(struct platform_device *pdev)
-- 
2.13.7


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

* [PATCH v3 7/7] Input: add IOC3 serio driver
  2019-06-13 17:06 [PATCH v3 0/7] Use MFD framework for SGI IOC3 drivers Thomas Bogendoerfer
                   ` (4 preceding siblings ...)
  2019-06-13 17:06 ` [PATCH v3 6/7] MIPS: SGI-IP27: fix readb/writeb addressing Thomas Bogendoerfer
@ 2019-06-13 17:06 ` Thomas Bogendoerfer
  2019-07-01  8:19   ` Dmitry Torokhov
       [not found] ` <20190613170636.6647-6-tbogendoerfer@suse.de>
  6 siblings, 1 reply; 13+ messages in thread
From: Thomas Bogendoerfer @ 2019-06-13 17:06 UTC (permalink / raw)
  To: Ralf Baechle, Paul Burton, James Hogan, Dmitry Torokhov,
	Lee Jones, David S. Miller, Srinivas Kandagatla,
	Alessandro Zummo, Alexandre Belloni, Greg Kroah-Hartman,
	Jiri Slaby, linux-mips, linux-kernel, linux-input, netdev,
	linux-rtc, linux-serial

This patch adds a platform driver for supporting keyboard and mouse
interface of SGI IOC3 chips.

Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
---
 drivers/input/serio/Kconfig   |  10 +++
 drivers/input/serio/Makefile  |   1 +
 drivers/input/serio/ioc3kbd.c | 158 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 169 insertions(+)
 create mode 100644 drivers/input/serio/ioc3kbd.c

diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index f3e18f8ef9ca..373a1646019e 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -165,6 +165,16 @@ config SERIO_MACEPS2
 	  To compile this driver as a module, choose M here: the
 	  module will be called maceps2.
 
+config SERIO_SGI_IOC3
+	tristate "SGI IOC3 PS/2 controller"
+	depends on SGI_MFD_IOC3
+	help
+	  Say Y here if you have an SGI Onyx2, SGI Octane or IOC3 PCI card
+	  and you want to attach and use a keyboard, mouse, or both.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ioc3kbd.
+
 config SERIO_LIBPS2
 	tristate "PS/2 driver library"
 	depends on SERIO_I8042 || SERIO_I8042=n
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index 67950a5ccb3f..6d97bad7b844 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_HIL_MLC)		+= hp_sdc_mlc.o hil_mlc.o
 obj-$(CONFIG_SERIO_PCIPS2)	+= pcips2.o
 obj-$(CONFIG_SERIO_PS2MULT)	+= ps2mult.o
 obj-$(CONFIG_SERIO_MACEPS2)	+= maceps2.o
+obj-$(CONFIG_SERIO_SGI_IOC3)	+= ioc3kbd.o
 obj-$(CONFIG_SERIO_LIBPS2)	+= libps2.o
 obj-$(CONFIG_SERIO_RAW)		+= serio_raw.o
 obj-$(CONFIG_SERIO_AMS_DELTA)	+= ams_delta_serio.o
diff --git a/drivers/input/serio/ioc3kbd.c b/drivers/input/serio/ioc3kbd.c
new file mode 100644
index 000000000000..26fcf57465d6
--- /dev/null
+++ b/drivers/input/serio/ioc3kbd.c
@@ -0,0 +1,158 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * SGI IOC3 PS/2 controller driver for linux
+ *
+ * Copyright (C) 2019 Thomas Bogendoerfer <tbogendoerfer@suse.de>
+ *
+ * Based on code Copyright (C) 2005 Stanislaw Skowronek <skylark@unaligned.org>
+ *               Copyright (C) 2009 Johannes Dickgreber <tanzy@gmx.de>
+ */
+
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/serio.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#include <asm/sn/ioc3.h>
+
+struct ioc3kbd_data {
+	struct ioc3_serioregs __iomem *regs;
+	struct serio *kbd, *aux;
+};
+
+static int ioc3kbd_write(struct serio *dev, u8 val)
+{
+	struct ioc3kbd_data *d = dev->port_data;
+	unsigned long timeout = 0;
+	u32 mask;
+
+	mask = (dev == d->aux) ? KM_CSR_M_WRT_PEND : KM_CSR_K_WRT_PEND;
+	while ((readl(&d->regs->km_csr) & mask) && (timeout < 1000)) {
+		udelay(100);
+		timeout++;
+	}
+
+	if (timeout >= 1000)
+		return -1;
+
+	writel(val, dev == d->aux ?  &d->regs->m_wd : &d->regs->k_wd);
+
+	return 0;
+}
+
+static irqreturn_t ioc3kbd_intr(int itq, void *dev_id)
+{
+	struct ioc3kbd_data *d = dev_id;
+	u32 data_k, data_m;
+
+	data_k = readl(&d->regs->k_rd);
+	data_m = readl(&d->regs->m_rd);
+
+	if (data_k & KM_RD_VALID_0)
+		serio_interrupt(d->kbd,
+		(data_k >> KM_RD_DATA_0_SHIFT) & 0xff, 0);
+	if (data_k & KM_RD_VALID_1)
+		serio_interrupt(d->kbd,
+		(data_k >> KM_RD_DATA_1_SHIFT) & 0xff, 0);
+	if (data_k & KM_RD_VALID_2)
+		serio_interrupt(d->kbd,
+		(data_k >> KM_RD_DATA_2_SHIFT) & 0xff, 0);
+	if (data_m & KM_RD_VALID_0)
+		serio_interrupt(d->aux,
+		(data_m >> KM_RD_DATA_0_SHIFT) & 0xff, 0);
+	if (data_m & KM_RD_VALID_1)
+		serio_interrupt(d->aux,
+		(data_m >> KM_RD_DATA_1_SHIFT) & 0xff, 0);
+	if (data_m & KM_RD_VALID_2)
+		serio_interrupt(d->aux,
+		(data_m >> KM_RD_DATA_2_SHIFT) & 0xff, 0);
+
+	return 0;
+}
+
+static int ioc3kbd_probe(struct platform_device *pdev)
+{
+	struct ioc3_serioregs __iomem *regs;
+	struct device *dev = &pdev->dev;
+	struct ioc3kbd_data *d;
+	struct serio *sk, *sa;
+	struct resource *mem;
+	int irq, ret;
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	regs = devm_ioremap_resource(&pdev->dev, mem);
+	if (IS_ERR(regs))
+		return PTR_ERR(regs);
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return -ENXIO;
+
+	d = devm_kzalloc(&pdev->dev, sizeof(struct ioc3kbd_data), GFP_KERNEL);
+	if (!d)
+		return -ENOMEM;
+
+	ret = devm_request_irq(&pdev->dev, irq, ioc3kbd_intr, IRQF_SHARED,
+			       "ioc3-kbd", d);
+	if (ret) {
+		dev_err(&pdev->dev, "could not request IRQ %d\n", irq);
+		return ret;
+	}
+
+	sk = kzalloc(sizeof(struct serio), GFP_KERNEL);
+	if (!sk)
+		return -ENOMEM;
+
+	sa = kzalloc(sizeof(struct serio), GFP_KERNEL);
+	if (!sa) {
+		kfree(sk);
+		return -ENOMEM;
+	}
+
+	sk->id.type = SERIO_8042;
+	sk->write = ioc3kbd_write;
+	snprintf(sk->name, sizeof(sk->name), "IOC3 keyboard %d", pdev->id);
+	snprintf(sk->phys, sizeof(sk->phys), "ioc3/serio%dkbd", pdev->id);
+	sk->port_data = d;
+	sk->dev.parent = &pdev->dev;
+
+	sa->id.type = SERIO_8042;
+	sa->write = ioc3kbd_write;
+	snprintf(sa->name, sizeof(sa->name), "IOC3 auxiliary %d", pdev->id);
+	snprintf(sa->phys, sizeof(sa->phys), "ioc3/serio%daux", pdev->id);
+	sa->port_data = d;
+	sa->dev.parent = dev;
+
+	d->regs = regs;
+	d->kbd = sk;
+	d->aux = sa;
+
+	platform_set_drvdata(pdev, d);
+	serio_register_port(d->kbd);
+	serio_register_port(d->aux);
+	return 0;
+}
+
+static int ioc3kbd_remove(struct platform_device *pdev)
+{
+	struct ioc3kbd_data *d = platform_get_drvdata(pdev);
+
+	serio_unregister_port(d->kbd);
+	serio_unregister_port(d->aux);
+	return 0;
+}
+
+static struct platform_driver ioc3kbd_driver = {
+	.probe          = ioc3kbd_probe,
+	.remove         = ioc3kbd_remove,
+	.driver = {
+		.name = "ioc3-kbd",
+	},
+};
+module_platform_driver(ioc3kbd_driver);
+
+MODULE_AUTHOR("Thomas Bogendoerfer <tbogendoerfer@suse.de>");
+MODULE_DESCRIPTION("SGI IOC3 serio driver");
+MODULE_LICENSE("GPL");
-- 
2.13.7


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

* Re: [PATCH v3 5/7] mfd: ioc3: Add driver for SGI IOC3 chip
       [not found] ` <20190613170636.6647-6-tbogendoerfer@suse.de>
@ 2019-06-25  9:04   ` Thomas Bogendoerfer
  2019-06-25 12:56     ` Lee Jones
  2019-07-25 11:47   ` Lee Jones
  1 sibling, 1 reply; 13+ messages in thread
From: Thomas Bogendoerfer @ 2019-06-25  9:04 UTC (permalink / raw)
  To: Lee Jones, linux-mips, linux-kernel

On Thu, Jun 13, 2019 at 07:06:31PM +0200, Thomas Bogendoerfer wrote:
> SGI IOC3 chip has integrated ethernet, keyboard and mouse interface.
> It also supports connecting a SuperIO chip for serial and parallel
> interfaces. IOC3 is used inside various SGI systemboards and add-on
> cards with different equipped external interfaces.
> 
> Support for ethernet and serial interfaces were implemented inside
> the network driver. This patchset moves out the not network related
> parts to a new MFD driver, which takes care of card detection,
> setup of platform devices and interrupt distribution for the subdevices.
> 
> Serial portion: Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
> ---
>  arch/mips/include/asm/sn/ioc3.h     |  345 +++----
>  arch/mips/sgi-ip27/ip27-timer.c     |   20 -
>  drivers/mfd/Kconfig                 |   13 +
>  drivers/mfd/Makefile                |    1 +
>  drivers/mfd/ioc3.c                  |  683 +++++++++++++

Lee,

can you give me an indication, if the MFD changes are ok now
or if I need to improve it further.

Thanks,
Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

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

* Re: [PATCH v3 5/7] mfd: ioc3: Add driver for SGI IOC3 chip
  2019-06-25  9:04   ` [PATCH v3 5/7] mfd: ioc3: Add driver for SGI IOC3 chip Thomas Bogendoerfer
@ 2019-06-25 12:56     ` Lee Jones
  0 siblings, 0 replies; 13+ messages in thread
From: Lee Jones @ 2019-06-25 12:56 UTC (permalink / raw)
  To: Thomas Bogendoerfer; +Cc: linux-mips, linux-kernel

On Tue, 25 Jun 2019, Thomas Bogendoerfer wrote:

> On Thu, Jun 13, 2019 at 07:06:31PM +0200, Thomas Bogendoerfer wrote:
> > SGI IOC3 chip has integrated ethernet, keyboard and mouse interface.
> > It also supports connecting a SuperIO chip for serial and parallel
> > interfaces. IOC3 is used inside various SGI systemboards and add-on
> > cards with different equipped external interfaces.
> > 
> > Support for ethernet and serial interfaces were implemented inside
> > the network driver. This patchset moves out the not network related
> > parts to a new MFD driver, which takes care of card detection,
> > setup of platform devices and interrupt distribution for the subdevices.
> > 
> > Serial portion: Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > 
> > Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
> > ---
> >  arch/mips/include/asm/sn/ioc3.h     |  345 +++----
> >  arch/mips/sgi-ip27/ip27-timer.c     |   20 -
> >  drivers/mfd/Kconfig                 |   13 +
> >  drivers/mfd/Makefile                |    1 +
> >  drivers/mfd/ioc3.c                  |  683 +++++++++++++
> 
> Lee,
> 
> can you give me an indication, if the MFD changes are ok now
> or if I need to improve it further.

I will do, when I get to them.

My review list currently runs into the 50s.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH v3 7/7] Input: add IOC3 serio driver
  2019-06-13 17:06 ` [PATCH v3 7/7] Input: add IOC3 serio driver Thomas Bogendoerfer
@ 2019-07-01  8:19   ` Dmitry Torokhov
  0 siblings, 0 replies; 13+ messages in thread
From: Dmitry Torokhov @ 2019-07-01  8:19 UTC (permalink / raw)
  To: Thomas Bogendoerfer
  Cc: Ralf Baechle, Paul Burton, James Hogan, Lee Jones,
	David S. Miller, Srinivas Kandagatla, Alessandro Zummo,
	Alexandre Belloni, Greg Kroah-Hartman, Jiri Slaby, linux-mips,
	linux-kernel, linux-input, netdev, linux-rtc, linux-serial

Hi Thomas,

On Thu, Jun 13, 2019 at 07:06:33PM +0200, Thomas Bogendoerfer wrote:
> This patch adds a platform driver for supporting keyboard and mouse
> interface of SGI IOC3 chips.
> 
> Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
> ---
>  drivers/input/serio/Kconfig   |  10 +++
>  drivers/input/serio/Makefile  |   1 +
>  drivers/input/serio/ioc3kbd.c | 158 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 169 insertions(+)
>  create mode 100644 drivers/input/serio/ioc3kbd.c
> 
> diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
> index f3e18f8ef9ca..373a1646019e 100644
> --- a/drivers/input/serio/Kconfig
> +++ b/drivers/input/serio/Kconfig
> @@ -165,6 +165,16 @@ config SERIO_MACEPS2
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called maceps2.
>  
> +config SERIO_SGI_IOC3
> +	tristate "SGI IOC3 PS/2 controller"
> +	depends on SGI_MFD_IOC3
> +	help
> +	  Say Y here if you have an SGI Onyx2, SGI Octane or IOC3 PCI card
> +	  and you want to attach and use a keyboard, mouse, or both.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ioc3kbd.
> +
>  config SERIO_LIBPS2
>  	tristate "PS/2 driver library"
>  	depends on SERIO_I8042 || SERIO_I8042=n
> diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
> index 67950a5ccb3f..6d97bad7b844 100644
> --- a/drivers/input/serio/Makefile
> +++ b/drivers/input/serio/Makefile
> @@ -20,6 +20,7 @@ obj-$(CONFIG_HIL_MLC)		+= hp_sdc_mlc.o hil_mlc.o
>  obj-$(CONFIG_SERIO_PCIPS2)	+= pcips2.o
>  obj-$(CONFIG_SERIO_PS2MULT)	+= ps2mult.o
>  obj-$(CONFIG_SERIO_MACEPS2)	+= maceps2.o
> +obj-$(CONFIG_SERIO_SGI_IOC3)	+= ioc3kbd.o
>  obj-$(CONFIG_SERIO_LIBPS2)	+= libps2.o
>  obj-$(CONFIG_SERIO_RAW)		+= serio_raw.o
>  obj-$(CONFIG_SERIO_AMS_DELTA)	+= ams_delta_serio.o
> diff --git a/drivers/input/serio/ioc3kbd.c b/drivers/input/serio/ioc3kbd.c
> new file mode 100644
> index 000000000000..26fcf57465d6
> --- /dev/null
> +++ b/drivers/input/serio/ioc3kbd.c
> @@ -0,0 +1,158 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * SGI IOC3 PS/2 controller driver for linux
> + *
> + * Copyright (C) 2019 Thomas Bogendoerfer <tbogendoerfer@suse.de>
> + *
> + * Based on code Copyright (C) 2005 Stanislaw Skowronek <skylark@unaligned.org>
> + *               Copyright (C) 2009 Johannes Dickgreber <tanzy@gmx.de>
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/serio.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +
> +#include <asm/sn/ioc3.h>
> +
> +struct ioc3kbd_data {
> +	struct ioc3_serioregs __iomem *regs;
> +	struct serio *kbd, *aux;
> +};
> +
> +static int ioc3kbd_write(struct serio *dev, u8 val)
> +{
> +	struct ioc3kbd_data *d = dev->port_data;
> +	unsigned long timeout = 0;
> +	u32 mask;
> +
> +	mask = (dev == d->aux) ? KM_CSR_M_WRT_PEND : KM_CSR_K_WRT_PEND;
> +	while ((readl(&d->regs->km_csr) & mask) && (timeout < 1000)) {
> +		udelay(100);
> +		timeout++;
> +	}
> +
> +	if (timeout >= 1000)
> +		return -1;

-ETIMEDOUT?

> +
> +	writel(val, dev == d->aux ?  &d->regs->m_wd : &d->regs->k_wd);

Nit: there are 2 spaces after ?, only one is needed.

> +
> +	return 0;
> +}
> +
> +static irqreturn_t ioc3kbd_intr(int itq, void *dev_id)
> +{
> +	struct ioc3kbd_data *d = dev_id;
> +	u32 data_k, data_m;
> +
> +	data_k = readl(&d->regs->k_rd);
> +	data_m = readl(&d->regs->m_rd);
> +
> +	if (data_k & KM_RD_VALID_0)
> +		serio_interrupt(d->kbd,
> +		(data_k >> KM_RD_DATA_0_SHIFT) & 0xff, 0);

This is weird formatting, you need one more tab here.

> +	if (data_k & KM_RD_VALID_1)
> +		serio_interrupt(d->kbd,
> +		(data_k >> KM_RD_DATA_1_SHIFT) & 0xff, 0);
> +	if (data_k & KM_RD_VALID_2)
> +		serio_interrupt(d->kbd,
> +		(data_k >> KM_RD_DATA_2_SHIFT) & 0xff, 0);
> +	if (data_m & KM_RD_VALID_0)
> +		serio_interrupt(d->aux,
> +		(data_m >> KM_RD_DATA_0_SHIFT) & 0xff, 0);
> +	if (data_m & KM_RD_VALID_1)
> +		serio_interrupt(d->aux,
> +		(data_m >> KM_RD_DATA_1_SHIFT) & 0xff, 0);
> +	if (data_m & KM_RD_VALID_2)
> +		serio_interrupt(d->aux,
> +		(data_m >> KM_RD_DATA_2_SHIFT) & 0xff, 0);
> +
> +	return 0;
> +}
> +
> +static int ioc3kbd_probe(struct platform_device *pdev)
> +{
> +	struct ioc3_serioregs __iomem *regs;
> +	struct device *dev = &pdev->dev;
> +	struct ioc3kbd_data *d;
> +	struct serio *sk, *sa;
> +	struct resource *mem;
> +	int irq, ret;
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	regs = devm_ioremap_resource(&pdev->dev, mem);

We have a brand new helper: devm_platform_ioremap_resource()

> +	if (IS_ERR(regs))
> +		return PTR_ERR(regs);
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0)
> +		return -ENXIO;
> +
> +	d = devm_kzalloc(&pdev->dev, sizeof(struct ioc3kbd_data), GFP_KERNEL);

I think we nor prefer deriving type from the pointer:

	d = evm_kzalloc(&pdev->dev, sizeof(*d), GFP_KERNEL);

> +	if (!d)
> +		return -ENOMEM;
> +
> +	ret = devm_request_irq(&pdev->dev, irq, ioc3kbd_intr, IRQF_SHARED,
> +			       "ioc3-kbd", d);
> +	if (ret) {
> +		dev_err(&pdev->dev, "could not request IRQ %d\n", irq);
> +		return ret;
> +	}

You need to make sure that interrupt will not fire while serio ports are
not yet allocated/registered. Is there a way to inhibit interrupt
generation on controller side?

> +
> +	sk = kzalloc(sizeof(struct serio), GFP_KERNEL);

	sk = kzalloc(sizeof(*sk), GFP_KERNEL);

> +	if (!sk)
> +		return -ENOMEM;
> +
> +	sa = kzalloc(sizeof(struct serio), GFP_KERNEL);

	sa = kzalloc(sizeof(*sa), GFP_KERNEL);

> +	if (!sa) {
> +		kfree(sk);
> +		return -ENOMEM;
> +	}
> +
> +	sk->id.type = SERIO_8042;
> +	sk->write = ioc3kbd_write;
> +	snprintf(sk->name, sizeof(sk->name), "IOC3 keyboard %d", pdev->id);
> +	snprintf(sk->phys, sizeof(sk->phys), "ioc3/serio%dkbd", pdev->id);
> +	sk->port_data = d;
> +	sk->dev.parent = &pdev->dev;
> +
> +	sa->id.type = SERIO_8042;
> +	sa->write = ioc3kbd_write;
> +	snprintf(sa->name, sizeof(sa->name), "IOC3 auxiliary %d", pdev->id);
> +	snprintf(sa->phys, sizeof(sa->phys), "ioc3/serio%daux", pdev->id);
> +	sa->port_data = d;
> +	sa->dev.parent = dev;
> +
> +	d->regs = regs;
> +	d->kbd = sk;
> +	d->aux = sa;
> +
> +	platform_set_drvdata(pdev, d);
> +	serio_register_port(d->kbd);
> +	serio_register_port(d->aux);
> +	return 0;
> +}
> +
> +static int ioc3kbd_remove(struct platform_device *pdev)
> +{
> +	struct ioc3kbd_data *d = platform_get_drvdata(pdev);
> +
> +	serio_unregister_port(d->kbd);
> +	serio_unregister_port(d->aux);

If you unregister ports while interrupt is registered/enabled you may
get a crash.

> +	return 0;
> +}
> +
> +static struct platform_driver ioc3kbd_driver = {
> +	.probe          = ioc3kbd_probe,
> +	.remove         = ioc3kbd_remove,
> +	.driver = {
> +		.name = "ioc3-kbd",
> +	},
> +};
> +module_platform_driver(ioc3kbd_driver);
> +
> +MODULE_AUTHOR("Thomas Bogendoerfer <tbogendoerfer@suse.de>");
> +MODULE_DESCRIPTION("SGI IOC3 serio driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.13.7
> 

-- 
Dmitry

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

* Re: [PATCH v3 5/7] mfd: ioc3: Add driver for SGI IOC3 chip
       [not found] ` <20190613170636.6647-6-tbogendoerfer@suse.de>
  2019-06-25  9:04   ` [PATCH v3 5/7] mfd: ioc3: Add driver for SGI IOC3 chip Thomas Bogendoerfer
@ 2019-07-25 11:47   ` Lee Jones
  2019-07-29 18:45     ` Thomas Bogendoerfer
  1 sibling, 1 reply; 13+ messages in thread
From: Lee Jones @ 2019-07-25 11:47 UTC (permalink / raw)
  To: Thomas Bogendoerfer
  Cc: Ralf Baechle, Paul Burton, James Hogan, Dmitry Torokhov,
	David S. Miller, Srinivas Kandagatla, Alessandro Zummo,
	Alexandre Belloni, Greg Kroah-Hartman, Jiri Slaby, linux-mips,
	linux-kernel, linux-input, netdev, linux-rtc, linux-serial

On Thu, 13 Jun 2019, Thomas Bogendoerfer wrote:

> SGI IOC3 chip has integrated ethernet, keyboard and mouse interface.
> It also supports connecting a SuperIO chip for serial and parallel
> interfaces. IOC3 is used inside various SGI systemboards and add-on
> cards with different equipped external interfaces.
> 
> Support for ethernet and serial interfaces were implemented inside
> the network driver. This patchset moves out the not network related
> parts to a new MFD driver, which takes care of card detection,
> setup of platform devices and interrupt distribution for the subdevices.
> 
> Serial portion: Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
> ---
>  arch/mips/include/asm/sn/ioc3.h     |  345 +++----
>  arch/mips/sgi-ip27/ip27-timer.c     |   20 -

>  drivers/mfd/Kconfig                 |   13 +
>  drivers/mfd/Makefile                |    1 +
>  drivers/mfd/ioc3.c                  |  683 +++++++++++++

A vast improvement, but still a little work to do.

>  drivers/net/ethernet/sgi/Kconfig    |    4 +-
>  drivers/net/ethernet/sgi/ioc3-eth.c | 1932 ++++++++++++++---------------------
>  drivers/tty/serial/8250/8250_ioc3.c |   98 ++
>  drivers/tty/serial/8250/Kconfig     |   11 +
>  drivers/tty/serial/8250/Makefile    |    1 +
>  10 files changed, 1693 insertions(+), 1415 deletions(-)
>  create mode 100644 drivers/mfd/ioc3.c
>  create mode 100644 drivers/tty/serial/8250/8250_ioc3.c

[...]

> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index a17d275bf1d4..5c9f1bd9bd0a 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1989,5 +1989,18 @@ config RAVE_SP_CORE
>  	  Select this to get support for the Supervisory Processor
>  	  device found on several devices in RAVE line of hardware.
>  
> +config SGI_MFD_IOC3
> +	tristate "SGI IOC3 core driver"
> +	depends on PCI && MIPS
> +	select MFD_CORE
> +	help
> +	  This option enables basic support for the SGI IOC3-based
> +	  controller cards.  This option does not enable any specific
> +	  functions on such a card, but provides necessary infrastructure
> +	  for other drivers to utilize.
> +
> +	  If you have an SGI Origin, Octane, or a PCI IOC3 card,
> +	  then say Y. Otherwise say N.
> +
>  endmenu
>  endif
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 52b1a90ff515..bba0d9eb0b18 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -249,3 +249,4 @@ obj-$(CONFIG_MFD_SC27XX_PMIC)	+= sprd-sc27xx-spi.o
>  obj-$(CONFIG_RAVE_SP_CORE)	+= rave-sp.o
>  obj-$(CONFIG_MFD_ROHM_BD718XX)	+= rohm-bd718x7.o
>  obj-$(CONFIG_MFD_STMFX) 	+= stmfx.o
> +obj-$(CONFIG_SGI_MFD_IOC3)	+= ioc3.o
> diff --git a/drivers/mfd/ioc3.c b/drivers/mfd/ioc3.c
> new file mode 100644
> index 000000000000..0c0d1b3475d0
> --- /dev/null
> +++ b/drivers/mfd/ioc3.c
> @@ -0,0 +1,683 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * SGI IOC3 multifunction device driver
> + *
> + * Copyright (C) 2018, 2019 Thomas Bogendoerfer <tbogendoerfer@suse.de>
> + *
> + * Based on work by:
> + *   Stanislaw Skowronek <skylark@unaligned.org>
> + *   Joshua Kinard <kumba@gentoo.org>
> + *   Brent Casavant <bcasavan@sgi.com> - IOC4 master driver
> + *   Pat Gefre <pfg@sgi.com> - IOC3 serial port IRQ demuxer
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/errno.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/core.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/platform_data/sgi-w1.h>
> +#include <linux/rtc/ds1685.h>
> +
> +#include <asm/pci/bridge.h>
> +#include <asm/sn/ioc3.h>
> +
> +#define IOC3_IRQ_SERIAL_A	6
> +#define IOC3_IRQ_SERIAL_B	15
> +#define IOC3_IRQ_KBD		22
> +#define IOC3_IRQ_ETH_DOMAIN	23
> +
> +static int ioc3_serial_id;
> +static int ioc3_eth_id;
> +static int ioc3_kbd_id;
> +
> +struct ioc3_priv_data {
> +	struct irq_domain *domain;
> +	struct ioc3 __iomem *regs;
> +	struct pci_dev *pdev;
> +	int domain_irq;
> +};
> +
> +static void ioc3_irq_ack(struct irq_data *d)
> +{
> +	struct ioc3_priv_data *ipd = irq_data_get_irq_chip_data(d);
> +	unsigned int hwirq = irqd_to_hwirq(d);
> +
> +	writel(BIT(hwirq), &ipd->regs->sio_ir);
> +}
> +
> +static void ioc3_irq_mask(struct irq_data *d)
> +{
> +	struct ioc3_priv_data *ipd = irq_data_get_irq_chip_data(d);
> +	unsigned int hwirq = irqd_to_hwirq(d);
> +
> +	writel(BIT(hwirq), &ipd->regs->sio_iec);
> +}
> +
> +static void ioc3_irq_unmask(struct irq_data *d)
> +{
> +	struct ioc3_priv_data *ipd = irq_data_get_irq_chip_data(d);
> +	unsigned int hwirq = irqd_to_hwirq(d);
> +
> +	writel(BIT(hwirq), &ipd->regs->sio_ies);
> +}
> +
> +static struct irq_chip ioc3_irq_chip = {
> +	.name		= "IOC3",
> +	.irq_ack	= ioc3_irq_ack,
> +	.irq_mask	= ioc3_irq_mask,
> +	.irq_unmask	= ioc3_irq_unmask,
> +};
> +
> +#define IOC3_LVL_MASK	(BIT(IOC3_IRQ_SERIAL_A) | BIT(IOC3_IRQ_SERIAL_B))
> +
> +static int ioc3_irq_domain_map(struct irq_domain *d, unsigned int irq,
> +			      irq_hw_number_t hwirq)
> +{
> +	/* use level irqs for every interrupt contained in IOC3_LVL_MASK */

Nit: Could you use proper grammar (less the full stops) in the
comments please?  Start with an uppercase character and things like
"irq" should be "IRQ", etc.

> +	if (BIT(hwirq) & IOC3_LVL_MASK)
> +		irq_set_chip_and_handler(irq, &ioc3_irq_chip, handle_level_irq);
> +	else
> +		irq_set_chip_and_handler(irq, &ioc3_irq_chip, handle_edge_irq);
> +
> +	irq_set_chip_data(irq, d->host_data);
> +	return 0;
> +}
> +
> +static const struct irq_domain_ops ioc3_irq_domain_ops = {
> +	.map = ioc3_irq_domain_map,
> +};
> +
> +static void ioc3_irq_handler(struct irq_desc *desc)
> +{
> +	struct irq_domain *domain = irq_desc_get_handler_data(desc);
> +	struct ioc3_priv_data *ipd = domain->host_data;
> +	struct ioc3 __iomem *regs = ipd->regs;
> +	u32 pending, mask;
> +	unsigned int irq;
> +
> +	pending = readl(&regs->sio_ir);
> +	mask = readl(&regs->sio_ies);
> +	pending &= mask; /* mask off not enabled but pending irqs */
> +
> +	if (mask & BIT(IOC3_IRQ_ETH_DOMAIN))
> +		/* if eth irq is enabled we need to check in eth irq regs */
> +		if (readl(&regs->eth.eisr) & readl(&regs->eth.eier))
> +			pending |= IOC3_IRQ_ETH_DOMAIN;
> +
> +	if (pending) {
> +		irq = irq_find_mapping(domain, __ffs(pending));
> +		if (irq)
> +			generic_handle_irq(irq);
> +	} else  {
> +		spurious_interrupt();
> +	}
> +}
> +
> +/*
> + * System boards/BaseIOs use more interrupt pins of the bridge asic

"ASIC"

> + * to which the IOC3 is connected. Since the IOC3 MFD driver
> + * knows wiring of these extra pins, we use the map_irq function
> + * to get interrupts activated
> + */
> +static int ioc3_map_irq(struct pci_dev *pdev, int pin)
> +{
> +	struct pci_host_bridge *hbrg = pci_find_host_bridge(pdev->bus);
> +
> +	return hbrg->map_irq(pdev, pin, 0);
> +}
> +
> +static int ioc3_irq_domain_setup(struct ioc3_priv_data *ipd, int irq)
> +{
> +	struct irq_domain *domain;
> +	struct fwnode_handle *fn;
> +
> +	fn = irq_domain_alloc_named_fwnode("IOC3");
> +	if (!fn)
> +		goto err;
> +
> +	domain = irq_domain_create_linear(fn, 24, &ioc3_irq_domain_ops, ipd);
> +	if (!domain)
> +		goto err;
> +
> +	irq_domain_free_fwnode(fn);
> +	ipd->domain = domain;
> +
> +	irq_set_chained_handler_and_data(irq, ioc3_irq_handler, domain);
> +	ipd->domain_irq = irq;
> +	return 0;
> +err:
> +	dev_err(&ipd->pdev->dev, "irq domain setup failed\n");
> +	return -ENOMEM;
> +}
> +
> +static struct resource ioc3_uarta_resources[] = {
> +	DEFINE_RES_MEM(offsetof(struct ioc3, sregs.uarta),
> +		       sizeof_field(struct ioc3, sregs.uarta)),
> +	DEFINE_RES_IRQ(IOC3_IRQ_SERIAL_A)
> +};
> +
> +static struct resource ioc3_uartb_resources[] = {
> +	DEFINE_RES_MEM(offsetof(struct ioc3, sregs.uartb),
> +		       sizeof_field(struct ioc3, sregs.uartb)),
> +	DEFINE_RES_IRQ(IOC3_IRQ_SERIAL_B)
> +};
> +
> +static struct mfd_cell ioc3_serial_cells[] = {
> +	{
> +		.name = "ioc3-serial8250",
> +		.resources = ioc3_uarta_resources,
> +		.num_resources = ARRAY_SIZE(ioc3_uarta_resources),
> +		.id = 0,
> +	},
> +	{
> +		.name = "ioc3-serial8250",
> +		.resources = ioc3_uartb_resources,
> +		.num_resources = ARRAY_SIZE(ioc3_uartb_resources),
> +		.id = 1,

Any reason for not using PLATFORM_DEVID_AUTO.

> +	}
> +};
> +
> +static int ioc3_serial_setup(struct ioc3_priv_data *ipd)
> +{
> +	int ret;
> +
> +	/* set gpio pins for RS232/RS422 mode selection */
> +	writel(GPCR_UARTA_MODESEL | GPCR_UARTB_MODESEL,
> +		&ipd->regs->gpcr_s);
> +	/* select RS232 mode for uart a */
> +	writel(0, &ipd->regs->gppr[6]);
> +	/* select RS232 mode for uart b */
> +	writel(0, &ipd->regs->gppr[7]);
> +
> +	/* switch both ports to 16650 mode */
> +	writel(readl(&ipd->regs->port_a.sscr) & ~SSCR_DMA_EN,
> +	       &ipd->regs->port_a.sscr);
> +	writel(readl(&ipd->regs->port_b.sscr) & ~SSCR_DMA_EN,
> +	       &ipd->regs->port_b.sscr);
> +	udelay(1000); /* wait until mode switch is done */
> +
> +	ret = mfd_add_devices(&ipd->pdev->dev, ioc3_serial_id,

Any reason for not using PLATFORM_DEVID_AUTO.

> +			      ioc3_serial_cells, ARRAY_SIZE(ioc3_serial_cells),
> +			      &ipd->pdev->resource[0], 0, ipd->domain);
> +	if (ret) {
> +		dev_err(&ipd->pdev->dev, "Failed to add 16550 subdevs\n");
> +		return ret;
> +	}
> +	ioc3_serial_id += 2;

When is this likely to be re-read?

> +	return 0;
> +}
> +
> +static struct resource ioc3_kbd_resources[] = {
> +	DEFINE_RES_MEM(offsetof(struct ioc3, serio),
> +		       sizeof_field(struct ioc3, serio)),
> +	DEFINE_RES_IRQ(IOC3_IRQ_KBD)
> +};
> +
> +static struct mfd_cell ioc3_kbd_cells[] = {
> +	{
> +		.name = "ioc3-kbd",
> +		.resources = ioc3_kbd_resources,
> +		.num_resources = ARRAY_SIZE(ioc3_kbd_resources),
> +	}
> +};
> +
> +static int ioc3_kbd_setup(struct ioc3_priv_data *ipd)
> +{
> +	int ret;
> +
> +	ret = mfd_add_devices(&ipd->pdev->dev, ioc3_kbd_id, ioc3_kbd_cells,
> +			      ARRAY_SIZE(ioc3_kbd_cells),
> +			      &ipd->pdev->resource[0], 0, ipd->domain);
> +	if (ret) {
> +		dev_err(&ipd->pdev->dev, "Failed to add 16550 subdevs\n");
> +		return ret;
> +	}
> +	ioc3_kbd_id++;

Nit: '\n' here

> +	return 0;
> +}
> +
> +static struct resource ioc3_eth_resources[] = {
> +	DEFINE_RES_MEM(offsetof(struct ioc3, eth),
> +		       sizeof_field(struct ioc3, eth)),
> +	DEFINE_RES_MEM(offsetof(struct ioc3, ssram),
> +		       sizeof_field(struct ioc3, ssram)),
> +	DEFINE_RES_IRQ(0)
> +};
> +
> +static struct resource ioc3_w1_resources[] = {
> +	DEFINE_RES_MEM(offsetof(struct ioc3, mcr),
> +		       sizeof_field(struct ioc3, mcr)),
> +};
> +static struct sgi_w1_platform_data ioc3_w1_platform_data;
> +
> +static struct mfd_cell ioc3_eth_cells[] = {
> +	{
> +		.name = "ioc3-eth",
> +		.resources = ioc3_eth_resources,
> +		.num_resources = ARRAY_SIZE(ioc3_eth_resources),
> +	},
> +	{
> +		.name = "sgi_w1",
> +		.resources = ioc3_w1_resources,
> +		.num_resources = ARRAY_SIZE(ioc3_w1_resources),
> +		.platform_data = &ioc3_w1_platform_data,
> +		.pdata_size = sizeof(ioc3_w1_platform_data),
> +	}
> +};
> +
> +static int ioc3_eth_setup(struct ioc3_priv_data *ipd, bool use_domain)
> +{
> +	int irq = ipd->pdev->irq;
> +	int ret;
> +
> +	/* enable One-Wire bus */
> +	writel(GPCR_MLAN_EN, &ipd->regs->gpcr_s);
> +
> +	/* generate unique identifier */
> +	snprintf(ioc3_w1_platform_data.dev_id,
> +		 sizeof(ioc3_w1_platform_data.dev_id), "ioc3-%012llx",
> +		 ipd->pdev->resource->start);
> +
> +	if (use_domain)
> +		irq = irq_create_mapping(ipd->domain, IOC3_IRQ_ETH_DOMAIN);
> +
> +	ret = mfd_add_devices(&ipd->pdev->dev, ioc3_eth_id, ioc3_eth_cells,
> +			      ARRAY_SIZE(ioc3_eth_cells),
> +			      &ipd->pdev->resource[0], irq, NULL);
> +	if (ret) {
> +		dev_err(&ipd->pdev->dev, "Failed to add ETH/W1 subdev\n");
> +		return ret;
> +	}
> +

Nit: Remove this line and place it below:

> +	ioc3_eth_id++;

Nit: '\n' here

> +	return 0;
> +}
> +
> +#define M48T35_REG_SIZE	32768	/* size of m48t35 registers */
> +
> +static struct resource ioc3_m48t35_resources[] = {
> +	DEFINE_RES_MEM(IOC3_BYTEBUS_DEV0, M48T35_REG_SIZE)
> +};
> +
> +static struct mfd_cell ioc3_m48t35_cells[] = {
> +	{
> +		.name = "rtc-m48t35",
> +		.resources = ioc3_m48t35_resources,
> +		.num_resources = ARRAY_SIZE(ioc3_m48t35_resources),
> +	}
> +};
> +
> +static int ioc3_m48t35_setup(struct ioc3_priv_data *ipd)
> +{
> +	int ret;
> +
> +	ret = mfd_add_devices(&ipd->pdev->dev, PLATFORM_DEVID_AUTO,
> +			      ioc3_m48t35_cells, ARRAY_SIZE(ioc3_m48t35_cells),
> +			      &ipd->pdev->resource[0], 0, ipd->domain);
> +	if (ret)
> +		dev_err(&ipd->pdev->dev, "Failed to add M48T35 subdev\n");
> +
> +	return ret;
> +}
> +
> +/*
> + * On IP30 the RTC (a DS1687) is behind the IOC3 on the generic
> + * ByteBus regions. We have to write the RTC address of interest to
> + * IOC3_BYTEBUS_DEV1, then read the data from IOC3_BYTEBUS_DEV2.
> + * rtc->regs already points to IOC3_BYTEBUS_DEV1.
> + */
> +#define IP30_RTC_ADDR(rtc) (rtc->regs)
> +#define IP30_RTC_DATA(rtc) ((rtc->regs) + IOC3_BYTEBUS_DEV2 - IOC3_BYTEBUS_DEV1)
> +
> +static u8 ip30_rtc_read(struct ds1685_priv *rtc, int reg)
> +{
> +	writeb((reg & 0x7f), IP30_RTC_ADDR(rtc));
> +	return readb(IP30_RTC_DATA(rtc));
> +}
> +
> +static void ip30_rtc_write(struct ds1685_priv *rtc, int reg, u8 value)
> +{
> +	writeb((reg & 0x7f), IP30_RTC_ADDR(rtc));
> +	writeb(value, IP30_RTC_DATA(rtc));
> +}

Why is this not in the RTC driver?

> +static struct ds1685_rtc_platform_data ip30_rtc_platform_data = {
> +	.bcd_mode = false,
> +	.no_irq = false,
> +	.uie_unsupported = true,
> +	.alloc_io_resources = true,

> +	.plat_read = ip30_rtc_read,
> +	.plat_write = ip30_rtc_write,

Call-backs in a non-subsystem API is pretty ugly IMHO.

Where are these called from?

> +};
> +
> +static struct resource ioc3_rtc_ds1685_resources[] = {
> +	DEFINE_RES_MEM(IOC3_BYTEBUS_DEV1,
> +		       IOC3_BYTEBUS_DEV2 - IOC3_BYTEBUS_DEV1 + 1),
> +	DEFINE_RES_IRQ(0)
> +};
> +
> +static struct mfd_cell ioc3_ds1685_cells[] = {
> +	{
> +		.name = "rtc-ds1685",
> +		.resources = ioc3_rtc_ds1685_resources,
> +		.num_resources = ARRAY_SIZE(ioc3_rtc_ds1685_resources),
> +		.platform_data = &ip30_rtc_platform_data,
> +		.pdata_size = sizeof(ip30_rtc_platform_data),

> +		.id = -1,

Use the #define.  Same goes for below.

> +	}
> +};
> +
> +static int ioc3_ds1685_setup(struct ioc3_priv_data *ipd)
> +{
> +	int ret, irq;
> +
> +	irq = ioc3_map_irq(ipd->pdev, 6);

Nit: '\n' here

> +	ret = mfd_add_devices(&ipd->pdev->dev, 0, ioc3_ds1685_cells,
> +			      ARRAY_SIZE(ioc3_ds1685_cells),
> +			      &ipd->pdev->resource[0], irq, NULL);
> +	if (ret)
> +		dev_err(&ipd->pdev->dev, "Failed to add DS1685 subdev\n");
> +
> +	return ret;
> +};
> +
> +
> +static struct resource ioc3_leds_resources[] = {
> +	DEFINE_RES_MEM(offsetof(struct ioc3, gppr[0]),
> +		       sizeof_field(struct ioc3, gppr[0])),
> +	DEFINE_RES_MEM(offsetof(struct ioc3, gppr[1]),
> +		       sizeof_field(struct ioc3, gppr[1])),
> +};
> +
> +static struct mfd_cell ioc3_led_cells[] = {
> +	{
> +		.name = "ip30-leds",
> +		.resources = ioc3_leds_resources,
> +		.num_resources = ARRAY_SIZE(ioc3_leds_resources),
> +		.id = -1,
> +	}
> +};
> +
> +static int ioc3_led_setup(struct ioc3_priv_data *ipd)
> +{
> +	int ret;
> +
> +	ret = mfd_add_devices(&ipd->pdev->dev, 0, ioc3_led_cells,
> +			      ARRAY_SIZE(ioc3_led_cells),
> +			      &ipd->pdev->resource[0], 0, ipd->domain);
> +	if (ret)
> +		dev_err(&ipd->pdev->dev, "Failed to add LED subdev\n");

Nit: '\n' here

> +	return ret;
> +}
> +
> +static int ip27_baseio_setup(struct ioc3_priv_data *ipd)
> +{
> +	int ret, io_irq;
> +
> +	io_irq = ioc3_map_irq(ipd->pdev, PCI_SLOT(ipd->pdev->devfn) + 2);
> +	ret = ioc3_irq_domain_setup(ipd, io_irq);
> +	if (ret)
> +		return ret;

Nit: '\n' here

> +	ret = ioc3_eth_setup(ipd, false);
> +	if (ret)
> +		return ret;

Nit: '\n' here

> +	ret = ioc3_serial_setup(ipd);
> +	if (ret)
> +		return ret;

Nit: '\n' here

Etc, etc, etc ... same for all of the instances below.

> +	return ioc3_m48t35_setup(ipd);
> +}
> +
> +static int ip27_baseio6g_setup(struct ioc3_priv_data *ipd)
> +{
> +	int ret, io_irq;
> +
> +	io_irq = ioc3_map_irq(ipd->pdev, PCI_SLOT(ipd->pdev->devfn) + 2);
> +	ret = ioc3_irq_domain_setup(ipd, io_irq);
> +	if (ret)
> +		return ret;
> +	ret = ioc3_eth_setup(ipd, false);
> +	if (ret)
> +		return ret;
> +	ret = ioc3_serial_setup(ipd);
> +	if (ret)
> +		return ret;
> +	ret = ioc3_m48t35_setup(ipd);
> +	if (ret)
> +		return ret;
> +	return ioc3_kbd_setup(ipd);
> +}
> +
> +static int ip27_mio_setup(struct ioc3_priv_data *ipd)
> +{
> +	int ret;
> +
> +	ret = ioc3_irq_domain_setup(ipd, ipd->pdev->irq);
> +	if (ret)
> +		return ret;
> +	ret = ioc3_serial_setup(ipd);
> +	if (ret)
> +		return ret;
> +	return ioc3_kbd_setup(ipd);
> +}
> +
> +static int ip29_sysboard_setup(struct ioc3_priv_data *ipd)
> +{
> +	int ret, io_irq;
> +
> +	io_irq = ioc3_map_irq(ipd->pdev, PCI_SLOT(ipd->pdev->devfn) + 1);
> +	ret = ioc3_irq_domain_setup(ipd, io_irq);
> +	if (ret)
> +		return ret;
> +	ret = ioc3_eth_setup(ipd, false);
> +	if (ret)
> +		return ret;
> +	ret = ioc3_serial_setup(ipd);
> +	if (ret)
> +		return ret;
> +	ret = ioc3_m48t35_setup(ipd);
> +	if (ret)
> +		return ret;
> +	return ioc3_kbd_setup(ipd);
> +}
> +
> +static int ip30_sysboard_setup(struct ioc3_priv_data *ipd)
> +{
> +	int ret, io_irq;
> +
> +	io_irq = ioc3_map_irq(ipd->pdev, PCI_SLOT(ipd->pdev->devfn) + 2);
> +	ret = ioc3_irq_domain_setup(ipd, io_irq);
> +	if (ret)
> +		return ret;
> +	ret = ioc3_eth_setup(ipd, false);
> +	if (ret)
> +		return ret;
> +	ret = ioc3_serial_setup(ipd);
> +	if (ret)
> +		return ret;
> +	ret = ioc3_kbd_setup(ipd);
> +	if (ret)
> +		return ret;
> +	ret = ioc3_ds1685_setup(ipd);
> +	if (ret)
> +		return ret;
> +	return ioc3_led_setup(ipd);
> +}
> +
> +static int ioc3_menet_setup(struct ioc3_priv_data *ipd)
> +{
> +	int ret, io_irq;
> +
> +	io_irq = ioc3_map_irq(ipd->pdev, PCI_SLOT(ipd->pdev->devfn) + 4);
> +	ret = ioc3_irq_domain_setup(ipd, io_irq);
> +	if (ret)
> +		return ret;
> +	ret = ioc3_eth_setup(ipd, false);
> +	if (ret)
> +		return ret;
> +	return ioc3_serial_setup(ipd);
> +}
> +
> +static int ioc3_menet4_setup(struct ioc3_priv_data *ipd)
> +{
> +	return  ioc3_eth_setup(ipd, false);
> +}
> +
> +static int ioc3_cad_duo_setup(struct ioc3_priv_data *ipd)
> +{
> +	int ret;
> +
> +	ret = ioc3_irq_domain_setup(ipd, ipd->pdev->irq);
> +	if (ret)
> +		return ret;
> +	ret = ioc3_eth_setup(ipd, true);
> +	if (ret)
> +		return ret;
> +	return ioc3_kbd_setup(ipd);
> +}
> +
> +#define IOC3_SID(_name, _sid, _setup) \
> +	{								   \
> +		.name = _name,						   \
> +		.sid = (PCI_VENDOR_ID_SGI << 16) | IOC3_SUBSYS_ ## _sid,   \
> +		.setup = _setup,					   \
> +	}
> +
> +static struct {
> +	const char *name;
> +	u32 sid;
> +	int (*setup)(struct ioc3_priv_data *ipd);
> +} ioc3_infos[] = {

IMHO it's neater if you separate the definition and static data part.

> +	IOC3_SID("IP27 BaseIO6G", IP27_BASEIO6G, &ip27_baseio6g_setup),
> +	IOC3_SID("IP27 MIO", IP27_MIO, &ip27_mio_setup),
> +	IOC3_SID("IP27 BaseIO", IP27_BASEIO, &ip27_baseio_setup),
> +	IOC3_SID("IP29 System Board", IP29_SYSBOARD, &ip29_sysboard_setup),
> +	IOC3_SID("IP30 System Board", IP30_SYSBOARD, &ip30_sysboard_setup),
> +	IOC3_SID("MENET", MENET, &ioc3_menet_setup),
> +	IOC3_SID("MENET4", MENET4, &ioc3_menet4_setup)
> +};
> +
> +static int ioc3_setup(struct ioc3_priv_data *ipd)
> +{
> +	u32 sid;
> +	int i;
> +
> +	/* Clear IRQs */
> +	writel(~0, &ipd->regs->sio_iec);
> +	writel(~0, &ipd->regs->sio_ir);
> +	writel(0, &ipd->regs->eth.eier);
> +	writel(~0, &ipd->regs->eth.eisr);
> +
> +	/* read subsystem vendor id and subsystem id */
> +	pci_read_config_dword(ipd->pdev, PCI_SUBSYSTEM_VENDOR_ID, &sid);
> +
> +	for (i = 0; i < ARRAY_SIZE(ioc3_infos); i++)
> +		if (sid == ioc3_infos[i].sid) {
> +			pr_info("ioc3: %s\n", ioc3_infos[i].name);
> +			return ioc3_infos[i].setup(ipd);
> +		}
> +
> +	/* treat everything not identified by PCI subid as CAD DUO */
> +	pr_info("ioc3: CAD DUO\n");
> +	return ioc3_cad_duo_setup(ipd);
> +}
> +
> +static int ioc3_mfd_probe(struct pci_dev *pdev,
> +			  const struct pci_device_id *pci_id)
> +{
> +	struct ioc3_priv_data *ipd;
> +	struct ioc3 __iomem *regs;
> +	int ret;
> +
> +	ret = pci_enable_device(pdev);
> +	if (ret)
> +		return ret;
> +
> +	pci_write_config_byte(pdev, PCI_LATENCY_TIMER, 64);

What does 64 mean here?  Define it perhaps?

> +	pci_set_master(pdev);
> +
> +	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> +	if (ret) {
> +		dev_warn(&pdev->dev, "Warning: couldn_t set 64-bit DMA mask\n");

Remove the "Warning:" part please.

"Failed to set 64-bit DMA mask - trying 32-bit" ?

> +		ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> +		if (ret) {
> +			dev_err(&pdev->dev, "Can't set DMA mask, aborting\n");
> +			return ret;
> +		}
> +	}
> +
> +	/* Set up per-IOC3 data */
> +	ipd = devm_kzalloc(&pdev->dev, sizeof(struct ioc3_priv_data),
> +			   GFP_KERNEL);
> +	if (!ipd) {
> +		ret = -ENOMEM;
> +		goto out_disable_device;
> +	}
> +	ipd->pdev = pdev;
> +
> +	/*
> +	 * Map all IOC3 registers.  These are shared between subdevices
> +	 * so the main IOC3 module manages them.
> +	 */
> +	regs = devm_ioremap(&pdev->dev, pci_resource_start(pdev, 0),
> +			    pci_resource_len(pdev, 0));
> +	if (!regs) {
> +		pr_warn("ioc3: Unable to remap PCI BAR for %s.\n",
> +			pci_name(pdev));

Why are you using pr_warn() here?

> +		ret = -ENOMEM;
> +		goto out_disable_device;
> +	}
> +	ipd->regs = regs;
> +
> +	/* Track PCI-device specific data */
> +	pci_set_drvdata(pdev, ipd);
> +
> +	ret = ioc3_setup(ipd);
> +	if (ret)
> +		goto out_disable_device;
> +
> +	return 0;
> +
> +out_disable_device:
> +	pci_disable_device(pdev);
> +	return ret;
> +}
> +
> +static void ioc3_mfd_remove(struct pci_dev *pdev)
> +{
> +	struct ioc3_priv_data *ipd;
> +
> +	ipd = pci_get_drvdata(pdev);
> +
> +	/* Clear and disable all IRQs */
> +	writel(~0, &ipd->regs->sio_iec);
> +	writel(~0, &ipd->regs->sio_ir);
> +
> +	/* Release resources */
> +	if (ipd->domain) {
> +		irq_domain_remove(ipd->domain);
> +		free_irq(ipd->domain_irq, (void *)ipd);
> +	}
> +	pci_disable_device(pdev);
> +}
> +
> +static struct pci_device_id ioc3_mfd_id_table[] = {
> +	{ PCI_VENDOR_ID_SGI, PCI_DEVICE_ID_SGI_IOC3, PCI_ANY_ID, PCI_ANY_ID },
> +	{ 0, },
> +};
> +MODULE_DEVICE_TABLE(pci, ioc3_mfd_id_table);
> +
> +static struct pci_driver ioc3_mfd_driver = {
> +	.name = "IOC3",
> +	.id_table = ioc3_mfd_id_table,
> +	.probe = ioc3_mfd_probe,
> +	.remove = ioc3_mfd_remove,
> +};
> +
> +module_pci_driver(ioc3_mfd_driver);
> +
> +MODULE_AUTHOR("Thomas Bogendoerfer <tbogendoerfer@suse.de>");
> +MODULE_DESCRIPTION("SGI IOC3 MFD driver");
> +MODULE_LICENSE("GPL");

GPL v2 ?

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH v3 5/7] mfd: ioc3: Add driver for SGI IOC3 chip
  2019-07-25 11:47   ` Lee Jones
@ 2019-07-29 18:45     ` Thomas Bogendoerfer
  2019-08-09 12:22       ` Alexandre Belloni
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Bogendoerfer @ 2019-07-29 18:45 UTC (permalink / raw)
  To: Lee Jones
  Cc: Ralf Baechle, Paul Burton, James Hogan, Dmitry Torokhov,
	David S. Miller, Srinivas Kandagatla, Alessandro Zummo,
	Alexandre Belloni, Greg Kroah-Hartman, Jiri Slaby, linux-mips,
	linux-kernel, linux-input, netdev, linux-rtc, linux-serial

On Thu, 25 Jul 2019 12:47:16 +0100
Lee Jones <lee.jones@linaro.org> wrote:

> On Thu, 13 Jun 2019, Thomas Bogendoerfer wrote:
> > +/*
> > + * On IP30 the RTC (a DS1687) is behind the IOC3 on the generic
> > + * ByteBus regions. We have to write the RTC address of interest to
> > + * IOC3_BYTEBUS_DEV1, then read the data from IOC3_BYTEBUS_DEV2.
> > + * rtc->regs already points to IOC3_BYTEBUS_DEV1.
> > + */
> > +#define IP30_RTC_ADDR(rtc) (rtc->regs)
> > +#define IP30_RTC_DATA(rtc) ((rtc->regs) + IOC3_BYTEBUS_DEV2 - IOC3_BYTEBUS_DEV1)
> > +
> > +static u8 ip30_rtc_read(struct ds1685_priv *rtc, int reg)
> > +{
> > +	writeb((reg & 0x7f), IP30_RTC_ADDR(rtc));
> > +	return readb(IP30_RTC_DATA(rtc));
> > +}
> > +
> > +static void ip30_rtc_write(struct ds1685_priv *rtc, int reg, u8 value)
> > +{
> > +	writeb((reg & 0x7f), IP30_RTC_ADDR(rtc));
> > +	writeb(value, IP30_RTC_DATA(rtc));
> > +}
> 
> Why is this not in the RTC driver?

because rtc1685 is used in different systems and accessing the chip
differs between those systems. 

> > +static struct ds1685_rtc_platform_data ip30_rtc_platform_data = {
> > +	.bcd_mode = false,
> > +	.no_irq = false,
> > +	.uie_unsupported = true,
> > +	.alloc_io_resources = true,
> 
> > +	.plat_read = ip30_rtc_read,
> > +	.plat_write = ip30_rtc_write,
> 
> Call-backs in a non-subsystem API is pretty ugly IMHO.

I agree

> Where are these called from?

drivers/rtc/rtc-ds1685.c

I could do the same as done for serial8250 and add an additional .c file
in  drivers/rtc which handles this for SGI-IP30. Alexandre would this work
for you as well ?

> > +#define IOC3_SID(_name, _sid, _setup) \
> > +	{								   \
> > +		.name = _name,						   \
> > +		.sid = (PCI_VENDOR_ID_SGI << 16) | IOC3_SUBSYS_ ## _sid,   \
> > +		.setup = _setup,					   \
> > +	}
> > +
> > +static struct {
> > +	const char *name;
> > +	u32 sid;
> > +	int (*setup)(struct ioc3_priv_data *ipd);
> > +} ioc3_infos[] = {
> 
> IMHO it's neater if you separate the definition and static data part.

I don't quite understand what you mean here. Should I move the #define at
the beginning of the file ? Why is it neater ?

Thomas.

-- 
SUSE Linux GmbH
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)

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

* Re: [PATCH v3 5/7] mfd: ioc3: Add driver for SGI IOC3 chip
  2019-07-29 18:45     ` Thomas Bogendoerfer
@ 2019-08-09 12:22       ` Alexandre Belloni
  0 siblings, 0 replies; 13+ messages in thread
From: Alexandre Belloni @ 2019-08-09 12:22 UTC (permalink / raw)
  To: Thomas Bogendoerfer
  Cc: Lee Jones, Ralf Baechle, Paul Burton, James Hogan,
	Dmitry Torokhov, David S. Miller, Srinivas Kandagatla,
	Alessandro Zummo, Greg Kroah-Hartman, Jiri Slaby, linux-mips,
	linux-kernel, linux-input, netdev, linux-rtc, linux-serial

On 29/07/2019 20:45:57+0200, Thomas Bogendoerfer wrote:
> On Thu, 25 Jul 2019 12:47:16 +0100
> Lee Jones <lee.jones@linaro.org> wrote:
> 
> > On Thu, 13 Jun 2019, Thomas Bogendoerfer wrote:
> > > +/*
> > > + * On IP30 the RTC (a DS1687) is behind the IOC3 on the generic
> > > + * ByteBus regions. We have to write the RTC address of interest to
> > > + * IOC3_BYTEBUS_DEV1, then read the data from IOC3_BYTEBUS_DEV2.
> > > + * rtc->regs already points to IOC3_BYTEBUS_DEV1.
> > > + */
> > > +#define IP30_RTC_ADDR(rtc) (rtc->regs)
> > > +#define IP30_RTC_DATA(rtc) ((rtc->regs) + IOC3_BYTEBUS_DEV2 - IOC3_BYTEBUS_DEV1)
> > > +
> > > +static u8 ip30_rtc_read(struct ds1685_priv *rtc, int reg)
> > > +{
> > > +	writeb((reg & 0x7f), IP30_RTC_ADDR(rtc));
> > > +	return readb(IP30_RTC_DATA(rtc));
> > > +}
> > > +
> > > +static void ip30_rtc_write(struct ds1685_priv *rtc, int reg, u8 value)
> > > +{
> > > +	writeb((reg & 0x7f), IP30_RTC_ADDR(rtc));
> > > +	writeb(value, IP30_RTC_DATA(rtc));
> > > +}
> > 
> > Why is this not in the RTC driver?
> 
> because rtc1685 is used in different systems and accessing the chip
> differs between those systems. 
> 
> > > +static struct ds1685_rtc_platform_data ip30_rtc_platform_data = {
> > > +	.bcd_mode = false,
> > > +	.no_irq = false,
> > > +	.uie_unsupported = true,
> > > +	.alloc_io_resources = true,
> > 
> > > +	.plat_read = ip30_rtc_read,
> > > +	.plat_write = ip30_rtc_write,
> > 
> > Call-backs in a non-subsystem API is pretty ugly IMHO.
> 
> I agree
> 
> > Where are these called from?
> 
> drivers/rtc/rtc-ds1685.c
> 
> I could do the same as done for serial8250 and add an additional .c file
> in  drivers/rtc which handles this for SGI-IP30. Alexandre would this work
> for you as well ?
> 

As it is not particularly big, you could put that directly in
rtc-ds1685.c.


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

end of thread, back to index

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-13 17:06 [PATCH v3 0/7] Use MFD framework for SGI IOC3 drivers Thomas Bogendoerfer
2019-06-13 17:06 ` [PATCH v3 1/7] nvmem: core: add nvmem_device_find Thomas Bogendoerfer
2019-06-13 17:06 ` [PATCH v3 2/7] MIPS: PCI: refactor ioc3 special handling Thomas Bogendoerfer
2019-06-13 17:06 ` [PATCH v3 3/7] MIPS: PCI: use information from 1-wire PROM for IOC3 detection Thomas Bogendoerfer
2019-06-13 17:06 ` [PATCH v3 4/7] MIPS: SGI-IP27: remove ioc3 ethernet init Thomas Bogendoerfer
2019-06-13 17:06 ` [PATCH v3 6/7] MIPS: SGI-IP27: fix readb/writeb addressing Thomas Bogendoerfer
2019-06-13 17:06 ` [PATCH v3 7/7] Input: add IOC3 serio driver Thomas Bogendoerfer
2019-07-01  8:19   ` Dmitry Torokhov
     [not found] ` <20190613170636.6647-6-tbogendoerfer@suse.de>
2019-06-25  9:04   ` [PATCH v3 5/7] mfd: ioc3: Add driver for SGI IOC3 chip Thomas Bogendoerfer
2019-06-25 12:56     ` Lee Jones
2019-07-25 11:47   ` Lee Jones
2019-07-29 18:45     ` Thomas Bogendoerfer
2019-08-09 12:22       ` Alexandre Belloni

Linux-MIPS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mips/0 linux-mips/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mips linux-mips/ https://lore.kernel.org/linux-mips \
		linux-mips@vger.kernel.org linux-mips@archiver.kernel.org
	public-inbox-index linux-mips


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-mips


AGPL code for this site: git clone https://public-inbox.org/ public-inbox