All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maxime Ripard <mripard@kernel.org>
To: Andre Przywara <andre.przywara@arm.com>
Cc: Chen-Yu Tsai <wens@csie.org>, Mark Rutland <mark.rutland@arm.com>,
	Rob Herring <robh+dt@kernel.org>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-sunxi@googlegroups.com,
	Icenowy Zheng <icenowy@aosc.io>
Subject: Re: [PATCH 3/3] ARM: dts: sun8i: R40: Add SPI controllers nodes and pinmuxes
Date: Thu, 2 Jan 2020 10:57:11 +0100	[thread overview]
Message-ID: <20200102095711.dkd2cnbyitz6mvyx@gilmour.lan> (raw)
In-Reply-To: <20200102012657.9278-4-andre.przywara@arm.com>

[-- Attachment #1: Type: text/plain, Size: 1834 bytes --]

Hi,

On Thu, Jan 02, 2020 at 01:26:57AM +0000, Andre Przywara wrote:
> The Allwinner R40 SoC contains four SPI controllers, using the newer
> sun6i design (but at the legacy addresses).
> The controller seems to be fully compatible to the A64 one, so no driver
> changes are necessary.
> The first three controller can be used on two sets of pins, but SPI3 is
> only routed to one set on Port A.
>
> Tested by connecting a SPI flash to a Bananapi M2 Berry on the SPI0
> PortC header pins.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/arm/boot/dts/sun8i-r40.dtsi | 89 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 89 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
> index 8dcbc4465fbb..af437391dcf4 100644
> --- a/arch/arm/boot/dts/sun8i-r40.dtsi
> +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
> @@ -418,6 +418,41 @@
>  				bias-pull-up;
>  			};
>
> +			spi0_pc_pins: spi0-pc-pins {
> +				pins = "PC0", "PC1", "PC2", "PC23";
> +				function = "spi0";
> +			};
> +
> +			spi0_pi_pins: spi0-pi-pins {
> +				pins = "PI10", "PI11", "PI12", "PI13", "PI14";
> +				function = "spi0";
> +			};

This split doesn't really work though :/

The PC pins group has MOSI, MISO, CLK and CS0, while the PI pins group
has CS0, CLK, MOSI, MISO and CS1.

Meaning that if a board uses a GPIO CS pin, we can't really express
that, and any board using the PI pins for its SPI bus will try to
claim CS0 and CS1, no matter how many devices are connected on the bus
(and if there's one, there might be something else connected to PI14).

And you can't have a board using CS1 with the PC signals either.

You should split away the CS pins into separate groups, like we're
doing with the A20 for example.

And please add /omit-if-no-ref/ to those groups.

Thanks!
Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Maxime Ripard <mripard@kernel.org>
To: Andre Przywara <andre.przywara@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	devicetree@vger.kernel.org, linux-sunxi@googlegroups.com,
	linux-kernel@vger.kernel.org, Chen-Yu Tsai <wens@csie.org>,
	Rob Herring <robh+dt@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	Icenowy Zheng <icenowy@aosc.io>
Subject: Re: [PATCH 3/3] ARM: dts: sun8i: R40: Add SPI controllers nodes and pinmuxes
Date: Thu, 2 Jan 2020 10:57:11 +0100	[thread overview]
Message-ID: <20200102095711.dkd2cnbyitz6mvyx@gilmour.lan> (raw)
In-Reply-To: <20200102012657.9278-4-andre.przywara@arm.com>


[-- Attachment #1.1: Type: text/plain, Size: 1834 bytes --]

Hi,

On Thu, Jan 02, 2020 at 01:26:57AM +0000, Andre Przywara wrote:
> The Allwinner R40 SoC contains four SPI controllers, using the newer
> sun6i design (but at the legacy addresses).
> The controller seems to be fully compatible to the A64 one, so no driver
> changes are necessary.
> The first three controller can be used on two sets of pins, but SPI3 is
> only routed to one set on Port A.
>
> Tested by connecting a SPI flash to a Bananapi M2 Berry on the SPI0
> PortC header pins.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/arm/boot/dts/sun8i-r40.dtsi | 89 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 89 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
> index 8dcbc4465fbb..af437391dcf4 100644
> --- a/arch/arm/boot/dts/sun8i-r40.dtsi
> +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
> @@ -418,6 +418,41 @@
>  				bias-pull-up;
>  			};
>
> +			spi0_pc_pins: spi0-pc-pins {
> +				pins = "PC0", "PC1", "PC2", "PC23";
> +				function = "spi0";
> +			};
> +
> +			spi0_pi_pins: spi0-pi-pins {
> +				pins = "PI10", "PI11", "PI12", "PI13", "PI14";
> +				function = "spi0";
> +			};

This split doesn't really work though :/

The PC pins group has MOSI, MISO, CLK and CS0, while the PI pins group
has CS0, CLK, MOSI, MISO and CS1.

Meaning that if a board uses a GPIO CS pin, we can't really express
that, and any board using the PI pins for its SPI bus will try to
claim CS0 and CS1, no matter how many devices are connected on the bus
(and if there's one, there might be something else connected to PI14).

And you can't have a board using CS1 with the PC signals either.

You should split away the CS pins into separate groups, like we're
doing with the A20 for example.

And please add /omit-if-no-ref/ to those groups.

Thanks!
Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-01-02  9:57 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-02  1:26 [PATCH 0/3] ARM: dts: sun8i: R40: DT fixes and updates Andre Przywara
2020-01-02  1:26 ` Andre Przywara
2020-01-02  1:26 ` [PATCH 1/3] ARM: dts: sun8i: R40: Upgrade GICC reg size to 8K Andre Przywara
2020-01-02  1:26   ` Andre Przywara
2020-01-02  9:31   ` Maxime Ripard
2020-01-02  9:31     ` Maxime Ripard
2020-01-02  1:26 ` [PATCH 2/3] ARM: dts: sun8i: R40: Add PMU node Andre Przywara
2020-01-02  1:26   ` Andre Przywara
2020-01-02  9:32   ` Maxime Ripard
2020-01-02  9:32     ` Maxime Ripard
2020-01-02  1:26 ` [PATCH 3/3] ARM: dts: sun8i: R40: Add SPI controllers nodes and pinmuxes Andre Przywara
2020-01-02  1:26   ` Andre Przywara
2020-01-02  9:57   ` Maxime Ripard [this message]
2020-01-02  9:57     ` Maxime Ripard
2020-01-02 10:41     ` Andre Przywara
2020-01-02 10:41       ` Andre Przywara
2020-01-02 10:54       ` Chen-Yu Tsai
2020-01-02 10:54         ` Chen-Yu Tsai
2020-01-04 10:04       ` Maxime Ripard
2020-01-04 10:04         ` Maxime Ripard
2020-01-05 16:40         ` André Przywara
2020-01-05 16:40           ` André Przywara
2020-01-05 18:28           ` Maxime Ripard
2020-01-05 18:28             ` Maxime Ripard

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=20200102095711.dkd2cnbyitz6mvyx@gilmour.lan \
    --to=mripard@kernel.org \
    --cc=andre.przywara@arm.com \
    --cc=devicetree@vger.kernel.org \
    --cc=icenowy@aosc.io \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sunxi@googlegroups.com \
    --cc=mark.rutland@arm.com \
    --cc=robh+dt@kernel.org \
    --cc=wens@csie.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.