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

From: Krzysztof Kozlowski
Date: Mon Nov 25 2024 - 06:38:20 EST


On 25/11/2024 12:31, Krzysztof Kozlowski 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.
>
> Reported-by: Sasha Levin <sashal@xxxxxxxxxx>
> Closes: https://lore.kernel.org/all/Z0NUdoG17EwuCigT@sashalap/
> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>
> Cc: Matthias Brugger <matthias.bgg@xxxxxxxxx>
> Cc: Chen-Yu Tsai <wenst@xxxxxxxxxxxx>
> Cc: Conor Dooley <conor@xxxxxxxxxx>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> ---
> drivers/of/base.c | 26 ++++++++++++++++++++++++--
> 1 file changed, 24 insertions(+), 2 deletions(-)
>
> 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;
> +


This was not intentional, I'll fix it in v2.

Obviously this code is not really SMP aware, but even with store tearing
I don't think it will be issue. Worst case the
of_machine_compatible_match() will be called more than one, which is not
fatal and might not justify atomics or locks.

Best regards,
Krzysztof