mbox series

[v9,00/11] Restructure the rpmsg_char driver and introduce rpmsg_ctrl driver

Message ID 20220124102524.295783-1-arnaud.pouliquen@foss.st.com (mailing list archive)
Headers show
Series Restructure the rpmsg_char driver and introduce rpmsg_ctrl driver | expand

Message

Arnaud POULIQUEN Jan. 24, 2022, 10:25 a.m. UTC
Updates from V8 [1]:
- rebase on 5.17-rc1 + rpmsg char cdev release fixes[2][3]
- updates based on Bjorn Andersson's comments:
  - remove rpmsg_create_default_ept API, set directly the ept->priv in rpmsg_chrdev_probe
    function.
  - rework commit message in [8/9]rpmsg: char: Introduce the "rpmsg-raw" channel

Patchset description:

The current rpmsg_char module implements a /dev/rpmsg_ctrl interface that provides the ability to
instantiate char devices (/dev/rpmsgX) associated with an rpmsg endpoint for communication with the
remote processor.
This implementation fits with QCOM rpmsg backend but not with the magement by chanel implemented
in the generic rpmsg virtio backend.
This series restructures the rpmsg_char driver to decorrelate the control part from the data part
in order to improve its compatible with the rpmsg virtio backend.

Objective:
- Expose a /dev/rpmsg_ctrlX interface for the application that is no longer dedicated to the
  rpmsg_char but generalized to all rpmsg services. This offers capability to create and destroy
  rpmsg channels from a user's application initiative (using the new RPMSG_CREATE_DEV_IOCTL and
  RPMSG_DESTROY_DEV_IOCTL controls).
  An application will be able to create/establish an rpmsg communication channel to communicate
  with the remote processor, and not only wait the remote processor initiative.
  This is interesting for example to establish a temporary communication link for diagnosis,
  calibration, debugging... or instantiate  new data flows on some user actions.
- Add capability to probe the rpmsg_char device at the initiative of the remote processor
 (rpmsg service announcement mechanism).
  This allows platforms based on the rpmsg virtio backend to create the /dev/rpmgX interface with
  a rpmsg name service announcement.

Subsets:
  - Extract the control part of the char dev and create the rpmsg_ctrl.c file (patches 1 to 6)
  - Introduce the "rpmsg-raw" channel in rpmsg_char(patches 7 to 10)
  - Introduce the RPMSG_CREATE_DEV_IOCTL IOCTL and RPMSG_DESTROY_DEV_IOCTL to instantiate RPMsg
    devices (patch 11)
    The application can then create or release a channel by specifying:
       - the name service of the device to instantiate.   
       - the source address.
       - the destination address.

This series has be applied and tested on 'commit e783362eb54c ("Linux 5.17-rc1") +
rpmsg_char cdev release fixes [2][3]

[1] https://lkml.org/lkml/2021/12/7/186
[2] https://lkml.org/lkml/2022/1/10/1129
[3] https://lkml.org/lkml/2022/1/10/1130

Arnaud Pouliquen (11):
  rpmsg: char: Export eptdev create and destroy functions
  rpmsg: Create the rpmsg class in core instead of in rpmsg char
  rpmsg: Move the rpmsg control device from rpmsg_char to rpmsg_ctrl
  arm: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
  RISC-V: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
  arm64: defconfig: Config that had RPMSG_CHAR now gets RPMSG_CTRL
  rpmsg: Update rpmsg_chrdev_register_device function
  rpmsg: char: Refactor rpmsg_chrdev_eptdev_create function
  rpmsg: char: Add possibility to use default endpoint of the rpmsg
    device
  rpmsg: char: Introduce the "rpmsg-raw" channel
  rpmsg: ctrl: Introduce new RPMSG_CREATE/RELEASE_DEV_IOCTL controls

 arch/arm/configs/qcom_defconfig   |   1 +
 arch/arm64/configs/defconfig      |   1 +
 arch/riscv/configs/defconfig      |   1 +
 arch/riscv/configs/rv32_defconfig |   1 +
 drivers/rpmsg/Kconfig             |   8 +
 drivers/rpmsg/Makefile            |   1 +
 drivers/rpmsg/qcom_glink_native.c |   2 +-
 drivers/rpmsg/qcom_smd.c          |   2 +-
 drivers/rpmsg/rpmsg_char.c        | 231 +++++++++++-----------------
 drivers/rpmsg/rpmsg_char.h        |  46 ++++++
 drivers/rpmsg/rpmsg_core.c        |  15 +-
 drivers/rpmsg/rpmsg_ctrl.c        | 243 ++++++++++++++++++++++++++++++
 drivers/rpmsg/rpmsg_internal.h    |  10 +-
 drivers/rpmsg/virtio_rpmsg_bus.c  |   2 +-
 include/uapi/linux/rpmsg.h        |  10 ++
 15 files changed, 419 insertions(+), 155 deletions(-)
 create mode 100644 drivers/rpmsg/rpmsg_char.h
 create mode 100644 drivers/rpmsg/rpmsg_ctrl.c

Comments

Philipp Rossak Feb. 23, 2022, 9:28 p.m. UTC | #1
Hi Arnaud,

thanks for working on this! I'm currently testing/using this patch 
series on my imx7d project because it adds the capability that the 
remote processor can register it's endpoints dynamically (as mentioned 
in the objectives).

After a few tests, debugging, and checking the openamp specification [1] 
I think that you missed the second ns_announcement that should be sent 
from linux master to the slave after it created the channel/endpoint. 
Without this second announcement the remote processor is not able to 
send messages to the linux master because it doesn't know the 
destination address until it receives a message from the linux master.

Cheers,
Philipp


[1]: 
https://github.com/OpenAMP/open-amp/blob/main/docs/img/coprocessor-rpmsg-ns.png

