Rename mbox according to action instead of 'mbox0' and 'mbox1'
Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
drivers/firmware/mtk-adsp-ipc.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/mtk-adsp-ipc.c b/drivers/firmware/mtk-adsp-ipc.c
index cb255a99170c..3de94765d659 100644
--- a/drivers/firmware/mtk-adsp-ipc.c
+++ b/drivers/firmware/mtk-adsp-ipc.c
@@ -83,7 +83,11 @@ static int mtk_adsp_ipc_probe(struct platform_device *pdev)
return -ENOMEM;
for (i = 0; i < MTK_ADSP_MBOX_NUM; i++) {
- chan_name = kasprintf(GFP_KERNEL, "mbox%d", i);
+ if (i < MTK_ADSP_MBOX_NUM / 2)
+ chan_name = kasprintf(GFP_KERNEL, "rep");
+ else
+ chan_name = kasprintf(GFP_KERNEL, "req");
+
if (!chan_name) {
ret = -ENOMEM;
goto out;
--
2.18.0
Il 09/06/22 10:31, Tinghan Shen ha scritto: > Rename mbox according to action instead of 'mbox0' and 'mbox1' > > Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com> > --- > drivers/firmware/mtk-adsp-ipc.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/firmware/mtk-adsp-ipc.c b/drivers/firmware/mtk-adsp-ipc.c > index cb255a99170c..3de94765d659 100644 > --- a/drivers/firmware/mtk-adsp-ipc.c > +++ b/drivers/firmware/mtk-adsp-ipc.c > @@ -83,7 +83,11 @@ static int mtk_adsp_ipc_probe(struct platform_device *pdev) > return -ENOMEM; > > for (i = 0; i < MTK_ADSP_MBOX_NUM; i++) { > - chan_name = kasprintf(GFP_KERNEL, "mbox%d", i); > + if (i < MTK_ADSP_MBOX_NUM / 2) > + chan_name = kasprintf(GFP_KERNEL, "rep"); > + else > + chan_name = kasprintf(GFP_KERNEL, "req"); > + > if (!chan_name) { > ret = -ENOMEM; > goto out; At this point, just call them "reply" and "request", as that simply provides a perfectly clear explanation. Besides, I'm sorry but I really don't like this code, it's really too much fragile and will have to be changed entirely if a third mbox is introduced. I can suggest a cooler way: static const char * const adsp_mbox_ch_names[MTK_ADSP_MBOX_NUM] = { "rep", "req" }; for (i = 0; i < ARRAY_SIZE(adsp_mbox_ch_names); i++) { /* we can delete chan_name and also avoid a kfree if we do... */ .... code .... adsp_chan->ch = mbox_request_channel_byname(cl, adsp_mbox_ch_names[i]); ... etc etc ... } Cheers, Angelo
Hi Angelo, On Mon, 2022-06-13 at 14:37 +0200, AngeloGioacchino Del Regno wrote: > Il 09/06/22 10:31, Tinghan Shen ha scritto: > > Rename mbox according to action instead of 'mbox0' and 'mbox1' > > > > Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com> > > --- > > drivers/firmware/mtk-adsp-ipc.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/firmware/mtk-adsp-ipc.c b/drivers/firmware/mtk-adsp-ipc.c > > index cb255a99170c..3de94765d659 100644 > > --- a/drivers/firmware/mtk-adsp-ipc.c > > +++ b/drivers/firmware/mtk-adsp-ipc.c > > @@ -83,7 +83,11 @@ static int mtk_adsp_ipc_probe(struct platform_device *pdev) > > return -ENOMEM; > > > > for (i = 0; i < MTK_ADSP_MBOX_NUM; i++) { > > - chan_name = kasprintf(GFP_KERNEL, "mbox%d", i); > > + if (i < MTK_ADSP_MBOX_NUM / 2) > > + chan_name = kasprintf(GFP_KERNEL, "rep"); > > + else > > + chan_name = kasprintf(GFP_KERNEL, "req"); > > + > > if (!chan_name) { > > ret = -ENOMEM; > > goto out; > > At this point, just call them "reply" and "request", as that simply provides a > perfectly clear explanation. > > Besides, I'm sorry but I really don't like this code, it's really too much > fragile and will have to be changed entirely if a third mbox is introduced. > > I can suggest a cooler way: > > static const char * const adsp_mbox_ch_names[MTK_ADSP_MBOX_NUM] = { "rep", "req" }; > > for (i = 0; i < ARRAY_SIZE(adsp_mbox_ch_names); i++) { > /* we can delete chan_name and also avoid a kfree if we do... */ > > .... code .... > > adsp_chan->ch = mbox_request_channel_byname(cl, adsp_mbox_ch_names[i]); > > ... etc etc ... > } > > Cheers, > Angelo Ok, I'll update in the next version. Thank you! Best regards, TingHan
© 2016 - 2024 Red Hat, Inc.