Date   

CIP IRC weekly meeting today

masashi.kudo@cybertrust.co.jp <masashi.kudo@...>
 

Hi all,

Kindly be reminded to attend the weekly meeting through IRC to discuss technical topics with CIP kernel today.

*Please note that the IRC meeting was rescheduled to UTC (GMT) 09:00 starting from the first week of Apr. according to TSC meeting*
https://www.timeanddate.com/worldclock/meetingdetails.html?year=2020&month=10&day=22&hour=9&min=0&sec=0&p1=224&p2=179&p3=136&p4=37&p5=241&p6=248

USWest USEast UK DE TW JP
02:00 05:00 10:00 11:00 17:00 18:00

Channel:
* irc:chat.freenode.net:6667/cip

Last meeting minutes:
https://irclogs.baserock.org/meetings/cip/2020/10/cip.2020-10-15-09.00.log.html

Agenda:

* Action item
1. Combine root filesystem with kselftest binary - iwamatsu
2. Check whether CVE-2019-0145, CVE-2019-0147, CVE-2019-0148 needs to be backported to 4.4 - masashi910

* Kernel maintenance updates
* Kernel testing
* CIP Security
* AOB

The meeting will take 30 min, although it can be extended to an hour if it makes sense and those involved in the topics can stay. Otherwise, the topic will be taken offline or in the next meeting.

Best regards,
--
M. Kudo
Cybertrust Japan Co., Ltd.


Re: [RFC PATCH 4.19.y-cip 38/50] PCI: rcar: Move shareable code to a common file

Lad Prabhakar
 

Hi Pavel,

Thank you for the review.

-----Original Message-----
From: Pavel Machek <pavel@...>
Sent: 21 October 2020 20:07
To: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...>
Cc: cip-dev@...; Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@...>; Pavel Machek <pavel@...>; Biju Das
<biju.das.jz@...>
Subject: Re: [RFC PATCH 4.19.y-cip 38/50] PCI: rcar: Move shareable code to a common file

Hi!

commit 78a0d7f2f5a31357bce68012d886507b4cf33598 upstream.

Move shareable code to common file pcie-rcar.c and the #defines to
pcie-rcar.h so that the common code can be reused with endpoint driver.
There are no functional changes with this patch for the host controller
driver.
Whoa.

So... original patch _moved_ shared code to new place.

This version creates another copy of shared code, probably subtly
different from the other one.

Is that good idea? Won't two copies cause problems depending on
.config? Could we share code, as the mainline does?
My main concern was this produces a big diff which would make it difficult for review. And CONFIG_PCIE_RCAR_HOST by default selects CONFIG_PCIE_RCAR which builds the host driver.
pcie-rcar.c is the same as mainline only that pcie-rcar-host.c is untouched here.

If you are OK ill post the similar changes to pcie-rcar-host.c as done in the actual upstream patch.

Anyway, patches up to previous one -- [37/50] arm64: defconfig: Enable
CONFIG_PCIE_RCAR_HOST look okay to me, so if you can send them as
non-RFC version, I cal likely apply them.
I shall get on posting the 2nd bunch of non-RFC.

Cheers,
Prabhakar

Best regards,
Pavel

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


Re: [RFC PATCH 4.19.y-cip 28/50] PCI: endpoint: Add notification for core init completion

Lad Prabhakar
 

Hi Pavel,

Thank you for the review.

-----Original Message-----
From: Pavel Machek <pavel@...>
Sent: 21 October 2020 20:01
To: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...>
Cc: cip-dev@...; Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@...>; Pavel Machek <pavel@...>; Biju Das
<biju.das.jz@...>
Subject: Re: [RFC PATCH 4.19.y-cip 28/50] PCI: endpoint: Add notification for core init completion

Hi!

From: Vidya Sagar <vidyas@...>

commit 0ef22dcf0c1871888c4c0ee46a9d9c494f2fe997 upstream.

Add support to send notifications to EPF from EPC once the core
registers initialization is complete.