On 24.01.22 11:25, Arnaud Pouliquen wrote:
> Updates from V8 [1]:
> - rebase on 5.17-rc1 + rpmsg char cdev release fixes[2][3]
> - updates based on Bjorn Andersson's comments:
>    - remove rpmsg_create_default_ept API, set directly the ept->priv in rpmsg_chrdev_probe
>      function.
>    - rework commit message in [8/9]rpmsg: char: Introduce the "rpmsg-raw" channel
> 
> Patchset description:
> 
> The current rpmsg_char module implements a /dev/rpmsg_ctrl interface that provides the ability to
> instantiate char devices (/dev/rpmsgX) associated with an rpmsg endpoint for communication with the
> remote processor.
> This implementation fits with QCOM rpmsg backend but not with the magement by chanel implemented
> in the generic rpmsg virtio backend.
> This series restructures the rpmsg_char driver to decorrelate the control part from the data part
> in order to improve its compatible with the rpmsg virtio backend.
> 
> Objective:
> - Expose a /dev/rpmsg_ctrlX interface for the application that is no longer dedicated to the
>    rpmsg_char but generalized to all rpmsg services. This offers capability to create and destroy
>    rpmsg channels from a user's application initiative (using the new RPMSG_CREATE_DEV_IOCTL and
>    RPMSG_DESTROY_DEV_IOCTL controls).
>    An application will be able to create/establish an rpmsg communication channel to communicate
>    with the remote processor, and not only wait the remote processor initiative.
>    This is interesting for example to establish a temporary communication link for diagnosis,
>    calibration, debugging... or instantiate  new data flows on some user actions.
> - Add capability to probe the rpmsg_char device at the initiative of the remote processor
>   (rpmsg service announcement mechanism).
>    This allows platforms based on the rpmsg virtio backend to create the /dev/rpmgX interface with
>    a rpmsg name service announcement.
> 
> Subsets:
>    - Extract the control part of the char dev and create the rpmsg_ctrl.c file (patches 1 to 6)
>    - Introduce the "rpmsg-raw" channel in rpmsg_char(patches 7 to 10)
>    - Introduce the RPMSG_CREATE_DEV_IOCTL IOCTL and RPMSG_DESTROY_DEV_IOCTL to instantiate RPMsg
>      devices (patch 11)
>      The application can then create or release a channel by specifying:
>         - the name service of the device to instantiate.
>         - the source address.
>         - the destination address.
> 
> This series has be applied and tested on 'commit e783362eb54c ("Linux 5.17-rc1") +
> rpmsg_char cdev release fixes [2][3]
> 
> [1] https://lkml.org/lkml/2021/12/7/186
> [2] https://lkml.org/lkml/2022/1/10/1129
> [3] https://lkml.org/lkml/2022/1/10/1130
> 
> Arnaud Pouliquen (11):
>    rpmsg: char: Export eptdev create and destroy functions
>    rpmsg: Create the rpmsg class in core instead of in rpmsg char
>    rpmsg: Move the rpmsg control device from rpmsg_char to rpmsg_ctrl
>    arm: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
>    RISC-V: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
>    arm64: defconfig: Config that had RPMSG_CHAR now gets RPMSG_CTRL
>    rpmsg: Update rpmsg_chrdev_register_device function
>    rpmsg: char: Refactor rpmsg_chrdev_eptdev_create function
>    rpmsg: char: Add possibility to use default endpoint of the rpmsg
>      device
>    rpmsg: char: Introduce the "rpmsg-raw" channel
>    rpmsg: ctrl: Introduce new RPMSG_CREATE/RELEASE_DEV_IOCTL controls
> 
>   arch/arm/configs/qcom_defconfig   |   1 +
>   arch/arm64/configs/defconfig      |   1 +
>   arch/riscv/configs/defconfig      |   1 +
>   arch/riscv/configs/rv32_defconfig |   1 +
>   drivers/rpmsg/Kconfig             |   8 +
>   drivers/rpmsg/Makefile            |   1 +
>   drivers/rpmsg/qcom_glink_native.c |   2 +-
>   drivers/rpmsg/qcom_smd.c          |   2 +-
>   drivers/rpmsg/rpmsg_char.c        | 231 +++++++++++-----------------
>   drivers/rpmsg/rpmsg_char.h        |  46 ++++++
>   drivers/rpmsg/rpmsg_core.c        |  15 +-
>   drivers/rpmsg/rpmsg_ctrl.c        | 243 ++++++++++++++++++++++++++++++
>   drivers/rpmsg/rpmsg_internal.h    |  10 +-
>   drivers/rpmsg/virtio_rpmsg_bus.c  |   2 +-
>   include/uapi/linux/rpmsg.h        |  10 ++
>   15 files changed, 419 insertions(+), 155 deletions(-)
>   create mode 100644 drivers/rpmsg/rpmsg_char.h
>   create mode 100644 drivers/rpmsg/rpmsg_ctrl.c
>
Arnaud POULIQUEN Feb. 24, 2022, 8:29 a.m. UTC | #2
Hi Philipp,

On 2/23/22 22:28, Philipp Rossak wrote:
> Hi Arnaud,
> 
> thanks for working on this! I'm currently testing/using this patch
> series on my imx7d project because it adds the capability that the
> remote processor can register it's endpoints dynamically (as mentioned
> in the objectives).

Thanks for your feedback on this work! 
Don't hesitate to add your tested-by, this help maintainers for the reviews. 

> 
> After a few tests, debugging, and checking the openamp specification [1]
> I think that you missed the second ns_announcement that should be sent
> from linux master to the slave after it created the channel/endpoint.
> Without this second announcement the remote processor is not able to
> send messages to the linux master because it doesn't know the
> destination address until it receives a message from the linux master.

Yes I detected this issues, it is not related to the series
but to the remoteproc_virtio backend.

As you mentioned, after the ns announcement from Linux, the remote processor
send first messages. But the Linux virtio does not do the match between the
local channel created and the remote endpoint.

This is a feature that is missing in the rpmsg virtio, and perhaps in rpmsg protocol
itself (a ns annoucement ack message or something similar).


A fix for the remoteproc virtio is available here:
https://github.com/arnopo/meta-st-stm32mp-oss/commit/3e57fe73bd19c9bb835ac5a118e50727758b0b96

Don't hesitate to give me feedback on the fix, if you test it.

I plan to propose the fix after this series.    

Thanks,
Arnaud

