All of lore.kernel.org
 help / color / mirror / Atom feed
From: Martin Sperl <martin@sperl.org>
To: Stephen Warren <swarren@wwwdotorg.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
	devicetree@vger.kernel.org,
	Florian Fainelli <f.fainelli@gmail.com>,
	Pawel Moll <pawel.moll@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Linus Walleij <linus.walleij@linaro.org>,
	linux-kernel@vger.kernel.org, Eric Anholt <eric@anholt.net>,
	Rob Herring <robh+dt@kernel.org>,
	linux-rpi-kernel@lists.infradead.org, linux-gpio@vger.kernel.org,
	Kumar Gala <galak@codeaurora.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/5] ARM: bcm2835: Define standard pinctrl groups in the gpio node.
Date: Fri, 4 Mar 2016 10:27:49 +0100	[thread overview]
Message-ID: <6D770203-ED53-413D-AC28-B89A5C48DA8C@sperl.org> (raw)
In-Reply-To: <56D8AAA2.60907@wwwdotorg.org>


> On 03.03.2016, at 22:20, Stephen Warren <swarren@wwwdotorg.org> wrote:
> 
> On 02/26/2016 11:19 AM, Eric Anholt wrote:
>> The BCM2835-ARM-Peripherals.pdf documentation specifies what the
>> function selects do for the pins, and there are a bunch of obvious
>> groupings to be made.  With these created, we'll be able to replace
>> bcm2835-rpi.dtsi's main "set all of these pins to alt0" with
>> references to specific groups we want enabled.
> 
>> diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
> 
>> +			spi0_gpio7: spi0_gpio7 {
>> +				brcm,pins = <7 8 9 10 11>;
>> +				brcm,function = <BCM2835_FSEL_ALT0>;
>> +			};
> 
> This is too many pins.
> 
> - It includes both MOSI and MISO, although a particular use-case may only use 1 of those.
> 
> - It includes both chip-select signals, whereas a particular use-case may use 0, 1, or 2 of those. This is especially true since IIRC the mainline bcm283x SPI driver wants to only use GPIOs for chip-selects, not SPI-controller-generated chip-select signals, to avoid some issues with the HW generation of these signals.
That is true: the spi-bcm2835 driver requires GPIO usage for chip-select
to make all those latency optimizations work (but also to avoid some
spi-dma issues).
The reason behind it is that there are observed short term “glitches”
on native CS whenever the SPI control register is touched - even with 
identical values.
And GPIO controlled CS solves this issue (and Mark Brown said that
the GPIO-cs interface is now preferred anyway - hence the auxiliary
spi only implement gpio-cs and requires the CS set as OUTPUT, but
unlike the main spi this does not have “remapping” support for
legacy device-trees (as there never was a driver-version that supported
native-cs).

Maybe split the SPI-portion into 2 sections:
* the SCK, MOSI, MISO (pin 9 to 11) with ALT_0
* the CS GPIOs (standard pins are 7 and 8) with OUTPUT.

That way it is easy to override only this section (plus the gpio-cs property inside the spi node) to extend the number of chip selects or use different mappings.

> 
> I believe a similar comment applies to other SPI nodes too.
I guess the same “splitting” approach should be taken here as well...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Martin Sperl <martin@sperl.org>
To: Stephen Warren <swarren@wwwdotorg.org>
Cc: Eric Anholt <eric@anholt.net>,
	Mark Rutland <mark.rutland@arm.com>,
	devicetree@vger.kernel.org,
	Florian Fainelli <f.fainelli@gmail.com>,
	Pawel Moll <pawel.moll@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Linus Walleij <linus.walleij@linaro.org>,
	linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org,
	Rob Herring <robh+dt@kernel.org>,
	linux-rpi-kernel@lists.infradead.org,
	Kumar Gala <galak@codeaurora.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/5] ARM: bcm2835: Define standard pinctrl groups in the gpio node.
Date: Fri, 4 Mar 2016 10:27:49 +0100	[thread overview]
Message-ID: <6D770203-ED53-413D-AC28-B89A5C48DA8C@sperl.org> (raw)
In-Reply-To: <56D8AAA2.60907@wwwdotorg.org>


