Re: [PATCH 02/15] dt-bindings: gce: mt8195: Add CMDQ_SYNC_TOKEN_SECURE_THR_EOF event id

From: Jason-JH Lin (林睿祥)
Date: Mon Sep 25 2023 - 22:45:39 EST


On Mon, 2023-09-25 at 11:28 +0200, Krzysztof Kozlowski wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> On 25/09/2023 11:11, Jason-JH Lin (林睿祥) wrote:
> > On Mon, 2023-09-25 at 08:42 +0200, Krzysztof Kozlowski wrote:
> >>
> >> External email : Please do not click links or open attachments
> until
> >> you have verified the sender or the content.
> >> On 25/09/2023 07:05, Jason-JH Lin (林睿祥) wrote:
> >>> Hi Krzysztof,
> >>>
> >>> Thanks for the reviews.
> >>>
> >>> On Sat, 2023-09-23 at 20:01 +0200, Krzysztof Kozlowski wrote:
> >>>>
> >>>> External email : Please do not click links or open attachments
> >> until
> >>>> you have verified the sender or the content.
> >>>> On 18/09/2023 21:21, Jason-JH.Lin wrote:
> >>>>> CMDQ_SYNC_TOKEN_SECURE_THR_EOF is used as secure irq to notify
> >> CMDQ
> >>>>> driver in the normal world that GCE secure thread has completed
> a
> >>>> task
> >>>>> in thee secure world.
> >>>>
> >>>> How can #define be added after its usage? Does it even make any
> >> sense
> >>>> of
> >>>> being separate patch?
> >>>>
> >>>
> >>> This definition is used in the mt8195.dts at [PATCH 15/15] and
> the
> >> CMDQ
> >>
> >> No, the define is used in previous patch, which means your
> patchset
> >> is
> >> not bisectable and not tested.
> >>
> >
> > Do you mean this patch should add before patch 1?
> >
> >
> > The example of dts in patch 1 is used the definition of mt8188, so
> I
> > think I can add this patch to define the gce event id for mt8195
> after
> > patch 1.
> >
> > I will swap the patch 1 and the patch 2 in the next version, if
> that
> > can make it more appropriate.
>
> Really, test your patches. Each of them individually. Appropriateness
> is
> one thing, but broken bisectability is an error.
>
> Anyway, the patch is logically part of binding adding this, so it
> should
> be squashed. Don't create some weird patch ordering where every
> define
> or every function is in its own patch. I commented about it multiple
> times in this patchset.
>

OK, I'll re-order the definition before the patch using it or squash
the definition to the patch using it.

Regards,
Jason-JH.Lin

> Best regards,
> Krzysztof
>