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 devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
