Topics

If you are using PCIe EP, speak up was Re: [RFC PATCH 4.19.y-cip 00/50] Add PCIe EP support for Renesas R-Car Gen3 and RZ/G2x


Pavel Machek
 

Hi!

This patch series adds support for PCIe EP on Renesas R-Car Gen3 and
RZ/G2x platforms.
I quickly went through a series and code seems reasonably nice.

* Since the changes are huge I am sending the patches as RFC.
And yes, it is quite big, which might be a problem. OTOH only Renesas
seems to have PCIe EP drivers enabled in their CIP defconfigs, so
there's good chance noone else in CIP project is using this code.

[If someone else _is_ using it or is considering using it, please
speak up.]

Could we get better explanation for 24/ of the series? spinlock is
okay as long as code inside does not sleep, does not neccessarily have
to do with interrupts.

Should 30/ and 31/ be submitted to stable?

* Required EP framework changes and fixes are ported as well.
* All the patches have been cheery picked from upstream kernel.
* Patches [43, 44, 45, 46, 48]/50 are picked from linux-next.
Ok, so we definitely want them in upstream, not in -next. And it might
be good to wait a bit after merge, so it gets some testing in upstream.

* I was skeptic with patch 36/50 "Rename pcie-rcar.c to pcie-rcar-host.c"
this is required as patch 38/50 adds a new file named pcie-rcar.c. Open
for suggestions if this can be handled differently.
* In patch 37/48 I have dropped the changes for host driver as the patch
doesn't apply cleanly and manually applying it was resulting in a
big diff.
Let me take a look at these in bigger detail.

* As the changes touches three other controller drivers I have build tested them
as done similarly while upstreaming R-Car Gen3 PCIe EP driver.
Will this be tested somehow by our automated tests?

Best regards,
Pavel
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


Lad Prabhakar
 

Hi Pavel,

-----Original Message-----
From: cip-dev@lists.cip-project.org <cip-dev@lists.cip-project.org> On Behalf Of Pavel Machek via lists.cip-project.org
Sent: 14 October 2020 10:39
To: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>
Cc: cip-dev@lists.cip-project.org; Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>; Pavel Machek <pavel@denx.de>; Biju Das
<biju.das.jz@bp.renesas.com>
Subject: [cip-dev] If you are using PCIe EP, speak up was Re: [RFC PATCH 4.19.y-cip 00/50] Add PCIe EP support for Renesas R-Car Gen3 and
RZ/G2x

Hi!

This patch series adds support for PCIe EP on Renesas R-Car Gen3 and
RZ/G2x platforms.
I quickly went through a series and code seems reasonably nice.
Thank you for the review. That’s a relief 😊

* Since the changes are huge I am sending the patches as RFC.
And yes, it is quite big, which might be a problem. OTOH only Renesas
seems to have PCIe EP drivers enabled in their CIP defconfigs, so
there's good chance noone else in CIP project is using this code.
Fingers crossed.

[If someone else _is_ using it or is considering using it, please
speak up.]

Could we get better explanation for 24/ of the series? spinlock is
okay as long as code inside does not sleep, does not neccessarily have
to do with interrupts.
Sure this was cherry picked from upstream I haven’t modified the description. Moreover this I had include because the subsequent patches could apply cleanly.

Do want me to just elaborate the commit message you mean ?

Should 30/ and 31/ be submitted to stable?
Yes I do agree.

* Required EP framework changes and fixes are ported as well.
* All the patches have been cheery picked from upstream kernel.
* Patches [43, 44, 45, 46, 48]/50 are picked from linux-next.
Ok, so we definitely want them in upstream, not in -next. And it might
be good to wait a bit after merge, so it gets some testing in upstream.
Sure Ill wait for them, (fyi these are just dt binding documentation and PCI-ID addition patches)

* I was skeptic with patch 36/50 "Rename pcie-rcar.c to pcie-rcar-host.c"
this is required as patch 38/50 adds a new file named pcie-rcar.c. Open
for suggestions if this can be handled differently.
* In patch 37/48 I have dropped the changes for host driver as the patch
doesn't apply cleanly and manually applying it was resulting in a
big diff.
Let me take a look at these in bigger detail.
Sure.

* As the changes touches three other controller drivers I have build tested them
as done similarly while upstreaming R-Car Gen3 PCIe EP driver.
Will this be tested somehow by our automated tests?
You mean the R-Car Gen3 PCIe EP driver ?

