[PATCH v6 0/4] Add Microchip IPC mailbox

Valentina Fernandez posted 4 patches 1 year ago
.../bindings/mailbox/microchip,sbi-ipc.yaml   | 123 +++++
arch/riscv/include/asm/vendorid_list.h        |   1 +
arch/riscv/kernel/smp.c                       |   1 +
drivers/mailbox/Kconfig                       |  13 +
drivers/mailbox/Makefile                      |   2 +
drivers/mailbox/mailbox-mchp-ipc-sbi.c        | 504 ++++++++++++++++++
include/linux/mailbox/mchp-ipc.h              |  33 ++
7 files changed, 677 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml
create mode 100644 drivers/mailbox/mailbox-mchp-ipc-sbi.c
create mode 100644 include/linux/mailbox/mchp-ipc.h
[PATCH v6 0/4] Add Microchip IPC mailbox
Posted by Valentina Fernandez 1 year ago
Hello all,

This series adds support for the Microchip Inter-Processor Communication
(IPC) mailbox driver.

Microchip's family of RISC-V SoCs typically has one or more clusters
that can be configured to run in Asymmetric Multi-Processing (AMP) mode.

The Microchip IPC is used to send messages between processors using
an interrupt signaling mechanism. The driver uses the RISC-V Supervisor
Binary Interface (SBI) to communicate with software running in machine
mode (M-mode) to access the IPC hardware block.

Additional details on the Microchip vendor extension and the IPC
function IDs described in the driver can be found in the following
documentation:

https://github.com/linux4microchip/microchip-sbi-ecall-extension

The PIC64GX MPU has a Mi-V IHC block, this will be added to the PIC64GX
dts after the initial upstreaming [1].

[1] https://patchwork.kernel.org/project/linux-riscv/patch/20240725121609.13101-18-pierre-henry.moussay@microchip.com/

Changes in v6:
- update bindings to explain why 'reg' is not needed with microchip,sbi-ipc compatible

Changes in v5:
- change interrup-names property to use a list instead of pattern
- move assitionalProperties after allOf : block
- update reg property in dt binding example to use lowercase hex

Changes in v4:
- specify that microchip,miv-ihc-rtl-v2 is intended for use with MIV IHC Soft IP
- drop items array and use const value for compatible strings
- add a contraint to microchip,ihc-chan-disabled-mask property
- minor improvements to "'#mbox-cells' description

Changes in v3:
- Fix incorrent formatting around '=' in dt binding examples
- Add per compatible descriptions in dt binding
- Add '>' in certain dt binding descriptions to keep paragraphs maintained
- export __cpuid_to_hartid_map to compile mailbox driver as module
- Drop unused enum ipc_irq_type
- rename struct mchp_ipc_probe to mchp_ipc_mbox_info
- rename struct ipc_chan_info to mchp_ipc_sbi_chan
- rename struct microchip_ipc to mchp_ipc_sbi_mbox
- use phys_addr_t for __pa()
- drop mchp_ipc_get_chan_id function
- use num_chans in mbox_controller
- Fix buf_base_tx and buf_base_rx sizes using max and kmalloc

Changes in v2:
- use kmalloc and __pa() instead of DMA API
- fix size of buf_base to avoid potential buffer overflow
- add kernel doc for exported functions (mchp_ipc_get_chan_id)
- use EXPORT_SYMBOL_GPL instead of EXPORT_SYMBOL
- drop unnecessary blank line and fix alignment issues
- drop of_match_ptr
- move MODULE_DEVICE_TABLE next to the definition
- reword subject from riscv: asm: vendorid_list to riscv: sbi: vendorid_list
- remove the word "driver" from dt-binding commit subject
- make interrupt-names a required property for all cases
- add dependency on COMPILE_TEST and ARCH_MICROCHIP

Regards,
Valentina

