Hello Mark, On 7 July 2014 13:02, Mark Brown <broonie@xxxxxxxxxx> wrote: > On Mon, Jul 07, 2014 at 11:51:38AM +0530, Naveen Krishna Ch wrote: >> On 2 July 2014 22:26, Mark Brown <broonie@xxxxxxxxxx> wrote: >> > On Fri, Jun 13, 2014 at 09:29:50AM +0530, Naveen Krishna Chatradhi wrote: > >> >> Hence, spi-s3c64xx.c is broken since "Jun 21 11:26:12 2013" and >> >> considering the time with no compliants about the breakage. > >> > I'm not clear what the breakage is? Some boards are broken but what's >> > the driver issue? > >> ToT was broken for few boards >> exynos4412-trats2.dts, exynos4210-smdkv310.dts and exynos5250-smdk5250.dts > >> With some DTS changes SPI works well, spi-s3c64xx.c driver had no issues. > > No, you're not answering my question - to repeat, what is the breakage? The Documentation/devicetree/bindings/spi/spi-samsung.txt describes "cs-gpio" as a controller specific property. The dts entries for SPI in exynos4412-trats2.dts, exynos4210-smdkv310.dts and exynos5250-smdk5250.dts boards have the "cs-gpio" property defined under controller-data node, which is inside the SPI device node &spi_1 { controller-data { cs-gpio = <>; }; }; But, _probe() of spi-s3c64xx.c driver looks for "cs-gpio" in the SPI device node and sets a flag "sdd->cs_gpio = false" (If the property is not available) &spi_1 { cs-gpio = <>; }; the sdd->cs_gpio flag is checked before actually getting the gpios from the controller-data node if (sdd->cs_gpio) cs->line = of_get_named_gpio(data_np, "cs-gpio", 0); Hence, SPI was failing on those boards. 1. As the SPI core and several drivers were changed to work with DT property "cs-gpios" (plural) defined under SPI node. 2. Since the commit 3146beec21b64f4551fcf0ac148381d54dc41b1b "spi: s3c64xx: Added provision for dedicated cs pin" Dated: Fri Jun 21 11:26:12 2013 +0530 For the above 2 reasons, It was decided to drop the backward compatibility of using "cs-gpio"(singular) in controller-data. Instead, start supporting "cs-gpios"(plural) in the SPI node. > >> > Also I'd need to check but are you sure that GPIO 0 is not valid? > >> gpio_is_valid() returns true for >> "number >= 0 && number < ARCH_NR_GPIOS" > > Right, so this means that any board that is using the internal chip > select with zero as default in their platform data is broken by this > change. using gpio_is_valid() to "sdd->cs_gpio" flag every where to check for the validity was a review comment. Which seems to fail for internal chip select with zero. I can submit another version with"sdd->cs_gpio" flag for this purpose. -- Thanks & Regards, (: Nav :) -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html