Cheers,
Prabhakar

Best regards,
Pavel
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647


Lad Prabhakar
 

Hi Pavel,

-----Original Message-----
From: cip-dev@lists.cip-project.org <cip-dev@lists.cip-project.org> On Behalf Of Pavel Machek via lists.cip-project.org
Sent: 14 October 2020 10:39
To: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>
Cc: cip-dev@lists.cip-project.org; Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>; Pavel Machek <pavel@denx.de>; Biju Das
<biju.das.jz@bp.renesas.com>
Subject: [cip-dev] If you are using PCIe EP, speak up was Re: [RFC PATCH 4.19.y-cip 00/50] Add PCIe EP support for Renesas R-Car Gen3 and
RZ/G2x

Hi!

This patch series adds support for PCIe EP on Renesas R-Car Gen3 and
RZ/G2x platforms.
I quickly went through a series and code seems reasonably nice.

* Since the changes are huge I am sending the patches as RFC.
And yes, it is quite big, which might be a problem. OTOH only Renesas
seems to have PCIe EP drivers enabled in their CIP defconfigs, so
there's good chance noone else in CIP project is using this code.

[If someone else _is_ using it or is considering using it, please
speak up.]
We haven't received any response yet, is it OK if I send a non RFC version or shall we wait for couple of days more ?

Cheers,
Prabhakar

Could we get better explanation for 24/ of the series? spinlock is
okay as long as code inside does not sleep, does not neccessarily have
to do with interrupts.

Should 30/ and 31/ be submitted to stable?

* Required EP framework changes and fixes are ported as well.
* All the patches have been cheery picked from upstream kernel.
* Patches [43, 44, 45, 46, 48]/50 are picked from linux-next.
Ok, so we definitely want them in upstream, not in -next. And it might
be good to wait a bit after merge, so it gets some testing in upstream.

* I was skeptic with patch 36/50 "Rename pcie-rcar.c to pcie-rcar-host.c"
this is required as patch 38/50 adds a new file named pcie-rcar.c. Open
for suggestions if this can be handled differently.
* In patch 37/48 I have dropped the changes for host driver as the patch
doesn't apply cleanly and manually applying it was resulting in a
big diff.
Let me take a look at these in bigger detail.

* As the changes touches three other controller drivers I have build tested them
as done similarly while upstreaming R-Car Gen3 PCIe EP driver.
Will this be tested somehow by our automated tests?

Best regards,
Pavel
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


Pavel Machek
 

Hi!

I quickly went through a series and code seems reasonably nice.

* Since the changes are huge I am sending the patches as RFC.
And yes, it is quite big, which might be a problem. OTOH only Renesas
seems to have PCIe EP drivers enabled in their CIP defconfigs, so
there's good chance noone else in CIP project is using this code.

[If someone else _is_ using it or is considering using it, please
speak up.]
We haven't received any response yet, is it OK if I send a non RFC version or shall we wait for couple of days more ?
No need to retransmit just now. This version is good enough for
review, let me take a closer look and submit some comments.

Best regards,
Pavel
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


Pavel Machek
 

Hi!

This patch series adds support for PCIe EP on Renesas R-Car Gen3 and
RZ/G2x platforms.
I quickly went through a series and code seems reasonably nice.

* Since the changes are huge I am sending the patches as RFC.
And yes, it is quite big, which might be a problem. OTOH only Renesas
seems to have PCIe EP drivers enabled in their CIP defconfigs, so
there's good chance noone else in CIP project is using this code.

[If someone else _is_ using it or is considering using it, please
speak up.]
We haven't received any response yet, is it OK if I send a non RFC
version or shall we wait for couple of days more ?
I guess I'd like non-RFC version of patches 1-22 in a series. I
believe it makes sense to add 30, 32, 49, 50 to them, as they are
simple and fix a bug.

Would that work for you?

Best regards,
Pavel
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


Lad Prabhakar
 

Hi Pavel,

