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.
wangseok.lee (5):
dt-bindings: pci: Add ARTPEC-8 PCIe controller
dt-bindings: phy: Add ARTPEC-8 PCIe phy
PCI: axis: Add ARTPEC-8 PCIe controller driver
phy: Add ARTPEC-8 PCIe PHY driver
MAINTAINERS: Add maintainer for Axis ARTPEC-8 PCIe PHY driver
.../bindings/pci/axis,artpec8-pcie-ep.yaml | 110 +++
.../devicetree/bindings/pci/axis,artpec8-pcie.yaml | 117 +++
.../bindings/phy/axis,artpec8-pcie-phy.yaml | 67 ++
MAINTAINERS | 2 +
drivers/pci/controller/dwc/Kconfig | 31 +
drivers/pci/controller/dwc/Makefile | 1 +
drivers/pci/controller/dwc/pcie-artpec8.c | 912 +++++++++++++++++++++
drivers/phy/Kconfig | 1 +
drivers/phy/Makefile | 1 +
drivers/phy/artpec/Kconfig | 9 +
drivers/phy/artpec/Makefile | 2 +
drivers/phy/artpec/phy-artpec8-pcie.c | 879 ++++++++++++++++++++
12 files changed, 2132 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pci/axis,artpec8-pcie-ep.yaml
create mode 100644 Documentation/devicetree/bindings/pci/axis,artpec8-pcie.yaml
create mode 100644 Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml
create mode 100644 drivers/pci/controller/dwc/pcie-artpec8.c
create mode 100644 drivers/phy/artpec/Kconfig
create mode 100644 drivers/phy/artpec/Makefile
create mode 100644 drivers/phy/artpec/phy-artpec8-pcie.c
--
2.9.5
On 28/03/2022 11:02, 이왕석 wrote:
>> --------- Original Message ---------
>> Sender : Krzysztof Kozlowski <[email protected]>
>> 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.
> 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
On 29/03/2022 05:49, 이왕석 wrote:
>> --------- Original Message ---------
>> Sender : Krzysztof Kozlowski <[email protected]>
>> 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 <[email protected]>
>>>> 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 <[email protected]>
>>>>>> 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
> --------- Original Message ---------
> Sender : Krzysztof Kozlowski
> Date : 2022-03-29 15:41 (GMT+09:00)
> Title : Re: [PATCH 0/5] Add support for Axis, ARTPEC-8 PCIe driver
>
> On 29/03/2022 05:49, 이왕석 wrote:
>>> --------- Original Message ---------
>>> Sender : Krzysztof Kozlowski
>>> 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
>>>>> 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
>>>>>>> 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
Hi,
Sorry for delay response.
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.
The reset method and type of PHY for initialization are different.
The overall sysreg configuration is different
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.
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.
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.
Thank you.
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
> 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.
On 22-04-22, 08:57, Wangseok Lee wrote:
> > On?18/04/2022?09:20,?Wangseok?Lee?wrote:
> > 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.
I think havong a common driver helps everyone, many vendors do that
already. If you have a technical issue of adding and maintaining a
common driver upstream we would be eager to understand and help with
that...
--
~Vinod