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[] = {
>>   
>>   static int __init of_platform_default_populate_init(void)
>>   {
>> -	struct device_node *node;
>> -
> 
> As both if/else clauses need 'node', I'd keep this declared here.

Ok.

> 
>>   	device_links_supplier_sync_state_pause();
>>   
>>   	if (!of_have_populated_dt())
>>   		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;
>> +			}
>> +		}
>>   
>> -	node = of_find_node_by_path("/firmware");
>> -	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);
>> +		}
>>   
>> -	node = of_get_compatible_child(of_chosen, "simple-framebuffer");
>> -	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);
>>   
>> -	/* Populate everything else. */
>> -	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);
> 
> 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 <robh@kernel.org>

Thank you.

Best regards
Thomas

> 
> 
>> +
>> +		/* Populate everything else. */
>> +		of_platform_default_populate(NULL, NULL, NULL);
>> +	}
>>   
>>   	return 0;
>>   }
>> @@ -558,7 +605,6 @@ static int __init of_platform_sync_state_init(void)
>>   	return 0;
>>   }
>>   late_initcall_sync(of_platform_sync_state_init);
>> -#endif
>>   
>>   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