[Skiboot] [PATCH] flash: Move flash node under ibm,opal/flash/

[Cyril Bur]

On Mon,  1 Aug 2016 15:47:18 -0500
Jack Miller <jack at codezen.org> wrote:

> This changes the boot ABI, so it's only active for P9 and later systems,
> even though it's unrelated to hardware changes. There is an associated
> Linux change to properly search for this node as well.
> 
> This change properly specifies #address-cells for the flash node, so we
> can avoid DTC errors like
> 
> ---
> device tree: Warning (reg_format): "reg" property in /ibm,opal/flash at 0
> has invalid length (8 bytes) (#address-cells == 0, #size-cells == 0)
> ---
> 
> Base on a patch from Cyril Bur
> 
> Signed-off-by: Jack Miller <jack at codezen.org>
> ---
>  core/flash.c | 31 ++++++++++++++++++++++++-------
>  1 file changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/core/flash.c b/core/flash.c
> index e9c1f7d..0d3a160 100644
> --- a/core/flash.c
> +++ b/core/flash.c
> @@ -187,19 +187,36 @@ static int flash_nvram_probe(struct flash *flash, struct ffs_handle *ffs)
>  
>  static struct dt_node *flash_add_dt_node(struct flash *flash, int id)
>  {
> -	struct dt_node *flash_node;
> +	struct dt_node *flash_node, *flash_dir;
>  
> -	flash_node = dt_new_addr(opal_node, "flash", id);
> -	dt_add_property_strings(flash_node, "compatible", "ibm,opal-flash");
> +	/* Boot ABI on P9+, flash node is in ibm,opal/flash/flash at 0 */
> +
> +	if (proc_gen >= proc_gen_p9) {
> +		flash_dir = dt_new(opal_node, "flash");
> +		assert(flash_dir);
> +
> +		dt_add_property_cells(flash_dir, "#address-cells", 1);
> +		dt_add_property_cells(flash_dir, "#size-cells", 0);

Recently Mikey wrote quite a lot to get everything working 64bit, which made
this catch my eye. Don't we want #size-cells, 1 (or 2... ...) and
#address-cells, 0? What we're going to write in there is the size.

I think the fact that both "dt_add_property_cells()" and the powernv-flash
driver in linux ignore the #size-cells/#address-cells props means that it is
working but I dread to think what happens if we do respect them. On >32bit
flashes (mambo bogusdisk flashes) 2x32bit cells written in regs which when
interpreted as one 64bit number makes the correct size. 

Probably easiest to use 2 for #size-cells? Although then we'd need to ensure
dt_add_property_cells(); gets that right for flashes <32bit.

Looking more into dt_add_property_cells() using it to write >32bit values is
probably not the way to go... I feel like that "0," in the call is saving us.

Cyril

> +
> +		flash_node = dt_new_addr(flash_dir, "flash", id);
> +		assert(flash_node);
> +
> +	/* <= P8, flash is ibm,opal/flash at 0, with legacy cell settings */
> +
> +	} else {
> +		flash_node = dt_new_addr(opal_node, "flash", id);
> +		assert(flash_node);
> +
> +		dt_add_property_cells(flash_node, "#address-cells", 1);
> +		dt_add_property_cells(flash_node, "#size-cells", 1);
> +	}
> +
> +	dt_add_property_string(flash_node, "compatible", "ibm,opal-flash");
>  	dt_add_property_cells(flash_node, "ibm,opal-id", id);
>  	dt_add_property_cells(flash_node, "reg", 0, flash->size);
>  	dt_add_property_cells(flash_node, "ibm,flash-block-size",
>  			flash->block_size);
>  
> -	/* we fix to 32-bits */
> -	dt_add_property_cells(flash_node, "#address-cells", 1);
> -	dt_add_property_cells(flash_node, "#size-cells", 1);
> -
>  	return flash_node;
>  }
>