> 
> Cheers,
> Philipp
> 
> 
> [1]:
> https://github.com/OpenAMP/open-amp/blob/main/docs/img/coprocessor-rpmsg-ns.png
> 
> 
> On 24.01.22 11:25, Arnaud Pouliquen wrote:
>> Updates from V8 [1]:
>> - rebase on 5.17-rc1 + rpmsg char cdev release fixes[2][3]
>> - updates based on Bjorn Andersson's comments:
>>    - remove rpmsg_create_default_ept API, set directly the ept->priv
>> in rpmsg_chrdev_probe
>>      function.
>>    - rework commit message in [8/9]rpmsg: char: Introduce the
>> "rpmsg-raw" channel
>>
>> Patchset description:
>>
>> The current rpmsg_char module implements a /dev/rpmsg_ctrl interface
>> that provides the ability to
>> instantiate char devices (/dev/rpmsgX) associated with an rpmsg
>> endpoint for communication with the
>> remote processor.
>> This implementation fits with QCOM rpmsg backend but not with the
>> magement by chanel implemented
>> in the generic rpmsg virtio backend.
>> This series restructures the rpmsg_char driver to decorrelate the
>> control part from the data part
>> in order to improve its compatible with the rpmsg virtio backend.
>>
>> Objective:
>> - Expose a /dev/rpmsg_ctrlX interface for the application that is no
>> longer dedicated to the
>>    rpmsg_char but generalized to all rpmsg services. This offers
>> capability to create and destroy
>>    rpmsg channels from a user's application initiative (using the new
>> RPMSG_CREATE_DEV_IOCTL and
>>    RPMSG_DESTROY_DEV_IOCTL controls).
>>    An application will be able to create/establish an rpmsg
>> communication channel to communicate
>>    with the remote processor, and not only wait the remote processor
>> initiative.
>>    This is interesting for example to establish a temporary
>> communication link for diagnosis,
>>    calibration, debugging... or instantiate  new data flows on some
>> user actions.
>> - Add capability to probe the rpmsg_char device at the initiative of
>> the remote processor
>>   (rpmsg service announcement mechanism).
>>    This allows platforms based on the rpmsg virtio backend to create
>> the /dev/rpmgX interface with
>>    a rpmsg name service announcement.
>>
>> Subsets:
>>    - Extract the control part of the char dev and create the
>> rpmsg_ctrl.c file (patches 1 to 6)
>>    - Introduce the "rpmsg-raw" channel in rpmsg_char(patches 7 to 10)
>>    - Introduce the RPMSG_CREATE_DEV_IOCTL IOCTL and
>> RPMSG_DESTROY_DEV_IOCTL to instantiate RPMsg
>>      devices (patch 11)
>>      The application can then create or release a channel by specifying:
>>         - the name service of the device to instantiate.
>>         - the source address.
>>         - the destination address.
>>
>> This series has be applied and tested on 'commit e783362eb54c ("Linux
>> 5.17-rc1") +
>> rpmsg_char cdev release fixes [2][3]
>>
>> [1] https://lkml.org/lkml/2021/12/7/186
>> [2] https://lkml.org/lkml/2022/1/10/1129
>> [3] https://lkml.org/lkml/2022/1/10/1130
>>
>> Arnaud Pouliquen (11):
>>    rpmsg: char: Export eptdev create and destroy functions
>>    rpmsg: Create the rpmsg class in core instead of in rpmsg char
>>    rpmsg: Move the rpmsg control device from rpmsg_char to rpmsg_ctrl
>>    arm: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
>>    RISC-V: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
>>    arm64: defconfig: Config that had RPMSG_CHAR now gets RPMSG_CTRL
>>    rpmsg: Update rpmsg_chrdev_register_device function
>>    rpmsg: char: Refactor rpmsg_chrdev_eptdev_create function
>>    rpmsg: char: Add possibility to use default endpoint of the rpmsg
>>      device
>>    rpmsg: char: Introduce the "rpmsg-raw" channel
>>    rpmsg: ctrl: Introduce new RPMSG_CREATE/RELEASE_DEV_IOCTL controls
>>
>>   arch/arm/configs/qcom_defconfig   |   1 +
>>   arch/arm64/configs/defconfig      |   1 +
>>   arch/riscv/configs/defconfig      |   1 +
>>   arch/riscv/configs/rv32_defconfig |   1 +
>>   drivers/rpmsg/Kconfig             |   8 +
>>   drivers/rpmsg/Makefile            |   1 +
>>   drivers/rpmsg/qcom_glink_native.c |   2 +-
>>   drivers/rpmsg/qcom_smd.c          |   2 +-
>>   drivers/rpmsg/rpmsg_char.c        | 231 +++++++++++-----------------
>>   drivers/rpmsg/rpmsg_char.h        |  46 ++++++
>>   drivers/rpmsg/rpmsg_core.c        |  15 +-
>>   drivers/rpmsg/rpmsg_ctrl.c        | 243 ++++++++++++++++++++++++++++++
>>   drivers/rpmsg/rpmsg_internal.h    |  10 +-
>>   drivers/rpmsg/virtio_rpmsg_bus.c  |   2 +-
>>   include/uapi/linux/rpmsg.h        |  10 ++
>>   15 files changed, 419 insertions(+), 155 deletions(-)
>>   create mode 100644 drivers/rpmsg/rpmsg_char.h
>>   create mode 100644 drivers/rpmsg/rpmsg_ctrl.c
>>
Philipp Rossak Feb. 25, 2022, 9:45 p.m. UTC | #3
Hi Arnaud,

On 24.02.22 09:29, Arnaud POULIQUEN wrote:
> Hi Philipp,
> 
> On 2/23/22 22:28, Philipp Rossak wrote:
>> Hi Arnaud,
>>
>> thanks for working on this! I'm currently testing/using this patch
>> series on my imx7d project because it adds the capability that the
>> remote processor can register it's endpoints dynamically (as mentioned
>> in the objectives).
> 
> Thanks for your feedback on this work!
> Don't hesitate to add your tested-by, this help maintainers for the reviews.
> 
I will do this.
>>
>> After a few tests, debugging, and checking the openamp specification [1]
>> I think that you missed the second ns_announcement that should be sent
>> from linux master to the slave after it created the channel/endpoint.
>> Without this second announcement the remote processor is not able to
>> send messages to the linux master because it doesn't know the
>> destination address until it receives a message from the linux master.
> 
> Yes I detected this issues, it is not related to the series
> but to the remoteproc_virtio backend.
> 
> As you mentioned, after the ns announcement from Linux, the remote processor
> send first messages. But the Linux virtio does not do the match between the
> local channel created and the remote endpoint.
> 

I'm not sure if we talk about the same. I'm basically talking about the 
dynamic binding, not dynamic endpoint creation.
I think I already found the issue. I will try to get a bit more into detail.

1. Linux: starts co-processor via remoteproc
2. co-processor: boots and reaches the point where it creates the 
endpoint like it is done in this ST example[1].
Be aware the src address is RPMSG_ADDR_ANY
3. co-processor: reaches the point where it sends the ns_announcement to 
linux ns endpoint
4. linux: receives the ns announcment, creates the channel, bindes the 
endpoint and checks here [2] if the source address is not RPMSG_ADDR_ANY 
and in this case it is not sending a ns_announcement (that's the issue 
when we use dynamic endpoints)
5. linux: according the openamp spec [3] it should now send the 
ns_announcement to the co-processor (slave)
6. co-processor: should receive the ns announcement and binds now the 
endpoint
7. co-processor: can now send messages to linux

This is basically what I'm expecting.


Do you think this is a bug or is the dynamic endpoint binding not 
handled? This line is there since ever [4] ...

Any other thoughts about this?

> This is a feature that is missing in the rpmsg virtio, and perhaps in rpmsg protocol
> itself (a ns annoucement ack message or something similar).
> 
> 
> A fix for the remoteproc virtio is available here:
> https://github.com/arnopo/meta-st-stm32mp-oss/commit/3e57fe73bd19c9bb835ac5a118e50727758b0b96
> 
> Don't hesitate to give me feedback on the fix, if you test it.

I added it to my branch and till now I don't see any side effects
> 
> I plan to propose the fix after this series.
> 
> Thanks,
> Arnaud
> 
>>
>> Cheers,
>> Philipp
>>

Cheers,
Philipp

[1]: 
https://github.com/STMicroelectronics/STM32CubeMP1/blob/master/Projects/STM32MP157C-DK2/Applications/OpenAMP/OpenAMP_raw/Src/openamp.c#L242

[2]: 
https://elixir.bootlin.com/linux/v5.17-rc5/source/drivers/rpmsg/virtio_rpmsg_bus.c#L425

[3]: 
https://github.com/OpenAMP/open-amp/blob/main/docs/img/coprocessor-rpmsg-ns.png

[4]: 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bcabbccabffe7326f046f25737ba1084f463c65c
Arnaud POULIQUEN Feb. 28, 2022, 9:02 a.m. UTC | #4
Hi Philipp,

On 2/25/22 22:45, Philipp Rossak wrote:
> Hi Arnaud,
> 
> On 24.02.22 09:29, Arnaud POULIQUEN wrote:
>> Hi Philipp,
>>
>> On 2/23/22 22:28, Philipp Rossak wrote:
>>> Hi Arnaud,
>>>
>>> thanks for working on this! I'm currently testing/using this patch
>>> series on my imx7d project because it adds the capability that the
>>> remote processor can register it's endpoints dynamically (as mentioned
>>> in the objectives).
>>
>> Thanks for your feedback on this work!
>> Don't hesitate to add your tested-by, this help maintainers for the
>> reviews.
>>
> I will do this.
>>>
>>> After a few tests, debugging, and checking the openamp specification [1]
>>> I think that you missed the second ns_announcement that should be sent
>>> from linux master to the slave after it created the channel/endpoint.
>>> Without this second announcement the remote processor is not able to
>>> send messages to the linux master because it doesn't know the
>>> destination address until it receives a message from the linux master.
>>
>> Yes I detected this issues, it is not related to the series
>> but to the remoteproc_virtio backend.
>>
>> As you mentioned, after the ns announcement from Linux, the remote
>> processor
>> send first messages. But the Linux virtio does not do the match
>> between the
>> local channel created and the remote endpoint.
>>
> 
> I'm not sure if we talk about the same. I'm basically talking about the
> dynamic binding, not dynamic endpoint creation.

