Re: [PATCH V2 02/10] mailbox: tegra-hsp: Add HSP(Hardware Synchronization Primitives) driver
On 07/08/2016 05:33 AM, Sivaram Nair wrote: On Thu, Jul 07, 2016 at 02:37:27PM +0800, Joseph Lo wrote: On 07/06/2016 08:23 PM, Alexandre Courbot wrote: On Wed, Jul 6, 2016 at 6:06 PM, Joseph Lowrote: On 07/06/2016 03:05 PM, Alexandre Courbot wrote: On Tue, Jul 5, 2016 at 6:04 PM, Joseph Lo wrote: The Tegra HSP mailbox driver implements the signaling doorbell-based interprocessor communication (IPC) for remote processors currently. The HSP HW modules support some different features for that, which are shared mailboxes, shared semaphores, arbitrated semaphores, and doorbells. And there are multiple HSP HW instances on the chip. So the driver is extendable to support more features for different IPC requirement. The driver of remote processor can use it as a mailbox client and deal with the IPC protocol to synchronize the data communications. Signed-off-by: Joseph Lo --- Changes in V2: - Update the driver to support the binding changes in V2 - it's extendable to support multiple HSP sub-modules on the same HSP HW block now. --- drivers/mailbox/Kconfig | 9 + drivers/mailbox/Makefile| 2 + drivers/mailbox/tegra-hsp.c | 418 3 files changed, 429 insertions(+) create mode 100644 drivers/mailbox/tegra-hsp.c diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig index 5305923752d2..fe584cb54720 100644 --- a/drivers/mailbox/Kconfig +++ b/drivers/mailbox/Kconfig @@ -114,6 +114,15 @@ config MAILBOX_TEST Test client to help with testing new Controller driver implementations. +config TEGRA_HSP_MBOX + bool "Tegra HSP(Hardware Synchronization Primitives) Driver" Space missing before the opening parenthesis (same in the patch title btw). Okay. + depends on ARCH_TEGRA_186_SOC + help + The Tegra HSP driver is used for the interprocessor communication + between different remote processors and host processors on Tegra186 + and later SoCs. Say Y here if you want to have this support. + If unsure say N. Since this option is selected automatically by ARCH_TEGRA_186_SOC, you should probably drop the last 2 sentences. Okay. + config XGENE_SLIMPRO_MBOX tristate "APM SoC X-Gene SLIMpro Mailbox Controller" depends on ARCH_XGENE diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile index 0be3e742bb7d..26d8f91c7fea 100644 --- a/drivers/mailbox/Makefile +++ b/drivers/mailbox/Makefile @@ -25,3 +25,5 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o obj-$(CONFIG_HI6220_MBOX) += hi6220-mailbox.o + +obj-${CONFIG_TEGRA_HSP_MBOX} += tegra-hsp.o diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c new file mode 100644 index ..93c3ef58f29f --- /dev/null +++ b/drivers/mailbox/tegra-hsp.c @@ -0,0 +1,418 @@ +/* + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + */ + +#include +#include +#include +#include +#include +#include +#include + +#define HSP_INT_DIMENSIONING 0x380 +#define HSP_nSM_OFFSET 0 +#define HSP_nSS_OFFSET 4 +#define HSP_nAS_OFFSET 8 +#define HSP_nDB_OFFSET 12 +#define HSP_nSI_OFFSET 16 Would be nice to have comments to understand what SM, SS, AS, etc. stand for (Shared Mailboxes, Shared Semaphores, Arbitrated Semaphores but you need to look at the patch description to understand that). A top-of-file comment explaning the necessary concepts to read this code would do the trick. Yes, will fix that. +#define HSP_nINT_MASK 0xf + +#define HSP_DB_REG_TRIGGER 0x0 +#define HSP_DB_REG_ENABLE 0x4 +#define HSP_DB_REG_RAW 0x8 +#define HSP_DB_REG_PENDING 0xc + +#define HSP_DB_CCPLEX 1 +#define HSP_DB_BPMP3 Maybe turn this into enum and use that type for tegra_hsp_db_chan::db_id? Also have MAX_NUM_HSP_DB here, since it is related to these values? Okay. + +#define MAX_NUM_HSP_CHAN 32 +#define MAX_NUM_HSP_DB 7 + +#define hsp_db_offset(i, d) \ + (d->base + ((1 + (d->nr_sm >> 1) + d->nr_ss + d->nr_as) << 16) + \ + (i) * 0x100) + +struct tegra_hsp_db_chan { + int master_id; + int db_id; +}; + +struct tegra_hsp_mbox_chan { + int type; + union { + struct tegra_hsp_db_chan db_chan; + }; +}; + +struct tegra_hsp_mbox { + struct
Re: [PATCH V2 02/10] mailbox: tegra-hsp: Add HSP(Hardware Synchronization Primitives) driver
On 07/08/2016 05:33 AM, Sivaram Nair wrote: On Thu, Jul 07, 2016 at 02:37:27PM +0800, Joseph Lo wrote: On 07/06/2016 08:23 PM, Alexandre Courbot wrote: On Wed, Jul 6, 2016 at 6:06 PM, Joseph Lo wrote: On 07/06/2016 03:05 PM, Alexandre Courbot wrote: On Tue, Jul 5, 2016 at 6:04 PM, Joseph Lo wrote: The Tegra HSP mailbox driver implements the signaling doorbell-based interprocessor communication (IPC) for remote processors currently. The HSP HW modules support some different features for that, which are shared mailboxes, shared semaphores, arbitrated semaphores, and doorbells. And there are multiple HSP HW instances on the chip. So the driver is extendable to support more features for different IPC requirement. The driver of remote processor can use it as a mailbox client and deal with the IPC protocol to synchronize the data communications. Signed-off-by: Joseph Lo --- Changes in V2: - Update the driver to support the binding changes in V2 - it's extendable to support multiple HSP sub-modules on the same HSP HW block now. --- drivers/mailbox/Kconfig | 9 + drivers/mailbox/Makefile| 2 + drivers/mailbox/tegra-hsp.c | 418 3 files changed, 429 insertions(+) create mode 100644 drivers/mailbox/tegra-hsp.c diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig index 5305923752d2..fe584cb54720 100644 --- a/drivers/mailbox/Kconfig +++ b/drivers/mailbox/Kconfig @@ -114,6 +114,15 @@ config MAILBOX_TEST Test client to help with testing new Controller driver implementations. +config TEGRA_HSP_MBOX + bool "Tegra HSP(Hardware Synchronization Primitives) Driver" Space missing before the opening parenthesis (same in the patch title btw). Okay. + depends on ARCH_TEGRA_186_SOC + help + The Tegra HSP driver is used for the interprocessor communication + between different remote processors and host processors on Tegra186 + and later SoCs. Say Y here if you want to have this support. + If unsure say N. Since this option is selected automatically by ARCH_TEGRA_186_SOC, you should probably drop the last 2 sentences. Okay. + config XGENE_SLIMPRO_MBOX tristate "APM SoC X-Gene SLIMpro Mailbox Controller" depends on ARCH_XGENE diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile index 0be3e742bb7d..26d8f91c7fea 100644 --- a/drivers/mailbox/Makefile +++ b/drivers/mailbox/Makefile @@ -25,3 +25,5 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o obj-$(CONFIG_HI6220_MBOX) += hi6220-mailbox.o + +obj-${CONFIG_TEGRA_HSP_MBOX} += tegra-hsp.o diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c new file mode 100644 index ..93c3ef58f29f --- /dev/null +++ b/drivers/mailbox/tegra-hsp.c @@ -0,0 +1,418 @@ +/* + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + */ + +#include +#include +#include +#include +#include +#include +#include + +#define HSP_INT_DIMENSIONING 0x380 +#define HSP_nSM_OFFSET 0 +#define HSP_nSS_OFFSET 4 +#define HSP_nAS_OFFSET 8 +#define HSP_nDB_OFFSET 12 +#define HSP_nSI_OFFSET 16 Would be nice to have comments to understand what SM, SS, AS, etc. stand for (Shared Mailboxes, Shared Semaphores, Arbitrated Semaphores but you need to look at the patch description to understand that). A top-of-file comment explaning the necessary concepts to read this code would do the trick. Yes, will fix that. +#define HSP_nINT_MASK 0xf + +#define HSP_DB_REG_TRIGGER 0x0 +#define HSP_DB_REG_ENABLE 0x4 +#define HSP_DB_REG_RAW 0x8 +#define HSP_DB_REG_PENDING 0xc + +#define HSP_DB_CCPLEX 1 +#define HSP_DB_BPMP3 Maybe turn this into enum and use that type for tegra_hsp_db_chan::db_id? Also have MAX_NUM_HSP_DB here, since it is related to these values? Okay. + +#define MAX_NUM_HSP_CHAN 32 +#define MAX_NUM_HSP_DB 7 + +#define hsp_db_offset(i, d) \ + (d->base + ((1 + (d->nr_sm >> 1) + d->nr_ss + d->nr_as) << 16) + \ + (i) * 0x100) + +struct tegra_hsp_db_chan { + int master_id; + int db_id; +}; + +struct tegra_hsp_mbox_chan { + int type; + union { + struct tegra_hsp_db_chan db_chan; + }; +}; + +struct tegra_hsp_mbox { + struct mbox_controller *mbox; + void __iomem *base; +
Re: [PATCH V2 02/10] mailbox: tegra-hsp: Add HSP(Hardware Synchronization Primitives) driver
On 07/08/2016 05:10 AM, Sivaram Nair wrote: On Tue, Jul 05, 2016 at 05:04:23PM +0800, Joseph Lo wrote: The Tegra HSP mailbox driver implements the signaling doorbell-based interprocessor communication (IPC) for remote processors currently. The HSP HW modules support some different features for that, which are shared mailboxes, shared semaphores, arbitrated semaphores, and doorbells. And there are multiple HSP HW instances on the chip. So the driver is extendable to support more features for different IPC requirement. The driver of remote processor can use it as a mailbox client and deal with the IPC protocol to synchronize the data communications. Signed-off-by: Joseph Lo--- Changes in V2: - Update the driver to support the binding changes in V2 - it's extendable to support multiple HSP sub-modules on the same HSP HW block now. --- drivers/mailbox/Kconfig | 9 + drivers/mailbox/Makefile| 2 + drivers/mailbox/tegra-hsp.c | 418 3 files changed, 429 insertions(+) create mode 100644 drivers/mailbox/tegra-hsp.c diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig index 5305923752d2..fe584cb54720 100644 --- a/drivers/mailbox/Kconfig +++ b/drivers/mailbox/Kconfig @@ -114,6 +114,15 @@ config MAILBOX_TEST Test client to help with testing new Controller driver implementations. +config TEGRA_HSP_MBOX + bool "Tegra HSP(Hardware Synchronization Primitives) Driver" + depends on ARCH_TEGRA_186_SOC + help + The Tegra HSP driver is used for the interprocessor communication + between different remote processors and host processors on Tegra186 + and later SoCs. Say Y here if you want to have this support. + If unsure say N. + config XGENE_SLIMPRO_MBOX tristate "APM SoC X-Gene SLIMpro Mailbox Controller" depends on ARCH_XGENE diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile index 0be3e742bb7d..26d8f91c7fea 100644 --- a/drivers/mailbox/Makefile +++ b/drivers/mailbox/Makefile @@ -25,3 +25,5 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o obj-$(CONFIG_HI6220_MBOX) += hi6220-mailbox.o + +obj-${CONFIG_TEGRA_HSP_MBOX} += tegra-hsp.o diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c new file mode 100644 index ..93c3ef58f29f --- /dev/null +++ b/drivers/mailbox/tegra-hsp.c @@ -0,0 +1,418 @@ +/* + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + */ + +#include +#include +#include +#include +#include +#include +#include + +#define HSP_INT_DIMENSIONING 0x380 +#define HSP_nSM_OFFSET 0 +#define HSP_nSS_OFFSET 4 +#define HSP_nAS_OFFSET 8 +#define HSP_nDB_OFFSET 12 +#define HSP_nSI_OFFSET 16 +#define HSP_nINT_MASK 0xf + +#define HSP_DB_REG_TRIGGER 0x0 +#define HSP_DB_REG_ENABLE 0x4 +#define HSP_DB_REG_RAW 0x8 +#define HSP_DB_REG_PENDING 0xc + +#define HSP_DB_CCPLEX 1 +#define HSP_DB_BPMP3 + +#define MAX_NUM_HSP_CHAN 32 Is this an arbitrarily chosen number? Ah, this should be MAX_NUM_HSP_DB_CHAN now. But the mbox driver still needs a max channel number, I will check how to enhance it properly with multiple HSP modules support in the same driver. Maybe 4 channels for SM, AS, SS and DB. And the sub channels for different functions under them. Then it may able to fix the double loop issue in the hsp_db_irq function. +#define MAX_NUM_HSP_DB 7 + +#define hsp_db_offset(i, d) \ + (d->base + ((1 + (d->nr_sm >> 1) + d->nr_ss + d->nr_as) << 16) + \ + (i) * 0x100) + +struct tegra_hsp_db_chan { + int master_id; + int db_id; These should be unsigned? Yes, will fix them. +}; + +struct tegra_hsp_mbox_chan { + int type; This too... + union { + struct tegra_hsp_db_chan db_chan; + }; +}; Why do we need to use a union? Because we only support DB right now, there is only one member in the union. We can add something like sm_chan here when we need to support that later. + +struct tegra_hsp_mbox { + struct mbox_controller *mbox; + void __iomem *base; + void __iomem *db_base[MAX_NUM_HSP_DB]; + int db_irq; + int nr_sm; + int nr_as; + int nr_ss; + int nr_db; + int nr_si; + spinlock_t lock; You might need to change this to a mutex - see
Re: [PATCH V2 02/10] mailbox: tegra-hsp: Add HSP(Hardware Synchronization Primitives) driver
On 07/08/2016 05:10 AM, Sivaram Nair wrote: On Tue, Jul 05, 2016 at 05:04:23PM +0800, Joseph Lo wrote: The Tegra HSP mailbox driver implements the signaling doorbell-based interprocessor communication (IPC) for remote processors currently. The HSP HW modules support some different features for that, which are shared mailboxes, shared semaphores, arbitrated semaphores, and doorbells. And there are multiple HSP HW instances on the chip. So the driver is extendable to support more features for different IPC requirement. The driver of remote processor can use it as a mailbox client and deal with the IPC protocol to synchronize the data communications. Signed-off-by: Joseph Lo --- Changes in V2: - Update the driver to support the binding changes in V2 - it's extendable to support multiple HSP sub-modules on the same HSP HW block now. --- drivers/mailbox/Kconfig | 9 + drivers/mailbox/Makefile| 2 + drivers/mailbox/tegra-hsp.c | 418 3 files changed, 429 insertions(+) create mode 100644 drivers/mailbox/tegra-hsp.c diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig index 5305923752d2..fe584cb54720 100644 --- a/drivers/mailbox/Kconfig +++ b/drivers/mailbox/Kconfig @@ -114,6 +114,15 @@ config MAILBOX_TEST Test client to help with testing new Controller driver implementations. +config TEGRA_HSP_MBOX + bool "Tegra HSP(Hardware Synchronization Primitives) Driver" + depends on ARCH_TEGRA_186_SOC + help + The Tegra HSP driver is used for the interprocessor communication + between different remote processors and host processors on Tegra186 + and later SoCs. Say Y here if you want to have this support. + If unsure say N. + config XGENE_SLIMPRO_MBOX tristate "APM SoC X-Gene SLIMpro Mailbox Controller" depends on ARCH_XGENE diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile index 0be3e742bb7d..26d8f91c7fea 100644 --- a/drivers/mailbox/Makefile +++ b/drivers/mailbox/Makefile @@ -25,3 +25,5 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o obj-$(CONFIG_HI6220_MBOX) += hi6220-mailbox.o + +obj-${CONFIG_TEGRA_HSP_MBOX} += tegra-hsp.o diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c new file mode 100644 index ..93c3ef58f29f --- /dev/null +++ b/drivers/mailbox/tegra-hsp.c @@ -0,0 +1,418 @@ +/* + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + */ + +#include +#include +#include +#include +#include +#include +#include + +#define HSP_INT_DIMENSIONING 0x380 +#define HSP_nSM_OFFSET 0 +#define HSP_nSS_OFFSET 4 +#define HSP_nAS_OFFSET 8 +#define HSP_nDB_OFFSET 12 +#define HSP_nSI_OFFSET 16 +#define HSP_nINT_MASK 0xf + +#define HSP_DB_REG_TRIGGER 0x0 +#define HSP_DB_REG_ENABLE 0x4 +#define HSP_DB_REG_RAW 0x8 +#define HSP_DB_REG_PENDING 0xc + +#define HSP_DB_CCPLEX 1 +#define HSP_DB_BPMP3 + +#define MAX_NUM_HSP_CHAN 32 Is this an arbitrarily chosen number? Ah, this should be MAX_NUM_HSP_DB_CHAN now. But the mbox driver still needs a max channel number, I will check how to enhance it properly with multiple HSP modules support in the same driver. Maybe 4 channels for SM, AS, SS and DB. And the sub channels for different functions under them. Then it may able to fix the double loop issue in the hsp_db_irq function. +#define MAX_NUM_HSP_DB 7 + +#define hsp_db_offset(i, d) \ + (d->base + ((1 + (d->nr_sm >> 1) + d->nr_ss + d->nr_as) << 16) + \ + (i) * 0x100) + +struct tegra_hsp_db_chan { + int master_id; + int db_id; These should be unsigned? Yes, will fix them. +}; + +struct tegra_hsp_mbox_chan { + int type; This too... + union { + struct tegra_hsp_db_chan db_chan; + }; +}; Why do we need to use a union? Because we only support DB right now, there is only one member in the union. We can add something like sm_chan here when we need to support that later. + +struct tegra_hsp_mbox { + struct mbox_controller *mbox; + void __iomem *base; + void __iomem *db_base[MAX_NUM_HSP_DB]; + int db_irq; + int nr_sm; + int nr_as; + int nr_ss; + int nr_db; + int nr_si; + spinlock_t lock; You might need to change this to a mutex - see below. OK, will
Re: [PATCH V2 02/10] mailbox: tegra-hsp: Add HSP(Hardware Synchronization Primitives) driver
On Thu, Jul 07, 2016 at 02:37:27PM +0800, Joseph Lo wrote: > On 07/06/2016 08:23 PM, Alexandre Courbot wrote: > >On Wed, Jul 6, 2016 at 6:06 PM, Joseph Lowrote: > >>On 07/06/2016 03:05 PM, Alexandre Courbot wrote: > >>> > >>>On Tue, Jul 5, 2016 at 6:04 PM, Joseph Lo wrote: > > The Tegra HSP mailbox driver implements the signaling doorbell-based > interprocessor communication (IPC) for remote processors currently. The > HSP HW modules support some different features for that, which are > shared mailboxes, shared semaphores, arbitrated semaphores, and > doorbells. And there are multiple HSP HW instances on the chip. So the > driver is extendable to support more features for different IPC > requirement. > > The driver of remote processor can use it as a mailbox client and deal > with the IPC protocol to synchronize the data communications. > > Signed-off-by: Joseph Lo > --- > Changes in V2: > - Update the driver to support the binding changes in V2 > - it's extendable to support multiple HSP sub-modules on the same HSP HW > block > now. > --- > drivers/mailbox/Kconfig | 9 + > drivers/mailbox/Makefile| 2 + > drivers/mailbox/tegra-hsp.c | 418 > > 3 files changed, 429 insertions(+) > create mode 100644 drivers/mailbox/tegra-hsp.c > > diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig > index 5305923752d2..fe584cb54720 100644 > --- a/drivers/mailbox/Kconfig > +++ b/drivers/mailbox/Kconfig > @@ -114,6 +114,15 @@ config MAILBOX_TEST > Test client to help with testing new Controller driver > implementations. > > +config TEGRA_HSP_MBOX > + bool "Tegra HSP(Hardware Synchronization Primitives) Driver" > >>> > >>> > >>>Space missing before the opening parenthesis (same in the patch title > >>>btw). > >> > >>Okay. > >>> > >>> > + depends on ARCH_TEGRA_186_SOC > + help > + The Tegra HSP driver is used for the interprocessor > communication > + between different remote processors and host processors on > Tegra186 > + and later SoCs. Say Y here if you want to have this support. > + If unsure say N. > >>> > >>> > >>>Since this option is selected automatically by ARCH_TEGRA_186_SOC, you > >>>should probably drop the last 2 sentences. > >> > >>Okay. > >> > >>> > + > config XGENE_SLIMPRO_MBOX > tristate "APM SoC X-Gene SLIMpro Mailbox Controller" > depends on ARCH_XGENE > diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile > index 0be3e742bb7d..26d8f91c7fea 100644 > --- a/drivers/mailbox/Makefile > +++ b/drivers/mailbox/Makefile > @@ -25,3 +25,5 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o > obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o > > obj-$(CONFIG_HI6220_MBOX) += hi6220-mailbox.o > + > +obj-${CONFIG_TEGRA_HSP_MBOX} += tegra-hsp.o > diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c > new file mode 100644 > index ..93c3ef58f29f > --- /dev/null > +++ b/drivers/mailbox/tegra-hsp.c > @@ -0,0 +1,418 @@ > +/* > + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but > WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License > for > + * more details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define HSP_INT_DIMENSIONING 0x380 > +#define HSP_nSM_OFFSET 0 > +#define HSP_nSS_OFFSET 4 > +#define HSP_nAS_OFFSET 8 > +#define HSP_nDB_OFFSET 12 > +#define HSP_nSI_OFFSET 16 > >>> > >>> > >>>Would be nice to have comments to understand what SM, SS, AS, etc. > >>>stand for (Shared Mailboxes, Shared Semaphores, Arbitrated Semaphores > >>>but you need to look at the patch description to understand that). A > >>>top-of-file comment explaning the necessary concepts to read this code > >>>would do the trick. > >> > >>Yes, will fix that. > >>> > >>> > +#define HSP_nINT_MASK 0xf > + > +#define HSP_DB_REG_TRIGGER 0x0 > +#define HSP_DB_REG_ENABLE 0x4 > +#define HSP_DB_REG_RAW 0x8 > +#define
Re: [PATCH V2 02/10] mailbox: tegra-hsp: Add HSP(Hardware Synchronization Primitives) driver
On Thu, Jul 07, 2016 at 02:37:27PM +0800, Joseph Lo wrote: > On 07/06/2016 08:23 PM, Alexandre Courbot wrote: > >On Wed, Jul 6, 2016 at 6:06 PM, Joseph Lo wrote: > >>On 07/06/2016 03:05 PM, Alexandre Courbot wrote: > >>> > >>>On Tue, Jul 5, 2016 at 6:04 PM, Joseph Lo wrote: > > The Tegra HSP mailbox driver implements the signaling doorbell-based > interprocessor communication (IPC) for remote processors currently. The > HSP HW modules support some different features for that, which are > shared mailboxes, shared semaphores, arbitrated semaphores, and > doorbells. And there are multiple HSP HW instances on the chip. So the > driver is extendable to support more features for different IPC > requirement. > > The driver of remote processor can use it as a mailbox client and deal > with the IPC protocol to synchronize the data communications. > > Signed-off-by: Joseph Lo > --- > Changes in V2: > - Update the driver to support the binding changes in V2 > - it's extendable to support multiple HSP sub-modules on the same HSP HW > block > now. > --- > drivers/mailbox/Kconfig | 9 + > drivers/mailbox/Makefile| 2 + > drivers/mailbox/tegra-hsp.c | 418 > > 3 files changed, 429 insertions(+) > create mode 100644 drivers/mailbox/tegra-hsp.c > > diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig > index 5305923752d2..fe584cb54720 100644 > --- a/drivers/mailbox/Kconfig > +++ b/drivers/mailbox/Kconfig > @@ -114,6 +114,15 @@ config MAILBOX_TEST > Test client to help with testing new Controller driver > implementations. > > +config TEGRA_HSP_MBOX > + bool "Tegra HSP(Hardware Synchronization Primitives) Driver" > >>> > >>> > >>>Space missing before the opening parenthesis (same in the patch title > >>>btw). > >> > >>Okay. > >>> > >>> > + depends on ARCH_TEGRA_186_SOC > + help > + The Tegra HSP driver is used for the interprocessor > communication > + between different remote processors and host processors on > Tegra186 > + and later SoCs. Say Y here if you want to have this support. > + If unsure say N. > >>> > >>> > >>>Since this option is selected automatically by ARCH_TEGRA_186_SOC, you > >>>should probably drop the last 2 sentences. > >> > >>Okay. > >> > >>> > + > config XGENE_SLIMPRO_MBOX > tristate "APM SoC X-Gene SLIMpro Mailbox Controller" > depends on ARCH_XGENE > diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile > index 0be3e742bb7d..26d8f91c7fea 100644 > --- a/drivers/mailbox/Makefile > +++ b/drivers/mailbox/Makefile > @@ -25,3 +25,5 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o > obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o > > obj-$(CONFIG_HI6220_MBOX) += hi6220-mailbox.o > + > +obj-${CONFIG_TEGRA_HSP_MBOX} += tegra-hsp.o > diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c > new file mode 100644 > index ..93c3ef58f29f > --- /dev/null > +++ b/drivers/mailbox/tegra-hsp.c > @@ -0,0 +1,418 @@ > +/* > + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but > WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License > for > + * more details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define HSP_INT_DIMENSIONING 0x380 > +#define HSP_nSM_OFFSET 0 > +#define HSP_nSS_OFFSET 4 > +#define HSP_nAS_OFFSET 8 > +#define HSP_nDB_OFFSET 12 > +#define HSP_nSI_OFFSET 16 > >>> > >>> > >>>Would be nice to have comments to understand what SM, SS, AS, etc. > >>>stand for (Shared Mailboxes, Shared Semaphores, Arbitrated Semaphores > >>>but you need to look at the patch description to understand that). A > >>>top-of-file comment explaning the necessary concepts to read this code > >>>would do the trick. > >> > >>Yes, will fix that. > >>> > >>> > +#define HSP_nINT_MASK 0xf > + > +#define HSP_DB_REG_TRIGGER 0x0 > +#define HSP_DB_REG_ENABLE 0x4 > +#define HSP_DB_REG_RAW 0x8 > +#define HSP_DB_REG_PENDING 0xc > + > +#define
Re: [PATCH V2 02/10] mailbox: tegra-hsp: Add HSP(Hardware Synchronization Primitives) driver
On Tue, Jul 05, 2016 at 05:04:23PM +0800, Joseph Lo wrote: > The Tegra HSP mailbox driver implements the signaling doorbell-based > interprocessor communication (IPC) for remote processors currently. The > HSP HW modules support some different features for that, which are > shared mailboxes, shared semaphores, arbitrated semaphores, and > doorbells. And there are multiple HSP HW instances on the chip. So the > driver is extendable to support more features for different IPC > requirement. > > The driver of remote processor can use it as a mailbox client and deal > with the IPC protocol to synchronize the data communications. > > Signed-off-by: Joseph Lo> --- > Changes in V2: > - Update the driver to support the binding changes in V2 > - it's extendable to support multiple HSP sub-modules on the same HSP HW block > now. > --- > drivers/mailbox/Kconfig | 9 + > drivers/mailbox/Makefile| 2 + > drivers/mailbox/tegra-hsp.c | 418 > > 3 files changed, 429 insertions(+) > create mode 100644 drivers/mailbox/tegra-hsp.c > > diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig > index 5305923752d2..fe584cb54720 100644 > --- a/drivers/mailbox/Kconfig > +++ b/drivers/mailbox/Kconfig > @@ -114,6 +114,15 @@ config MAILBOX_TEST > Test client to help with testing new Controller driver > implementations. > > +config TEGRA_HSP_MBOX > + bool "Tegra HSP(Hardware Synchronization Primitives) Driver" > + depends on ARCH_TEGRA_186_SOC > + help > + The Tegra HSP driver is used for the interprocessor communication > + between different remote processors and host processors on Tegra186 > + and later SoCs. Say Y here if you want to have this support. > + If unsure say N. > + > config XGENE_SLIMPRO_MBOX > tristate "APM SoC X-Gene SLIMpro Mailbox Controller" > depends on ARCH_XGENE > diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile > index 0be3e742bb7d..26d8f91c7fea 100644 > --- a/drivers/mailbox/Makefile > +++ b/drivers/mailbox/Makefile > @@ -25,3 +25,5 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o > obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o > > obj-$(CONFIG_HI6220_MBOX)+= hi6220-mailbox.o > + > +obj-${CONFIG_TEGRA_HSP_MBOX} += tegra-hsp.o > diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c > new file mode 100644 > index ..93c3ef58f29f > --- /dev/null > +++ b/drivers/mailbox/tegra-hsp.c > @@ -0,0 +1,418 @@ > +/* > + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define HSP_INT_DIMENSIONING 0x380 > +#define HSP_nSM_OFFSET 0 > +#define HSP_nSS_OFFSET 4 > +#define HSP_nAS_OFFSET 8 > +#define HSP_nDB_OFFSET 12 > +#define HSP_nSI_OFFSET 16 > +#define HSP_nINT_MASK0xf > + > +#define HSP_DB_REG_TRIGGER 0x0 > +#define HSP_DB_REG_ENABLE0x4 > +#define HSP_DB_REG_RAW 0x8 > +#define HSP_DB_REG_PENDING 0xc > + > +#define HSP_DB_CCPLEX1 > +#define HSP_DB_BPMP 3 > + > +#define MAX_NUM_HSP_CHAN 32 Is this an arbitrarily chosen number? > +#define MAX_NUM_HSP_DB 7 > + > +#define hsp_db_offset(i, d) \ > + (d->base + ((1 + (d->nr_sm >> 1) + d->nr_ss + d->nr_as) << 16) + \ > + (i) * 0x100) > + > +struct tegra_hsp_db_chan { > + int master_id; > + int db_id; These should be unsigned? > +}; > + > +struct tegra_hsp_mbox_chan { > + int type; This too... > + union { > + struct tegra_hsp_db_chan db_chan; > + }; > +}; Why do we need to use a union? > + > +struct tegra_hsp_mbox { > + struct mbox_controller *mbox; > + void __iomem *base; > + void __iomem *db_base[MAX_NUM_HSP_DB]; > + int db_irq; > + int nr_sm; > + int nr_as; > + int nr_ss; > + int nr_db; > + int nr_si; > + spinlock_t lock; You might need to change this to a mutex - see below. > +}; > + > +static inline u32 hsp_readl(void __iomem *base, int reg) > +{ > + return readl(base + reg); > +} > + > +static inline void hsp_writel(void __iomem *base, int reg, u32 val) > +{ > + writel(val, base + reg); > + readl(base + reg); > +} > + > +static int hsp_db_can_ring(void __iomem *db_base) > +{ > + u32 reg; > + > + reg = hsp_readl(db_base,
Re: [PATCH V2 02/10] mailbox: tegra-hsp: Add HSP(Hardware Synchronization Primitives) driver
On Tue, Jul 05, 2016 at 05:04:23PM +0800, Joseph Lo wrote: > The Tegra HSP mailbox driver implements the signaling doorbell-based > interprocessor communication (IPC) for remote processors currently. The > HSP HW modules support some different features for that, which are > shared mailboxes, shared semaphores, arbitrated semaphores, and > doorbells. And there are multiple HSP HW instances on the chip. So the > driver is extendable to support more features for different IPC > requirement. > > The driver of remote processor can use it as a mailbox client and deal > with the IPC protocol to synchronize the data communications. > > Signed-off-by: Joseph Lo > --- > Changes in V2: > - Update the driver to support the binding changes in V2 > - it's extendable to support multiple HSP sub-modules on the same HSP HW block > now. > --- > drivers/mailbox/Kconfig | 9 + > drivers/mailbox/Makefile| 2 + > drivers/mailbox/tegra-hsp.c | 418 > > 3 files changed, 429 insertions(+) > create mode 100644 drivers/mailbox/tegra-hsp.c > > diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig > index 5305923752d2..fe584cb54720 100644 > --- a/drivers/mailbox/Kconfig > +++ b/drivers/mailbox/Kconfig > @@ -114,6 +114,15 @@ config MAILBOX_TEST > Test client to help with testing new Controller driver > implementations. > > +config TEGRA_HSP_MBOX > + bool "Tegra HSP(Hardware Synchronization Primitives) Driver" > + depends on ARCH_TEGRA_186_SOC > + help > + The Tegra HSP driver is used for the interprocessor communication > + between different remote processors and host processors on Tegra186 > + and later SoCs. Say Y here if you want to have this support. > + If unsure say N. > + > config XGENE_SLIMPRO_MBOX > tristate "APM SoC X-Gene SLIMpro Mailbox Controller" > depends on ARCH_XGENE > diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile > index 0be3e742bb7d..26d8f91c7fea 100644 > --- a/drivers/mailbox/Makefile > +++ b/drivers/mailbox/Makefile > @@ -25,3 +25,5 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o > obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o > > obj-$(CONFIG_HI6220_MBOX)+= hi6220-mailbox.o > + > +obj-${CONFIG_TEGRA_HSP_MBOX} += tegra-hsp.o > diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c > new file mode 100644 > index ..93c3ef58f29f > --- /dev/null > +++ b/drivers/mailbox/tegra-hsp.c > @@ -0,0 +1,418 @@ > +/* > + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define HSP_INT_DIMENSIONING 0x380 > +#define HSP_nSM_OFFSET 0 > +#define HSP_nSS_OFFSET 4 > +#define HSP_nAS_OFFSET 8 > +#define HSP_nDB_OFFSET 12 > +#define HSP_nSI_OFFSET 16 > +#define HSP_nINT_MASK0xf > + > +#define HSP_DB_REG_TRIGGER 0x0 > +#define HSP_DB_REG_ENABLE0x4 > +#define HSP_DB_REG_RAW 0x8 > +#define HSP_DB_REG_PENDING 0xc > + > +#define HSP_DB_CCPLEX1 > +#define HSP_DB_BPMP 3 > + > +#define MAX_NUM_HSP_CHAN 32 Is this an arbitrarily chosen number? > +#define MAX_NUM_HSP_DB 7 > + > +#define hsp_db_offset(i, d) \ > + (d->base + ((1 + (d->nr_sm >> 1) + d->nr_ss + d->nr_as) << 16) + \ > + (i) * 0x100) > + > +struct tegra_hsp_db_chan { > + int master_id; > + int db_id; These should be unsigned? > +}; > + > +struct tegra_hsp_mbox_chan { > + int type; This too... > + union { > + struct tegra_hsp_db_chan db_chan; > + }; > +}; Why do we need to use a union? > + > +struct tegra_hsp_mbox { > + struct mbox_controller *mbox; > + void __iomem *base; > + void __iomem *db_base[MAX_NUM_HSP_DB]; > + int db_irq; > + int nr_sm; > + int nr_as; > + int nr_ss; > + int nr_db; > + int nr_si; > + spinlock_t lock; You might need to change this to a mutex - see below. > +}; > + > +static inline u32 hsp_readl(void __iomem *base, int reg) > +{ > + return readl(base + reg); > +} > + > +static inline void hsp_writel(void __iomem *base, int reg, u32 val) > +{ > + writel(val, base + reg); > + readl(base + reg); > +} > + > +static int hsp_db_can_ring(void __iomem *db_base) > +{ > + u32 reg; > + > + reg = hsp_readl(db_base, HSP_DB_REG_ENABLE); >
Re: [PATCH V2 02/10] mailbox: tegra-hsp: Add HSP(Hardware Synchronization Primitives) driver
On 07/07/2016 12:50 AM, Stephen Warren wrote: On 07/06/2016 03:06 AM, Joseph Lo wrote: On 07/06/2016 03:05 PM, Alexandre Courbot wrote: On Tue, Jul 5, 2016 at 6:04 PM, Joseph Lowrote: The Tegra HSP mailbox driver implements the signaling doorbell-based interprocessor communication (IPC) for remote processors currently. The HSP HW modules support some different features for that, which are shared mailboxes, shared semaphores, arbitrated semaphores, and doorbells. And there are multiple HSP HW instances on the chip. So the driver is extendable to support more features for different IPC requirement. The driver of remote processor can use it as a mailbox client and deal with the IPC protocol to synchronize the data communications. diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c +static irqreturn_t hsp_db_irq(int irq, void *p) +{ + struct tegra_hsp_mbox *hsp_mbox = p; + ulong val; + int master_id; + + val = (ulong)hsp_readl(hsp_mbox->db_base[HSP_DB_CCPLEX], + HSP_DB_REG_PENDING); + hsp_writel(hsp_mbox->db_base[HSP_DB_CCPLEX], HSP_DB_REG_PENDING, val); + + spin_lock(_mbox->lock); + for_each_set_bit(master_id, , MAX_NUM_HSP_CHAN) { + struct mbox_chan *chan; + struct tegra_hsp_mbox_chan *mchan; + int i; + + for (i = 0; i < MAX_NUM_HSP_CHAN; i++) { I wonder if this could not be optimized. You are doing a double loop on MAX_NUM_HSP_CHAN to look for an identical master_id. Since it seems like the same master_id cannot be used twice (considering that the inner loop only processes the first match), couldn't you just select the free channel in of_hsp_mbox_xlate() by doing >chans[master_id] (and returning an error if it is already used), then simply getting chan as _mbox->mbox->chans[master_id] instead of having the inner loop below? That would remove the need for the second loop. That was exactly what I did in the V1, which only supported one HSP sub-module per HSP HW block. So we can just use the master_id as the mbox channel ID. Meanwhile, the V2 is purposed to support multiple HSP sub-modules to be running on the same HSP HW block. The "ID" between different modules could be conflict. So I dropped the mechanism that used the master_id as the mbox channel ID. I haven't looked at the code in this patch since I'm mainly concerned about the DT bindings. However, I will say that nothing in the change to the mailbox specifier in DT should have required /any/ changes to the code, except to add a single check to validate that the "mailbox type" encoded into the top 16 bits of the mailbox ID were 0, and hence represented a doorbell rather than anything else. Any enhancements to support other mailbox types could have happened later, and I doubt would require anything dynamic even then. Yes, I only add the code for that change. Maybe some glue code for the extend-ability to support more HSP modules in the future. +static int tegra_hsp_db_init(struct tegra_hsp_mbox *hsp_mbox, +struct mbox_chan *mchan, int master_id) +{ + struct platform_device *pdev = to_platform_device(hsp_mbox->mbox->dev); + struct tegra_hsp_mbox_chan *hsp_mbox_chan; + int ret; + + if (!hsp_mbox->db_irq) { + int i; + + hsp_mbox->db_irq = platform_get_irq_byname(pdev, "doorbell"); Getting the IRQ sounds more like a job for probe() - I don't see the benefit of lazy-doing it? We only need the IRQ when the client is requesting the DB service. For other HSP sub-modules, they are using different IRQ. So I didn't do that at probe time. All resources provided by other devices/drivers must be acquired at probe time, since that's the only time it's possible to defer probe if the provider of the resource is not available. If you don't follow that rule, what happens is: 1) This driver probes. 2) Some other driver calls tegra_hsp_db_init(), and it fails since the provider of the IRQ is not yet available. This likely ends up returning something other than -EPROBE_DEFER since the HSP driver was found successfully (thus there is no deferred probe situation as far as the mailbox core is concerned), it's just that the mailbox channel lookup/init/... failed. 3) The other driver's probe() fails due to this, but since the error wasn't a probe deferral, the other driver's probe() is never retried. Agree, will fix this. Thanks, -Joseph
Re: [PATCH V2 02/10] mailbox: tegra-hsp: Add HSP(Hardware Synchronization Primitives) driver
On 07/07/2016 12:50 AM, Stephen Warren wrote: On 07/06/2016 03:06 AM, Joseph Lo wrote: On 07/06/2016 03:05 PM, Alexandre Courbot wrote: On Tue, Jul 5, 2016 at 6:04 PM, Joseph Lo wrote: The Tegra HSP mailbox driver implements the signaling doorbell-based interprocessor communication (IPC) for remote processors currently. The HSP HW modules support some different features for that, which are shared mailboxes, shared semaphores, arbitrated semaphores, and doorbells. And there are multiple HSP HW instances on the chip. So the driver is extendable to support more features for different IPC requirement. The driver of remote processor can use it as a mailbox client and deal with the IPC protocol to synchronize the data communications. diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c +static irqreturn_t hsp_db_irq(int irq, void *p) +{ + struct tegra_hsp_mbox *hsp_mbox = p; + ulong val; + int master_id; + + val = (ulong)hsp_readl(hsp_mbox->db_base[HSP_DB_CCPLEX], + HSP_DB_REG_PENDING); + hsp_writel(hsp_mbox->db_base[HSP_DB_CCPLEX], HSP_DB_REG_PENDING, val); + + spin_lock(_mbox->lock); + for_each_set_bit(master_id, , MAX_NUM_HSP_CHAN) { + struct mbox_chan *chan; + struct tegra_hsp_mbox_chan *mchan; + int i; + + for (i = 0; i < MAX_NUM_HSP_CHAN; i++) { I wonder if this could not be optimized. You are doing a double loop on MAX_NUM_HSP_CHAN to look for an identical master_id. Since it seems like the same master_id cannot be used twice (considering that the inner loop only processes the first match), couldn't you just select the free channel in of_hsp_mbox_xlate() by doing >chans[master_id] (and returning an error if it is already used), then simply getting chan as _mbox->mbox->chans[master_id] instead of having the inner loop below? That would remove the need for the second loop. That was exactly what I did in the V1, which only supported one HSP sub-module per HSP HW block. So we can just use the master_id as the mbox channel ID. Meanwhile, the V2 is purposed to support multiple HSP sub-modules to be running on the same HSP HW block. The "ID" between different modules could be conflict. So I dropped the mechanism that used the master_id as the mbox channel ID. I haven't looked at the code in this patch since I'm mainly concerned about the DT bindings. However, I will say that nothing in the change to the mailbox specifier in DT should have required /any/ changes to the code, except to add a single check to validate that the "mailbox type" encoded into the top 16 bits of the mailbox ID were 0, and hence represented a doorbell rather than anything else. Any enhancements to support other mailbox types could have happened later, and I doubt would require anything dynamic even then. Yes, I only add the code for that change. Maybe some glue code for the extend-ability to support more HSP modules in the future. +static int tegra_hsp_db_init(struct tegra_hsp_mbox *hsp_mbox, +struct mbox_chan *mchan, int master_id) +{ + struct platform_device *pdev = to_platform_device(hsp_mbox->mbox->dev); + struct tegra_hsp_mbox_chan *hsp_mbox_chan; + int ret; + + if (!hsp_mbox->db_irq) { + int i; + + hsp_mbox->db_irq = platform_get_irq_byname(pdev, "doorbell"); Getting the IRQ sounds more like a job for probe() - I don't see the benefit of lazy-doing it? We only need the IRQ when the client is requesting the DB service. For other HSP sub-modules, they are using different IRQ. So I didn't do that at probe time. All resources provided by other devices/drivers must be acquired at probe time, since that's the only time it's possible to defer probe if the provider of the resource is not available. If you don't follow that rule, what happens is: 1) This driver probes. 2) Some other driver calls tegra_hsp_db_init(), and it fails since the provider of the IRQ is not yet available. This likely ends up returning something other than -EPROBE_DEFER since the HSP driver was found successfully (thus there is no deferred probe situation as far as the mailbox core is concerned), it's just that the mailbox channel lookup/init/... failed. 3) The other driver's probe() fails due to this, but since the error wasn't a probe deferral, the other driver's probe() is never retried. Agree, will fix this. Thanks, -Joseph
Re: [PATCH V2 02/10] mailbox: tegra-hsp: Add HSP(Hardware Synchronization Primitives) driver
On 07/06/2016 08:23 PM, Alexandre Courbot wrote: On Wed, Jul 6, 2016 at 6:06 PM, Joseph Lowrote: On 07/06/2016 03:05 PM, Alexandre Courbot wrote: On Tue, Jul 5, 2016 at 6:04 PM, Joseph Lo wrote: The Tegra HSP mailbox driver implements the signaling doorbell-based interprocessor communication (IPC) for remote processors currently. The HSP HW modules support some different features for that, which are shared mailboxes, shared semaphores, arbitrated semaphores, and doorbells. And there are multiple HSP HW instances on the chip. So the driver is extendable to support more features for different IPC requirement. The driver of remote processor can use it as a mailbox client and deal with the IPC protocol to synchronize the data communications. Signed-off-by: Joseph Lo --- Changes in V2: - Update the driver to support the binding changes in V2 - it's extendable to support multiple HSP sub-modules on the same HSP HW block now. --- drivers/mailbox/Kconfig | 9 + drivers/mailbox/Makefile| 2 + drivers/mailbox/tegra-hsp.c | 418 3 files changed, 429 insertions(+) create mode 100644 drivers/mailbox/tegra-hsp.c diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig index 5305923752d2..fe584cb54720 100644 --- a/drivers/mailbox/Kconfig +++ b/drivers/mailbox/Kconfig @@ -114,6 +114,15 @@ config MAILBOX_TEST Test client to help with testing new Controller driver implementations. +config TEGRA_HSP_MBOX + bool "Tegra HSP(Hardware Synchronization Primitives) Driver" Space missing before the opening parenthesis (same in the patch title btw). Okay. + depends on ARCH_TEGRA_186_SOC + help + The Tegra HSP driver is used for the interprocessor communication + between different remote processors and host processors on Tegra186 + and later SoCs. Say Y here if you want to have this support. + If unsure say N. Since this option is selected automatically by ARCH_TEGRA_186_SOC, you should probably drop the last 2 sentences. Okay. + config XGENE_SLIMPRO_MBOX tristate "APM SoC X-Gene SLIMpro Mailbox Controller" depends on ARCH_XGENE diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile index 0be3e742bb7d..26d8f91c7fea 100644 --- a/drivers/mailbox/Makefile +++ b/drivers/mailbox/Makefile @@ -25,3 +25,5 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o obj-$(CONFIG_HI6220_MBOX) += hi6220-mailbox.o + +obj-${CONFIG_TEGRA_HSP_MBOX} += tegra-hsp.o diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c new file mode 100644 index ..93c3ef58f29f --- /dev/null +++ b/drivers/mailbox/tegra-hsp.c @@ -0,0 +1,418 @@ +/* + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + */ + +#include +#include +#include +#include +#include +#include +#include + +#define HSP_INT_DIMENSIONING 0x380 +#define HSP_nSM_OFFSET 0 +#define HSP_nSS_OFFSET 4 +#define HSP_nAS_OFFSET 8 +#define HSP_nDB_OFFSET 12 +#define HSP_nSI_OFFSET 16 Would be nice to have comments to understand what SM, SS, AS, etc. stand for (Shared Mailboxes, Shared Semaphores, Arbitrated Semaphores but you need to look at the patch description to understand that). A top-of-file comment explaning the necessary concepts to read this code would do the trick. Yes, will fix that. +#define HSP_nINT_MASK 0xf + +#define HSP_DB_REG_TRIGGER 0x0 +#define HSP_DB_REG_ENABLE 0x4 +#define HSP_DB_REG_RAW 0x8 +#define HSP_DB_REG_PENDING 0xc + +#define HSP_DB_CCPLEX 1 +#define HSP_DB_BPMP3 Maybe turn this into enum and use that type for tegra_hsp_db_chan::db_id? Also have MAX_NUM_HSP_DB here, since it is related to these values? Okay. + +#define MAX_NUM_HSP_CHAN 32 +#define MAX_NUM_HSP_DB 7 + +#define hsp_db_offset(i, d) \ + (d->base + ((1 + (d->nr_sm >> 1) + d->nr_ss + d->nr_as) << 16) + \ + (i) * 0x100) + +struct tegra_hsp_db_chan { + int master_id; + int db_id; +}; + +struct tegra_hsp_mbox_chan { + int type; + union { + struct tegra_hsp_db_chan db_chan; + }; +}; + +struct tegra_hsp_mbox { + struct mbox_controller *mbox; + void __iomem *base; + void __iomem *db_base[MAX_NUM_HSP_DB]; +
Re: [PATCH V2 02/10] mailbox: tegra-hsp: Add HSP(Hardware Synchronization Primitives) driver
On 07/06/2016 08:23 PM, Alexandre Courbot wrote: On Wed, Jul 6, 2016 at 6:06 PM, Joseph Lo wrote: On 07/06/2016 03:05 PM, Alexandre Courbot wrote: On Tue, Jul 5, 2016 at 6:04 PM, Joseph Lo wrote: The Tegra HSP mailbox driver implements the signaling doorbell-based interprocessor communication (IPC) for remote processors currently. The HSP HW modules support some different features for that, which are shared mailboxes, shared semaphores, arbitrated semaphores, and doorbells. And there are multiple HSP HW instances on the chip. So the driver is extendable to support more features for different IPC requirement. The driver of remote processor can use it as a mailbox client and deal with the IPC protocol to synchronize the data communications. Signed-off-by: Joseph Lo --- Changes in V2: - Update the driver to support the binding changes in V2 - it's extendable to support multiple HSP sub-modules on the same HSP HW block now. --- drivers/mailbox/Kconfig | 9 + drivers/mailbox/Makefile| 2 + drivers/mailbox/tegra-hsp.c | 418 3 files changed, 429 insertions(+) create mode 100644 drivers/mailbox/tegra-hsp.c diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig index 5305923752d2..fe584cb54720 100644 --- a/drivers/mailbox/Kconfig +++ b/drivers/mailbox/Kconfig @@ -114,6 +114,15 @@ config MAILBOX_TEST Test client to help with testing new Controller driver implementations. +config TEGRA_HSP_MBOX + bool "Tegra HSP(Hardware Synchronization Primitives) Driver" Space missing before the opening parenthesis (same in the patch title btw). Okay. + depends on ARCH_TEGRA_186_SOC + help + The Tegra HSP driver is used for the interprocessor communication + between different remote processors and host processors on Tegra186 + and later SoCs. Say Y here if you want to have this support. + If unsure say N. Since this option is selected automatically by ARCH_TEGRA_186_SOC, you should probably drop the last 2 sentences. Okay. + config XGENE_SLIMPRO_MBOX tristate "APM SoC X-Gene SLIMpro Mailbox Controller" depends on ARCH_XGENE diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile index 0be3e742bb7d..26d8f91c7fea 100644 --- a/drivers/mailbox/Makefile +++ b/drivers/mailbox/Makefile @@ -25,3 +25,5 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o obj-$(CONFIG_HI6220_MBOX) += hi6220-mailbox.o + +obj-${CONFIG_TEGRA_HSP_MBOX} += tegra-hsp.o diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c new file mode 100644 index ..93c3ef58f29f --- /dev/null +++ b/drivers/mailbox/tegra-hsp.c @@ -0,0 +1,418 @@ +/* + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + */ + +#include +#include +#include +#include +#include +#include +#include + +#define HSP_INT_DIMENSIONING 0x380 +#define HSP_nSM_OFFSET 0 +#define HSP_nSS_OFFSET 4 +#define HSP_nAS_OFFSET 8 +#define HSP_nDB_OFFSET 12 +#define HSP_nSI_OFFSET 16 Would be nice to have comments to understand what SM, SS, AS, etc. stand for (Shared Mailboxes, Shared Semaphores, Arbitrated Semaphores but you need to look at the patch description to understand that). A top-of-file comment explaning the necessary concepts to read this code would do the trick. Yes, will fix that. +#define HSP_nINT_MASK 0xf + +#define HSP_DB_REG_TRIGGER 0x0 +#define HSP_DB_REG_ENABLE 0x4 +#define HSP_DB_REG_RAW 0x8 +#define HSP_DB_REG_PENDING 0xc + +#define HSP_DB_CCPLEX 1 +#define HSP_DB_BPMP3 Maybe turn this into enum and use that type for tegra_hsp_db_chan::db_id? Also have MAX_NUM_HSP_DB here, since it is related to these values? Okay. + +#define MAX_NUM_HSP_CHAN 32 +#define MAX_NUM_HSP_DB 7 + +#define hsp_db_offset(i, d) \ + (d->base + ((1 + (d->nr_sm >> 1) + d->nr_ss + d->nr_as) << 16) + \ + (i) * 0x100) + +struct tegra_hsp_db_chan { + int master_id; + int db_id; +}; + +struct tegra_hsp_mbox_chan { + int type; + union { + struct tegra_hsp_db_chan db_chan; + }; +}; + +struct tegra_hsp_mbox { + struct mbox_controller *mbox; + void __iomem *base; + void __iomem *db_base[MAX_NUM_HSP_DB]; + int db_irq; + int nr_sm; + int nr_as; +
Re: [PATCH V2 02/10] mailbox: tegra-hsp: Add HSP(Hardware Synchronization Primitives) driver
On 07/06/2016 03:06 AM, Joseph Lo wrote: On 07/06/2016 03:05 PM, Alexandre Courbot wrote: On Tue, Jul 5, 2016 at 6:04 PM, Joseph Lowrote: The Tegra HSP mailbox driver implements the signaling doorbell-based interprocessor communication (IPC) for remote processors currently. The HSP HW modules support some different features for that, which are shared mailboxes, shared semaphores, arbitrated semaphores, and doorbells. And there are multiple HSP HW instances on the chip. So the driver is extendable to support more features for different IPC requirement. The driver of remote processor can use it as a mailbox client and deal with the IPC protocol to synchronize the data communications. diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c +static irqreturn_t hsp_db_irq(int irq, void *p) +{ + struct tegra_hsp_mbox *hsp_mbox = p; + ulong val; + int master_id; + + val = (ulong)hsp_readl(hsp_mbox->db_base[HSP_DB_CCPLEX], + HSP_DB_REG_PENDING); + hsp_writel(hsp_mbox->db_base[HSP_DB_CCPLEX], HSP_DB_REG_PENDING, val); + + spin_lock(_mbox->lock); + for_each_set_bit(master_id, , MAX_NUM_HSP_CHAN) { + struct mbox_chan *chan; + struct tegra_hsp_mbox_chan *mchan; + int i; + + for (i = 0; i < MAX_NUM_HSP_CHAN; i++) { I wonder if this could not be optimized. You are doing a double loop on MAX_NUM_HSP_CHAN to look for an identical master_id. Since it seems like the same master_id cannot be used twice (considering that the inner loop only processes the first match), couldn't you just select the free channel in of_hsp_mbox_xlate() by doing >chans[master_id] (and returning an error if it is already used), then simply getting chan as _mbox->mbox->chans[master_id] instead of having the inner loop below? That would remove the need for the second loop. That was exactly what I did in the V1, which only supported one HSP sub-module per HSP HW block. So we can just use the master_id as the mbox channel ID. Meanwhile, the V2 is purposed to support multiple HSP sub-modules to be running on the same HSP HW block. The "ID" between different modules could be conflict. So I dropped the mechanism that used the master_id as the mbox channel ID. I haven't looked at the code in this patch since I'm mainly concerned about the DT bindings. However, I will say that nothing in the change to the mailbox specifier in DT should have required /any/ changes to the code, except to add a single check to validate that the "mailbox type" encoded into the top 16 bits of the mailbox ID were 0, and hence represented a doorbell rather than anything else. Any enhancements to support other mailbox types could have happened later, and I doubt would require anything dynamic even then. +static int tegra_hsp_db_init(struct tegra_hsp_mbox *hsp_mbox, +struct mbox_chan *mchan, int master_id) +{ + struct platform_device *pdev = to_platform_device(hsp_mbox->mbox->dev); + struct tegra_hsp_mbox_chan *hsp_mbox_chan; + int ret; + + if (!hsp_mbox->db_irq) { + int i; + + hsp_mbox->db_irq = platform_get_irq_byname(pdev, "doorbell"); Getting the IRQ sounds more like a job for probe() - I don't see the benefit of lazy-doing it? We only need the IRQ when the client is requesting the DB service. For other HSP sub-modules, they are using different IRQ. So I didn't do that at probe time. All resources provided by other devices/drivers must be acquired at probe time, since that's the only time it's possible to defer probe if the provider of the resource is not available. If you don't follow that rule, what happens is: 1) This driver probes. 2) Some other driver calls tegra_hsp_db_init(), and it fails since the provider of the IRQ is not yet available. This likely ends up returning something other than -EPROBE_DEFER since the HSP driver was found successfully (thus there is no deferred probe situation as far as the mailbox core is concerned), it's just that the mailbox channel lookup/init/... failed. 3) The other driver's probe() fails due to this, but since the error wasn't a probe deferral, the other driver's probe() is never retried.
Re: [PATCH V2 02/10] mailbox: tegra-hsp: Add HSP(Hardware Synchronization Primitives) driver
On 07/06/2016 03:06 AM, Joseph Lo wrote: On 07/06/2016 03:05 PM, Alexandre Courbot wrote: On Tue, Jul 5, 2016 at 6:04 PM, Joseph Lo wrote: The Tegra HSP mailbox driver implements the signaling doorbell-based interprocessor communication (IPC) for remote processors currently. The HSP HW modules support some different features for that, which are shared mailboxes, shared semaphores, arbitrated semaphores, and doorbells. And there are multiple HSP HW instances on the chip. So the driver is extendable to support more features for different IPC requirement. The driver of remote processor can use it as a mailbox client and deal with the IPC protocol to synchronize the data communications. diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c +static irqreturn_t hsp_db_irq(int irq, void *p) +{ + struct tegra_hsp_mbox *hsp_mbox = p; + ulong val; + int master_id; + + val = (ulong)hsp_readl(hsp_mbox->db_base[HSP_DB_CCPLEX], + HSP_DB_REG_PENDING); + hsp_writel(hsp_mbox->db_base[HSP_DB_CCPLEX], HSP_DB_REG_PENDING, val); + + spin_lock(_mbox->lock); + for_each_set_bit(master_id, , MAX_NUM_HSP_CHAN) { + struct mbox_chan *chan; + struct tegra_hsp_mbox_chan *mchan; + int i; + + for (i = 0; i < MAX_NUM_HSP_CHAN; i++) { I wonder if this could not be optimized. You are doing a double loop on MAX_NUM_HSP_CHAN to look for an identical master_id. Since it seems like the same master_id cannot be used twice (considering that the inner loop only processes the first match), couldn't you just select the free channel in of_hsp_mbox_xlate() by doing >chans[master_id] (and returning an error if it is already used), then simply getting chan as _mbox->mbox->chans[master_id] instead of having the inner loop below? That would remove the need for the second loop. That was exactly what I did in the V1, which only supported one HSP sub-module per HSP HW block. So we can just use the master_id as the mbox channel ID. Meanwhile, the V2 is purposed to support multiple HSP sub-modules to be running on the same HSP HW block. The "ID" between different modules could be conflict. So I dropped the mechanism that used the master_id as the mbox channel ID. I haven't looked at the code in this patch since I'm mainly concerned about the DT bindings. However, I will say that nothing in the change to the mailbox specifier in DT should have required /any/ changes to the code, except to add a single check to validate that the "mailbox type" encoded into the top 16 bits of the mailbox ID were 0, and hence represented a doorbell rather than anything else. Any enhancements to support other mailbox types could have happened later, and I doubt would require anything dynamic even then. +static int tegra_hsp_db_init(struct tegra_hsp_mbox *hsp_mbox, +struct mbox_chan *mchan, int master_id) +{ + struct platform_device *pdev = to_platform_device(hsp_mbox->mbox->dev); + struct tegra_hsp_mbox_chan *hsp_mbox_chan; + int ret; + + if (!hsp_mbox->db_irq) { + int i; + + hsp_mbox->db_irq = platform_get_irq_byname(pdev, "doorbell"); Getting the IRQ sounds more like a job for probe() - I don't see the benefit of lazy-doing it? We only need the IRQ when the client is requesting the DB service. For other HSP sub-modules, they are using different IRQ. So I didn't do that at probe time. All resources provided by other devices/drivers must be acquired at probe time, since that's the only time it's possible to defer probe if the provider of the resource is not available. If you don't follow that rule, what happens is: 1) This driver probes. 2) Some other driver calls tegra_hsp_db_init(), and it fails since the provider of the IRQ is not yet available. This likely ends up returning something other than -EPROBE_DEFER since the HSP driver was found successfully (thus there is no deferred probe situation as far as the mailbox core is concerned), it's just that the mailbox channel lookup/init/... failed. 3) The other driver's probe() fails due to this, but since the error wasn't a probe deferral, the other driver's probe() is never retried.
Re: [PATCH V2 02/10] mailbox: tegra-hsp: Add HSP(Hardware Synchronization Primitives) driver
On Wed, Jul 6, 2016 at 6:06 PM, Joseph Lowrote: > On 07/06/2016 03:05 PM, Alexandre Courbot wrote: >> >> On Tue, Jul 5, 2016 at 6:04 PM, Joseph Lo wrote: >>> >>> The Tegra HSP mailbox driver implements the signaling doorbell-based >>> interprocessor communication (IPC) for remote processors currently. The >>> HSP HW modules support some different features for that, which are >>> shared mailboxes, shared semaphores, arbitrated semaphores, and >>> doorbells. And there are multiple HSP HW instances on the chip. So the >>> driver is extendable to support more features for different IPC >>> requirement. >>> >>> The driver of remote processor can use it as a mailbox client and deal >>> with the IPC protocol to synchronize the data communications. >>> >>> Signed-off-by: Joseph Lo >>> --- >>> Changes in V2: >>> - Update the driver to support the binding changes in V2 >>> - it's extendable to support multiple HSP sub-modules on the same HSP HW >>> block >>>now. >>> --- >>> drivers/mailbox/Kconfig | 9 + >>> drivers/mailbox/Makefile| 2 + >>> drivers/mailbox/tegra-hsp.c | 418 >>> >>> 3 files changed, 429 insertions(+) >>> create mode 100644 drivers/mailbox/tegra-hsp.c >>> >>> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig >>> index 5305923752d2..fe584cb54720 100644 >>> --- a/drivers/mailbox/Kconfig >>> +++ b/drivers/mailbox/Kconfig >>> @@ -114,6 +114,15 @@ config MAILBOX_TEST >>>Test client to help with testing new Controller driver >>>implementations. >>> >>> +config TEGRA_HSP_MBOX >>> + bool "Tegra HSP(Hardware Synchronization Primitives) Driver" >> >> >> Space missing before the opening parenthesis (same in the patch title >> btw). > > Okay. >> >> >>> + depends on ARCH_TEGRA_186_SOC >>> + help >>> + The Tegra HSP driver is used for the interprocessor >>> communication >>> + between different remote processors and host processors on >>> Tegra186 >>> + and later SoCs. Say Y here if you want to have this support. >>> + If unsure say N. >> >> >> Since this option is selected automatically by ARCH_TEGRA_186_SOC, you >> should probably drop the last 2 sentences. > > Okay. > >> >>> + >>> config XGENE_SLIMPRO_MBOX >>> tristate "APM SoC X-Gene SLIMpro Mailbox Controller" >>> depends on ARCH_XGENE >>> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile >>> index 0be3e742bb7d..26d8f91c7fea 100644 >>> --- a/drivers/mailbox/Makefile >>> +++ b/drivers/mailbox/Makefile >>> @@ -25,3 +25,5 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o >>> obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o >>> >>> obj-$(CONFIG_HI6220_MBOX) += hi6220-mailbox.o >>> + >>> +obj-${CONFIG_TEGRA_HSP_MBOX} += tegra-hsp.o >>> diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c >>> new file mode 100644 >>> index ..93c3ef58f29f >>> --- /dev/null >>> +++ b/drivers/mailbox/tegra-hsp.c >>> @@ -0,0 +1,418 @@ >>> +/* >>> + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. >>> + * >>> + * This program is free software; you can redistribute it and/or modify >>> it >>> + * under the terms and conditions of the GNU General Public License, >>> + * version 2, as published by the Free Software Foundation. >>> + * >>> + * This program is distributed in the hope it will be useful, but >>> WITHOUT >>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or >>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License >>> for >>> + * more details. >>> + */ >>> + >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> + >>> +#define HSP_INT_DIMENSIONING 0x380 >>> +#define HSP_nSM_OFFSET 0 >>> +#define HSP_nSS_OFFSET 4 >>> +#define HSP_nAS_OFFSET 8 >>> +#define HSP_nDB_OFFSET 12 >>> +#define HSP_nSI_OFFSET 16 >> >> >> Would be nice to have comments to understand what SM, SS, AS, etc. >> stand for (Shared Mailboxes, Shared Semaphores, Arbitrated Semaphores >> but you need to look at the patch description to understand that). A >> top-of-file comment explaning the necessary concepts to read this code >> would do the trick. > > Yes, will fix that. >> >> >>> +#define HSP_nINT_MASK 0xf >>> + >>> +#define HSP_DB_REG_TRIGGER 0x0 >>> +#define HSP_DB_REG_ENABLE 0x4 >>> +#define HSP_DB_REG_RAW 0x8 >>> +#define HSP_DB_REG_PENDING 0xc >>> + >>> +#define HSP_DB_CCPLEX 1 >>> +#define HSP_DB_BPMP3 >> >> >> Maybe turn this into enum and use that type for >> tegra_hsp_db_chan::db_id? Also have MAX_NUM_HSP_DB here, since it is >> related to these values? > > Okay. > >> >>> + >>> +#define MAX_NUM_HSP_CHAN 32 >>> +#define MAX_NUM_HSP_DB 7 >>> + >>> +#define hsp_db_offset(i, d) \ >>> +
Re: [PATCH V2 02/10] mailbox: tegra-hsp: Add HSP(Hardware Synchronization Primitives) driver
On Wed, Jul 6, 2016 at 6:06 PM, Joseph Lo wrote: > On 07/06/2016 03:05 PM, Alexandre Courbot wrote: >> >> On Tue, Jul 5, 2016 at 6:04 PM, Joseph Lo wrote: >>> >>> The Tegra HSP mailbox driver implements the signaling doorbell-based >>> interprocessor communication (IPC) for remote processors currently. The >>> HSP HW modules support some different features for that, which are >>> shared mailboxes, shared semaphores, arbitrated semaphores, and >>> doorbells. And there are multiple HSP HW instances on the chip. So the >>> driver is extendable to support more features for different IPC >>> requirement. >>> >>> The driver of remote processor can use it as a mailbox client and deal >>> with the IPC protocol to synchronize the data communications. >>> >>> Signed-off-by: Joseph Lo >>> --- >>> Changes in V2: >>> - Update the driver to support the binding changes in V2 >>> - it's extendable to support multiple HSP sub-modules on the same HSP HW >>> block >>>now. >>> --- >>> drivers/mailbox/Kconfig | 9 + >>> drivers/mailbox/Makefile| 2 + >>> drivers/mailbox/tegra-hsp.c | 418 >>> >>> 3 files changed, 429 insertions(+) >>> create mode 100644 drivers/mailbox/tegra-hsp.c >>> >>> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig >>> index 5305923752d2..fe584cb54720 100644 >>> --- a/drivers/mailbox/Kconfig >>> +++ b/drivers/mailbox/Kconfig >>> @@ -114,6 +114,15 @@ config MAILBOX_TEST >>>Test client to help with testing new Controller driver >>>implementations. >>> >>> +config TEGRA_HSP_MBOX >>> + bool "Tegra HSP(Hardware Synchronization Primitives) Driver" >> >> >> Space missing before the opening parenthesis (same in the patch title >> btw). > > Okay. >> >> >>> + depends on ARCH_TEGRA_186_SOC >>> + help >>> + The Tegra HSP driver is used for the interprocessor >>> communication >>> + between different remote processors and host processors on >>> Tegra186 >>> + and later SoCs. Say Y here if you want to have this support. >>> + If unsure say N. >> >> >> Since this option is selected automatically by ARCH_TEGRA_186_SOC, you >> should probably drop the last 2 sentences. > > Okay. > >> >>> + >>> config XGENE_SLIMPRO_MBOX >>> tristate "APM SoC X-Gene SLIMpro Mailbox Controller" >>> depends on ARCH_XGENE >>> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile >>> index 0be3e742bb7d..26d8f91c7fea 100644 >>> --- a/drivers/mailbox/Makefile >>> +++ b/drivers/mailbox/Makefile >>> @@ -25,3 +25,5 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o >>> obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o >>> >>> obj-$(CONFIG_HI6220_MBOX) += hi6220-mailbox.o >>> + >>> +obj-${CONFIG_TEGRA_HSP_MBOX} += tegra-hsp.o >>> diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c >>> new file mode 100644 >>> index ..93c3ef58f29f >>> --- /dev/null >>> +++ b/drivers/mailbox/tegra-hsp.c >>> @@ -0,0 +1,418 @@ >>> +/* >>> + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. >>> + * >>> + * This program is free software; you can redistribute it and/or modify >>> it >>> + * under the terms and conditions of the GNU General Public License, >>> + * version 2, as published by the Free Software Foundation. >>> + * >>> + * This program is distributed in the hope it will be useful, but >>> WITHOUT >>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or >>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License >>> for >>> + * more details. >>> + */ >>> + >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> + >>> +#define HSP_INT_DIMENSIONING 0x380 >>> +#define HSP_nSM_OFFSET 0 >>> +#define HSP_nSS_OFFSET 4 >>> +#define HSP_nAS_OFFSET 8 >>> +#define HSP_nDB_OFFSET 12 >>> +#define HSP_nSI_OFFSET 16 >> >> >> Would be nice to have comments to understand what SM, SS, AS, etc. >> stand for (Shared Mailboxes, Shared Semaphores, Arbitrated Semaphores >> but you need to look at the patch description to understand that). A >> top-of-file comment explaning the necessary concepts to read this code >> would do the trick. > > Yes, will fix that. >> >> >>> +#define HSP_nINT_MASK 0xf >>> + >>> +#define HSP_DB_REG_TRIGGER 0x0 >>> +#define HSP_DB_REG_ENABLE 0x4 >>> +#define HSP_DB_REG_RAW 0x8 >>> +#define HSP_DB_REG_PENDING 0xc >>> + >>> +#define HSP_DB_CCPLEX 1 >>> +#define HSP_DB_BPMP3 >> >> >> Maybe turn this into enum and use that type for >> tegra_hsp_db_chan::db_id? Also have MAX_NUM_HSP_DB here, since it is >> related to these values? > > Okay. > >> >>> + >>> +#define MAX_NUM_HSP_CHAN 32 >>> +#define MAX_NUM_HSP_DB 7 >>> + >>> +#define hsp_db_offset(i, d) \ >>> + (d->base + ((1 + (d->nr_sm >> 1) + d->nr_ss + d->nr_as) << 16) +
Re: [PATCH V2 02/10] mailbox: tegra-hsp: Add HSP(Hardware Synchronization Primitives) driver
On 07/06/2016 03:05 PM, Alexandre Courbot wrote: On Tue, Jul 5, 2016 at 6:04 PM, Joseph Lowrote: The Tegra HSP mailbox driver implements the signaling doorbell-based interprocessor communication (IPC) for remote processors currently. The HSP HW modules support some different features for that, which are shared mailboxes, shared semaphores, arbitrated semaphores, and doorbells. And there are multiple HSP HW instances on the chip. So the driver is extendable to support more features for different IPC requirement. The driver of remote processor can use it as a mailbox client and deal with the IPC protocol to synchronize the data communications. Signed-off-by: Joseph Lo --- Changes in V2: - Update the driver to support the binding changes in V2 - it's extendable to support multiple HSP sub-modules on the same HSP HW block now. --- drivers/mailbox/Kconfig | 9 + drivers/mailbox/Makefile| 2 + drivers/mailbox/tegra-hsp.c | 418 3 files changed, 429 insertions(+) create mode 100644 drivers/mailbox/tegra-hsp.c diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig index 5305923752d2..fe584cb54720 100644 --- a/drivers/mailbox/Kconfig +++ b/drivers/mailbox/Kconfig @@ -114,6 +114,15 @@ config MAILBOX_TEST Test client to help with testing new Controller driver implementations. +config TEGRA_HSP_MBOX + bool "Tegra HSP(Hardware Synchronization Primitives) Driver" Space missing before the opening parenthesis (same in the patch title btw). Okay. + depends on ARCH_TEGRA_186_SOC + help + The Tegra HSP driver is used for the interprocessor communication + between different remote processors and host processors on Tegra186 + and later SoCs. Say Y here if you want to have this support. + If unsure say N. Since this option is selected automatically by ARCH_TEGRA_186_SOC, you should probably drop the last 2 sentences. Okay. + config XGENE_SLIMPRO_MBOX tristate "APM SoC X-Gene SLIMpro Mailbox Controller" depends on ARCH_XGENE diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile index 0be3e742bb7d..26d8f91c7fea 100644 --- a/drivers/mailbox/Makefile +++ b/drivers/mailbox/Makefile @@ -25,3 +25,5 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o obj-$(CONFIG_HI6220_MBOX) += hi6220-mailbox.o + +obj-${CONFIG_TEGRA_HSP_MBOX} += tegra-hsp.o diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c new file mode 100644 index ..93c3ef58f29f --- /dev/null +++ b/drivers/mailbox/tegra-hsp.c @@ -0,0 +1,418 @@ +/* + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + */ + +#include +#include +#include +#include +#include +#include +#include + +#define HSP_INT_DIMENSIONING 0x380 +#define HSP_nSM_OFFSET 0 +#define HSP_nSS_OFFSET 4 +#define HSP_nAS_OFFSET 8 +#define HSP_nDB_OFFSET 12 +#define HSP_nSI_OFFSET 16 Would be nice to have comments to understand what SM, SS, AS, etc. stand for (Shared Mailboxes, Shared Semaphores, Arbitrated Semaphores but you need to look at the patch description to understand that). A top-of-file comment explaning the necessary concepts to read this code would do the trick. Yes, will fix that. +#define HSP_nINT_MASK 0xf + +#define HSP_DB_REG_TRIGGER 0x0 +#define HSP_DB_REG_ENABLE 0x4 +#define HSP_DB_REG_RAW 0x8 +#define HSP_DB_REG_PENDING 0xc + +#define HSP_DB_CCPLEX 1 +#define HSP_DB_BPMP3 Maybe turn this into enum and use that type for tegra_hsp_db_chan::db_id? Also have MAX_NUM_HSP_DB here, since it is related to these values? Okay. + +#define MAX_NUM_HSP_CHAN 32 +#define MAX_NUM_HSP_DB 7 + +#define hsp_db_offset(i, d) \ + (d->base + ((1 + (d->nr_sm >> 1) + d->nr_ss + d->nr_as) << 16) + \ + (i) * 0x100) + +struct tegra_hsp_db_chan { + int master_id; + int db_id; +}; + +struct tegra_hsp_mbox_chan { + int type; + union { + struct tegra_hsp_db_chan db_chan; + }; +}; + +struct tegra_hsp_mbox { + struct mbox_controller *mbox; + void __iomem *base; + void __iomem *db_base[MAX_NUM_HSP_DB]; + int db_irq; + int nr_sm; + int nr_as; + int nr_ss; + int nr_db; + int nr_si; + spinlock_t lock; +}; + +static
Re: [PATCH V2 02/10] mailbox: tegra-hsp: Add HSP(Hardware Synchronization Primitives) driver
On 07/06/2016 03:05 PM, Alexandre Courbot wrote: On Tue, Jul 5, 2016 at 6:04 PM, Joseph Lo wrote: The Tegra HSP mailbox driver implements the signaling doorbell-based interprocessor communication (IPC) for remote processors currently. The HSP HW modules support some different features for that, which are shared mailboxes, shared semaphores, arbitrated semaphores, and doorbells. And there are multiple HSP HW instances on the chip. So the driver is extendable to support more features for different IPC requirement. The driver of remote processor can use it as a mailbox client and deal with the IPC protocol to synchronize the data communications. Signed-off-by: Joseph Lo --- Changes in V2: - Update the driver to support the binding changes in V2 - it's extendable to support multiple HSP sub-modules on the same HSP HW block now. --- drivers/mailbox/Kconfig | 9 + drivers/mailbox/Makefile| 2 + drivers/mailbox/tegra-hsp.c | 418 3 files changed, 429 insertions(+) create mode 100644 drivers/mailbox/tegra-hsp.c diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig index 5305923752d2..fe584cb54720 100644 --- a/drivers/mailbox/Kconfig +++ b/drivers/mailbox/Kconfig @@ -114,6 +114,15 @@ config MAILBOX_TEST Test client to help with testing new Controller driver implementations. +config TEGRA_HSP_MBOX + bool "Tegra HSP(Hardware Synchronization Primitives) Driver" Space missing before the opening parenthesis (same in the patch title btw). Okay. + depends on ARCH_TEGRA_186_SOC + help + The Tegra HSP driver is used for the interprocessor communication + between different remote processors and host processors on Tegra186 + and later SoCs. Say Y here if you want to have this support. + If unsure say N. Since this option is selected automatically by ARCH_TEGRA_186_SOC, you should probably drop the last 2 sentences. Okay. + config XGENE_SLIMPRO_MBOX tristate "APM SoC X-Gene SLIMpro Mailbox Controller" depends on ARCH_XGENE diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile index 0be3e742bb7d..26d8f91c7fea 100644 --- a/drivers/mailbox/Makefile +++ b/drivers/mailbox/Makefile @@ -25,3 +25,5 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o obj-$(CONFIG_HI6220_MBOX) += hi6220-mailbox.o + +obj-${CONFIG_TEGRA_HSP_MBOX} += tegra-hsp.o diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c new file mode 100644 index ..93c3ef58f29f --- /dev/null +++ b/drivers/mailbox/tegra-hsp.c @@ -0,0 +1,418 @@ +/* + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + */ + +#include +#include +#include +#include +#include +#include +#include + +#define HSP_INT_DIMENSIONING 0x380 +#define HSP_nSM_OFFSET 0 +#define HSP_nSS_OFFSET 4 +#define HSP_nAS_OFFSET 8 +#define HSP_nDB_OFFSET 12 +#define HSP_nSI_OFFSET 16 Would be nice to have comments to understand what SM, SS, AS, etc. stand for (Shared Mailboxes, Shared Semaphores, Arbitrated Semaphores but you need to look at the patch description to understand that). A top-of-file comment explaning the necessary concepts to read this code would do the trick. Yes, will fix that. +#define HSP_nINT_MASK 0xf + +#define HSP_DB_REG_TRIGGER 0x0 +#define HSP_DB_REG_ENABLE 0x4 +#define HSP_DB_REG_RAW 0x8 +#define HSP_DB_REG_PENDING 0xc + +#define HSP_DB_CCPLEX 1 +#define HSP_DB_BPMP3 Maybe turn this into enum and use that type for tegra_hsp_db_chan::db_id? Also have MAX_NUM_HSP_DB here, since it is related to these values? Okay. + +#define MAX_NUM_HSP_CHAN 32 +#define MAX_NUM_HSP_DB 7 + +#define hsp_db_offset(i, d) \ + (d->base + ((1 + (d->nr_sm >> 1) + d->nr_ss + d->nr_as) << 16) + \ + (i) * 0x100) + +struct tegra_hsp_db_chan { + int master_id; + int db_id; +}; + +struct tegra_hsp_mbox_chan { + int type; + union { + struct tegra_hsp_db_chan db_chan; + }; +}; + +struct tegra_hsp_mbox { + struct mbox_controller *mbox; + void __iomem *base; + void __iomem *db_base[MAX_NUM_HSP_DB]; + int db_irq; + int nr_sm; + int nr_as; + int nr_ss; + int nr_db; + int nr_si; + spinlock_t lock; +}; + +static inline u32 hsp_readl(void __iomem *base,
Re: [PATCH V2 02/10] mailbox: tegra-hsp: Add HSP(Hardware Synchronization Primitives) driver
On Tue, Jul 5, 2016 at 6:04 PM, Joseph Lowrote: > The Tegra HSP mailbox driver implements the signaling doorbell-based > interprocessor communication (IPC) for remote processors currently. The > HSP HW modules support some different features for that, which are > shared mailboxes, shared semaphores, arbitrated semaphores, and > doorbells. And there are multiple HSP HW instances on the chip. So the > driver is extendable to support more features for different IPC > requirement. > > The driver of remote processor can use it as a mailbox client and deal > with the IPC protocol to synchronize the data communications. > > Signed-off-by: Joseph Lo > --- > Changes in V2: > - Update the driver to support the binding changes in V2 > - it's extendable to support multiple HSP sub-modules on the same HSP HW block > now. > --- > drivers/mailbox/Kconfig | 9 + > drivers/mailbox/Makefile| 2 + > drivers/mailbox/tegra-hsp.c | 418 > > 3 files changed, 429 insertions(+) > create mode 100644 drivers/mailbox/tegra-hsp.c > > diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig > index 5305923752d2..fe584cb54720 100644 > --- a/drivers/mailbox/Kconfig > +++ b/drivers/mailbox/Kconfig > @@ -114,6 +114,15 @@ config MAILBOX_TEST > Test client to help with testing new Controller driver > implementations. > > +config TEGRA_HSP_MBOX > + bool "Tegra HSP(Hardware Synchronization Primitives) Driver" Space missing before the opening parenthesis (same in the patch title btw). > + depends on ARCH_TEGRA_186_SOC > + help > + The Tegra HSP driver is used for the interprocessor communication > + between different remote processors and host processors on Tegra186 > + and later SoCs. Say Y here if you want to have this support. > + If unsure say N. Since this option is selected automatically by ARCH_TEGRA_186_SOC, you should probably drop the last 2 sentences. > + > config XGENE_SLIMPRO_MBOX > tristate "APM SoC X-Gene SLIMpro Mailbox Controller" > depends on ARCH_XGENE > diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile > index 0be3e742bb7d..26d8f91c7fea 100644 > --- a/drivers/mailbox/Makefile > +++ b/drivers/mailbox/Makefile > @@ -25,3 +25,5 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o > obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o > > obj-$(CONFIG_HI6220_MBOX) += hi6220-mailbox.o > + > +obj-${CONFIG_TEGRA_HSP_MBOX} += tegra-hsp.o > diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c > new file mode 100644 > index ..93c3ef58f29f > --- /dev/null > +++ b/drivers/mailbox/tegra-hsp.c > @@ -0,0 +1,418 @@ > +/* > + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define HSP_INT_DIMENSIONING 0x380 > +#define HSP_nSM_OFFSET 0 > +#define HSP_nSS_OFFSET 4 > +#define HSP_nAS_OFFSET 8 > +#define HSP_nDB_OFFSET 12 > +#define HSP_nSI_OFFSET 16 Would be nice to have comments to understand what SM, SS, AS, etc. stand for (Shared Mailboxes, Shared Semaphores, Arbitrated Semaphores but you need to look at the patch description to understand that). A top-of-file comment explaning the necessary concepts to read this code would do the trick. > +#define HSP_nINT_MASK 0xf > + > +#define HSP_DB_REG_TRIGGER 0x0 > +#define HSP_DB_REG_ENABLE 0x4 > +#define HSP_DB_REG_RAW 0x8 > +#define HSP_DB_REG_PENDING 0xc > + > +#define HSP_DB_CCPLEX 1 > +#define HSP_DB_BPMP3 Maybe turn this into enum and use that type for tegra_hsp_db_chan::db_id? Also have MAX_NUM_HSP_DB here, since it is related to these values? > + > +#define MAX_NUM_HSP_CHAN 32 > +#define MAX_NUM_HSP_DB 7 > + > +#define hsp_db_offset(i, d) \ > + (d->base + ((1 + (d->nr_sm >> 1) + d->nr_ss + d->nr_as) << 16) + \ > + (i) * 0x100) > + > +struct tegra_hsp_db_chan { > + int master_id; > + int db_id; > +}; > + > +struct tegra_hsp_mbox_chan { > + int type; > + union { > + struct tegra_hsp_db_chan db_chan; > + }; > +}; > + > +struct tegra_hsp_mbox { > + struct mbox_controller *mbox; > + void __iomem *base; > + void __iomem *db_base[MAX_NUM_HSP_DB]; > + int db_irq; > + int
Re: [PATCH V2 02/10] mailbox: tegra-hsp: Add HSP(Hardware Synchronization Primitives) driver
On Tue, Jul 5, 2016 at 6:04 PM, Joseph Lo wrote: > The Tegra HSP mailbox driver implements the signaling doorbell-based > interprocessor communication (IPC) for remote processors currently. The > HSP HW modules support some different features for that, which are > shared mailboxes, shared semaphores, arbitrated semaphores, and > doorbells. And there are multiple HSP HW instances on the chip. So the > driver is extendable to support more features for different IPC > requirement. > > The driver of remote processor can use it as a mailbox client and deal > with the IPC protocol to synchronize the data communications. > > Signed-off-by: Joseph Lo > --- > Changes in V2: > - Update the driver to support the binding changes in V2 > - it's extendable to support multiple HSP sub-modules on the same HSP HW block > now. > --- > drivers/mailbox/Kconfig | 9 + > drivers/mailbox/Makefile| 2 + > drivers/mailbox/tegra-hsp.c | 418 > > 3 files changed, 429 insertions(+) > create mode 100644 drivers/mailbox/tegra-hsp.c > > diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig > index 5305923752d2..fe584cb54720 100644 > --- a/drivers/mailbox/Kconfig > +++ b/drivers/mailbox/Kconfig > @@ -114,6 +114,15 @@ config MAILBOX_TEST > Test client to help with testing new Controller driver > implementations. > > +config TEGRA_HSP_MBOX > + bool "Tegra HSP(Hardware Synchronization Primitives) Driver" Space missing before the opening parenthesis (same in the patch title btw). > + depends on ARCH_TEGRA_186_SOC > + help > + The Tegra HSP driver is used for the interprocessor communication > + between different remote processors and host processors on Tegra186 > + and later SoCs. Say Y here if you want to have this support. > + If unsure say N. Since this option is selected automatically by ARCH_TEGRA_186_SOC, you should probably drop the last 2 sentences. > + > config XGENE_SLIMPRO_MBOX > tristate "APM SoC X-Gene SLIMpro Mailbox Controller" > depends on ARCH_XGENE > diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile > index 0be3e742bb7d..26d8f91c7fea 100644 > --- a/drivers/mailbox/Makefile > +++ b/drivers/mailbox/Makefile > @@ -25,3 +25,5 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o > obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o > > obj-$(CONFIG_HI6220_MBOX) += hi6220-mailbox.o > + > +obj-${CONFIG_TEGRA_HSP_MBOX} += tegra-hsp.o > diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c > new file mode 100644 > index ..93c3ef58f29f > --- /dev/null > +++ b/drivers/mailbox/tegra-hsp.c > @@ -0,0 +1,418 @@ > +/* > + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define HSP_INT_DIMENSIONING 0x380 > +#define HSP_nSM_OFFSET 0 > +#define HSP_nSS_OFFSET 4 > +#define HSP_nAS_OFFSET 8 > +#define HSP_nDB_OFFSET 12 > +#define HSP_nSI_OFFSET 16 Would be nice to have comments to understand what SM, SS, AS, etc. stand for (Shared Mailboxes, Shared Semaphores, Arbitrated Semaphores but you need to look at the patch description to understand that). A top-of-file comment explaning the necessary concepts to read this code would do the trick. > +#define HSP_nINT_MASK 0xf > + > +#define HSP_DB_REG_TRIGGER 0x0 > +#define HSP_DB_REG_ENABLE 0x4 > +#define HSP_DB_REG_RAW 0x8 > +#define HSP_DB_REG_PENDING 0xc > + > +#define HSP_DB_CCPLEX 1 > +#define HSP_DB_BPMP3 Maybe turn this into enum and use that type for tegra_hsp_db_chan::db_id? Also have MAX_NUM_HSP_DB here, since it is related to these values? > + > +#define MAX_NUM_HSP_CHAN 32 > +#define MAX_NUM_HSP_DB 7 > + > +#define hsp_db_offset(i, d) \ > + (d->base + ((1 + (d->nr_sm >> 1) + d->nr_ss + d->nr_as) << 16) + \ > + (i) * 0x100) > + > +struct tegra_hsp_db_chan { > + int master_id; > + int db_id; > +}; > + > +struct tegra_hsp_mbox_chan { > + int type; > + union { > + struct tegra_hsp_db_chan db_chan; > + }; > +}; > + > +struct tegra_hsp_mbox { > + struct mbox_controller *mbox; > + void __iomem *base; > + void __iomem *db_base[MAX_NUM_HSP_DB]; > + int db_irq; > + int nr_sm; > + int nr_as; > + int
[PATCH V2 02/10] mailbox: tegra-hsp: Add HSP(Hardware Synchronization Primitives) driver
The Tegra HSP mailbox driver implements the signaling doorbell-based interprocessor communication (IPC) for remote processors currently. The HSP HW modules support some different features for that, which are shared mailboxes, shared semaphores, arbitrated semaphores, and doorbells. And there are multiple HSP HW instances on the chip. So the driver is extendable to support more features for different IPC requirement. The driver of remote processor can use it as a mailbox client and deal with the IPC protocol to synchronize the data communications. Signed-off-by: Joseph Lo--- Changes in V2: - Update the driver to support the binding changes in V2 - it's extendable to support multiple HSP sub-modules on the same HSP HW block now. --- drivers/mailbox/Kconfig | 9 + drivers/mailbox/Makefile| 2 + drivers/mailbox/tegra-hsp.c | 418 3 files changed, 429 insertions(+) create mode 100644 drivers/mailbox/tegra-hsp.c diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig index 5305923752d2..fe584cb54720 100644 --- a/drivers/mailbox/Kconfig +++ b/drivers/mailbox/Kconfig @@ -114,6 +114,15 @@ config MAILBOX_TEST Test client to help with testing new Controller driver implementations. +config TEGRA_HSP_MBOX + bool "Tegra HSP(Hardware Synchronization Primitives) Driver" + depends on ARCH_TEGRA_186_SOC + help + The Tegra HSP driver is used for the interprocessor communication + between different remote processors and host processors on Tegra186 + and later SoCs. Say Y here if you want to have this support. + If unsure say N. + config XGENE_SLIMPRO_MBOX tristate "APM SoC X-Gene SLIMpro Mailbox Controller" depends on ARCH_XGENE diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile index 0be3e742bb7d..26d8f91c7fea 100644 --- a/drivers/mailbox/Makefile +++ b/drivers/mailbox/Makefile @@ -25,3 +25,5 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o obj-$(CONFIG_HI6220_MBOX) += hi6220-mailbox.o + +obj-${CONFIG_TEGRA_HSP_MBOX} += tegra-hsp.o diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c new file mode 100644 index ..93c3ef58f29f --- /dev/null +++ b/drivers/mailbox/tegra-hsp.c @@ -0,0 +1,418 @@ +/* + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + */ + +#include +#include +#include +#include +#include +#include +#include + +#define HSP_INT_DIMENSIONING 0x380 +#define HSP_nSM_OFFSET 0 +#define HSP_nSS_OFFSET 4 +#define HSP_nAS_OFFSET 8 +#define HSP_nDB_OFFSET 12 +#define HSP_nSI_OFFSET 16 +#define HSP_nINT_MASK 0xf + +#define HSP_DB_REG_TRIGGER 0x0 +#define HSP_DB_REG_ENABLE 0x4 +#define HSP_DB_REG_RAW 0x8 +#define HSP_DB_REG_PENDING 0xc + +#define HSP_DB_CCPLEX 1 +#define HSP_DB_BPMP3 + +#define MAX_NUM_HSP_CHAN 32 +#define MAX_NUM_HSP_DB 7 + +#define hsp_db_offset(i, d) \ + (d->base + ((1 + (d->nr_sm >> 1) + d->nr_ss + d->nr_as) << 16) + \ + (i) * 0x100) + +struct tegra_hsp_db_chan { + int master_id; + int db_id; +}; + +struct tegra_hsp_mbox_chan { + int type; + union { + struct tegra_hsp_db_chan db_chan; + }; +}; + +struct tegra_hsp_mbox { + struct mbox_controller *mbox; + void __iomem *base; + void __iomem *db_base[MAX_NUM_HSP_DB]; + int db_irq; + int nr_sm; + int nr_as; + int nr_ss; + int nr_db; + int nr_si; + spinlock_t lock; +}; + +static inline u32 hsp_readl(void __iomem *base, int reg) +{ + return readl(base + reg); +} + +static inline void hsp_writel(void __iomem *base, int reg, u32 val) +{ + writel(val, base + reg); + readl(base + reg); +} + +static int hsp_db_can_ring(void __iomem *db_base) +{ + u32 reg; + + reg = hsp_readl(db_base, HSP_DB_REG_ENABLE); + + return !!(reg & BIT(HSP_DB_MASTER_CCPLEX)); +} + +static irqreturn_t hsp_db_irq(int irq, void *p) +{ + struct tegra_hsp_mbox *hsp_mbox = p; + ulong val; + int master_id; + + val = (ulong)hsp_readl(hsp_mbox->db_base[HSP_DB_CCPLEX], + HSP_DB_REG_PENDING); + hsp_writel(hsp_mbox->db_base[HSP_DB_CCPLEX], HSP_DB_REG_PENDING, val); + + spin_lock(_mbox->lock); + for_each_set_bit(master_id, ,
[PATCH V2 02/10] mailbox: tegra-hsp: Add HSP(Hardware Synchronization Primitives) driver
The Tegra HSP mailbox driver implements the signaling doorbell-based interprocessor communication (IPC) for remote processors currently. The HSP HW modules support some different features for that, which are shared mailboxes, shared semaphores, arbitrated semaphores, and doorbells. And there are multiple HSP HW instances on the chip. So the driver is extendable to support more features for different IPC requirement. The driver of remote processor can use it as a mailbox client and deal with the IPC protocol to synchronize the data communications. Signed-off-by: Joseph Lo --- Changes in V2: - Update the driver to support the binding changes in V2 - it's extendable to support multiple HSP sub-modules on the same HSP HW block now. --- drivers/mailbox/Kconfig | 9 + drivers/mailbox/Makefile| 2 + drivers/mailbox/tegra-hsp.c | 418 3 files changed, 429 insertions(+) create mode 100644 drivers/mailbox/tegra-hsp.c diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig index 5305923752d2..fe584cb54720 100644 --- a/drivers/mailbox/Kconfig +++ b/drivers/mailbox/Kconfig @@ -114,6 +114,15 @@ config MAILBOX_TEST Test client to help with testing new Controller driver implementations. +config TEGRA_HSP_MBOX + bool "Tegra HSP(Hardware Synchronization Primitives) Driver" + depends on ARCH_TEGRA_186_SOC + help + The Tegra HSP driver is used for the interprocessor communication + between different remote processors and host processors on Tegra186 + and later SoCs. Say Y here if you want to have this support. + If unsure say N. + config XGENE_SLIMPRO_MBOX tristate "APM SoC X-Gene SLIMpro Mailbox Controller" depends on ARCH_XGENE diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile index 0be3e742bb7d..26d8f91c7fea 100644 --- a/drivers/mailbox/Makefile +++ b/drivers/mailbox/Makefile @@ -25,3 +25,5 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o obj-$(CONFIG_HI6220_MBOX) += hi6220-mailbox.o + +obj-${CONFIG_TEGRA_HSP_MBOX} += tegra-hsp.o diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c new file mode 100644 index ..93c3ef58f29f --- /dev/null +++ b/drivers/mailbox/tegra-hsp.c @@ -0,0 +1,418 @@ +/* + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + */ + +#include +#include +#include +#include +#include +#include +#include + +#define HSP_INT_DIMENSIONING 0x380 +#define HSP_nSM_OFFSET 0 +#define HSP_nSS_OFFSET 4 +#define HSP_nAS_OFFSET 8 +#define HSP_nDB_OFFSET 12 +#define HSP_nSI_OFFSET 16 +#define HSP_nINT_MASK 0xf + +#define HSP_DB_REG_TRIGGER 0x0 +#define HSP_DB_REG_ENABLE 0x4 +#define HSP_DB_REG_RAW 0x8 +#define HSP_DB_REG_PENDING 0xc + +#define HSP_DB_CCPLEX 1 +#define HSP_DB_BPMP3 + +#define MAX_NUM_HSP_CHAN 32 +#define MAX_NUM_HSP_DB 7 + +#define hsp_db_offset(i, d) \ + (d->base + ((1 + (d->nr_sm >> 1) + d->nr_ss + d->nr_as) << 16) + \ + (i) * 0x100) + +struct tegra_hsp_db_chan { + int master_id; + int db_id; +}; + +struct tegra_hsp_mbox_chan { + int type; + union { + struct tegra_hsp_db_chan db_chan; + }; +}; + +struct tegra_hsp_mbox { + struct mbox_controller *mbox; + void __iomem *base; + void __iomem *db_base[MAX_NUM_HSP_DB]; + int db_irq; + int nr_sm; + int nr_as; + int nr_ss; + int nr_db; + int nr_si; + spinlock_t lock; +}; + +static inline u32 hsp_readl(void __iomem *base, int reg) +{ + return readl(base + reg); +} + +static inline void hsp_writel(void __iomem *base, int reg, u32 val) +{ + writel(val, base + reg); + readl(base + reg); +} + +static int hsp_db_can_ring(void __iomem *db_base) +{ + u32 reg; + + reg = hsp_readl(db_base, HSP_DB_REG_ENABLE); + + return !!(reg & BIT(HSP_DB_MASTER_CCPLEX)); +} + +static irqreturn_t hsp_db_irq(int irq, void *p) +{ + struct tegra_hsp_mbox *hsp_mbox = p; + ulong val; + int master_id; + + val = (ulong)hsp_readl(hsp_mbox->db_base[HSP_DB_CCPLEX], + HSP_DB_REG_PENDING); + hsp_writel(hsp_mbox->db_base[HSP_DB_CCPLEX], HSP_DB_REG_PENDING, val); + + spin_lock(_mbox->lock); + for_each_set_bit(master_id, , MAX_NUM_HSP_CHAN) { +