All of lore.kernel.org
 help / color / mirror / Atom feed
From: Manivannan Sadhasivam <mani@kernel.org>
To: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Cc: Rob Herring <robh@kernel.org>, Bjorn Helgaas <helgaas@kernel.org>,
	linuxarm@huawei.com, mauro.chehab@huawei.com,
	Kishon Vijay Abraham I <kishon@ti.com>,
	Vinod Koul <vkoul@kernel.org>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-phy@lists.infradead.org
Subject: Re: [PATCH v5 2/8] dt-bindings: phy: Add bindings for HiKey 970 PCIe PHY
Date: Wed, 14 Jul 2021 23:12:25 +0530	[thread overview]
Message-ID: <20210714174225.GA8988@workstation> (raw)
In-Reply-To: <20210714091435.322d68b1@coco.lan>

Hi Mauro,

On Wed, Jul 14, 2021 at 09:14:35AM +0200, Mauro Carvalho Chehab wrote:
> Em Tue, 13 Jul 2021 20:26:49 -0600
> Rob Herring <robh@kernel.org> escreveu:
> 
> > On Tue, Jul 13, 2021 at 08:28:35AM +0200, Mauro Carvalho Chehab wrote:
> 
> > > +  reset-gpios:
> > > +    description: PCI PERST reset GPIOs
> > > +    maxItems: 4  
> > 
> > Hiding the 4 ports in the phy?
> 
> Rob,
> 
> I'm not trying to hide anything.
> 
> There are several differences with regards to how PERST# is handled between
> HiKey 960 and HiKey 970.
> 
> From hardware perspective, you can see the schematics of both boards:
> 
> 	https://github.com/96boards/documentation/raw/master/consumer/hikey/hikey960/hardware-docs/HiKey960_SoC_Reference_Manual.pdf
> 	https://www.96boards.org/documentation/consumer/hikey/hikey970/hardware-docs/files/hikey970-schematics.pdf
> 
> The 960 PHY has the SoC directly connected to a PCIE M.2 slot 
> (model 10130616) without any external bridge chipset. It uses a single 
> GPIO (GPIO 089) for the PERST# signal, connected via a voltage converter
> (from 1.8V to 3.3V).
> 
> 	$ lspci
> 	00:00.0 PCI bridge: Huawei Technologies Co., Ltd. Device 3660 (rev 01)
> 
> The 970 PHY has an external PCI bridge chipset (PLX Technology PEX 8606).
> Besides the bridge, the hardware comes with an Ethernet PCI adapter, a
> M.2 slot and a mini-PCIe connector. Each one with its own PERST# signal,
> mapped to different GPIO pins, and each one using its own voltage
> converter.
> 
> 	$ lspci
> 	00:00.0 PCI bridge: Huawei Technologies Co., Ltd. Device 3670 (rev 01)
> 	01:00.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
> 	02:01.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
> 	02:04.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
> 	02:05.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
> 	02:07.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
> 	02:09.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
> 	06:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 07)
> 
> On other words, there are 4 GPIOs mapped to different PERST# pins in
> the hardware:
> 
> - GPIO 56 is connected to the PERST# pin at PEX 8606;
> - GPIO 25 is connected to the PERST# pin at the M.2 slot;
> - GPIO 220 is connected to the PERST# pin at the PCIe mini slot;
> - GPIO 203 is connected to the PERST# pin at the Ethernet chipset.
> 
> Maybe due to different electrical requirements, the hardware design
> use different GPIOs instead of feeding them altogether.
> 
> Anyway, the fact is that the PHY on 970 has 4 different GPIOs that are
> need in order for the hardware to work. and this is specific to this
> particular PHY.
> 

I'm not sure about this. That fact that the PCIe device's PERST# signal
wired to different GPIOs doesn't mean that those GPIOs belong to the PHY.
Those GPIOs should be independent of the PCIe core controlled manually
by the driver.

I think this issue is somewhat similar to the one we are dealing on the
Qcom platforms [1] where each PCIe device uses a different GPIO and voltage
config to operate. And those need to be active for the link training to
succeed.