Regarding your following description, yes it is not exactly the same issue.
  

> I think I already found the issue. I will try to get a bit more into
> detail.
> 
> 1. Linux: starts co-processor via remoteproc
> 2. co-processor: boots and reaches the point where it creates the
> endpoint like it is done in this ST example[1].
> Be aware the src address is RPMSG_ADDR_ANY
> 3. co-processor: reaches the point where it sends the ns_announcement to
> linux ns endpoint
> 4. linux: receives the ns announcment, creates the channel, bindes the
> endpoint and checks here [2] if the source address is not RPMSG_ADDR_ANY
> and in this case it is not sending a ns_announcement (that's the issue
> when we use dynamic endpoints)

The ns annoucement is used to notify the remote processor that a new channel
has been created locally. Today the ns anoucement is not used to inform that

The local endpoint has been binded.

This behavior is something that as already been identified as a limitation in
the virtio rpmsg.

Xiang Xiao had proposed some time ago a mechanism for the OpenAMP [5], the
Linux part is missing. We need a common solution between Linux and OpenAMP, but
that also compatible with legacy. 
From MPOV Xiang's approach seem a good starting point.  
If you are interesting on working on this enhancement of the rpmsg virtio, feel
free to do so.

> 5. linux: according the openamp spec [3] it should now send the
> ns_announcement to the co-processor (slave)
> 6. co-processor: should receive the ns announcement and binds now the
> endpoint
> 7. co-processor: can now send messages to linux
> 
> This is basically what I'm expecting.
> 
> 
> Do you think this is a bug or is the dynamic endpoint binding not
> handled? This line is there since ever [4] ...

As you mentioned this is a legacy limitation that should be addressed.
It is not related to this work.

Thanks,
Arnaud

> 
> Any other thoughts about this?
> 
>> This is a feature that is missing in the rpmsg virtio, and perhaps in
>> rpmsg protocol
>> itself (a ns annoucement ack message or something similar).
>>
>>
>> A fix for the remoteproc virtio is available here:
>> https://github.com/arnopo/meta-st-stm32mp-oss/commit/3e57fe73bd19c9bb835ac5a118e50727758b0b96
>>
>>
>> Don't hesitate to give me feedback on the fix, if you test it.
> 
> I added it to my branch and till now I don't see any side effects
>>
>> I plan to propose the fix after this series.
>>
>> Thanks,
>> Arnaud
>>
>>>
>>> Cheers,
>>> Philipp
>>>
> 
> Cheers,
> Philipp
> 
> [1]:
> https://github.com/STMicroelectronics/STM32CubeMP1/blob/master/Projects/STM32MP157C-DK2/Applications/OpenAMP/OpenAMP_raw/Src/openamp.c#L242
> 
> 
> [2]:
> https://elixir.bootlin.com/linux/v5.17-rc5/source/drivers/rpmsg/virtio_rpmsg_bus.c#L425
> 
> 
> [3]:
> https://github.com/OpenAMP/open-amp/blob/main/docs/img/coprocessor-rpmsg-ns.png
> 
> 
> [4]:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bcabbccabffe7326f046f25737ba1084f463c65c
> 

[5] https://github.com/OpenAMP/open-amp/pull/160
Arnaud POULIQUEN March 24, 2022, 5:36 p.m. UTC | #5
Hi Bjorn,

On 1/24/22 11:25, Arnaud Pouliquen wrote:
> Updates from V8 [1]:
> - rebase on 5.17-rc1 + rpmsg char cdev release fixes[2][3]
> - updates based on Bjorn Andersson's comments:
>   - remove rpmsg_create_default_ept API, set directly the ept->priv in rpmsg_chrdev_probe
>     function.
>   - rework commit message in [8/9]rpmsg: char: Introduce the "rpmsg-raw" channel
> 
> Patchset description:
> 
> The current rpmsg_char module implements a /dev/rpmsg_ctrl interface that provides the ability to
> instantiate char devices (/dev/rpmsgX) associated with an rpmsg endpoint for communication with the
> remote processor.
> This implementation fits with QCOM rpmsg backend but not with the magement by chanel implemented
> in the generic rpmsg virtio backend.
> This series restructures the rpmsg_char driver to decorrelate the control part from the data part
> in order to improve its compatible with the rpmsg virtio backend.
> 
> Objective:
> - Expose a /dev/rpmsg_ctrlX interface for the application that is no longer dedicated to the
>   rpmsg_char but generalized to all rpmsg services. This offers capability to create and destroy
>   rpmsg channels from a user's application initiative (using the new RPMSG_CREATE_DEV_IOCTL and
>   RPMSG_DESTROY_DEV_IOCTL controls).
>   An application will be able to create/establish an rpmsg communication channel to communicate
>   with the remote processor, and not only wait the remote processor initiative.
>   This is interesting for example to establish a temporary communication link for diagnosis,
>   calibration, debugging... or instantiate  new data flows on some user actions.
> - Add capability to probe the rpmsg_char device at the initiative of the remote processor
>  (rpmsg service announcement mechanism).
>   This allows platforms based on the rpmsg virtio backend to create the /dev/rpmgX interface with
>   a rpmsg name service announcement.
> 
> Subsets:
>   - Extract the control part of the char dev and create the rpmsg_ctrl.c file (patches 1 to 6)
>   - Introduce the "rpmsg-raw" channel in rpmsg_char(patches 7 to 10)
>   - Introduce the RPMSG_CREATE_DEV_IOCTL IOCTL and RPMSG_DESTROY_DEV_IOCTL to instantiate RPMsg
>     devices (patch 11)
>     The application can then create or release a channel by specifying:
>        - the name service of the device to instantiate.   
>        - the source address.
>        - the destination address.
> 
> This series has be applied and tested on 'commit e783362eb54c ("Linux 5.17-rc1") +
> rpmsg_char cdev release fixes [2][3]
> 
> [1] https://lkml.org/lkml/2021/12/7/186
> [2] https://lkml.org/lkml/2022/1/10/1129
> [3] https://lkml.org/lkml/2022/1/10/1130
> 
> Arnaud Pouliquen (11):
>   rpmsg: char: Export eptdev create and destroy functions
>   rpmsg: Create the rpmsg class in core instead of in rpmsg char
>   rpmsg: Move the rpmsg control device from rpmsg_char to rpmsg_ctrl


>   arm: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
>   RISC-V: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
>   arm64: defconfig: Config that had RPMSG_CHAR now gets RPMSG_CTRL

Thank you for merging this series!

I can't see in the "for next" branch[1] the 3 patches above that update configs
Are you expecting a specific action from me?   

[1]https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git

Regards,
Arnaud