-----Original Message-----
From: cip-dev@lists.cip-project.org <cip-dev@lists.cip-project.org> On Behalf Of Pavel Machek via lists.cip-project.org
Sent: 20 October 2020 13:02
To: cip-dev@lists.cip-project.org
Cc: Pavel Machek <pavel@denx.de>; Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>; Biju Das <biju.das.jz@bp.renesas.com>
Subject: Re: [cip-dev] If you are using PCIe EP, speak up was Re: [RFC PATCH 4.19.y-cip 00/50] Add PCIe EP support for Renesas R-Car Gen3
and RZ/G2x

Hi!

This patch series adds support for PCIe EP on Renesas R-Car Gen3 and
RZ/G2x platforms.
I quickly went through a series and code seems reasonably nice.

* Since the changes are huge I am sending the patches as RFC.
And yes, it is quite big, which might be a problem. OTOH only Renesas
seems to have PCIe EP drivers enabled in their CIP defconfigs, so
there's good chance noone else in CIP project is using this code.

[If someone else _is_ using it or is considering using it, please
speak up.]
We haven't received any response yet, is it OK if I send a non RFC
version or shall we wait for couple of days more ?
I guess I'd like non-RFC version of patches 1-22 in a series. I
believe it makes sense to add 30, 32, 49, 50 to them, as they are
simple and fix a bug.

Would that work for you?
Sure ill get on posting the above mentioned patches as non-RFC in a series.

How do we tackle with rest of the patches ?

Cheers,
Prabhakar

Best regards,
Pavel
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


Pavel Machek
 

Hi!

This patch series adds support for PCIe EP on Renesas R-Car Gen3 and
RZ/G2x platforms.
I quickly went through a series and code seems reasonably nice.

* Since the changes are huge I am sending the patches as RFC.
And yes, it is quite big, which might be a problem. OTOH only Renesas
seems to have PCIe EP drivers enabled in their CIP defconfigs, so
there's good chance noone else in CIP project is using this code.

[If someone else _is_ using it or is considering using it, please
speak up.]
We haven't received any response yet, is it OK if I send a non RFC
version or shall we wait for couple of days more ?
I guess I'd like non-RFC version of patches 1-22 in a series. I
believe it makes sense to add 30, 32, 49, 50 to them, as they are
simple and fix a bug.

Would that work for you?
Sure ill get on posting the above mentioned patches as non-RFC in a series.

How do we tackle with rest of the patches ?
Well... I applied this batch. If someone can explain the mutex
vs. spinlock thing, then I guess we can do next batch up to 35...

OTOH I did not go through those patches in detail, and RFC is good
enough for review, so... maybe you can just wait.

Best regards,
Pavel
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


Lad Prabhakar
 

Hi Pavel,

-----Original Message-----
From: cip-dev@lists.cip-project.org <cip-dev@lists.cip-project.org> On Behalf Of Pavel Machek via lists.cip-project.org
Sent: 20 October 2020 21:48
To: cip-dev@lists.cip-project.org
Cc: Pavel Machek <pavel@denx.de>; Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>; Biju Das <biju.das.jz@bp.renesas.com>
Subject: Re: [cip-dev] If you are using PCIe EP, speak up was Re: [RFC PATCH 4.19.y-cip 00/50] Add PCIe EP support for Renesas R-Car Gen3
and RZ/G2x

Hi!

This patch series adds support for PCIe EP on Renesas R-Car Gen3 and
RZ/G2x platforms.
I quickly went through a series and code seems reasonably nice.

* Since the changes are huge I am sending the patches as RFC.
And yes, it is quite big, which might be a problem. OTOH only Renesas
seems to have PCIe EP drivers enabled in their CIP defconfigs, so
there's good chance noone else in CIP project is using this code.

[If someone else _is_ using it or is considering using it, please
speak up.]
We haven't received any response yet, is it OK if I send a non RFC
version or shall we wait for couple of days more ?
I guess I'd like non-RFC version of patches 1-22 in a series. I
believe it makes sense to add 30, 32, 49, 50 to them, as they are
simple and fix a bug.

Would that work for you?
Sure ill get on posting the above mentioned patches as non-RFC in a series.

How do we tackle with rest of the patches ?
Well... I applied this batch. If someone can explain the mutex
vs. spinlock thing, then I guess we can do next batch up to 35...
Thank you for queuing in the patches.

I have replied on patch 24/50 to add my two cents.

OTOH I did not go through those patches in detail, and RFC is good
enough for review, so... maybe you can just wait.
Sure will wait.

Cheers,
Prabhakar

Best regards,
Pavel
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany