Re: [PATCH 13/24] PCI: dwc: Add ZTE ZX279128S host controller driver

["Stefan Dösinger" <stefandoesinger@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1189 bytes --]

Hi,

I can't say much about the substance of this patch as I haven't dealt with 
PCIe before. What follows are just some gut feeling comments:

Am Samstag, 10. Oktober 2026, 14:59:51 Mitteleuropäische Sommerzeit schrieb 
Navid Ghahremani:
> +static int zx279128s_pcie_init_crm(struct zx279128s_pcie *pcie)
> +{
...
> +       /* Ungate the clocks */
> +       ret = regmap_write_bits(crm2, CRM2_CLK, port->clk_on, port->clk_on);
> +       if (ret)
> +               return ret;
> +       fsleep(50);

If this is a clock control it would be better to write a clock driver that 
manages the crm2 region and exposes a proper clock handle.

For the phy parts that are part of topcrm: drivers/phy has PCIe PHY drivers. 
Maybe one of them happens to match the hardware registers, although that is 
unlikely given that the registers are inside topcrm. Either way adding #phy-
cells to topcrm and isolating the phy controls in a phy driver would be better 
than having these two syscon controls.

By and large the zx279128s_pcie_init_crm has a bad code smell: It looks like 
an LLM translation of ZTE's code to me without understanding what the 
registers actually do.

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 870 bytes --]