>   rpmsg: Update rpmsg_chrdev_register_device function
>   rpmsg: char: Refactor rpmsg_chrdev_eptdev_create function
>   rpmsg: char: Add possibility to use default endpoint of the rpmsg
>     device
>   rpmsg: char: Introduce the "rpmsg-raw" channel
>   rpmsg: ctrl: Introduce new RPMSG_CREATE/RELEASE_DEV_IOCTL controls
> 
>  arch/arm/configs/qcom_defconfig   |   1 +
>  arch/arm64/configs/defconfig      |   1 +
>  arch/riscv/configs/defconfig      |   1 +
>  arch/riscv/configs/rv32_defconfig |   1 +
>  drivers/rpmsg/Kconfig             |   8 +
>  drivers/rpmsg/Makefile            |   1 +
>  drivers/rpmsg/qcom_glink_native.c |   2 +-
>  drivers/rpmsg/qcom_smd.c          |   2 +-
>  drivers/rpmsg/rpmsg_char.c        | 231 +++++++++++-----------------
>  drivers/rpmsg/rpmsg_char.h        |  46 ++++++
>  drivers/rpmsg/rpmsg_core.c        |  15 +-
>  drivers/rpmsg/rpmsg_ctrl.c        | 243 ++++++++++++++++++++++++++++++
>  drivers/rpmsg/rpmsg_internal.h    |  10 +-
>  drivers/rpmsg/virtio_rpmsg_bus.c  |   2 +-
>  include/uapi/linux/rpmsg.h        |  10 ++
>  15 files changed, 419 insertions(+), 155 deletions(-)
>  create mode 100644 drivers/rpmsg/rpmsg_char.h
>  create mode 100644 drivers/rpmsg/rpmsg_ctrl.c
>
Mathieu Poirier March 25, 2022, 3:59 p.m. UTC | #6
On Thu, Mar 24, 2022 at 06:36:23PM +0100, Arnaud POULIQUEN wrote:
> Hi Bjorn,
> 
> On 1/24/22 11:25, Arnaud Pouliquen wrote:
> > Updates from V8 [1]:
> > - rebase on 5.17-rc1 + rpmsg char cdev release fixes[2][3]
> > - updates based on Bjorn Andersson's comments:
> >   - remove rpmsg_create_default_ept API, set directly the ept->priv in rpmsg_chrdev_probe
> >     function.
> >   - rework commit message in [8/9]rpmsg: char: Introduce the "rpmsg-raw" channel
> > 
> > Patchset description:
> > 
> > The current rpmsg_char module implements a /dev/rpmsg_ctrl interface that provides the ability to
> > instantiate char devices (/dev/rpmsgX) associated with an rpmsg endpoint for communication with the
> > remote processor.
> > This implementation fits with QCOM rpmsg backend but not with the magement by chanel implemented
> > in the generic rpmsg virtio backend.
> > This series restructures the rpmsg_char driver to decorrelate the control part from the data part
> > in order to improve its compatible with the rpmsg virtio backend.
> > 
> > Objective:
> > - Expose a /dev/rpmsg_ctrlX interface for the application that is no longer dedicated to the
> >   rpmsg_char but generalized to all rpmsg services. This offers capability to create and destroy
> >   rpmsg channels from a user's application initiative (using the new RPMSG_CREATE_DEV_IOCTL and
> >   RPMSG_DESTROY_DEV_IOCTL controls).
> >   An application will be able to create/establish an rpmsg communication channel to communicate
> >   with the remote processor, and not only wait the remote processor initiative.
> >   This is interesting for example to establish a temporary communication link for diagnosis,
> >   calibration, debugging... or instantiate  new data flows on some user actions.
> > - Add capability to probe the rpmsg_char device at the initiative of the remote processor
> >  (rpmsg service announcement mechanism).
> >   This allows platforms based on the rpmsg virtio backend to create the /dev/rpmgX interface with
> >   a rpmsg name service announcement.
> > 
> > Subsets:
> >   - Extract the control part of the char dev and create the rpmsg_ctrl.c file (patches 1 to 6)
> >   - Introduce the "rpmsg-raw" channel in rpmsg_char(patches 7 to 10)
> >   - Introduce the RPMSG_CREATE_DEV_IOCTL IOCTL and RPMSG_DESTROY_DEV_IOCTL to instantiate RPMsg
> >     devices (patch 11)
> >     The application can then create or release a channel by specifying:
> >        - the name service of the device to instantiate.   
> >        - the source address.
> >        - the destination address.
> > 
> > This series has be applied and tested on 'commit e783362eb54c ("Linux 5.17-rc1") +
> > rpmsg_char cdev release fixes [2][3]
> > 
> > [1] https://lkml.org/lkml/2021/12/7/186
> > [2] https://lkml.org/lkml/2022/1/10/1129
> > [3] https://lkml.org/lkml/2022/1/10/1130
> > 
> > Arnaud Pouliquen (11):
> >   rpmsg: char: Export eptdev create and destroy functions
> >   rpmsg: Create the rpmsg class in core instead of in rpmsg char
> >   rpmsg: Move the rpmsg control device from rpmsg_char to rpmsg_ctrl
> 
> 
> >   arm: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
> >   RISC-V: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
> >   arm64: defconfig: Config that had RPMSG_CHAR now gets RPMSG_CTRL
> 
> Thank you for merging this series!
> 
> I can't see in the "for next" branch[1] the 3 patches above that update configs
> Are you expecting a specific action from me?   

Those patches will need to go through the Arm, RISC-V and arm64 subsystems.  The
mailing list for those subsystems has been CC'ed but that isn't enough to get
the maintainers' attention.  

I suggest sending another patchset with those 3 patches that CC the maintainers
directly.  For the Arm patch I suggest adding Linus Walleij.

Thanks,
Mathieu