> On 03.03.2016, at 22:20, Stephen Warren <swarren@wwwdotorg.org> wrote:
> 
> On 02/26/2016 11:19 AM, Eric Anholt wrote:
>> The BCM2835-ARM-Peripherals.pdf documentation specifies what the
>> function selects do for the pins, and there are a bunch of obvious
>> groupings to be made.  With these created, we'll be able to replace
>> bcm2835-rpi.dtsi's main "set all of these pins to alt0" with
>> references to specific groups we want enabled.
> 
>> diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
> 
>> +			spi0_gpio7: spi0_gpio7 {
>> +				brcm,pins = <7 8 9 10 11>;
>> +				brcm,function = <BCM2835_FSEL_ALT0>;
>> +			};
> 
> This is too many pins.
> 
> - It includes both MOSI and MISO, although a particular use-case may only use 1 of those.
> 
> - It includes both chip-select signals, whereas a particular use-case may use 0, 1, or 2 of those. This is especially true since IIRC the mainline bcm283x SPI driver wants to only use GPIOs for chip-selects, not SPI-controller-generated chip-select signals, to avoid some issues with the HW generation of these signals.
That is true: the spi-bcm2835 driver requires GPIO usage for chip-select
to make all those latency optimizations work (but also to avoid some
spi-dma issues).
The reason behind it is that there are observed short term “glitches”
on native CS whenever the SPI control register is touched - even with 
identical values.
And GPIO controlled CS solves this issue (and Mark Brown said that
the GPIO-cs interface is now preferred anyway - hence the auxiliary
spi only implement gpio-cs and requires the CS set as OUTPUT, but
unlike the main spi this does not have “remapping” support for
legacy device-trees (as there never was a driver-version that supported
native-cs).

Maybe split the SPI-portion into 2 sections:
* the SCK, MOSI, MISO (pin 9 to 11) with ALT_0
* the CS GPIOs (standard pins are 7 and 8) with OUTPUT.

That way it is easy to override only this section (plus the gpio-cs property inside the spi node) to extend the number of chip selects or use different mappings.

> 
> I believe a similar comment applies to other SPI nodes too.
I guess the same “splitting” approach should be taken here as well...

WARNING: multiple messages have this Message-ID (diff)
From: martin@sperl.org (Martin Sperl)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/5] ARM: bcm2835: Define standard pinctrl groups in the gpio node.
Date: Fri, 4 Mar 2016 10:27:49 +0100	[thread overview]
Message-ID: <6D770203-ED53-413D-AC28-B89A5C48DA8C@sperl.org> (raw)
In-Reply-To: <56D8AAA2.60907@wwwdotorg.org>


