Re: [PATCH v1] spi: fix client driver breakages when using GPIO descriptors

From: Sven Van Asbroeck <thesven73@gmail.com>
To: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Mark Brown <broonie@kernel.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Jonathan Cameron <jonathan.cameron@huawei.com>,
	Simon Han <z.han@kunbus.com>, Lukas Wunner <lukas@wunner.de>,
	linux-spi <linux-spi@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v1] spi: fix client driver breakages when using GPIO descriptors
Date: Mon, 9 Nov 2020 09:41:18 -0500	[thread overview]
Message-ID: <CAGngYiVu3cXtzb5PaoDOoyqjuuohLQ+em6Keg-qgDFFn2tdp=Q@mail.gmail.com> (raw)
In-Reply-To: <CAHp75VfP1R7bXV6nWWnovWB5BMFcNNEmwBQXheBCUVDbr=xXGA@mail.gmail.com>

Hi Andy, thank you for looking at this patch !

On Mon, Nov 9, 2020 at 9:24 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> Sounds like "many SPI drivers have to be fixed".
>

I don't disagree. Fact is that after the imx cspi bus driver was converted
to gpio descriptors, most spi client drivers broke. It would be great if this
could be fixed. Any method that the community can find a consensus on,
would be great :)

One the one hand: the fact that many spi client drivers just overwrite
flags and values in their parent bus structure, doesn't sound idiomatic.
I guess those spi->... values should really be opaque, and we should
be using accessor functions, eg.:

    static int acme_probe(struct spi_device *spi)
    {
        ...
        // won't touch SPI_CS_HIGH flag
        spi_set_mode_clock(spi, SPI_MODE_0);
        ...
    }

On the other hand, it sounds very confusing to set SPI_CS_HIGH on
all spi buses that use gpio descriptors: especially because gpiolib
already handles absolutely everything related to polarity. And the
SPI_CS_HIGH flag gets set even for chip-selects that are active-low.