Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

spi: s3c64xx: use the generic SPI "cs-gpios" property

The s3c64xx SPI driver uses a custom DT binding to specify
the GPIO used to drive the chip select (CS) line instead of
using the generic "cs-gpios" property already defined in:
Documentation/devicetree/bindings/spi/spi-bus.txt.

It's unfortunate that drivers are not using standard bindings
and creating custom ones instead but in most cases this can't
be changed without breaking Device Tree backward compatibility.

But in the case of this driver, its DT binding has been broken
for more than a year. Since after commit (dated June, 21 2013):

3146bee ("spi: s3c64xx: Added provision for dedicated cs pin")

DT backward compatibility was broken and nobody noticed until
now when the commit was reverted. So it seems to be safe to
change the binding to use the standard SPI "cs-gpios" property
instead of using a custom one just for this driver.

This patch also allows boards that don't use a GPIO pin for the
CS to work with the driver since the SPI core will take care of
setting spi->cs_gpio to -ENOENT if a board wants to use the built
in CS instead of a GPIO as explained in the SPI bus DT binding:
Documentation/devicetree/bindings/spi/spi-bus.txt.

For non-DT platforms, spi->cs_gpio will be set to -ENOENT as well
unless they specify a GPIO pin in their platform data. So both
native and GPIO chip select is also supported for legacy boards.

The above use case was what motivated commit 3146bee which broke
the DT binding backward compatibility in the first place.

Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
[javier.martinez@collabora.co.uk: split changes and improve commit message]
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Signed-off-by: Mark Brown <broonie@linaro.org>

authored by

Naveen Krishna Chatradhi and committed by
Mark Brown
306972ce e2689b94

+29 -20
+29 -20
drivers/spi/spi-s3c64xx.c
··· 773 773 return ERR_PTR(-ENOMEM); 774 774 } 775 775 776 - cs->line = of_get_named_gpio(data_np, "cs-gpio", 0); 777 - if (!gpio_is_valid(cs->line)) { 778 - dev_err(&spi->dev, "chip select gpio is not specified or invalid\n"); 779 - kfree(cs); 780 - of_node_put(data_np); 781 - return ERR_PTR(-EINVAL); 782 - } 783 - 784 776 of_property_read_u32(data_np, "samsung,spi-feedback-delay", &fb_delay); 785 777 cs->fb_delay = fb_delay; 786 778 of_node_put(data_np); ··· 793 801 int err; 794 802 795 803 sdd = spi_master_get_devdata(spi->master); 796 - if (!cs && spi->dev.of_node) { 804 + if (spi->dev.of_node) { 797 805 cs = s3c64xx_get_slave_ctrldata(spi); 798 806 spi->controller_data = cs; 807 + } else if (cs) { 808 + /* On non-DT platforms the SPI core will set spi->cs_gpio 809 + * to -ENOENT. The GPIO pin used to drive the chip select 810 + * is defined by using platform data so spi->cs_gpio value 811 + * has to be override to have the proper GPIO pin number. 812 + */ 813 + spi->cs_gpio = cs->line; 799 814 } 800 815 801 816 if (IS_ERR_OR_NULL(cs)) { ··· 811 812 } 812 813 813 814 if (!spi_get_ctldata(spi)) { 814 - err = gpio_request_one(cs->line, GPIOF_OUT_INIT_HIGH, 815 - dev_name(&spi->dev)); 816 - if (err) { 817 - dev_err(&spi->dev, 818 - "Failed to get /CS gpio [%d]: %d\n", 819 - cs->line, err); 820 - goto err_gpio_req; 815 + if (gpio_is_valid(spi->cs_gpio)) { 816 + err = gpio_request_one(spi->cs_gpio, GPIOF_OUT_INIT_HIGH, 817 + dev_name(&spi->dev)); 818 + if (err) { 819 + dev_err(&spi->dev, 820 + "Failed to get /CS gpio [%d]: %d\n", 821 + spi->cs_gpio, err); 822 + goto err_gpio_req; 823 + } 821 824 } 822 - 823 - spi->cs_gpio = cs->line; 824 825 825 826 spi_set_ctldata(spi, cs); 826 827 } ··· 874 875 /* setup() returns with device de-selected */ 875 876 writel(S3C64XX_SPI_SLAVE_SIG_INACT, sdd->regs + S3C64XX_SPI_SLAVE_SEL); 876 877 877 - gpio_free(cs->line); 878 + if (gpio_is_valid(spi->cs_gpio)) 879 + gpio_free(spi->cs_gpio); 878 880 spi_set_ctldata(spi, NULL); 879 881 880 882 err_gpio_req: ··· 889 889 { 890 890 struct s3c64xx_spi_csinfo *cs = spi_get_ctldata(spi); 891 891 892 - if (cs) { 892 + if (gpio_is_valid(spi->cs_gpio)) { 893 893 gpio_free(spi->cs_gpio); 894 894 if (spi->dev.of_node) 895 895 kfree(cs); 896 + else { 897 + /* On non-DT platforms, the SPI core sets 898 + * spi->cs_gpio to -ENOENT and .setup() 899 + * overrides it with the GPIO pin value 900 + * passed using platform data. 901 + */ 902 + spi->cs_gpio = -ENOENT; 903 + } 896 904 } 905 + 897 906 spi_set_ctldata(spi, NULL); 898 907 } 899 908