+/**
+ * pci_epc_init_notify() - Notify the EPF device that EPC device's core
+ * initialization is completed.
+ * @epc: the EPC device whose core initialization is completeds
+ *
+ * Invoke to Notify the EPF device that the EPC device's initialization
+ * is completed.
+ */
+void pci_epc_init_notify(struct pci_epc *epc)
+{
+ if (!epc || IS_ERR(epc))
+ return;
+
+ atomic_notifier_call_chain(&epc->notifier, CORE_INIT, NULL);
+}
+EXPORT_SYMBOL_GPL(pci_epc_init_notify);
Is this used somewhere? This adds symbol but noone calls this, and
AFAICT it is not used in the rest of the series, either.
Yep none users (only dwc uses it mainline).

We can merge it, anyway, I guess, but... explanation would be welcome.
Ill post it as part of non-RFC feel free to drop it.

Cheers,
Prabhakar

Best regards,
Pavel

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


Re: [RFC PATCH 4.19.y-cip 38/50] PCI: rcar: Move shareable code to a common file

Pavel Machek
 

Hi!

commit 78a0d7f2f5a31357bce68012d886507b4cf33598 upstream.

Move shareable code to common file pcie-rcar.c and the #defines to
pcie-rcar.h so that the common code can be reused with endpoint driver.
There are no functional changes with this patch for the host controller
driver.
Whoa.

So... original patch _moved_ shared code to new place.

This version creates another copy of shared code, probably subtly
different from the other one.

Is that good idea? Won't two copies cause problems depending on
.config? Could we share code, as the mainline does?

Anyway, patches up to previous one -- [37/50] arm64: defconfig: Enable
CONFIG_PCIE_RCAR_HOST look okay to me, so if you can send them as
non-RFC version, I cal likely apply them.

Best regards,
Pavel

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


Re: [RFC PATCH 4.19.y-cip 28/50] PCI: endpoint: Add notification for core init completion

Pavel Machek
 

Hi!

From: Vidya Sagar <vidyas@...>

commit 0ef22dcf0c1871888c4c0ee46a9d9c494f2fe997 upstream.

Add support to send notifications to EPF from EPC once the core
registers initialization is complete.

+/**
+ * pci_epc_init_notify() - Notify the EPF device that EPC device's core
+ * initialization is completed.
+ * @epc: the EPC device whose core initialization is completeds
+ *
+ * Invoke to Notify the EPF device that the EPC device's initialization
+ * is completed.
+ */
+void pci_epc_init_notify(struct pci_epc *epc)
+{
+ if (!epc || IS_ERR(epc))
+ return;
+
+ atomic_notifier_call_chain(&epc->notifier, CORE_INIT, NULL);
+}
+EXPORT_SYMBOL_GPL(pci_epc_init_notify);
Is this used somewhere? This adds symbol but noone calls this, and
AFAICT it is not used in the rest of the series, either.

We can merge it, anyway, I guess, but... explanation would be welcome.

Best regards,
Pavel

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


Re: [RFC PATCH 4.19.y-cip 24/50] PCI: endpoint: Replace spinlock with mutex

Pavel Machek
 

Hi!

commit 3d3248dbd018502f654064c78efcd2e165ab3486 upstream.

The pci_epc_ops is not intended to be invoked from interrupt context.
Hence replace spin_lock_irqsave and spin_unlock_irqrestore with
mutex_lock and mutex_unlock respectively.
Could I get some kind of explanation why this is good idea?
Apart of one mentioned above other point I would add is on a single core machine mutex_lock/unlock would be good choice.

Also to add the callbacks in controller driver might sleep. For example in raise_irq callback [1], [2].

As long as code protected by the locks does not sleep, spinlocks are
okay... (but they should not need "_irqsave" variants).

They are likely to have better performance, too, when protected code
is small and fast.
I do agree with the above two points *if the code isn't sleeping*.
Okay, we can't really protect sleeping code with a mutex.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/cip/linux-cip.git/tree/drivers/pci/controller/pcie-rockchip-ep.c?h=linux-4.19.y-cip#n410
But this one is not sleeping. It is mdelay(), not msleep().

[2] https://git.kernel.org/pub/scm/linux/kernel/git/cip/linux-cip.git/tree/drivers/pci/controller/pcie-cadence-ep.c?h=linux-4.19.y-cip#n310
And same here.