> 
> [1]https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git
> 
> Regards,
> Arnaud
> 
> >   rpmsg: Update rpmsg_chrdev_register_device function
> >   rpmsg: char: Refactor rpmsg_chrdev_eptdev_create function
> >   rpmsg: char: Add possibility to use default endpoint of the rpmsg
> >     device
> >   rpmsg: char: Introduce the "rpmsg-raw" channel
> >   rpmsg: ctrl: Introduce new RPMSG_CREATE/RELEASE_DEV_IOCTL controls
> > 
> >  arch/arm/configs/qcom_defconfig   |   1 +
> >  arch/arm64/configs/defconfig      |   1 +
> >  arch/riscv/configs/defconfig      |   1 +
> >  arch/riscv/configs/rv32_defconfig |   1 +
> >  drivers/rpmsg/Kconfig             |   8 +
> >  drivers/rpmsg/Makefile            |   1 +
> >  drivers/rpmsg/qcom_glink_native.c |   2 +-
> >  drivers/rpmsg/qcom_smd.c          |   2 +-
> >  drivers/rpmsg/rpmsg_char.c        | 231 +++++++++++-----------------
> >  drivers/rpmsg/rpmsg_char.h        |  46 ++++++
> >  drivers/rpmsg/rpmsg_core.c        |  15 +-
> >  drivers/rpmsg/rpmsg_ctrl.c        | 243 ++++++++++++++++++++++++++++++
> >  drivers/rpmsg/rpmsg_internal.h    |  10 +-
> >  drivers/rpmsg/virtio_rpmsg_bus.c  |   2 +-
> >  include/uapi/linux/rpmsg.h        |  10 ++
> >  15 files changed, 419 insertions(+), 155 deletions(-)
> >  create mode 100644 drivers/rpmsg/rpmsg_char.h
> >  create mode 100644 drivers/rpmsg/rpmsg_ctrl.c
> >
Arnaud POULIQUEN March 25, 2022, 5:05 p.m. UTC | #7
On 3/25/22 16:59, Mathieu Poirier wrote:
> On Thu, Mar 24, 2022 at 06:36:23PM +0100, Arnaud POULIQUEN wrote:
>> Hi Bjorn,
>>
>> On 1/24/22 11:25, Arnaud Pouliquen wrote:
>>> Updates from V8 [1]:
>>> - rebase on 5.17-rc1 + rpmsg char cdev release fixes[2][3]
>>> - updates based on Bjorn Andersson's comments:
>>>   - remove rpmsg_create_default_ept API, set directly the ept->priv in rpmsg_chrdev_probe
>>>     function.
>>>   - rework commit message in [8/9]rpmsg: char: Introduce the "rpmsg-raw" channel
>>>
>>> Patchset description:
>>>
>>> The current rpmsg_char module implements a /dev/rpmsg_ctrl interface that provides the ability to
>>> instantiate char devices (/dev/rpmsgX) associated with an rpmsg endpoint for communication with the
>>> remote processor.
>>> This implementation fits with QCOM rpmsg backend but not with the magement by chanel implemented
>>> in the generic rpmsg virtio backend.
>>> This series restructures the rpmsg_char driver to decorrelate the control part from the data part
>>> in order to improve its compatible with the rpmsg virtio backend.
>>>
>>> Objective:
>>> - Expose a /dev/rpmsg_ctrlX interface for the application that is no longer dedicated to the
>>>   rpmsg_char but generalized to all rpmsg services. This offers capability to create and destroy
>>>   rpmsg channels from a user's application initiative (using the new RPMSG_CREATE_DEV_IOCTL and
>>>   RPMSG_DESTROY_DEV_IOCTL controls).
>>>   An application will be able to create/establish an rpmsg communication channel to communicate
>>>   with the remote processor, and not only wait the remote processor initiative.
>>>   This is interesting for example to establish a temporary communication link for diagnosis,
>>>   calibration, debugging... or instantiate  new data flows on some user actions.
>>> - Add capability to probe the rpmsg_char device at the initiative of the remote processor
>>>  (rpmsg service announcement mechanism).
>>>   This allows platforms based on the rpmsg virtio backend to create the /dev/rpmgX interface with
>>>   a rpmsg name service announcement.
>>>
>>> Subsets:
>>>   - Extract the control part of the char dev and create the rpmsg_ctrl.c file (patches 1 to 6)
>>>   - Introduce the "rpmsg-raw" channel in rpmsg_char(patches 7 to 10)
>>>   - Introduce the RPMSG_CREATE_DEV_IOCTL IOCTL and RPMSG_DESTROY_DEV_IOCTL to instantiate RPMsg
>>>     devices (patch 11)
>>>     The application can then create or release a channel by specifying:
>>>        - the name service of the device to instantiate.   
>>>        - the source address.
>>>        - the destination address.
>>>
>>> This series has be applied and tested on 'commit e783362eb54c ("Linux 5.17-rc1") +
>>> rpmsg_char cdev release fixes [2][3]
>>>
>>> [1] https://lkml.org/lkml/2021/12/7/186
>>> [2] https://lkml.org/lkml/2022/1/10/1129
>>> [3] https://lkml.org/lkml/2022/1/10/1130
>>>
>>> Arnaud Pouliquen (11):
>>>   rpmsg: char: Export eptdev create and destroy functions
>>>   rpmsg: Create the rpmsg class in core instead of in rpmsg char
>>>   rpmsg: Move the rpmsg control device from rpmsg_char to rpmsg_ctrl
>>
>>
>>>   arm: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
>>>   RISC-V: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
>>>   arm64: defconfig: Config that had RPMSG_CHAR now gets RPMSG_CTRL
>>
>> Thank you for merging this series!
>>
>> I can't see in the "for next" branch[1] the 3 patches above that update configs
>> Are you expecting a specific action from me?   
> 
> Those patches will need to go through the Arm, RISC-V and arm64 subsystems.  The
> mailing list for those subsystems has been CC'ed but that isn't enough to get
> the maintainers' attention.  
> 
> I suggest sending another patchset with those 3 patches that CC the maintainers
> directly.  For the Arm patch I suggest adding Linus Walleij.

I will do what you suggest. 

My concerns in this case is about the scheduling of the integration.
I suppose that sending a second patchset for configs requests that the
rpmsg char series is first applied
But on the other hand this may lead to some failures as the RPMSG_CTRL is now
needed to create the /dev/rpmsg_ctrl0

so probably, I need to do this as fixup patch.

FYI the RISC-V patch as been reviewed by Anup Patel

Thanks,
Arnaud

