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

Groups > linux.kernel > #1664442 > unrolled thread

[PATCH v2 4/4] arm64: dts: rockchip: use cs-gpios for cros_ec_spi

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

  [PATCH v2 4/4] arm64: dts: rockchip: use cs-gpios for cros_ec_spi Jeffy Chen <jeffy.chen@rock-chips.com> - 2017-06-13 07:30 +0200
    Re: [PATCH v2 4/4] arm64: dts: rockchip: use cs-gpios for cros_ec_spi Brian Norris <briannorris@chromium.org> - 2017-06-13 20:00 +0200
      Re: [PATCH v2 4/4] arm64: dts: rockchip: use cs-gpios for cros_ec_spi Mark Brown <broonie@kernel.org> - 2017-06-13 20:30 +0200

#1664442 — [PATCH v2 4/4] arm64: dts: rockchip: use cs-gpios for cros_ec_spi

The cros_ec requires CS line to be active after last message. But the CS
would be toggled when powering off/on rockchip spi, which breaks ec xfer.
Use GPIO CS to prevent that.

Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---

Changes in v2:
Fix wrong pinconf for spi5_cs0.

 arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
index eb50593..b34a51d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
@@ -790,6 +790,8 @@ ap_i2c_audio: &i2c8 {
 &spi5 {
 	status = "okay";
 
+	cs-gpios = <&gpio2 23 GPIO_ACTIVE_HIGH>;
+
 	cros_ec: ec@0 {
 		compatible = "google,cros-ec-spi";
 		reg = <0>;
@@ -813,6 +815,10 @@ ap_i2c_audio: &i2c8 {
 	};
 };
 
+&spi5_cs0 {
+	rockchip,pins = <RK_GPIO2 23 RK_FUNC_GPIO &pcfg_output_high>;
+};
+
 &tsadc {
 	status = "okay";
 
-- 
2.1.4

[toc] | [next] | [standalone]

#1665069

On Tue, Jun 13, 2017 at 01:25:43PM +0800, Jeffy Chen wrote:
> The cros_ec requires CS line to be active after last message. But the CS
> would be toggled when powering off/on rockchip spi, which breaks ec xfer.
> Use GPIO CS to prevent that.

I suppose this change is fine. (At least, I don't have a good reason not
to do this.)

But I still wonder whether this is something that the SPI core can be
expected to handle. drivers/mfd/cros_ec_spi.c already sets the
appropriate trans->cs_change bits, to ensure CS remains active in
between certain messages (all under spi_bus_lock()). But you're
suggesting that your bus controller may deassert CS if you runtime
suspend the device (e.g., in between messages).

So, is your controller just peculiar? Or should the SPI core avoid
autosuspending the bus controller when it's been instructed to keep CS
active? Any thoughts Mark?

> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> ---
> 
> Changes in v2:
> Fix wrong pinconf for spi5_cs0.
> 
>  arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> index eb50593..b34a51d 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> @@ -790,6 +790,8 @@ ap_i2c_audio: &i2c8 {
>  &spi5 {
>  	status = "okay";
>  
> +	cs-gpios = <&gpio2 23 GPIO_ACTIVE_HIGH>;

This isn't actually true; it's not active high, it's active low. I guess it
doesn't really matter because the SPI framework uses the gpio_* APIs, which
don't account for the ACTIVE_* flags, and it has separate flags for uncommon
device polarity ("spi-cs-high").

Brian

> +
>  	cros_ec: ec@0 {
>  		compatible = "google,cros-ec-spi";
>  		reg = <0>;
> @@ -813,6 +815,10 @@ ap_i2c_audio: &i2c8 {
>  	};
>  };
>  
> +&spi5_cs0 {
> +	rockchip,pins = <RK_GPIO2 23 RK_FUNC_GPIO &pcfg_output_high>;
> +};
> +
>  &tsadc {
>  	status = "okay";
>  
> -- 
> 2.1.4
> 
> 

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

#1665101

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

On Tue, Jun 13, 2017 at 10:50:44AM -0700, Brian Norris wrote:
> On Tue, Jun 13, 2017 at 01:25:43PM +0800, Jeffy Chen wrote:
> > The cros_ec requires CS line to be active after last message. But the CS
> > would be toggled when powering off/on rockchip spi, which breaks ec xfer.
> > Use GPIO CS to prevent that.

> I suppose this change is fine. (At least, I don't have a good reason not
> to do this.)

> But I still wonder whether this is something that the SPI core can be
> expected to handle. drivers/mfd/cros_ec_spi.c already sets the
> appropriate trans->cs_change bits, to ensure CS remains active in
> between certain messages (all under spi_bus_lock()). But you're
> suggesting that your bus controller may deassert CS if you runtime
> suspend the device (e.g., in between messages).

> So, is your controller just peculiar? Or should the SPI core avoid
> autosuspending the bus controller when it's been instructed to keep CS
> active? Any thoughts Mark?

This sounds like the controller being unusual - though frankly the
ChromeOS chip select usage is also odd so it's fairly rare for something
like this to come up.  I'd not expect a runtime suspend to loose the pin
state, though possibly through use of pinctrl rather than the
controller.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web