If there's a place which does sleep with the spinlock held, I'd still
be curious.

OTOH, 1 msec is already threshold where mutex makes sense, so... this
is okay.

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


Re: Request for Camera CSI support for IMX

Lakshmi Natarajan <lakshmi.natarajan@...>
 

Hello Jan,

Thanks for the details. This gives us an idea on what we could do next.
Thanks for your immediate support. We will get in touch with NXP for this.

Regards,
Lakshmi


On Wed, Oct 21, 2020 at 4:27 PM Jan Kiszka <jan.kiszka@...> wrote:
Hi Lakshmi,

On 21.10.20 11:50, Lakshmi Natarajan wrote:
> Hello Jan,
>
> This is regarding requests for CSI support in the IMX processor.
> We were using Yocto + NXP provided kernel to use the CSI peripheral of
> the IMX6 processor. We have complete support from NXP for their kernel
> along with the Yocto framework.
> We want to move to CIP 4.19 + ISAR framework.
> When we checked CIP kernel 4.19 - there was no support for CSI interface
> as well as there were some differences in the arch support too.
> After study of the NXP provided framework, we found that they use the
> kernel from https://source.codeaurora.org/external/imx/linux-imxand
> commit ID 0f9917c56d5995e1dc3bde5658e2d7bc865464de.
> Even linux kernel 5.9 doesn't have the imx specific support.

That's unfortunate.

> Can you guide us on how we should proceed with the same - will it be
> possible for CIP to add IMX support (or) should we request NXP to add
> the patch for CIP kernel 4.19

Yes, please reach out to NXP and ask what their answer for upstream is,
today or at least for the future. It's their issue in the first place.
Also for that, you should prepare with naming the kernel interfaces or
drivers you need in more details.

If NXP cannot or do not want to answer this appropriately, plan B) will
be identifying the patches they provide on top of the upstream kernel,
optimally 4.19. Those patches then needs to be applied on top of the CIP
kernel but, as they are unfortunately downstream only, you will then
have to maintain them for your product and can't propose them for the
CIP kernel.

Best regards,
Jan

>
> Regards,
> Lakshmi
>
>
> On Tue, Oct 20, 2020 at 5:48 PM Rajashree Sankar
> <rajashree.sankar@... <mailto:rajashree.sankar@...>> wrote:
>
>     Hello Jan,
>     We have used Yocto Framework provided by NXP from the following link
>     <https://www.nxp.com/design/development-boards/i-mx-evaluation-and-development-boards/evaluation-kit-for-the-i-mx-6ull-and-6ulz-applications-processor:MCIMX6ULL-EVK?fpsp=1#documentsandsoftware>.Refer
>     to the Document
>
>
>           L4.19.35_1.1.0_LINUX_DOCS
>           <https://www.nxp.com/webapp/Download?colCode=L4.19.35_1.1.0_LINUX_DOCS>
>           in the above mentioned link.
>
>     The DT-bindings and the IMX Camera capture information will be
>     available by following the steps mentioned in the document
>     i.MX_Yocto_Project_User's_Guide from the above link.
>     The Kernel which we are using is fetched from the bitbake files
>     attached below( linux-yocto_4.19.bb <http://linux-yocto_4.19.bb> and
>     its append).Could you please let us know whether CSI  Camera capture
>     supported in this kernel be included in the Linux CIP?
>
>     Thanks and Regards,
>     Rajashree Sankar
>
>
>
> CONFIDENTIALITY
> This e-mail message and any attachments thereto, is intended only for
> use by the addressee(s) named herein and may contain legally privileged
> and/or confidential information. If you are not the intended recipient
> of this e-mail message, you are hereby notified that any dissemination,
> distribution or copying of this e-mail message, and any attachments
> thereto, is strictly prohibited. If you have received this e-mail
> message in error, please immediately notify the sender and permanently
> delete the original and any copies of this email and any prints thereof.
> ABSENT AN EXPRESS STATEMENT TO THE CONTRARY HEREINABOVE, THIS E-MAIL IS
> NOT INTENDED AS A SUBSTITUTE FOR A WRITING. Notwithstanding the Uniform
> Electronic Transactions Act or the applicability of any other law of
> similar substance and effect, absent an express statement to the
> contrary hereinabove, this e-mail message its contents, and any
> attachments hereto are not intended to represent an offer or acceptance
> to enter into a contract and are not otherwise intended to bind the
> sender, Sanmina Corporation (or any of its subsidiaries), or any other
> person or entity.

