linux-kernel - Re: [PATCH linux v2 2/3] spi: aspeed: Handle custom decoding ranges

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20221017185730.GB2264550-robh@kernel.org>
Date:   Mon, 17 Oct 2022 13:57:30 -0500
From:   Rob Herring <robh@...nel.org>
To:     Cédric Le Goater <clg@...d.org>
Cc:     linux-spi@...r.kernel.org, Mark Brown <broonie@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        devicetree@...r.kernel.org, linux-aspeed@...ts.ozlabs.org,
        openbmc@...ts.ozlabs.org, Joel Stanley <joel@....id.au>,
        Andrew Jeffery <andrew@...id.au>,
        Chin-Ting Kuo <chin-ting_kuo@...eedtech.com>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Naresh Solanki <naresh.solanki@...ements.com>
Subject: Re: [PATCH linux v2 2/3] spi: aspeed: Handle custom decoding ranges

On Mon, Oct 17, 2022 at 11:16:23AM +0200, Cédric Le Goater wrote:
> The "ranges" property predefines settings for the decoding ranges of
> each CS. If found in the DT, the driver applies the settings at probe
> time. The default behavior is to set the decoding range of each CS
> using the flash device size when the spi slave is setup.
> 
> Cc: Naresh Solanki <naresh.solanki@...ements.com>
> Cc: Chin-Ting Kuo <chin-ting_kuo@...eedtech.com>
> Signed-off-by: Cédric Le Goater <clg@...d.org>
> ---
>  drivers/spi/spi-aspeed-smc.c | 65 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 64 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-aspeed-smc.c b/drivers/spi/spi-aspeed-smc.c
> index b90571396a60..75e1d08bbd00 100644
> --- a/drivers/spi/spi-aspeed-smc.c
> +++ b/drivers/spi/spi-aspeed-smc.c
> @@ -96,6 +96,7 @@ struct aspeed_spi {
>  	u32			 ahb_base_phy;
>  	u32			 ahb_window_size;
>  	struct device		*dev;
> +	bool                     fixed_windows;
>  
>  	struct clk		*clk;
>  	u32			 clk_freq;
> @@ -382,6 +383,7 @@ static const char *aspeed_spi_get_name(struct spi_mem *mem)
>  
>  struct aspeed_spi_window {
>  	u32 cs;
> +	u32 reg;
>  	u32 offset;
>  	u32 size;
>  };
> @@ -396,6 +398,7 @@ static void aspeed_spi_get_windows(struct aspeed_spi *aspi,
>  	for (cs = 0; cs < aspi->data->max_cs; cs++) {
>  		reg_val = readl(aspi->regs + CE0_SEGMENT_ADDR_REG + cs * 4);
>  		windows[cs].cs = cs;
> +		windows[cs].reg = reg_val;
>  		windows[cs].size = data->segment_end(aspi, reg_val) -
>  			data->segment_start(aspi, reg_val);
>  		windows[cs].offset = data->segment_start(aspi, reg_val) - aspi->ahb_base_phy;
> @@ -572,7 +575,8 @@ static int aspeed_spi_dirmap_create(struct spi_mem_dirmap_desc *desc)
>  	if (op->data.dir != SPI_MEM_DATA_IN)
>  		return -EOPNOTSUPP;
>  
> -	aspeed_spi_chip_adjust_window(chip, desc->info.offset, desc->info.length);
> +	if (!aspi->fixed_windows)
> +		aspeed_spi_chip_adjust_window(chip, desc->info.offset, desc->info.length);
>  
>  	if (desc->info.length > chip->ahb_window_size)
>  		dev_warn(aspi->dev, "CE%d window (%dMB) too small for mapping",
> @@ -712,6 +716,61 @@ static void aspeed_spi_enable(struct aspeed_spi *aspi, bool enable)
>  		aspeed_spi_chip_enable(aspi, cs, enable);
>  }
>  
> +static int aspeed_spi_chip_read_ranges(struct device_node *node, struct aspeed_spi *aspi)
> +{
> +	const char *range_prop = "ranges";
> +	struct property *prop;
> +	struct aspeed_spi_window ranges[ASPEED_SPI_MAX_NUM_CS];
> +	int prop_size;
> +	int count;
> +	int ret;
> +	int i;
> +
> +	prop = of_find_property(node, range_prop, &prop_size);
> +	if (!prop)
> +		return 0;

Parsing common properties yourself is generally a bad sign.

> +
> +	count = prop_size / sizeof(*ranges);
> +	if (count > aspi->data->max_cs) {
> +		dev_err(aspi->dev, "invalid '%s' property %d\n", range_prop, count);
> +		return -EINVAL;
> +	}
> +
> +	if (count < aspi->data->max_cs)
> +		dev_dbg(aspi->dev, "'%s' property does not cover all CE\n",
> +			range_prop);
> +
> +	ret = of_property_read_u32_array(node, range_prop, (u32 *)ranges, count * 4);

You've just gotten it horribly wrong because you ignored #size-cells and 
#address-cells.

Rob