On Tue, Nov 2, 2021 at 12:44 PM Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx> wrote: > > Hi Bjorn, > > Em Tue, 2 Nov 2021 11:06:12 -0500 > Bjorn Helgaas <helgaas@xxxxxxxxxx> escreveu: > > > > + > > > + /* Per-slot clkreq */ > > > + int n_gpio_clkreq; > > > + int gpio_id_clkreq[MAX_PCI_SLOTS]; > > > + const char *clkreq_names[MAX_PCI_SLOTS]; > > > > I think there's been previous discussion about this, but I didn't > > follow it, so I'm just double-checking that this is what we want here. > > > > IIUC, this (MAX_PCI_SLOTS, "hisilicon,clken-gpios") applies to an > > external PEX 8606 bridge, which seems a little strange to be > > hard-coded into the kirin driver this way. > > > > I see that "hisilicon,clken-gpios" is optional, but what if some > > platform connects all 6 lanes? What if there's a different bridge > > altogether? Keep in mind this is HiKey. There's never been a 2nd board upstream for the SoCs, the boards have a short lifespan in terms of both support and availability. And yet Hikey manages to do multiple complicated things on the board design. I have a hard time caring... > > > > I'll assume this is actually the way we want thing unless I hear > > otherwise. > > Yes, there was past discussions about that with Rob, with regards > to how the DT would represent it, which got reflected at the code. At first I thought these were CLKREQ connections which absolutely should be per port/bridge like PERST, but they are not. They are simply enables for the refclk's and pretty specific to HiKey. > At the end, it was decided to just add a single property inside PCIe: > > > pcie@f4000000 { > compatible = "hisilicon,kirin970-pcie"; > ... > hisilicon,clken-gpios = <&gpio27 3 0>, <&gpio17 0 0>, > <&gpio20 6 0>; > > I don't think this is a problem, as, if some day another bridge would > need a larger number of slots, it is just a matter of changing the > number at the MAX_PCI_SLOTS, as this controls only the size of the array > (and the check for array overflow when parsing the properties). It is a problem that your host bridge driver has hardcoded board specifics. That's not the first time you've heard that from me. But given the board-to-soc ratio of Hikey is 1:1, I don't care that much. Rob