Valentina Fernandez (4):
  riscv: sbi: vendorid_list: Add Microchip Technology to the vendor list
  riscv: export __cpuid_to_hartid_map
  dt-bindings: mailbox: add binding for Microchip IPC mailbox controller
  mailbox: add Microchip IPC support

 .../bindings/mailbox/microchip,sbi-ipc.yaml   | 123 +++++
 arch/riscv/include/asm/vendorid_list.h        |   1 +
 arch/riscv/kernel/smp.c                       |   1 +
 drivers/mailbox/Kconfig                       |  13 +
 drivers/mailbox/Makefile                      |   2 +
 drivers/mailbox/mailbox-mchp-ipc-sbi.c        | 504 ++++++++++++++++++
 include/linux/mailbox/mchp-ipc.h              |  33 ++
 7 files changed, 677 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml
 create mode 100644 drivers/mailbox/mailbox-mchp-ipc-sbi.c
 create mode 100644 include/linux/mailbox/mchp-ipc.h

-- 
2.34.1
Re: [PATCH v6 0/4] Add Microchip IPC mailbox
Posted by Klara Modin 6 months ago
Hi,

On 2024-12-17 11:31:30 +0000, Valentina Fernandez wrote:
> Hello all,
> 
> This series adds support for the Microchip Inter-Processor Communication
> (IPC) mailbox driver.
> 
> Microchip's family of RISC-V SoCs typically has one or more clusters
> that can be configured to run in Asymmetric Multi-Processing (AMP) mode.
> 
> The Microchip IPC is used to send messages between processors using
> an interrupt signaling mechanism. The driver uses the RISC-V Supervisor
> Binary Interface (SBI) to communicate with software running in machine
> mode (M-mode) to access the IPC hardware block.
> 
> Additional details on the Microchip vendor extension and the IPC
> function IDs described in the driver can be found in the following
> documentation:
> 
> https://github.com/linux4microchip/microchip-sbi-ecall-extension
> 
> The PIC64GX MPU has a Mi-V IHC block, this will be added to the PIC64GX
> dts after the initial upstreaming [1].
> 
> [1] https://patchwork.kernel.org/project/linux-riscv/patch/20240725121609.13101-18-pierre-henry.moussay@microchip.com/
> 
> Changes in v6:
> - update bindings to explain why 'reg' is not needed with microchip,sbi-ipc compatible
> 
> Changes in v5:
> - change interrup-names property to use a list instead of pattern
> - move assitionalProperties after allOf : block
> - update reg property in dt binding example to use lowercase hex
> 
> Changes in v4:
> - specify that microchip,miv-ihc-rtl-v2 is intended for use with MIV IHC Soft IP
> - drop items array and use const value for compatible strings
> - add a contraint to microchip,ihc-chan-disabled-mask property
> - minor improvements to "'#mbox-cells' description
> 
> Changes in v3:
> - Fix incorrent formatting around '=' in dt binding examples
> - Add per compatible descriptions in dt binding
> - Add '>' in certain dt binding descriptions to keep paragraphs maintained
> - export __cpuid_to_hartid_map to compile mailbox driver as module
> - Drop unused enum ipc_irq_type
> - rename struct mchp_ipc_probe to mchp_ipc_mbox_info
> - rename struct ipc_chan_info to mchp_ipc_sbi_chan
> - rename struct microchip_ipc to mchp_ipc_sbi_mbox
> - use phys_addr_t for __pa()
> - drop mchp_ipc_get_chan_id function
> - use num_chans in mbox_controller
> - Fix buf_base_tx and buf_base_rx sizes using max and kmalloc
> 
> Changes in v2:
> - use kmalloc and __pa() instead of DMA API
> - fix size of buf_base to avoid potential buffer overflow
> - add kernel doc for exported functions (mchp_ipc_get_chan_id)
> - use EXPORT_SYMBOL_GPL instead of EXPORT_SYMBOL
> - drop unnecessary blank line and fix alignment issues
> - drop of_match_ptr
> - move MODULE_DEVICE_TABLE next to the definition
> - reword subject from riscv: asm: vendorid_list to riscv: sbi: vendorid_list
> - remove the word "driver" from dt-binding commit subject
> - make interrupt-names a required property for all cases
> - add dependency on COMPILE_TEST and ARCH_MICROCHIP
> 
> Regards,
> Valentina
> 
> Valentina Fernandez (4):
>   riscv: sbi: vendorid_list: Add Microchip Technology to the vendor list
>   riscv: export __cpuid_to_hartid_map
>   dt-bindings: mailbox: add binding for Microchip IPC mailbox controller
>   mailbox: add Microchip IPC support
> 
>  .../bindings/mailbox/microchip,sbi-ipc.yaml   | 123 +++++
>  arch/riscv/include/asm/vendorid_list.h        |   1 +
>  arch/riscv/kernel/smp.c                       |   1 +
>  drivers/mailbox/Kconfig                       |  13 +
>  drivers/mailbox/Makefile                      |   2 +
>  drivers/mailbox/mailbox-mchp-ipc-sbi.c        | 504 ++++++++++++++++++
>  include/linux/mailbox/mchp-ipc.h              |  33 ++
>  7 files changed, 677 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml
>  create mode 100644 drivers/mailbox/mailbox-mchp-ipc-sbi.c
>  create mode 100644 include/linux/mailbox/mchp-ipc.h
> 
> -- 
> 2.34.1
> 