--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

CONFIDENTIALITY
This e-mail message and any attachments thereto, is intended only for use by the addressee(s) named herein and may contain legally privileged and/or confidential information. If you are not the intended recipient of this e-mail message, you are hereby notified that any dissemination, distribution or copying of this e-mail message, and any attachments thereto, is strictly prohibited. If you have received this e-mail message in error, please immediately notify the sender and permanently delete the original and any copies of this email and any prints thereof.
ABSENT AN EXPRESS STATEMENT TO THE CONTRARY HEREINABOVE, THIS E-MAIL IS NOT INTENDED AS A SUBSTITUTE FOR A WRITING. Notwithstanding the Uniform Electronic Transactions Act or the applicability of any other law of similar substance and effect, absent an express statement to the contrary hereinabove, this e-mail message its contents, and any attachments hereto are not intended to represent an offer or acceptance to enter into a contract and are not otherwise intended to bind the sender, Sanmina Corporation (or any of its subsidiaries), or any other person or entity.


Re: Request for Camera CSI support for IMX

Jan Kiszka
 

Hi Lakshmi,

On 21.10.20 11:50, Lakshmi Natarajan wrote:
Hello Jan,
This is regarding requests for CSI support in the IMX processor.
We were using Yocto + NXP provided kernel to use the CSI peripheral of the IMX6 processor. We have complete support from NXP for their kernel along with the Yocto framework.
We want to move to CIP 4.19 + ISAR framework.
When we checked CIP kernel 4.19 - there was no support for CSI interface as well as there were some differences in the arch support too.
After study of the NXP provided framework, we found that they use the kernel from https://source.codeaurora.org/external/imx/linux-imxand commit ID 0f9917c56d5995e1dc3bde5658e2d7bc865464de.
Even linux kernel 5.9 doesn't have the imx specific support.
That's unfortunate.

Can you guide us on how we should proceed with the same - will it be possible for CIP to add IMX support (or) should we request NXP to add the patch for CIP kernel 4.19
Yes, please reach out to NXP and ask what their answer for upstream is, today or at least for the future. It's their issue in the first place. Also for that, you should prepare with naming the kernel interfaces or drivers you need in more details.

If NXP cannot or do not want to answer this appropriately, plan B) will be identifying the patches they provide on top of the upstream kernel, optimally 4.19. Those patches then needs to be applied on top of the CIP kernel but, as they are unfortunately downstream only, you will then have to maintain them for your product and can't propose them for the CIP kernel.

Best regards,
Jan

Regards,
Lakshmi
On Tue, Oct 20, 2020 at 5:48 PM Rajashree Sankar <rajashree.sankar@... <mailto:rajashree.sankar@...>> wrote:
Hello Jan,
We have used Yocto Framework provided by NXP from the following link
<https://www.nxp.com/design/development-boards/i-mx-evaluation-and-development-boards/evaluation-kit-for-the-i-mx-6ull-and-6ulz-applications-processor:MCIMX6ULL-EVK?fpsp=1#documentsandsoftware>.Refer
to the Document
L4.19.35_1.1.0_LINUX_DOCS
<https://www.nxp.com/webapp/Download?colCode=L4.19.35_1.1.0_LINUX_DOCS> in the above mentioned link.
The DT-bindings and the IMX Camera capture information will be
available by following the steps mentioned in the document
i.MX_Yocto_Project_User's_Guide from the above link.
The Kernel which we are using is fetched from the bitbake files
attached below( linux-yocto_4.19.bb <http://linux-yocto_4.19.bb> and
its append).Could you please let us know whether CSI  Camera capture
supported in this kernel be included in the Linux CIP?
Thanks and Regards,
Rajashree Sankar
CONFIDENTIALITY
This e-mail message and any attachments thereto, is intended only for use by the addressee(s) named herein and may contain legally privileged and/or confidential information. If you are not the intended recipient of this e-mail message, you are hereby notified that any dissemination, distribution or copying of this e-mail message, and any attachments thereto, is strictly prohibited. If you have received this e-mail message in error, please immediately notify the sender and permanently delete the original and any copies of this email and any prints thereof.
ABSENT AN EXPRESS STATEMENT TO THE CONTRARY HEREINABOVE, THIS E-MAIL IS NOT INTENDED AS A SUBSTITUTE FOR A WRITING. Notwithstanding the Uniform Electronic Transactions Act or the applicability of any other law of similar substance and effect, absent an express statement to the contrary hereinabove, this e-mail message its contents, and any attachments hereto are not intended to represent an offer or acceptance to enter into a contract and are not otherwise intended to bind the sender, Sanmina Corporation (or any of its subsidiaries), or any other person or entity.
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux


