linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Geraldo Nascimento <geraldogabriel@gmail.com>
To: Shawn Lin <shawn.lin@rock-chips.com>
Cc: "Ye Zhang" <ye.zhang@rock-chips.com>,
	"Lorenzo Pieralisi" <lpieralisi@kernel.org>,
	"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
	"Manivannan Sadhasivam" <mani@kernel.org>,
	"Rob Herring" <robh@kernel.org>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Heiko Stuebner" <heiko@sntech.de>,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Johan Jonker" <jbx6244@gmail.com>,
	linux-rockchip@lists.infradead.org
Subject: Re: [PATCH] arm64: dts: rockchip: align bindings to PCIe spec
Date: Tue, 11 Nov 2025 02:06:53 -0300	[thread overview]
Message-ID: <aRLEbfsmXnGwyigS@geday> (raw)
In-Reply-To: <d9e257bd-806c-48b4-bb22-f1342e9fc15a@rock-chips.com>

On Fri, Nov 07, 2025 at 11:01:04AM +0800, Shawn Lin wrote:
> + Ye Zhang
> 
> 在 2025/11/07 星期五 10:43, Geraldo Nascimento 写道:
> > On Wed, Nov 05, 2025 at 04:56:36PM +0800, Shawn Lin wrote:
> >> 在 2025/11/05 星期三 16:18, Geraldo Nascimento 写道:
> >>> Hi Shawn, glad to hear from you.
> >>>
> >>> Perhaps the following change is better? It resolves the issue
> >>> without the added complication of open drain. After you questioned
> >>> if open drain is actually part of the spec, I remembered that
> >>> GPIO_OPEN_DRAIN is actually (GPIO_SINGLE_ENDED | GPIO_LINE_OPEN_DRAIN)
> >>> so I decided to test with just GPIO_SINGLE_ENDED and it works.
> > 
> > Shawn,
> > 
> > I quote from the PCIe Mini Card Electromechanical Specification Rev 1.2
> > 
> > "3.4.1. Logic Signal Requirements
> > 
> > The 3.3V card logic levels for single-ended digital signals (WAKE#,
> > CLKREQ#, PERST#, and W_DISABLE#) are given in Table 3-7. [...]"
> > 
> > So while you are correct that PERST# is most definitely not Open Drain,
> > there's evidence on the spec that defines this signal as Single-Ended.
> > 
> 
> This's true. But I couldn't find any user in dts using either
> GPIO_SINGLE_ENDED or GPIO_OPEN_DRAIN for PCIe PERST#. I'm curious
> how these two flags affect actual behavior of chips. Ye, could you
> please help check it?
>

While I haven't heard from Ye Zhang still your comment instigated
me to dig deeper, thank you Shawn Lin. What I discovered I believe
is a bug in the Rockchip driver for the GPIO subsystem. Look:

diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
index 47174eb3ba76..5387c78ea11c 100644
--- a/drivers/gpio/gpio-rockchip.c
+++ b/drivers/gpio/gpio-rockchip.c
@@ -272,9 +272,10 @@ static int rockchip_gpio_direction_input(struct gpio_chip *gc,
 static int rockchip_gpio_direction_output(struct gpio_chip *gc,
 					  unsigned int offset, int value)
 {
-	rockchip_gpio_set(gc, offset, value);
 
-	return rockchip_gpio_set_direction(gc, offset, false);
+	rockchip_gpio_set_direction(gc, offset, false);
+
+	return rockchip_gpio_set(gc, offset, value);
 }
 
 /*

It seems to me the current logic is inverted, i.e. GPIO Port A Data
Register can't be successfully written if direction output is not set
yet.

I have to double-check with printk() but from what I see here it may
be very possible that first call to gpiod_get_index() will not set
proper value and only subsequent calls made to gpiod set_value()
will begin to set value.

For what it is worth, with the diff the workaround to set as open
source/emitter with pulldown or set open drain with pullup no longer
works, i.e. PCIe initial link training fails.

The workaround to drop TPVPERL still works, i.e. PCIe initial link
training proceeds, system operational.

Thanks,
Geraldo Nascimento

> > Thanks,
> > Geraldo Nascimento
> > 
> 


  parent reply	other threads:[~2025-11-11  5:07 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-05  5:55 [PATCH] arm64: dts: rockchip: align bindings to PCIe spec Geraldo Nascimento
2025-11-05  6:35 ` Shawn Lin
2025-11-05  8:18   ` Geraldo Nascimento
2025-11-05  8:56     ` Shawn Lin
2025-11-05 20:02       ` Geraldo Nascimento
2025-11-07  2:43       ` Geraldo Nascimento
2025-11-07  3:01         ` Shawn Lin
2025-11-08 22:12           ` Sebastian Reichel
2025-11-08 22:43             ` Geraldo Nascimento
2025-11-11  5:06           ` Geraldo Nascimento [this message]
     [not found]             ` <AGsAmwCFJj0ZQ4vKzrqC84rs.3.1762847224180.Hmail.ye.zhang@rock-chips.com>
2025-11-12  8:03               ` Geraldo Nascimento
2025-11-13  1:09                 ` Geraldo Nascimento
2025-11-14  4:41                   ` Geraldo Nascimento
2025-11-14  9:16                     ` Shawn Lin
2025-11-14 20:34                       ` Geraldo Nascimento
2025-11-15  2:21                         ` Shawn Lin
2025-11-15  7:02                           ` Geraldo Nascimento

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=aRLEbfsmXnGwyigS@geday \
    --to=geraldogabriel@gmail.com \
    --cc=bhelgaas@google.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=heiko@sntech.de \
    --cc=jbx6244@gmail.com \
    --cc=krzk+dt@kernel.org \
    --cc=kwilczynski@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=lpieralisi@kernel.org \
    --cc=mani@kernel.org \
    --cc=robh@kernel.org \
    --cc=shawn.lin@rock-chips.com \
    --cc=ye.zhang@rock-chips.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).