> On 03.03.2016, at 22:20, Stephen Warren <swarren@wwwdotorg.org> wrote:
> 
> On 02/26/2016 11:19 AM, Eric Anholt wrote:
>> The BCM2835-ARM-Peripherals.pdf documentation specifies what the
>> function selects do for the pins, and there are a bunch of obvious
>> groupings to be made.  With these created, we'll be able to replace
>> bcm2835-rpi.dtsi's main "set all of these pins to alt0" with
>> references to specific groups we want enabled.
> 
>> diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
> 
>> +			spi0_gpio7: spi0_gpio7 {
>> +				brcm,pins = <7 8 9 10 11>;
>> +				brcm,function = <BCM2835_FSEL_ALT0>;
>> +			};
> 
> This is too many pins.
> 
> - It includes both MOSI and MISO, although a particular use-case may only use 1 of those.
> 
> - It includes both chip-select signals, whereas a particular use-case may use 0, 1, or 2 of those. This is especially true since IIRC the mainline bcm283x SPI driver wants to only use GPIOs for chip-selects, not SPI-controller-generated chip-select signals, to avoid some issues with the HW generation of these signals.
That is true: the spi-bcm2835 driver requires GPIO usage for chip-select
to make all those latency optimizations work (but also to avoid some
spi-dma issues).
The reason behind it is that there are observed short term ?glitches?
on native CS whenever the SPI control register is touched - even with 
identical values.
And GPIO controlled CS solves this issue (and Mark Brown said that
the GPIO-cs interface is now preferred anyway - hence the auxiliary
spi only implement gpio-cs and requires the CS set as OUTPUT, but
unlike the main spi this does not have ?remapping? support for
legacy device-trees (as there never was a driver-version that supported
native-cs).

Maybe split the SPI-portion into 2 sections:
* the SCK, MOSI, MISO (pin 9 to 11) with ALT_0
* the CS GPIOs (standard pins are 7 and 8) with OUTPUT.

That way it is easy to override only this section (plus the gpio-cs property inside the spi node) to extend the number of chip selects or use different mappings.

> 
> I believe a similar comment applies to other SPI nodes too.
I guess the same ?splitting? approach should be taken here as well...

  parent reply	other threads:[~2016-03-04  9:27 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-26 18:19 [PATCH 0/5] BCM2835 pinctrl DT rework (resend) Eric Anholt
2016-02-26 18:19 ` Eric Anholt
2016-02-26 18:19 ` [PATCH 1/5] ARM: bcm2835: Define standard pinctrl groups in the gpio node Eric Anholt
2016-03-03 21:20   ` Stephen Warren
2016-03-03 21:20     ` Stephen Warren
     [not found]     ` <56D8AAA2.60907-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2016-03-03 22:23       ` Eric Anholt
2016-03-03 22:23         ` Eric Anholt
2016-03-03 22:23         ` Eric Anholt
2016-03-03 22:32         ` Stephen Warren
2016-03-03 22:32           ` Stephen Warren
2016-03-04  9:27     ` Martin Sperl [this message]
2016-03-04  9:27       ` Martin Sperl
2016-03-04  9:27       ` Martin Sperl
2016-02-26 18:19 ` [PATCH 2/5] ARM: bcm2835: Replace alt0/i2s_alt[02] with standard groups Eric Anholt
2016-03-03 21:26   ` Stephen Warren
2016-03-03 21:26     ` Stephen Warren
2016-03-03 22:28     ` Eric Anholt
2016-03-03 22:28       ` Eric Anholt
2016-03-03 22:28       ` Eric Anholt
2016-03-03 22:34       ` Stephen Warren
2016-03-03 22:34         ` Stephen Warren
2016-03-08  8:24   ` Linus Walleij
2016-03-08  8:24     ` Linus Walleij
2016-03-08  8:24     ` Linus Walleij
2016-03-08 16:42     ` Stephen Warren
2016-03-08 16:42       ` Stephen Warren
2016-03-08 16:42       ` Stephen Warren
2016-02-26 18:19 ` [PATCH 3/5] ARM: bcm2835: Move the emmc pin group to bcm283x.dtsi Eric Anholt
2016-02-26 18:19 ` [PATCH 4/5] ARM: bcm2835: Add a group for mapping pins 48-53 to sdhost Eric Anholt
2016-02-26 18:19 ` [PATCH 5/5] ARM: bcm2835: Move most RPi default pin groups to their devices Eric Anholt
2016-03-08  8:25   ` Linus Walleij
2016-03-08  8:25     ` Linus Walleij
2016-03-08  8:25     ` Linus Walleij
     [not found] <1456425661-26123-1-git-send-email-eric@anholt.net>
2016-02-25 18:40 ` [PATCH 1/5] ARM: bcm2835: Define standard pinctrl groups in the gpio node Eric Anholt
2016-02-25 18:40   ` Eric Anholt
2016-02-25 22:24   ` Eric Anholt
2016-02-25 22:24     ` Eric Anholt

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=6D770203-ED53-413D-AC28-B89A5C48DA8C@sperl.org \
    --to=martin@sperl.org \
    --cc=devicetree@vger.kernel.org \
    --cc=eric@anholt.net \
    --cc=f.fainelli@gmail.com \
    --cc=galak@codeaurora.org \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rpi-kernel@lists.infradead.org \
    --cc=mark.rutland@arm.com \
    --cc=pawel.moll@arm.com \
    --cc=robh+dt@kernel.org \
    --cc=swarren@wwwdotorg.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.