devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tomasz Figa <tomasz.figa@gmail.com>
To: Doug Anderson <dianders@chromium.org>,
	Naveen Krishna Ch <naveenkrishna.ch@gmail.com>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>,
	Naveen Krishna Chatradhi <ch.naveen@samsung.com>,
	"linux-samsung-soc@vger.kernel.org"
	<linux-samsung-soc@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	spi-devel-general@lists.sourceforge.net,
	"broonie@kernel.org" <broonie@kernel.org>,
	Grant Likely <grant.likely@secretlab.ca>,
	Jaswinder Singh <jaswinder.singh@linaro.org>,
	Kukjin Kim <kgene.kim@samsung.com>, "cpgs ." <cpgs@samsung.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Subject: Re: [PATCH 1/2 v2] spi: s3c64xx: use "cs-gpios" from spi node instead of "cs-gpio"
Date: Tue, 10 Jun 2014 21:43:59 +0200	[thread overview]
Message-ID: <53975FFF.7010209@gmail.com> (raw)
In-Reply-To: <CAD=FV=XtdgKEg7fTHe+tsFM1Y_YwqNNk_DBW7ZX83YERGJsHBg@mail.gmail.com>

On 10.06.2014 20:09, Doug Anderson wrote:
> Naveen / Sylwester,
> 
> On Tue, Jun 10, 2014 at 4:00 AM, Naveen Krishna Ch
> <naveenkrishna.ch@gmail.com> wrote:
>>> Can we support both "cs-gpio" and "cs-gpios" for backward compatibility ?
>>> After your change all DTBs using the original pattern will not work with
>>> new kernels any more. At least I would expect such backward compatibility
>>> maintained for few kernel releases.
>>
>> The reason behind removing the "cs-gpio" or not providing backward
>> compatibility was
>>
>> 1. Since spi-core started using "cs-gpios" string from spi device node
>> several months ago.
>>   The spi-s3c64xx.c driver is partially broken for more than 6 months.
>>
>> 2. Supporting "cs-gpio" would add extra bit of code.
>>
>> I've corrected the dts files that were using "cs-gpio" under
>> "controller-data"(child node)
>> to use "cs-gpio" from spi device node (parent node).
>>
>> I will make another version if you insist.
> 
> Yup, as near as I can tell this has been broken since (3146bee spi:
> s3c64xx: Added provision for dedicated cs pin).  That landed June 25th
> of 2013 into the SPI tree.
> 
> ...so clearly nobody was really testing Samsung's SPI driver on ToT
> Linux.  1 year of unnoticed brokenness seems like enough time that we
> could do away with the old code.  If someone comes out of the woodwork
> and claims a dire need to support the old binding then it can be added
> then.
> 
> In-tree users of this appear to be:
> 
> arch/arm/boot/dts/exynos4210-smdkv310.dts:
>  cs-gpio = <&gpc1 2 0>;
> arch/arm/boot/dts/exynos4412-trats2.dts:
>  cs-gpio = <&gpb 5 0>;
> arch/arm/boot/dts/exynos5250-smdk5250.dts:
>  cs-gpio = <&gpa2 5 0>;
> 
> ...and I guess nobody has bothered using the SPI flash on those boards
> for the last year?

On Trats2, it's actually a camera sensor, not SPI flash...

Still, that's really a shame that:

a) such patch went in (i.e. its brokenness was not spotted in review),
b) the regression was not spotted.

Anyway, IMHO this justifies removing the old binding then.

Best regards,
Tomasz

  reply	other threads:[~2014-06-10 19:43 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-10 10:07 [PATCH 0/2 v2] spi: s3c64xx: use "cs-gpios" in spi node instead of "cs-gpio" Naveen Krishna Chatradhi
2014-06-10 10:08 ` [PATCH 1/2 v2] spi: s3c64xx: use "cs-gpios" from " Naveen Krishna Chatradhi
2014-06-10 10:39   ` Sylwester Nawrocki
2014-06-10 11:00     ` Naveen Krishna Ch
2014-06-10 18:09       ` Doug Anderson
2014-06-10 19:43         ` Tomasz Figa [this message]
2014-06-10 20:32         ` Rob Herring
2014-06-11 12:22           ` Sylwester Nawrocki
2014-06-10 18:26   ` Doug Anderson
2014-06-10 19:25     ` Tomasz Figa
     [not found]     ` <CAD=FV=WgvyF-P+69SdmCGX0jxJ3FVxpa+AcgQKg-Emya_W-w3Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-06-11  6:10       ` Naveen Krishna Ch
2014-06-10 19:49   ` Tomasz Figa
2014-06-10 19:58     ` Doug Anderson
2014-06-10 19:59       ` Tomasz Figa
2014-06-10 20:21         ` Doug Anderson
2014-06-11  6:16     ` Naveen Krishna Ch
2014-06-10 10:08 ` [PATCH 2/2 v2] ARM: DTS: move "cs-gpio" from "controller-data" to under spi node Naveen Krishna Chatradhi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53975FFF.7010209@gmail.com \
    --to=tomasz.figa@gmail.com \
    --cc=broonie@kernel.org \
    --cc=ch.naveen@samsung.com \
    --cc=cpgs@samsung.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dianders@chromium.org \
    --cc=grant.likely@secretlab.ca \
    --cc=jaswinder.singh@linaro.org \
    --cc=javier.martinez@collabora.co.uk \
    --cc=kgene.kim@samsung.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=naveenkrishna.ch@gmail.com \
    --cc=s.nawrocki@samsung.com \
    --cc=spi-devel-general@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).