> 
> Thanks,
> Mathieu
> 
>>
>> [1]https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git
>>
>> Regards,
>> Arnaud
>>
>>>   rpmsg: Update rpmsg_chrdev_register_device function
>>>   rpmsg: char: Refactor rpmsg_chrdev_eptdev_create function
>>>   rpmsg: char: Add possibility to use default endpoint of the rpmsg
>>>     device
>>>   rpmsg: char: Introduce the "rpmsg-raw" channel
>>>   rpmsg: ctrl: Introduce new RPMSG_CREATE/RELEASE_DEV_IOCTL controls
>>>
>>>  arch/arm/configs/qcom_defconfig   |   1 +
>>>  arch/arm64/configs/defconfig      |   1 +
>>>  arch/riscv/configs/defconfig      |   1 +
>>>  arch/riscv/configs/rv32_defconfig |   1 +
>>>  drivers/rpmsg/Kconfig             |   8 +
>>>  drivers/rpmsg/Makefile            |   1 +
>>>  drivers/rpmsg/qcom_glink_native.c |   2 +-
>>>  drivers/rpmsg/qcom_smd.c          |   2 +-
>>>  drivers/rpmsg/rpmsg_char.c        | 231 +++++++++++-----------------
>>>  drivers/rpmsg/rpmsg_char.h        |  46 ++++++
>>>  drivers/rpmsg/rpmsg_core.c        |  15 +-
>>>  drivers/rpmsg/rpmsg_ctrl.c        | 243 ++++++++++++++++++++++++++++++
>>>  drivers/rpmsg/rpmsg_internal.h    |  10 +-
>>>  drivers/rpmsg/virtio_rpmsg_bus.c  |   2 +-
>>>  include/uapi/linux/rpmsg.h        |  10 ++
>>>  15 files changed, 419 insertions(+), 155 deletions(-)
>>>  create mode 100644 drivers/rpmsg/rpmsg_char.h
>>>  create mode 100644 drivers/rpmsg/rpmsg_ctrl.c
>>>
Mathieu Poirier March 25, 2022, 5:27 p.m. UTC | #8
On Fri, 25 Mar 2022 at 11:05, Arnaud POULIQUEN
<arnaud.pouliquen@foss.st.com> wrote:
>
>
>
> On 3/25/22 16:59, Mathieu Poirier wrote:
> > On Thu, Mar 24, 2022 at 06:36:23PM +0100, Arnaud POULIQUEN wrote:
> >> Hi Bjorn,
> >>
> >> On 1/24/22 11:25, Arnaud Pouliquen wrote:
> >>> Updates from V8 [1]:
> >>> - rebase on 5.17-rc1 + rpmsg char cdev release fixes[2][3]
> >>> - updates based on Bjorn Andersson's comments:
> >>>   - remove rpmsg_create_default_ept API, set directly the ept->priv in rpmsg_chrdev_probe
> >>>     function.
> >>>   - rework commit message in [8/9]rpmsg: char: Introduce the "rpmsg-raw" channel
> >>>
> >>> Patchset description:
> >>>
> >>> The current rpmsg_char module implements a /dev/rpmsg_ctrl interface that provides the ability to
> >>> instantiate char devices (/dev/rpmsgX) associated with an rpmsg endpoint for communication with the
> >>> remote processor.
> >>> This implementation fits with QCOM rpmsg backend but not with the magement by chanel implemented
> >>> in the generic rpmsg virtio backend.
> >>> This series restructures the rpmsg_char driver to decorrelate the control part from the data part
> >>> in order to improve its compatible with the rpmsg virtio backend.
> >>>
> >>> Objective:
> >>> - Expose a /dev/rpmsg_ctrlX interface for the application that is no longer dedicated to the
> >>>   rpmsg_char but generalized to all rpmsg services. This offers capability to create and destroy
> >>>   rpmsg channels from a user's application initiative (using the new RPMSG_CREATE_DEV_IOCTL and
> >>>   RPMSG_DESTROY_DEV_IOCTL controls).
> >>>   An application will be able to create/establish an rpmsg communication channel to communicate
> >>>   with the remote processor, and not only wait the remote processor initiative.
> >>>   This is interesting for example to establish a temporary communication link for diagnosis,
> >>>   calibration, debugging... or instantiate  new data flows on some user actions.
> >>> - Add capability to probe the rpmsg_char device at the initiative of the remote processor
> >>>  (rpmsg service announcement mechanism).
> >>>   This allows platforms based on the rpmsg virtio backend to create the /dev/rpmgX interface with
> >>>   a rpmsg name service announcement.
> >>>
> >>> Subsets:
> >>>   - Extract the control part of the char dev and create the rpmsg_ctrl.c file (patches 1 to 6)
> >>>   - Introduce the "rpmsg-raw" channel in rpmsg_char(patches 7 to 10)
> >>>   - Introduce the RPMSG_CREATE_DEV_IOCTL IOCTL and RPMSG_DESTROY_DEV_IOCTL to instantiate RPMsg
> >>>     devices (patch 11)
> >>>     The application can then create or release a channel by specifying:
> >>>        - the name service of the device to instantiate.
> >>>        - the source address.
> >>>        - the destination address.
> >>>
> >>> This series has be applied and tested on 'commit e783362eb54c ("Linux 5.17-rc1") +
> >>> rpmsg_char cdev release fixes [2][3]
> >>>
> >>> [1] https://lkml.org/lkml/2021/12/7/186
> >>> [2] https://lkml.org/lkml/2022/1/10/1129
> >>> [3] https://lkml.org/lkml/2022/1/10/1130
> >>>
> >>> Arnaud Pouliquen (11):
> >>>   rpmsg: char: Export eptdev create and destroy functions
> >>>   rpmsg: Create the rpmsg class in core instead of in rpmsg char
> >>>   rpmsg: Move the rpmsg control device from rpmsg_char to rpmsg_ctrl
> >>
> >>
> >>>   arm: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
> >>>   RISC-V: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
> >>>   arm64: defconfig: Config that had RPMSG_CHAR now gets RPMSG_CTRL
> >>
> >> Thank you for merging this series!
> >>
> >> I can't see in the "for next" branch[1] the 3 patches above that update configs
> >> Are you expecting a specific action from me?
> >
> > Those patches will need to go through the Arm, RISC-V and arm64 subsystems.  The
> > mailing list for those subsystems has been CC'ed but that isn't enough to get
> > the maintainers' attention.
> >
> > I suggest sending another patchset with those 3 patches that CC the maintainers
> > directly.  For the Arm patch I suggest adding Linus Walleij.
>
> I will do what you suggest.
>
> My concerns in this case is about the scheduling of the integration.
> I suppose that sending a second patchset for configs requests that the
> rpmsg char series is first applied

Right, but the rpmsg_char series has been applied.

> But on the other hand this may lead to some failures as the RPMSG_CTRL is now
> needed to create the /dev/rpmsg_ctrl0
>

Possibly, but right now there is no other way.

> so probably, I need to do this as fixup patch.
>

Indeed, this can be applied as a fix in rc1.

> FYI the RISC-V patch as been reviewed by Anup Patel
>

... but Anup does not maintain any of the defconfig files.

