| Message ID | 20241125113151.107812-1-krzysztof.kozlowski@linaro.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | of: Add Google Juniper to excluded default cells list | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | fail | build log |
On Mon, Nov 25, 2024 at 5:32 AM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 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. I'm also wondering if ranges is also missing? Rob
On Mon, Nov 25, 2024 at 11:46 PM Rob Herring <robh@kernel.org> wrote: > > On Mon, Nov 25, 2024 at 5:32 AM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> 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. Unfortunately this isn't the only platform affected. AFAIK basically any ARM-based Chromebook out there has the same issue. > 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. The regs point to a memory region that contains bits left by coreboot, the coreboot table and the CBMEM region. The latter encompasses the former, and also includes things such as the coreboot boot log. These are covered by the binding firmware/coreboot.txt and supported by drivers under drivers/firmware/google/. > I'm also wondering if ranges is also missing? It has an empty "ranges" property. I dug through our repository, and it looks like the code was added [1] back in the ARMv7 days for the Nyan (Tegra K1) series of Chromebooks. The code was never updated to use 64-bit addresses. ChenYu [1] https://crrev.com/c/203693
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@linaro.org> 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(); }
diff --git a/drivers/of/base.c b/drivers/of/base.c index a8b0c42bdc8e..13f0b2877ee0 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -56,6 +56,16 @@ DEFINE_MUTEX(of_mutex); */ DEFINE_RAW_SPINLOCK(devtree_lock); +/* + * List of machines running old firmware without explicit #address-cells and + * #size-cells values for parent nodes, which are most likely not going get any + * update. + */ +static const char * const excluded_default_cells_compats[] = { + "google,juniper", + NULL +}; + bool of_node_name_eq(const struct device_node *np, const char *name) { const char *node_name; @@ -91,6 +101,17 @@ static bool __of_node_is_type(const struct device_node *np, const char *type) IS_ENABLED(CONFIG_SPARC) \ ) +static bool excluded_default_cells_machines(void) +{ + /* Do not repeat the machine checks for every bus */ + static int excluded_machine = -1; + + if (excluded_machine < 0) + excluded_machine = of_machine_compatible_match(excluded_default_cells_compats); + + return !!excluded_machine; +} + int of_bus_n_addr_cells(struct device_node *np) { u32 cells; @@ -103,7 +124,7 @@ int of_bus_n_addr_cells(struct device_node *np) * is deprecated. Any platforms which hit this warning should * be added to the excluded list. */ - WARN_ONCE(!EXCLUDED_DEFAULT_CELLS_PLATFORMS, + WARN_ONCE(!EXCLUDED_DEFAULT_CELLS_PLATFORMS && !excluded_default_cells_machines(), "Missing '#address-cells' in %pOF\n", np); } return OF_ROOT_NODE_ADDR_CELLS_DEFAULT; @@ -125,12 +146,13 @@ int of_bus_n_size_cells(struct device_node *np) for (; np; np = np->parent) { if (!of_property_read_u32(np, "#size-cells", &cells)) return cells; + /* * Default root value and walking parent nodes for "#size-cells" * is deprecated. Any platforms which hit this warning should * be added to the excluded list. */ - WARN_ONCE(!EXCLUDED_DEFAULT_CELLS_PLATFORMS, + WARN_ONCE(!EXCLUDED_DEFAULT_CELLS_PLATFORMS && !excluded_default_cells_machines(), "Missing '#size-cells' in %pOF\n", np); } return OF_ROOT_NODE_SIZE_CELLS_DEFAULT;
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. Reported-by: Sasha Levin <sashal@kernel.org> Closes: https://lore.kernel.org/all/Z0NUdoG17EwuCigT@sashalap/ Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> Cc: Matthias Brugger <matthias.bgg@gmail.com> Cc: Chen-Yu Tsai <wenst@chromium.org> Cc: Conor Dooley <conor@kernel.org> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- drivers/of/base.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-)