Re: [PATCH 1/3] dt-bindings: mediatek: mt8188: Add binding for MM & INFRA IOMMU

Re: [PATCH 1/3] dt-bindings: mediatek: mt8188: Add binding for MM & INFRA IOMMU

[Krzysztof Kozlowski]

On 04/08/2022 12:58, Chengci.Xu wrote:
> This patch adds descriptions for mt8188 IOMMU which also use ARM
> Short-Descriptor translation table format.
> 


Thank you for your patch. There is something to discuss/improve.

>      then:
>        required:
> diff --git a/include/dt-bindings/memory/mt8188-memory-port.h
>  b/include/dt-bindings/memory/mt8188-memory-port.h
> new file mode 100644
> index 000000000000..612fd366c3a7
> --- /dev/null
> +++ b/include/dt-bindings/memory/mt8188-memory-port.h

Use vendor prefix in filename, so mediatek,mt8188-memory-port.h

> @@ -0,0 +1,482 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */


This should be dual-license, if possible.


> +/*
> + * Copyright (c) 2022 MediaTek Inc.
> + * Author: Chengci Xu <chengci.xu@mediatek.com>
> + */
> +#ifndef _DT_BINDINGS_MEMORY_MT8188_LARB_PORT_H_
> +#define _DT_BINDINGS_MEMORY_MT8188_LARB_PORT_H_
> +
> +#include <dt-bindings/memory/mtk-memory-port.h>
> +
> +/*
> + * MM IOMMU supports 16GB dma address. We separate it to four ranges:
> + * 0 ~ 4G; 4G ~ 8G; 8G ~ 12G; 12G ~ 16G, we could adjust these masters
> + * locate in anyone region. BUT:
> + * a) Make sure all the ports inside a larb are in one range.
> + * b) The iova of any master can NOT cross the 4G/8G/12G boundary.
> + *
> + * This is the suggested mapping in this SoC:
> + *
> + * modules    dma-address-region	larbs-ports
> + * disp         0 ~ 4G                  larb0/1/2/3
> + * vcodec      4G ~ 8G                  larb19/21/23
> + * cam/mdp     8G ~ 12G                 the other larbs.
> + * N/A         12G ~ 16G
> + * CCU0   0x24000_0000 ~ 0x243ff_ffff   larb27: port 0/1
> + * CCU1   0x24400_0000 ~ 0x247ff_ffff   larb27: port 2/3
> + *
> + * This SoC have two MM IOMMU HWs, this is the connected information:
> + * iommu-vdo: larb0/2/5/9/10/11A/11C/13/16B/17B/19/21
> + * iommu-vpp: larb1/3/4/6/7/11B/12/14/15/16A/17A/23/27
> + */
> +
> +/* MM IOMMU larbs */
> +#define SMI_L0_ID		(0)

No need for ().

> +#define SMI_L1_ID		(1)
> +#define SMI_L2_ID		(2)
> +#define SMI_L3_ID		(3)\


Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: mediatek: mt8188: Add binding for MM & INFRA IOMMU

[Krzysztof Kozlowski]

On 18/08/2022 10:26, Chengci.Xu wrote:
>>>      then:
>>>        required:
>>> diff --git a/include/dt-bindings/memory/mt8188-memory-port.h
>>>  b/include/dt-bindings/memory/mt8188-memory-port.h
>>> new file mode 100644
>>> index 000000000000..612fd366c3a7
>>> --- /dev/null
>>> +++ b/include/dt-bindings/memory/mt8188-memory-port.h
>>
>> Use vendor prefix in filename, so mediatek,mt8188-memory-port.h
> 
> Is this a new mandatory rules to add a vendor prefix in dt-bindings
> file name? we have never add a vendor prefix before.
> 
> If possible, we'd like not to use vendor prefix for the consistency of
> fliename's format(mtxxxx-memory-port.h).
> Such as "mt8195-memory-port.h" and "mt8186-memory-port.h".

That's a generic rule, although loosely applied. The consistency is
rather to have a vendor prefix everywhere. Why Mediatek should have an
exception?

Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: mediatek: mt8188: Add binding for MM & INFRA IOMMU

[Krzysztof Kozlowski]

On 18/08/2022 10:26, Chengci.Xu wrote:
> On Thu, 2022-08-04 at 13:13 +0200, Krzysztof Kozlowski wrote:
>> On 04/08/2022 12:58, Chengci.Xu wrote:
>>> This patch adds descriptions for mt8188 IOMMU which also use ARM
>>> Short-Descriptor translation table format.
>>>
>>
>>
>> Thank you for your patch. There is something to discuss/improve.
> 
> Tkanks for your review.
> 

BTW, none of your emails seem to reach devicetree mailing list:
https://lore.kernel.org/linux-devicetree/20220804105834.626-1-chengci.xu@mediatek.com/
which means they won't pop-up in Rob's Patchwork and won't tested by bot.
https://patchwork.ozlabs.org/project/devicetree-bindings/list/?series=&submitter=84571&state=*&q=&archive=&delegate=

I wonder if this is the same issue as with other Mediatek emails
filtered by spam...

Anyway, you should sort it out, otherwise patches won't get to Rob and I
won't apply them.

Best regards,
Krzysztof