Building this driver as a module with SMP=n fails becuase the non-SMP
cpuid_to_hartid_map() depends on the non-exported boot_cpu_hartid
(discovered via randconfig).

From the description of the driver it doesn't seem to be very useful
with SMP=n, would it make sense to have it depend on SMP rather than
exporting boot_cpu_hartid?

Regards,
Klara Modin
Re: [PATCH v6 0/4] Add Microchip IPC mailbox
Posted by Conor Dooley 6 months ago
On Mon, Jun 16, 2025 at 08:44:01PM +0200, Klara Modin wrote:
> On 2024-12-17 11:31:30 +0000, Valentina Fernandez wrote:
> > 
> > Valentina Fernandez (4):
> >   riscv: sbi: vendorid_list: Add Microchip Technology to the vendor list
> >   riscv: export __cpuid_to_hartid_map
> >   dt-bindings: mailbox: add binding for Microchip IPC mailbox controller
> >   mailbox: add Microchip IPC support
> > 
> >  .../bindings/mailbox/microchip,sbi-ipc.yaml   | 123 +++++
> >  arch/riscv/include/asm/vendorid_list.h        |   1 +
> >  arch/riscv/kernel/smp.c                       |   1 +
> >  drivers/mailbox/Kconfig                       |  13 +
> >  drivers/mailbox/Makefile                      |   2 +
> >  drivers/mailbox/mailbox-mchp-ipc-sbi.c        | 504 ++++++++++++++++++
> >  include/linux/mailbox/mchp-ipc.h              |  33 ++
> >  7 files changed, 677 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mailbox/microchip,sbi-ipc.yaml
> >  create mode 100644 drivers/mailbox/mailbox-mchp-ipc-sbi.c
> >  create mode 100644 include/linux/mailbox/mchp-ipc.h
> > 
> > -- 
> > 2.34.1
> > 
> 
> Building this driver as a module with SMP=n fails becuase the non-SMP
> cpuid_to_hartid_map() depends on the non-exported boot_cpu_hartid
> (discovered via randconfig).
> 
> >From the description of the driver it doesn't seem to be very useful
> with SMP=n, would it make sense to have it depend on SMP rather than
> exporting boot_cpu_hartid?

No, I think boot_cpu_hartid needs to get exported, the driver is
intended for use in AMP situations so the kernel config may have SMP
disabled.
Re: [PATCH v6 0/4] Add Microchip IPC mailbox
Posted by Nathan Chancellor 11 months ago
Hi Jassi,

On Tue, Dec 17, 2024 at 11:31:30AM +0000, Valentina Fernandez wrote:
...
> This series adds support for the Microchip Inter-Processor Communication
> (IPC) mailbox driver.
...
> Valentina Fernandez (4):
>   riscv: sbi: vendorid_list: Add Microchip Technology to the vendor list
>   riscv: export __cpuid_to_hartid_map
>   dt-bindings: mailbox: add binding for Microchip IPC mailbox controller
>   mailbox: add Microchip IPC support

