2022-11-06 10:40:43

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] dt-bindings: gce: add gce header file for mt8188

On 04/11/2022 08:44, Elvis.Wang wrote:
> From: Elvis Wang <[email protected]>
>
> add gce header file to define the gce thread priority, gce subsys id,
> event and constant for mt8188.
>
> Signed-off-by: Elvis Wang <[email protected]>
> ---
> Thanks for the reviews, I have removed the incorrect reviewed message.
> ---
> include/dt-bindings/gce/mediatek,mt8188-gce.h | 966 ++++++++++++++++++
> 1 file changed, 966 insertions(+)
> create mode 100644 include/dt-bindings/gce/mediatek,mt8188-gce.h
>
> diff --git a/include/dt-bindings/gce/mediatek,mt8188-gce.h b/include/dt-bindings/gce/mediatek,mt8188-gce.h
> new file mode 100644
> index 000000000000..e2e2c56016a1
> --- /dev/null
> +++ b/include/dt-bindings/gce/mediatek,mt8188-gce.h


What is a "gce" subsystem or type of hardware? Why do you put headers in
"gce"?

Don't continue this pattern. Mediatek devices do not get specific folders...

Best regards,
Krzysztof



2022-11-07 12:02:29

by Matthias Brugger

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] dt-bindings: gce: add gce header file for mt8188



On 06/11/2022 11:27, Krzysztof Kozlowski wrote:
> On 04/11/2022 08:44, Elvis.Wang wrote:
>> From: Elvis Wang <[email protected]>
>>
>> add gce header file to define the gce thread priority, gce subsys id,
>> event and constant for mt8188.
>>
>> Signed-off-by: Elvis Wang <[email protected]>
>> ---
>> Thanks for the reviews, I have removed the incorrect reviewed message.
>> ---
>> include/dt-bindings/gce/mediatek,mt8188-gce.h | 966 ++++++++++++++++++
>> 1 file changed, 966 insertions(+)
>> create mode 100644 include/dt-bindings/gce/mediatek,mt8188-gce.h
>>
>> diff --git a/include/dt-bindings/gce/mediatek,mt8188-gce.h b/include/dt-bindings/gce/mediatek,mt8188-gce.h
>> new file mode 100644
>> index 000000000000..e2e2c56016a1
>> --- /dev/null
>> +++ b/include/dt-bindings/gce/mediatek,mt8188-gce.h
>
>
> What is a "gce" subsystem or type of hardware? Why do you put headers in
> "gce"?
>

Yes, I think we should move all header files from the gce folder into the
mailbox folder.

Regards,
Matthias

> Don't continue this pattern. Mediatek devices do not get specific folders...
>
> Best regards,
> Krzysztof
>

2022-11-22 10:07:49

by Elvis Wang (王军)

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] dt-bindings: gce: add gce header file for mt8188

On Mon, 2022-11-07 at 12:18 +0100, Matthias Brugger wrote:
>
> On 06/11/2022 11:27, Krzysztof Kozlowski wrote:
> > On 04/11/2022 08:44, Elvis.Wang wrote:
> > > From: Elvis Wang <[email protected]>
> > >
> > > add gce header file to define the gce thread priority, gce subsys
> > > id,
> > > event and constant for mt8188.
> > >
> > > Signed-off-by: Elvis Wang <[email protected]>
> > > ---
> > > Thanks for the reviews, I have removed the incorrect reviewed
> > > message.
> > > ---
> > > include/dt-bindings/gce/mediatek,mt8188-gce.h | 966
> > > ++++++++++++++++++
> > > 1 file changed, 966 insertions(+)
> > > create mode 100644 include/dt-bindings/gce/mediatek,mt8188-
> > > gce.h
> > >
> > > diff --git a/include/dt-bindings/gce/mediatek,mt8188-gce.h
> > > b/include/dt-bindings/gce/mediatek,mt8188-gce.h
> > > new file mode 100644
> > > index 000000000000..e2e2c56016a1
> > > --- /dev/null
> > > +++ b/include/dt-bindings/gce/mediatek,mt8188-gce.h
> >
> >
> > What is a "gce" subsystem or type of hardware? Why do you put
> > headers in
> > "gce"?
> >
>
> Yes, I think we should move all header files from the gce folder into
> the
> mailbox folder.
>
> Regards,
> Matthias

There are still a lot of header files undre gce. If all of them are
moved, it will also involve the modification of yaml and corresponding
project dts files, which requires more effort. Can we deal with the
8188 part first?

Best regards,
Elvis

>
> > Don't continue this pattern. Mediatek devices do not get specific
> > folders...
> >
> > Best regards,
> > Krzysztof
> >

2022-11-22 10:42:09

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] dt-bindings: gce: add gce header file for mt8188

On 22/11/2022 10:10, Elvis Wang (王军) wrote:
> On Mon, 2022-11-07 at 12:18 +0100, Matthias Brugger wrote:
>>
>> On 06/11/2022 11:27, Krzysztof Kozlowski wrote:
>>> On 04/11/2022 08:44, Elvis.Wang wrote:
>>>> From: Elvis Wang <[email protected]>
>>>>
>>>> add gce header file to define the gce thread priority, gce subsys
>>>> id,
>>>> event and constant for mt8188.
>>>>
>>>> Signed-off-by: Elvis Wang <[email protected]>
>>>> ---
>>>> Thanks for the reviews, I have removed the incorrect reviewed
>>>> message.
>>>> ---
>>>> include/dt-bindings/gce/mediatek,mt8188-gce.h | 966
>>>> ++++++++++++++++++
>>>> 1 file changed, 966 insertions(+)
>>>> create mode 100644 include/dt-bindings/gce/mediatek,mt8188-
>>>> gce.h
>>>>
>>>> diff --git a/include/dt-bindings/gce/mediatek,mt8188-gce.h
>>>> b/include/dt-bindings/gce/mediatek,mt8188-gce.h
>>>> new file mode 100644
>>>> index 000000000000..e2e2c56016a1
>>>> --- /dev/null
>>>> +++ b/include/dt-bindings/gce/mediatek,mt8188-gce.h
>>>
>>>
>>> What is a "gce" subsystem or type of hardware? Why do you put
>>> headers in
>>> "gce"?
>>>
>>
>> Yes, I think we should move all header files from the gce folder into
>> the
>> mailbox folder.
>>
>> Regards,
>> Matthias
>
> There are still a lot of header files undre gce. If all of them are
> moved, it will also involve the modification of yaml and corresponding
> project dts files, which requires more effort. Can we deal with the
> 8188 part first?

Sure, just put the new bindings in proper location (mailbox if this is a
Mailbox).

Best regards,
Krzysztof