So perhaps we should aim for a common solution? The GPIO and voltage
layout should be described in DT for each port exposed by the SoC/board.

Thanks,
Mani

[1] https://lkml.org/lkml/2021/6/21/1524

WARNING: multiple messages have this Message-ID (diff)
From: Manivannan Sadhasivam <mani@kernel.org>
To: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Cc: Rob Herring <robh@kernel.org>, Bjorn Helgaas <helgaas@kernel.org>,
	linuxarm@huawei.com, mauro.chehab@huawei.com,
	Kishon Vijay Abraham I <kishon@ti.com>,
	Vinod Koul <vkoul@kernel.org>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-phy@lists.infradead.org
Subject: Re: [PATCH v5 2/8] dt-bindings: phy: Add bindings for HiKey 970 PCIe PHY
Date: Wed, 14 Jul 2021 23:12:25 +0530	[thread overview]
Message-ID: <20210714174225.GA8988@workstation> (raw)
In-Reply-To: <20210714091435.322d68b1@coco.lan>

Hi Mauro,

On Wed, Jul 14, 2021 at 09:14:35AM +0200, Mauro Carvalho Chehab wrote:
> Em Tue, 13 Jul 2021 20:26:49 -0600
> Rob Herring <robh@kernel.org> escreveu:
> 
> > On Tue, Jul 13, 2021 at 08:28:35AM +0200, Mauro Carvalho Chehab wrote:
> 
> > > +  reset-gpios:
> > > +    description: PCI PERST reset GPIOs
> > > +    maxItems: 4  
> > 
> > Hiding the 4 ports in the phy?
> 
> Rob,
> 
> I'm not trying to hide anything.
> 
> There are several differences with regards to how PERST# is handled between
> HiKey 960 and HiKey 970.
> 
> From hardware perspective, you can see the schematics of both boards:
> 
> 	https://github.com/96boards/documentation/raw/master/consumer/hikey/hikey960/hardware-docs/HiKey960_SoC_Reference_Manual.pdf
> 	https://www.96boards.org/documentation/consumer/hikey/hikey970/hardware-docs/files/hikey970-schematics.pdf
> 
> The 960 PHY has the SoC directly connected to a PCIE M.2 slot 
> (model 10130616) without any external bridge chipset. It uses a single 
> GPIO (GPIO 089) for the PERST# signal, connected via a voltage converter
> (from 1.8V to 3.3V).
> 
> 	$ lspci
> 	00:00.0 PCI bridge: Huawei Technologies Co., Ltd. Device 3660 (rev 01)
> 
> The 970 PHY has an external PCI bridge chipset (PLX Technology PEX 8606).
> Besides the bridge, the hardware comes with an Ethernet PCI adapter, a
> M.2 slot and a mini-PCIe connector. Each one with its own PERST# signal,
> mapped to different GPIO pins, and each one using its own voltage
> converter.
> 
> 	$ lspci
> 	00:00.0 PCI bridge: Huawei Technologies Co., Ltd. Device 3670 (rev 01)
> 	01:00.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
> 	02:01.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
> 	02:04.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
> 	02:05.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
> 	02:07.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
> 	02:09.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
> 	06:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 07)
> 
> On other words, there are 4 GPIOs mapped to different PERST# pins in
> the hardware:
> 
> - GPIO 56 is connected to the PERST# pin at PEX 8606;
> - GPIO 25 is connected to the PERST# pin at the M.2 slot;
> - GPIO 220 is connected to the PERST# pin at the PCIe mini slot;
> - GPIO 203 is connected to the PERST# pin at the Ethernet chipset.
> 
> Maybe due to different electrical requirements, the hardware design
> use different GPIOs instead of feeding them altogether.
> 
> Anyway, the fact is that the PHY on 970 has 4 different GPIOs that are
> need in order for the hardware to work. and this is specific to this
> particular PHY.
> 

I'm not sure about this. That fact that the PCIe device's PERST# signal
wired to different GPIOs doesn't mean that those GPIOs belong to the PHY.
Those GPIOs should be independent of the PCIe core controlled manually
by the driver.

