[PATCH 0/6] omap-ssi cleanups + dvfs support

[PATCH 0/6] omap-ssi cleanups + dvfs support

[Sebastian Reichel]

Hi,

The following patches add a few cleanups to the omap-ssi driver,
fix module unloading (and reloading) and merge omap-ssi and
omap-ssi-port into the same module to avoid a circular dependency
introduced by the last patch.

P.S.: The last patch has already been sent in this patchset
(https://lkml.org/lkml/2016/1/30/283), but could not be applied
due to the circular dependency.

-- Sebastian

Sebastian Reichel (6):
  HSI: omap_ssi_port: switch to gpiod API
  HSI: omap_ssi: fix module unloading
  HSI: omap_ssi: make sure probe stays available
  HSI: omap_ssi: fix removal of port platform device
  HSI: omap_ssi: built omap_ssi and omap_ssi_port into one module
  HSI: omap-ssi: add clk change support

 drivers/hsi/controllers/Kconfig                    |   5 -
 drivers/hsi/controllers/Makefile                   |   4 +-
 drivers/hsi/controllers/omap_ssi.h                 |  12 ++-
 .../controllers/{omap_ssi.c => omap_ssi_core.c}    | 106 ++++++++++++++++++---
 drivers/hsi/controllers/omap_ssi_port.c            |  84 ++++++++--------
 5 files changed, 146 insertions(+), 65 deletions(-)
 rename drivers/hsi/controllers/{omap_ssi.c => omap_ssi_core.c} (86%)

-- 
2.8.1

[PATCH 1/6] HSI: omap_ssi_port: switch to gpiod API

[Sebastian Reichel]

Simplify driver by switching to new gpio descriptor based API.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/hsi/controllers/omap_ssi.c      |  1 -
 drivers/hsi/controllers/omap_ssi.h      |  4 ++--
 drivers/hsi/controllers/omap_ssi_port.c | 31 ++++++++++---------------------
 3 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/drivers/hsi/controllers/omap_ssi.c b/drivers/hsi/controllers/omap_ssi.c
index 27b91f14ba7a..c582229d1cd2 100644
--- a/drivers/hsi/controllers/omap_ssi.c
+++ b/drivers/hsi/controllers/omap_ssi.c
@@ -24,7 +24,6 @@
 #include <linux/err.h>
 #include <linux/ioport.h>
 #include <linux/io.h>
-#include <linux/gpio.h>
 #include <linux/clk.h>
 #include <linux/device.h>
 #include <linux/platform_device.h>
diff --git a/drivers/hsi/controllers/omap_ssi.h b/drivers/hsi/controllers/omap_ssi.h
index f9aaf37262be..1fa028078a3c 100644
--- a/drivers/hsi/controllers/omap_ssi.h
+++ b/drivers/hsi/controllers/omap_ssi.h
@@ -27,7 +27,7 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/hsi/hsi.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 
@@ -97,7 +97,7 @@ struct omap_ssi_port {
 	struct list_head	brkqueue;
 	unsigned int		irq;
 	int			wake_irq;
-	int			wake_gpio;
+	struct gpio_desc	*wake_gpio;
 	struct tasklet_struct	pio_tasklet;
 	struct tasklet_struct	wake_tasklet;
 	bool			wktest:1; /* FIXME: HACK to be removed */
diff --git a/drivers/hsi/controllers/omap_ssi_port.c b/drivers/hsi/controllers/omap_ssi_port.c
index e80a66e20998..948bdc7946fb 100644
--- a/drivers/hsi/controllers/omap_ssi_port.c
+++ b/drivers/hsi/controllers/omap_ssi_port.c
@@ -24,7 +24,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/pm_runtime.h>
 
-#include <linux/of_gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/debugfs.h>
 
 #include "omap_ssi_regs.h"
@@ -43,7 +43,7 @@ static inline int hsi_dummy_cl(struct hsi_client *cl __maybe_unused)
 static inline unsigned int ssi_wakein(struct hsi_port *port)
 {
 	struct omap_ssi_port *omap_port = hsi_port_drvdata(port);
-	return gpio_get_value(omap_port->wake_gpio);
+	return gpiod_get_value(omap_port->wake_gpio);
 }
 
 #ifdef CONFIG_DEBUG_FS
@@ -1036,12 +1036,12 @@ static int __init ssi_wake_irq(struct hsi_port *port,
 	int cawake_irq;
 	int err;
 
-	if (omap_port->wake_gpio == -1) {
+	if (!omap_port->wake_gpio) {
 		omap_port->wake_irq = -1;
 		return 0;
 	}
 
-	cawake_irq = gpio_to_irq(omap_port->wake_gpio);
+	cawake_irq = gpiod_to_irq(omap_port->wake_gpio);
 
 	omap_port->wake_irq = cawake_irq;
 	tasklet_init(&omap_port->wake_tasklet, ssi_wake_tasklet,
@@ -1111,7 +1111,7 @@ static int __init ssi_port_probe(struct platform_device *pd)
 	struct omap_ssi_port *omap_port;
 	struct hsi_controller *ssi = dev_get_drvdata(pd->dev.parent);
 	struct omap_ssi_controller *omap_ssi = hsi_controller_drvdata(ssi);
-	int cawake_gpio = 0;
+	struct gpio_desc *cawake_gpio = NULL;
 	u32 port_id;
 	int err;
 
@@ -1147,20 +1147,10 @@ static int __init ssi_port_probe(struct platform_device *pd)
 		goto error;
 	}
 
-	err = of_get_named_gpio(np, "ti,ssi-cawake-gpio", 0);
-	if (err < 0) {
-		dev_err(&pd->dev, "DT data is missing cawake gpio (err=%d)\n",
-			err);
-		goto error;
-	}
-	cawake_gpio = err;
-
-	err = devm_gpio_request_one(&port->device, cawake_gpio, GPIOF_DIR_IN,
-		"cawake");
-	if (err) {
-		dev_err(&pd->dev, "could not request cawake gpio (err=%d)!\n",
-			err);
-		err = -ENXIO;
+	cawake_gpio = devm_gpiod_get(&pd->dev, "ti,ssi-cawake", GPIOD_IN);
+	if (IS_ERR(cawake_gpio)) {
+		err = PTR_ERR(cawake_gpio);
+		dev_err(&pd->dev, "couldn't get cawake gpio (err=%d)!\n", err);
 		goto error;
 	}
 
@@ -1219,8 +1209,7 @@ static int __init ssi_port_probe(struct platform_device *pd)
 
 	hsi_add_clients_from_dt(port, np);
 
-	dev_info(&pd->dev, "ssi port %u successfully initialized (cawake=%d)\n",
-		port_id, cawake_gpio);
+	dev_info(&pd->dev, "ssi port %u successfully initialized\n", port_id);
 
 	return 0;
 
-- 
2.8.1

Re: [PATCH 1/6] HSI: omap_ssi_port: switch to gpiod API

[Pavel Machek]

On Sat 2016-04-30 04:09:08, Sebastian Reichel wrote:
> Simplify driver by switching to new gpio descriptor based API.
> 
> Signed-off-by: Sebastian Reichel <sre@kernel.org>

Acked-by: Pavel Machek <pavel@ucw.cz>
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH 2/6] HSI: omap_ssi: fix module unloading

[Sebastian Reichel]

Removal of ssi controller debugfs directory must
happen after the clients have been removed from
it.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/hsi/controllers/omap_ssi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hsi/controllers/omap_ssi.c b/drivers/hsi/controllers/omap_ssi.c
index c582229d1cd2..2dd46b219af2 100644
--- a/drivers/hsi/controllers/omap_ssi.c
+++ b/drivers/hsi/controllers/omap_ssi.c
@@ -526,6 +526,9 @@ static int __exit ssi_remove(struct platform_device *pd)
 {
 	struct hsi_controller *ssi = platform_get_drvdata(pd);
 
+	/* cleanup of of_platform_populate() call */
+	device_for_each_child(&pd->dev, NULL, ssi_remove_ports);
+
 #ifdef CONFIG_DEBUG_FS
 	ssi_debug_remove_ctrl(ssi);
 #endif
@@ -534,9 +537,6 @@ static int __exit ssi_remove(struct platform_device *pd)
 
 	pm_runtime_disable(&pd->dev);
 
-	/* cleanup of of_platform_populate() call */
-	device_for_each_child(&pd->dev, NULL, ssi_remove_ports);
-
 	return 0;
 }
 
-- 
2.8.1

Re: [PATCH 2/6] HSI: omap_ssi: fix module unloading

[Pavel Machek]

On Sat 2016-04-30 04:09:09, Sebastian Reichel wrote:
> Removal of ssi controller debugfs directory must
> happen after the clients have been removed from
> it.
> 
> Signed-off-by: Sebastian Reichel <sre@kernel.org>

Acked-by: Pavel Machek <pavel@ucw.cz>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH 3/6] HSI: omap_ssi: make sure probe stays available

[Sebastian Reichel]

device can be unbind/rebind, so probe should
stay available.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/hsi/controllers/omap_ssi.c      | 17 +++++++++--------
 drivers/hsi/controllers/omap_ssi_port.c | 19 ++++++++++---------
 2 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/hsi/controllers/omap_ssi.c b/drivers/hsi/controllers/omap_ssi.c
index 2dd46b219af2..ffb921482e76 100644
--- a/drivers/hsi/controllers/omap_ssi.c
+++ b/drivers/hsi/controllers/omap_ssi.c
@@ -140,7 +140,7 @@ static const struct file_operations ssi_gdd_regs_fops = {
 	.release	= single_release,
 };
 
-static int __init ssi_debug_add_ctrl(struct hsi_controller *ssi)
+static int ssi_debug_add_ctrl(struct hsi_controller *ssi)
 {
 	struct omap_ssi_controller *omap_ssi = hsi_controller_drvdata(ssi);
 	struct dentry *dir;
@@ -290,7 +290,7 @@ static unsigned long ssi_get_clk_rate(struct hsi_controller *ssi)
 	return rate;
 }
 
-static int __init ssi_get_iomem(struct platform_device *pd,
+static int ssi_get_iomem(struct platform_device *pd,
 		const char *name, void __iomem **pbase, dma_addr_t *phy)
 {
 	struct resource *mem;
@@ -310,7 +310,7 @@ static int __init ssi_get_iomem(struct platform_device *pd,
 	return 0;
 }
 
-static int __init ssi_add_controller(struct hsi_controller *ssi,
+static int ssi_add_controller(struct hsi_controller *ssi,
 						struct platform_device *pd)
 {
 	struct omap_ssi_controller *omap_ssi;
@@ -386,7 +386,7 @@ out_err:
 	return err;
 }
 
-static int __init ssi_hw_init(struct hsi_controller *ssi)
+static int ssi_hw_init(struct hsi_controller *ssi)
 {
 	struct omap_ssi_controller *omap_ssi = hsi_controller_drvdata(ssi);
 	unsigned int i;
@@ -456,7 +456,7 @@ static int ssi_remove_ports(struct device *dev, void *c)
 	return 0;
 }
 
-static int __init ssi_probe(struct platform_device *pd)
+static int ssi_probe(struct platform_device *pd)
 {
 	struct platform_device *childpdev;
 	struct device_node *np = pd->dev.of_node;
@@ -522,7 +522,7 @@ out1:
 	return err;
 }
 
-static int __exit ssi_remove(struct platform_device *pd)
+static int ssi_remove(struct platform_device *pd)
 {
 	struct hsi_controller *ssi = platform_get_drvdata(pd);
 
@@ -592,7 +592,8 @@ MODULE_DEVICE_TABLE(of, omap_ssi_of_match);
 #endif
 
 static struct platform_driver ssi_pdriver = {
-	.remove	= __exit_p(ssi_remove),
+	.probe = ssi_probe,
+	.remove	= ssi_remove,
 	.driver	= {
 		.name	= "omap_ssi",
 		.pm     = DEV_PM_OPS,
@@ -600,7 +601,7 @@ static struct platform_driver ssi_pdriver = {
 	},
 };
 
-module_platform_driver_probe(ssi_pdriver, ssi_probe);
+module_platform_driver(ssi_pdriver);
 
 MODULE_ALIAS("platform:omap_ssi");
 MODULE_AUTHOR("Carlos Chinea <carlos.chinea@nokia.com>");
diff --git a/drivers/hsi/controllers/omap_ssi_port.c b/drivers/hsi/controllers/omap_ssi_port.c
index 948bdc7946fb..530095ed39e7 100644
--- a/drivers/hsi/controllers/omap_ssi_port.c
+++ b/drivers/hsi/controllers/omap_ssi_port.c
@@ -171,7 +171,7 @@ static int ssi_div_set(void *data, u64 val)
 
 DEFINE_SIMPLE_ATTRIBUTE(ssi_sst_div_fops, ssi_div_get, ssi_div_set, "%llu\n");
 
-static int __init ssi_debug_add_port(struct omap_ssi_port *omap_port,
+static int ssi_debug_add_port(struct omap_ssi_port *omap_port,
 				     struct dentry *dir)
 {
 	struct hsi_port *port = to_hsi_port(omap_port->dev);
@@ -1007,7 +1007,7 @@ static irqreturn_t ssi_wake_isr(int irq __maybe_unused, void *ssi_port)
 	return IRQ_HANDLED;
 }
 
-static int __init ssi_port_irq(struct hsi_port *port,
+static int ssi_port_irq(struct hsi_port *port,
 						struct platform_device *pd)
 {
 	struct omap_ssi_port *omap_port = hsi_port_drvdata(port);
@@ -1029,7 +1029,7 @@ static int __init ssi_port_irq(struct hsi_port *port,
 	return err;
 }
 
-static int __init ssi_wake_irq(struct hsi_port *port,
+static int ssi_wake_irq(struct hsi_port *port,
 						struct platform_device *pd)
 {
 	struct omap_ssi_port *omap_port = hsi_port_drvdata(port);
@@ -1060,7 +1060,7 @@ static int __init ssi_wake_irq(struct hsi_port *port,
 	return err;
 }
 
-static void __init ssi_queues_init(struct omap_ssi_port *omap_port)
+static void ssi_queues_init(struct omap_ssi_port *omap_port)
 {
 	unsigned int ch;
 
@@ -1071,7 +1071,7 @@ static void __init ssi_queues_init(struct omap_ssi_port *omap_port)
 	INIT_LIST_HEAD(&omap_port->brkqueue);
 }
 
-static int __init ssi_port_get_iomem(struct platform_device *pd,
+static int ssi_port_get_iomem(struct platform_device *pd,
 		const char *name, void __iomem **pbase, dma_addr_t *phy)
 {
 	struct hsi_port *port = platform_get_drvdata(pd);
@@ -1104,7 +1104,7 @@ static int __init ssi_port_get_iomem(struct platform_device *pd,
 	return 0;
 }
 
-static int __init ssi_port_probe(struct platform_device *pd)
+static int ssi_port_probe(struct platform_device *pd)
 {
 	struct device_node *np = pd->dev.of_node;
 	struct hsi_port *port;
@@ -1217,7 +1217,7 @@ error:
 	return err;
 }
 
-static int __exit ssi_port_remove(struct platform_device *pd)
+static int ssi_port_remove(struct platform_device *pd)
 {
 	struct hsi_port *port = platform_get_drvdata(pd);
 	struct omap_ssi_port *omap_port = hsi_port_drvdata(port);
@@ -1370,7 +1370,8 @@ MODULE_DEVICE_TABLE(of, omap_ssi_port_of_match);
 #endif
 
 static struct platform_driver ssi_port_pdriver = {
-	.remove	= __exit_p(ssi_port_remove),
+	.probe = ssi_port_probe,
+	.remove	= ssi_port_remove,
 	.driver	= {
 		.name	= "omap_ssi_port",
 		.of_match_table = omap_ssi_port_of_match,
@@ -1378,7 +1379,7 @@ static struct platform_driver ssi_port_pdriver = {
 	},
 };
 
-module_platform_driver_probe(ssi_port_pdriver, ssi_port_probe);
+module_platform_driver(ssi_port_pdriver);
 
 MODULE_ALIAS("platform:omap_ssi_port");
 MODULE_AUTHOR("Carlos Chinea <carlos.chinea@nokia.com>");
-- 
2.8.1

[PATCH 4/6] HSI: omap_ssi: fix removal of port platform device

[Sebastian Reichel]

This avoids removal of the HSI port device when
only the platform port device should be removed
and clears the POPULATED bit in the DT node, so
that a new platform device is created when the
driver is probed again.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/hsi/controllers/omap_ssi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/hsi/controllers/omap_ssi.c b/drivers/hsi/controllers/omap_ssi.c
index ffb921482e76..68dfdaa19938 100644
--- a/drivers/hsi/controllers/omap_ssi.c
+++ b/drivers/hsi/controllers/omap_ssi.c
@@ -451,6 +451,10 @@ static int ssi_remove_ports(struct device *dev, void *c)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 
+	if (!dev->of_node)
+		return 0;
+
+	of_node_clear_flag(dev->of_node, OF_POPULATED);
 	of_device_unregister(pdev);
 
 	return 0;
-- 
2.8.1

Re: [PATCH 4/6] HSI: omap_ssi: fix removal of port platform device

[Pavel Machek]

On Sat 2016-04-30 04:09:11, Sebastian Reichel wrote:
> This avoids removal of the HSI port device when
> only the platform port device should be removed
> and clears the POPULATED bit in the DT node, so
> that a new platform device is created when the
> driver is probed again.
> 
> Signed-off-by: Sebastian Reichel <sre@kernel.org>

3,4: Acked-by: Pavel Machek <pavel@ucw.cz>


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH 5/6] HSI: omap_ssi: built omap_ssi and omap_ssi_port into one module

[Sebastian Reichel]

Merge omap_ssi and omap_ssi_port into one module. This
fixes problems with module cycle dependencies introduced
by future patches.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/hsi/controllers/Kconfig                         |  5 -----
 drivers/hsi/controllers/Makefile                        |  4 ++--
 drivers/hsi/controllers/omap_ssi.h                      |  2 ++
 drivers/hsi/controllers/{omap_ssi.c => omap_ssi_core.c} | 17 ++++++++++++++++-
 drivers/hsi/controllers/omap_ssi_port.c                 | 16 +---------------
 5 files changed, 21 insertions(+), 23 deletions(-)
 rename drivers/hsi/controllers/{omap_ssi.c => omap_ssi_core.c} (97%)

diff --git a/drivers/hsi/controllers/Kconfig b/drivers/hsi/controllers/Kconfig
index 6aba27808172..084ec97eec64 100644
--- a/drivers/hsi/controllers/Kconfig
+++ b/drivers/hsi/controllers/Kconfig
@@ -12,8 +12,3 @@ config OMAP_SSI
 	  If you say Y here, you will enable the OMAP SSI hardware driver.
 
 	  If unsure, say N.
-
-config OMAP_SSI_PORT
-	tristate
-	default m if OMAP_SSI=m
-	default y if OMAP_SSI=y
diff --git a/drivers/hsi/controllers/Makefile b/drivers/hsi/controllers/Makefile
index d2665cf9c545..7aba9c7f71bb 100644
--- a/drivers/hsi/controllers/Makefile
+++ b/drivers/hsi/controllers/Makefile
@@ -2,5 +2,5 @@
 # Makefile for HSI controllers drivers
 #
 
-obj-$(CONFIG_OMAP_SSI)		+= omap_ssi.o
-obj-$(CONFIG_OMAP_SSI_PORT)	+= omap_ssi_port.o
+omap_ssi-objs		+= omap_ssi_core.o omap_ssi_port.o
+obj-$(CONFIG_OMAP_SSI)	+= omap_ssi.o
diff --git a/drivers/hsi/controllers/omap_ssi.h b/drivers/hsi/controllers/omap_ssi.h
index 1fa028078a3c..e493321cb0c3 100644
--- a/drivers/hsi/controllers/omap_ssi.h
+++ b/drivers/hsi/controllers/omap_ssi.h
@@ -164,4 +164,6 @@ struct omap_ssi_controller {
 #endif
 };
 
+extern struct platform_driver ssi_port_pdriver;
+
 #endif /* __LINUX_HSI_OMAP_SSI_H__ */
diff --git a/drivers/hsi/controllers/omap_ssi.c b/drivers/hsi/controllers/omap_ssi_core.c
similarity index 97%
rename from drivers/hsi/controllers/omap_ssi.c
rename to drivers/hsi/controllers/omap_ssi_core.c
index 68dfdaa19938..535c76038288 100644
--- a/drivers/hsi/controllers/omap_ssi.c
+++ b/drivers/hsi/controllers/omap_ssi_core.c
@@ -605,7 +605,22 @@ static struct platform_driver ssi_pdriver = {
 	},
 };
 
-module_platform_driver(ssi_pdriver);
+static int __init ssi_init(void) {
+	int ret;
+
+	ret = platform_driver_register(&ssi_pdriver);
+	if (ret)
+		return ret;
+
+	return platform_driver_register(&ssi_port_pdriver);
+}
+module_init(ssi_init);
+
+static void __exit ssi_exit(void) {
+	platform_driver_unregister(&ssi_port_pdriver);
+	platform_driver_unregister(&ssi_pdriver);
+}
+module_exit(ssi_exit);
 
 MODULE_ALIAS("platform:omap_ssi");
 MODULE_AUTHOR("Carlos Chinea <carlos.chinea@nokia.com>");
diff --git a/drivers/hsi/controllers/omap_ssi_port.c b/drivers/hsi/controllers/omap_ssi_port.c
index 530095ed39e7..1569bbb53ee8 100644
--- a/drivers/hsi/controllers/omap_ssi_port.c
+++ b/drivers/hsi/controllers/omap_ssi_port.c
@@ -1117,11 +1117,6 @@ static int ssi_port_probe(struct platform_device *pd)
 
 	dev_dbg(&pd->dev, "init ssi port...\n");
 
-	if (!try_module_get(ssi->owner)) {
-		dev_err(&pd->dev, "could not increment parent module refcount\n");
-		return -ENODEV;
-	}
-
 	if (!ssi->port || !omap_ssi->port) {
 		dev_err(&pd->dev, "ssi controller not initialized!\n");
 		err = -ENODEV;
@@ -1242,7 +1237,6 @@ static int ssi_port_remove(struct platform_device *pd)
 
 	omap_ssi->port[omap_port->port_id] = NULL;
 	platform_set_drvdata(pd, NULL);
-	module_put(ssi->owner);
 	pm_runtime_disable(&pd->dev);
 
 	return 0;
@@ -1369,7 +1363,7 @@ MODULE_DEVICE_TABLE(of, omap_ssi_port_of_match);
 #define omap_ssi_port_of_match NULL
 #endif
 
-static struct platform_driver ssi_port_pdriver = {
+struct platform_driver ssi_port_pdriver = {
 	.probe = ssi_port_probe,
 	.remove	= ssi_port_remove,
 	.driver	= {
@@ -1378,11 +1372,3 @@ static struct platform_driver ssi_port_pdriver = {
 		.pm	= DEV_PM_OPS,
 	},
 };
-
-module_platform_driver(ssi_port_pdriver);
-
-MODULE_ALIAS("platform:omap_ssi_port");
-MODULE_AUTHOR("Carlos Chinea <carlos.chinea@nokia.com>");
-MODULE_AUTHOR("Sebastian Reichel <sre@kernel.org>");
-MODULE_DESCRIPTION("Synchronous Serial Interface Port Driver");
-MODULE_LICENSE("GPL v2");
-- 
2.8.1

Re: [PATCH 5/6] HSI: omap_ssi: built omap_ssi and omap_ssi_port into one module

[Pavel Machek]

Hi!

> Merge omap_ssi and omap_ssi_port into one module. This
> fixes problems with module cycle dependencies introduced
> by future patches.

Interesting way of saying that this prepares us for future patch
:-).


> +++ b/drivers/hsi/controllers/Makefile
> @@ -2,5 +2,5 @@
>  # Makefile for HSI controllers drivers
>  #
>  
> -obj-$(CONFIG_OMAP_SSI)		+= omap_ssi.o
> -obj-$(CONFIG_OMAP_SSI_PORT)	+= omap_ssi_port.o
> +omap_ssi-objs		+= omap_ssi_core.o omap_ssi_port.o
> +obj-$(CONFIG_OMAP_SSI)	+= omap_ssi.o

Can you simply do

obj-$(CONFIG_OMAP_SSI)       +=  omap_ssi_core.o omap_ssi_port.o

instead?

Thanks,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 5/6] HSI: omap_ssi: built omap_ssi and omap_ssi_port into one module

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 698 bytes --]

Hi,

On Sun, May 01, 2016 at 11:43:49AM +0200, Pavel Machek wrote:
> > +++ b/drivers/hsi/controllers/Makefile
> > @@ -2,5 +2,5 @@
> >  # Makefile for HSI controllers drivers
> >  #
> >  
> > -obj-$(CONFIG_OMAP_SSI)		+= omap_ssi.o
> > -obj-$(CONFIG_OMAP_SSI_PORT)	+= omap_ssi_port.o
> > +omap_ssi-objs		+= omap_ssi_core.o omap_ssi_port.o
> > +obj-$(CONFIG_OMAP_SSI)	+= omap_ssi.o
> 
> Can you simply do
> 
> obj-$(CONFIG_OMAP_SSI)       +=  omap_ssi_core.o omap_ssi_port.o

No, that would result in omap_ssi_core.ko and omap_ssi_port.ko if
CONFIG_OMAP_SSI is enabled. Basically it's an optimized variant of
the current behaviour, but it still builds two modules.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 5/6] HSI: omap_ssi: built omap_ssi and omap_ssi_port into one module

[Pavel Machek]

On Sun 2016-05-01 21:34:10, Sebastian Reichel wrote:
> Hi,
> 
> On Sun, May 01, 2016 at 11:43:49AM +0200, Pavel Machek wrote:
> > > +++ b/drivers/hsi/controllers/Makefile
> > > @@ -2,5 +2,5 @@
> > >  # Makefile for HSI controllers drivers
> > >  #
> > >  
> > > -obj-$(CONFIG_OMAP_SSI)		+= omap_ssi.o
> > > -obj-$(CONFIG_OMAP_SSI_PORT)	+= omap_ssi_port.o
> > > +omap_ssi-objs		+= omap_ssi_core.o omap_ssi_port.o
> > > +obj-$(CONFIG_OMAP_SSI)	+= omap_ssi.o
> > 
> > Can you simply do
> > 
> > obj-$(CONFIG_OMAP_SSI)       +=  omap_ssi_core.o omap_ssi_port.o
> 
> No, that would result in omap_ssi_core.ko and omap_ssi_port.ko if
> CONFIG_OMAP_SSI is enabled. Basically it's an optimized variant of
> the current behaviour, but it still builds two modules.

Aha... I was not thinking about modular build.

Acked-by: Pavel Machek <pavel@ucw.cz>

Thanks,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 5/6] HSI: omap_ssi: built omap_ssi and omap_ssi_port into one module

[Tony Lindgren]

* Sebastian Reichel <sre@kernel.org> [160429 19:11]:
> Merge omap_ssi and omap_ssi_port into one module. This
> fixes problems with module cycle dependencies introduced
> by future patches.

Can you please check against the hardware for the split?
For reference, below is what I dumped out from dm3730 for
the modules on the L4 interconnect:

0x48000000 + 0x40000 + 0x18000 = 0x48058000, size 0x1000, parent with sysc
 0x48000000 + 0x40000 + 0x19000 = 0x48059000, size 0x1000, gdd
 0x48000000 + 0x40000 + 0x1a000 = 0x4805a000, size 0x1000, ssi_port1
 0x48000000 + 0x40000 + 0x1b000 = 0x4805b000, size 0x1000, ssi_port2

0x48000000 + 0x40000 + 0x1c000 = 0x4805c000, size 0x1000, target agent

So the parent target module at 0x48058000 controls everything
with the sysc register. The gdd, ssi_port1 and ssi_port2 are
children of the parent target module at 0x48058000 and should
not have any sysc related registers.

Can you please check if gdd, ssi_port1 and ssi_port2 have any
sysc related registers too? :) If they do, then they too can
idle on their own but most likely still depend on the parent
module.

The target agent above is a separate module with the
interconnect related registers, no need to do anything with
that AFAIK.

I believe this is the same for 34xx too but have not dumped it
out of the hardware. I can do that if the above does not match
what you're seeing.

If we want to have separate driver modules, you can do this:

1. Have the parent target module at 0x4805800 do PM runtime
   calls, they then propagate to the hwmod code properly for
   the ti,hwmods = "ssi" entry. This module can be minimal,
   and can also have child devices within it's first 0x1000
   sized range if needed.

2. Have the parent target module probe the child device
   drivers as needed with of_platform_populate() at the end
   of it's probe. The children can't be pm_runtime_irq_safe
   as it permanently blocks the idling of the parent.

3. Have the the parent target module at 0x4805800 implement
   PM runtime for it's children by registering
   struct dev_pm_ops for them.

If you really want to have them all as a single module then
that should work too as long as there's only one set of sysc
related registers.

Regards,

Tony

Re: [PATCH 5/6] HSI: omap_ssi: built omap_ssi and omap_ssi_port into one module

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 3721 bytes --]

Hi,

On Tue, May 03, 2016 at 10:32:39AM -0700, Tony Lindgren wrote:
> * Sebastian Reichel <sre@kernel.org> [160429 19:11]:
> > Merge omap_ssi and omap_ssi_port into one module. This
> > fixes problems with module cycle dependencies introduced
> > by future patches.
> 
> Can you please check against the hardware for the split?

This only merges the kernel modules. There are still
multiple devices.

> For reference, below is what I dumped out from dm3730 for the
> modules on the L4 interconnect:
> 
> 0x48000000 + 0x40000 + 0x18000 = 0x48058000, size 0x1000, parent with sysc
>  0x48000000 + 0x40000 + 0x19000 = 0x48059000, size 0x1000, gdd
>  0x48000000 + 0x40000 + 0x1a000 = 0x4805a000, size 0x1000, ssi_port1
>  0x48000000 + 0x40000 + 0x1b000 = 0x4805b000, size 0x1000, ssi_port2
> 
> 0x48000000 + 0x40000 + 0x1c000 = 0x4805c000, size 0x1000, target agent
> 
> So the parent target module at 0x48058000 controls everything
> with the sysc register. The gdd, ssi_port1 and ssi_port2 are
> children of the parent target module at 0x48058000 and should
> not have any sysc related registers.
> 
> Can you please check if gdd, ssi_port1 and ssi_port2 have any
> sysc related registers too? :) If they do, then they too can
> idle on their own but most likely still depend on the parent
> module.

The original driver from Nokia (I don't have proper documentation
[the SSI related parts are censored in the public OMAP TRM]) does
not give hints about any port related SYSC registers. Also it used
just one platform device for the whole ssi module. I'm pretty sure,
that the SSI stuff shares one set of SYSC registers.

> The target agent above is a separate module with the interconnect
> related registers, no need to do anything with that AFAIK.

The target agent is not referenced at all in Nokia's driver.

> I believe this is the same for 34xx too but have not dumped it
> out of the hardware. I can do that if the above does not match
> what you're seeing.

Parent with sysc/gdd/port1/port2 looks familiar.

> If we want to have separate driver modules, you can do this:
> 
> 1. Have the parent target module at 0x4805800 do PM runtime
>    calls, they then propagate to the hwmod code properly for
>    the ti,hwmods = "ssi" entry. This module can be minimal,
>    and can also have child devices within it's first 0x1000
>    sized range if needed.
>
> 2. Have the parent target module probe the child device
>    drivers as needed with of_platform_populate() at the end
>    of it's probe. The children can't be pm_runtime_irq_safe
>    as it permanently blocks the idling of the parent.
> 
> 3. Have the the parent target module at 0x4805800 implement
>    PM runtime for it's children by registering
>    struct dev_pm_ops for them.
>
> If you really want to have them all as a single module then
> that should work too as long as there's only one set of sysc
> related registers.

AFAIK there is only one set of sysc registers for the whole SSI
module, which must be active if any of the SSI related registers is
accessed. I think we should keep the current structure (ports being
sub-devices of the core), so runtime PM API will just work.

At the moment it does not work because of pm_runtime_irq_safe. I'm
currently working on that (my work-in-progress branch is [0]). With
the changes from this branch runtime PM status looks fine in sysfs
(I have not yet checked if SoC goes to idle if I enable runtime pm
for tty e.t.c.) also there are most likely still some "sleeping
function call from atomic context" bugs.

[0] https://git.kernel.org/cgit/linux/kernel/git/sre/linux-hsi.git/log/?h=runtime-pm-fixes

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 5/6] HSI: omap_ssi: built omap_ssi and omap_ssi_port into one module

[Tony Lindgren]

* Sebastian Reichel <sre@kernel.org> [160509 13:44]:
> Hi,
> 
> On Tue, May 03, 2016 at 10:32:39AM -0700, Tony Lindgren wrote:
> > * Sebastian Reichel <sre@kernel.org> [160429 19:11]:
> > > Merge omap_ssi and omap_ssi_port into one module. This
> > > fixes problems with module cycle dependencies introduced
> > > by future patches.
> > 
> > Can you please check against the hardware for the split?
> 
> This only merges the kernel modules. There are still
> multiple devices.

OK

> > For reference, below is what I dumped out from dm3730 for the
> > modules on the L4 interconnect:
> > 
> > 0x48000000 + 0x40000 + 0x18000 = 0x48058000, size 0x1000, parent with sysc
> >  0x48000000 + 0x40000 + 0x19000 = 0x48059000, size 0x1000, gdd
> >  0x48000000 + 0x40000 + 0x1a000 = 0x4805a000, size 0x1000, ssi_port1
> >  0x48000000 + 0x40000 + 0x1b000 = 0x4805b000, size 0x1000, ssi_port2
> > 
> > 0x48000000 + 0x40000 + 0x1c000 = 0x4805c000, size 0x1000, target agent
> > 
> > So the parent target module at 0x48058000 controls everything
> > with the sysc register. The gdd, ssi_port1 and ssi_port2 are
> > children of the parent target module at 0x48058000 and should
> > not have any sysc related registers.
> > 
> > Can you please check if gdd, ssi_port1 and ssi_port2 have any
> > sysc related registers too? :) If they do, then they too can
> > idle on their own but most likely still depend on the parent
> > module.
> 
> The original driver from Nokia (I don't have proper documentation
> [the SSI related parts are censored in the public OMAP TRM]) does
> not give hints about any port related SYSC registers. Also it used
> just one platform device for the whole ssi module. I'm pretty sure,
> that the SSI stuff shares one set of SYSC registers.

OK

> > The target agent above is a separate module with the interconnect
> > related registers, no need to do anything with that AFAIK.
> 
> The target agent is not referenced at all in Nokia's driver.

Yes chances are you don't have to do anything with that.

> > I believe this is the same for 34xx too but have not dumped it
> > out of the hardware. I can do that if the above does not match
> > what you're seeing.
> 
> Parent with sysc/gdd/port1/port2 looks familiar.
> 
> > If we want to have separate driver modules, you can do this:
> > 
> > 1. Have the parent target module at 0x4805800 do PM runtime
> >    calls, they then propagate to the hwmod code properly for
> >    the ti,hwmods = "ssi" entry. This module can be minimal,
> >    and can also have child devices within it's first 0x1000
> >    sized range if needed.
> >
> > 2. Have the parent target module probe the child device
> >    drivers as needed with of_platform_populate() at the end
> >    of it's probe. The children can't be pm_runtime_irq_safe
> >    as it permanently blocks the idling of the parent.
> > 
> > 3. Have the the parent target module at 0x4805800 implement
> >    PM runtime for it's children by registering
> >    struct dev_pm_ops for them.
> >
> > If you really want to have them all as a single module then
> > that should work too as long as there's only one set of sysc
> > related registers.
> 
> AFAIK there is only one set of sysc registers for the whole SSI
> module, which must be active if any of the SSI related registers is
> accessed. I think we should keep the current structure (ports being
> sub-devices of the core), so runtime PM API will just work.

OK makes sense, good to hear there's only one sysc register.

> At the moment it does not work because of pm_runtime_irq_safe. I'm
> currently working on that (my work-in-progress branch is [0]). With
> the changes from this branch runtime PM status looks fine in sysfs
> (I have not yet checked if SoC goes to idle if I enable runtime pm
> for tty e.t.c.) also there are most likely still some "sleeping
> function call from atomic context" bugs.

Yes pm_runtime_irq_safe is not nice as it permanently enables the
parent.. To avoid that you should just remove that and set up
delayed work where needed.

Regards,

Tony


> [0] https://git.kernel.org/cgit/linux/kernel/git/sre/linux-hsi.git/log/?h=runtime-pm-fixes

[PATCH 6/6] HSI: omap-ssi: add clk change support

[Sebastian Reichel]

This adds support for frequency changes of the SSI
functional clock, which may occur due to DVFS.

Signed-off-By: Sebastian Reichel <sre@kernel.org>
---
 drivers/hsi/controllers/omap_ssi.h      |  6 ++++
 drivers/hsi/controllers/omap_ssi_core.c | 63 +++++++++++++++++++++++++++++++++
 drivers/hsi/controllers/omap_ssi_port.c | 20 +++++++++++
 3 files changed, 89 insertions(+)

diff --git a/drivers/hsi/controllers/omap_ssi.h b/drivers/hsi/controllers/omap_ssi.h
index e493321cb0c3..7b4dec2c69ff 100644
--- a/drivers/hsi/controllers/omap_ssi.h
+++ b/drivers/hsi/controllers/omap_ssi.h
@@ -134,6 +134,8 @@ struct gdd_trn {
  * @gdd_tasklet: bottom half for DMA transfers
  * @gdd_trn: Array of GDD transaction data for ongoing GDD transfers
  * @lock: lock to serialize access to GDD
+ * @fck_nb: DVFS notfifier block
+ * @fck_rate: clock rate
  * @loss_count: To follow if we need to restore context or not
  * @max_speed: Maximum TX speed (Kb/s) set by the clients.
  * @sysconfig: SSI controller saved context
@@ -151,6 +153,7 @@ struct omap_ssi_controller {
 	struct tasklet_struct	gdd_tasklet;
 	struct gdd_trn		gdd_trn[SSI_MAX_GDD_LCH];
 	spinlock_t		lock;
+	struct notifier_block	fck_nb;
 	unsigned long		fck_rate;
 	u32			loss_count;
 	u32			max_speed;
@@ -164,6 +167,9 @@ struct omap_ssi_controller {
 #endif
 };
 
+void omap_ssi_port_update_fclk(struct hsi_controller *ssi,
+			       struct omap_ssi_port *omap_port);
+
 extern struct platform_driver ssi_port_pdriver;
 
 #endif /* __LINUX_HSI_OMAP_SSI_H__ */
diff --git a/drivers/hsi/controllers/omap_ssi_core.c b/drivers/hsi/controllers/omap_ssi_core.c
index 535c76038288..15b2a600d77b 100644
--- a/drivers/hsi/controllers/omap_ssi_core.c
+++ b/drivers/hsi/controllers/omap_ssi_core.c
@@ -290,6 +290,64 @@ static unsigned long ssi_get_clk_rate(struct hsi_controller *ssi)
 	return rate;
 }
 
+static int ssi_clk_event(struct notifier_block *nb, unsigned long event,
+								void *data)
+{
+	struct omap_ssi_controller *omap_ssi = container_of(nb,
+					struct omap_ssi_controller, fck_nb);
+	struct hsi_controller *ssi = to_hsi_controller(omap_ssi->dev);
+	struct clk_notifier_data *clk_data = data;
+	struct omap_ssi_port *omap_port;
+	int i;
+
+	switch (event) {
+	case PRE_RATE_CHANGE:
+		dev_dbg(&ssi->device, "pre rate change\n");
+
+		for (i = 0; i < ssi->num_ports; i++) {
+			omap_port = omap_ssi->port[i];
+
+			if (!omap_port)
+				continue;
+
+			/* Workaround for SWBREAK + CAwake down race in CMT */
+			tasklet_disable(&omap_port->wake_tasklet);
+
+			/* stop all ssi communication */
+			pinctrl_pm_select_idle_state(omap_port->pdev);
+			udelay(1); /* wait for racing frames */
+		}
+
+		break;
+	case ABORT_RATE_CHANGE:
+		dev_dbg(&ssi->device, "abort rate change\n");
+		/* Fall through */
+	case POST_RATE_CHANGE:
+		dev_dbg(&ssi->device, "post rate change (%lu -> %lu)\n",
+			clk_data->old_rate, clk_data->new_rate);
+		omap_ssi->fck_rate = DIV_ROUND_CLOSEST(clk_data->new_rate, 1000); /* KHz */
+
+		for (i = 0; i < ssi->num_ports; i++) {
+			omap_port = omap_ssi->port[i];
+
+			if (!omap_port)
+				continue;
+
+			omap_ssi_port_update_fclk(ssi, omap_port);
+
+			/* resume ssi communication */
+			pinctrl_pm_select_default_state(omap_port->pdev);
+			tasklet_enable(&omap_port->wake_tasklet);
+		}
+
+		break;
+	default:
+		break;
+	}
+
+	return NOTIFY_DONE;
+}
+
 static int ssi_get_iomem(struct platform_device *pd,
 		const char *name, void __iomem **pbase, dma_addr_t *phy)
 {
@@ -369,6 +427,10 @@ static int ssi_add_controller(struct hsi_controller *ssi,
 		goto out_err;
 	}
 
+	omap_ssi->fck_nb.notifier_call = ssi_clk_event;
+	omap_ssi->fck_nb.priority = INT_MAX;
+	clk_notifier_register(omap_ssi->fck, &omap_ssi->fck_nb);
+
 	/* TODO: find register, which can be used to detect context loss */
 	omap_ssi->get_loss = NULL;
 
@@ -432,6 +494,7 @@ static void ssi_remove_controller(struct hsi_controller *ssi)
 	int id = ssi->id;
 	tasklet_kill(&omap_ssi->gdd_tasklet);
 	hsi_unregister_controller(ssi);
+	clk_notifier_unregister(omap_ssi->fck, &omap_ssi->fck_nb);
 	ida_simple_remove(&platform_omap_ssi_ida, id);
 }
 
diff --git a/drivers/hsi/controllers/omap_ssi_port.c b/drivers/hsi/controllers/omap_ssi_port.c
index 1569bbb53ee8..98b22e88085c 100644
--- a/drivers/hsi/controllers/omap_ssi_port.c
+++ b/drivers/hsi/controllers/omap_ssi_port.c
@@ -23,6 +23,7 @@
 #include <linux/platform_device.h>
 #include <linux/dma-mapping.h>
 #include <linux/pm_runtime.h>
+#include <linux/delay.h>
 
 #include <linux/gpio/consumer.h>
 #include <linux/debugfs.h>
@@ -514,6 +515,11 @@ static int ssi_flush(struct hsi_client *cl)
 
 	pm_runtime_get_sync(omap_port->pdev);
 	spin_lock_bh(&omap_port->lock);
+
+	/* stop all ssi communication */
+	pinctrl_pm_select_idle_state(omap_port->pdev);
+	udelay(1); /* wait for racing frames */
+
 	/* Stop all DMA transfers */
 	for (i = 0; i < SSI_MAX_GDD_LCH; i++) {
 		msg = omap_ssi->gdd_trn[i].msg;
@@ -550,6 +556,10 @@ static int ssi_flush(struct hsi_client *cl)
 		ssi_flush_queue(&omap_port->rxqueue[i], NULL);
 	}
 	ssi_flush_queue(&omap_port->brkqueue, NULL);
+
+	/* Resume SSI communication */
+	pinctrl_pm_select_default_state(omap_port->pdev);
+
 	spin_unlock_bh(&omap_port->lock);
 	pm_runtime_put_sync(omap_port->pdev);
 
@@ -1302,6 +1312,16 @@ static int ssi_restore_divisor(struct omap_ssi_port *omap_port)
 	return 0;
 }
 
+void omap_ssi_port_update_fclk(struct hsi_controller *ssi,
+			       struct omap_ssi_port *omap_port)
+{
+	/* update divisor */
+	u32 div = ssi_calculate_div(ssi);
+	omap_port->sst.divisor = div;
+	ssi_restore_divisor(omap_port);
+}
+EXPORT_SYMBOL_GPL(omap_ssi_port_update_fclk);
+
 static int omap_ssi_port_runtime_suspend(struct device *dev)
 {
 	struct hsi_port *port = dev_get_drvdata(dev);
-- 
2.8.1

Re: [PATCH 6/6] HSI: omap-ssi: add clk change support

[Pavel Machek]

On Sat 2016-04-30 04:09:13, Sebastian Reichel wrote:
> This adds support for frequency changes of the SSI
> functional clock, which may occur due to DVFS.
> 
> Signed-off-By: Sebastian Reichel <sre@kernel.org>

Acked-by: Pavel Machek <pavel@ucw.cz>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html