Re: [PATCH v2] spi/pl022: add devicetree support

[Thierry Reding <thierry.reding@avionic-design.de>]

[-- Attachment #1: Type: text/plain, Size: 2743 bytes --]

On Mon, Jun 25, 2012 at 06:06:28PM -0300, Alexandre Pereira da Silva wrote:
[...]
> diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
[...]
> @@ -1733,7 +1744,7 @@ static int calculate_effective_freq(struct pl022 *pl022, int freq, struct
>   * A piece of default chip info unless the platform
>   * supplies it.
>   */
> -static const struct pl022_config_chip pl022_default_chip_info = {
> +static struct pl022_config_chip pl022_default_chip_info = {
>  	.com_mode = POLLING_TRANSFER,
>  	.iface = SSP_INTERFACE_MOTOROLA_SPI,
>  	.hierarchy = SSP_SLAVE,

What's the reason for removing the const? You're only reading data from
the structure.

[...]
> @@ -1998,8 +2039,14 @@ pl022_probe(struct amba_device *adev, const struct amba_id *id)
>  		goto err_no_pdata;
>  	}
>  
> +	num_cs = platform_info->num_chipselect;
> +#ifdef CONFIG_OF
> +	of_property_read_u32(np, "pl022,num-chipselects", &num_cs);
> +#endif

This...

[...]
> @@ -2017,13 +2064,40 @@ pl022_probe(struct amba_device *adev, const struct amba_id *id)
>  	 * on this board
>  	 */
>  	master->bus_num = platform_info->bus_id;
> -	master->num_chipselect = platform_info->num_chipselect;
> +	master->num_chipselect = num_cs;
>  	master->cleanup = pl022_cleanup;
>  	master->setup = pl022_setup;
>  	master->prepare_transfer_hardware = pl022_prepare_transfer_hardware;
>  	master->transfer_one_message = pl022_transfer_one_message;
>  	master->unprepare_transfer_hardware = pl022_unprepare_transfer_hardware;
>  	master->rt = platform_info->rt;
> +	master->dev.of_node = dev->of_node;
> +
> +#ifdef CONFIG_OF
> +	for (i = 0; i < num_cs; i++) {
> +		int cs_gpio = of_get_named_gpio(np, "cs-gpios", i);
> +
> +		if (cs_gpio == -EPROBE_DEFER) {
> +			status = -EPROBE_DEFER;
> +			goto err_no_gpio;
> +		}
> +
> +		pl022->chipselect[i] = cs_gpio;
> +
> +		if (gpio_is_valid(cs_gpio)) {
> +			if (gpio_request(cs_gpio, "ssp-pl022"))
> +				dev_err(&adev->dev,
> +					"could not request %d gpio\n", cs_gpio);
> +			else if (gpio_direction_output(cs_gpio, 1))
> +				dev_err(&adev->dev,
> +					"could set gpio %d as output\n",
> +					cs_gpio);
> +		}
> +	}
> +#else
> +	for (i = 0; i < num_cs; i++)
> +		pl022->chipselect[i] = -EINVAL;
> +#endif

... and this would be good candidates for IS_ENABLED(CONFIG_OF).

>  
>  	/*
>  	 * Supports mode 0-3, loopback, and active low CS. Transfers are
> @@ -2130,6 +2204,9 @@ pl022_probe(struct amba_device *adev, const struct amba_id *id)
>   err_no_ioremap:
>  	amba_release_regions(adev);
>   err_no_ioregion:
> + #ifdef CONFIG_OF
> + err_no_gpio:
> + #endif

If you use the IS_ENABLED() macro, then you no longer need these either.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]