Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1302239 > unrolled thread

Re: [PATCH v3 1/3] spi: atmel: add support for the internal chip-select of the spi controller

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v3 1/3] spi: atmel: add support for the internal chip-select of the spi controller Måns Rullgård <mans@mansr.com> - 2016-01-05 23:00 +0100
    Re: [PATCH v3 1/3] spi: atmel: add support for the internal  chip-select of the spi controller Mark Brown <broonie@kernel.org> - 2016-01-07 16:50 +0100
      Re: [PATCH v3 1/3] spi: atmel: add support for the internal chip-select of the spi controller Måns Rullgård <mans@mansr.com> - 2016-01-07 17:20 +0100

#1302239 — Re: [PATCH v3 1/3] spi: atmel: add support for the internal chip-select of the spi controller

Cyrille Pitchen <cyrille.pitchen@atmel.com> writes:

> This patch relies on the CSAAT (Chip Select Active After Transfer) feature
> introduced by the version 2 of the spi controller. This new mode allows to
> use properly the internal chip-select output pin of the spi controller
> instead of using external gpios. Consequently, the "cs-gpios" device-tree
> property becomes optional.
>
> When the new CSAAT bit is set into the Chip Select Register, the internal
> chip-select output pin remains asserted till both the following conditions
> become true:
> - the LASTXFER bit is set into the Control Register (or the Transmit Data
>   Register)
> - the Transmit Data Register and its shift register are empty.
>
> WARNING: if the LASTXFER bit is set into the Control Register then new
> data are written into the Transmit Data Register fast enough to keep its
> shifter not empty, the chip-select output pin remains asserted. Only when
> the shifter becomes empty, the chip-select output pin is unasserted.
>
> When the CSAAT bit is clear in the Chip Select Register, the LASTXFER bit
> is ignored in both the Control Register and the Transmit Data Register.
> The internal chip-select output pin remains active as long as the Transmit
> Data Register or its shift register are not empty.
>
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> ---
>  drivers/spi/spi-atmel.c | 37 ++++++++++++++++++++++++++++---------
>  1 file changed, 28 insertions(+), 9 deletions(-)

[...]

> @@ -1338,6 +1350,13 @@ static int atmel_spi_probe(struct platform_device *pdev)
>
>  	atmel_get_caps(as);
>
> +	as->use_cs_gpios = true;
> +	if (atmel_spi_is_v2(as) &&
> +	    !of_get_property(pdev->dev.of_node, "cs-gpios", NULL)) {
> +		as->use_cs_gpios = false;
> +		master->num_chipselect = 4;
> +	}

This part breaks the AVR32 boards and probably anything else that
doesn't use devicetree but does use GPIOs for chip select.

-- 
Måns Rullgård
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1303682 — Re: [PATCH v3 1/3] spi: atmel: add support for the internal chip-select of the spi controller

[Multipart message — attachments visible in raw view] — view raw

On Tue, Jan 05, 2016 at 09:50:54PM +0000, Måns Rullgård wrote:
> Cyrille Pitchen <cyrille.pitchen@atmel.com> writes:

> > +	as->use_cs_gpios = true;
> > +	if (atmel_spi_is_v2(as) &&
> > +	    !of_get_property(pdev->dev.of_node, "cs-gpios", NULL)) {
> > +		as->use_cs_gpios = false;
> > +		master->num_chipselect = 4;
> > +	}

> This part breaks the AVR32 boards and probably anything else that
> doesn't use devicetree but does use GPIOs for chip select.

Shouldn't this just be setting defaults for the case where nothing is
provided?

[toc] | [prev] | [next] | [standalone]

#1303712

Mark Brown <broonie@kernel.org> writes:

> On Tue, Jan 05, 2016 at 09:50:54PM +0000, Måns Rullgård wrote:
>> Cyrille Pitchen <cyrille.pitchen@atmel.com> writes:
>
>> > +	as->use_cs_gpios = true;
>> > +	if (atmel_spi_is_v2(as) &&
>> > +	    !of_get_property(pdev->dev.of_node, "cs-gpios", NULL)) {
>> > +		as->use_cs_gpios = false;
>> > +		master->num_chipselect = 4;
>> > +	}
>
>> This part breaks the AVR32 boards and probably anything else that
>> doesn't use devicetree but does use GPIOs for chip select.
>
> Shouldn't this just be setting defaults for the case where nothing is
> provided?

Traditionally the platform data included a GPIO number to use and that
was that.  At some point DT support was added wherein the platform data
is overridden by cs_gpio from struct spi_device if this is valid (it is
set to a negative value by default).  Thus far all was well.  Then came
this patch.  It assumes that everybody uses DT and treats a missing
cs-gpios property as indication that the controller's own pins should be
used.  It also assumes that if any device uses GPIO for CS all of them
do, even though the SPI core driver might provide a partial list
(probably since many boards don't use all the available chip selects,
but it doesn't prevent someone abusing this).

To work correctly in call cases, this driver should use, for each
peripheral, the following priority:

- spi->cs_gpio (filled from DT or -ENOENT)
- GPIO from platform data
- controller chip select pin

The trouble is that there is no way to reliably tell a valid GPIO number
of zero in the platform data from an unset value.  In practice, I
believe existing old boards using this driver all use a non-zero GPIO
(the AVR32 platform code requires this), so checking for a non-zero
number is probably sufficient.  I'll cook up a patch for this unless
someone objects.

-- 
Måns Rullgård
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web