Re: [PATCH v5 2/8] dt-bindings: phy: Add bindings for HiKey 970 PCIe PHY

[Mauro Carvalho Chehab]

Em Wed, 14 Jul 2021 08:17:05 -0600
Rob Herring <robh@xxxxxxxxxx> escreveu:

> On Wed, Jul 14, 2021 at 1:14 AM Mauro Carvalho Chehab
> <mchehab+huawei@xxxxxxxxxx> wrote:
> >
> > Em Tue, 13 Jul 2021 20:26:49 -0600
> > Rob Herring <robh@xxxxxxxxxx> 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.  
> 
> This hierarchy could be done on any board. It has nothing to do with the PHY.

True, but right now, the pci-bus.yaml prevents it, as it allows just 
one reset GPIO[1]:

  reset-gpios:
    description: GPIO controlled connection to PERST# signal
    maxItems: 1

[1] https://github.com/robherring/dt-schema/blob/master/schemas/pci/pci-bus.yaml

If the schema will be changed to allow multiple reset-gpios, It should
be possible to keep this at the pcie-kirin.c driver with something like:

	static int kirin_pcie_power_on(struct kirin_pcie *kirin_pcie)
	{
		int ret;

		ret = phy_init(kirin_pcie->phy);
		if (ret)
			return ret;

		ret = phy_power_on(kirin_pcie->phy);

                /* perst assert Endpoints */
                usleep_range(21000, 23000);
                for (i = 0; i < phy->n_gpio_resets; i++) {
                        ret = gpio_direction_output(phy->gpio_id_reset[i], 1);
                        if (ret) {
				phy_power_off(kirin_pcie->phy);
                                return ret;
			}
                }
                usleep_range(10000, 11000);

		return phy_reset(kirin_pcie->phy);
	}

This would work for both 960 - where phy_reset() is not needed,
and for 970, where it would set the eye diagram for the PHY.

Should I send a patch for pci-bus.yaml via github?

Thanks,
Mauro