I see this series is now in next-20250120 but you only applied patches 3
and 4, so the build is broken:

  In file included from drivers/mailbox/mailbox-mchp-ipc-sbi.c:22:
  drivers/mailbox/mailbox-mchp-ipc-sbi.c: In function 'mchp_ipc_sbi_chan_send':
  drivers/mailbox/mailbox-mchp-ipc-sbi.c:29:42: error: 'MICROCHIP_VENDOR_ID' undeclared (first use in this function)
     29 |                                          MICROCHIP_VENDOR_ID)
        |                                          ^~~~~~~~~~~~~~~~~~~
  arch/riscv/include/asm/sbi.h:436:56: note: in definition of macro 'sbi_ecall'
    436 |                 __sbi_ecall(a0, a1, a2, a3, a4, a5, f, e)
        |                                                        ^
  drivers/mailbox/mailbox-mchp-ipc-sbi.c:121:25: note: in expansion of macro 'SBI_EXT_MICROCHIP_TECHNOLOGY'
    121 |         ret = sbi_ecall(SBI_EXT_MICROCHIP_TECHNOLOGY, command, channel,
        |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
  drivers/mailbox/mailbox-mchp-ipc-sbi.c:29:42: note: each undeclared identifier is reported only once for each function it appears in
     29 |                                          MICROCHIP_VENDOR_ID)
        |                                          ^~~~~~~~~~~~~~~~~~~
  arch/riscv/include/asm/sbi.h:436:56: note: in definition of macro 'sbi_ecall'
    436 |                 __sbi_ecall(a0, a1, a2, a3, a4, a5, f, e)
        |                                                        ^
  drivers/mailbox/mailbox-mchp-ipc-sbi.c:121:25: note: in expansion of macro 'SBI_EXT_MICROCHIP_TECHNOLOGY'
    121 |         ret = sbi_ecall(SBI_EXT_MICROCHIP_TECHNOLOGY, command, channel,
        |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
  drivers/mailbox/mailbox-mchp-ipc-sbi.c: In function 'mchp_ipc_sbi_send':
  drivers/mailbox/mailbox-mchp-ipc-sbi.c:29:42: error: 'MICROCHIP_VENDOR_ID' undeclared (first use in this function)
     29 |                                          MICROCHIP_VENDOR_ID)
        |                                          ^~~~~~~~~~~~~~~~~~~
  arch/riscv/include/asm/sbi.h:436:56: note: in definition of macro 'sbi_ecall'
    436 |                 __sbi_ecall(a0, a1, a2, a3, a4, a5, f, e)
        |                                                        ^
  drivers/mailbox/mailbox-mchp-ipc-sbi.c:134:25: note: in expansion of macro 'SBI_EXT_MICROCHIP_TECHNOLOGY'
    134 |         ret = sbi_ecall(SBI_EXT_MICROCHIP_TECHNOLOGY, command, address,
        |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
  drivers/mailbox/mailbox-mchp-ipc-sbi.c: In function 'mchp_ipc_probe':
  drivers/mailbox/mailbox-mchp-ipc-sbi.c:29:42: error: 'MICROCHIP_VENDOR_ID' undeclared (first use in this function)
     29 |                                          MICROCHIP_VENDOR_ID)
        |                                          ^~~~~~~~~~~~~~~~~~~
  drivers/mailbox/mailbox-mchp-ipc-sbi.c:420:35: note: in expansion of macro 'SBI_EXT_MICROCHIP_TECHNOLOGY'
    420 |         ret = sbi_probe_extension(SBI_EXT_MICROCHIP_TECHNOLOGY);
        |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~

Can you please fix this? It is a little frustrating this is happening
just as the merge window opens...