> Thanks,
> Arnaud
>
> >
> > Thanks,
> > Mathieu
> >
> >>
> >> [1]https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git
> >>
> >> Regards,
> >> Arnaud
> >>
> >>>   rpmsg: Update rpmsg_chrdev_register_device function
> >>>   rpmsg: char: Refactor rpmsg_chrdev_eptdev_create function
> >>>   rpmsg: char: Add possibility to use default endpoint of the rpmsg
> >>>     device
> >>>   rpmsg: char: Introduce the "rpmsg-raw" channel
> >>>   rpmsg: ctrl: Introduce new RPMSG_CREATE/RELEASE_DEV_IOCTL controls
> >>>
> >>>  arch/arm/configs/qcom_defconfig   |   1 +
> >>>  arch/arm64/configs/defconfig      |   1 +
> >>>  arch/riscv/configs/defconfig      |   1 +
> >>>  arch/riscv/configs/rv32_defconfig |   1 +
> >>>  drivers/rpmsg/Kconfig             |   8 +
> >>>  drivers/rpmsg/Makefile            |   1 +
> >>>  drivers/rpmsg/qcom_glink_native.c |   2 +-
> >>>  drivers/rpmsg/qcom_smd.c          |   2 +-
> >>>  drivers/rpmsg/rpmsg_char.c        | 231 +++++++++++-----------------
> >>>  drivers/rpmsg/rpmsg_char.h        |  46 ++++++
> >>>  drivers/rpmsg/rpmsg_core.c        |  15 +-
> >>>  drivers/rpmsg/rpmsg_ctrl.c        | 243 ++++++++++++++++++++++++++++++
> >>>  drivers/rpmsg/rpmsg_internal.h    |  10 +-
> >>>  drivers/rpmsg/virtio_rpmsg_bus.c  |   2 +-
> >>>  include/uapi/linux/rpmsg.h        |  10 ++
> >>>  15 files changed, 419 insertions(+), 155 deletions(-)
> >>>  create mode 100644 drivers/rpmsg/rpmsg_char.h
> >>>  create mode 100644 drivers/rpmsg/rpmsg_ctrl.c
> >>>
Arnaud POULIQUEN March 25, 2022, 5:52 p.m. UTC | #9
On 3/25/22 18:27, Mathieu Poirier wrote:
> On Fri, 25 Mar 2022 at 11:05, Arnaud POULIQUEN
> <arnaud.pouliquen@foss.st.com> wrote:
>>
>>
>>
>> On 3/25/22 16:59, Mathieu Poirier wrote:
>>> On Thu, Mar 24, 2022 at 06:36:23PM +0100, Arnaud POULIQUEN wrote:
>>>> Hi Bjorn,
>>>>
>>>> On 1/24/22 11:25, Arnaud Pouliquen wrote:
>>>>> Updates from V8 [1]:
>>>>> - rebase on 5.17-rc1 + rpmsg char cdev release fixes[2][3]
>>>>> - updates based on Bjorn Andersson's comments:
>>>>>   - remove rpmsg_create_default_ept API, set directly the ept->priv in rpmsg_chrdev_probe
>>>>>     function.
>>>>>   - rework commit message in [8/9]rpmsg: char: Introduce the "rpmsg-raw" channel
>>>>>
>>>>> Patchset description:
>>>>>
>>>>> The current rpmsg_char module implements a /dev/rpmsg_ctrl interface that provides the ability to
>>>>> instantiate char devices (/dev/rpmsgX) associated with an rpmsg endpoint for communication with the
>>>>> remote processor.
>>>>> This implementation fits with QCOM rpmsg backend but not with the magement by chanel implemented
>>>>> in the generic rpmsg virtio backend.
>>>>> This series restructures the rpmsg_char driver to decorrelate the control part from the data part
>>>>> in order to improve its compatible with the rpmsg virtio backend.
>>>>>
>>>>> Objective:
>>>>> - Expose a /dev/rpmsg_ctrlX interface for the application that is no longer dedicated to the
>>>>>   rpmsg_char but generalized to all rpmsg services. This offers capability to create and destroy
>>>>>   rpmsg channels from a user's application initiative (using the new RPMSG_CREATE_DEV_IOCTL and
>>>>>   RPMSG_DESTROY_DEV_IOCTL controls).
>>>>>   An application will be able to create/establish an rpmsg communication channel to communicate
>>>>>   with the remote processor, and not only wait the remote processor initiative.
>>>>>   This is interesting for example to establish a temporary communication link for diagnosis,
>>>>>   calibration, debugging... or instantiate  new data flows on some user actions.
>>>>> - Add capability to probe the rpmsg_char device at the initiative of the remote processor
>>>>>  (rpmsg service announcement mechanism).
>>>>>   This allows platforms based on the rpmsg virtio backend to create the /dev/rpmgX interface with
>>>>>   a rpmsg name service announcement.
>>>>>
>>>>> Subsets:
>>>>>   - Extract the control part of the char dev and create the rpmsg_ctrl.c file (patches 1 to 6)
>>>>>   - Introduce the "rpmsg-raw" channel in rpmsg_char(patches 7 to 10)
>>>>>   - Introduce the RPMSG_CREATE_DEV_IOCTL IOCTL and RPMSG_DESTROY_DEV_IOCTL to instantiate RPMsg
>>>>>     devices (patch 11)
>>>>>     The application can then create or release a channel by specifying:
>>>>>        - the name service of the device to instantiate.
>>>>>        - the source address.
>>>>>        - the destination address.
>>>>>
>>>>> This series has be applied and tested on 'commit e783362eb54c ("Linux 5.17-rc1") +
>>>>> rpmsg_char cdev release fixes [2][3]
>>>>>
>>>>> [1] https://lkml.org/lkml/2021/12/7/186
>>>>> [2] https://lkml.org/lkml/2022/1/10/1129
>>>>> [3] https://lkml.org/lkml/2022/1/10/1130
>>>>>
>>>>> Arnaud Pouliquen (11):
>>>>>   rpmsg: char: Export eptdev create and destroy functions
>>>>>   rpmsg: Create the rpmsg class in core instead of in rpmsg char
>>>>>   rpmsg: Move the rpmsg control device from rpmsg_char to rpmsg_ctrl
>>>>
>>>>
>>>>>   arm: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
>>>>>   RISC-V: configs: Configs that had RPMSG_CHAR now get RPMSG_CTRL
>>>>>   arm64: defconfig: Config that had RPMSG_CHAR now gets RPMSG_CTRL
>>>>
>>>> Thank you for merging this series!
>>>>
>>>> I can't see in the "for next" branch[1] the 3 patches above that update configs
>>>> Are you expecting a specific action from me?
>>>
>>> Those patches will need to go through the Arm, RISC-V and arm64 subsystems.  The
>>> mailing list for those subsystems has been CC'ed but that isn't enough to get
>>> the maintainers' attention.
>>>
>>> I suggest sending another patchset with those 3 patches that CC the maintainers
>>> directly.  For the Arm patch I suggest adding Linus Walleij.
>>
>> I will do what you suggest.
>>
>> My concerns in this case is about the scheduling of the integration.
>> I suppose that sending a second patchset for configs requests that the
>> rpmsg char series is first applied
> 
> Right, but the rpmsg_char series has been applied.
> 
>> But on the other hand this may lead to some failures as the RPMSG_CTRL is now
>> needed to create the /dev/rpmsg_ctrl0
>>
> 
> Possibly, but right now there is no other way.
> 
>> so probably, I need to do this as fixup patch.
>>
> 
> Indeed, this can be applied as a fix in rc1.

Yes, that's what I had in mind, I'll do it that way.

Thanks,
Arnaud

> 
>> FYI the RISC-V patch as been reviewed by Anup Patel
>>
> 
> ... but Anup does not maintain any of the defconfig files.
> 
>> Thanks,
>> Arnaud
>>
>>>
>>> Thanks,
>>> Mathieu
>>>
>>>>
>>>> [1]https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git
>>>>
>>>> Regards,
>>>> Arnaud
>>>>
>>>>>   rpmsg: Update rpmsg_chrdev_register_device function
>>>>>   rpmsg: char: Refactor rpmsg_chrdev_eptdev_create function
>>>>>   rpmsg: char: Add possibility to use default endpoint of the rpmsg
>>>>>     device
>>>>>   rpmsg: char: Introduce the "rpmsg-raw" channel
>>>>>   rpmsg: ctrl: Introduce new RPMSG_CREATE/RELEASE_DEV_IOCTL controls
>>>>>
>>>>>  arch/arm/configs/qcom_defconfig   |   1 +
>>>>>  arch/arm64/configs/defconfig      |   1 +
>>>>>  arch/riscv/configs/defconfig      |   1 +
>>>>>  arch/riscv/configs/rv32_defconfig |   1 +
>>>>>  drivers/rpmsg/Kconfig             |   8 +
>>>>>  drivers/rpmsg/Makefile            |   1 +
>>>>>  drivers/rpmsg/qcom_glink_native.c |   2 +-
>>>>>  drivers/rpmsg/qcom_smd.c          |   2 +-
>>>>>  drivers/rpmsg/rpmsg_char.c        | 231 +++++++++++-----------------
>>>>>  drivers/rpmsg/rpmsg_char.h        |  46 ++++++
>>>>>  drivers/rpmsg/rpmsg_core.c        |  15 +-
>>>>>  drivers/rpmsg/rpmsg_ctrl.c        | 243 ++++++++++++++++++++++++++++++
>>>>>  drivers/rpmsg/rpmsg_internal.h    |  10 +-
>>>>>  drivers/rpmsg/virtio_rpmsg_bus.c  |   2 +-
>>>>>  include/uapi/linux/rpmsg.h        |  10 ++
>>>>>  15 files changed, 419 insertions(+), 155 deletions(-)
>>>>>  create mode 100644 drivers/rpmsg/rpmsg_char.h
>>>>>  create mode 100644 drivers/rpmsg/rpmsg_ctrl.c
>>>>>