[PATCH 1/3 v5] spi: s3c64xx: fix broken "cs_gpios" usage in the driver

Hello Mark,

On 7 July 2014 13:02, Mark Brown [off-list ref] wrote:

On Mon, Jul 07, 2014 at 11:51:38AM +0530, Naveen Krishna Ch wrote:

quoted

On 2 July 2014 22:26, Mark Brown [off-list ref] wrote:

quoted

On Fri, Jun 13, 2014 at 09:29:50AM +0530, Naveen Krishna Chatradhi wrote:

quoted

quoted

quoted

Hence, spi-s3c64xx.c is broken since "Jun 21 11:26:12 2013" and
considering the time with no compliants about the breakage.

quoted

quoted

I'm not clear what the breakage is?  Some boards are broken but what's
the driver issue?

quoted

ToT was broken for few boards
exynos4412-trats2.dts, exynos4210-smdkv310.dts and exynos5250-smdk5250.dts

quoted

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.

quoted

quoted

Also I'd need to check but are you sure that GPIO 0 is not valid?

quoted

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 :)