Re: [PATCH 0/5] Add support for Axis, ARTPEC-8 PCIe driver

[Krzysztof Kozlowski <krzk@kernel.org>]

On 29/03/2022 05:49, 이왕석 wrote:
>> --------- Original Message ---------
>> Sender : Krzysztof Kozlowski <krzk@kernel.org>
>> Date : 2022-03-28 20:44 (GMT+9)
>> Title : Re: [PATCH 0/5] Add support for Axis, ARTPEC-8 PCIe driver
>>
>> On 28/03/2022 13:29, 이왕석 wrote:
>>>>  --------- Original Message ---------
>>>>  Sender : Krzysztof Kozlowski <krzk@kernel.org>
>>>>  Date : 2022-03-28 18:38 (GMT+9)
>>>>  Title : Re: [PATCH 0/5] Add support for Axis, ARTPEC-8 PCIe driver
>>>>
>>>>  On 28/03/2022 11:02, 이왕석 wrote:
>>>>>>   --------- Original Message ---------
>>>>>>   Sender : Krzysztof Kozlowski <krzk@kernel.org>
>>>>>>   Date : 2022-03-28 16:12 (GMT+9)
>>>>>>   Title : Re: [PATCH 0/5] Add support for Axis, ARTPEC-8 PCIe driver
>>>>>>
>>>>>>   On 28/03/2022 03:44, 이왕석 wrote:
>>>>>>>    This series patches include newly PCIe support for Axis ARTPEC-8 SoC.
>>>>>>>    ARTPEC-8 is the SoC platform of Axis Communications.
>>>>>>>    PCIe controller driver and phy driver have been newly added.
>>>>>>>    There is also a new MAINTAINER in the addition of phy driver.
>>>>>>>    PCIe controller is designed based on Design-Ware PCIe controller IP
>>>>>>>    and PCIe phy is desinged based on SAMSUNG PHY IP.
>>>>>>>    It also includes modifications to the Design-Ware controller driver to 
>>>>>>>    run the 64bit-based ARTPEC-8 PCIe controller driver.
>>>>>>>    It consists of 6 patches in total.
>>>>>>>    
>>>>>>>    This series has been tested on AXIS SW bring-up board 
>>>>>>>    with ARTPEC-8 chipset.
>>>>>>
>>>>>>   You lost mail threading. This makes reading this difficult for us. Plus
>>>>>>   you sent something non-applicable (patch #2), so please resend.
>>>>>>
>>>>>>   Knowing recent Samsung reluctance to extend existing drivers and always
>>>>>>   duplicate, please provide description/analysis why this driver cannot be
>>>>>>   combined with existing driver. The answer like: we need several syscon
>>>>>>   because we do not implement other frameworks (like interconnect) are not
>>>>>>   valid.
>>>>>>
>>>>>>   Best regards,
>>>>>>   Krzysztof
>>>>>   
>>>>>   Hello, Krzysztof
>>>>>   Thanks for your review.
>>>>>   
>>>>>   patch#2 was sent to the wrong format so sent again.
>>>>>   Sorry for causing confusion.
>>>>   
>>>>  The first sending was HTML. Second was broken text, so still not working.
>>>>
>>>>  Please resend everything with proper threading.
>>>  
>>>  Hello, Krzysztof
>>>  
>>>  I sent patch#2 three times.
>>>  due to the influence of the email system,
>>>  there was something wrong with the first and second mails.
>>>  Sorry for causing confusion.
>>>  Did you receive the third patch i sent you?
>>
>> Maybe, I don't know. It's not threaded so it's difficult to find it
>> among other 100 emails...
> 
> I think you also received a normal patch# 2.
> 
>>>   
>>>>>   This patch is specialized in Artpec-8, 
>>>>>   the SoC Platform of Axis Communication, and is newly applied.
>>>>>   Since the target SoC platform is different from the driver previously 
>>>>>   used by Samsung, it is difficult to merge with the existing driver.
>>>>
>>>>  Recently I always saw such answers and sometimes it was true, sometimes
>>>>  not. What is exactly different?
>>>>
>>>>  Best regards,
>>>>  Krzysztof
>>>  
>>>  The main reason this patch should be added is that
>>>  this patch is not the driver applied to exynos platform.
>>
>> Still this does not explain why you need separate driver.
> 
> PCIe driver of artpec-8 is not available in exynos platform.
> because the PCIe of artpec and exynos have very different 
> hardware in SoC design.
> Not only it is the SoC different, 
> but the hardware design of PCIe is also different.
> Therefore, we are using driver's compatible 
> as axis, artpec8-pcie rather than samsung, artpec8-pcie.

You keep repeating the same over and over. What is different? Drivers
can support different devices, I already wrote it. Just because device
is different does not mean it should have separate driver.

> 
>>>  Because the SoC platform is different, 
>>>  the IP configuration of PCIe is also different.
>>
>> What is exactly different? Usually drivers can support IP blocks with
>> some differences...
>>
>>>  We will organize a driver for Artpec-8 platform and 
>>>  if there is no special reason, maintain this 
>>>  without adding it from the next series.
>>
>> I don't understand this.
>>
>>
>> Best regards,
>> Krzysztof
> 
> 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