Cheers,
Nathan
Re: [PATCH v6 0/4] Add Microchip IPC mailbox
Posted by Jassi Brar 11 months ago
On Mon, Jan 20, 2025 at 6:20 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> Hi Jassi,
>
> On Tue, Dec 17, 2024 at 11:31:30AM +0000, Valentina Fernandez wrote:
> ...
> > This series adds support for the Microchip Inter-Processor Communication
> > (IPC) mailbox driver.
> ...
> > Valentina Fernandez (4):
> >   riscv: sbi: vendorid_list: Add Microchip Technology to the vendor list
> >   riscv: export __cpuid_to_hartid_map
> >   dt-bindings: mailbox: add binding for Microchip IPC mailbox controller
> >   mailbox: add Microchip IPC support
>
> I see this series is now in next-20250120 but you only applied patches 3
> and 4, so the build is broken:
>
>   In file included from drivers/mailbox/mailbox-mchp-ipc-sbi.c:22:
>   drivers/mailbox/mailbox-mchp-ipc-sbi.c: In function 'mchp_ipc_sbi_chan_send':
>   drivers/mailbox/mailbox-mchp-ipc-sbi.c:29:42: error: 'MICROCHIP_VENDOR_ID' undeclared (first use in this function)
>      29 |                                          MICROCHIP_VENDOR_ID)
>         |                                          ^~~~~~~~~~~~~~~~~~~
>   arch/riscv/include/asm/sbi.h:436:56: note: in definition of macro 'sbi_ecall'
>     436 |                 __sbi_ecall(a0, a1, a2, a3, a4, a5, f, e)
>         |                                                        ^
>   drivers/mailbox/mailbox-mchp-ipc-sbi.c:121:25: note: in expansion of macro 'SBI_EXT_MICROCHIP_TECHNOLOGY'
>     121 |         ret = sbi_ecall(SBI_EXT_MICROCHIP_TECHNOLOGY, command, channel,
>         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   drivers/mailbox/mailbox-mchp-ipc-sbi.c:29:42: note: each undeclared identifier is reported only once for each function it appears in
>      29 |                                          MICROCHIP_VENDOR_ID)
>         |                                          ^~~~~~~~~~~~~~~~~~~
>   arch/riscv/include/asm/sbi.h:436:56: note: in definition of macro 'sbi_ecall'
>     436 |                 __sbi_ecall(a0, a1, a2, a3, a4, a5, f, e)
>         |                                                        ^
>   drivers/mailbox/mailbox-mchp-ipc-sbi.c:121:25: note: in expansion of macro 'SBI_EXT_MICROCHIP_TECHNOLOGY'
>     121 |         ret = sbi_ecall(SBI_EXT_MICROCHIP_TECHNOLOGY, command, channel,
>         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   drivers/mailbox/mailbox-mchp-ipc-sbi.c: In function 'mchp_ipc_sbi_send':
>   drivers/mailbox/mailbox-mchp-ipc-sbi.c:29:42: error: 'MICROCHIP_VENDOR_ID' undeclared (first use in this function)
>      29 |                                          MICROCHIP_VENDOR_ID)
>         |                                          ^~~~~~~~~~~~~~~~~~~
>   arch/riscv/include/asm/sbi.h:436:56: note: in definition of macro 'sbi_ecall'
>     436 |                 __sbi_ecall(a0, a1, a2, a3, a4, a5, f, e)
>         |                                                        ^
>   drivers/mailbox/mailbox-mchp-ipc-sbi.c:134:25: note: in expansion of macro 'SBI_EXT_MICROCHIP_TECHNOLOGY'
>     134 |         ret = sbi_ecall(SBI_EXT_MICROCHIP_TECHNOLOGY, command, address,
>         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   drivers/mailbox/mailbox-mchp-ipc-sbi.c: In function 'mchp_ipc_probe':
>   drivers/mailbox/mailbox-mchp-ipc-sbi.c:29:42: error: 'MICROCHIP_VENDOR_ID' undeclared (first use in this function)
>      29 |                                          MICROCHIP_VENDOR_ID)
>         |                                          ^~~~~~~~~~~~~~~~~~~
>   drivers/mailbox/mailbox-mchp-ipc-sbi.c:420:35: note: in expansion of macro 'SBI_EXT_MICROCHIP_TECHNOLOGY'
>     420 |         ret = sbi_probe_extension(SBI_EXT_MICROCHIP_TECHNOLOGY);
>         |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Can you please fix this? It is a little frustrating this is happening
> just as the merge window opens...
>
Usually the platform specific stuff goes via the platform tree, for
example there isn't anything 'mailbox' about MICROCHIP_VENDOR_ID and
__cpuid_to_hartid_map.
I pick only mailbox patches unless the maintainer acks and wants me to
pick platform ones. Though I should have noted that I am picking only
mailbox stuff, but ok I will queue them now.
Thnx