Re: Request for Camera CSI support for IMX

Lakshmi Natarajan <lakshmi.natarajan@...>
 

Hello Jan,

This is regarding requests for CSI support in the IMX processor.
We were using Yocto + NXP provided kernel to use the CSI peripheral of the IMX6 processor. We have complete support from NXP for their kernel along with the Yocto framework.
We want to move to CIP 4.19 + ISAR framework.
When we checked CIP kernel 4.19 - there was no support for CSI interface as well as there were some differences in the arch support too.
After study of the NXP provided framework, we found that they use the kernel from https://source.codeaurora.org/external/imx/linux-imx and commit ID 0f9917c56d5995e1dc3bde5658e2d7bc865464de.
Even linux kernel 5.9 doesn't have the imx specific support.
Can you guide us on how we should proceed with the same - will it be possible for CIP to add IMX support (or) should we request NXP to add the patch for CIP kernel 4.19

Regards,
Lakshmi


On Tue, Oct 20, 2020 at 5:48 PM Rajashree Sankar <rajashree.sankar@...> wrote:
Hello Jan,
We have used Yocto Framework provided by NXP from the following link.Refer to the Document 

L4.19.35_1.1.0_LINUX_DOCS  in the above mentioned link.

The DT-bindings and the IMX Camera capture information will be available by following the steps mentioned in the document i.MX_Yocto_Project_User's_Guide from the above link.
The Kernel which we are using is fetched from the bitbake files attached below( linux-yocto_4.19.bb and its append).Could you please let us know whether CSI  Camera capture supported in this kernel be included in the Linux CIP?

Thanks and Regards,
Rajashree Sankar


CONFIDENTIALITY
This e-mail message and any attachments thereto, is intended only for use by the addressee(s) named herein and may contain legally privileged and/or confidential information. If you are not the intended recipient of this e-mail message, you are hereby notified that any dissemination, distribution or copying of this e-mail message, and any attachments thereto, is strictly prohibited. If you have received this e-mail message in error, please immediately notify the sender and permanently delete the original and any copies of this email and any prints thereof.
ABSENT AN EXPRESS STATEMENT TO THE CONTRARY HEREINABOVE, THIS E-MAIL IS NOT INTENDED AS A SUBSTITUTE FOR A WRITING. Notwithstanding the Uniform Electronic Transactions Act or the applicability of any other law of similar substance and effect, absent an express statement to the contrary hereinabove, this e-mail message its contents, and any attachments hereto are not intended to represent an offer or acceptance to enter into a contract and are not otherwise intended to bind the sender, Sanmina Corporation (or any of its subsidiaries), or any other person or entity.


Re: 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

Lad Prabhakar
 

Hi Pavel,

-----Original Message-----
From: cip-dev@... <cip-dev@...> On Behalf Of Pavel Machek via lists.cip-project.org
Sent: 20 October 2020 21:48
To: cip-dev@...
Cc: Pavel Machek <pavel@...>; Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@...>; Biju Das <biju.das.jz@...>
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


Re: [RFC PATCH 4.19.y-cip 24/50] PCI: endpoint: Replace spinlock with mutex

