2023-10-23 09:42:27

by Fei Shao

[permalink] [raw]
Subject: Re: [PATCH v2 6/9] mailbox: mediatek: Add CMDQ driver support for mt8188

Hi Jason,

On Mon, Oct 23, 2023 at 12:58 PM Jason-JH.Lin <[email protected]> wrote:
>
> Add CMDQ driver support for mt8188 by adding its compatible and
> driver data in CMDQ driver.
>
> Signed-off-by: Jason-JH.Lin <[email protected]>
> ---
> drivers/mailbox/mtk-cmdq-mailbox.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> index 56fe01cd9731..3bdfb9a60614 100644
> --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> @@ -704,12 +704,20 @@ static const struct gce_plat gce_plat_v7 = {
> .gce_num = 1
> };
>
> +static const struct gce_plat gce_plat_v8 = {
> + .thread_nr = 32,
> + .shift = 3,
> + .control_by_sw = true,
> + .gce_num = 2
> +};
> +
> static const struct of_device_id cmdq_of_ids[] = {
> {.compatible = "mediatek,mt8173-gce", .data = (void *)&gce_plat_v2},
> {.compatible = "mediatek,mt8183-gce", .data = (void *)&gce_plat_v3},
> {.compatible = "mediatek,mt8186-gce", .data = (void *)&gce_plat_v7},
> {.compatible = "mediatek,mt6779-gce", .data = (void *)&gce_plat_v4},
> {.compatible = "mediatek,mt8192-gce", .data = (void *)&gce_plat_v5},
> + {.compatible = "mediatek,mt8188-gce", .data = (void *)&gce_plat_v8},
I guess I understand your intention here... but the ordering doesn't
make sense to most people. Put this line after mt8186.

It's up to you to ignore the outlier mt6779 for now or send a separate
patch to fully sort the list by the compatible strings.

Regards,
Fei


> {.compatible = "mediatek,mt8195-gce", .data = (void *)&gce_plat_v6},
> {}
> };
> --
> 2.18.0
>
>


2023-10-25 06:33:53

by Jason-JH.Lin

[permalink] [raw]
Subject: Re: [PATCH v2 6/9] mailbox: mediatek: Add CMDQ driver support for mt8188

Hi Fei,

Thanks for the reviews.

On Mon, 2023-10-23 at 17:41 +0800, Fei Shao wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> Hi Jason,
>
> On Mon, Oct 23, 2023 at 12:58 PM Jason-JH.Lin <
> [email protected]> wrote:
> >
> > Add CMDQ driver support for mt8188 by adding its compatible and
> > driver data in CMDQ driver.
> >
> > Signed-off-by: Jason-JH.Lin <[email protected]>
> > ---
> > drivers/mailbox/mtk-cmdq-mailbox.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c
> b/drivers/mailbox/mtk-cmdq-mailbox.c
> > index 56fe01cd9731..3bdfb9a60614 100644
> > --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> > @@ -704,12 +704,20 @@ static const struct gce_plat gce_plat_v7 = {
> > .gce_num = 1
> > };
> >
> > +static const struct gce_plat gce_plat_v8 = {
> > + .thread_nr = 32,
> > + .shift = 3,
> > + .control_by_sw = true,
> > + .gce_num = 2
> > +};
> > +
> > static const struct of_device_id cmdq_of_ids[] = {
> > {.compatible = "mediatek,mt8173-gce", .data = (void
> *)&gce_plat_v2},
> > {.compatible = "mediatek,mt8183-gce", .data = (void
> *)&gce_plat_v3},
> > {.compatible = "mediatek,mt8186-gce", .data = (void
> *)&gce_plat_v7},
> > {.compatible = "mediatek,mt6779-gce", .data = (void
> *)&gce_plat_v4},
> > {.compatible = "mediatek,mt8192-gce", .data = (void
> *)&gce_plat_v5},
> > + {.compatible = "mediatek,mt8188-gce", .data = (void
> *)&gce_plat_v8},
> I guess I understand your intention here... but the ordering doesn't
> make sense to most people. Put this line after mt8186.
>
> It's up to you to ignore the outlier mt6779 for now or send a
> separate
> patch to fully sort the list by the compatible strings.
>
> Regards,
> Fei
>

I'll move this patch out of the secure cmdq series and fix the sorting
problem. Thanks!

Regards,
Jason-JH.Lin

>
> > {.compatible = "mediatek,mt8195-gce", .data = (void
> *)&gce_plat_v6},
> > {}
> > };
> > --
> > 2.18.0
> >
> >