On Tue, Jun 26, 2012 at 3:09 AM, Thierry Reding <thierry.reding@xxxxxxxxxxxxxxxxx> wrote: > 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. I are right. I will remove this change. It was a left-over from an older version. > [...] >> @@ -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. Thanks for the suggestion, looks cleaner. -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html