| Message ID | 20220609083101.24195-3-tinghan.shen@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add MT8186 ADSP dt-binding | expand |
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
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;
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(-)