Message ID | 20201012141933.9652-1-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
---|---|
Headers | show |
Series | Add PCIe EP support for Renesas R-Car Gen3 and RZ/G2x | expand |
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
Hi! > * 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. I have no good ideas how to avoid rename. Given the change in locking previously in the series, manual review of changes in this area might be good idea, anyway. In 4.19, there were 75 patches in this area so far, and 5 patches that would reject due to the rename. pavel@amd:~/cip/krc$ git log --pretty=oneline v4.19.. drivers/pci/controller/ | wc -l 75 pavel@amd:~/cip/krc$ git log --pretty=oneline v4.19.. drivers/pci/controller/pcie-rcar*.c | wc -l 5 ...so we'll get a bit of additional workload from this. It would be more scary if there were -rt specific changes in this area, but hopefully there are none. I expect that you are willing to help if -stable or -rt introduces conflicting patches in this area? Best regards, Pavel
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
Hi Pavel, > -----Original Message----- > From: Pavel Machek <pavel@denx.de> > Sent: 14 October 2020 11:08 > 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: Re: [RFC PATCH 4.19.y-cip 00/50] Add PCIe EP support for Renesas R-Car Gen3 and RZ/G2x > > Hi! > > > * 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. > > I have no good ideas how to avoid rename. > > Given the change in locking previously in the series, manual review of > changes in this area might be good idea, anyway. > The locking changes are wrt EPF framework anyway. OK so I assume you are happy to manual changes to PCIe host driver while posting non RFC version. > In 4.19, there were 75 patches in this area so far, and 5 patches that > would reject due to the rename. > > pavel@amd:~/cip/krc$ git log --pretty=oneline v4.19.. drivers/pci/controller/ | wc -l > 75 > pavel@amd:~/cip/krc$ git log --pretty=oneline v4.19.. drivers/pci/controller/pcie-rcar*.c | wc -l > 5 > > ...so we'll get a bit of additional workload from this. It would be > more scary if there were -rt specific changes in this area, but > hopefully there are none. > > I expect that you are willing to help if -stable or -rt introduces > conflicting patches in this area? > indeed. 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 -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#5581): https://lists.cip-project.org/g/cip-dev/message/5581 Mute This Topic: https://lists.cip-project.org/mt/77461654/4520388 Group Owner: cip-dev+owner@lists.cip-project.org Unsubscribe: https://lists.cip-project.org/g/cip-dev/leave/8129055/727948398/xyzzy [cip-dev@archiver.kernel.org] -=-=-=-=-=-=-=-=-=-=-=-
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 -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#5591): https://lists.cip-project.org/g/cip-dev/message/5591 Mute This Topic: https://lists.cip-project.org/mt/77501866/4520388 Group Owner: cip-dev+owner@lists.cip-project.org Unsubscribe: https://lists.cip-project.org/g/cip-dev/leave/8129055/727948398/xyzzy [cip-dev@archiver.kernel.org] -=-=-=-=-=-=-=-=-=-=-=-
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
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
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 -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#5596): https://lists.cip-project.org/g/cip-dev/message/5596 Mute This Topic: https://lists.cip-project.org/mt/77501866/4520388 Group Owner: cip-dev+owner@lists.cip-project.org Unsubscribe: https://lists.cip-project.org/g/cip-dev/leave/8129055/727948398/xyzzy [cip-dev@archiver.kernel.org] -=-=-=-=-=-=-=-=-=-=-=-
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
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 -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#5629): https://lists.cip-project.org/g/cip-dev/message/5629 Mute This Topic: https://lists.cip-project.org/mt/77501866/4520388 Group Owner: cip-dev+owner@lists.cip-project.org Unsubscribe: https://lists.cip-project.org/g/cip-dev/leave/8129055/727948398/xyzzy [cip-dev@archiver.kernel.org] -=-=-=-=-=-=-=-=-=-=-=-