Lad Prabhakar
 

Hi Pavel,

-----Original Message-----
From: Pavel Machek <pavel@...>
Sent: 20 October 2020 21:44
To: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...>
Cc: cip-dev@...; Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@...>; Pavel Machek <pavel@...>; Biju Das
<biju.das.jz@...>
Subject: Re: [RFC PATCH 4.19.y-cip 24/50] PCI: endpoint: Replace spinlock with mutex

Hi!

commit 3d3248dbd018502f654064c78efcd2e165ab3486 upstream.

The pci_epc_ops is not intended to be invoked from interrupt context.
Hence replace spin_lock_irqsave and spin_unlock_irqrestore with
mutex_lock and mutex_unlock respectively.
Could I get some kind of explanation why this is good idea?
Apart of one mentioned above other point I would add is on a single core machine mutex_lock/unlock would be good choice.

Also to add the callbacks in controller driver might sleep. For example in raise_irq callback [1], [2].

As long as code protected by the locks does not sleep, spinlocks are
okay... (but they should not need "_irqsave" variants).

They are likely to have better performance, too, when protected code
is small and fast.
I do agree with the above two points *if the code isn't sleeping*.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/cip/linux-cip.git/tree/drivers/pci/controller/pcie-rockchip-ep.c?h=linux-4.19.y-cip#n410
[2] https://git.kernel.org/pub/scm/linux/kernel/git/cip/linux-cip.git/tree/drivers/pci/controller/pcie-cadence-ep.c?h=linux-4.19.y-cip#n310

Cheers,
Prabhakar

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


Re: [PATCH 4.19.y-cip 00/26] Fixes and extension to PCIe EPF

Lad Prabhakar
 

Hi Pavel,

-----Original Message-----
From: Pavel Machek <pavel@...>
Sent: 20 October 2020 21:41
To: cip-dev@...
Cc: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@...>; Pavel Machek <pavel@...>; Biju Das <biju.das.jz@...>;
Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...>
Subject: Re: [cip-dev] [PATCH 4.19.y-cip 00/26] Fixes and extension to PCIe EPF

Hi!

This patch series is part of RFC series [1] ("Add PCIe EP support for
Renesas R-Car Gen3 and RZ/G2x"). For making it more cleaner and easier
to review series [1] is split up as suggested by Pavel, patches 1-22,
30, 32, 49, 50 are included in this set from [1].
Thanks, applied and pushed out.
Thank you.

Cheers,
Prabhakar

Our automated testing did not find anything wrong (but I don't think
that means much).

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


Re: 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.]
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


Re: [RFC PATCH 4.19.y-cip 24/50] PCI: endpoint: Replace spinlock with mutex

Pavel Machek
 

Hi!

commit 3d3248dbd018502f654064c78efcd2e165ab3486 upstream.

The pci_epc_ops is not intended to be invoked from interrupt context.
Hence replace spin_lock_irqsave and spin_unlock_irqrestore with
mutex_lock and mutex_unlock respectively.
Could I get some kind of explanation why this is good idea?

As long as code protected by the locks does not sleep, spinlocks are
okay... (but they should not need "_irqsave" variants).

They are likely to have better performance, too, when protected code
is small and fast.

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


Re: [PATCH 4.19.y-cip 00/26] Fixes and extension to PCIe EPF

Pavel Machek
 

Hi!

This patch series is part of RFC series [1] ("Add PCIe EP support for
Renesas R-Car Gen3 and RZ/G2x"). For making it more cleaner and easier
to review series [1] is split up as suggested by Pavel, patches 1-22,
30, 32, 49, 50 are included in this set from [1].
Thanks, applied and pushed out.

Our automated testing did not find anything wrong (but I don't think
that means much).

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


[PATCH 4.19.y-cip 26/26] tools: PCI: Fix fd leakage

Lad Prabhakar
 

From: Hewenliang <hewenliang4@...>

commit 3c379a59b4795d7279d38c623e74b9790345a32b upstream.

We should close fd before the return of run_test.

Fixes: 3f2ed8134834 ("tools: PCI: Add a userspace tool to test PCI endpoint")
Signed-off-by: Hewenliang <hewenliang4@...>
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@...>
Acked-by: Kishon Vijay Abraham I <kishon@...>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...>
---
tools/pci/pcitest.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/tools/pci/pcitest.c b/tools/pci/pcitest.c
index b7e3b6a64956..9f3d2e584ce4 100644
--- a/tools/pci/pcitest.c
+++ b/tools/pci/pcitest.c
@@ -140,6 +140,7 @@ static int run_test(struct pci_test *test)
}

