> On 18/04/2022 09:20, Wangseok Lee wrote: >>>> Also, as you know, >>>> exynos driver is designed according to exynos SoC platform, >>>> so both function and variable names start with exynos. >>> >>> That's hardly a problem... >>> >>>> Compared to the existing exynos driver, >>>> you can see that the structure and type of function are different. >>> >>> No, I cannot see it. You coded the driver that way, you can code it in >>> other way. >>> >>>> For this reason, it is difficult to use the existing exynos driver >>>> for artpec. >>> >>> Naming of functions and structures is not making it difficult. That's >>> not the reason. >>> >>>> Our idea is to register a new PCIe driver for artpec-8 SoC platform >>>> and maintain it in the future. >>> >>> We also want to maintain Exynos PCIe driver in the future. >>> >>> Best regards, >>> Krzysztof >> >> Hi, >> Sorry for delay response. > > Sure, happens, but I don't remember the discussion, so replying that > late will not help your cause. > > You know that if you (you as Samsung) worked with upstream, e.g. by > extending the drivers and keeping them in shape, it would be much easier > for you (again you as Samsung) to actually add new features? It's much > better/effective approach than the path of pushing every time new driver > with explanation like "we do not want to maintain older driver, so we > want a new one"... > >> I have listed some parts that are different from exynos pcie driver. >> >> PHY driver >> PHY is different, so register map is also different. >> Three reference clock options are available in ARTPEC-8. >> It operates by selecting one clock among XO, IO, and SOC PLL. >> However, the exynos phy driver sets one ref clk though sysreg. > > It usually trivial to code such difference in the driver. > >> The reset method and type of PHY for initialization are different. >> The overall sysreg configuration is different > > Indeed. > >> Artpec-8 requires a separate sequence for phy tuning, >> but it does not exist in exynos phy driver. >> Artpec-8 requires pcs resources, but exynos phy driver does not exist. >> > > For the phy driver indeed it might require much effort to create one driver. > >> Controller driver >> Sub controller is different, so register map is also different. >> And it is different handles lane control, link control, PHY clocking, >> reset, interrupt control. >> The number and type of clock resources used are different. >> The overall sysreg configuration is different >> >> Also artpec-8 is performed in dual mode that supports both RC and EP. >> As described above, the PHY and sub ontroller are different >> and the regiser map is different. >> sysreg is also different. And there are differences such as reset. >> The driver will be much more complicated if both hardwares should be >> supported in the same driver. > > Maybe, quite probably. The reluctance to extend any existing code makes > me doubting this, but I admit that there are many differences. > >> For these reasons, my opinion is that better to create >> a phy, controller both driver with a new file. >> Please let me know your opinion. > > At the end it's mostly the decision of PCIe and phy subsystem > maintainers whether they want to have separate drivers for DWC PCIe > blocks in ARMv8 Samsung SoCs. > > In any case, the driver code looks like copied-pasted from some vendor > sources, so you need to bring it to shape. > > Best regards, > Krzysztof Hi, Thank you for your kindness review over several e-mails. I will improve on PATCH 1,2,3,4 and i will return with v2. (yaml and phy, controller driver code improvement) At that time, i would like ask to you and phy, controller maintainer's opinion again. Thank you.
