Re: [PATCH v1 8/8] drm/etnaviv: Add a helper to get a pointer to the first available node

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 2023/7/17 18:07, Lucas Stach wrote:
Am Freitag, dem 23.06.2023 um 18:08 +0800 schrieb Sui Jingfeng:
From: Sui Jingfeng <suijingfeng@xxxxxxxxxxx>

This make the code in etnaviv_pdev_probe() less twisted, drop the reference
to device node after finished. Also kill a double blank line.

I can't spot the double blank line you claim to remove.

Signed-off-by: Sui Jingfeng <suijingfeng@xxxxxxxxxxx>
---
  drivers/gpu/drm/etnaviv/etnaviv_drv.c | 32 ++++++++++++++++++---------
  1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 7d0eeab3e8b7..3446f8eabf59 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -27,6 +27,19 @@
   * DRM operations:
   */
+/* If the DT contains at least one available GPU, return a pointer to it */
+
I think the code in the function is simple enough that we don't need a
comment explaining what it does.

Because the DT could disable GPU cores by add "status=disabled" property.

So, only the word *available* in this comments is deserved.

But I'm fine to delete the comment for this function, will be fixed at the next version.

Thanks for reviewing.

Regards,
Lucas

+static struct device_node *etnaviv_of_first_node(void)
+{
+	struct device_node *np;
+
+	for_each_compatible_node(np, NULL, "vivante,gc") {
+		if (of_device_is_available(np))
+			return np;
+	}
+
+	return NULL;
+}
static void load_gpu(struct drm_device *dev)
  {
@@ -587,7 +600,7 @@ static const struct component_master_ops etnaviv_master_ops = {
  static int etnaviv_pdev_probe(struct platform_device *pdev)
  {
  	struct device *dev = &pdev->dev;
-	struct device_node *first_node = NULL;
+	struct device_node *first_node;
  	struct component_match *match = NULL;
if (!dev->platform_data) {
@@ -597,11 +610,10 @@ static int etnaviv_pdev_probe(struct platform_device *pdev)
  			if (!of_device_is_available(core_node))
  				continue;
- if (!first_node)
-				first_node = core_node;
-
  			drm_of_component_match_add(&pdev->dev, &match,
  						   component_compare_of, core_node);
+
+			of_node_put(core_node);
  		}
  	} else {
  		char **names = dev->platform_data;
@@ -634,8 +646,11 @@ static int etnaviv_pdev_probe(struct platform_device *pdev)
  	 * device as the GPU we found. This assumes that all Vivante
  	 * GPUs in the system share the same DMA constraints.
  	 */
-	if (first_node)
+	first_node = etnaviv_of_first_node();
+	if (first_node) {
  		of_dma_configure(&pdev->dev, first_node, true);
+		of_node_put(first_node);
+	}
return component_master_add_with_match(dev, &etnaviv_master_ops, match);
  }
@@ -709,17 +724,14 @@ static int __init etnaviv_init(void)
  	 * If the DT contains at least one available GPU device, instantiate
  	 * the DRM platform device.
  	 */
-	for_each_compatible_node(np, NULL, "vivante,gc") {
-		if (!of_device_is_available(np))
-			continue;
+	np = etnaviv_of_first_node();
+	if (np) {
  		of_node_put(np);
ret = etnaviv_create_platform_device("etnaviv",
  						     &etnaviv_platform_device);
  		if (ret)
  			goto unregister_platform_driver;
-
-		break;
  	}
return 0;



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux