* [PATCH 0/3] drm: pl111: Fix module loading issues
@ 2020-04-09 1:39 Rob Herring
2020-04-09 1:39 ` Rob Herring
` (3 more replies)
0 siblings, 4 replies; 26+ messages in thread
From: Rob Herring @ 2020-04-09 1:39 UTC (permalink / raw)
To: Eric Anholt; +Cc: Linus Walleij, linux-arm-kernel
This is part of a wider effort to fully modularize ARCH_VEXPRESS. The
pl111 driver has a few issues with module loading and unloading. The
setup for VExpress is more complicated than it needs to be, so let's
simplify it.
Rob
Rob Herring (3):
drm: pl111: Fix module autoloading
drm: pl111: Simplify vexpress init
drm: pl111: Move VExpress setup into versatile init
drivers/gpu/drm/pl111/Makefile | 1 -
drivers/gpu/drm/pl111/pl111_drv.c | 1 +
drivers/gpu/drm/pl111/pl111_versatile.c | 135 +++++++++++++++++------
drivers/gpu/drm/pl111/pl111_vexpress.c | 138 ------------------------
drivers/gpu/drm/pl111/pl111_vexpress.h | 29 -----
5 files changed, 103 insertions(+), 201 deletions(-)
delete mode 100644 drivers/gpu/drm/pl111/pl111_vexpress.c
delete mode 100644 drivers/gpu/drm/pl111/pl111_vexpress.h
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 1/3] drm: pl111: Fix module autoloading
2020-04-09 1:39 [PATCH 0/3] drm: pl111: Fix module loading issues Rob Herring
@ 2020-04-09 1:39 ` Rob Herring
2020-04-09 1:39 ` Rob Herring
` (2 subsequent siblings)
3 siblings, 0 replies; 26+ messages in thread
From: Rob Herring @ 2020-04-09 1:39 UTC (permalink / raw)
To: Eric Anholt; +Cc: Linus Walleij, dri-devel, linux-arm-kernel
Add a missing MODULE_DEVICE_TABLE entry to fix module autoloading.
Cc: Eric Anholt <eric@anholt.net>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
drivers/gpu/drm/pl111/pl111_drv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
index aa8aa8d9e405..7f405b1374c2 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -444,6 +444,7 @@ static const struct amba_id pl111_id_table[] = {
},
{0, 0},
};
+MODULE_DEVICE_TABLE(amba, pl111_id_table);
static struct amba_driver pl111_amba_driver __maybe_unused = {
.drv = {
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 1/3] drm: pl111: Fix module autoloading
@ 2020-04-09 1:39 ` Rob Herring
0 siblings, 0 replies; 26+ messages in thread
From: Rob Herring @ 2020-04-09 1:39 UTC (permalink / raw)
To: Eric Anholt; +Cc: dri-devel, linux-arm-kernel
Add a missing MODULE_DEVICE_TABLE entry to fix module autoloading.
Cc: Eric Anholt <eric@anholt.net>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
drivers/gpu/drm/pl111/pl111_drv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
index aa8aa8d9e405..7f405b1374c2 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -444,6 +444,7 @@ static const struct amba_id pl111_id_table[] = {
},
{0, 0},
};
+MODULE_DEVICE_TABLE(amba, pl111_id_table);
static struct amba_driver pl111_amba_driver __maybe_unused = {
.drv = {
--
2.20.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 2/3] drm: pl111: Simplify vexpress init
2020-04-09 1:39 [PATCH 0/3] drm: pl111: Fix module loading issues Rob Herring
@ 2020-04-09 1:39 ` Rob Herring
2020-04-09 1:39 ` Rob Herring
` (2 subsequent siblings)
3 siblings, 0 replies; 26+ messages in thread
From: Rob Herring @ 2020-04-09 1:39 UTC (permalink / raw)
To: Eric Anholt; +Cc: Linus Walleij, dri-devel, linux-arm-kernel
The init VExpress variants currently instantiates a 'muxfpga' driver for
the sole purpose of getting a regmap for it. There's no reason to
instantiate a driver and doing so just complicates things. The muxfpga
driver also isn't unregistered properly on module unload. Let's
just simplify all this this by just calling
devm_regmap_init_vexpress_config() directly.
Cc: Eric Anholt <eric@anholt.net>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
drivers/gpu/drm/pl111/pl111_versatile.c | 21 +++----------
drivers/gpu/drm/pl111/pl111_vexpress.c | 42 -------------------------
drivers/gpu/drm/pl111/pl111_vexpress.h | 7 -----
3 files changed, 4 insertions(+), 66 deletions(-)
diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c
index 09aeaffb7660..8c2551088f26 100644
--- a/drivers/gpu/drm/pl111/pl111_versatile.c
+++ b/drivers/gpu/drm/pl111/pl111_versatile.c
@@ -8,6 +8,7 @@
#include <linux/of.h>
#include <linux/of_platform.h>
#include <linux/regmap.h>
+#include <linux/vexpress.h>
#include "pl111_versatile.h"
#include "pl111_vexpress.h"
@@ -325,17 +326,8 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
versatile_clcd_type = (enum versatile_clcd)clcd_id->data;
/* Versatile Express special handling */
- if (versatile_clcd_type == VEXPRESS_CLCD_V2M) {
+ if (IS_ENABLED(CONFIG_VEXPRESS_CONFIG) && versatile_clcd_type == VEXPRESS_CLCD_V2M) {
struct platform_device *pdev;
-
- /* Registers a driver for the muxfpga */
- ret = vexpress_muxfpga_init();
- if (ret) {
- dev_err(dev, "unable to initialize muxfpga driver\n");
- of_node_put(np);
- return ret;
- }
-
/* Call into deep Vexpress configuration API */
pdev = of_find_device_by_node(np);
if (!pdev) {
@@ -343,13 +335,8 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
of_node_put(np);
return -EPROBE_DEFER;
}
- map = dev_get_drvdata(&pdev->dev);
- if (!map) {
- dev_err(dev, "sysreg has not yet probed\n");
- platform_device_put(pdev);
- of_node_put(np);
- return -EPROBE_DEFER;
- }
+ map = devm_regmap_init_vexpress_config(&pdev->dev);
+ platform_device_put(pdev);
} else {
map = syscon_node_to_regmap(np);
}
diff --git a/drivers/gpu/drm/pl111/pl111_vexpress.c b/drivers/gpu/drm/pl111/pl111_vexpress.c
index 350570fe06b5..37ed3f1da820 100644
--- a/drivers/gpu/drm/pl111/pl111_vexpress.c
+++ b/drivers/gpu/drm/pl111/pl111_vexpress.c
@@ -94,45 +94,3 @@ int pl111_vexpress_clcd_init(struct device *dev,
return 0;
}
-
-/*
- * This sets up the regmap pointer that will then be retrieved by
- * the detection code in pl111_versatile.c and passed in to the
- * pl111_vexpress_clcd_init() function above.
- */
-static int vexpress_muxfpga_probe(struct platform_device *pdev)
-{
- struct device *dev = &pdev->dev;
- struct regmap *map;
-
- map = devm_regmap_init_vexpress_config(&pdev->dev);
- if (IS_ERR(map))
- return PTR_ERR(map);
- dev_set_drvdata(dev, map);
-
- return 0;
-}
-
-static const struct of_device_id vexpress_muxfpga_match[] = {
- { .compatible = "arm,vexpress-muxfpga", },
- {}
-};
-
-static struct platform_driver vexpress_muxfpga_driver = {
- .driver = {
- .name = "vexpress-muxfpga",
- .of_match_table = of_match_ptr(vexpress_muxfpga_match),
- },
- .probe = vexpress_muxfpga_probe,
-};
-
-int vexpress_muxfpga_init(void)
-{
- int ret;
-
- ret = platform_driver_register(&vexpress_muxfpga_driver);
- /* -EBUSY just means this driver is already registered */
- if (ret == -EBUSY)
- ret = 0;
- return ret;
-}
diff --git a/drivers/gpu/drm/pl111/pl111_vexpress.h b/drivers/gpu/drm/pl111/pl111_vexpress.h
index 5d3681bb4c00..bb54864ca91e 100644
--- a/drivers/gpu/drm/pl111/pl111_vexpress.h
+++ b/drivers/gpu/drm/pl111/pl111_vexpress.h
@@ -10,8 +10,6 @@ int pl111_vexpress_clcd_init(struct device *dev,
struct pl111_drm_dev_private *priv,
struct regmap *map);
-int vexpress_muxfpga_init(void);
-
#else
static inline int pl111_vexpress_clcd_init(struct device *dev,
@@ -21,9 +19,4 @@ static inline int pl111_vexpress_clcd_init(struct device *dev,
return -ENODEV;
}
-static inline int vexpress_muxfpga_init(void)
-{
- return 0;
-}
-
#endif
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 2/3] drm: pl111: Simplify vexpress init
@ 2020-04-09 1:39 ` Rob Herring
0 siblings, 0 replies; 26+ messages in thread
From: Rob Herring @ 2020-04-09 1:39 UTC (permalink / raw)
To: Eric Anholt; +Cc: dri-devel, linux-arm-kernel
The init VExpress variants currently instantiates a 'muxfpga' driver for
the sole purpose of getting a regmap for it. There's no reason to
instantiate a driver and doing so just complicates things. The muxfpga
driver also isn't unregistered properly on module unload. Let's
just simplify all this this by just calling
devm_regmap_init_vexpress_config() directly.
Cc: Eric Anholt <eric@anholt.net>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
drivers/gpu/drm/pl111/pl111_versatile.c | 21 +++----------
drivers/gpu/drm/pl111/pl111_vexpress.c | 42 -------------------------
drivers/gpu/drm/pl111/pl111_vexpress.h | 7 -----
3 files changed, 4 insertions(+), 66 deletions(-)
diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c
index 09aeaffb7660..8c2551088f26 100644
--- a/drivers/gpu/drm/pl111/pl111_versatile.c
+++ b/drivers/gpu/drm/pl111/pl111_versatile.c
@@ -8,6 +8,7 @@
#include <linux/of.h>
#include <linux/of_platform.h>
#include <linux/regmap.h>
+#include <linux/vexpress.h>
#include "pl111_versatile.h"
#include "pl111_vexpress.h"
@@ -325,17 +326,8 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
versatile_clcd_type = (enum versatile_clcd)clcd_id->data;
/* Versatile Express special handling */
- if (versatile_clcd_type == VEXPRESS_CLCD_V2M) {
+ if (IS_ENABLED(CONFIG_VEXPRESS_CONFIG) && versatile_clcd_type == VEXPRESS_CLCD_V2M) {
struct platform_device *pdev;
-
- /* Registers a driver for the muxfpga */
- ret = vexpress_muxfpga_init();
- if (ret) {
- dev_err(dev, "unable to initialize muxfpga driver\n");
- of_node_put(np);
- return ret;
- }
-
/* Call into deep Vexpress configuration API */
pdev = of_find_device_by_node(np);
if (!pdev) {
@@ -343,13 +335,8 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
of_node_put(np);
return -EPROBE_DEFER;
}
- map = dev_get_drvdata(&pdev->dev);
- if (!map) {
- dev_err(dev, "sysreg has not yet probed\n");
- platform_device_put(pdev);
- of_node_put(np);
- return -EPROBE_DEFER;
- }
+ map = devm_regmap_init_vexpress_config(&pdev->dev);
+ platform_device_put(pdev);
} else {
map = syscon_node_to_regmap(np);
}
diff --git a/drivers/gpu/drm/pl111/pl111_vexpress.c b/drivers/gpu/drm/pl111/pl111_vexpress.c
index 350570fe06b5..37ed3f1da820 100644
--- a/drivers/gpu/drm/pl111/pl111_vexpress.c
+++ b/drivers/gpu/drm/pl111/pl111_vexpress.c
@@ -94,45 +94,3 @@ int pl111_vexpress_clcd_init(struct device *dev,
return 0;
}
-
-/*
- * This sets up the regmap pointer that will then be retrieved by
- * the detection code in pl111_versatile.c and passed in to the
- * pl111_vexpress_clcd_init() function above.
- */
-static int vexpress_muxfpga_probe(struct platform_device *pdev)
-{
- struct device *dev = &pdev->dev;
- struct regmap *map;
-
- map = devm_regmap_init_vexpress_config(&pdev->dev);
- if (IS_ERR(map))
- return PTR_ERR(map);
- dev_set_drvdata(dev, map);
-
- return 0;
-}
-
-static const struct of_device_id vexpress_muxfpga_match[] = {
- { .compatible = "arm,vexpress-muxfpga", },
- {}
-};
-
-static struct platform_driver vexpress_muxfpga_driver = {
- .driver = {
- .name = "vexpress-muxfpga",
- .of_match_table = of_match_ptr(vexpress_muxfpga_match),
- },
- .probe = vexpress_muxfpga_probe,
-};
-
-int vexpress_muxfpga_init(void)
-{
- int ret;
-
- ret = platform_driver_register(&vexpress_muxfpga_driver);
- /* -EBUSY just means this driver is already registered */
- if (ret == -EBUSY)
- ret = 0;
- return ret;
-}
diff --git a/drivers/gpu/drm/pl111/pl111_vexpress.h b/drivers/gpu/drm/pl111/pl111_vexpress.h
index 5d3681bb4c00..bb54864ca91e 100644
--- a/drivers/gpu/drm/pl111/pl111_vexpress.h
+++ b/drivers/gpu/drm/pl111/pl111_vexpress.h
@@ -10,8 +10,6 @@ int pl111_vexpress_clcd_init(struct device *dev,
struct pl111_drm_dev_private *priv,
struct regmap *map);
-int vexpress_muxfpga_init(void);
-
#else
static inline int pl111_vexpress_clcd_init(struct device *dev,
@@ -21,9 +19,4 @@ static inline int pl111_vexpress_clcd_init(struct device *dev,
return -ENODEV;
}
-static inline int vexpress_muxfpga_init(void)
-{
- return 0;
-}
-
#endif
--
2.20.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 3/3] drm: pl111: Move VExpress setup into versatile init
2020-04-09 1:39 [PATCH 0/3] drm: pl111: Fix module loading issues Rob Herring
@ 2020-04-09 1:39 ` Rob Herring
2020-04-09 1:39 ` Rob Herring
` (2 subsequent siblings)
3 siblings, 0 replies; 26+ messages in thread
From: Rob Herring @ 2020-04-09 1:39 UTC (permalink / raw)
To: Eric Anholt; +Cc: Linus Walleij, dri-devel, linux-arm-kernel
Since the VExpress setup in pl111_vexpress.c is now just a single
function call, let's move it into pl111_versatile.c and we can further
simplify pl111_versatile_init() by moving the other pieces for VExpress
into pl111_vexpress_clcd_init().
Cc: Eric Anholt <eric@anholt.net>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
drivers/gpu/drm/pl111/Makefile | 1 -
drivers/gpu/drm/pl111/pl111_versatile.c | 122 ++++++++++++++++++++----
drivers/gpu/drm/pl111/pl111_vexpress.c | 96 -------------------
drivers/gpu/drm/pl111/pl111_vexpress.h | 22 -----
4 files changed, 102 insertions(+), 139 deletions(-)
delete mode 100644 drivers/gpu/drm/pl111/pl111_vexpress.c
delete mode 100644 drivers/gpu/drm/pl111/pl111_vexpress.h
diff --git a/drivers/gpu/drm/pl111/Makefile b/drivers/gpu/drm/pl111/Makefile
index 0c70f0e91d21..67d430d433e0 100644
--- a/drivers/gpu/drm/pl111/Makefile
+++ b/drivers/gpu/drm/pl111/Makefile
@@ -3,7 +3,6 @@ pl111_drm-y += pl111_display.o \
pl111_versatile.o \
pl111_drv.o
-pl111_drm-$(CONFIG_ARCH_VEXPRESS) += pl111_vexpress.o
pl111_drm-$(CONFIG_ARCH_NOMADIK) += pl111_nomadik.o
pl111_drm-$(CONFIG_DEBUG_FS) += pl111_debugfs.o
diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c
index 8c2551088f26..1b55f977e945 100644
--- a/drivers/gpu/drm/pl111/pl111_versatile.c
+++ b/drivers/gpu/drm/pl111/pl111_versatile.c
@@ -11,7 +11,6 @@
#include <linux/vexpress.h>
#include "pl111_versatile.h"
-#include "pl111_vexpress.h"
#include "pl111_drm.h"
static struct regmap *versatile_syscon_map;
@@ -309,13 +308,110 @@ static const struct pl111_variant_data pl111_vexpress = {
.broken_clockdivider = true,
};
+#define VEXPRESS_FPGAMUX_MOTHERBOARD 0x00
+#define VEXPRESS_FPGAMUX_DAUGHTERBOARD_1 0x01
+#define VEXPRESS_FPGAMUX_DAUGHTERBOARD_2 0x02
+
+static int pl111_vexpress_clcd_init(struct device *dev, struct device_node *np,
+ struct pl111_drm_dev_private *priv)
+{
+ struct platform_device *pdev;
+ struct device_node *root;
+ struct device_node *child;
+ struct device_node *ct_clcd = NULL;
+ struct regmap *map;
+ bool has_coretile_clcd = false;
+ bool has_coretile_hdlcd = false;
+ bool mux_motherboard = true;
+ u32 val;
+ int ret;
+
+ if (!IS_ENABLED(CONFIG_VEXPRESS_CONFIG))
+ return -ENODEV;
+
+ /*
+ * Check if we have a CLCD or HDLCD on the core tile by checking if a
+ * CLCD or HDLCD is available in the root of the device tree.
+ */
+ root = of_find_node_by_path("/");
+ if (!root)
+ return -EINVAL;
+
+ for_each_available_child_of_node(root, child) {
+ if (of_device_is_compatible(child, "arm,pl111")) {
+ has_coretile_clcd = true;
+ ct_clcd = child;
+ break;
+ }
+ if (of_device_is_compatible(child, "arm,hdlcd")) {
+ has_coretile_hdlcd = true;
+ of_node_put(child);
+ break;
+ }
+ }
+
+ of_node_put(root);
+
+ /*
+ * If there is a coretile HDLCD and it has a driver,
+ * do not mux the CLCD on the motherboard to the DVI.
+ */
+ if (has_coretile_hdlcd && IS_ENABLED(CONFIG_DRM_HDLCD))
+ mux_motherboard = false;
+
+ /*
+ * On the Vexpress CA9 we let the CLCD on the coretile
+ * take precedence, so also in this case do not mux the
+ * motherboard to the DVI.
+ */
+ if (has_coretile_clcd)
+ mux_motherboard = false;
+
+ if (mux_motherboard) {
+ dev_info(dev, "DVI muxed to motherboard CLCD\n");
+ val = VEXPRESS_FPGAMUX_MOTHERBOARD;
+ } else if (ct_clcd == dev->of_node) {
+ dev_info(dev,
+ "DVI muxed to daughterboard 1 (core tile) CLCD\n");
+ val = VEXPRESS_FPGAMUX_DAUGHTERBOARD_1;
+ } else {
+ dev_info(dev, "core tile graphics present\n");
+ dev_info(dev, "this device will be deactivated\n");
+ return -ENODEV;
+ }
+
+ /* Call into deep Vexpress configuration API */
+ pdev = of_find_device_by_node(np);
+ if (!pdev) {
+ dev_err(dev, "can't find the sysreg device, deferring\n");
+ return -EPROBE_DEFER;
+ }
+
+ map = devm_regmap_init_vexpress_config(&pdev->dev);
+ if (IS_ERR(map)) {
+ platform_device_put(pdev);
+ return PTR_ERR(map);
+ }
+
+ ret = regmap_write(map, 0, val);
+ platform_device_put(pdev);
+ if (ret) {
+ dev_err(dev, "error setting DVI muxmode\n");
+ return -ENODEV;
+ }
+
+ priv->variant = &pl111_vexpress;
+ dev_info(dev, "initializing Versatile Express PL111\n");
+
+ return 0;
+}
+
int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
{
const struct of_device_id *clcd_id;
enum versatile_clcd versatile_clcd_type;
struct device_node *np;
struct regmap *map;
- int ret;
np = of_find_matching_node_and_match(NULL, versatile_clcd_of_match,
&clcd_id);
@@ -326,17 +422,10 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
versatile_clcd_type = (enum versatile_clcd)clcd_id->data;
/* Versatile Express special handling */
- if (IS_ENABLED(CONFIG_VEXPRESS_CONFIG) && versatile_clcd_type == VEXPRESS_CLCD_V2M) {
- struct platform_device *pdev;
- /* Call into deep Vexpress configuration API */
- pdev = of_find_device_by_node(np);
- if (!pdev) {
- dev_err(dev, "can't find the sysreg device, deferring\n");
- of_node_put(np);
- return -EPROBE_DEFER;
- }
- map = devm_regmap_init_vexpress_config(&pdev->dev);
- platform_device_put(pdev);
+ if (versatile_clcd_type == VEXPRESS_CLCD_V2M) {
+ int ret = pl111_vexpress_clcd_init(dev, np, priv);
+ of_node_put(np);
+ return ret;
} else {
map = syscon_node_to_regmap(np);
}
@@ -380,13 +469,6 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
priv->variant_display_disable = pl111_realview_clcd_disable;
dev_info(dev, "set up callbacks for RealView PL111\n");
break;
- case VEXPRESS_CLCD_V2M:
- priv->variant = &pl111_vexpress;
- dev_info(dev, "initializing Versatile Express PL111\n");
- ret = pl111_vexpress_clcd_init(dev, priv, map);
- if (ret)
- return ret;
- break;
default:
dev_info(dev, "unknown Versatile system controller\n");
break;
diff --git a/drivers/gpu/drm/pl111/pl111_vexpress.c b/drivers/gpu/drm/pl111/pl111_vexpress.c
deleted file mode 100644
index 37ed3f1da820..000000000000
--- a/drivers/gpu/drm/pl111/pl111_vexpress.c
+++ /dev/null
@@ -1,96 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Versatile Express PL111 handling
- * Copyright (C) 2018 Linus Walleij
- *
- * This module binds to the "arm,vexpress-muxfpga" device on the
- * Versatile Express configuration bus and sets up which CLCD instance
- * gets muxed out on the DVI bridge.
- */
-#include <linux/device.h>
-#include <linux/module.h>
-#include <linux/regmap.h>
-#include <linux/vexpress.h>
-#include <linux/platform_device.h>
-#include <linux/of.h>
-#include <linux/of_address.h>
-#include <linux/of_platform.h>
-#include "pl111_drm.h"
-#include "pl111_vexpress.h"
-
-#define VEXPRESS_FPGAMUX_MOTHERBOARD 0x00
-#define VEXPRESS_FPGAMUX_DAUGHTERBOARD_1 0x01
-#define VEXPRESS_FPGAMUX_DAUGHTERBOARD_2 0x02
-
-int pl111_vexpress_clcd_init(struct device *dev,
- struct pl111_drm_dev_private *priv,
- struct regmap *map)
-{
- struct device_node *root;
- struct device_node *child;
- struct device_node *ct_clcd = NULL;
- bool has_coretile_clcd = false;
- bool has_coretile_hdlcd = false;
- bool mux_motherboard = true;
- u32 val;
- int ret;
-
- /*
- * Check if we have a CLCD or HDLCD on the core tile by checking if a
- * CLCD or HDLCD is available in the root of the device tree.
- */
- root = of_find_node_by_path("/");
- if (!root)
- return -EINVAL;
-
- for_each_available_child_of_node(root, child) {
- if (of_device_is_compatible(child, "arm,pl111")) {
- has_coretile_clcd = true;
- ct_clcd = child;
- break;
- }
- if (of_device_is_compatible(child, "arm,hdlcd")) {
- has_coretile_hdlcd = true;
- of_node_put(child);
- break;
- }
- }
-
- of_node_put(root);
-
- /*
- * If there is a coretile HDLCD and it has a driver,
- * do not mux the CLCD on the motherboard to the DVI.
- */
- if (has_coretile_hdlcd && IS_ENABLED(CONFIG_DRM_HDLCD))
- mux_motherboard = false;
-
- /*
- * On the Vexpress CA9 we let the CLCD on the coretile
- * take precedence, so also in this case do not mux the
- * motherboard to the DVI.
- */
- if (has_coretile_clcd)
- mux_motherboard = false;
-
- if (mux_motherboard) {
- dev_info(dev, "DVI muxed to motherboard CLCD\n");
- val = VEXPRESS_FPGAMUX_MOTHERBOARD;
- } else if (ct_clcd == dev->of_node) {
- dev_info(dev,
- "DVI muxed to daughterboard 1 (core tile) CLCD\n");
- val = VEXPRESS_FPGAMUX_DAUGHTERBOARD_1;
- } else {
- dev_info(dev, "core tile graphics present\n");
- dev_info(dev, "this device will be deactivated\n");
- return -ENODEV;
- }
-
- ret = regmap_write(map, 0, val);
- if (ret) {
- dev_err(dev, "error setting DVI muxmode\n");
- return -ENODEV;
- }
-
- return 0;
-}
diff --git a/drivers/gpu/drm/pl111/pl111_vexpress.h b/drivers/gpu/drm/pl111/pl111_vexpress.h
deleted file mode 100644
index bb54864ca91e..000000000000
--- a/drivers/gpu/drm/pl111/pl111_vexpress.h
+++ /dev/null
@@ -1,22 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-
-struct device;
-struct pl111_drm_dev_private;
-struct regmap;
-
-#ifdef CONFIG_ARCH_VEXPRESS
-
-int pl111_vexpress_clcd_init(struct device *dev,
- struct pl111_drm_dev_private *priv,
- struct regmap *map);
-
-#else
-
-static inline int pl111_vexpress_clcd_init(struct device *dev,
- struct pl111_drm_dev_private *priv,
- struct regmap *map)
-{
- return -ENODEV;
-}
-
-#endif
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 3/3] drm: pl111: Move VExpress setup into versatile init
@ 2020-04-09 1:39 ` Rob Herring
0 siblings, 0 replies; 26+ messages in thread
From: Rob Herring @ 2020-04-09 1:39 UTC (permalink / raw)
To: Eric Anholt; +Cc: dri-devel, linux-arm-kernel
Since the VExpress setup in pl111_vexpress.c is now just a single
function call, let's move it into pl111_versatile.c and we can further
simplify pl111_versatile_init() by moving the other pieces for VExpress
into pl111_vexpress_clcd_init().
Cc: Eric Anholt <eric@anholt.net>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
drivers/gpu/drm/pl111/Makefile | 1 -
drivers/gpu/drm/pl111/pl111_versatile.c | 122 ++++++++++++++++++++----
drivers/gpu/drm/pl111/pl111_vexpress.c | 96 -------------------
drivers/gpu/drm/pl111/pl111_vexpress.h | 22 -----
4 files changed, 102 insertions(+), 139 deletions(-)
delete mode 100644 drivers/gpu/drm/pl111/pl111_vexpress.c
delete mode 100644 drivers/gpu/drm/pl111/pl111_vexpress.h
diff --git a/drivers/gpu/drm/pl111/Makefile b/drivers/gpu/drm/pl111/Makefile
index 0c70f0e91d21..67d430d433e0 100644
--- a/drivers/gpu/drm/pl111/Makefile
+++ b/drivers/gpu/drm/pl111/Makefile
@@ -3,7 +3,6 @@ pl111_drm-y += pl111_display.o \
pl111_versatile.o \
pl111_drv.o
-pl111_drm-$(CONFIG_ARCH_VEXPRESS) += pl111_vexpress.o
pl111_drm-$(CONFIG_ARCH_NOMADIK) += pl111_nomadik.o
pl111_drm-$(CONFIG_DEBUG_FS) += pl111_debugfs.o
diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c
index 8c2551088f26..1b55f977e945 100644
--- a/drivers/gpu/drm/pl111/pl111_versatile.c
+++ b/drivers/gpu/drm/pl111/pl111_versatile.c
@@ -11,7 +11,6 @@
#include <linux/vexpress.h>
#include "pl111_versatile.h"
-#include "pl111_vexpress.h"
#include "pl111_drm.h"
static struct regmap *versatile_syscon_map;
@@ -309,13 +308,110 @@ static const struct pl111_variant_data pl111_vexpress = {
.broken_clockdivider = true,
};
+#define VEXPRESS_FPGAMUX_MOTHERBOARD 0x00
+#define VEXPRESS_FPGAMUX_DAUGHTERBOARD_1 0x01
+#define VEXPRESS_FPGAMUX_DAUGHTERBOARD_2 0x02
+
+static int pl111_vexpress_clcd_init(struct device *dev, struct device_node *np,
+ struct pl111_drm_dev_private *priv)
+{
+ struct platform_device *pdev;
+ struct device_node *root;
+ struct device_node *child;
+ struct device_node *ct_clcd = NULL;
+ struct regmap *map;
+ bool has_coretile_clcd = false;
+ bool has_coretile_hdlcd = false;
+ bool mux_motherboard = true;
+ u32 val;
+ int ret;
+
+ if (!IS_ENABLED(CONFIG_VEXPRESS_CONFIG))
+ return -ENODEV;
+
+ /*
+ * Check if we have a CLCD or HDLCD on the core tile by checking if a
+ * CLCD or HDLCD is available in the root of the device tree.
+ */
+ root = of_find_node_by_path("/");
+ if (!root)
+ return -EINVAL;
+
+ for_each_available_child_of_node(root, child) {
+ if (of_device_is_compatible(child, "arm,pl111")) {
+ has_coretile_clcd = true;
+ ct_clcd = child;
+ break;
+ }
+ if (of_device_is_compatible(child, "arm,hdlcd")) {
+ has_coretile_hdlcd = true;
+ of_node_put(child);
+ break;
+ }
+ }
+
+ of_node_put(root);
+
+ /*
+ * If there is a coretile HDLCD and it has a driver,
+ * do not mux the CLCD on the motherboard to the DVI.
+ */
+ if (has_coretile_hdlcd && IS_ENABLED(CONFIG_DRM_HDLCD))
+ mux_motherboard = false;
+
+ /*
+ * On the Vexpress CA9 we let the CLCD on the coretile
+ * take precedence, so also in this case do not mux the
+ * motherboard to the DVI.
+ */
+ if (has_coretile_clcd)
+ mux_motherboard = false;
+
+ if (mux_motherboard) {
+ dev_info(dev, "DVI muxed to motherboard CLCD\n");
+ val = VEXPRESS_FPGAMUX_MOTHERBOARD;
+ } else if (ct_clcd == dev->of_node) {
+ dev_info(dev,
+ "DVI muxed to daughterboard 1 (core tile) CLCD\n");
+ val = VEXPRESS_FPGAMUX_DAUGHTERBOARD_1;
+ } else {
+ dev_info(dev, "core tile graphics present\n");
+ dev_info(dev, "this device will be deactivated\n");
+ return -ENODEV;
+ }
+
+ /* Call into deep Vexpress configuration API */
+ pdev = of_find_device_by_node(np);
+ if (!pdev) {
+ dev_err(dev, "can't find the sysreg device, deferring\n");
+ return -EPROBE_DEFER;
+ }
+
+ map = devm_regmap_init_vexpress_config(&pdev->dev);
+ if (IS_ERR(map)) {
+ platform_device_put(pdev);
+ return PTR_ERR(map);
+ }
+
+ ret = regmap_write(map, 0, val);
+ platform_device_put(pdev);
+ if (ret) {
+ dev_err(dev, "error setting DVI muxmode\n");
+ return -ENODEV;
+ }
+
+ priv->variant = &pl111_vexpress;
+ dev_info(dev, "initializing Versatile Express PL111\n");
+
+ return 0;
+}
+
int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
{
const struct of_device_id *clcd_id;
enum versatile_clcd versatile_clcd_type;
struct device_node *np;
struct regmap *map;
- int ret;
np = of_find_matching_node_and_match(NULL, versatile_clcd_of_match,
&clcd_id);
@@ -326,17 +422,10 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
versatile_clcd_type = (enum versatile_clcd)clcd_id->data;
/* Versatile Express special handling */
- if (IS_ENABLED(CONFIG_VEXPRESS_CONFIG) && versatile_clcd_type == VEXPRESS_CLCD_V2M) {
- struct platform_device *pdev;
- /* Call into deep Vexpress configuration API */
- pdev = of_find_device_by_node(np);
- if (!pdev) {
- dev_err(dev, "can't find the sysreg device, deferring\n");
- of_node_put(np);
- return -EPROBE_DEFER;
- }
- map = devm_regmap_init_vexpress_config(&pdev->dev);
- platform_device_put(pdev);
+ if (versatile_clcd_type == VEXPRESS_CLCD_V2M) {
+ int ret = pl111_vexpress_clcd_init(dev, np, priv);
+ of_node_put(np);
+ return ret;
} else {
map = syscon_node_to_regmap(np);
}
@@ -380,13 +469,6 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
priv->variant_display_disable = pl111_realview_clcd_disable;
dev_info(dev, "set up callbacks for RealView PL111\n");
break;
- case VEXPRESS_CLCD_V2M:
- priv->variant = &pl111_vexpress;
- dev_info(dev, "initializing Versatile Express PL111\n");
- ret = pl111_vexpress_clcd_init(dev, priv, map);
- if (ret)
- return ret;
- break;
default:
dev_info(dev, "unknown Versatile system controller\n");
break;
diff --git a/drivers/gpu/drm/pl111/pl111_vexpress.c b/drivers/gpu/drm/pl111/pl111_vexpress.c
deleted file mode 100644
index 37ed3f1da820..000000000000
--- a/drivers/gpu/drm/pl111/pl111_vexpress.c
+++ /dev/null
@@ -1,96 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Versatile Express PL111 handling
- * Copyright (C) 2018 Linus Walleij
- *
- * This module binds to the "arm,vexpress-muxfpga" device on the
- * Versatile Express configuration bus and sets up which CLCD instance
- * gets muxed out on the DVI bridge.
- */
-#include <linux/device.h>
-#include <linux/module.h>
-#include <linux/regmap.h>
-#include <linux/vexpress.h>
-#include <linux/platform_device.h>
-#include <linux/of.h>
-#include <linux/of_address.h>
-#include <linux/of_platform.h>
-#include "pl111_drm.h"
-#include "pl111_vexpress.h"
-
-#define VEXPRESS_FPGAMUX_MOTHERBOARD 0x00
-#define VEXPRESS_FPGAMUX_DAUGHTERBOARD_1 0x01
-#define VEXPRESS_FPGAMUX_DAUGHTERBOARD_2 0x02
-
-int pl111_vexpress_clcd_init(struct device *dev,
- struct pl111_drm_dev_private *priv,
- struct regmap *map)
-{
- struct device_node *root;
- struct device_node *child;
- struct device_node *ct_clcd = NULL;
- bool has_coretile_clcd = false;
- bool has_coretile_hdlcd = false;
- bool mux_motherboard = true;
- u32 val;
- int ret;
-
- /*
- * Check if we have a CLCD or HDLCD on the core tile by checking if a
- * CLCD or HDLCD is available in the root of the device tree.
- */
- root = of_find_node_by_path("/");
- if (!root)
- return -EINVAL;
-
- for_each_available_child_of_node(root, child) {
- if (of_device_is_compatible(child, "arm,pl111")) {
- has_coretile_clcd = true;
- ct_clcd = child;
- break;
- }
- if (of_device_is_compatible(child, "arm,hdlcd")) {
- has_coretile_hdlcd = true;
- of_node_put(child);
- break;
- }
- }
-
- of_node_put(root);
-
- /*
- * If there is a coretile HDLCD and it has a driver,
- * do not mux the CLCD on the motherboard to the DVI.
- */
- if (has_coretile_hdlcd && IS_ENABLED(CONFIG_DRM_HDLCD))
- mux_motherboard = false;
-
- /*
- * On the Vexpress CA9 we let the CLCD on the coretile
- * take precedence, so also in this case do not mux the
- * motherboard to the DVI.
- */
- if (has_coretile_clcd)
- mux_motherboard = false;
-
- if (mux_motherboard) {
- dev_info(dev, "DVI muxed to motherboard CLCD\n");
- val = VEXPRESS_FPGAMUX_MOTHERBOARD;
- } else if (ct_clcd == dev->of_node) {
- dev_info(dev,
- "DVI muxed to daughterboard 1 (core tile) CLCD\n");
- val = VEXPRESS_FPGAMUX_DAUGHTERBOARD_1;
- } else {
- dev_info(dev, "core tile graphics present\n");
- dev_info(dev, "this device will be deactivated\n");
- return -ENODEV;
- }
-
- ret = regmap_write(map, 0, val);
- if (ret) {
- dev_err(dev, "error setting DVI muxmode\n");
- return -ENODEV;
- }
-
- return 0;
-}
diff --git a/drivers/gpu/drm/pl111/pl111_vexpress.h b/drivers/gpu/drm/pl111/pl111_vexpress.h
deleted file mode 100644
index bb54864ca91e..000000000000
--- a/drivers/gpu/drm/pl111/pl111_vexpress.h
+++ /dev/null
@@ -1,22 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-
-struct device;
-struct pl111_drm_dev_private;
-struct regmap;
-
-#ifdef CONFIG_ARCH_VEXPRESS
-
-int pl111_vexpress_clcd_init(struct device *dev,
- struct pl111_drm_dev_private *priv,
- struct regmap *map);
-
-#else
-
-static inline int pl111_vexpress_clcd_init(struct device *dev,
- struct pl111_drm_dev_private *priv,
- struct regmap *map)
-{
- return -ENODEV;
-}
-
-#endif
--
2.20.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 1/3] drm: pl111: Fix module autoloading
2020-04-09 1:39 ` Rob Herring
@ 2020-04-09 14:12 ` Sam Ravnborg
-1 siblings, 0 replies; 26+ messages in thread
From: Sam Ravnborg @ 2020-04-09 14:12 UTC (permalink / raw)
To: Rob Herring; +Cc: Eric Anholt, linux-arm-kernel, dri-devel
Hi Rob.
On Wed, Apr 08, 2020 at 07:39:45PM -0600, Rob Herring wrote:
> Add a missing MODULE_DEVICE_TABLE entry to fix module autoloading.
>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Rob Herring <robh@kernel.org>
Change looks straightforward.
Acked-by: Sam Ravnborg <sam@ravnborg.org>
> ---
> drivers/gpu/drm/pl111/pl111_drv.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
> index aa8aa8d9e405..7f405b1374c2 100644
> --- a/drivers/gpu/drm/pl111/pl111_drv.c
> +++ b/drivers/gpu/drm/pl111/pl111_drv.c
> @@ -444,6 +444,7 @@ static const struct amba_id pl111_id_table[] = {
> },
> {0, 0},
> };
> +MODULE_DEVICE_TABLE(amba, pl111_id_table);
>
> static struct amba_driver pl111_amba_driver __maybe_unused = {
> .drv = {
> --
> 2.20.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/3] drm: pl111: Fix module autoloading
@ 2020-04-09 14:12 ` Sam Ravnborg
0 siblings, 0 replies; 26+ messages in thread
From: Sam Ravnborg @ 2020-04-09 14:12 UTC (permalink / raw)
To: Rob Herring; +Cc: linux-arm-kernel, dri-devel
Hi Rob.
On Wed, Apr 08, 2020 at 07:39:45PM -0600, Rob Herring wrote:
> Add a missing MODULE_DEVICE_TABLE entry to fix module autoloading.
>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Rob Herring <robh@kernel.org>
Change looks straightforward.
Acked-by: Sam Ravnborg <sam@ravnborg.org>
> ---
> drivers/gpu/drm/pl111/pl111_drv.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
> index aa8aa8d9e405..7f405b1374c2 100644
> --- a/drivers/gpu/drm/pl111/pl111_drv.c
> +++ b/drivers/gpu/drm/pl111/pl111_drv.c
> @@ -444,6 +444,7 @@ static const struct amba_id pl111_id_table[] = {
> },
> {0, 0},
> };
> +MODULE_DEVICE_TABLE(amba, pl111_id_table);
>
> static struct amba_driver pl111_amba_driver __maybe_unused = {
> .drv = {
> --
> 2.20.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/3] drm: pl111: Simplify vexpress init
2020-04-09 1:39 ` Rob Herring
@ 2020-04-09 14:16 ` Sam Ravnborg
-1 siblings, 0 replies; 26+ messages in thread
From: Sam Ravnborg @ 2020-04-09 14:16 UTC (permalink / raw)
To: Rob Herring; +Cc: Eric Anholt, linux-arm-kernel, dri-devel
Hi Rob.
On Wed, Apr 08, 2020 at 07:39:46PM -0600, Rob Herring wrote:
> The init VExpress variants currently instantiates a 'muxfpga' driver for
> the sole purpose of getting a regmap for it. There's no reason to
> instantiate a driver and doing so just complicates things. The muxfpga
> driver also isn't unregistered properly on module unload. Let's
> just simplify all this this by just calling
> devm_regmap_init_vexpress_config() directly.
>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Rob Herring <robh@kernel.org>
Procastinating, so I took a look at this.
Nice simplification - on nit below.
Acked-by: Sam Ravnborg <sam@ravnborg.org>
> ---
> drivers/gpu/drm/pl111/pl111_versatile.c | 21 +++----------
> drivers/gpu/drm/pl111/pl111_vexpress.c | 42 -------------------------
> drivers/gpu/drm/pl111/pl111_vexpress.h | 7 -----
> 3 files changed, 4 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c
> index 09aeaffb7660..8c2551088f26 100644
> --- a/drivers/gpu/drm/pl111/pl111_versatile.c
> +++ b/drivers/gpu/drm/pl111/pl111_versatile.c
> @@ -8,6 +8,7 @@
> #include <linux/of.h>
> #include <linux/of_platform.h>
> #include <linux/regmap.h>
> +#include <linux/vexpress.h>
>
> #include "pl111_versatile.h"
> #include "pl111_vexpress.h"
> @@ -325,17 +326,8 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
> versatile_clcd_type = (enum versatile_clcd)clcd_id->data;
>
> /* Versatile Express special handling */
> - if (versatile_clcd_type == VEXPRESS_CLCD_V2M) {
> + if (IS_ENABLED(CONFIG_VEXPRESS_CONFIG) && versatile_clcd_type == VEXPRESS_CLCD_V2M) {
> struct platform_device *pdev;
> -
> - /* Registers a driver for the muxfpga */
> - ret = vexpress_muxfpga_init();
> - if (ret) {
> - dev_err(dev, "unable to initialize muxfpga driver\n");
> - of_node_put(np);
> - return ret;
> - }
> -
> /* Call into deep Vexpress configuration API */
> pdev = of_find_device_by_node(np);
> if (!pdev) {
> @@ -343,13 +335,8 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
> of_node_put(np);
> return -EPROBE_DEFER;
> }
> - map = dev_get_drvdata(&pdev->dev);
> - if (!map) {
> - dev_err(dev, "sysreg has not yet probed\n");
> - platform_device_put(pdev);
> - of_node_put(np);
> - return -EPROBE_DEFER;
> - }
> + map = devm_regmap_init_vexpress_config(&pdev->dev);
> + platform_device_put(pdev);
> } else {
> map = syscon_node_to_regmap(np);
> }
On the following line there is:
if (IS_ERR(map)) {
dev_err(dev, "no Versatile syscon regmap\n");
return PTR_ERR(map);
}
The error message no longer tell if this was
devm_regmap_init_vexpress_config() or syscon_node_to_regmap() that
caused the error.
Sam
> diff --git a/drivers/gpu/drm/pl111/pl111_vexpress.c b/drivers/gpu/drm/pl111/pl111_vexpress.c
> index 350570fe06b5..37ed3f1da820 100644
> --- a/drivers/gpu/drm/pl111/pl111_vexpress.c
> +++ b/drivers/gpu/drm/pl111/pl111_vexpress.c
> @@ -94,45 +94,3 @@ int pl111_vexpress_clcd_init(struct device *dev,
>
> return 0;
> }
> -
> -/*
> - * This sets up the regmap pointer that will then be retrieved by
> - * the detection code in pl111_versatile.c and passed in to the
> - * pl111_vexpress_clcd_init() function above.
> - */
> -static int vexpress_muxfpga_probe(struct platform_device *pdev)
> -{
> - struct device *dev = &pdev->dev;
> - struct regmap *map;
> -
> - map = devm_regmap_init_vexpress_config(&pdev->dev);
> - if (IS_ERR(map))
> - return PTR_ERR(map);
> - dev_set_drvdata(dev, map);
> -
> - return 0;
> -}
> -
> -static const struct of_device_id vexpress_muxfpga_match[] = {
> - { .compatible = "arm,vexpress-muxfpga", },
> - {}
> -};
> -
> -static struct platform_driver vexpress_muxfpga_driver = {
> - .driver = {
> - .name = "vexpress-muxfpga",
> - .of_match_table = of_match_ptr(vexpress_muxfpga_match),
> - },
> - .probe = vexpress_muxfpga_probe,
> -};
> -
> -int vexpress_muxfpga_init(void)
> -{
> - int ret;
> -
> - ret = platform_driver_register(&vexpress_muxfpga_driver);
> - /* -EBUSY just means this driver is already registered */
> - if (ret == -EBUSY)
> - ret = 0;
> - return ret;
> -}
> diff --git a/drivers/gpu/drm/pl111/pl111_vexpress.h b/drivers/gpu/drm/pl111/pl111_vexpress.h
> index 5d3681bb4c00..bb54864ca91e 100644
> --- a/drivers/gpu/drm/pl111/pl111_vexpress.h
> +++ b/drivers/gpu/drm/pl111/pl111_vexpress.h
> @@ -10,8 +10,6 @@ int pl111_vexpress_clcd_init(struct device *dev,
> struct pl111_drm_dev_private *priv,
> struct regmap *map);
>
> -int vexpress_muxfpga_init(void);
> -
> #else
>
> static inline int pl111_vexpress_clcd_init(struct device *dev,
> @@ -21,9 +19,4 @@ static inline int pl111_vexpress_clcd_init(struct device *dev,
> return -ENODEV;
> }
>
> -static inline int vexpress_muxfpga_init(void)
> -{
> - return 0;
> -}
> -
> #endif
> --
> 2.20.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/3] drm: pl111: Simplify vexpress init
@ 2020-04-09 14:16 ` Sam Ravnborg
0 siblings, 0 replies; 26+ messages in thread
From: Sam Ravnborg @ 2020-04-09 14:16 UTC (permalink / raw)
To: Rob Herring; +Cc: linux-arm-kernel, dri-devel
Hi Rob.
On Wed, Apr 08, 2020 at 07:39:46PM -0600, Rob Herring wrote:
> The init VExpress variants currently instantiates a 'muxfpga' driver for
> the sole purpose of getting a regmap for it. There's no reason to
> instantiate a driver and doing so just complicates things. The muxfpga
> driver also isn't unregistered properly on module unload. Let's
> just simplify all this this by just calling
> devm_regmap_init_vexpress_config() directly.
>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Rob Herring <robh@kernel.org>
Procastinating, so I took a look at this.
Nice simplification - on nit below.
Acked-by: Sam Ravnborg <sam@ravnborg.org>
> ---
> drivers/gpu/drm/pl111/pl111_versatile.c | 21 +++----------
> drivers/gpu/drm/pl111/pl111_vexpress.c | 42 -------------------------
> drivers/gpu/drm/pl111/pl111_vexpress.h | 7 -----
> 3 files changed, 4 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c
> index 09aeaffb7660..8c2551088f26 100644
> --- a/drivers/gpu/drm/pl111/pl111_versatile.c
> +++ b/drivers/gpu/drm/pl111/pl111_versatile.c
> @@ -8,6 +8,7 @@
> #include <linux/of.h>
> #include <linux/of_platform.h>
> #include <linux/regmap.h>
> +#include <linux/vexpress.h>
>
> #include "pl111_versatile.h"
> #include "pl111_vexpress.h"
> @@ -325,17 +326,8 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
> versatile_clcd_type = (enum versatile_clcd)clcd_id->data;
>
> /* Versatile Express special handling */
> - if (versatile_clcd_type == VEXPRESS_CLCD_V2M) {
> + if (IS_ENABLED(CONFIG_VEXPRESS_CONFIG) && versatile_clcd_type == VEXPRESS_CLCD_V2M) {
> struct platform_device *pdev;
> -
> - /* Registers a driver for the muxfpga */
> - ret = vexpress_muxfpga_init();
> - if (ret) {
> - dev_err(dev, "unable to initialize muxfpga driver\n");
> - of_node_put(np);
> - return ret;
> - }
> -
> /* Call into deep Vexpress configuration API */
> pdev = of_find_device_by_node(np);
> if (!pdev) {
> @@ -343,13 +335,8 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
> of_node_put(np);
> return -EPROBE_DEFER;
> }
> - map = dev_get_drvdata(&pdev->dev);
> - if (!map) {
> - dev_err(dev, "sysreg has not yet probed\n");
> - platform_device_put(pdev);
> - of_node_put(np);
> - return -EPROBE_DEFER;
> - }
> + map = devm_regmap_init_vexpress_config(&pdev->dev);
> + platform_device_put(pdev);
> } else {
> map = syscon_node_to_regmap(np);
> }
On the following line there is:
if (IS_ERR(map)) {
dev_err(dev, "no Versatile syscon regmap\n");
return PTR_ERR(map);
}
The error message no longer tell if this was
devm_regmap_init_vexpress_config() or syscon_node_to_regmap() that
caused the error.
Sam
> diff --git a/drivers/gpu/drm/pl111/pl111_vexpress.c b/drivers/gpu/drm/pl111/pl111_vexpress.c
> index 350570fe06b5..37ed3f1da820 100644
> --- a/drivers/gpu/drm/pl111/pl111_vexpress.c
> +++ b/drivers/gpu/drm/pl111/pl111_vexpress.c
> @@ -94,45 +94,3 @@ int pl111_vexpress_clcd_init(struct device *dev,
>
> return 0;
> }
> -
> -/*
> - * This sets up the regmap pointer that will then be retrieved by
> - * the detection code in pl111_versatile.c and passed in to the
> - * pl111_vexpress_clcd_init() function above.
> - */
> -static int vexpress_muxfpga_probe(struct platform_device *pdev)
> -{
> - struct device *dev = &pdev->dev;
> - struct regmap *map;
> -
> - map = devm_regmap_init_vexpress_config(&pdev->dev);
> - if (IS_ERR(map))
> - return PTR_ERR(map);
> - dev_set_drvdata(dev, map);
> -
> - return 0;
> -}
> -
> -static const struct of_device_id vexpress_muxfpga_match[] = {
> - { .compatible = "arm,vexpress-muxfpga", },
> - {}
> -};
> -
> -static struct platform_driver vexpress_muxfpga_driver = {
> - .driver = {
> - .name = "vexpress-muxfpga",
> - .of_match_table = of_match_ptr(vexpress_muxfpga_match),
> - },
> - .probe = vexpress_muxfpga_probe,
> -};
> -
> -int vexpress_muxfpga_init(void)
> -{
> - int ret;
> -
> - ret = platform_driver_register(&vexpress_muxfpga_driver);
> - /* -EBUSY just means this driver is already registered */
> - if (ret == -EBUSY)
> - ret = 0;
> - return ret;
> -}
> diff --git a/drivers/gpu/drm/pl111/pl111_vexpress.h b/drivers/gpu/drm/pl111/pl111_vexpress.h
> index 5d3681bb4c00..bb54864ca91e 100644
> --- a/drivers/gpu/drm/pl111/pl111_vexpress.h
> +++ b/drivers/gpu/drm/pl111/pl111_vexpress.h
> @@ -10,8 +10,6 @@ int pl111_vexpress_clcd_init(struct device *dev,
> struct pl111_drm_dev_private *priv,
> struct regmap *map);
>
> -int vexpress_muxfpga_init(void);
> -
> #else
>
> static inline int pl111_vexpress_clcd_init(struct device *dev,
> @@ -21,9 +19,4 @@ static inline int pl111_vexpress_clcd_init(struct device *dev,
> return -ENODEV;
> }
>
> -static inline int vexpress_muxfpga_init(void)
> -{
> - return 0;
> -}
> -
> #endif
> --
> 2.20.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/3] drm: pl111: Move VExpress setup into versatile init
2020-04-09 1:39 ` Rob Herring
@ 2020-04-09 14:34 ` Sam Ravnborg
-1 siblings, 0 replies; 26+ messages in thread
From: Sam Ravnborg @ 2020-04-09 14:34 UTC (permalink / raw)
To: Rob Herring; +Cc: Eric Anholt, linux-arm-kernel, dri-devel
Hi Rob.
On Wed, Apr 08, 2020 at 07:39:47PM -0600, Rob Herring wrote:
> Since the VExpress setup in pl111_vexpress.c is now just a single
> function call, let's move it into pl111_versatile.c and we can further
> simplify pl111_versatile_init() by moving the other pieces for VExpress
> into pl111_vexpress_clcd_init().
>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Rob Herring <robh@kernel.org>
Patch looks good - nits below, but anyway:
Acked-by: Sam Ravnborg <sam@ravnborg.org>
> ---
> drivers/gpu/drm/pl111/Makefile | 1 -
> drivers/gpu/drm/pl111/pl111_versatile.c | 122 ++++++++++++++++++++----
> drivers/gpu/drm/pl111/pl111_vexpress.c | 96 -------------------
> drivers/gpu/drm/pl111/pl111_vexpress.h | 22 -----
> 4 files changed, 102 insertions(+), 139 deletions(-)
> delete mode 100644 drivers/gpu/drm/pl111/pl111_vexpress.c
> delete mode 100644 drivers/gpu/drm/pl111/pl111_vexpress.h
>
> diff --git a/drivers/gpu/drm/pl111/Makefile b/drivers/gpu/drm/pl111/Makefile
> index 0c70f0e91d21..67d430d433e0 100644
> --- a/drivers/gpu/drm/pl111/Makefile
> +++ b/drivers/gpu/drm/pl111/Makefile
> @@ -3,7 +3,6 @@ pl111_drm-y += pl111_display.o \
> pl111_versatile.o \
> pl111_drv.o
>
> -pl111_drm-$(CONFIG_ARCH_VEXPRESS) += pl111_vexpress.o
> pl111_drm-$(CONFIG_ARCH_NOMADIK) += pl111_nomadik.o
> pl111_drm-$(CONFIG_DEBUG_FS) += pl111_debugfs.o
>
> diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c
> index 8c2551088f26..1b55f977e945 100644
> --- a/drivers/gpu/drm/pl111/pl111_versatile.c
> +++ b/drivers/gpu/drm/pl111/pl111_versatile.c
> @@ -11,7 +11,6 @@
> #include <linux/vexpress.h>
>
> #include "pl111_versatile.h"
> -#include "pl111_vexpress.h"
> #include "pl111_drm.h"
>
> static struct regmap *versatile_syscon_map;
> @@ -309,13 +308,110 @@ static const struct pl111_variant_data pl111_vexpress = {
> .broken_clockdivider = true,
> };
>
> +#define VEXPRESS_FPGAMUX_MOTHERBOARD 0x00
> +#define VEXPRESS_FPGAMUX_DAUGHTERBOARD_1 0x01
> +#define VEXPRESS_FPGAMUX_DAUGHTERBOARD_2 0x02
> +
> +static int pl111_vexpress_clcd_init(struct device *dev, struct device_node *np,
> + struct pl111_drm_dev_private *priv)
> +{
> + struct platform_device *pdev;
> + struct device_node *root;
> + struct device_node *child;
> + struct device_node *ct_clcd = NULL;
> + struct regmap *map;
> + bool has_coretile_clcd = false;
> + bool has_coretile_hdlcd = false;
> + bool mux_motherboard = true;
> + u32 val;
> + int ret;
> +
> + if (!IS_ENABLED(CONFIG_VEXPRESS_CONFIG))
> + return -ENODEV;
Thats was a simple way to get out of this.
Maybe a WARN() was in place, because should we ever hit this?
> +
> + /*
> + * Check if we have a CLCD or HDLCD on the core tile by checking if a
> + * CLCD or HDLCD is available in the root of the device tree.
> + */
> + root = of_find_node_by_path("/");
> + if (!root)
> + return -EINVAL;
> +
> + for_each_available_child_of_node(root, child) {
> + if (of_device_is_compatible(child, "arm,pl111")) {
> + has_coretile_clcd = true;
> + ct_clcd = child;
> + break;
> + }
> + if (of_device_is_compatible(child, "arm,hdlcd")) {
> + has_coretile_hdlcd = true;
> + of_node_put(child);
> + break;
> + }
> + }
> +
> + of_node_put(root);
> +
> + /*
> + * If there is a coretile HDLCD and it has a driver,
> + * do not mux the CLCD on the motherboard to the DVI.
> + */
> + if (has_coretile_hdlcd && IS_ENABLED(CONFIG_DRM_HDLCD))
> + mux_motherboard = false;
> +
> + /*
> + * On the Vexpress CA9 we let the CLCD on the coretile
> + * take precedence, so also in this case do not mux the
> + * motherboard to the DVI.
> + */
> + if (has_coretile_clcd)
> + mux_motherboard = false;
> +
> + if (mux_motherboard) {
> + dev_info(dev, "DVI muxed to motherboard CLCD\n");
> + val = VEXPRESS_FPGAMUX_MOTHERBOARD;
> + } else if (ct_clcd == dev->of_node) {
> + dev_info(dev,
> + "DVI muxed to daughterboard 1 (core tile) CLCD\n");
> + val = VEXPRESS_FPGAMUX_DAUGHTERBOARD_1;
> + } else {
> + dev_info(dev, "core tile graphics present\n");
> + dev_info(dev, "this device will be deactivated\n");
> + return -ENODEV;
> + }
> +
> + /* Call into deep Vexpress configuration API */
> + pdev = of_find_device_by_node(np);
> + if (!pdev) {
> + dev_err(dev, "can't find the sysreg device, deferring\n");
> + return -EPROBE_DEFER;
> + }
> +
> + map = devm_regmap_init_vexpress_config(&pdev->dev);
> + if (IS_ERR(map)) {
> + platform_device_put(pdev);
> + return PTR_ERR(map);
> + }
> +
> + ret = regmap_write(map, 0, val);
> + platform_device_put(pdev);
> + if (ret) {
> + dev_err(dev, "error setting DVI muxmode\n");
> + return -ENODEV;
> + }
> +
> + priv->variant = &pl111_vexpress;
> + dev_info(dev, "initializing Versatile Express PL111\n");
> +
> + return 0;
> +}
> +
> int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
> {
> const struct of_device_id *clcd_id;
> enum versatile_clcd versatile_clcd_type;
> struct device_node *np;
> struct regmap *map;
> - int ret;
>
> np = of_find_matching_node_and_match(NULL, versatile_clcd_of_match,
> &clcd_id);
> @@ -326,17 +422,10 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
> versatile_clcd_type = (enum versatile_clcd)clcd_id->data;
>
> /* Versatile Express special handling */
> - if (IS_ENABLED(CONFIG_VEXPRESS_CONFIG) && versatile_clcd_type == VEXPRESS_CLCD_V2M) {
> - struct platform_device *pdev;
> - /* Call into deep Vexpress configuration API */
> - pdev = of_find_device_by_node(np);
> - if (!pdev) {
> - dev_err(dev, "can't find the sysreg device, deferring\n");
> - of_node_put(np);
> - return -EPROBE_DEFER;
> - }
> - map = devm_regmap_init_vexpress_config(&pdev->dev);
> - platform_device_put(pdev);
> + if (versatile_clcd_type == VEXPRESS_CLCD_V2M) {
> + int ret = pl111_vexpress_clcd_init(dev, np, priv);
> + of_node_put(np);
> + return ret;
It is often in error situations one return early, but in this case it is
a normal case. Maybe a small comment?
> } else {
> map = syscon_node_to_regmap(np);
> }
> @@ -380,13 +469,6 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
> priv->variant_display_disable = pl111_realview_clcd_disable;
> dev_info(dev, "set up callbacks for RealView PL111\n");
> break;
> - case VEXPRESS_CLCD_V2M:
> - priv->variant = &pl111_vexpress;
> - dev_info(dev, "initializing Versatile Express PL111\n");
> - ret = pl111_vexpress_clcd_init(dev, priv, map);
> - if (ret)
> - return ret;
> - break;
The switch no longer includes VEXPRESS_CLCD_V2M - because we will never
reach the switch in this case.
I guess some gcc falgs may cause a warning that not all enums are
accounted for. But then again, the default may supress it.
Anyway, I noticed so you got a comment.
I think current code is fine.
Sam
> default:
> dev_info(dev, "unknown Versatile system controller\n");
> break;
> diff --git a/drivers/gpu/drm/pl111/pl111_vexpress.c b/drivers/gpu/drm/pl111/pl111_vexpress.c
> deleted file mode 100644
> index 37ed3f1da820..000000000000
> --- a/drivers/gpu/drm/pl111/pl111_vexpress.c
> +++ /dev/null
> @@ -1,96 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - * Versatile Express PL111 handling
> - * Copyright (C) 2018 Linus Walleij
> - *
> - * This module binds to the "arm,vexpress-muxfpga" device on the
> - * Versatile Express configuration bus and sets up which CLCD instance
> - * gets muxed out on the DVI bridge.
> - */
> -#include <linux/device.h>
> -#include <linux/module.h>
> -#include <linux/regmap.h>
> -#include <linux/vexpress.h>
> -#include <linux/platform_device.h>
> -#include <linux/of.h>
> -#include <linux/of_address.h>
> -#include <linux/of_platform.h>
> -#include "pl111_drm.h"
> -#include "pl111_vexpress.h"
> -
> -#define VEXPRESS_FPGAMUX_MOTHERBOARD 0x00
> -#define VEXPRESS_FPGAMUX_DAUGHTERBOARD_1 0x01
> -#define VEXPRESS_FPGAMUX_DAUGHTERBOARD_2 0x02
> -
> -int pl111_vexpress_clcd_init(struct device *dev,
> - struct pl111_drm_dev_private *priv,
> - struct regmap *map)
> -{
> - struct device_node *root;
> - struct device_node *child;
> - struct device_node *ct_clcd = NULL;
> - bool has_coretile_clcd = false;
> - bool has_coretile_hdlcd = false;
> - bool mux_motherboard = true;
> - u32 val;
> - int ret;
> -
> - /*
> - * Check if we have a CLCD or HDLCD on the core tile by checking if a
> - * CLCD or HDLCD is available in the root of the device tree.
> - */
> - root = of_find_node_by_path("/");
> - if (!root)
> - return -EINVAL;
> -
> - for_each_available_child_of_node(root, child) {
> - if (of_device_is_compatible(child, "arm,pl111")) {
> - has_coretile_clcd = true;
> - ct_clcd = child;
> - break;
> - }
> - if (of_device_is_compatible(child, "arm,hdlcd")) {
> - has_coretile_hdlcd = true;
> - of_node_put(child);
> - break;
> - }
> - }
> -
> - of_node_put(root);
> -
> - /*
> - * If there is a coretile HDLCD and it has a driver,
> - * do not mux the CLCD on the motherboard to the DVI.
> - */
> - if (has_coretile_hdlcd && IS_ENABLED(CONFIG_DRM_HDLCD))
> - mux_motherboard = false;
> -
> - /*
> - * On the Vexpress CA9 we let the CLCD on the coretile
> - * take precedence, so also in this case do not mux the
> - * motherboard to the DVI.
> - */
> - if (has_coretile_clcd)
> - mux_motherboard = false;
> -
> - if (mux_motherboard) {
> - dev_info(dev, "DVI muxed to motherboard CLCD\n");
> - val = VEXPRESS_FPGAMUX_MOTHERBOARD;
> - } else if (ct_clcd == dev->of_node) {
> - dev_info(dev,
> - "DVI muxed to daughterboard 1 (core tile) CLCD\n");
> - val = VEXPRESS_FPGAMUX_DAUGHTERBOARD_1;
> - } else {
> - dev_info(dev, "core tile graphics present\n");
> - dev_info(dev, "this device will be deactivated\n");
> - return -ENODEV;
> - }
> -
> - ret = regmap_write(map, 0, val);
> - if (ret) {
> - dev_err(dev, "error setting DVI muxmode\n");
> - return -ENODEV;
> - }
> -
> - return 0;
> -}
> diff --git a/drivers/gpu/drm/pl111/pl111_vexpress.h b/drivers/gpu/drm/pl111/pl111_vexpress.h
> deleted file mode 100644
> index bb54864ca91e..000000000000
> --- a/drivers/gpu/drm/pl111/pl111_vexpress.h
> +++ /dev/null
> @@ -1,22 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -
> -struct device;
> -struct pl111_drm_dev_private;
> -struct regmap;
> -
> -#ifdef CONFIG_ARCH_VEXPRESS
> -
> -int pl111_vexpress_clcd_init(struct device *dev,
> - struct pl111_drm_dev_private *priv,
> - struct regmap *map);
> -
> -#else
> -
> -static inline int pl111_vexpress_clcd_init(struct device *dev,
> - struct pl111_drm_dev_private *priv,
> - struct regmap *map)
> -{
> - return -ENODEV;
> -}
> -
> -#endif
> --
> 2.20.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/3] drm: pl111: Move VExpress setup into versatile init
@ 2020-04-09 14:34 ` Sam Ravnborg
0 siblings, 0 replies; 26+ messages in thread
From: Sam Ravnborg @ 2020-04-09 14:34 UTC (permalink / raw)
To: Rob Herring; +Cc: linux-arm-kernel, dri-devel
Hi Rob.
On Wed, Apr 08, 2020 at 07:39:47PM -0600, Rob Herring wrote:
> Since the VExpress setup in pl111_vexpress.c is now just a single
> function call, let's move it into pl111_versatile.c and we can further
> simplify pl111_versatile_init() by moving the other pieces for VExpress
> into pl111_vexpress_clcd_init().
>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Rob Herring <robh@kernel.org>
Patch looks good - nits below, but anyway:
Acked-by: Sam Ravnborg <sam@ravnborg.org>
> ---
> drivers/gpu/drm/pl111/Makefile | 1 -
> drivers/gpu/drm/pl111/pl111_versatile.c | 122 ++++++++++++++++++++----
> drivers/gpu/drm/pl111/pl111_vexpress.c | 96 -------------------
> drivers/gpu/drm/pl111/pl111_vexpress.h | 22 -----
> 4 files changed, 102 insertions(+), 139 deletions(-)
> delete mode 100644 drivers/gpu/drm/pl111/pl111_vexpress.c
> delete mode 100644 drivers/gpu/drm/pl111/pl111_vexpress.h
>
> diff --git a/drivers/gpu/drm/pl111/Makefile b/drivers/gpu/drm/pl111/Makefile
> index 0c70f0e91d21..67d430d433e0 100644
> --- a/drivers/gpu/drm/pl111/Makefile
> +++ b/drivers/gpu/drm/pl111/Makefile
> @@ -3,7 +3,6 @@ pl111_drm-y += pl111_display.o \
> pl111_versatile.o \
> pl111_drv.o
>
> -pl111_drm-$(CONFIG_ARCH_VEXPRESS) += pl111_vexpress.o
> pl111_drm-$(CONFIG_ARCH_NOMADIK) += pl111_nomadik.o
> pl111_drm-$(CONFIG_DEBUG_FS) += pl111_debugfs.o
>
> diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c
> index 8c2551088f26..1b55f977e945 100644
> --- a/drivers/gpu/drm/pl111/pl111_versatile.c
> +++ b/drivers/gpu/drm/pl111/pl111_versatile.c
> @@ -11,7 +11,6 @@
> #include <linux/vexpress.h>
>
> #include "pl111_versatile.h"
> -#include "pl111_vexpress.h"
> #include "pl111_drm.h"
>
> static struct regmap *versatile_syscon_map;
> @@ -309,13 +308,110 @@ static const struct pl111_variant_data pl111_vexpress = {
> .broken_clockdivider = true,
> };
>
> +#define VEXPRESS_FPGAMUX_MOTHERBOARD 0x00
> +#define VEXPRESS_FPGAMUX_DAUGHTERBOARD_1 0x01
> +#define VEXPRESS_FPGAMUX_DAUGHTERBOARD_2 0x02
> +
> +static int pl111_vexpress_clcd_init(struct device *dev, struct device_node *np,
> + struct pl111_drm_dev_private *priv)
> +{
> + struct platform_device *pdev;
> + struct device_node *root;
> + struct device_node *child;
> + struct device_node *ct_clcd = NULL;
> + struct regmap *map;
> + bool has_coretile_clcd = false;
> + bool has_coretile_hdlcd = false;
> + bool mux_motherboard = true;
> + u32 val;
> + int ret;
> +
> + if (!IS_ENABLED(CONFIG_VEXPRESS_CONFIG))
> + return -ENODEV;
Thats was a simple way to get out of this.
Maybe a WARN() was in place, because should we ever hit this?
> +
> + /*
> + * Check if we have a CLCD or HDLCD on the core tile by checking if a
> + * CLCD or HDLCD is available in the root of the device tree.
> + */
> + root = of_find_node_by_path("/");
> + if (!root)
> + return -EINVAL;
> +
> + for_each_available_child_of_node(root, child) {
> + if (of_device_is_compatible(child, "arm,pl111")) {
> + has_coretile_clcd = true;
> + ct_clcd = child;
> + break;
> + }
> + if (of_device_is_compatible(child, "arm,hdlcd")) {
> + has_coretile_hdlcd = true;
> + of_node_put(child);
> + break;
> + }
> + }
> +
> + of_node_put(root);
> +
> + /*
> + * If there is a coretile HDLCD and it has a driver,
> + * do not mux the CLCD on the motherboard to the DVI.
> + */
> + if (has_coretile_hdlcd && IS_ENABLED(CONFIG_DRM_HDLCD))
> + mux_motherboard = false;
> +
> + /*
> + * On the Vexpress CA9 we let the CLCD on the coretile
> + * take precedence, so also in this case do not mux the
> + * motherboard to the DVI.
> + */
> + if (has_coretile_clcd)
> + mux_motherboard = false;
> +
> + if (mux_motherboard) {
> + dev_info(dev, "DVI muxed to motherboard CLCD\n");
> + val = VEXPRESS_FPGAMUX_MOTHERBOARD;
> + } else if (ct_clcd == dev->of_node) {
> + dev_info(dev,
> + "DVI muxed to daughterboard 1 (core tile) CLCD\n");
> + val = VEXPRESS_FPGAMUX_DAUGHTERBOARD_1;
> + } else {
> + dev_info(dev, "core tile graphics present\n");
> + dev_info(dev, "this device will be deactivated\n");
> + return -ENODEV;
> + }
> +
> + /* Call into deep Vexpress configuration API */
> + pdev = of_find_device_by_node(np);
> + if (!pdev) {
> + dev_err(dev, "can't find the sysreg device, deferring\n");
> + return -EPROBE_DEFER;
> + }
> +
> + map = devm_regmap_init_vexpress_config(&pdev->dev);
> + if (IS_ERR(map)) {
> + platform_device_put(pdev);
> + return PTR_ERR(map);
> + }
> +
> + ret = regmap_write(map, 0, val);
> + platform_device_put(pdev);
> + if (ret) {
> + dev_err(dev, "error setting DVI muxmode\n");
> + return -ENODEV;
> + }
> +
> + priv->variant = &pl111_vexpress;
> + dev_info(dev, "initializing Versatile Express PL111\n");
> +
> + return 0;
> +}
> +
> int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
> {
> const struct of_device_id *clcd_id;
> enum versatile_clcd versatile_clcd_type;
> struct device_node *np;
> struct regmap *map;
> - int ret;
>
> np = of_find_matching_node_and_match(NULL, versatile_clcd_of_match,
> &clcd_id);
> @@ -326,17 +422,10 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
> versatile_clcd_type = (enum versatile_clcd)clcd_id->data;
>
> /* Versatile Express special handling */
> - if (IS_ENABLED(CONFIG_VEXPRESS_CONFIG) && versatile_clcd_type == VEXPRESS_CLCD_V2M) {
> - struct platform_device *pdev;
> - /* Call into deep Vexpress configuration API */
> - pdev = of_find_device_by_node(np);
> - if (!pdev) {
> - dev_err(dev, "can't find the sysreg device, deferring\n");
> - of_node_put(np);
> - return -EPROBE_DEFER;
> - }
> - map = devm_regmap_init_vexpress_config(&pdev->dev);
> - platform_device_put(pdev);
> + if (versatile_clcd_type == VEXPRESS_CLCD_V2M) {
> + int ret = pl111_vexpress_clcd_init(dev, np, priv);
> + of_node_put(np);
> + return ret;
It is often in error situations one return early, but in this case it is
a normal case. Maybe a small comment?
> } else {
> map = syscon_node_to_regmap(np);
> }
> @@ -380,13 +469,6 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
> priv->variant_display_disable = pl111_realview_clcd_disable;
> dev_info(dev, "set up callbacks for RealView PL111\n");
> break;
> - case VEXPRESS_CLCD_V2M:
> - priv->variant = &pl111_vexpress;
> - dev_info(dev, "initializing Versatile Express PL111\n");
> - ret = pl111_vexpress_clcd_init(dev, priv, map);
> - if (ret)
> - return ret;
> - break;
The switch no longer includes VEXPRESS_CLCD_V2M - because we will never
reach the switch in this case.
I guess some gcc falgs may cause a warning that not all enums are
accounted for. But then again, the default may supress it.
Anyway, I noticed so you got a comment.
I think current code is fine.
Sam
> default:
> dev_info(dev, "unknown Versatile system controller\n");
> break;
> diff --git a/drivers/gpu/drm/pl111/pl111_vexpress.c b/drivers/gpu/drm/pl111/pl111_vexpress.c
> deleted file mode 100644
> index 37ed3f1da820..000000000000
> --- a/drivers/gpu/drm/pl111/pl111_vexpress.c
> +++ /dev/null
> @@ -1,96 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - * Versatile Express PL111 handling
> - * Copyright (C) 2018 Linus Walleij
> - *
> - * This module binds to the "arm,vexpress-muxfpga" device on the
> - * Versatile Express configuration bus and sets up which CLCD instance
> - * gets muxed out on the DVI bridge.
> - */
> -#include <linux/device.h>
> -#include <linux/module.h>
> -#include <linux/regmap.h>
> -#include <linux/vexpress.h>
> -#include <linux/platform_device.h>
> -#include <linux/of.h>
> -#include <linux/of_address.h>
> -#include <linux/of_platform.h>
> -#include "pl111_drm.h"
> -#include "pl111_vexpress.h"
> -
> -#define VEXPRESS_FPGAMUX_MOTHERBOARD 0x00
> -#define VEXPRESS_FPGAMUX_DAUGHTERBOARD_1 0x01
> -#define VEXPRESS_FPGAMUX_DAUGHTERBOARD_2 0x02
> -
> -int pl111_vexpress_clcd_init(struct device *dev,
> - struct pl111_drm_dev_private *priv,
> - struct regmap *map)
> -{
> - struct device_node *root;
> - struct device_node *child;
> - struct device_node *ct_clcd = NULL;
> - bool has_coretile_clcd = false;
> - bool has_coretile_hdlcd = false;
> - bool mux_motherboard = true;
> - u32 val;
> - int ret;
> -
> - /*
> - * Check if we have a CLCD or HDLCD on the core tile by checking if a
> - * CLCD or HDLCD is available in the root of the device tree.
> - */
> - root = of_find_node_by_path("/");
> - if (!root)
> - return -EINVAL;
> -
> - for_each_available_child_of_node(root, child) {
> - if (of_device_is_compatible(child, "arm,pl111")) {
> - has_coretile_clcd = true;
> - ct_clcd = child;
> - break;
> - }
> - if (of_device_is_compatible(child, "arm,hdlcd")) {
> - has_coretile_hdlcd = true;
> - of_node_put(child);
> - break;
> - }
> - }
> -
> - of_node_put(root);
> -
> - /*
> - * If there is a coretile HDLCD and it has a driver,
> - * do not mux the CLCD on the motherboard to the DVI.
> - */
> - if (has_coretile_hdlcd && IS_ENABLED(CONFIG_DRM_HDLCD))
> - mux_motherboard = false;
> -
> - /*
> - * On the Vexpress CA9 we let the CLCD on the coretile
> - * take precedence, so also in this case do not mux the
> - * motherboard to the DVI.
> - */
> - if (has_coretile_clcd)
> - mux_motherboard = false;
> -
> - if (mux_motherboard) {
> - dev_info(dev, "DVI muxed to motherboard CLCD\n");
> - val = VEXPRESS_FPGAMUX_MOTHERBOARD;
> - } else if (ct_clcd == dev->of_node) {
> - dev_info(dev,
> - "DVI muxed to daughterboard 1 (core tile) CLCD\n");
> - val = VEXPRESS_FPGAMUX_DAUGHTERBOARD_1;
> - } else {
> - dev_info(dev, "core tile graphics present\n");
> - dev_info(dev, "this device will be deactivated\n");
> - return -ENODEV;
> - }
> -
> - ret = regmap_write(map, 0, val);
> - if (ret) {
> - dev_err(dev, "error setting DVI muxmode\n");
> - return -ENODEV;
> - }
> -
> - return 0;
> -}
> diff --git a/drivers/gpu/drm/pl111/pl111_vexpress.h b/drivers/gpu/drm/pl111/pl111_vexpress.h
> deleted file mode 100644
> index bb54864ca91e..000000000000
> --- a/drivers/gpu/drm/pl111/pl111_vexpress.h
> +++ /dev/null
> @@ -1,22 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -
> -struct device;
> -struct pl111_drm_dev_private;
> -struct regmap;
> -
> -#ifdef CONFIG_ARCH_VEXPRESS
> -
> -int pl111_vexpress_clcd_init(struct device *dev,
> - struct pl111_drm_dev_private *priv,
> - struct regmap *map);
> -
> -#else
> -
> -static inline int pl111_vexpress_clcd_init(struct device *dev,
> - struct pl111_drm_dev_private *priv,
> - struct regmap *map)
> -{
> - return -ENODEV;
> -}
> -
> -#endif
> --
> 2.20.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/3] drm: pl111: Move VExpress setup into versatile init
2020-04-09 14:34 ` Sam Ravnborg
@ 2020-04-17 17:33 ` Rob Herring
-1 siblings, 0 replies; 26+ messages in thread
From: Rob Herring @ 2020-04-17 17:33 UTC (permalink / raw)
To: Sam Ravnborg
Cc: Eric Anholt,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
dri-devel
On Thu, Apr 9, 2020 at 9:34 AM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Rob.
>
> On Wed, Apr 08, 2020 at 07:39:47PM -0600, Rob Herring wrote:
> > Since the VExpress setup in pl111_vexpress.c is now just a single
> > function call, let's move it into pl111_versatile.c and we can further
> > simplify pl111_versatile_init() by moving the other pieces for VExpress
> > into pl111_vexpress_clcd_init().
> >
> > Cc: Eric Anholt <eric@anholt.net>
> > Cc: dri-devel@lists.freedesktop.org
> > Signed-off-by: Rob Herring <robh@kernel.org>
> Patch looks good - nits below, but anyway:
>
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
>
>
> > ---
> > drivers/gpu/drm/pl111/Makefile | 1 -
> > drivers/gpu/drm/pl111/pl111_versatile.c | 122 ++++++++++++++++++++----
> > drivers/gpu/drm/pl111/pl111_vexpress.c | 96 -------------------
> > drivers/gpu/drm/pl111/pl111_vexpress.h | 22 -----
> > 4 files changed, 102 insertions(+), 139 deletions(-)
> > delete mode 100644 drivers/gpu/drm/pl111/pl111_vexpress.c
> > delete mode 100644 drivers/gpu/drm/pl111/pl111_vexpress.h
> >
> > diff --git a/drivers/gpu/drm/pl111/Makefile b/drivers/gpu/drm/pl111/Makefile
> > index 0c70f0e91d21..67d430d433e0 100644
> > --- a/drivers/gpu/drm/pl111/Makefile
> > +++ b/drivers/gpu/drm/pl111/Makefile
> > @@ -3,7 +3,6 @@ pl111_drm-y += pl111_display.o \
> > pl111_versatile.o \
> > pl111_drv.o
> >
> > -pl111_drm-$(CONFIG_ARCH_VEXPRESS) += pl111_vexpress.o
> > pl111_drm-$(CONFIG_ARCH_NOMADIK) += pl111_nomadik.o
> > pl111_drm-$(CONFIG_DEBUG_FS) += pl111_debugfs.o
> >
> > diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c
> > index 8c2551088f26..1b55f977e945 100644
> > --- a/drivers/gpu/drm/pl111/pl111_versatile.c
> > +++ b/drivers/gpu/drm/pl111/pl111_versatile.c
> > @@ -11,7 +11,6 @@
> > #include <linux/vexpress.h>
> >
> > #include "pl111_versatile.h"
> > -#include "pl111_vexpress.h"
> > #include "pl111_drm.h"
> >
> > static struct regmap *versatile_syscon_map;
> > @@ -309,13 +308,110 @@ static const struct pl111_variant_data pl111_vexpress = {
> > .broken_clockdivider = true,
> > };
> >
> > +#define VEXPRESS_FPGAMUX_MOTHERBOARD 0x00
> > +#define VEXPRESS_FPGAMUX_DAUGHTERBOARD_1 0x01
> > +#define VEXPRESS_FPGAMUX_DAUGHTERBOARD_2 0x02
> > +
> > +static int pl111_vexpress_clcd_init(struct device *dev, struct device_node *np,
> > + struct pl111_drm_dev_private *priv)
> > +{
> > + struct platform_device *pdev;
> > + struct device_node *root;
> > + struct device_node *child;
> > + struct device_node *ct_clcd = NULL;
> > + struct regmap *map;
> > + bool has_coretile_clcd = false;
> > + bool has_coretile_hdlcd = false;
> > + bool mux_motherboard = true;
> > + u32 val;
> > + int ret;
> > +
> > + if (!IS_ENABLED(CONFIG_VEXPRESS_CONFIG))
> > + return -ENODEV;
> Thats was a simple way to get out of this.
> Maybe a WARN() was in place, because should we ever hit this?
We didn't warn before if misconfigured. Adding an error print at the
caller should be enough.
> > +
> > + /*
> > + * Check if we have a CLCD or HDLCD on the core tile by checking if a
> > + * CLCD or HDLCD is available in the root of the device tree.
> > + */
> > + root = of_find_node_by_path("/");
> > + if (!root)
> > + return -EINVAL;
> > +
> > + for_each_available_child_of_node(root, child) {
> > + if (of_device_is_compatible(child, "arm,pl111")) {
> > + has_coretile_clcd = true;
> > + ct_clcd = child;
> > + break;
> > + }
> > + if (of_device_is_compatible(child, "arm,hdlcd")) {
> > + has_coretile_hdlcd = true;
> > + of_node_put(child);
> > + break;
> > + }
> > + }
> > +
> > + of_node_put(root);
> > +
> > + /*
> > + * If there is a coretile HDLCD and it has a driver,
> > + * do not mux the CLCD on the motherboard to the DVI.
> > + */
> > + if (has_coretile_hdlcd && IS_ENABLED(CONFIG_DRM_HDLCD))
> > + mux_motherboard = false;
> > +
> > + /*
> > + * On the Vexpress CA9 we let the CLCD on the coretile
> > + * take precedence, so also in this case do not mux the
> > + * motherboard to the DVI.
> > + */
> > + if (has_coretile_clcd)
> > + mux_motherboard = false;
> > +
> > + if (mux_motherboard) {
> > + dev_info(dev, "DVI muxed to motherboard CLCD\n");
> > + val = VEXPRESS_FPGAMUX_MOTHERBOARD;
> > + } else if (ct_clcd == dev->of_node) {
> > + dev_info(dev,
> > + "DVI muxed to daughterboard 1 (core tile) CLCD\n");
> > + val = VEXPRESS_FPGAMUX_DAUGHTERBOARD_1;
> > + } else {
> > + dev_info(dev, "core tile graphics present\n");
> > + dev_info(dev, "this device will be deactivated\n");
> > + return -ENODEV;
> > + }
> > +
> > + /* Call into deep Vexpress configuration API */
> > + pdev = of_find_device_by_node(np);
> > + if (!pdev) {
> > + dev_err(dev, "can't find the sysreg device, deferring\n");
> > + return -EPROBE_DEFER;
> > + }
> > +
> > + map = devm_regmap_init_vexpress_config(&pdev->dev);
> > + if (IS_ERR(map)) {
> > + platform_device_put(pdev);
> > + return PTR_ERR(map);
> > + }
> > +
> > + ret = regmap_write(map, 0, val);
> > + platform_device_put(pdev);
> > + if (ret) {
> > + dev_err(dev, "error setting DVI muxmode\n");
> > + return -ENODEV;
> > + }
> > +
> > + priv->variant = &pl111_vexpress;
> > + dev_info(dev, "initializing Versatile Express PL111\n");
> > +
> > + return 0;
> > +}
> > +
> > int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
> > {
> > const struct of_device_id *clcd_id;
> > enum versatile_clcd versatile_clcd_type;
> > struct device_node *np;
> > struct regmap *map;
> > - int ret;
> >
> > np = of_find_matching_node_and_match(NULL, versatile_clcd_of_match,
> > &clcd_id);
> > @@ -326,17 +422,10 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
> > versatile_clcd_type = (enum versatile_clcd)clcd_id->data;
> >
> > /* Versatile Express special handling */
> > - if (IS_ENABLED(CONFIG_VEXPRESS_CONFIG) && versatile_clcd_type == VEXPRESS_CLCD_V2M) {
> > - struct platform_device *pdev;
> > - /* Call into deep Vexpress configuration API */
> > - pdev = of_find_device_by_node(np);
> > - if (!pdev) {
> > - dev_err(dev, "can't find the sysreg device, deferring\n");
> > - of_node_put(np);
> > - return -EPROBE_DEFER;
> > - }
> > - map = devm_regmap_init_vexpress_config(&pdev->dev);
> > - platform_device_put(pdev);
> > + if (versatile_clcd_type == VEXPRESS_CLCD_V2M) {
> > + int ret = pl111_vexpress_clcd_init(dev, np, priv);
> > + of_node_put(np);
> > + return ret;
> It is often in error situations one return early, but in this case it is
> a normal case. Maybe a small comment?
I'm adding a print here to address your other comments and I think
that should be sufficient. I'll also move this up so the Vexpress
handling is before everything else.
>
>
> > } else {
And I don't need this else.
> > map = syscon_node_to_regmap(np);
> > }
> > @@ -380,13 +469,6 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
> > priv->variant_display_disable = pl111_realview_clcd_disable;
> > dev_info(dev, "set up callbacks for RealView PL111\n");
> > break;
> > - case VEXPRESS_CLCD_V2M:
> > - priv->variant = &pl111_vexpress;
> > - dev_info(dev, "initializing Versatile Express PL111\n");
> > - ret = pl111_vexpress_clcd_init(dev, priv, map);
> > - if (ret)
> > - return ret;
> > - break;
>
> The switch no longer includes VEXPRESS_CLCD_V2M - because we will never
> reach the switch in this case.
> I guess some gcc falgs may cause a warning that not all enums are
> accounted for. But then again, the default may supress it.
Yes, default suppresses that.
Rob
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/3] drm: pl111: Move VExpress setup into versatile init
@ 2020-04-17 17:33 ` Rob Herring
0 siblings, 0 replies; 26+ messages in thread
From: Rob Herring @ 2020-04-17 17:33 UTC (permalink / raw)
To: Sam Ravnborg
Cc: moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
dri-devel
On Thu, Apr 9, 2020 at 9:34 AM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Rob.
>
> On Wed, Apr 08, 2020 at 07:39:47PM -0600, Rob Herring wrote:
> > Since the VExpress setup in pl111_vexpress.c is now just a single
> > function call, let's move it into pl111_versatile.c and we can further
> > simplify pl111_versatile_init() by moving the other pieces for VExpress
> > into pl111_vexpress_clcd_init().
> >
> > Cc: Eric Anholt <eric@anholt.net>
> > Cc: dri-devel@lists.freedesktop.org
> > Signed-off-by: Rob Herring <robh@kernel.org>
> Patch looks good - nits below, but anyway:
>
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
>
>
> > ---
> > drivers/gpu/drm/pl111/Makefile | 1 -
> > drivers/gpu/drm/pl111/pl111_versatile.c | 122 ++++++++++++++++++++----
> > drivers/gpu/drm/pl111/pl111_vexpress.c | 96 -------------------
> > drivers/gpu/drm/pl111/pl111_vexpress.h | 22 -----
> > 4 files changed, 102 insertions(+), 139 deletions(-)
> > delete mode 100644 drivers/gpu/drm/pl111/pl111_vexpress.c
> > delete mode 100644 drivers/gpu/drm/pl111/pl111_vexpress.h
> >
> > diff --git a/drivers/gpu/drm/pl111/Makefile b/drivers/gpu/drm/pl111/Makefile
> > index 0c70f0e91d21..67d430d433e0 100644
> > --- a/drivers/gpu/drm/pl111/Makefile
> > +++ b/drivers/gpu/drm/pl111/Makefile
> > @@ -3,7 +3,6 @@ pl111_drm-y += pl111_display.o \
> > pl111_versatile.o \
> > pl111_drv.o
> >
> > -pl111_drm-$(CONFIG_ARCH_VEXPRESS) += pl111_vexpress.o
> > pl111_drm-$(CONFIG_ARCH_NOMADIK) += pl111_nomadik.o
> > pl111_drm-$(CONFIG_DEBUG_FS) += pl111_debugfs.o
> >
> > diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c
> > index 8c2551088f26..1b55f977e945 100644
> > --- a/drivers/gpu/drm/pl111/pl111_versatile.c
> > +++ b/drivers/gpu/drm/pl111/pl111_versatile.c
> > @@ -11,7 +11,6 @@
> > #include <linux/vexpress.h>
> >
> > #include "pl111_versatile.h"
> > -#include "pl111_vexpress.h"
> > #include "pl111_drm.h"
> >
> > static struct regmap *versatile_syscon_map;
> > @@ -309,13 +308,110 @@ static const struct pl111_variant_data pl111_vexpress = {
> > .broken_clockdivider = true,
> > };
> >
> > +#define VEXPRESS_FPGAMUX_MOTHERBOARD 0x00
> > +#define VEXPRESS_FPGAMUX_DAUGHTERBOARD_1 0x01
> > +#define VEXPRESS_FPGAMUX_DAUGHTERBOARD_2 0x02
> > +
> > +static int pl111_vexpress_clcd_init(struct device *dev, struct device_node *np,
> > + struct pl111_drm_dev_private *priv)
> > +{
> > + struct platform_device *pdev;
> > + struct device_node *root;
> > + struct device_node *child;
> > + struct device_node *ct_clcd = NULL;
> > + struct regmap *map;
> > + bool has_coretile_clcd = false;
> > + bool has_coretile_hdlcd = false;
> > + bool mux_motherboard = true;
> > + u32 val;
> > + int ret;
> > +
> > + if (!IS_ENABLED(CONFIG_VEXPRESS_CONFIG))
> > + return -ENODEV;
> Thats was a simple way to get out of this.
> Maybe a WARN() was in place, because should we ever hit this?
We didn't warn before if misconfigured. Adding an error print at the
caller should be enough.
> > +
> > + /*
> > + * Check if we have a CLCD or HDLCD on the core tile by checking if a
> > + * CLCD or HDLCD is available in the root of the device tree.
> > + */
> > + root = of_find_node_by_path("/");
> > + if (!root)
> > + return -EINVAL;
> > +
> > + for_each_available_child_of_node(root, child) {
> > + if (of_device_is_compatible(child, "arm,pl111")) {
> > + has_coretile_clcd = true;
> > + ct_clcd = child;
> > + break;
> > + }
> > + if (of_device_is_compatible(child, "arm,hdlcd")) {
> > + has_coretile_hdlcd = true;
> > + of_node_put(child);
> > + break;
> > + }
> > + }
> > +
> > + of_node_put(root);
> > +
> > + /*
> > + * If there is a coretile HDLCD and it has a driver,
> > + * do not mux the CLCD on the motherboard to the DVI.
> > + */
> > + if (has_coretile_hdlcd && IS_ENABLED(CONFIG_DRM_HDLCD))
> > + mux_motherboard = false;
> > +
> > + /*
> > + * On the Vexpress CA9 we let the CLCD on the coretile
> > + * take precedence, so also in this case do not mux the
> > + * motherboard to the DVI.
> > + */
> > + if (has_coretile_clcd)
> > + mux_motherboard = false;
> > +
> > + if (mux_motherboard) {
> > + dev_info(dev, "DVI muxed to motherboard CLCD\n");
> > + val = VEXPRESS_FPGAMUX_MOTHERBOARD;
> > + } else if (ct_clcd == dev->of_node) {
> > + dev_info(dev,
> > + "DVI muxed to daughterboard 1 (core tile) CLCD\n");
> > + val = VEXPRESS_FPGAMUX_DAUGHTERBOARD_1;
> > + } else {
> > + dev_info(dev, "core tile graphics present\n");
> > + dev_info(dev, "this device will be deactivated\n");
> > + return -ENODEV;
> > + }
> > +
> > + /* Call into deep Vexpress configuration API */
> > + pdev = of_find_device_by_node(np);
> > + if (!pdev) {
> > + dev_err(dev, "can't find the sysreg device, deferring\n");
> > + return -EPROBE_DEFER;
> > + }
> > +
> > + map = devm_regmap_init_vexpress_config(&pdev->dev);
> > + if (IS_ERR(map)) {
> > + platform_device_put(pdev);
> > + return PTR_ERR(map);
> > + }
> > +
> > + ret = regmap_write(map, 0, val);
> > + platform_device_put(pdev);
> > + if (ret) {
> > + dev_err(dev, "error setting DVI muxmode\n");
> > + return -ENODEV;
> > + }
> > +
> > + priv->variant = &pl111_vexpress;
> > + dev_info(dev, "initializing Versatile Express PL111\n");
> > +
> > + return 0;
> > +}
> > +
> > int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
> > {
> > const struct of_device_id *clcd_id;
> > enum versatile_clcd versatile_clcd_type;
> > struct device_node *np;
> > struct regmap *map;
> > - int ret;
> >
> > np = of_find_matching_node_and_match(NULL, versatile_clcd_of_match,
> > &clcd_id);
> > @@ -326,17 +422,10 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
> > versatile_clcd_type = (enum versatile_clcd)clcd_id->data;
> >
> > /* Versatile Express special handling */
> > - if (IS_ENABLED(CONFIG_VEXPRESS_CONFIG) && versatile_clcd_type == VEXPRESS_CLCD_V2M) {
> > - struct platform_device *pdev;
> > - /* Call into deep Vexpress configuration API */
> > - pdev = of_find_device_by_node(np);
> > - if (!pdev) {
> > - dev_err(dev, "can't find the sysreg device, deferring\n");
> > - of_node_put(np);
> > - return -EPROBE_DEFER;
> > - }
> > - map = devm_regmap_init_vexpress_config(&pdev->dev);
> > - platform_device_put(pdev);
> > + if (versatile_clcd_type == VEXPRESS_CLCD_V2M) {
> > + int ret = pl111_vexpress_clcd_init(dev, np, priv);
> > + of_node_put(np);
> > + return ret;
> It is often in error situations one return early, but in this case it is
> a normal case. Maybe a small comment?
I'm adding a print here to address your other comments and I think
that should be sufficient. I'll also move this up so the Vexpress
handling is before everything else.
>
>
> > } else {
And I don't need this else.
> > map = syscon_node_to_regmap(np);
> > }
> > @@ -380,13 +469,6 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
> > priv->variant_display_disable = pl111_realview_clcd_disable;
> > dev_info(dev, "set up callbacks for RealView PL111\n");
> > break;
> > - case VEXPRESS_CLCD_V2M:
> > - priv->variant = &pl111_vexpress;
> > - dev_info(dev, "initializing Versatile Express PL111\n");
> > - ret = pl111_vexpress_clcd_init(dev, priv, map);
> > - if (ret)
> > - return ret;
> > - break;
>
> The switch no longer includes VEXPRESS_CLCD_V2M - because we will never
> reach the switch in this case.
> I guess some gcc falgs may cause a warning that not all enums are
> accounted for. But then again, the default may supress it.
Yes, default suppresses that.
Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/3] drm: pl111: Simplify vexpress init
2020-04-09 14:16 ` Sam Ravnborg
@ 2020-04-17 17:43 ` Rob Herring
-1 siblings, 0 replies; 26+ messages in thread
From: Rob Herring @ 2020-04-17 17:43 UTC (permalink / raw)
To: Sam Ravnborg
Cc: Eric Anholt,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
dri-devel
On Thu, Apr 9, 2020 at 9:16 AM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Rob.
>
> On Wed, Apr 08, 2020 at 07:39:46PM -0600, Rob Herring wrote:
> > The init VExpress variants currently instantiates a 'muxfpga' driver for
> > the sole purpose of getting a regmap for it. There's no reason to
> > instantiate a driver and doing so just complicates things. The muxfpga
> > driver also isn't unregistered properly on module unload. Let's
> > just simplify all this this by just calling
> > devm_regmap_init_vexpress_config() directly.
> >
> > Cc: Eric Anholt <eric@anholt.net>
> > Cc: dri-devel@lists.freedesktop.org
> > Signed-off-by: Rob Herring <robh@kernel.org>
>
> Procastinating, so I took a look at this.
> Nice simplification - on nit below.
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> > ---
> > drivers/gpu/drm/pl111/pl111_versatile.c | 21 +++----------
> > drivers/gpu/drm/pl111/pl111_vexpress.c | 42 -------------------------
> > drivers/gpu/drm/pl111/pl111_vexpress.h | 7 -----
> > 3 files changed, 4 insertions(+), 66 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c
> > index 09aeaffb7660..8c2551088f26 100644
> > --- a/drivers/gpu/drm/pl111/pl111_versatile.c
> > +++ b/drivers/gpu/drm/pl111/pl111_versatile.c
> > @@ -8,6 +8,7 @@
> > #include <linux/of.h>
> > #include <linux/of_platform.h>
> > #include <linux/regmap.h>
> > +#include <linux/vexpress.h>
> >
> > #include "pl111_versatile.h"
> > #include "pl111_vexpress.h"
> > @@ -325,17 +326,8 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
> > versatile_clcd_type = (enum versatile_clcd)clcd_id->data;
> >
> > /* Versatile Express special handling */
> > - if (versatile_clcd_type == VEXPRESS_CLCD_V2M) {
> > + if (IS_ENABLED(CONFIG_VEXPRESS_CONFIG) && versatile_clcd_type == VEXPRESS_CLCD_V2M) {
> > struct platform_device *pdev;
> > -
> > - /* Registers a driver for the muxfpga */
> > - ret = vexpress_muxfpga_init();
> > - if (ret) {
> > - dev_err(dev, "unable to initialize muxfpga driver\n");
> > - of_node_put(np);
> > - return ret;
> > - }
> > -
> > /* Call into deep Vexpress configuration API */
> > pdev = of_find_device_by_node(np);
> > if (!pdev) {
> > @@ -343,13 +335,8 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
> > of_node_put(np);
> > return -EPROBE_DEFER;
> > }
> > - map = dev_get_drvdata(&pdev->dev);
> > - if (!map) {
> > - dev_err(dev, "sysreg has not yet probed\n");
> > - platform_device_put(pdev);
> > - of_node_put(np);
> > - return -EPROBE_DEFER;
> > - }
> > + map = devm_regmap_init_vexpress_config(&pdev->dev);
> > + platform_device_put(pdev);
> > } else {
> > map = syscon_node_to_regmap(np);
> > }
>
> On the following line there is:
> if (IS_ERR(map)) {
> dev_err(dev, "no Versatile syscon regmap\n");
> return PTR_ERR(map);
> }
>
> The error message no longer tell if this was
> devm_regmap_init_vexpress_config() or syscon_node_to_regmap() that
> caused the error.
Hopefully you'd know what platform you are on.
In any case, it's changed after patch 3.
Rob
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/3] drm: pl111: Simplify vexpress init
@ 2020-04-17 17:43 ` Rob Herring
0 siblings, 0 replies; 26+ messages in thread
From: Rob Herring @ 2020-04-17 17:43 UTC (permalink / raw)
To: Sam Ravnborg
Cc: moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
dri-devel
On Thu, Apr 9, 2020 at 9:16 AM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Rob.
>
> On Wed, Apr 08, 2020 at 07:39:46PM -0600, Rob Herring wrote:
> > The init VExpress variants currently instantiates a 'muxfpga' driver for
> > the sole purpose of getting a regmap for it. There's no reason to
> > instantiate a driver and doing so just complicates things. The muxfpga
> > driver also isn't unregistered properly on module unload. Let's
> > just simplify all this this by just calling
> > devm_regmap_init_vexpress_config() directly.
> >
> > Cc: Eric Anholt <eric@anholt.net>
> > Cc: dri-devel@lists.freedesktop.org
> > Signed-off-by: Rob Herring <robh@kernel.org>
>
> Procastinating, so I took a look at this.
> Nice simplification - on nit below.
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> > ---
> > drivers/gpu/drm/pl111/pl111_versatile.c | 21 +++----------
> > drivers/gpu/drm/pl111/pl111_vexpress.c | 42 -------------------------
> > drivers/gpu/drm/pl111/pl111_vexpress.h | 7 -----
> > 3 files changed, 4 insertions(+), 66 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c
> > index 09aeaffb7660..8c2551088f26 100644
> > --- a/drivers/gpu/drm/pl111/pl111_versatile.c
> > +++ b/drivers/gpu/drm/pl111/pl111_versatile.c
> > @@ -8,6 +8,7 @@
> > #include <linux/of.h>
> > #include <linux/of_platform.h>
> > #include <linux/regmap.h>
> > +#include <linux/vexpress.h>
> >
> > #include "pl111_versatile.h"
> > #include "pl111_vexpress.h"
> > @@ -325,17 +326,8 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
> > versatile_clcd_type = (enum versatile_clcd)clcd_id->data;
> >
> > /* Versatile Express special handling */
> > - if (versatile_clcd_type == VEXPRESS_CLCD_V2M) {
> > + if (IS_ENABLED(CONFIG_VEXPRESS_CONFIG) && versatile_clcd_type == VEXPRESS_CLCD_V2M) {
> > struct platform_device *pdev;
> > -
> > - /* Registers a driver for the muxfpga */
> > - ret = vexpress_muxfpga_init();
> > - if (ret) {
> > - dev_err(dev, "unable to initialize muxfpga driver\n");
> > - of_node_put(np);
> > - return ret;
> > - }
> > -
> > /* Call into deep Vexpress configuration API */
> > pdev = of_find_device_by_node(np);
> > if (!pdev) {
> > @@ -343,13 +335,8 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
> > of_node_put(np);
> > return -EPROBE_DEFER;
> > }
> > - map = dev_get_drvdata(&pdev->dev);
> > - if (!map) {
> > - dev_err(dev, "sysreg has not yet probed\n");
> > - platform_device_put(pdev);
> > - of_node_put(np);
> > - return -EPROBE_DEFER;
> > - }
> > + map = devm_regmap_init_vexpress_config(&pdev->dev);
> > + platform_device_put(pdev);
> > } else {
> > map = syscon_node_to_regmap(np);
> > }
>
> On the following line there is:
> if (IS_ERR(map)) {
> dev_err(dev, "no Versatile syscon regmap\n");
> return PTR_ERR(map);
> }
>
> The error message no longer tell if this was
> devm_regmap_init_vexpress_config() or syscon_node_to_regmap() that
> caused the error.
Hopefully you'd know what platform you are on.
In any case, it's changed after patch 3.
Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/3] drm: pl111: Fix module autoloading
2020-04-09 1:39 ` Rob Herring
@ 2020-04-17 20:09 ` Linus Walleij
-1 siblings, 0 replies; 26+ messages in thread
From: Linus Walleij @ 2020-04-17 20:09 UTC (permalink / raw)
To: Rob Herring; +Cc: Eric Anholt, open list:DRM PANEL DRIVERS, Linux ARM
On Thu, Apr 9, 2020 at 3:39 AM Rob Herring <robh@kernel.org> wrote:
> Add a missing MODULE_DEVICE_TABLE entry to fix module autoloading.
>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Rob Herring <robh@kernel.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/3] drm: pl111: Fix module autoloading
@ 2020-04-17 20:09 ` Linus Walleij
0 siblings, 0 replies; 26+ messages in thread
From: Linus Walleij @ 2020-04-17 20:09 UTC (permalink / raw)
To: Rob Herring; +Cc: open list:DRM PANEL DRIVERS, Linux ARM
On Thu, Apr 9, 2020 at 3:39 AM Rob Herring <robh@kernel.org> wrote:
> Add a missing MODULE_DEVICE_TABLE entry to fix module autoloading.
>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Rob Herring <robh@kernel.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/3] drm: pl111: Simplify vexpress init
2020-04-09 1:39 ` Rob Herring
@ 2020-04-17 20:23 ` Linus Walleij
-1 siblings, 0 replies; 26+ messages in thread
From: Linus Walleij @ 2020-04-17 20:23 UTC (permalink / raw)
To: Rob Herring; +Cc: Eric Anholt, open list:DRM PANEL DRIVERS, Linux ARM
On Thu, Apr 9, 2020 at 3:39 AM Rob Herring <robh@kernel.org> wrote:
> The init VExpress variants currently instantiates a 'muxfpga' driver for
> the sole purpose of getting a regmap for it. There's no reason to
> instantiate a driver and doing so just complicates things. The muxfpga
> driver also isn't unregistered properly on module unload. Let's
> just simplify all this this by just calling
> devm_regmap_init_vexpress_config() directly.
>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Rob Herring <robh@kernel.org>
OK... looking at this.
> + if (IS_ENABLED(CONFIG_VEXPRESS_CONFIG) && versatile_clcd_type == VEXPRESS_CLCD_V2M) {
> struct platform_device *pdev;
> -
> - /* Registers a driver for the muxfpga */
> - ret = vexpress_muxfpga_init();
> - if (ret) {
> - dev_err(dev, "unable to initialize muxfpga driver\n");
> - of_node_put(np);
> - return ret;
> - }
> -
> /* Call into deep Vexpress configuration API */
> pdev = of_find_device_by_node(np);
So this finds the platform device for compatible "arm,vexpress-muxfpga",
ha!
> + map = devm_regmap_init_vexpress_config(&pdev->dev);
> + platform_device_put(pdev);
So then you can just do it like that.
Clever! Hats off for digging through my (unnecessary complex) code.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/3] drm: pl111: Simplify vexpress init
@ 2020-04-17 20:23 ` Linus Walleij
0 siblings, 0 replies; 26+ messages in thread
From: Linus Walleij @ 2020-04-17 20:23 UTC (permalink / raw)
To: Rob Herring; +Cc: open list:DRM PANEL DRIVERS, Linux ARM
On Thu, Apr 9, 2020 at 3:39 AM Rob Herring <robh@kernel.org> wrote:
> The init VExpress variants currently instantiates a 'muxfpga' driver for
> the sole purpose of getting a regmap for it. There's no reason to
> instantiate a driver and doing so just complicates things. The muxfpga
> driver also isn't unregistered properly on module unload. Let's
> just simplify all this this by just calling
> devm_regmap_init_vexpress_config() directly.
>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Rob Herring <robh@kernel.org>
OK... looking at this.
> + if (IS_ENABLED(CONFIG_VEXPRESS_CONFIG) && versatile_clcd_type == VEXPRESS_CLCD_V2M) {
> struct platform_device *pdev;
> -
> - /* Registers a driver for the muxfpga */
> - ret = vexpress_muxfpga_init();
> - if (ret) {
> - dev_err(dev, "unable to initialize muxfpga driver\n");
> - of_node_put(np);
> - return ret;
> - }
> -
> /* Call into deep Vexpress configuration API */
> pdev = of_find_device_by_node(np);
So this finds the platform device for compatible "arm,vexpress-muxfpga",
ha!
> + map = devm_regmap_init_vexpress_config(&pdev->dev);
> + platform_device_put(pdev);
So then you can just do it like that.
Clever! Hats off for digging through my (unnecessary complex) code.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/3] drm: pl111: Move VExpress setup into versatile init
2020-04-09 1:39 ` Rob Herring
@ 2020-04-17 20:27 ` Linus Walleij
-1 siblings, 0 replies; 26+ messages in thread
From: Linus Walleij @ 2020-04-17 20:27 UTC (permalink / raw)
To: Rob Herring; +Cc: Eric Anholt, open list:DRM PANEL DRIVERS, Linux ARM
On Thu, Apr 9, 2020 at 3:39 AM Rob Herring <robh@kernel.org> wrote:
> Since the VExpress setup in pl111_vexpress.c is now just a single
> function call, let's move it into pl111_versatile.c and we can further
> simplify pl111_versatile_init() by moving the other pieces for VExpress
> into pl111_vexpress_clcd_init().
>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Rob Herring <robh@kernel.org>
Yeah that's much nicer, the other boards get a copy of the
Vexpress code but it's so little so it doesn't matter and besides
the Vexpress already had copies of the other boards init code.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/3] drm: pl111: Move VExpress setup into versatile init
@ 2020-04-17 20:27 ` Linus Walleij
0 siblings, 0 replies; 26+ messages in thread
From: Linus Walleij @ 2020-04-17 20:27 UTC (permalink / raw)
To: Rob Herring; +Cc: open list:DRM PANEL DRIVERS, Linux ARM
On Thu, Apr 9, 2020 at 3:39 AM Rob Herring <robh@kernel.org> wrote:
> Since the VExpress setup in pl111_vexpress.c is now just a single
> function call, let's move it into pl111_versatile.c and we can further
> simplify pl111_versatile_init() by moving the other pieces for VExpress
> into pl111_vexpress_clcd_init().
>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Rob Herring <robh@kernel.org>
Yeah that's much nicer, the other boards get a copy of the
Vexpress code but it's so little so it doesn't matter and besides
the Vexpress already had copies of the other boards init code.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 0/3] drm: pl111: Fix module loading issues
2020-04-09 1:39 [PATCH 0/3] drm: pl111: Fix module loading issues Rob Herring
` (2 preceding siblings ...)
2020-04-09 1:39 ` Rob Herring
@ 2020-04-17 20:28 ` Linus Walleij
3 siblings, 0 replies; 26+ messages in thread
From: Linus Walleij @ 2020-04-17 20:28 UTC (permalink / raw)
To: Rob Herring; +Cc: Eric Anholt, Linux ARM
On Thu, Apr 9, 2020 at 3:39 AM Rob Herring <robh@kernel.org> wrote:
> This is part of a wider effort to fully modularize ARCH_VEXPRESS. The
> pl111 driver has a few issues with module loading and unloading. The
> setup for VExpress is more complicated than it needs to be, so let's
> simplify it.
Just go ahead and push these to the DRM tree when you feel
confident, have you tested it on real hardware BTW?
Yours,
Linus Walleij
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/3] drm: pl111: Move VExpress setup into versatile init
2020-04-17 20:27 ` Linus Walleij
@ 2020-04-19 17:12 ` Rob Herring
-1 siblings, 0 replies; 26+ messages in thread
From: Rob Herring @ 2020-04-19 17:12 UTC (permalink / raw)
To: Linus Walleij; +Cc: Eric Anholt, open list:DRM PANEL DRIVERS, Linux ARM
On Fri, Apr 17, 2020 at 3:27 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Thu, Apr 9, 2020 at 3:39 AM Rob Herring <robh@kernel.org> wrote:
>
> > Since the VExpress setup in pl111_vexpress.c is now just a single
> > function call, let's move it into pl111_versatile.c and we can further
> > simplify pl111_versatile_init() by moving the other pieces for VExpress
> > into pl111_vexpress_clcd_init().
> >
> > Cc: Eric Anholt <eric@anholt.net>
> > Cc: dri-devel@lists.freedesktop.org
> > Signed-off-by: Rob Herring <robh@kernel.org>
>
> Yeah that's much nicer, the other boards get a copy of the
> Vexpress code but it's so little so it doesn't matter and besides
> the Vexpress already had copies of the other boards init code.
It shouldn't change. The compiler should be smart enough to drop all
the code when IS_ENABLED(CONFIG_ARCH_VEXPRESS) is false.
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Thanks.
Rob
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/3] drm: pl111: Move VExpress setup into versatile init
@ 2020-04-19 17:12 ` Rob Herring
0 siblings, 0 replies; 26+ messages in thread
From: Rob Herring @ 2020-04-19 17:12 UTC (permalink / raw)
To: Linus Walleij; +Cc: open list:DRM PANEL DRIVERS, Linux ARM
On Fri, Apr 17, 2020 at 3:27 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Thu, Apr 9, 2020 at 3:39 AM Rob Herring <robh@kernel.org> wrote:
>
> > Since the VExpress setup in pl111_vexpress.c is now just a single
> > function call, let's move it into pl111_versatile.c and we can further
> > simplify pl111_versatile_init() by moving the other pieces for VExpress
> > into pl111_vexpress_clcd_init().
> >
> > Cc: Eric Anholt <eric@anholt.net>
> > Cc: dri-devel@lists.freedesktop.org
> > Signed-off-by: Rob Herring <robh@kernel.org>
>
> Yeah that's much nicer, the other boards get a copy of the
> Vexpress code but it's so little so it doesn't matter and besides
> the Vexpress already had copies of the other boards init code.
It shouldn't change. The compiler should be smart enough to drop all
the code when IS_ENABLED(CONFIG_ARCH_VEXPRESS) is false.
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Thanks.
Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2020-04-19 17:14 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-04-09 1:39 [PATCH 0/3] drm: pl111: Fix module loading issues Rob Herring
2020-04-09 1:39 ` [PATCH 1/3] drm: pl111: Fix module autoloading Rob Herring
2020-04-09 1:39 ` Rob Herring
2020-04-09 14:12 ` Sam Ravnborg
2020-04-09 14:12 ` Sam Ravnborg
2020-04-17 20:09 ` Linus Walleij
2020-04-17 20:09 ` Linus Walleij
2020-04-09 1:39 ` [PATCH 2/3] drm: pl111: Simplify vexpress init Rob Herring
2020-04-09 1:39 ` Rob Herring
2020-04-09 14:16 ` Sam Ravnborg
2020-04-09 14:16 ` Sam Ravnborg
2020-04-17 17:43 ` Rob Herring
2020-04-17 17:43 ` Rob Herring
2020-04-17 20:23 ` Linus Walleij
2020-04-17 20:23 ` Linus Walleij
2020-04-09 1:39 ` [PATCH 3/3] drm: pl111: Move VExpress setup into versatile init Rob Herring
2020-04-09 1:39 ` Rob Herring
2020-04-09 14:34 ` Sam Ravnborg
2020-04-09 14:34 ` Sam Ravnborg
2020-04-17 17:33 ` Rob Herring
2020-04-17 17:33 ` Rob Herring
2020-04-17 20:27 ` Linus Walleij
2020-04-17 20:27 ` Linus Walleij
2020-04-19 17:12 ` Rob Herring
2020-04-19 17:12 ` Rob Herring
2020-04-17 20:28 ` [PATCH 0/3] drm: pl111: Fix module loading issues Linus Walleij
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.