fflush(stdout);
+ close(fd);
return (ret < 0) ? ret : 1 - ret; /* return 0 if test succeeded */
}

--
2.17.1


[PATCH 4.19.y-cip 25/26] tools: PCI: Exit with error code when test fails

Lad Prabhakar
 

From: Jean-Jacques Hiblot <jjhiblot@...>

commit b71f0a0b1e3fea212a6a5042ced8b48a81738ac9 upstream.

This makes it easier to use pcitest in automated setups.

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@...>
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@...>
Acked-by: Kishon Vijay Abraham I <kishon@...>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...>
---
tools/pci/pcitest.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/pci/pcitest.c b/tools/pci/pcitest.c
index 4c5be77c211f..b7e3b6a64956 100644
--- a/tools/pci/pcitest.c
+++ b/tools/pci/pcitest.c
@@ -140,6 +140,7 @@ static int run_test(struct pci_test *test)
}

fflush(stdout);
+ return (ret < 0) ? ret : 1 - ret; /* return 0 if test succeeded */
}

int main(int argc, char **argv)
@@ -228,6 +229,5 @@ int main(int argc, char **argv)
return -EINVAL;
}

- run_test(test);
- return 0;
+ return run_test(test);
}
--
2.17.1


[PATCH 4.19.y-cip 24/26] PCI: dwc: Fix dw_pcie_ep_raise_msix_irq() to get correct MSI-X table address

Lad Prabhakar
 

From: Kishon Vijay Abraham I <kishon@...>

commit 6f5e193bfb55963ce5f4f68cc927f371ddb0913b upstream.

commit beb4641a787d ("PCI: dwc: Add MSI-X callbacks handler"),
in order to raise MSI-X interrupt, obtained MSIX table address from
Base Address Register (BAR). However BAR only holds PCI address
programmed by the host whereas the MSI-X table should be in the local
memory.

Store the MSI-X table address (virtual address) as part of ->set_bar()
callback and use that to get the message address and message data
here.

Fixes: beb4641a787d ("PCI: dwc: Add MSI-X callbacks handler")
Signed-off-by: Kishon Vijay Abraham I <kishon@...>
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@...>
[PL: Dropped changes to designware driver]
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...>
---
drivers/pci/endpoint/pci-epf-core.c | 2 ++
include/linux/pci-epf.h | 15 +++++++++++++++
2 files changed, 17 insertions(+)

diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
index fb1306de8f40..93ebe916949e 100644
--- a/drivers/pci/endpoint/pci-epf-core.c
+++ b/drivers/pci/endpoint/pci-epf-core.c
@@ -99,6 +99,7 @@ void pci_epf_free_space(struct pci_epf *epf, void *addr, enum pci_barno bar)
epf->bar[bar].phys_addr);

epf->bar[bar].phys_addr = 0;
+ epf->bar[bar].addr = NULL;
epf->bar[bar].size = 0;
epf->bar[bar].barno = 0;
epf->bar[bar].flags = 0;
@@ -135,6 +136,7 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
}