I think this issue is somewhat similar to the one we are dealing on the
Qcom platforms [1] where each PCIe device uses a different GPIO and voltage
config to operate. And those need to be active for the link training to
succeed.

So perhaps we should aim for a common solution? The GPIO and voltage
layout should be described in DT for each port exposed by the SoC/board.

Thanks,
Mani

[1] https://lkml.org/lkml/2021/6/21/1524

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

  parent reply	other threads:[~2021-07-14 17:42 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-13  6:28 [PATCH v5 0/8] Add support for Hikey 970 PCIe Mauro Carvalho Chehab
2021-07-13  6:28 ` Mauro Carvalho Chehab
2021-07-13  6:28 ` Mauro Carvalho Chehab
2021-07-13  6:28 ` [PATCH v5 1/8] dt-bindings: phy: Add bindings for HiKey 960 PCIe PHY Mauro Carvalho Chehab
2021-07-13  6:28   ` Mauro Carvalho Chehab
2021-07-14  2:22   ` Rob Herring
2021-07-14  2:22     ` Rob Herring
2021-07-13  6:28 ` [PATCH v5 2/8] dt-bindings: phy: Add bindings for HiKey 970 " Mauro Carvalho Chehab
2021-07-13  6:28   ` Mauro Carvalho Chehab
2021-07-14  2:26   ` Rob Herring
2021-07-14  2:26     ` Rob Herring
2021-07-14  7:14     ` Mauro Carvalho Chehab
2021-07-14  7:14       ` Mauro Carvalho Chehab
2021-07-14 14:17       ` Rob Herring
2021-07-14 14:17         ` Rob Herring
2021-07-14 14:31         ` Mauro Carvalho Chehab
2021-07-14 14:31           ` Mauro Carvalho Chehab
2021-07-14 17:42       ` Manivannan Sadhasivam [this message]
2021-07-14 17:42         ` Manivannan Sadhasivam
2021-07-15  6:37         ` Mauro Carvalho Chehab
2021-07-15  6:37           ` Mauro Carvalho Chehab
2021-07-19 15:26           ` Mauro Carvalho Chehab
2021-07-19 15:26             ` Mauro Carvalho Chehab
2021-07-27  8:11         ` Mauro Carvalho Chehab
2021-07-27  8:11           ` Mauro Carvalho Chehab
2021-07-13  6:28 ` [PATCH v5 3/8] dt-bindings: PCI: kirin: Fix compatible string Mauro Carvalho Chehab
2021-07-14  2:27   ` Rob Herring
2021-07-13  6:28 ` [PATCH v5 4/8] dt-bindings: PCI: kirin: Drop PHY properties Mauro Carvalho Chehab
2021-07-14  2:28   ` Rob Herring
2021-07-14 11:22     ` Mauro Carvalho Chehab
2021-07-16 11:22     ` Mauro Carvalho Chehab
2021-07-13  6:28 ` [PATCH v5 5/8] phy: HiSilicon: Add driver for Kirin 960 PCIe PHY Mauro Carvalho Chehab
2021-07-13  6:28   ` Mauro Carvalho Chehab
2021-07-13  6:28 ` [PATCH v5 6/8] phy: HiSilicon: add driver for Kirin 970 " Mauro Carvalho Chehab
2021-07-13  6:28   ` Mauro Carvalho Chehab
2021-07-13  6:28 ` [PATCH v5 7/8] PCI: kirin: Drop the PHY logic from the driver Mauro Carvalho Chehab
2021-07-13  6:28   ` Mauro Carvalho Chehab
2021-07-13  6:28 ` [PATCH v5 8/8] PCI: kirin: Use regmap for APB registers Mauro Carvalho Chehab

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=20210714174225.GA8988@workstation \
    --to=mani@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=helgaas@kernel.org \
    --cc=kishon@ti.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-phy@lists.infradead.org \
    --cc=linuxarm@huawei.com \
    --cc=mauro.chehab@huawei.com \
    --cc=mchehab+huawei@kernel.org \
    --cc=robh@kernel.org \
    --cc=vkoul@kernel.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.