Re: [PATCH v4 3/5] PCI: axis: Add ARTPEC-8 PCIe controller driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 20/07/2022 08:01, Wangseok Lee wrote:
> Add support Axis, ARTPEC-8 SoC. ARTPEC-8 is the SoC platform of Axis
> Communications. This is based on arm64 and support GEN4 & 2lane. This
> PCIe controller is based on DesignWare Hardware core and uses DesignWare
> core functions to implement the driver. "pcie-artpec6. c" supports artpec6
> and artpec7 H/W. artpec8 can not be expanded because H/W configuration is
> completely different from artpec6/7. PHY and sub controller are different.
> 
> Signed-off-by: Wangseok Lee <wangseok.lee@xxxxxxxxxxx>
> Signed-off-by: Jaeho Cho <jaeho79.cho@xxxxxxxxxxx>
> ---
> v3->v4 :
> -Remove unnecessary enum type
> -Fix indentation
> 

Thanks for the changes. This starts to look good, however I am not going
to ack it. This is also not a strong NAK, as I would respect Bjorn and
other maintainers decision.

I don't like the approach of creating only Artpec-8 specific driver.
Samsung heavily reuses its block in all Exynos devices. Now it re-uses
them for other designs as well. Therefore, even if merging with existing
Exynos PCIe driver is not feasible (we had such discussions), I expect
this to cover all Samsung Foundry PCIe devices. From all current designs
up to future licensed blocks, including some new Samsung Exynos SoC. Or
at least be ready for it.

However it seems you are interested only in achieving one goal here -
satisfy Axis. I believe it is not the "upstream approach". Next month
you come up with same driver for different customer and you keep
insisting "it's different!".

To get my ack I want to see something generic for Samsung Exynos SoC and
other licensed or designed blocks, instead of something made for only
one of your customers.

Best regards,
Krzysztof



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux