Re: [PATCH] of: Add Google Juniper to excluded default cells list

[Rob Herring <robh@xxxxxxxxxx>]

On Mon, Nov 25, 2024 at 09:46:10AM -0600, Rob Herring wrote:
> On Mon, Nov 25, 2024 at 5:32 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@xxxxxxxxxx> wrote:
> >
> > Google Juniper platforms have a very old bootloader which populates
> > /firmware node without proper address/size-cells leading to warnings:
> >
> >   Missing '#address-cells' in /firmware
> >   WARNING: CPU: 0 PID: 1 at drivers/of/base.c:106 of_bus_n_addr_cells+0x90/0xf0
> >   Modules linked in:
> >   CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.0 #1 933ab9971ff4d5dc58cb378a96f64c7f72e3454d
> >   Hardware name: Google juniper sku16 board (DT)
> >   ...
> >   Missing '#size-cells' in /firmware
> >   WARNING: CPU: 0 PID: 1 at drivers/of/base.c:133 of_bus_n_size_cells+0x90/0xf0
> >   Modules linked in:
> >   CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Tainted: G        W          6.12.0 #1 933ab9971ff4d5dc58cb378a96f64c7f72e3454d
> >   Tainted: [W]=WARN
> >   Hardware name: Google juniper sku16 board (DT)
> >
> > The platform won't receive updated bootloader/firmware so add it to
> > excluded platform list to silence the warning.
> 
> I'm interested to know what needs these in /firmware. /firmware is
> supposed to be for things without an MMIO interface. An alternative
> solution is to add the properties. That doesn't require
> CONFIG_OF_DYNAMIC and is often the approach powerpc uses.

Here's the fixup based approach I had in mind. The main possible issue I 
see is it doesn't run until of_core_init() which happens at driver core 
init. 

static void dup_parent_cells(struct device_node *np, const char *cellname)
{
	struct device_node __free(device_node) *parent = of_get_parent(np);
	struct property *prop;

	if (of_property_present(np, cellname))
		return;

	prop = of_find_property(parent, cellname, NULL);
	if (prop)
		prop = __of_prop_dup(prop, GFP_KERNEL);
	if (!prop)
		return;
	of_add_property(np, prop);
}

/*
 * Some Google Chromebooks have MMIO addresses in firmware nodes and fail to
 * populate /firmware node with #address-cells and #size-cells.
 */
static void fixup_firmware_cells(void)
{
	struct device_node __free(device_node) *np = of_find_node_by_path("/firmware");

	if (!np || !of_property_present(np, "ranges"))
		return;

	dup_parent_cells(np, "#size-cells");
	dup_parent_cells(np, "#address-cells");
}

void of_apply_fixups(void)
{
	fixup_firmware_cells();
}