Re: [PATCH v2 1/2] of: Create platform devices for OF framebuffers

[Thomas Zimmermann]

Hi

Am 19.04.22 um 15:30 schrieb Rob Herring:
...

-#ifndef CONFIG_PPC
  static const struct of_device_id reserved_mem_matches[] = {
        { .compatible = "qcom,rmtfs-mem" },
        { .compatible = "qcom,cmd-db" },
@@ -520,33 +519,81 @@ static const struct of_device_id reserved_mem_matches[] = 
{

  {
-       struct device_node *node;
-

static int __init of_platform_default_populate_init(void)

As both if/else clauses need 'node', I'd keep this declared here.

Ok.

        device_links_supplier_sync_state_pause();

                return -ENODEV;

-        * Handle certain compatibles explicitly, since we don't want to create
-        * platform_devices for every node in /reserved-memory with a
-        * "compatible",
-        */
-       for_each_matching_node(node, reserved_mem_matches)
-               of_platform_device_create(node, NULL, NULL);
+       if (IS_ENABLED(CONFIG_PPC)) {
+               struct device_node *boot_display = NULL;
+               struct device_node *node;
+               struct platform_device *dev;
+               int ret;
+
+               /* Check if we have a MacOS display without a node spec */
+               if (of_get_property(of_chosen, "linux,bootx-noscreen", NULL)) {
+                       /*
+                        * The old code tried to work out which node was the 
MacOS
+                        * display based on the address. I'm dropping that 
since the
+                        * lack of a node spec only happens with old BootX 
versions
+                        * (users can update) and with this code, they'll still 
get
+                        * a display (just not the palette hacks).
+                        */
+                       dev = platform_device_alloc("bootx-noscreen", 0);
+                       if (WARN_ON(!dev))
+                               return -ENOMEM;
+                       ret = platform_device_add(dev);
+                       if (WARN_ON(ret)) {
+                               platform_device_put(dev);
+                               return ret;
+                       }
+               }

-       if (node) {
-               of_platform_populate(node, NULL, NULL, NULL);
-               of_node_put(node);
-       }
+               /*
+                * For OF framebuffers, first create the device for the boot 
display,
+                * then for the other framebuffers. Only fail for the boot 
display;
+                * ignore errors for the rest.
+                */
+               for_each_node_by_type(node, "display") {
+                       if (!of_get_property(node, "linux,opened", NULL) ||
+                           !of_get_property(node, "linux,boot-display", NULL))
+                               continue;
+                       dev = of_platform_device_create(node, "of-display", 
NULL);
+                       if (WARN_ON(!dev))
+                               return -ENOMEM;
+                       boot_display = node;
+                       break;
+               }
+               for_each_node_by_type(node, "display") {
+                       if (!of_get_property(node, "linux,opened", NULL) || 
node == boot_display)
+                               continue;
+                       of_platform_device_create(node, "of-display", NULL);
+               }

-       of_platform_device_create(node, NULL, NULL);
-       of_node_put(node);
+       } else {
+               struct device_node *node;
+
+               /*
+                * Handle certain compatibles explicitly, since we don't want 
to create
+                * platform_devices for every node in /reserved-memory with a
+                * "compatible",
+                */
+               for_each_matching_node(node, reserved_mem_matches)
+                       of_platform_device_create(node, NULL, NULL);

-       of_platform_default_populate(NULL, NULL, NULL);
+               node = of_find_node_by_path("/firmware");
+               if (node) {
+                       of_platform_populate(node, NULL, NULL, NULL);
+                       of_node_put(node);
+               }
+
+               node = of_get_compatible_child(of_chosen, "simple-framebuffer");
+               of_platform_device_create(node, NULL, NULL);
+               of_node_put(node);

if (!of_have_populated_dt()) - /* - node = of_find_node_by_path("/firmware"); - node = of_get_compatible_child(of_chosen, "simple-framebuffer"); - /* Populate everything else. */

In v1, you supported "simple-framebuffer" on PPC. Don't we want to allow
that? Maybe no one cares ATM, but that could change. Either way:

Support for these framebuffers has always been mutually exclusive. The offb driver, which originally contained the code, depends on CONFIG_PPC. And PPC never supported simple-framebuffer anywhere.

Reviewed-by: Rob Herring <r...@kernel.org>

Thank you.

Best regards
Thomas

+
+               /* Populate everything else. */
+               of_platform_default_populate(NULL, NULL, NULL);
+       }

  }
@@ -558,7 +605,6 @@ static int __init of_platform_sync_state_init(void)
        return 0;
  }
  late_initcall_sync(of_platform_sync_state_init);
-#endif

  {

return 0; int of_platform_device_destroy(struct device *dev, void *data)

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

OpenPGP_signature
Description: OpenPGP digital signature