| Message ID | 1543806951-61848-4-git-send-email-f.fangjian@huawei.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | spi: dw-mmio: add ACPI support | expand |
On Mon, Dec 03, 2018 at 11:15:51AM +0800, Jay Fang wrote: > -static int of_spi_register_master(struct spi_controller *ctlr) > -{ > + if (IS_ENABLED(CONFIG_OF) && np) { > + for (i = 0; i < nb; i++) > + cs[i] = of_get_named_gpio(np, "cs-gpios", i); > + } else if (IS_ENABLED(CONFIG_ACPI) && ACPI_HANDLE(&ctlr->dev)) { > + for (i = 0; i < nb; i++) { > + desc = devm_gpiod_get_index(&ctlr->dev, "cs", > + i, GPIOD_ASIS); > + if (IS_ERR(desc)) > + continue; > + cs[i] = desc_to_gpio(desc); > + } > + } > return 0; Given that both properties are called "cs" won't devm_gpiod_get_index() do the right thing for DT systems as well as ACPI allowing us to move entirely to descriptors and remove the need for separate code paths here?
On Tue, Dec 4, 2018 at 5:33 PM Mark Brown <broonie@kernel.org> wrote: > On Mon, Dec 03, 2018 at 11:15:51AM +0800, Jay Fang wrote: > > > -static int of_spi_register_master(struct spi_controller *ctlr) > > -{ > > + if (IS_ENABLED(CONFIG_OF) && np) { > > + for (i = 0; i < nb; i++) > > + cs[i] = of_get_named_gpio(np, "cs-gpios", i); > > + } else if (IS_ENABLED(CONFIG_ACPI) && ACPI_HANDLE(&ctlr->dev)) { > > + for (i = 0; i < nb; i++) { > > + desc = devm_gpiod_get_index(&ctlr->dev, "cs", > > + i, GPIOD_ASIS); > > + if (IS_ERR(desc)) > > + continue; > > + cs[i] = desc_to_gpio(desc); > > + } > > + } > > return 0; > > Given that both properties are called "cs" won't devm_gpiod_get_index() > do the right thing for DT systems as well as ACPI allowing us to move > entirely to descriptors and remove the need for separate code paths here? Indeed moving the cs-gpios over to just using descriptors is on my TODO list and I have a rough patch cooking: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/commit/?h=gpio-descriptors-spi&id=963bff1e8feb020cac05325db84a37f81efdb0ea I think maybe I should expediate this patch[set] so there is a clean path forward for ACPI without having to do this: > + cs[i] = desc_to_gpio(desc); Which is essentially the waltz of two steps forward, one step back, *shudder* I'll try to get my patch into shape ASAP. Yours, Linus Walleij
Thank you Linus Walleij. And we will await your patchset. Please cc to linuxarm. Thank you, Jay On 2018/12/5 18:28, Linus Walleij wrote: > On Tue, Dec 4, 2018 at 5:33 PM Mark Brown <broonie@kernel.org> wrote: >> On Mon, Dec 03, 2018 at 11:15:51AM +0800, Jay Fang wrote: >> >>> -static int of_spi_register_master(struct spi_controller *ctlr) >>> -{ >>> + if (IS_ENABLED(CONFIG_OF) && np) { >>> + for (i = 0; i < nb; i++) >>> + cs[i] = of_get_named_gpio(np, "cs-gpios", i); >>> + } else if (IS_ENABLED(CONFIG_ACPI) && ACPI_HANDLE(&ctlr->dev)) { >>> + for (i = 0; i < nb; i++) { >>> + desc = devm_gpiod_get_index(&ctlr->dev, "cs", >>> + i, GPIOD_ASIS); >>> + if (IS_ERR(desc)) >>> + continue; >>> + cs[i] = desc_to_gpio(desc); >>> + } >>> + } >>> return 0; >> >> Given that both properties are called "cs" won't devm_gpiod_get_index() >> do the right thing for DT systems as well as ACPI allowing us to move >> entirely to descriptors and remove the need for separate code paths here? > > Indeed moving the cs-gpios over to just using descriptors is on my > TODO list and I have a rough patch cooking: > https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/commit/?h=gpio-descriptors-spi&id=963bff1e8feb020cac05325db84a37f81efdb0ea > > I think maybe I should expediate this patch[set] so there is a clean > path forward for ACPI without having to do this: > >> + cs[i] = desc_to_gpio(desc); > > Which is essentially the waltz of two steps forward, one step back, > *shudder* > > I'll try to get my patch into shape ASAP. > > Yours, > Linus Walleij > > . >
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 6ca5940..81d404a 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -2075,16 +2075,13 @@ struct spi_controller *__spi_alloc_controller(struct device *dev, } EXPORT_SYMBOL_GPL(__spi_alloc_controller); -#ifdef CONFIG_OF -static int of_spi_register_master(struct spi_controller *ctlr) +static int __spi_register_controller(struct spi_controller *ctlr) { int nb, i, *cs; struct device_node *np = ctlr->dev.of_node; + struct gpio_desc *desc; - if (!np) - return 0; - - nb = of_gpio_named_count(np, "cs-gpios"); + nb = gpiod_count(&ctlr->dev, "cs"); ctlr->num_chipselect = max_t(int, nb, ctlr->num_chipselect); /* Return error only for an incorrectly formed cs-gpios property */ @@ -2103,17 +2100,20 @@ static int of_spi_register_master(struct spi_controller *ctlr) for (i = 0; i < ctlr->num_chipselect; i++) cs[i] = -ENOENT; - for (i = 0; i < nb; i++) - cs[i] = of_get_named_gpio(np, "cs-gpios", i); - - return 0; -} -#else -static int of_spi_register_master(struct spi_controller *ctlr) -{ + if (IS_ENABLED(CONFIG_OF) && np) { + for (i = 0; i < nb; i++) + cs[i] = of_get_named_gpio(np, "cs-gpios", i); + } else if (IS_ENABLED(CONFIG_ACPI) && ACPI_HANDLE(&ctlr->dev)) { + for (i = 0; i < nb; i++) { + desc = devm_gpiod_get_index(&ctlr->dev, "cs", + i, GPIOD_ASIS); + if (IS_ERR(desc)) + continue; + cs[i] = desc_to_gpio(desc); + } + } return 0; } -#endif static int spi_controller_check_ops(struct spi_controller *ctlr) { @@ -2177,7 +2177,7 @@ int spi_register_controller(struct spi_controller *ctlr) return status; if (!spi_controller_is_slave(ctlr)) { - status = of_spi_register_master(ctlr); + status = __spi_register_controller(ctlr); if (status) return status; }
This will also allow to use cs-gpios for chip select in ACPI code with no modification in the driver binding, like it be used in DT. We could share almost all of the code with the DT path. Signed-off-by: Jay Fang <f.fangjian@huawei.com> --- drivers/spi/spi.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-)