epf->bar[bar].phys_addr = phys_addr;
+ epf->bar[bar].addr = space;
epf->bar[bar].size = size;
epf->bar[bar].barno = bar;
epf->bar[bar].flags |= upper_32_bits(size) ?
diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
index 2d6f07556682..bc5ce7afd79a 100644
--- a/include/linux/pci-epf.h
+++ b/include/linux/pci-epf.h
@@ -92,10 +92,12 @@ struct pci_epf_driver {
/**
* struct pci_epf_bar - represents the BAR of EPF device
* @phys_addr: physical address that should be mapped to the BAR
+ * @addr: virtual address corresponding to the @phys_addr
* @size: the size of the address space present in BAR
*/
struct pci_epf_bar {
dma_addr_t phys_addr;
+ void *addr;
size_t size;
enum pci_barno barno;
int flags;
@@ -127,6 +129,19 @@ struct pci_epf {
struct list_head list;
};

+/**
+ * struct pci_epf_msix_tbl - represents the MSIX table entry structure
+ * @msg_addr: Writes to this address will trigger MSIX interrupt in host
+ * @msg_data: Data that should be written to @msg_addr to trigger MSIX interrupt
+ * @vector_ctrl: Identifies if the function is prohibited from sending a message
+ * using this MSIX table entry
+ */
+struct pci_epf_msix_tbl {
+ u64 msg_addr;
+ u32 msg_data;
+ u32 vector_ctrl;
+};
+
#define to_pci_epf(epf_dev) container_of((epf_dev), struct pci_epf, dev)

#define pci_epf_register_driver(driver) \
--
2.17.1


[PATCH 4.19.y-cip 23/26] PCI: endpoint: Fix clearing start entry in configfs

Lad Prabhakar
 

From: Kunihiko Hayashi <hayashi.kunihiko@...>

commit f58d5f53c89479c12ad719c1960176442add5aaa upstream.

After an endpoint is started through configfs, if 0 is written to the
configfs entry 'start', the controller stops but the epc_group->start
value remains 1.

A subsequent unlinking of the function from the controller would trigger
a spurious WARN_ON_ONCE() in pci_epc_epf_unlink() despite right
behavior.

Fix it by setting epc_group->start = 0 when a controller is stopped
using configfs.

Fixes: d74679911610 ("PCI: endpoint: Introduce configfs entry for configuring EP functions")
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@...>
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@...>
Acked-by: Kishon Vijay Abraham I <kishon@...>
Cc: Kishon Vijay Abraham I <kishon@...>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...>
---
drivers/pci/endpoint/pci-ep-cfs.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/pci/endpoint/pci-ep-cfs.c b/drivers/pci/endpoint/pci-ep-cfs.c
index d1288a0bd530..4fead88257bb 100644
--- a/drivers/pci/endpoint/pci-ep-cfs.c
+++ b/drivers/pci/endpoint/pci-ep-cfs.c
@@ -58,6 +58,7 @@ static ssize_t pci_epc_start_store(struct config_item *item, const char *page,

if (!start) {
pci_epc_stop(epc);
+ epc_group->start = 0;
return len;
}

--
2.17.1


[PATCH 4.19.y-cip 22/26] PCI: endpoint: Cast the page number to phys_addr_t

Lad Prabhakar
 

From: Alan Mikhak <alan.mikhak@...>

commit daee4f4e42c792997f4fee47dcdfa65dd720ec02 upstream.

Modify pci_epc_mem_alloc_addr() to cast the variable 'pageno'
from type 'int' to 'phys_addr_t' before shifting left. This
cast is needed to avoid treating bit 31 of 'pageno' as the
sign bit which would otherwise get sign-extended to produce
a negative value. When added to the base address of PCI memory
space, the negative value would produce an invalid physical
address which falls before the start of the PCI memory space.

Signed-off-by: Alan Mikhak <alan.mikhak@...>
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@...>
Acked-by: Kishon Vijay Abraham I <kishon@...>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...>
---
drivers/pci/endpoint/pci-epc-mem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/endpoint/pci-epc-mem.c b/drivers/pci/endpoint/pci-epc-mem.c
index 0471643cf536..abfac1109a13 100644
--- a/drivers/pci/endpoint/pci-epc-mem.c
+++ b/drivers/pci/endpoint/pci-epc-mem.c
@@ -136,7 +136,7 @@ void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc,
if (pageno < 0)
goto ret;

- *phys_addr = mem->phys_base + (pageno << page_shift);
+ *phys_addr = mem->phys_base + ((phys_addr_t)pageno << page_shift);
virt_addr = ioremap(*phys_addr, size);
if (!virt_addr)
bitmap_release_region(mem->bitmap, pageno, order);
--
2.17.1

4521 - 4540 of 10158