Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller
On 06/07/2022 14:00, Tinghan Shen wrote:
> Hi Krzysztof,
>
> After discussing your message with our power team,
> we realized that we need your help to ensure we fully understand you.
>
> On Mon, 2022-07-04 at 14:38 +0200, Krzysztof Kozlowski wrote:
>> On 04/07/2022 12:00, Tinghan Shen wrote:
>>> Add power domains controller node for mt8195.
>>>
>>> Signed-off-by: Weiyi Lu
>>> Signed-off-by: Tinghan Shen
>>> ---
>>> arch/arm64/boot/dts/mediatek/mt8195.dtsi | 327 +++
>>> 1 file changed, 327 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> index 8d59a7da3271..d52e140d9271 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> @@ -10,6 +10,7 @@
>>> #include
>>> #include
>>> #include
>>> +#include
>>>
>>> / {
>>> compatible = "mediatek,mt8195";
>>> @@ -338,6 +339,332 @@
>>> #interrupt-cells = <2>;
>>> };
>>>
>>> + scpsys: syscon@10006000 {
>>> + compatible = "syscon", "simple-mfd";
>>
>> These compatibles cannot be alone.
>
> the scpsys sub node has the compatible of the power domain driver.
> do you suggest that the compatible in the sub node should move to here?
Not necessarily, depends. You have here device node representing system
registers. They need they own compatibles, just like everywhere in the
kernel (except the broken cases...).
Whether this should be compatible of power-domain driver, it depends
what this device node is. I don't know, I don't have your datasheets or
your architecture diagrams...
>
>>> + reg = <0 0x10006000 0 0x1000>;
>>> + #power-domain-cells = <1>;
>>
>> If it is simple MFD, then probably it is not a power domain provider.
>> Decide.
>
> this MFD device is the power controller on mt8195.
Then it is not a simple MFD but a power controller. Do not use
"simple-mfd" compatible.
> Some features need
> to do some operations on registers in this node. We think that implement
> the operation of these registers as the MFD device can provide flexibility
> for future use. We want to clarify if you're saying that an MFD device
> cannot be a power domain provider.
MFD device is Linuxism, so it has nothing to do here. I am talking only
about simple-mfd. simple-mfd is a simple device only instantiating
children and not providing anything to anyone. Neither to children. This
the most important part. The children do not depend on anything from
simple-mfd device. For example simple-mfd device can be shut down
(gated) and children should still operate. Being a power domain
controller, contradicts this usually.
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 02/16] dt-bindings: memory: mediatek: Update condition for mt8195 smi node
On 06/07/2022 15:48, Matthias Brugger wrote: > > > On 04/07/2022 14:36, Krzysztof Kozlowski wrote: >> On 04/07/2022 12:00, Tinghan Shen wrote: >>> The max clock items for the dts node with compatible >>> 'mediatek,mt8195-smi-sub-common' should be 3. >>> >>> However, the dtbs_check of such node will get following message, >>> arch/arm64/boot/dts/mediatek/mt8195-evb.dtb: smi@1401: clock-names: >>> ['apb', 'smi', 'gals0'] is too long >>> From schema: >>> Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml >>> >>> Remove the last 'else' checking to fix this error. >> >> Missing fixes tag. >> > > From my understanding, fixes tags are for patches that fix bugs (hw is not > working etc) and not a warning message from dtbs_check. So my point of view > would be to not add a fixes tag here. Not conforming to bindings is also a bug. Missing properties or wrong properties, even if hardware is working, is still a bug. If such bug is not visible now in Linux, might be visible later in the future or visible in different OS (DTS are used by other systems and pieces of software like bootloaders). Limiting this only to Linux and to current version (hardware still works) is OK for Linux drivers, but not for DTS. Therefore Fixes tag in general is applicable. Of course maybe to this one not really, maybe this is too trivial, or whatever, so I do not insist. But I insist on the principle - reasonable dtbs_check warnings are like compiler warnings - bugs which have to be fixed. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller
On 06/07/2022 15:41, Matthias Brugger wrote:
>
>
> On 04/07/2022 14:38, Krzysztof Kozlowski wrote:
>> On 04/07/2022 12:00, Tinghan Shen wrote:
>>> Add power domains controller node for mt8195.
>>>
>>> Signed-off-by: Weiyi Lu
>>> Signed-off-by: Tinghan Shen
>>> ---
>>> arch/arm64/boot/dts/mediatek/mt8195.dtsi | 327 +++
>>> 1 file changed, 327 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> index 8d59a7da3271..d52e140d9271 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
>>> @@ -10,6 +10,7 @@
>>> #include
>>> #include
>>> #include
>>> +#include
>>>
>>> / {
>>> compatible = "mediatek,mt8195";
>>> @@ -338,6 +339,332 @@
>>> #interrupt-cells = <2>;
>>> };
>>>
>>> + scpsys: syscon@10006000 {
>>> + compatible = "syscon", "simple-mfd";
>>
>> These compatibles cannot be alone.
>>
>
> You mean we would need something like "mediatek,scpsys" as dummy compatible
> that's not bound to any driver?
Yes. syscon (and simple-mfd) must always come with a specific compatible.
>
>>> + reg = <0 0x10006000 0 0x1000>;
>>> + #power-domain-cells = <1>;
>>
>> If it is simple MFD, then probably it is not a power domain provider.
>> Decide.
>
> The SCPSYS IP block of MediaTek SoCs group several functionality, one is the
> power domain controller. Others are not yet implemented, but defining the
> scpsys
> as a MFD will give us the possibility to do so in the future.
No, quite the opposite. Having simple-mfd prevents you from implementing
it correctly later as a driver, because you cannot remove it. It would
be ABI break.
It's fine to have one block being a simple MFD having several children,
but then it's not a power controller. Children could be such power
controller, but not simple-mfd. Rob explained this several times:
https://lore.kernel.org/all/yxhine00hg6hb...@robh.at.kernel.org/
https://lore.kernel.org/all/20220701000959.ga3588170-r...@kernel.org/
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 16/16] arm64: dts: mt8195: Add display node for vdosys0
On 04/07/2022 12:00, Tinghan Shen wrote:
> From: "Jason-JH.Lin"
>
> Add display node for vdosys0 of mt8195.
>
> Signed-off-by: Jason-JH.Lin
> Signed-off-by: Tinghan Shen
> ---
> arch/arm64/boot/dts/mediatek/mt8195.dtsi | 109 +++
> 1 file changed, 109 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> index 724c6ca837b6..faea8ef33e5a 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> @@ -1961,6 +1961,7 @@
> vdosys0: syscon@1c01a000 {
> compatible = "mediatek,mt8195-mmsys", "syscon";
> reg = <0 0x1c01a000 0 0x1000>;
> + mboxes = <&gce0 0 CMDQ_THR_PRIO_4>;
> #clock-cells = <1>;
> };
>
> @@ -1976,6 +1977,114 @@
> power-domains = <&spm MT8195_POWER_DOMAIN_VENC_CORE1>;
> };
>
> + ovl0: ovl@1c00 {
> + compatible = "mediatek,mt8195-disp-ovl",
> + "mediatek,mt8183-disp-ovl";
> + reg = <0 0x1c00 0 0x1000>;
> + interrupts = ;
> + power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
> + clocks = <&vdosys0 CLK_VDO0_DISP_OVL0>;
> + iommus = <&iommu_vdo M4U_PORT_L0_DISP_OVL0_RDMA0>;
> + mediatek,gce-client-reg =
> + <&gce0 SUBSYS_1c00 0x 0x1000>;
> + };
> +
> + rdma0: rdma@1c002000 {
> + compatible = "mediatek,mt8195-disp-rdma";
> + reg = <0 0x1c002000 0 0x1000>;
> + interrupts = ;
> + power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
> + clocks = <&vdosys0 CLK_VDO0_DISP_RDMA0>;
> + iommus = <&iommu_vdo M4U_PORT_L0_DISP_RDMA0>;
> + mediatek,gce-client-reg =
> + <&gce0 SUBSYS_1c00 0x2000 0x1000>;
> + };
> +
> + color0: color@1c003000 {
> + compatible = "mediatek,mt8195-disp-color",
> + "mediatek,mt8173-disp-color";
> + reg = <0 0x1c003000 0 0x1000>;
> + interrupts = ;
> + power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
> + clocks = <&vdosys0 CLK_VDO0_DISP_COLOR0>;
> + mediatek,gce-client-reg =
> + <&gce0 SUBSYS_1c00 0x3000 0x1000>;
> + };
> +
> + ccorr0: ccorr@1c004000 {
> + compatible = "mediatek,mt8195-disp-ccorr",
> + "mediatek,mt8192-disp-ccorr";
> + reg = <0 0x1c004000 0 0x1000>;
> + interrupts = ;
> + power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
> + clocks = <&vdosys0 CLK_VDO0_DISP_CCORR0>;
> + mediatek,gce-client-reg =
> + <&gce0 SUBSYS_1c00 0x4000 0x1000>;
> + };
> +
> + aal0: aal@1c005000 {
> + compatible = "mediatek,mt8195-disp-aal",
> + "mediatek,mt8183-disp-aal";
> + reg = <0 0x1c005000 0 0x1000>;
> + interrupts = ;
> + power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
> + clocks = <&vdosys0 CLK_VDO0_DISP_AAL0>;
> + mediatek,gce-client-reg =
> + <&gce0 SUBSYS_1c00 0x5000 0x1000>;
> + };
> +
> + gamma0: gamma@1c006000 {
> + compatible = "mediatek,mt8195-disp-gamma",
> + "mediatek,mt8183-disp-gamma";
> + reg = <0 0x1c006000 0 0x1000>;
> + interrupts = ;
> + power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
> + clocks = <&vdosys0 CLK_VDO0_DISP_GAMMA0>;
> + mediatek,gce-client-reg =
> + <&gce0 SUBSYS_1c00 0x6000 0x1000>;
> + };
> +
> + dither0: dither@1c007000 {
> + compatible = "mediatek,mt8195-disp-dither",
> + "mediatek,mt8183-disp-dither";
> + reg = <0 0x1c007000 0 0x1000>;
> + interrupts = ;
> + power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
> + clocks = <&vdosys0 CLK_VDO0_DISP_DITHER0>;
> + mediatek,gce-client-reg =
> + <&gce0 SUBSYS_1c00 0x7000 0x1000>;
> + };
> +
> + dsc0: dsc@1c009000 {
> + c
Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller
On 04/07/2022 12:00, Tinghan Shen wrote:
> Add power domains controller node for mt8195.
>
> Signed-off-by: Weiyi Lu
> Signed-off-by: Tinghan Shen
> ---
> arch/arm64/boot/dts/mediatek/mt8195.dtsi | 327 +++
> 1 file changed, 327 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> index 8d59a7da3271..d52e140d9271 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> @@ -10,6 +10,7 @@
> #include
> #include
> #include
> +#include
>
> / {
> compatible = "mediatek,mt8195";
> @@ -338,6 +339,332 @@
> #interrupt-cells = <2>;
> };
>
> + scpsys: syscon@10006000 {
> + compatible = "syscon", "simple-mfd";
These compatibles cannot be alone.
> + reg = <0 0x10006000 0 0x1000>;
> + #power-domain-cells = <1>;
If it is simple MFD, then probably it is not a power domain provider.
Decide.
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 02/16] dt-bindings: memory: mediatek: Update condition for mt8195 smi node
On 04/07/2022 12:00, Tinghan Shen wrote: > The max clock items for the dts node with compatible > 'mediatek,mt8195-smi-sub-common' should be 3. > > However, the dtbs_check of such node will get following message, > arch/arm64/boot/dts/mediatek/mt8195-evb.dtb: smi@1401: clock-names: > ['apb', 'smi', 'gals0'] is too long > From schema: > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml > > Remove the last 'else' checking to fix this error. Missing fixes tag. > > Signed-off-by: Tinghan Shen > --- > .../memory-controllers/mediatek,smi-common.yaml| 10 +- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git > a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml > > b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml > index a98b359bf909..e5f553e2e12a 100644 > --- > a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml > +++ > b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml > @@ -143,7 +143,15 @@ allOf: > - const: gals0 > - const: gals1 > > -else: # for gen2 HW that don't have gals > + - if: # for gen2 HW that don't have gals > + properties: > +compatible: > + enum: > +- mediatek,mt2712-smi-common > +- mediatek,mt8167-smi-common > +- mediatek,mt8173-smi-common > + Without looking at the code, it's impossible to understand what you are doing here. The commit msg says one, but you are doing something else. Write commit msg explaining what you want to achieve and what you are doing. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/4] iommu/exynos: Use lookup based approach to access v7 registers
On 02/07/2022 23:37, Sam Protsenko wrote:
> SysMMU v7 might have different register layouts (VM capable or non-VM
> capable). Check which layout is implemented in current SysMMU module and
> prepare the corresponding register table for futher usage. This way is
> faster and more elegant than checking corresponding condition (if it's
> VM or non-VM SysMMU) each time before accessing v7 registers. For now
> the register table contains only most basic registers needed to add the
> SysMMU v7 support.
>
> This patch is based on downstream work of next authors:
> - Janghyuck Kim
> - Daniel Mentz
>
> Signed-off-by: Sam Protsenko
> ---
> drivers/iommu/exynos-iommu.c | 46
> 1 file changed, 46 insertions(+)
>
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index df6ddbebbe2b..47017e8945c5 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -180,6 +180,47 @@ static u32 lv2ent_offset(sysmmu_iova_t iova)
>
> #define has_sysmmu(dev) (dev_iommu_priv_get(dev) != NULL)
>
> +#define MMU_REG(data, idx) \
> + ((data)->sfrbase + (data)->regs[idx].off)
I would expect to see users of this - convert all existing regs.
> +#define MMU_VM_REG(data, idx, vmid) \
> + (MMU_REG(data, idx) + (vmid) * (data)->regs[idx].mult)
> +
> +enum {
> + REG_SET_NON_VM,
> + REG_SET_VM,
> + MAX_REG_SET
> +};
> +
> +enum {
> + IDX_CTRL_VM,
> + IDX_CFG_VM,
> + IDX_FLPT_BASE,
Isn't this called REG_MMU_FLUSH? If yes, it's a bit confusing to see
different name used.
> + IDX_ALL_INV,
Isn't this called REG_MMU_FLUSH_ENTRY?
> + MAX_REG_IDX
> +};
> +
> +struct sysmmu_vm_reg {
> + unsigned int off; /* register offset */
> + unsigned int mult; /* VM index offset multiplier */
> +};
> +
> +static const struct sysmmu_vm_reg sysmmu_regs[MAX_REG_SET][MAX_REG_IDX] = {
> + /* Default register set (non-VM) */
> + {
> + /*
> + * SysMMUs without VM support do not have CTRL_VM and CFG_VM
> + * registers. Setting the offsets to 1 will trigger an unaligned
> + * access exception.
So why are you setting offset 1? To trigger unaligned access?
> + */
> + {0x1}, {0x1}, {0x000c}, {0x0010},
> + },
> + /* VM capable register set */
> + {
> + {0x8000, 0x1000}, {0x8004, 0x1000}, {0x800c, 0x1000},
> + {0x8010, 0x1000},
> + },
You add here quite a bit of dead code and some hard-coded numbers.
> +};
> +
> static struct device *dma_dev;
> static struct kmem_cache *lv2table_kmem_cache;
> static sysmmu_pte_t *zero_lv2_table;
> @@ -284,6 +325,7 @@ struct sysmmu_drvdata {
>
> /* v7 fields */
> bool has_vcr; /* virtual machine control register */
> + const struct sysmmu_vm_reg *regs; /* register set */
> };
>
> static struct exynos_iommu_domain *to_exynos_domain(struct iommu_domain *dom)
> @@ -407,6 +449,10 @@ static void sysmmu_get_hw_info(struct sysmmu_drvdata
> *data)
> __sysmmu_get_version(data);
> if (MMU_MAJ_VER(data->version) >= 7 && __sysmmu_has_capa1(data))
> __sysmmu_get_vcr(data);
> + if (data->has_vcr)
> + data->regs = sysmmu_regs[REG_SET_VM];
> + else
> + data->regs = sysmmu_regs[REG_SET_NON_VM];
This is set and not read.
>
> __sysmmu_disable_clocks(data);
> }
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/4] iommu/exynos: Check if SysMMU v7 has VM registers
On 02/07/2022 23:37, Sam Protsenko wrote:
> SysMMU v7 can have Virtual Machine registers, which implement multiple
> translation domains. The driver should know if it's true or not, as VM
> registers shouldn't be accessed if not present. Read corresponding
> capabilities register to obtain that info, and store it in driver data.
>
> Signed-off-by: Sam Protsenko
> ---
> drivers/iommu/exynos-iommu.c | 42 ++--
> 1 file changed, 36 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index 28f8c8d93aa3..df6ddbebbe2b 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -135,6 +135,9 @@ static u32 lv2ent_offset(sysmmu_iova_t iova)
> #define CFG_SYSSEL (1 << 22) /* System MMU 3.2 only */
> #define CFG_FLPDCACHE(1 << 20) /* System MMU 3.2+ only */
>
> +#define CAPA0_CAPA1_EXISTBIT(11)
> +#define CAPA1_VCR_ENABLEDBIT(14)
> +
> /* common registers */
> #define REG_MMU_CTRL 0x000
> #define REG_MMU_CFG 0x004
> @@ -171,6 +174,10 @@ static u32 lv2ent_offset(sysmmu_iova_t iova)
> #define REG_V5_FAULT_AR_VA 0x070
> #define REG_V5_FAULT_AW_VA 0x080
>
> +/* v7.x registers */
> +#define REG_V7_CAPA0 0x870
> +#define REG_V7_CAPA1 0x874
> +
> #define has_sysmmu(dev) (dev_iommu_priv_get(dev) != NULL)
>
> static struct device *dma_dev;
> @@ -274,6 +281,9 @@ struct sysmmu_drvdata {
> unsigned int version; /* our version */
>
> struct iommu_device iommu; /* IOMMU core handle */
> +
> + /* v7 fields */
> + bool has_vcr; /* virtual machine control register */
> };
>
> static struct exynos_iommu_domain *to_exynos_domain(struct iommu_domain *dom)
> @@ -364,11 +374,7 @@ static void __sysmmu_disable_clocks(struct
> sysmmu_drvdata *data)
>
> static void __sysmmu_get_version(struct sysmmu_drvdata *data)
> {
> - u32 ver;
> -
> - __sysmmu_enable_clocks(data);
> -
> - ver = readl(data->sfrbase + REG_MMU_VERSION);
> + const u32 ver = readl(data->sfrbase + REG_MMU_VERSION);
No need for const for local, non-pointer variables. There is no benefit
in preventing the modification and it is not a constant.
>
> /* controllers on some SoCs don't report proper version */
> if (ver == 0x8001u)
> @@ -378,6 +384,29 @@ static void __sysmmu_get_version(struct sysmmu_drvdata
> *data)
>
> dev_dbg(data->sysmmu, "hardware version: %d.%d\n",
> MMU_MAJ_VER(data->version), MMU_MIN_VER(data->version));
> +}
> +
> +static bool __sysmmu_has_capa1(struct sysmmu_drvdata *data)
> +{
> + const u32 capa0 = readl(data->sfrbase + REG_V7_CAPA0);
Same here and further.
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/4] iommu/exynos: Set correct dma mask for SysMMU v5+
On 02/07/2022 23:37, Sam Protsenko wrote:
> SysMMU v5+ supports 36 bit physical address space. Set corresponding DMA
> mask to avoid falling back to SWTLBIO usage in dma_map_single() because
> of failed dma_capable() check.
>
> The original code for this fix was suggested by Marek.
>
> Originally-by: Marek Szyprowski
This is some tip specific tag, I don't think checkpatch allows it.
Either use suggesgted-by or co-developed-by + SoB.
> Signed-off-by: Sam Protsenko
> ---
> drivers/iommu/exynos-iommu.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index 71f2018e23fe..28f8c8d93aa3 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -647,6 +647,14 @@ static int exynos_sysmmu_probe(struct platform_device
> *pdev)
> }
> }
>
> + if (MMU_MAJ_VER(data->version) >= 5) {
> + ret = dma_set_mask(dev, DMA_BIT_MASK(36));
> + if (ret) {
> + dev_err(dev, "Unable to set DMA mask: %d\n", ret);
Missing cleanup: iommu_device_unregister
and probably also: iommu_device_sysfs_remove
> + return ret;
> + }
> + }
> +
> /*
>* use the first registered sysmmu device for performing
>* dma mapping operations on iommu page tables (cpu cache flush)
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/exynos: Make driver independent of the system page size
On 23/06/2022 11:36, Marek Szyprowski wrote:
> PAGE_{SIZE,SHIFT} macros depend on the configured kernel's page size, but
> the driver expects values calculated as for 4KB pages. Fix this.
>
> Reported-by: Robin Murphy
> Signed-off-by: Marek Szyprowski
> ---
> Untested, because Exynos based boards I have doesn't boot with non-4KB
> page size for other reasons.
> ---
> drivers/iommu/exynos-iommu.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
Acked-by: Krzysztof Kozlowski
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH V3 5/8] dt-bindings: Add xen,grant-dma IOMMU description for xen-grant DMA ops
On 30/05/2022 23:00, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko
Thank you for your patch. There is something to discuss/improve.
> diff --git a/Documentation/devicetree/bindings/iommu/xen,grant-dma.yaml
> b/Documentation/devicetree/bindings/iommu/xen,grant-dma.yaml
> new file mode 100644
> index ..ab5765c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iommu/xen,grant-dma.yaml
> @@ -0,0 +1,49 @@
> +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iommu/xen,grant-dma.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Xen specific IOMMU for virtualized devices (e.g. virtio)
> +
> +maintainers:
> + - Stefano Stabellini
> +
> +description:
> + The reference to Xen specific IOMMU node using "iommus" property indicates
> + that Xen grant mappings need to be enabled for the device, and it specifies
> + the ID of the domain where the corresponding backend resides.
> + The binding is required to restrict memory access using Xen grant mappings.
> +
> +properties:
> + compatible:
> +const: xen,grant-dma
> +
> + '#iommu-cells':
> +const: 1
> +description:
> + Xen specific IOMMU is multiple-master IOMMU device.
> + The single cell describes the domid (domain ID) of the domain where
> + the backend is running.
> +
> +required:
> + - compatible
> + - "#iommu-cells"
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +xen_iommu {
No underscores in node names, generic node names, so this looks like
"iommu".
> +compatible = "xen,grant-dma";
> +#iommu-cells = <1>;
> +};
> +
> +virtio@3000 {
> +compatible = "virtio,mmio";
> +reg = <0x3000 0x100>;
> +interrupts = <41>;
> +
> +/* The backend is located in Xen domain with ID 1 */
> +iommus = <&xen_iommu 1>;
There is no need usually to give consumer examples in provider binding.
If there is nothing specific here (looks exactly like every IOMMU
consumer in Linux kernel), drop the consumer.
> +};
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/8] dt-bindings: iommu: mediatek: Add mediatek,pericfg phandle
On 17/05/2022 15:21, AngeloGioacchino Del Regno wrote: > Add property "mediatek,pericfg" to let the mtk_iommu driver retrieve > a phandle to the pericfg syscon instead of performing a per-soc > compatible lookup, as it was also done with infracfg. > > Signed-off-by: AngeloGioacchino Del Regno > > --- > Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml | 4 > 1 file changed, 4 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > index 78c72c22740b..a6cf9678271f 100644 > --- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > +++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > @@ -116,6 +116,10 @@ properties: >Refer to bindings/memory-controllers/mediatek,smi-larb.yaml. It must > sort >according to the local arbiter index, like larb0, larb1, larb2... > > + mediatek,pericfg: > +$ref: "/schemas/types.yaml#/definitions/phandle" No need for quotes. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/8] dt-bindings: iommu: mediatek: Add mediatek,infracfg phandle
On 17/05/2022 15:21, AngeloGioacchino Del Regno wrote: > Add property "mediatek,infracfg" to let the mtk_iommu driver retrieve > a phandle to the infracfg syscon instead of performing a per-soc > compatible lookup. > > Signed-off-by: AngeloGioacchino Del Regno > > --- > Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml | 4 > 1 file changed, 4 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > index 2ae3bbad7f1a..78c72c22740b 100644 > --- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > +++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > @@ -101,6 +101,10 @@ properties: > items: >- const: bclk > > + mediatek,infracfg: > +$ref: "/schemas/types.yaml#/definitions/phandle" No need for quotes. They are not present in other places. > +description: The phandle to the mediatek infracfg syscon > + >mediatek,larbs: > $ref: /schemas/types.yaml#/definitions/phandle-array > minItems: 1 Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] dt-bindings: arm-smmu: Add compatible for Qualcomm SC8280XP
On 03/05/2022 18:34, Bjorn Andersson wrote: > Add compatible for the Qualcomm SC8280XP platform to the ARM SMMU > DeviceTree binding. > > Signed-off-by: Bjorn Andersson Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 6/7] dt-bindings: serial: renesas, scif: R-Car V3U is R-Car Gen4
On 02/05/2022 15:34, Geert Uytterhoeven wrote: > Despite the name, R-Car V3U is the first member of the R-Car Gen4 > family. Hence move its compatible value to the R-Car Gen4 section. > Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 5/7] dt-bindings: serial: renesas,hscif: R-Car V3U is R-Car Gen4
On 02/05/2022 15:34, Geert Uytterhoeven wrote: > Despite the name, R-Car V3U is the first member of the R-Car Gen4 > family. Hence move its compatible value to the R-Car Gen4 section. > > Signed-off-by: Geert Uytterhoeven Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 4/7] dt-bindings: renesas,rcar-dmac: R-Car V3U is R-Car Gen4
On 02/05/2022 15:34, Geert Uytterhoeven wrote: > Despite the name, R-Car V3U is the first member of the R-Car Gen4 > family. Hence move its compatible value to the R-Car Gen4 section. > > Signed-off-by: Geert Uytterhoeven > --- > .../devicetree/bindings/dma/renesas,rcar-dmac.yaml | 10 -- > 1 file changed, 4 insertions(+), 6 deletions(-) Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 7/7] dt-bindings: watchdog: renesas,wdt: R-Car V3U is R-Car Gen4
On 02/05/2022 15:34, Geert Uytterhoeven wrote: > Despite the name, R-Car V3U is the first member of the R-Car Gen4 > family. Hence move its compatible value to the R-Car Gen4 section. > > Signed-off-by: Geert Uytterhoeven Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/7] dt-bindings: iommu: renesas, ipmmu-vmsa: R-Car V3U is R-Car Gen4
On 02/05/2022 15:34, Geert Uytterhoeven wrote: > Despite the name, R-Car V3U is the first member of the R-Car Gen4 > family. Hence move its compatible value to the R-Car Gen4 section. > > Signed-off-by: Geert Uytterhoeven Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/7] dt-bindings: i2c: renesas,rcar-i2c: R-Car V3U is R-Car Gen4
On 02/05/2022 15:34, Geert Uytterhoeven wrote: > Despite the name, R-Car V3U is the first member of the R-Car Gen4 > family. I2C on R-Car V3U also supports some extra features (e.g. Slave > Clock Stretch Select), which are supported by other R-Car Gen4 SoCs, but > not by any other R-Car Gen3 SoC. > > Hence move its compatible value to the R-Car Gen4 section. > > Signed-off-by: Geert Uytterhoeven Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/7] dt-bindings: gpio: renesas,rcar-gpio: R-Car V3U is R-Car Gen4
On 02/05/2022 15:34, Geert Uytterhoeven wrote: > Despite the name, R-Car V3U is the first member of the R-Car Gen4 > family. Hence move its compatible value to the R-Car Gen4 section. > > Signed-off-by: Geert Uytterhoeven Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/4] dt-bindings: arm-smmu: Add binding for SDX65 SMMU
On 02/05/2022 10:37, Rohit Agarwal wrote: > Add devicetree binding for Qualcomm SDX65 SMMU. > > Signed-off-by: Rohit Agarwal Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 01/11] dt-bindings: iommu: arm,smmu-v3: make PRI IRQ optional
On 28/04/2022 11:23, Robin Murphy wrote: > On 2022-04-28 07:56, Krzysztof Kozlowski wrote: >> On 27/04/2022 13:25, Andre Przywara wrote: >>> The Page Request Interface (PRI) is an optional PCIe feature. As such, a >>> SMMU would not need to handle it if the PCIe host bridge or the SMMU >>> itself do not implement it. Also an SMMU could be connected to a platform >>> device, without any PRI functionality whatsoever. >>> In all cases there would be no SMMU PRI queue interrupt to be wired up >>> to an interrupt controller. >>> >>> Relax the binding to allow specifying three interrupts, omitting the PRI >>> IRQ. At the moment, with the "eventq,gerror,priq,cmdq-sync" order, we >>> would need to sacrifice the command queue sync interrupt as well, which >>> might not be desired. >>> The Linux driver does not care about any order at all, just picks IRQs >>> based on their names, and treats all (wired) IRQs as optional. >> >> The last sentence is not a good explanation for the bindings. They are >> not about Linux and are used in other projects as well. >> >>> >>> Signed-off-by: Andre Przywara >>> --- >>> .../bindings/iommu/arm,smmu-v3.yaml | 21 ++- >>> 1 file changed, 16 insertions(+), 5 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml >>> b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml >>> index e87bfbcc69135..6b3111f1f06ce 100644 >>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml >>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml >>> @@ -37,12 +37,23 @@ properties: >>> hardware supports just a single, combined interrupt line. >>> If provided, then the combined interrupt will be used in >>> preference to >>> any others. >>> - - minItems: 2 >>> + - minItems: 1 >>> items: >>> - - const: eventq # Event Queue not empty >>> - - const: gerror # Global Error activated >>> - - const: priq # PRI Queue not empty >>> - - const: cmdq-sync # CMD_SYNC complete >>> + - enum: >>> + - eventq # Event Queue not empty >>> + - gerror # Global Error activated >>> + - cmdq-sync # CMD_SYNC complete >>> + - priq # PRI Queue not empty >>> + - enum: >>> + - gerror >>> + - cmdq-sync >>> + - priq >>> + - enum: >>> + - cmdq-sync >>> + - priq >>> + - enum: >>> + - cmdq-sync >>> + - priq >> >> The order should be strict, so if you want the first interrupt optional, >> then: >> oneOf: >> - items: >> ... 4 items list >> - items >> ... 3 items list > > All 4 interrupts are optional, though, since any of them could > potentially use an MSI instead. Do we really want to list out all 15 > combinations? Bah, I missed that part from commit msg. It's fine then. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/5] dt-bindings: iommu: sun50i: Add compatible for Allwinner D1
On 28/04/2022 03:03, Samuel Holland wrote: Thank you for your patch. There is something to discuss/improve. > + > +if: > + properties: > +compatible: > + contains: > +enum: > + - allwinner,sun50i-h6-iommu > + > +then: > + required: > +- resets else: properties: resets: false > Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 01/11] dt-bindings: iommu: arm,smmu-v3: make PRI IRQ optional
On 27/04/2022 13:25, Andre Przywara wrote: > The Page Request Interface (PRI) is an optional PCIe feature. As such, a > SMMU would not need to handle it if the PCIe host bridge or the SMMU > itself do not implement it. Also an SMMU could be connected to a platform > device, without any PRI functionality whatsoever. > In all cases there would be no SMMU PRI queue interrupt to be wired up > to an interrupt controller. > > Relax the binding to allow specifying three interrupts, omitting the PRI > IRQ. At the moment, with the "eventq,gerror,priq,cmdq-sync" order, we > would need to sacrifice the command queue sync interrupt as well, which > might not be desired. > The Linux driver does not care about any order at all, just picks IRQs > based on their names, and treats all (wired) IRQs as optional. The last sentence is not a good explanation for the bindings. They are not about Linux and are used in other projects as well. > > Signed-off-by: Andre Przywara > --- > .../bindings/iommu/arm,smmu-v3.yaml | 21 ++- > 1 file changed, 16 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml > b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml > index e87bfbcc69135..6b3111f1f06ce 100644 > --- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml > @@ -37,12 +37,23 @@ properties: >hardware supports just a single, combined interrupt line. >If provided, then the combined interrupt will be used in > preference to >any others. > - - minItems: 2 > + - minItems: 1 > items: > - - const: eventq # Event Queue not empty > - - const: gerror # Global Error activated > - - const: priq # PRI Queue not empty > - - const: cmdq-sync # CMD_SYNC complete > + - enum: > + - eventq # Event Queue not empty > + - gerror # Global Error activated > + - cmdq-sync # CMD_SYNC complete > + - priq # PRI Queue not empty > + - enum: > + - gerror > + - cmdq-sync > + - priq > + - enum: > + - cmdq-sync > + - priq > + - enum: > + - cmdq-sync > + - priq The order should be strict, so if you want the first interrupt optional, then: oneOf: - items: ... 4 items list - items ... 3 items list Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 2/7] dt-bindings: mmc: sdhci-msm: Document the SDX65 compatible
On 11/04/2022 11:50, Rohit Agarwal wrote: > The SDHCI controller on SDX65 is based on MSM SDHCI v5 IP. Hence, > document the compatible with "qcom,sdhci-msm-v5" as the fallback. > > Signed-off-by: Rohit Agarwal > --- > Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 1 + > 1 file changed, 1 insertion(+) > Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 4/7] dt-bindings: arm-smmu: Add binding for SDX65 SMMU
On 11/04/2022 11:50, Rohit Agarwal wrote: > Add devicetree binding for Qualcomm SDX65 SMMU. > > Signed-off-by: Rohit Agarwal > --- > Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 1 + > 1 file changed, 1 insertion(+) > Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 0/7] MT8186 SMI SUPPORT
On Thu, 13 Jan 2022 19:10:50 +0800, Yong Wu wrote: > This patchset adds mt8186 smi support. > mainly adds a sleep control function. > > Change note: > v3: a) Add a new binding patch for renaming "clock" to "clocks". > b) Reword the title for the binding patches, more detailed. > c) Add the sleep control error path: if err, return directly. >also change the log from dev_warn to dev_err. > > [...] Applied, thanks! [1/7] dt-bindings: memory: mtk-smi: Rename clock to clocks commit: 5bf7fa48374eafe29dbb30448a0b0c083853583f [2/7] dt-bindings: memory: mtk-smi: No need mediatek,larb-id for mt8167 commit: ddc3a324889686ec9b358de20fdeec0d2668c7a8 [3/7] dt-bindings: memory: mtk-smi: Correct minItems to 2 for the gals clocks commit: 996ebc0e332bfb3091395f9bd286d8349a57be62 [4/7] dt-bindings: memory: mediatek: Add mt8186 support commit: 6d86f23c35fe7b479ceef4d3f1eef925996945fd [5/7] memory: mtk-smi: Fix the return value for clk_bulk_prepare_enable commit: a6945f4566d4f77a4054720f6649ff921fe1ae64 [6/7] memory: mtk-smi: Add sleep ctrl function commit: 8956500e5d5bf541a94529b0bf4866dc0daf [7/7] memory: mtk-smi: mt8186: Add smi support commit: 86a010bfc73983aa8cd914f1e5f73962b0406678 Best regards, -- Krzysztof Kozlowski ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] dt-bindings: mediatek: mt8186: Add binding for MM iommu
On 25/01/2022 10:32, Yong Wu wrote: > Add mt8186 iommu binding. "-mm" means the iommu is for Multimedia. > > Signed-off-by: Yong Wu > --- > .../bindings/iommu/mediatek,iommu.yaml| 4 + > .../dt-bindings/memory/mt8186-memory-port.h | 217 ++ > 2 files changed, 221 insertions(+) > create mode 100644 include/dt-bindings/memory/mt8186-memory-port.h > Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC 2/3] iommu/samsung: Introduce Exynos sysmmu-v8 driver
On 20/01/2022 21:19, Sam Protsenko wrote: > Introduce new driver for modern Exynos ARMv8 SoCs, e.g. Exynos850. Also > it's used for Google's GS101 SoC. > > This is squashed commit, contains next patches of different authors. See > `iommu-exynos850-dev' branch for details: [1]. > > Original authors (Samsung): > > - Cho KyongHo > - Hyesoo Yu > - Janghyuck Kim > - Jinkyu Yang > > Some improvements were made by Google engineers: > > - Alex > - Carlos Llamas > - Daniel Mentz > - Erick Reyes > - J. Avila > - Jonglin Lee > - Mark Salyzyn > - Thierry Strudel > - Will McVicker > > [1] https://github.com/joe-skb7/linux/tree/iommu-exynos850-dev > > Signed-off-by: Sam Protsenko > --- > drivers/iommu/Kconfig | 13 + > drivers/iommu/Makefile |3 + > drivers/iommu/samsung-iommu-fault.c | 617 +++ > drivers/iommu/samsung-iommu-group.c | 50 + > drivers/iommu/samsung-iommu.c | 1521 +++ > drivers/iommu/samsung-iommu.h | 216 > 6 files changed, 2420 insertions(+) > create mode 100644 drivers/iommu/samsung-iommu-fault.c > create mode 100644 drivers/iommu/samsung-iommu-group.c > create mode 100644 drivers/iommu/samsung-iommu.c > create mode 100644 drivers/iommu/samsung-iommu.h > Existing driver supports several different Exynos SysMMU IP block versions. Several. Please explain why it cannot support one more version? Similarity of vendor driver with mainline is not an argument. > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig > index 3eb68fa1b8cc..78e7039f18aa 100644 > --- a/drivers/iommu/Kconfig > +++ b/drivers/iommu/Kconfig > @@ -452,6 +452,19 @@ config QCOM_IOMMU > help > Support for IOMMU on certain Qualcomm SoCs. > > +config SAMSUNG_IOMMU > + tristate "Samsung IOMMU Support" > + select ARM_DMA_USE_IOMMU > + select IOMMU_DMA > + select SAMSUNG_IOMMU_GROUP > + help > + Support for IOMMU on Samsung Exynos SoCs. > + > +config SAMSUNG_IOMMU_GROUP > + tristate "Samsung IOMMU Group Support" > + help > + Support for IOMMU group on Samsung Exynos SoCs. > + > config HYPERV_IOMMU > bool "Hyper-V x2APIC IRQ Handling" > depends on HYPERV && X86 > diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile > index bc7f730edbb0..a8bdf449f1d4 100644 > --- a/drivers/iommu/Makefile > +++ b/drivers/iommu/Makefile > @@ -27,6 +27,9 @@ obj-$(CONFIG_FSL_PAMU) += fsl_pamu.o fsl_pamu_domain.o > obj-$(CONFIG_S390_IOMMU) += s390-iommu.o > obj-$(CONFIG_HYPERV_IOMMU) += hyperv-iommu.o > obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o > +obj-$(CONFIG_SAMSUNG_IOMMU) += samsung_iommu.o > +samsung_iommu-objs += samsung-iommu.o samsung-iommu-fault.o > +obj-$(CONFIG_SAMSUNG_IOMMU_GROUP) += samsung-iommu-group.o > obj-$(CONFIG_IOMMU_SVA_LIB) += iommu-sva-lib.o io-pgfault.o > obj-$(CONFIG_SPRD_IOMMU) += sprd-iommu.o > obj-$(CONFIG_APPLE_DART) += apple-dart.o > diff --git a/drivers/iommu/samsung-iommu-fault.c > b/drivers/iommu/samsung-iommu-fault.c > new file mode 100644 > index ..c6b4259976c4 > --- /dev/null > +++ b/drivers/iommu/samsung-iommu-fault.c > @@ -0,0 +1,617 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2020 Samsung Electronics Co., Ltd. > + */ > + > +#define pr_fmt(fmt) "sysmmu: " fmt > + > +#include > +#include > +#include > + > +#include "samsung-iommu.h" > + > +#define MMU_TLB_INFO(n) (0x2000 + ((n) * 0x20)) > +#define MMU_CAPA1_NUM_TLB_SET(reg) (((reg) >> 16) & 0xFF) > +#define MMU_CAPA1_NUM_TLB_WAY(reg) ((reg) & 0xFF) > +#define MMU_CAPA1_SET_TLB_READ_ENTRY(tid, set, way, line)\ > + ((set) | ((way) << 8) | \ > + ((line) << 16) | ((tid) << 20)) > + > +#define MMU_TLB_ENTRY_VALID(reg) ((reg) >> 28) > +#define MMU_SBB_ENTRY_VALID(reg) ((reg) >> 28) > +#define MMU_VADDR_FROM_TLB(reg, idx) reg) & 0xC) | ((idx) & 0x3)) << > 12) > +#define MMU_VID_FROM_TLB(reg)(((reg) >> 20) & 0x7U) > +#define MMU_PADDR_FROM_TLB(reg) ((phys_addr_t)((reg) & > 0xFF) << 12) > +#define MMU_VADDR_FROM_SBB(reg) (((reg) & 0xF) << 12) > +#define MMU_VID_FROM_SBB(reg)(((reg) >> 20) & 0x7U) > +#define MMU_PADDR_FROM_SBB(reg) ((phys_addr_t)((reg) & > 0x3FF) << 10) > + > +#define REG_MMU_INT_STATUS 0x060 > +#define REG_MMU_INT_CLEAR0x064 > +#define REG_MMU_FAULT_RW_MASKGENMASK(20, 20) > +#define IS_READ_FAULT(x) (((x) & REG_MMU_FAULT_RW_MASK) == 0) > + > +#define SYSMMU_FAULT_PTW_ACCESS 0 > +#define SYSMMU_FAULT_PAGE_FAULT 1 > +#define SYSMMU_FAULT_ACCESS 2 > +#define SYSMMU_FAULT_RESERVED 3 > +#define SYSMMU_FAULT_UNKNOWN 4 > + > +#define SYSMMU_SEC_FAULT_MASK(BIT(SYSMMU_FAULT_PTW_ACCESS) | > \ > +
Re: [RFC 0/3] iommu/samsung: Introduce Exynos sysmmu-v8 driver
On 20/01/2022 21:19, Sam Protsenko wrote: > This is a draft of a new IOMMU driver used in modern Exynos SoCs (like > Exynos850) and Google's GS101 SoC (used in Pixel 6 phone). Most of its > code were taken from GS101 downstream kernel [1], with some extra > patches on top (fixes from Exynos850 downstream kernel and some porting > changes to adapt it to the mainline kernel). All development history can > be found at [2]. > > Similarities with existing exynos-iommu.c is minimal. I did some > analysis using similarity-tester tool: > > 8<>8 > $ sim_c -peu -S exynos-iommu.c "|" samsung-* > > exynos-iommu.c consists for 15 % of samsung-iommu.c material > exynos-iommu.c consists for 1 % of samsung-iommu-fault.c material > exynos-iommu.c consists for 3 % of samsung-iommu.h material > 8<>8 > > So the similarity is very low, most of that code is some boilerplate > that shouldn't be extracted to common code (like allocating the memory > and requesting clocks/interrupts in probe function). This is not a prove of lack of similarities. The vendor drivers have proven track of poor quality and a lot of code not compatible with Linux kernel style. Therefore comparing mainline driver, reviewed and well tested, with a vendor out-of-tree driver is wrong. You will almost always have 0% of similarities, because vendor kernel drivers are mostly developed from scratch instead of re-using existing drivers. Recently Samsung admitted it - if I extend existing driver, I will have to test old and new platform, so it is easier for me to write a new driver. No, this is not that approach we use it in mainline. Linaro should know it much better. > > It was tested on v5.4 Android kernel on Exynos850 (E850-96 board) with > DPU use-case (displaying some graphics to the screen). Also it > apparently works fine on v5.10 GS101 kernel (on Pixel 6). On mainline > kernel I managed to build, match and bind the driver. No real world test > was done, but the changes from v5.10 (where it works fine) are minimal > (see [2] for details). So I'm pretty sure the driver is functional. No, we do not take untested code or code for different out-of-tree kernels, not for mainline. I am pretty sure drivers is poor or not working. > > For this patch series I'd like to receive some high-level review for > driver's design and architecture. Coding style and API issues I can fix > later, when sending real (not RFC) series. Particularly I'd like to hear > some opinions about: > - namings: Kconfig option, file names, module name, compatible, etc > - modularity: should this driver be a different platform driver (like > in this series), or should it be integrated into existing > exynos-iommu.c driver somehow > - dt-bindings: does it look ok as it is, or some interface changes are > needed You sent bindings in TXT with dead code inside, and you ask if it is ok. I consider this approach that you sent whatever junk to us hoping that we will point all the issues instead of finding them by yourself. I am pretty sure you have several folks in Linaro who can perform first review and bring the code closer to mainline style. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC 1/3] dt-bindings: iommu: Add bindings for samsung,sysmmu-v8
On 20/01/2022 21:19, Sam Protsenko wrote:
> Only example of usage and header for now.
>
> Signed-off-by: Sam Protsenko
> ---
> .../bindings/iommu/samsung,sysmmu-v8.txt | 31 +
Please, don't copy paste bindings or entire drviers from vendor kernel.
It looks very bad. Instead, submit them in dtschema.
NAK.
> include/dt-bindings/soc/samsung,sysmmu-v8.h | 43 +++
> 2 files changed, 74 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/iommu/samsung,sysmmu-v8.txt
> create mode 100644 include/dt-bindings/soc/samsung,sysmmu-v8.h
>
> diff --git a/Documentation/devicetree/bindings/iommu/samsung,sysmmu-v8.txt
> b/Documentation/devicetree/bindings/iommu/samsung,sysmmu-v8.txt
> new file mode 100644
> index ..d6004ea4a746
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iommu/samsung,sysmmu-v8.txt
> @@ -0,0 +1,31 @@
> +Example (Exynos850, IOMMU for DPU usage):
> +
> + #include
> +
> + /* IOMMU group info */
> + iommu_group_dpu: iommu_group_dpu {
> + compatible = "samsung,sysmmu-group";
> + };
> +
> + sysmmu_dpu: sysmmu@130c {
> + compatible = "samsung,sysmmu-v8";
> + reg = <0x130c 0x9000>;
> + interrupts = ,
> + ;
> + qos = <15>;
> +
> + clocks = <&cmu_dpu CLK_GOUT_DPU_SMMU_CLK>;
> + clock-names = "gate";
> +
> + sysmmu,secure-irq;
> + sysmmu,secure_base = <0x130d>;
> + sysmmu,default_tlb = ;
> + sysmmu,tlb_property =
> + <1 TLB_CFG(BL1, PREFETCH_PREDICTION) (DIR_READ | (1 <<
> 16)) SYSMMU_ID_MASK(0x2, 0xF)>,
> + <2 TLB_CFG(BL1, PREFETCH_PREDICTION) (DIR_READ | (1 <<
> 16)) SYSMMU_ID_MASK(0x4, 0xF)>,
> + <3 TLB_CFG(BL1, PREFETCH_PREDICTION) (DIR_READ | (1 <<
> 16)) SYSMMU_ID_MASK(0x6, 0xF)>,
> + <4 TLB_CFG(BL1, PREFETCH_PREDICTION) (DIR_READ | (1 <<
> 16)) SYSMMU_ID_MASK(0x8, 0xF)>;
> + port-name = "DPU";
> + #iommu-cells = <0>;
> + //power-domains = <&pd_dpu>;
We try not to store dead code in kernel.
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] dt-bindings: Improve phandle-array schemas
On 19/01/2022 02:50, Rob Herring wrote: > The 'phandle-array' type is a bit ambiguous. It can be either just an > array of phandles or an array of phandles plus args. Many schemas for > phandle-array properties aren't clear in the schema which case applies > though the description usually describes it. > > The array of phandles case boils down to needing: > > items: > maxItems: 1 > > The phandle plus args cases should typically take this form: > > items: > - items: > - description: A phandle > - description: 1st arg cell > - description: 2nd arg cell > > With this change, some examples need updating so that the bracketing of > property values matches the schema. > Samsung and memory controller bits look good: Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 5/7] memory: mtk-smi: Fix the return value for clk_bulk_prepare_enable
On 13/01/2022 12:10, Yong Wu wrote: > Function clk_bulk_prepare_enable() returns 0 for success or a negative > number for error. Fix this code style issue. The message does not really make sense. If negative is returned, then the check (ret < 0) was correct. I guess you wanted to say that common code style is to check for any non-zero return value, just like it's implementation in clk.h does. I'll adjust the commit msg when applying. > > Signed-off-by: Yong Wu > Reviewed-by: AngeloGioacchino Del Regno > > --- > drivers/memory/mtk-smi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c > index b883dcc0bbfa..e7b1a22b12ea 100644 > --- a/drivers/memory/mtk-smi.c > +++ b/drivers/memory/mtk-smi.c > @@ -480,7 +480,7 @@ static int __maybe_unused mtk_smi_larb_resume(struct > device *dev) > int ret; > > ret = clk_bulk_prepare_enable(larb->smi.clk_num, larb->smi.clks); > - if (ret < 0) > + if (ret) > return ret; > > /* Configure the basic setting for this larb */ > Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 5/6] memory: mtk-smi: Add sleep ctrl function
On 11/01/2022 07:39, Yong Wu wrote:
> Sleep control means that when the larb goes to sleep, we should wait a bit
> until all the current commands are finished. Thus, when the larb runtime
> suspends, we need to enable this function to wait until all the existed
> commands are finished. When the larb resumes, just disable this function.
> This function only improves the safety of bus. Add a new flag for this
> function. Prepare for mt8186.
>
> Signed-off-by: Anan Sun
> Signed-off-by: Yong Wu
> ---
> drivers/memory/mtk-smi.c | 35 ++-
> 1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> index e7b1a22b12ea..d8552f4caba4 100644
> --- a/drivers/memory/mtk-smi.c
> +++ b/drivers/memory/mtk-smi.c
> @@ -8,6 +8,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -32,6 +33,10 @@
> #define SMI_DUMMY0x444
>
> /* SMI LARB */
> +#define SMI_LARB_SLP_CON0xc
> +#define SLP_PROT_EN BIT(0)
> +#define SLP_PROT_RDYBIT(16)
> +
> #define SMI_LARB_CMD_THRT_CON0x24
> #define SMI_LARB_THRT_RD_NU_LMT_MSK GENMASK(7, 4)
> #define SMI_LARB_THRT_RD_NU_LMT (5 << 4)
> @@ -81,6 +86,7 @@
>
> #define MTK_SMI_FLAG_THRT_UPDATE BIT(0)
> #define MTK_SMI_FLAG_SW_FLAG BIT(1)
> +#define MTK_SMI_FLAG_SLEEP_CTL BIT(2)
> #define MTK_SMI_CAPS(flags, _x) (!!((flags) & (_x)))
>
> struct mtk_smi_reg_pair {
> @@ -371,6 +377,26 @@ static const struct of_device_id mtk_smi_larb_of_ids[] =
> {
> {}
> };
>
> +static int mtk_smi_larb_sleep_ctrl_enable(struct mtk_smi_larb *larb)
> +{
> + int ret;
> + u32 tmp;
> +
> + writel_relaxed(SLP_PROT_EN, larb->base + SMI_LARB_SLP_CON);
> + ret = readl_poll_timeout_atomic(larb->base + SMI_LARB_SLP_CON,
> + tmp, !!(tmp & SLP_PROT_RDY), 10, 1000);
> + if (ret) {
> + /* TODO: Reset this larb if it fails here. */
> + dev_warn(larb->smi.dev, "sleep ctrl is not ready(0x%x).\n",
> tmp);
> + }
> + return ret;
> +}
> +
> +static void mtk_smi_larb_sleep_ctrl_disable(struct mtk_smi_larb *larb)
> +{
> + writel_relaxed(0, larb->base + SMI_LARB_SLP_CON);
> +}
> +
> static int mtk_smi_device_link_common(struct device *dev, struct device
> **com_dev)
> {
> struct platform_device *smi_com_pdev;
> @@ -483,6 +509,9 @@ static int __maybe_unused mtk_smi_larb_resume(struct
> device *dev)
> if (ret)
> return ret;
>
> + if (MTK_SMI_CAPS(larb->larb_gen->flags_general, MTK_SMI_FLAG_SLEEP_CTL))
> + mtk_smi_larb_sleep_ctrl_disable(larb);
> +
> /* Configure the basic setting for this larb */
> larb_gen->config_port(dev);
>
> @@ -492,9 +521,13 @@ static int __maybe_unused mtk_smi_larb_resume(struct
> device *dev)
> static int __maybe_unused mtk_smi_larb_suspend(struct device *dev)
> {
> struct mtk_smi_larb *larb = dev_get_drvdata(dev);
> + int ret = 0;
> +
> + if (MTK_SMI_CAPS(larb->larb_gen->flags_general, MTK_SMI_FLAG_SLEEP_CTL))
> + ret = mtk_smi_larb_sleep_ctrl_enable(larb);
>
> clk_bulk_disable_unprepare(larb->smi.clk_num, larb->smi.clks);
> - return 0;
> + return ret;
I am wondering whether disabling clocks in error case is a proper step.
On suspend error, the PM core won't run any further callbacks on this
device. This means, it won't be resumed and your clocks stay disabled. I
think you should return early and leave the device in active state in
case of error.
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 4/6] memory: mtk-smi: Fix the return value for clk_bulk_prepare_enable
On 11/01/2022 07:39, Yong Wu wrote:
> The successful return value for clk_bulk_prepare_enable is 0, rather than
> "< 0". Fix this.
I do not understand. The commit description does not match the code.
What is the error here?
>
> Fixes: 0e14917c57f9 ("memory: mtk-smi: Use clk_bulk clock ops")
There is no bug to fix...
> Signed-off-by: Yong Wu
> ---
> drivers/memory/mtk-smi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> index b883dcc0bbfa..e7b1a22b12ea 100644
> --- a/drivers/memory/mtk-smi.c
> +++ b/drivers/memory/mtk-smi.c
> @@ -480,7 +480,7 @@ static int __maybe_unused mtk_smi_larb_resume(struct
> device *dev)
> int ret;
>
> ret = clk_bulk_prepare_enable(larb->smi.clk_num, larb->smi.clks);
> - if (ret < 0)
> + if (ret)
> return ret;
>
> /* Configure the basic setting for this larb */
>
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 2/6] dt-bindings: memory: mtk-smi: Fix the larb clock/clock-names dtbs warning
On 11/01/2022 07:39, Yong Wu wrote:
> Mute the warning from "make dtbs_check":
>
> larb@14017000: clock-names: ['apb', 'smi'] is too short
> arch/arm64/boot/dts/mediatek/mt8183-evb.dt.yaml
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-burnet.dt.yaml
> ...
>
> larb@1601: clock-names: ['apb', 'smi'] is too short
> arch/arm64/boot/dts/mediatek/mt8183-evb.dt.yaml
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-burnet.dt.yaml
>
> larb@1701: clock-names: ['apb', 'smi'] is too short
> arch/arm64/boot/dts/mediatek/mt8183-evb.dt.yaml
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-burnet.dt.yaml
>
> If a platform's larb supports gals, there will be some larbs have one
> more "gals" clock while the others still only need "apb"/"smi" clocks,
> then the minItems for clock and clock-names are 2.
>
> Fixes: 27bb0e42855a ("dt-bindings: memory: mediatek: Convert SMI to DT
> schema")
> Signed-off-by: Yong Wu
> ---
> .../bindings/memory-controllers/mediatek,smi-larb.yaml | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git
> a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
> b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
> index 80907e357892..884c0c74e5e4 100644
> ---
> a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
> +++
> b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
> @@ -80,10 +80,10 @@ allOf:
> then:
>properties:
> clock:
Separate patch:
This should be clocks. The same in second if. Now it is simply not
working...
> - items:
Putting min/maxItems under items is wrong. The second if also needs the
fixing. Please make it a separate patch before this one.
The schema was clearly not tested before...
> -minItems: 3
> -maxItems: 3
> + minItems: 2
> + maxItems: 3
> clock-names:
> + minItems: 2
>items:
> - const: apb
> - const: smi
>
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/6] dt-bindings: memory: mtk-smi: Fix larb-id dtbs_check warning
On 11/01/2022 07:38, Yong Wu wrote:
> Mute the warning from "make dtbs_check":
>
> larb@14016000: 'mediatek,larb-id' is a required property
> arch/arm64/boot/dts/mediatek/mt8167-pumpkin.dt.yaml
> larb@15001000: 'mediatek,larb-id' is a required property
> arch/arm64/boot/dts/mediatek/mt8167-pumpkin.dt.yaml
> larb@1601: 'mediatek,larb-id' is a required property
> arch/arm64/boot/dts/mediatek/mt8167-pumpkin.dt.yaml
Please explain why larb-id is not necessary on mediatek,mt8167-smi-larb.
IOW, what logical error was there (except the dtschema pointed out issue).
>
> Fixes: 27bb0e42855a ("dt-bindings: memory: mediatek: Convert SMI to DT
> schema")
> Signed-off-by: Yong Wu
> ---
> .../bindings/memory-controllers/mediatek,smi-larb.yaml | 1 -
> 1 file changed, 1 deletion(-)
>
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/4] memory: mtk-smi: Add sleep ctrl function
On 03/12/2021 07:40, Yong Wu wrote:
> sleep control means that when the larb go to sleep, we should wait a bit
s/go/goes/
> until all the current commands are finished. thus, when the larb runtime
Please start every sentence with a capital letter.
> suspend, we need enable this function to wait until all the existed
s/suspend/suspends/
s/we need enable/we need to enable/
> command are finished. when the larb resume, just disable this function.
s/command/commands/
s/resume/resumes/
> This function only improve the safe of bus. Add a new flag for this
s/improve/improves/
s/the safe/the safety/
> function. Prepare for mt8186.
In total it is hard to parse, really.
>
> Signed-off-by: Anan Sun
> Signed-off-by: Yong Wu
> ---
> drivers/memory/mtk-smi.c | 39 +++
> 1 file changed, 35 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> index b883dcc0bbfa..4b59b28e4d73 100644
> --- a/drivers/memory/mtk-smi.c
> +++ b/drivers/memory/mtk-smi.c
> @@ -8,6 +8,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -32,6 +33,10 @@
> #define SMI_DUMMY0x444
>
> /* SMI LARB */
> +#define SMI_LARB_SLP_CON0x00c
> +#define SLP_PROT_EN BIT(0)
> +#define SLP_PROT_RDYBIT(16)
> +
> #define SMI_LARB_CMD_THRT_CON0x24
> #define SMI_LARB_THRT_RD_NU_LMT_MSK GENMASK(7, 4)
> #define SMI_LARB_THRT_RD_NU_LMT (5 << 4)
> @@ -81,6 +86,7 @@
>
> #define MTK_SMI_FLAG_THRT_UPDATE BIT(0)
> #define MTK_SMI_FLAG_SW_FLAG BIT(1)
> +#define MTK_SMI_FLAG_SLEEP_CTL BIT(2)
> #define MTK_SMI_CAPS(flags, _x) (!!((flags) & (_x)))
>
> struct mtk_smi_reg_pair {
> @@ -371,6 +377,24 @@ static const struct of_device_id mtk_smi_larb_of_ids[] =
> {
> {}
> };
>
> +static int mtk_smi_larb_sleep_ctrl(struct device *dev, bool to_sleep)
> +{
Make two functions instead. There is no single code reuse (shared)
between sleep and resume. In the same time bool arguments are confusing
when looking at caller and one never knows whether true means to resume
or to sleep. Having two functions is obvious. Obvious code is easier to
read and maintain.
> + struct mtk_smi_larb *larb = dev_get_drvdata(dev);
> + int ret = 0;
> + u32 tmp;
> +
> + if (to_sleep) {
> + writel_relaxed(SLP_PROT_EN, larb->base + SMI_LARB_SLP_CON);
> + ret = readl_poll_timeout_atomic(larb->base + SMI_LARB_SLP_CON,
> + tmp, !!(tmp & SLP_PROT_RDY),
> 10, 1000);
> + if (ret)
> + dev_warn(dev, "sleep ctrl is not ready(0x%x).\n", tmp);
> + } else {
> + writel_relaxed(0, larb->base + SMI_LARB_SLP_CON);
> + }
> + return ret;
> +}
> +
> static int mtk_smi_device_link_common(struct device *dev, struct device
> **com_dev)
> {
> struct platform_device *smi_com_pdev;
> @@ -477,24 +501,31 @@ static int __maybe_unused mtk_smi_larb_resume(struct
> device *dev)
> {
> struct mtk_smi_larb *larb = dev_get_drvdata(dev);
> const struct mtk_smi_larb_gen *larb_gen = larb->larb_gen;
> - int ret;
> + int ret = 0;
This line does not have a sense.
>
> ret = clk_bulk_prepare_enable(larb->smi.clk_num, larb->smi.clks);
> - if (ret < 0)
> + if (ret)
Why changing this?
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] memory: mtk-smi: Fix a null dereference for the ostd
On Mon, 8 Nov 2021 16:24:29 +0800, Yong Wu wrote: > We add the ostd setting for mt8195. It introduces a KE for the > previous SoC which doesn't have ostd setting. This is the log: > > Unable to handle kernel NULL pointer dereference at virtual address > 0080 > ... > pc : mtk_smi_larb_config_port_gen2_general+0x64/0x130 > lr : mtk_smi_larb_resume+0x54/0x98 > ... > Call trace: > mtk_smi_larb_config_port_gen2_general+0x64/0x130 > pm_generic_runtime_resume+0x2c/0x48 > __genpd_runtime_resume+0x30/0xa8 > genpd_runtime_resume+0x94/0x2c8 > __rpm_callback+0x44/0x150 > rpm_callback+0x6c/0x78 > rpm_resume+0x310/0x558 > __pm_runtime_resume+0x3c/0x88 > > [...] Applied, thanks! [1/1] memory: mtk-smi: Fix a null dereference for the ostd commit: 8c5ba21c16bd7f8e23b8740dead6eaf164b8caa0 Best regards, -- Krzysztof Kozlowski ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] memory: mtk-smi: Fix a null dereference for the ostd
On 01/11/2021 07:09, Yong Wu wrote: > On Fri, 2021-10-29 at 19:35 +0200, Krzysztof Kozlowski wrote: >> On 28/10/2021 07:50, Yong Wu wrote: >>> We add the ostd setting for mt8195. It introduces a abort for the >>> previous SoC which doesn't have ostd setting. This is the log: >>> >>> Unable to handle kernel NULL pointer dereference at virtual address >>> 0080 >>> ... >>> pc : mtk_smi_larb_config_port_gen2_general+0x64/0x130 >>> lr : mtk_smi_larb_resume+0x54/0x98 >>> ... >>> Call trace: >>> mtk_smi_larb_config_port_gen2_general+0x64/0x130 >>> pm_generic_runtime_resume+0x2c/0x48 >>> __genpd_runtime_resume+0x30/0xa8 >>> genpd_runtime_resume+0x94/0x2c8 >>> __rpm_callback+0x44/0x150 >>> rpm_callback+0x6c/0x78 >>> rpm_resume+0x310/0x558 >>> __pm_runtime_resume+0x3c/0x88 >>> >>> In the code: larbostd = larb->larb_gen->ostd[larb->larbid], >>> if "larb->larb_gen->ostd" is null, the "larbostd" is the offset, it >>> is >>> also a valid value, thus, use the larb->larb_gen->ostd as the >>> condition >>> inside the "for" loop. >> >> You need to write more clearly, what you are fixing here. >> >>> >>> Signed-off-by: Yong Wu >>> --- >>> Hi Krzysztof, >>> Could you help review and conside this as a fix for the mt8195 >>> patchset? >>> The mt8195 patchset are not in mainline, thus, I don't know its >>> sha-id, >>> and don't add Fixes tag. >>> Thanks >>> --- >>> drivers/memory/mtk-smi.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c >>> index b883dcc0bbfa..0262a59a2d6e 100644 >>> --- a/drivers/memory/mtk-smi.c >>> +++ b/drivers/memory/mtk-smi.c >>> @@ -257,7 +257,7 @@ static void >>> mtk_smi_larb_config_port_gen2_general(struct device *dev) >>> if (MTK_SMI_CAPS(flags_general, MTK_SMI_FLAG_SW_FLAG)) >>> writel_relaxed(SMI_LARB_SW_FLAG_1, larb->base + >>> SMI_LARB_SW_FLAG); >>> >>> - for (i = 0; i < SMI_LARB_PORT_NR_MAX && larbostd && >>> !!larbostd[i]; i++) >>> + for (i = 0; i < SMI_LARB_PORT_NR_MAX && larb->larb_gen->ostd && >>> !!larbostd[i]; i++) >>> writel_relaxed(larbostd[i], larb->base + >>> SMI_LARB_OSTDL_PORTx(i)); >> >> The code does not look good. You have already a dereference at line >> 244: >> >> const u8 *larbostd = larb->larb_gen->ostd[larb->larbid]; > > if larb->larb_gen->ostd is null, larbostd is the offset, e.g. 0x80 in > the log above. thus, we can not use "larbostd[i]" in the "for" loop. > > sorry for the unreadable. In this case, is the change ok? No, it's ok, I did not check the type of ostd and it's confusing a bit that it is defined as a pointer to an array but you actually use it as array of pointers to 32-elemenet arrays... Anyway I was mistaken and there will be indeed no dereference at the assignment, but for code clarity I would still prefer to do the check earlier, so: > > or like this: > > -const u8 *larbostd = larb->larb_gen->ostd[larb->larbid]; > +const u8 *larbostd = larb->larb_gen->ostd ? larb->larb_gen-ostd[larb- >> larbid] : NULL; Although I think now the proper type should be explicit. mtk_smi_larb_mt8195_ostd is an 28-element array of SMI_LARB_PORT_NR_MAX-element u8 arrays, therefore struct mtk_smi_larb_gen should be: const u8 (*ostd)[][SMI_LARB_PORT_NR_MAX]; Right? Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] memory: mtk-smi: Fix a null dereference for the ostd
On 28/10/2021 07:50, Yong Wu wrote:
> We add the ostd setting for mt8195. It introduces a abort for the
> previous SoC which doesn't have ostd setting. This is the log:
>
> Unable to handle kernel NULL pointer dereference at virtual address
> 0080
> ...
> pc : mtk_smi_larb_config_port_gen2_general+0x64/0x130
> lr : mtk_smi_larb_resume+0x54/0x98
> ...
> Call trace:
> mtk_smi_larb_config_port_gen2_general+0x64/0x130
> pm_generic_runtime_resume+0x2c/0x48
> __genpd_runtime_resume+0x30/0xa8
> genpd_runtime_resume+0x94/0x2c8
> __rpm_callback+0x44/0x150
> rpm_callback+0x6c/0x78
> rpm_resume+0x310/0x558
> __pm_runtime_resume+0x3c/0x88
>
> In the code: larbostd = larb->larb_gen->ostd[larb->larbid],
> if "larb->larb_gen->ostd" is null, the "larbostd" is the offset, it is
> also a valid value, thus, use the larb->larb_gen->ostd as the condition
> inside the "for" loop.
You need to write more clearly, what you are fixing here.
>
> Signed-off-by: Yong Wu
> ---
> Hi Krzysztof,
> Could you help review and conside this as a fix for the mt8195 patchset?
> The mt8195 patchset are not in mainline, thus, I don't know its sha-id,
> and don't add Fixes tag.
> Thanks
> ---
> drivers/memory/mtk-smi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> index b883dcc0bbfa..0262a59a2d6e 100644
> --- a/drivers/memory/mtk-smi.c
> +++ b/drivers/memory/mtk-smi.c
> @@ -257,7 +257,7 @@ static void mtk_smi_larb_config_port_gen2_general(struct
> device *dev)
> if (MTK_SMI_CAPS(flags_general, MTK_SMI_FLAG_SW_FLAG))
> writel_relaxed(SMI_LARB_SW_FLAG_1, larb->base +
> SMI_LARB_SW_FLAG);
>
> - for (i = 0; i < SMI_LARB_PORT_NR_MAX && larbostd && !!larbostd[i]; i++)
> + for (i = 0; i < SMI_LARB_PORT_NR_MAX && larb->larb_gen->ostd &&
> !!larbostd[i]; i++)
> writel_relaxed(larbostd[i], larb->base +
> SMI_LARB_OSTDL_PORTx(i));
The code does not look good. You have already a dereference at line 244:
const u8 *larbostd = larb->larb_gen->ostd[larb->larbid];
You are not fixing the NULL pointer dereference.
>
> for_each_set_bit(i, (unsigned long *)larb->mmu, 32) {
>
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 03/13] memory: mtk-smi: Use clk_bulk clock ops
On 15/10/2021 15:38, AngeloGioacchino Del Regno wrote:
>> Use clk_bulk interface instead of the orginal one to simplify the code.
>>
>> For SMI larbs: Require apb/smi clocks while gals is optional.
>> For SMI common: Require apb/smi/gals0/gal1 in has_gals case. Otherwise,
>> also only require apb/smi, No optional clk here.
>>
>> About the "has_gals" flag, for smi larbs, the gals clock also may be
>> optional even this platform support it. thus it always use
>> *_bulk_get_optional, then the flag has_gals is unnecessary. Remove it.
>> The smi_common's has_gals still keep it.
>>
>> Also remove clk fail logs since bulk interface already output fail log.
>>
>> Signed-off-by: Yong Wu
>
> Hello Yong,
> thanks for the patch! However, I have an improvement to point out:
>
>> ---
>> drivers/memory/mtk-smi.c | 143 +++
>> 1 file changed, 55 insertions(+), 88 deletions(-)
>>
>> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
>> index c5fb51f73b34..f91eaf5c3ab0 100644
>> --- a/drivers/memory/mtk-smi.c
>> +++ b/drivers/memory/mtk-smi.c
>> @@ -60,6 +60,20 @@ enum mtk_smi_gen {
>> MTK_SMI_GEN2
>> };
>>
>> +#define MTK_SMI_CLK_NR_MAX 4
>
> This refers to mtk_smi_common_clks[] and should be probably moved after that.
> In any case, I don't think that there's any need to manually define this as 4,
> as you can simply use the macro ARRAY_SIZE(mtk_smi_common_clks).
> Using that will make you able to not update this definition everytime an
> update
> occurs to the mtk_smi_common_clks array.
>
>> +
>> +/* larbs: Require apb/smi clocks while gals is optional. */
>> +static const char * const mtk_smi_larb_clks[] = {"apb", "smi", "gals"};
>> +#define MTK_SMI_LARB_REQ_CLK_NR 2
>> +#define MTK_SMI_LARB_OPT_CLK_NR 1
>> +
>> +/*
>> + * common: Require these four clocks in has_gals case. Otherwise, only
>> apb/smi are required.
>> + */
>> +static const char * const mtk_smi_common_clks[] = {"apb", "smi", "gals0",
>> "gals1"};
>> +#define MTK_SMI_COM_REQ_CLK_NR 2
>> +#define MTK_SMI_COM_GALS_REQ_CLK_NR MTK_SMI_CLK_NR_MAX
>> +
>
> Apart from that,
> Acked-By: AngeloGioacchino Del Regno
The patchset was merged around a month ago:
https://lore.kernel.org/lkml/163229303729.7874.4095337797772755570.b4...@canonical.com/
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 00/13] MT8195 SMI support
On Tue, 14 Sep 2021 19:36:50 +0800, Yong Wu wrote: > This patchset mainly adds SMI support for mt8195. > > Comparing with the previous version, add two new functions: > a) add smi sub common > b) add initial setting for smi-common and smi-larb. > > Change note: > v4:1) base on v5.15-rc1 >2) In the dt-binding: > a. add "else mediatek,smi: false." in the yaml. > b. Remove mediatek,smi_sub_common. since we have only 2 level currently, > It should be smi-sub-common if that node has "mediatek,smi". otherwise, > it is smi-common. > > [...] Applied, thanks! [01/13] dt-bindings: memory: mediatek: Add mt8195 smi binding commit: b01065eee432b3ae91a2c0aaab66c2cae2e9812d [02/13] dt-bindings: memory: mediatek: Add mt8195 smi sub common commit: 599e681a31a2dfa7359b8e420a1157ed015f840b [03/13] memory: mtk-smi: Use clk_bulk clock ops commit: 0e14917c57f9d8b9b7d4f41813849a29659447b3 [04/13] memory: mtk-smi: Rename smi_gen to smi_type commit: a5c18986f404206fdbc27f208620dc13bffb5657 [05/13] memory: mtk-smi: Adjust some code position commit: 534e0ad2ed4f9296a8c7ccb1a393bc4d5000dbad [06/13] memory: mtk-smi: Add error handle for smi_probe commit: 30b869e77a1c626190bbbe6b4e5f5382b0102ac3 [07/13] memory: mtk-smi: Add device link for smi-sub-common commit: 47404757702ec8aa5c8310cdf58a267081f0ce6c [08/13] memory: mtk-smi: Add clocks for smi-sub-common commit: 3e4f74e0ea5a6a6d6d825fd7afd8a10afbd1152c [09/13] memory: mtk-smi: Use devm_platform_ioremap_resource commit: 912fea8bf8d854aef967c82a279ffd20be0326d7 [10/13] memory: mtk-smi: mt8195: Add smi support commit: cc4f9dcd9c1543394d6ee0d635551a2bd96bcacb [11/13] memory: mtk-smi: mt8195: Add initial setting for smi-common commit: 431e9cab7097b5d5eb3cf2b04d4b12d272df85e0 [12/13] memory: mtk-smi: mt8195: Add initial setting for smi-larb commit: fe6dd2a4017d3dfbf4a530d95270a1d498a16a4c [13/13] MAINTAINERS: Add entry for MediaTek SMI commit: 93403ede5aa4edeec2c63541b185d9c4fc9ae1e4 Best regards, -- Krzysztof Kozlowski ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 00/13] MT8195 SMI support
On 10/08/2021 10:08, Yong Wu wrote: > This patchset mainly adds SMI support for mt8195. > > Comparing with the previous version, add two new functions: > a) add smi sub common > b) add initial setting for smi-common and smi-larb. > > Change note: > v3:1) In the dt-binding: >a. Change mediatek,smi type from phandle-array to phandle from Rob. >b. Add a new bool property (mediatek,smi_sub_common) > to indicate if this is smi-sub-common. >2) Change the clock using bulk parting. > keep the smi-common's has_gals flag. more strict. >3) More comment about larb initial setting. >4) Add a maintain patch The patchset looks good to me but I saw now comments from Rob, so I expect a resend. Therefore there is also time for additional review - maybe continued by Ikjoon Jang? If you sent a v4 with fixes rather soon and get ack from Rob, I could still try to get it into next merge window. After this weekend I won't be taking patches for this cycle and it will wait for the merge window to finish. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] dt-bindings: arm-smmu: Fix json-schema syntax
On Tue, 13 Jul 2021 at 10:27, Krzysztof Kozlowski
wrote:
>
> On Mon, 12 Jul 2021 at 16:14, Rob Herring wrote:
> >
> > On Tue, Jun 22, 2021 at 11:56 PM Krzysztof Kozlowski
> > wrote:
> > >
> > > On Mon, 21 Jun 2021 16:00:36 +0200, Thierry Reding wrote:
> > > > Commit 4287861dca9d ("dt-bindings: arm-smmu: Add Tegra186 compatible
> > > > string") introduced a jsonschema syntax error as a result of a rebase
> > > > gone wrong. Fix it.
> > >
> > > Applied, thanks!
> > >
> > > [1/1] dt-bindings: arm-smmu: Fix json-schema syntax
> > > commit: bf3ec9deaa33889630722c47f7bb86ba58872ea7
> >
> > Applied where? Now Linus's master is broken.
>
> To memory controller drivers tree. Pushed to soc folks some time ago:
> https://lore.kernel.org/lkml/20210625073604.13562-1-krzysztof.kozlow...@canonical.com/
Hi Rob,
The patch landed in the Linus' tree in v5.14-rc2.
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] dt-bindings: arm-smmu: Fix json-schema syntax
On Mon, 12 Jul 2021 at 16:14, Rob Herring wrote:
>
> On Tue, Jun 22, 2021 at 11:56 PM Krzysztof Kozlowski
> wrote:
> >
> > On Mon, 21 Jun 2021 16:00:36 +0200, Thierry Reding wrote:
> > > Commit 4287861dca9d ("dt-bindings: arm-smmu: Add Tegra186 compatible
> > > string") introduced a jsonschema syntax error as a result of a rebase
> > > gone wrong. Fix it.
> >
> > Applied, thanks!
> >
> > [1/1] dt-bindings: arm-smmu: Fix json-schema syntax
> > commit: bf3ec9deaa33889630722c47f7bb86ba58872ea7
>
> Applied where? Now Linus's master is broken.
To memory controller drivers tree. Pushed to soc folks some time ago:
https://lore.kernel.org/lkml/20210625073604.13562-1-krzysztof.kozlow...@canonical.com/
Cc: Arnd and Olof,
Any comments on merging these fixes? They should go to current RC.
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/9] memory: mtk-smi: Use clk_bulk instead of the clk ops
On 11/07/2021 10:29, Yong Wu wrote:
> On Thu, 2021-07-08 at 11:32 +0200, Krzysztof Kozlowski wrote:
>> On 16/06/2021 13:43, Yong Wu wrote:
>>> smi have many clocks: apb/smi/gals.
>>> This patch use clk_bulk interface instead of the orginal one to simply
>>> the code.
>>>
>>> gals is optional clk(some larbs may don't have gals). use clk_bulk_optional
>>> instead. and then remove the has_gals flag.
>>>
>>> Also remove clk fail logs since bulk interface already output fail log.
>>>
>>> Signed-off-by: Yong Wu
>>> ---
>>> drivers/memory/mtk-smi.c | 124 +++
>>> 1 file changed, 34 insertions(+), 90 deletions(-)
>>>
>>> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
>>> index c5fb51f73b34..bcd2bf130655 100644
>>> --- a/drivers/memory/mtk-smi.c
>>> +++ b/drivers/memory/mtk-smi.c
>>> @@ -60,9 +60,18 @@ enum mtk_smi_gen {
>>> MTK_SMI_GEN2
>>> };
>>>
>>> +#define MTK_SMI_CLK_NR_MAX 4
>>> +
>>> +static const char * const mtk_smi_common_clocks[] = {
>>> + "apb", "smi", "gals0", "gals1", /* glas is optional */
>>
>> Typo here - glas.
>
> Will Fix. Thanks.
>
>>
>>> +};
>>> +
>
> [snip]
>
>>> @@ -493,7 +449,7 @@ static int mtk_smi_common_probe(struct platform_device
>>> *pdev)
>>> struct device *dev = &pdev->dev;
>>> struct mtk_smi *common;
>>> struct resource *res;
>>> - int ret;
>>> + int i, ret;
>>>
>>> common = devm_kzalloc(dev, sizeof(*common), GFP_KERNEL);
>>> if (!common)
>>> @@ -501,23 +457,13 @@ static int mtk_smi_common_probe(struct
>>> platform_device *pdev)
>>> common->dev = dev;
>>> common->plat = of_device_get_match_data(dev);
>>>
>>> - common->clk_apb = devm_clk_get(dev, "apb");
>>> - if (IS_ERR(common->clk_apb))
>>> - return PTR_ERR(common->clk_apb);
>>> -
>>> - common->clk_smi = devm_clk_get(dev, "smi");
>>> - if (IS_ERR(common->clk_smi))
>>> - return PTR_ERR(common->clk_smi);
>>> + common->clk_num = ARRAY_SIZE(mtk_smi_common_clocks);
>>> + for (i = 0; i < common->clk_num; i++)
>>> + common->clks[i].id = mtk_smi_common_clocks[i];
>>>
>>> - if (common->plat->has_gals) {
>>> - common->clk_gals0 = devm_clk_get(dev, "gals0");
>>> - if (IS_ERR(common->clk_gals0))
>>> - return PTR_ERR(common->clk_gals0);
>>> -
>>> - common->clk_gals1 = devm_clk_get(dev, "gals1");
>>> - if (IS_ERR(common->clk_gals1))
>>> - return PTR_ERR(common->clk_gals1);
>>> - }
>>> + ret = devm_clk_bulk_get_optional(dev, common->clk_num, common->clks);
>>> + if (ret)
>>> + return ret;
>>
>> How do you handle now missing required clocks?
>
> It looks this is a common issue for this function which supports all the
> clocks could be optional. Is there common suggestion for this?
>
> For our case, the apb/smi clocks are required while "gals" are optional.
>
> thus, we should use devm_clk_bulk_get for the necessary clocks and
> devm_clk_bulk_get_optional for the optional ones. right?
Yes, I think that's the solution. Otherwise you might not have proper
clocks leading to accesses to disabled/gated hardware.
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 6/9] memory: mtk-smi: Add smi sub common support
On 16/06/2021 13:43, Yong Wu wrote: > This patch adds smi-sub-common support. some larbs may connect with the > smi-sub-common, then connect with smi-common. Please start sentences with capital letter. This (similarly to "This patch") appears in multiple patches. > > Before we create device link between smi-larb with smi-common, If we have > sub-common, we should use device link the smi-larb and smi-sub-common, > then use device link between the smi-sub-common with smi-common. This is > for enabling clock/power automatically. > Move the device link code to a new interface for reusing. > > there is no SW extra setting for smi-sub-common. > > Signed-off-by: Yong Wu > --- > drivers/memory/mtk-smi.c | 78 ++-- > 1 file changed, 52 insertions(+), 26 deletions(-) > Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 4/9] memory: mtk-smi: Rename smi_gen to smi_type
On 16/06/2021 13:43, Yong Wu wrote: > This is a preparing patch for adding smi sub common. Don't write "This patch". Use simple imperative: "Prepare for adding smi sub common." https://elixir.bootlin.com/linux/v5.13/source/Documentation/process/submitting-patches.rst#L89 > About the previou smi_gen, we have gen1/gen2 that stand for the generation > number for HW. I plan to add a new type(sub_common), then the "gen" is not > prober. this patch only change it to "type", No functional change. Same. > > Signed-off-by: Yong Wu > --- > drivers/memory/mtk-smi.c | 24 > 1 file changed, 12 insertions(+), 12 deletions(-) > Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/9] memory: mtk-smi: Use clk_bulk instead of the clk ops
On 16/06/2021 13:43, Yong Wu wrote:
> smi have many clocks: apb/smi/gals.
> This patch use clk_bulk interface instead of the orginal one to simply
> the code.
>
> gals is optional clk(some larbs may don't have gals). use clk_bulk_optional
> instead. and then remove the has_gals flag.
>
> Also remove clk fail logs since bulk interface already output fail log.
>
> Signed-off-by: Yong Wu
> ---
> drivers/memory/mtk-smi.c | 124 +++
> 1 file changed, 34 insertions(+), 90 deletions(-)
>
> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> index c5fb51f73b34..bcd2bf130655 100644
> --- a/drivers/memory/mtk-smi.c
> +++ b/drivers/memory/mtk-smi.c
> @@ -60,9 +60,18 @@ enum mtk_smi_gen {
> MTK_SMI_GEN2
> };
>
> +#define MTK_SMI_CLK_NR_MAX 4
> +
> +static const char * const mtk_smi_common_clocks[] = {
> + "apb", "smi", "gals0", "gals1", /* glas is optional */
Typo here - glas.
> +};
> +
> +static const char * const mtk_smi_larb_clocks[] = {
> + "apb", "smi", "gals"
> +};
> +
> struct mtk_smi_common_plat {
> enum mtk_smi_gen gen;
> - bool has_gals;
> u32 bus_sel; /* Balance some larbs to enter mmu0 or mmu1 */
> };
>
> @@ -70,13 +79,12 @@ struct mtk_smi_larb_gen {
> int port_in_larb[MTK_LARB_NR_MAX + 1];
> void (*config_port)(struct device *dev);
> unsigned intlarb_direct_to_common_mask;
> - boolhas_gals;
> };
>
> struct mtk_smi {
> struct device *dev;
> - struct clk *clk_apb, *clk_smi;
> - struct clk *clk_gals0, *clk_gals1;
> + unsigned intclk_num;
> + struct clk_bulk_dataclks[MTK_SMI_CLK_NR_MAX];
> struct clk *clk_async; /*only needed by mt2701*/
> union {
> void __iomem*smi_ao_base; /* only for gen1 */
> @@ -95,45 +103,6 @@ struct mtk_smi_larb { /* larb: local arbiter */
> unsigned char *bank;
> };
>
> -static int mtk_smi_clk_enable(const struct mtk_smi *smi)
> -{
> - int ret;
> -
> - ret = clk_prepare_enable(smi->clk_apb);
> - if (ret)
> - return ret;
> -
> - ret = clk_prepare_enable(smi->clk_smi);
> - if (ret)
> - goto err_disable_apb;
> -
> - ret = clk_prepare_enable(smi->clk_gals0);
> - if (ret)
> - goto err_disable_smi;
> -
> - ret = clk_prepare_enable(smi->clk_gals1);
> - if (ret)
> - goto err_disable_gals0;
> -
> - return 0;
> -
> -err_disable_gals0:
> - clk_disable_unprepare(smi->clk_gals0);
> -err_disable_smi:
> - clk_disable_unprepare(smi->clk_smi);
> -err_disable_apb:
> - clk_disable_unprepare(smi->clk_apb);
> - return ret;
> -}
> -
> -static void mtk_smi_clk_disable(const struct mtk_smi *smi)
> -{
> - clk_disable_unprepare(smi->clk_gals1);
> - clk_disable_unprepare(smi->clk_gals0);
> - clk_disable_unprepare(smi->clk_smi);
> - clk_disable_unprepare(smi->clk_apb);
> -}
> -
> int mtk_smi_larb_get(struct device *larbdev)
> {
> int ret = pm_runtime_resume_and_get(larbdev);
> @@ -270,7 +239,6 @@ static const struct mtk_smi_larb_gen mtk_smi_larb_mt6779
> = {
> };
>
> static const struct mtk_smi_larb_gen mtk_smi_larb_mt8183 = {
> - .has_gals = true,
> .config_port= mtk_smi_larb_config_port_gen2_general,
> .larb_direct_to_common_mask = BIT(2) | BIT(3) | BIT(7),
> /* IPU0 | IPU1 | CCU */
> @@ -320,6 +288,7 @@ static int mtk_smi_larb_probe(struct platform_device
> *pdev)
> struct device_node *smi_node;
> struct platform_device *smi_pdev;
> struct device_link *link;
> + int i, ret;
>
> larb = devm_kzalloc(dev, sizeof(*larb), GFP_KERNEL);
> if (!larb)
> @@ -331,22 +300,14 @@ static int mtk_smi_larb_probe(struct platform_device
> *pdev)
> if (IS_ERR(larb->base))
> return PTR_ERR(larb->base);
>
> - larb->smi.clk_apb = devm_clk_get(dev, "apb");
> - if (IS_ERR(larb->smi.clk_apb))
> - return PTR_ERR(larb->smi.clk_apb);
> -
> - larb->smi.clk_smi = devm_clk_get(dev, "smi");
> - if (IS_ERR(larb->smi.clk_smi))
> - return PTR_ERR(larb->smi.clk_smi);
> -
> - if (larb->larb_gen->has_gals) {
> - /* The larbs may still haven't gals even if the SoC support.*/
> - larb->smi.clk_gals0 = devm_clk_get(dev, "gals");
> - if (PTR_ERR(larb->smi.clk_gals0) == -ENOENT)
> - larb->smi.clk_gals0 = NULL;
> - else if (IS_ERR(larb->smi.clk_gals0))
> - return PTR_ERR(larb->smi.clk_gals0);
> - }
> + larb->smi.clk_num = ARRAY_SIZE(mtk_smi_larb_clocks);
> + for (i = 0; i < larb->smi.clk_num; i++)
> + larb->smi.clks[i].i
Re: [PATCH 1/9] dt-bindings: memory: mediatek: Add mt8195 smi binding
On 16/06/2021 13:43, Yong Wu wrote: > This patch adds mt8195 smi supporting in the bindings. > > In mt8195, there are two smi-common HW, one is for vdo(video output), > the other is for vpp(video processing pipe). They connects with different > smi-larbs, then some setting(bus_sel) is different. Differentiate them > with the compatible string. > > Something like this: > > IOMMU(VDO) IOMMU(VPP) >| | > SMI_COMMON_VDO SMI_COMMON_VPP > --- > | | ... | | ... > larb0 larb2 ...larb1 larb3... > > Signed-off-by: Yong Wu > --- > .../bindings/memory-controllers/mediatek,smi-common.yaml| 6 +- > .../bindings/memory-controllers/mediatek,smi-larb.yaml | 3 +++ > 2 files changed, 8 insertions(+), 1 deletion(-) > I cannot find it on devicetree list, it seems you did not Cc it. Please use scripts/get_maintainer.pl. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 02/24] dt-bindings: mediatek: mt8195: Add binding for infra IOMMU
On 30/06/2021 04:34, Yong Wu wrote: > In mt8195, we have a new IOMMU that is for INFRA IOMMU. its masters > mainly are PCIe and USB. Different with MM IOMMU, all these masters > connect with IOMMU directly, there is no mediatek,larbs property for > infra IOMMU. > > Another thing is about PCIe ports. currently the function > "of_iommu_configure_dev_id" only support the id number is 1, But our > PCIe have two ports, one is for reading and the other is for writing. > see more about the PCIe patch in this patchset. Thus, I only list > the reading id here and add the other id in our driver. > > Signed-off-by: Yong Wu > --- > .../bindings/iommu/mediatek,iommu.yaml | 14 +- > .../dt-bindings/memory/mt8195-memory-port.h| 18 ++ > include/dt-bindings/memory/mtk-memory-port.h | 2 ++ > 3 files changed, 33 insertions(+), 1 deletion(-) > Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 01/24] dt-bindings: mediatek: mt8195: Add binding for MM IOMMU
On 30/06/2021 04:34, Yong Wu wrote: > This patch adds descriptions for mt8195 IOMMU which also use ARM > Short-Descriptor translation table format. > > In mt8195, there are two smi-common HW and IOMMU, one is for vdo(video > output), the other is for vpp(video processing pipe). They connects > with different smi-larbs, then some setting(larbid_remap) is different. > Differentiate them with the compatible string. > > Something like this: > > IOMMU(VDO) IOMMU(VPP) >| | > SMI_COMMON_VDO SMI_COMMON_VPP > --- > | | ... | | ... > larb0 larb2 ...larb1 larb3... > > Another change is that we have a new IOMMU that is for infra master like > PCIe and USB. The infra master don't have the larb and ports, thus we > rename the port header file to mt8195-memory-port.h rather than > mt8195-larb-port.h. > > Also, the IOMMU is not only for MM, thus, we don't call it "m4u" which > means "MultiMedia Memory Management UNIT". thus, use the "iommu" as the > compatiable string. > > Signed-off-by: Yong Wu > --- > .../bindings/iommu/mediatek,iommu.yaml| 7 + > .../dt-bindings/memory/mt8195-memory-port.h | 390 ++ > 2 files changed, 397 insertions(+) > create mode 100644 include/dt-bindings/memory/mt8195-memory-port.h > I understand this will go through IOMMU tree. Do you know about any further patches for memory controllers which will need the header? Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] dt-bindings: arm-smmu: Fix json-schema syntax
On Mon, 21 Jun 2021 16:00:36 +0200, Thierry Reding wrote:
> Commit 4287861dca9d ("dt-bindings: arm-smmu: Add Tegra186 compatible
> string") introduced a jsonschema syntax error as a result of a rebase
> gone wrong. Fix it.
Applied, thanks!
[1/1] dt-bindings: arm-smmu: Fix json-schema syntax
commit: bf3ec9deaa33889630722c47f7bb86ba58872ea7
Best regards,
--
Krzysztof Kozlowski
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 2/9] dt-bindings: arm-smmu: Add Tegra186 compatible string
On 18/06/2021 21:47, Rob Herring wrote:
> On Thu, Jun 3, 2021 at 10:49 AM Thierry Reding
> wrote:
>>
>> From: Thierry Reding
>>
>> The ARM SMMU instantiations found on Tegra186 and later need inter-
>> operation with the memory controller in order to correctly program
>> stream ID overrides.
>>
>> Furthermore, on Tegra194 multiple instances of the SMMU can gang up
>> to achieve higher throughput. In order to do this, they have to be
>> programmed identically so that the memory controller can interleave
>> memory accesses between them.
>>
>> Add the Tegra186 compatible string to make sure the interoperation
>> with the memory controller can be enabled on that SoC generation.
>>
>> Signed-off-by: Thierry Reding
>> ---
>> Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 11 +--
>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
>> b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
>> index 9d27aa5111d4..1181b590db71 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
>> @@ -54,8 +54,14 @@ properties:
>>- const: arm,mmu-500
>>- description: NVIDIA SoCs that program two ARM MMU-500s identically
>> items:
>> + - description: NVIDIA SoCs that require memory controller interaction
>
> This is not valid jsonschema:
>
> /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml:
> properties:compatible:oneOf:4:items: 'anyOf' conditional failed, one
> must be fixed:
> None is not of type 'object', 'boolean'
> None is not of type 'array'
> from schema $id: http://json-schema.org/draft-07/schema#
> /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml:
> properties:compatible:oneOf:4:items: 'oneOf' conditional failed, one
> must be fixed:
> None is not of type 'object'
> None is not of type 'array'
> from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
> /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml:
> properties:compatible:oneOf:4:items: 'oneOf' conditional failed, one
> must be fixed:
> None is not of type 'object'
> None is not of type 'array'
> from schema $id: http://devicetree.org/meta-schemas/string-array.yaml#
> /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml:
> properties:compatible:oneOf:5:items: 'oneOf' conditional failed, one
> must be fixed:
> [{'enum': [{'const': 'nvidia,tegra194-smmu'}, {'const':
> 'nvidia,tegra186-smmu'}]}, {'const': 'nvidia,smmu-500'}] is not of
> type 'object'
> {'const': 'nvidia,tegra194-smmu'} is not of type 'string'
> {'const': 'nvidia,tegra186-smmu'} is not of type 'string'
> from schema $id: http://devicetree.org/meta-schemas/string-array.yaml#
>
>
> This was not reviewed nor tested since the DT list was not Cc'ed.
Ugh, I see now weird empty item on a list... and not only DT list was
skipped - Thierry did not Cc you either.
My bad, I did not check that patch thoroughly before applying.
Thierry, please Cc folks mentioned by get_maintainer.pl. Either sent a
fix or a revert, if fix needs more time.
Additionally, why the patch changes reg to "minItems: 1" for
nvidia,tegra194-smmu?
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 0/9] arm64: tegra: Prevent early SMMU faults
On Thu, 3 Jun 2021 18:46:23 +0200, Thierry Reding wrote: > this is a set of patches that is the result of earlier discussions > regarding early identity mappings that are needed to avoid SMMU faults > during early boot. > > The goal here is to avoid early identity mappings altogether and instead > postpone the need for the identity mappings to when devices are attached > to the SMMU. This works by making the SMMU driver coordinate with the > memory controller driver on when to start enforcing SMMU translations. > This makes Tegra behave in a more standard way and pushes the code to > deal with the Tegra-specific programming into the NVIDIA SMMU > implementation. > > [...] Applied, thanks! [1/9] memory: tegra: Implement SID override programming (no commit info) [2/9] dt-bindings: arm-smmu: Add Tegra186 compatible string commit: 4287861dca9d77490ee50de42aa3ada92da86c9d [3/9] - skipped [4/9] iommu/arm-smmu: tegra: Detect number of instances at runtime commit: 7ecbf253f8d64c08de28d16a66e3abbe873f6c9f [5/9] iommu/arm-smmu: tegra: Implement SID override programming commit: 8eb68595475ac5fcaaa3718a173283df48cb4ef1 [6/9] iommu/arm-smmu: Use Tegra implementation on Tegra186 commit: 2c1bc371268862a991a6498e1dddc8971b9076b8 Best regards, -- Krzysztof Kozlowski ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [GIT PULL] memory: Tegra memory controller for v5.14
On 10/06/2021 19:29, Will Deacon wrote: > On Thu, Jun 10, 2021 at 05:05:27PM +0200, Thierry Reding wrote: >> On Thu, Jun 10, 2021 at 04:23:56PM +0200, Krzysztof Kozlowski wrote: >>> On 10/06/2021 11:19, Thierry Reding wrote: >>>> On Tue, Jun 08, 2021 at 05:48:51PM +0100, Will Deacon wrote: >>>>> I can queue as much or as little of 2-6 as you like, but I would like to >>>>> avoid pulling in the memory controller queue into the arm smmu tree. But >>>>> yes, whichever of those I take, I can put them on a separate branch so >>>>> that you're not blocked for the later patches. >>>>> >>>>> You have a better handle on the dependencies, so please tell me what works >>>>> for you. I just want to make sure that at least patch 3 lands in my tree, >>>>> so we don't get late conflicts with other driver changes. >>>> >>>> Yes, if you could pick up patch 3 and send out a link with the stable >>>> branch, I think Krzysztof or I could pull in that branch and pick up the >>>> remaining patches. It'd be good if you could also ack the remaining SMMU >>>> patches so that ARM SoC knows that they've been sanctioned. >>>> >>>> Krzysztof: would you be okay with picking up patches 2 and 4-6 on top of >>>> your memory branch for v5.14? >>> >>> You mean the iommu patches? Yes, I can take them and later explain to >>> Arnd/Olof why they come through me. >> >> Okay, great. >> >> Will, can you provide that stable branch? Or would you prefer if I >> prepared it and sent you a pull request? We're kind of running out of >> time, since for ARM SoC the cut-off point for new material is usually >> -rc6 and that's coming up pretty fast. > > https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=for-thierry/arm-smmu Merged, thanks. I'll take ARM/SMMU patches from Thierry's patchset: 2, 4-6. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [GIT PULL] memory: Tegra memory controller for v5.14
On 10/06/2021 11:19, Thierry Reding wrote: > On Tue, Jun 08, 2021 at 05:48:51PM +0100, Will Deacon wrote: >> On Tue, Jun 08, 2021 at 04:38:48PM +0200, Thierry Reding wrote: >>> On Tue, Jun 08, 2021 at 01:01:29PM +0100, Will Deacon wrote: >>>> On Mon, Jun 07, 2021 at 10:49:10AM +0200, Krzysztof Kozlowski wrote: >>>>> This is the pull for you to base further SMMU aptches (prevent early SMMU >>>>> faults). >>>> >>>> This is a tonne of code for me to pull into the SMMU tree given that I only >>>> want one patch! >>>> >>>> Thierry, if I just stick: >>>> >>>> https://lore.kernel.org/r/20210603164632.1000458-4-thierry.red...@gmail.com >>>> >>>> on its own branch, can you stitch together whatever you need? >>> >>> I'm not sure I understand what you're proposing. For reference, here's >>> the set of patches that I sent out: >>> >>> 1. memory: tegra: Implement SID override programming >>> 2. dt-bindings: arm-smmu: Add Tegra186 compatible string >>> 3. iommu/arm-smmu: Implement ->probe_finalize() >>> 4. iommu/arm-smmu: tegra: Detect number of instances at runtime >>> 5. iommu/arm-smmu: tegra: Implement SID override programming >>> 6. iommu/arm-smmu: Use Tegra implementation on Tegra186 >>> 7. arm64: tegra: Use correct compatible string for Tegra186 SMMU >>> 8. arm64: tegra: Hook up memory controller to SMMU on Tegra186 >>> 9. arm64: tegra: Enable SMMU support on Tegra194 >>> >>> Krzysztof already picked up patch 1 and I was assuming that you'd pick >>> up 2-6 because they are to the ARM SMMU driver. However, if you're >>> primarily interested in just patch 3, which is more "core" ARM SMMU than >>> the rest, which are Tegra-specific, then I suppose what we could do is >>> for you to give an Acked-by on the rest (2, 4-6) and then Krzysztof or I >>> can pick them up and take them via ARM SoC, based on the stable branch >>> from your tree that only has patch 3. >> >> I think you previously said that patch 5 depends on patch 1, so I can't >> take 2-6 without also pulling in the memory controller queue. >> >>> Patch 6 touches arm-smmu-impl.c, though it's a two-line change that >>> touches only the Tegra-specific matching bit in arm_smmu_impl_init(), so >>> the likelihood of that conflicting with anything else is fairly small. >>> >>> Is that what you were proposing? >> >> I can queue as much or as little of 2-6 as you like, but I would like to >> avoid pulling in the memory controller queue into the arm smmu tree. But >> yes, whichever of those I take, I can put them on a separate branch so >> that you're not blocked for the later patches. >> >> You have a better handle on the dependencies, so please tell me what works >> for you. I just want to make sure that at least patch 3 lands in my tree, >> so we don't get late conflicts with other driver changes. > > Yes, if you could pick up patch 3 and send out a link with the stable > branch, I think Krzysztof or I could pull in that branch and pick up the > remaining patches. It'd be good if you could also ack the remaining SMMU > patches so that ARM SoC knows that they've been sanctioned. > > Krzysztof: would you be okay with picking up patches 2 and 4-6 on top of > your memory branch for v5.14? You mean the iommu patches? Yes, I can take them and later explain to Arnd/Olof why they come through me. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [GIT PULL] memory: Tegra memory controller for v5.14
On 07/06/2021 10:49, Krzysztof Kozlowski wrote: > Hi Olof and Arnd, > > Tegra memory controller driver changes with necessary dependency from Thierry > (which you will also get from him): > 1. Dmitry's power domain work on Tegra MC drivers, > 2. Necessary clock and regulator dependencies for Dmitry's work. > Hi Olof and Arnd, Just FYI, the stable tag from Thierry which I pulled here (and you will get from him as well) might cause COMPILE_TEST failures on specific configurations. Regular defconfigs and allyes/allmod should not be affected. I am giving heads up in case this lands in Linus later... There will be two fixes for this, sent already by Thierry: https://lore.kernel.org/lkml/20210609112806.3565057-1-thierry.red...@gmail.com/ 1. reset controller stubs: going via reset tree, 2. reserved memory section: probably going via my tree later. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[GIT PULL] memory: Tegra memory controller for v5.14
Hi Olof and Arnd, Tegra memory controller driver changes with necessary dependency from Thierry (which you will also get from him): 1. Dmitry's power domain work on Tegra MC drivers, 2. Necessary clock and regulator dependencies for Dmitry's work. Hi Thierry and Will, This is the pull for you to base further SMMU aptches (prevent early SMMU faults). Best regards, Krzysztof The following changes since commit 6efb943b8616ec53a5e444193dccf1af9ad627b5: Linux 5.13-rc1 (2021-05-09 14:17:44 -0700) are available in the Git repository at: https://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux-mem-ctrl.git tags/memory-controller-drv-tegra-5.14 for you to fetch changes up to 393d66fd2cacba3e6aa95d7bb38790bfb7b1cc3a: memory: tegra: Implement SID override programming (2021-06-03 21:50:43 +0200) Memory controller drivers for v5.14 - Tegra SoC 1. Enable compile testing of Tegra memory controller drivers. 2. Unify Tegra memory controller drivers. From Thierry Reding: "This set of patches converges the feature sets of the pre-Tegra186 and the post-Tegra186 memory controller drivers such that newer chips can take advantage of some features that were previously only implemented on earlier chips." 3. Implement SID override programming as part of work to prevent early SMMU faults. 4. Some simplifications, e.g. use devm-helpers. This pulls dedicated tag from Thierry Reding with necessary changes to Tegra memory controller drivers, as a pre-requisite to series applied here. The changes from Thierry's tree also include their own dependencies: regulator and clock driver changes. Dmitry Osipenko (18): clk: tegra30: Use 300MHz for video decoder by default clk: tegra: Fix refcounting of gate clocks clk: tegra: Ensure that PLLU configuration is applied properly clk: tegra: Halve SCLK rate on Tegra20 clk: tegra: Don't allow zero clock rate for PLLs clk: tegra: cclk: Handle thermal DIV2 CPU frequency throttling clk: tegra: Mark external clocks as not having reset control clk: tegra: Don't deassert reset on enabling clocks regulator: core: Add regulator_sync_voltage_rdev() soc/tegra: regulators: Bump voltages on system reboot soc/tegra: Add stub for soc_is_tegra() soc/tegra: Add devm_tegra_core_dev_init_opp_table() soc/tegra: fuse: Add stubs needed for compile-testing clk: tegra: Add stubs needed for compile-testing memory: tegra: Fix compilation warnings on 64bit platforms memory: tegra: Enable compile testing for all drivers memory: tegra20-emc: Use devm_tegra_core_dev_init_opp_table() memory: tegra30-emc: Use devm_tegra_core_dev_init_opp_table() Krzysztof Kozlowski (1): Merge tag 'tegra-for-5.14-memory' of https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux into for-v5.14/tegra-mc Thierry Reding (16): Merge branch 'for-5.14/regulator' into for-5.14/soc Merge branch 'for-5.14/clk' into for-5.14/memory Merge branch 'for-5.14/soc' into for-5.14/memory memory: tegra: Consolidate register fields memory: tegra: Unify struct tegra_mc across SoC generations memory: tegra: Introduce struct tegra_mc_ops memory: tegra: Push suspend/resume into SoC drivers memory: tegra: Make per-SoC setup more generic memory: tegra: Extract setup code into callback memory: tegra: Parameterize interrupt handler memory: tegra: Make IRQ support opitonal memory: tegra: Only initialize reset controller if available memory: tegra: Unify drivers memory: tegra: Add memory client IDs to tables memory: tegra: Split Tegra194 data into separate file memory: tegra: Implement SID override programming drivers/clk/tegra/clk-periph-gate.c | 80 +- drivers/clk/tegra/clk-periph.c | 11 + drivers/clk/tegra/clk-pll.c | 12 +- drivers/clk/tegra/clk-tegra-periph.c |6 +- drivers/clk/tegra/clk-tegra-super-cclk.c | 16 +- drivers/clk/tegra/clk-tegra20.c |6 +- drivers/clk/tegra/clk-tegra30.c |6 +- drivers/clk/tegra/clk.h |4 - drivers/iommu/tegra-smmu.c | 16 +- drivers/memory/tegra/Kconfig | 18 +- drivers/memory/tegra/Makefile|6 +- drivers/memory/tegra/mc.c| 321 +++--- drivers/memory/tegra/mc.h| 25 + drivers/memory/tegra/tegra114.c | 1245 -- drivers/memory/tegra/tegra124-emc.c |4 +- drivers/memory/tegra/tegra124.c | 1306 --- drivers/memory/tegra/tegra186.c | 1679 +- drivers/memory/tegra/tegra194.c | 1351 +++
Re: (subset) [PATCH v3 0/9] arm64: tegra: Prevent early SMMU faults
On Thu, 3 Jun 2021 18:46:23 +0200, Thierry Reding wrote: > this is a set of patches that is the result of earlier discussions > regarding early identity mappings that are needed to avoid SMMU faults > during early boot. > > The goal here is to avoid early identity mappings altogether and instead > postpone the need for the identity mappings to when devices are attached > to the SMMU. This works by making the SMMU driver coordinate with the > memory controller driver on when to start enforcing SMMU translations. > This makes Tegra behave in a more standard way and pushes the code to > deal with the Tegra-specific programming into the NVIDIA SMMU > implementation. > > [...] Applied, thanks! [1/9] memory: tegra: Implement SID override programming commit: 393d66fd2cacba3e6aa95d7bb38790bfb7b1cc3a Best regards, -- Krzysztof Kozlowski ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 00/10] arm64: tegra: Prevent early SMMU faults
On 02/06/2021 16:58, Thierry Reding wrote: > On Wed, Jun 02, 2021 at 12:40:49PM +0100, Will Deacon wrote: >> On Wed, Jun 02, 2021 at 12:44:58PM +0200, Krzysztof Kozlowski wrote: >>> On 02/06/2021 10:52, Thierry Reding wrote: >>>> On Wed, Jun 02, 2021 at 09:35:13AM +0200, Krzysztof Kozlowski wrote: >>>>> On 02/06/2021 09:33, Krzysztof Kozlowski wrote: >>>>>> On 01/06/2021 20:08, Thierry Reding wrote: >>>>>>> On Tue, Jun 01, 2021 at 01:26:46PM +0100, Will Deacon wrote: >>>>>>>> On Fri, May 28, 2021 at 07:05:28PM +0200, Thierry Reding wrote: >>>>>>>>> On Tue, Apr 20, 2021 at 07:26:09PM +0200, Thierry Reding wrote: >>>>>>>>>> From: Thierry Reding >>>>>>>>>> >>>>>>>> Probably best if I queue 3-6 on a separate branch once you send a v3, >>>>>>>> then Krzysztof can pull that in if he needs it. >>>>>>> >>>>>>> Patch 5 has a build-time dependency on patch 1, so they need to go in >>>>>>> together. The reason why I suggested Krzysztof pick these up is because >>>>>>> there is a restructuring series that this depends on, which will go into >>>>>>> Krzysztof's tree. So in order to pull in 3-6, you'd get a bunch of other >>>>>>> and mostly unrelated stuff as well. >>>>>> >>>>>> I missed that part... what other series are needed for this one? Except >>>>>> Dmitry's power management set I do not have anything in my sight for >>>>>> Tegras memory controllers. >>>>>> >>>>>> Anyway, I can take the memory bits and provide a stable tag with these. >>>>>> Recently there was quite a lot work around Tegra memory controllers, so >>>>>> this makes especially sense if new patches appear. >>>>> >>>>> OK, I think I have now the patchset you talked about - "memory: tegra: >>>>> Driver unification" v2, right? >>>> >>>> Yes, that's the one. That series is fairly self-contained, but Dmitry's >>>> power management set has dependencies that pull in the regulator, clock >>>> and ARM SoC trees. >>>> >>>> I did a test merge of the driver unification series with a branch that >>>> has Dmitry's patches and all the dependencies and there are no conflicts >>>> so that, fortunately, doesn't further complicates things. >>>> >>>> Do you want me to send you a pull request with Dmitry's memory >>>> controller changes? You could then apply the unification series on top, >>>> which should allow this SMMU series to apply cleanly on top of that. >>> >>> Makes sense and it looks quite bulletproof for future changes. Let's do >>> like this. I will apply your patch 1/10 from this v2 on top of it and >>> driver unification later. >> >> Okey doke. Thierry -- please let me know which patches I can queue. Having >> the SMMU driver changes in the IOMMU tree really helps in case of conflicts >> with other SMMU changes. As I say, I can put stuff on a separate branch for >> you if it helps. > > Given that the SMMU patches have a build-time dependency on the first > patch in the series, and the series depends on the unification series, I > think Krzysztof would have to pull the memory controller branch that I > have with Dmitry's work, apply the unification series on top and then > take patch 1 of this series on top of that. That's the state that you'd > have to pull into the SMMU tree to get the right dependencies. > > The SMMU pieces are the leaf of the dependency tree, so technically no > separate branch is needed, because I don't think anybody would have to > pull from it. However, a separate branch might make it easier to back > any of this work out if we have to. > > Krzysztof, I do plan on sending out a v3 of the unification series to > address the two cleanups that Dmitry and you have pointed out. After > that I can send out v3 of this series to fix the ordering issue that > Krishna had mentioned. After all of those are applied, would you be able > to provide a stable branch for Will's SMMU tree? Yes, I will provide a stable branch/tag. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 00/10] arm64: tegra: Prevent early SMMU faults
On 02/06/2021 16:53, Thierry Reding wrote: > On Wed, Jun 02, 2021 at 12:44:58PM +0200, Krzysztof Kozlowski wrote: >> On 02/06/2021 10:52, Thierry Reding wrote: >>> On Wed, Jun 02, 2021 at 09:35:13AM +0200, Krzysztof Kozlowski wrote: >>>> On 02/06/2021 09:33, Krzysztof Kozlowski wrote: >>>>> On 01/06/2021 20:08, Thierry Reding wrote: >>>>>> On Tue, Jun 01, 2021 at 01:26:46PM +0100, Will Deacon wrote: >>>>>>> On Fri, May 28, 2021 at 07:05:28PM +0200, Thierry Reding wrote: >>>>>>>> On Tue, Apr 20, 2021 at 07:26:09PM +0200, Thierry Reding wrote: >>>>>>>>> From: Thierry Reding >>>>>>>>> >>>>>>> Probably best if I queue 3-6 on a separate branch once you send a v3, >>>>>>> then Krzysztof can pull that in if he needs it. >>>>>> >>>>>> Patch 5 has a build-time dependency on patch 1, so they need to go in >>>>>> together. The reason why I suggested Krzysztof pick these up is because >>>>>> there is a restructuring series that this depends on, which will go into >>>>>> Krzysztof's tree. So in order to pull in 3-6, you'd get a bunch of other >>>>>> and mostly unrelated stuff as well. >>>>> >>>>> I missed that part... what other series are needed for this one? Except >>>>> Dmitry's power management set I do not have anything in my sight for >>>>> Tegras memory controllers. >>>>> >>>>> Anyway, I can take the memory bits and provide a stable tag with these. >>>>> Recently there was quite a lot work around Tegra memory controllers, so >>>>> this makes especially sense if new patches appear. >>>> >>>> OK, I think I have now the patchset you talked about - "memory: tegra: >>>> Driver unification" v2, right? >>> >>> Yes, that's the one. That series is fairly self-contained, but Dmitry's >>> power management set has dependencies that pull in the regulator, clock >>> and ARM SoC trees. >>> >>> I did a test merge of the driver unification series with a branch that >>> has Dmitry's patches and all the dependencies and there are no conflicts >>> so that, fortunately, doesn't further complicates things. >>> >>> Do you want me to send you a pull request with Dmitry's memory >>> controller changes? You could then apply the unification series on top, >>> which should allow this SMMU series to apply cleanly on top of that. >> >> Makes sense and it looks quite bulletproof for future changes. Let's do >> like this. I will apply your patch 1/10 from this v2 on top of it and >> driver unification later. > > The SMMU series here depends on the unification series, so the > unification series needs to go first. It'd be a fair bit of work to > reverse that because the ->probe_device() callback implemented by the > first patch of this SMMU series is part of the tegra_mc_ops structure > that's introduced in the unification series. Right, you already wrote it in the first email in this thread, I just reversed words in my head... Then as you wrote - take Dmitry's changes and share them via pull to me. I'll put on top the unification series, then #1 from this SMMU series and finally I'll provide a pull request for Will so his SMMU can go on. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 00/10] arm64: tegra: Prevent early SMMU faults
On 02/06/2021 10:52, Thierry Reding wrote: > On Wed, Jun 02, 2021 at 09:35:13AM +0200, Krzysztof Kozlowski wrote: >> On 02/06/2021 09:33, Krzysztof Kozlowski wrote: >>> On 01/06/2021 20:08, Thierry Reding wrote: >>>> On Tue, Jun 01, 2021 at 01:26:46PM +0100, Will Deacon wrote: >>>>> On Fri, May 28, 2021 at 07:05:28PM +0200, Thierry Reding wrote: >>>>>> On Tue, Apr 20, 2021 at 07:26:09PM +0200, Thierry Reding wrote: >>>>>>> From: Thierry Reding >>>>>>> >>>>> Probably best if I queue 3-6 on a separate branch once you send a v3, >>>>> then Krzysztof can pull that in if he needs it. >>>> >>>> Patch 5 has a build-time dependency on patch 1, so they need to go in >>>> together. The reason why I suggested Krzysztof pick these up is because >>>> there is a restructuring series that this depends on, which will go into >>>> Krzysztof's tree. So in order to pull in 3-6, you'd get a bunch of other >>>> and mostly unrelated stuff as well. >>> >>> I missed that part... what other series are needed for this one? Except >>> Dmitry's power management set I do not have anything in my sight for >>> Tegras memory controllers. >>> >>> Anyway, I can take the memory bits and provide a stable tag with these. >>> Recently there was quite a lot work around Tegra memory controllers, so >>> this makes especially sense if new patches appear. >> >> OK, I think I have now the patchset you talked about - "memory: tegra: >> Driver unification" v2, right? > > Yes, that's the one. That series is fairly self-contained, but Dmitry's > power management set has dependencies that pull in the regulator, clock > and ARM SoC trees. > > I did a test merge of the driver unification series with a branch that > has Dmitry's patches and all the dependencies and there are no conflicts > so that, fortunately, doesn't further complicates things. > > Do you want me to send you a pull request with Dmitry's memory > controller changes? You could then apply the unification series on top, > which should allow this SMMU series to apply cleanly on top of that. Makes sense and it looks quite bulletproof for future changes. Let's do like this. I will apply your patch 1/10 from this v2 on top of it and driver unification later. > I can also carry all these changes in the Tegra tree and send a PR in a > few days once this has seen a bit more testing in linux-next, which also > makes sure it's got a bit more testing in our internal test farm. > Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 00/10] arm64: tegra: Prevent early SMMU faults
On 02/06/2021 09:33, Krzysztof Kozlowski wrote: > On 01/06/2021 20:08, Thierry Reding wrote: >> On Tue, Jun 01, 2021 at 01:26:46PM +0100, Will Deacon wrote: >>> On Fri, May 28, 2021 at 07:05:28PM +0200, Thierry Reding wrote: >>>> On Tue, Apr 20, 2021 at 07:26:09PM +0200, Thierry Reding wrote: >>>>> From: Thierry Reding >>>>> >>> Probably best if I queue 3-6 on a separate branch once you send a v3, >>> then Krzysztof can pull that in if he needs it. >> >> Patch 5 has a build-time dependency on patch 1, so they need to go in >> together. The reason why I suggested Krzysztof pick these up is because >> there is a restructuring series that this depends on, which will go into >> Krzysztof's tree. So in order to pull in 3-6, you'd get a bunch of other >> and mostly unrelated stuff as well. > > I missed that part... what other series are needed for this one? Except > Dmitry's power management set I do not have anything in my sight for > Tegras memory controllers. > > Anyway, I can take the memory bits and provide a stable tag with these. > Recently there was quite a lot work around Tegra memory controllers, so > this makes especially sense if new patches appear. OK, I think I have now the patchset you talked about - "memory: tegra: Driver unification" v2, right? Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 00/10] arm64: tegra: Prevent early SMMU faults
On 01/06/2021 20:08, Thierry Reding wrote: > On Tue, Jun 01, 2021 at 01:26:46PM +0100, Will Deacon wrote: >> On Fri, May 28, 2021 at 07:05:28PM +0200, Thierry Reding wrote: >>> On Tue, Apr 20, 2021 at 07:26:09PM +0200, Thierry Reding wrote: From: Thierry Reding Hi, this is a set of patches that is the result of earlier discussions regarding early identity mappings that are needed to avoid SMMU faults during early boot. The goal here is to avoid early identity mappings altogether and instead postpone the need for the identity mappings to when devices are attached to the SMMU. This works by making the SMMU driver coordinate with the memory controller driver on when to start enforcing SMMU translations. This makes Tegra behave in a more standard way and pushes the code to deal with the Tegra-specific programming into the NVIDIA SMMU implementation. Compared to the original version of these patches, I've split the preparatory work into a separate patch series because it became very large and will be mostly uninteresting for this audience. Patch 1 provides a mechanism to program SID overrides at runtime. Patch 2 updates the ARM SMMU device tree bindings to include the Tegra186 compatible string as suggested by Robin during review. Patches 3 and 4 create the fundamentals in the SMMU driver to support this and also make this functionality available on Tegra186. Patch 5 hooks the ARM SMMU up to the memory controller so that the memory client stream ID overrides can be programmed at the right time. Patch 6 extends this mechanism to Tegra186 and patches 7-9 enable all of this through device tree updates. Patch 10 is included here to show how SMMU will be enabled for display controllers. However, it cannot be applied yet because the code to create identity mappings for potentially live framebuffers hasn't been merged yet. The end result is that various peripherals will have SMMU enabled, while the display controllers will keep using passthrough, as initially set up by firmware. Once the device tree bindings have been accepted and the SMMU driver has been updated to create identity mappings for the display controllers, they can be hooked up to the SMMU and the code in this series will automatically program the SID overrides to enable SMMU translations at the right time. Note that the series creates a compile time dependency between the memory controller and IOMMU trees. If it helps I can provide a branch for each tree, modelling the dependency, once the series has been reviewed. Changes in v2: - split off the preparatory work into a separate series (that needs to be applied first) - address review comments by Robin Thierry Thierry Reding (10): memory: tegra: Implement SID override programming dt-bindings: arm-smmu: Add Tegra186 compatible string iommu/arm-smmu: Implement ->probe_finalize() iommu/arm-smmu: tegra: Detect number of instances at runtime iommu/arm-smmu: tegra: Implement SID override programming iommu/arm-smmu: Use Tegra implementation on Tegra186 arm64: tegra: Use correct compatible string for Tegra186 SMMU arm64: tegra: Hook up memory controller to SMMU on Tegra186 arm64: tegra: Enable SMMU support on Tegra194 arm64: tegra: Enable SMMU support for display on Tegra194 .../devicetree/bindings/iommu/arm,smmu.yaml | 11 +- arch/arm64/boot/dts/nvidia/tegra186.dtsi | 4 +- arch/arm64/boot/dts/nvidia/tegra194.dtsi | 166 ++ drivers/iommu/arm/arm-smmu/arm-smmu-impl.c| 3 +- drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c | 90 -- drivers/iommu/arm/arm-smmu/arm-smmu.c | 13 ++ drivers/iommu/arm/arm-smmu/arm-smmu.h | 1 + drivers/memory/tegra/mc.c | 9 + drivers/memory/tegra/tegra186.c | 72 include/soc/tegra/mc.h| 3 + 10 files changed, 349 insertions(+), 23 deletions(-) >>> >>> Will, Robin, >>> >>> do you have any more comments on the ARM SMMU bits of this series? If >>> not, can you guys provide an Acked-by so that Krzysztof can pick this >>> (modulo the DT patches) up into the memory-controller tree for v5.14? >>> >>> I'll send out a v3 with the bisectibilitiy fix that Krishna pointed >>> out. >> >> Probably best if I queue 3-6 on a separate branch once you send a v3, >> then Krzysztof can pull that in if he needs it. > > Patch 5 has a build-time dependency on patch 1, so they need to go in > together. The reason why I suggested Krzysztof pick these up is because > there is a restructuring series that this depends on, which will go into > Krzysztof's tree. So in order to
Re: [PATCH v2 01/10] memory: tegra: Implement SID override programming
On 26/04/2021 14:13, Thierry Reding wrote:
> On Mon, Apr 26, 2021 at 10:28:43AM +0200, Krzysztof Kozlowski wrote:
(...)
>>> +
>>> + value = readl(mc->regs + client->regs.sid.override);
>>> + old = value & MC_SID_STREAMID_OVERRIDE_MASK;
>>> +
>>> + if (old != sid) {
>>> + dev_dbg(mc->dev, "overriding SID %x for %s with %x\n", old,
>>> + client->name, sid);
>>> + writel(sid, mc->regs + client->regs.sid.override);
>>> + }
>>> +}
>>> +
>>> +static int tegra186_mc_probe_device(struct tegra_mc *mc, struct device
>>> *dev)
>>> +{
>>> +#if IS_ENABLED(CONFIG_IOMMU_API)
>>
>> Is this part really build-time dependent? I don't see here any uses of
>> IOMMU specific fields, so maybe this should be runtime choice based on
>> enabled interconnect devices?
>
> Unfortunately it is. struct iommu_fwspec is an empty structure for
> !CONFIG_IOMMU_API, so the code below that tries to access fwspec->ids
> fails for !CONFIG_IOMMU_API configurations if we don't protect this with
> the preprocessor guard.
OK, thanks.
>
>>
>>> + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
>>> + struct of_phandle_args args;
>>> + unsigned int i, index = 0;
>>> +
>>> + while (!of_parse_phandle_with_args(dev->of_node, "interconnects",
>>> "#interconnect-cells",
>>> + index, &args)) {
>>> + if (args.np == mc->dev->of_node && args.args_count != 0) {
>>> + for (i = 0; i < mc->soc->num_clients; i++) {
>>> + const struct tegra_mc_client *client =
>>> &mc->soc->clients[i];
>>> +
>>> + if (client->id == args.args[0]) {
>>> + u32 sid = fwspec->ids[0] &
>>> MC_SID_STREAMID_OVERRIDE_MASK;
>>> +
>>> + tegra186_mc_client_sid_override(mc,
>>> client, sid);
>>> + }
>>> + }
>>> + }
>>> +
>>> + index++;
>>> + }
>>> +#endif
>>> +
>>> + return 0;
>>> +}
>>> +
>>> const struct tegra_mc_ops tegra186_mc_ops = {
>>> .probe = tegra186_mc_probe,
>>> .remove = tegra186_mc_remove,
>>> .resume = tegra186_mc_resume,
>>> + .probe_device = tegra186_mc_probe_device,
>>> };
>>>
>>> #if defined(CONFIG_ARCH_TEGRA_186_SOC)
>>> diff --git a/include/soc/tegra/mc.h b/include/soc/tegra/mc.h
>>> index 1387747d574b..bbad6330008b 100644
>>> --- a/include/soc/tegra/mc.h
>>> +++ b/include/soc/tegra/mc.h
>>> @@ -176,6 +176,7 @@ struct tegra_mc_ops {
>>> int (*suspend)(struct tegra_mc *mc);
>>> int (*resume)(struct tegra_mc *mc);
>>> irqreturn_t (*handle_irq)(int irq, void *data);
>>> + int (*probe_device)(struct tegra_mc *mc, struct device *dev);
>>> };
>>>
>>> struct tegra_mc_soc {
>>> @@ -240,4 +241,6 @@ devm_tegra_memory_controller_get(struct device *dev)
>>> }
>>> #endif
>>>
>>> +int tegra_mc_probe_device(struct tegra_mc *mc, struct device *dev);
>>> +
>>
>> What about !CONFIG_TEGRA_MC? I think arm-smmmu will fail.
>
> I think it doesn't fail because for !CONFIG_TEGRA_MC it basically throws
> away most of nvidia_smmu_impl_init() already because ERR_PTR(-ENODEV) is
> returned by devm_tegra_memory_controller_get() and so it unconditionally
> fails early on already.
>
> I say I /think/ that happens because I can't reproduce a build failure
> even if I manually tweak the .config such that ARM_SMMU is enabled and
> TEGRA_MC is disabled. But I can't say I fully understand why it's
> working, because, yes, the symbol definitely doesn't exist. But again,
> if the compiler is clever enough to figure out that that function can't
> be called anyway and doesn't even want it, why bother making it more
> complicated than it has to be?
Since you tested that case, it's fine.
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 01/10] memory: tegra: Implement SID override programming
On 20/04/2021 19:26, Thierry Reding wrote:
> From: Thierry Reding
>
> Instead of programming all SID overrides during early boot, perform the
> operation on-demand after the SMMU translations have been set up for a
> device. This reuses data from device tree to match memory clients for a
> device and programs the SID specified in device tree, which corresponds
> to the SID used for the SMMU context banks for the device.
>
> Signed-off-by: Thierry Reding
> ---
> drivers/memory/tegra/mc.c | 9 +
> drivers/memory/tegra/tegra186.c | 72 +
> include/soc/tegra/mc.h | 3 ++
> 3 files changed, 84 insertions(+)
>
> diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
> index c854639cf30c..bace5ecfe770 100644
> --- a/drivers/memory/tegra/mc.c
> +++ b/drivers/memory/tegra/mc.c
> @@ -97,6 +97,15 @@ struct tegra_mc *devm_tegra_memory_controller_get(struct
> device *dev)
> }
> EXPORT_SYMBOL_GPL(devm_tegra_memory_controller_get);
>
> +int tegra_mc_probe_device(struct tegra_mc *mc, struct device *dev)
> +{
> + if (mc->soc->ops && mc->soc->ops->probe_device)
> + return mc->soc->ops->probe_device(mc, dev);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(tegra_mc_probe_device);
> +
> static int tegra_mc_block_dma_common(struct tegra_mc *mc,
>const struct tegra_mc_reset *rst)
> {
> diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
> index 1f87915ccd62..e65eac5764d4 100644
> --- a/drivers/memory/tegra/tegra186.c
> +++ b/drivers/memory/tegra/tegra186.c
> @@ -4,6 +4,7 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> @@ -15,6 +16,10 @@
> #include
> #endif
>
> +#define MC_SID_STREAMID_OVERRIDE_MASK GENMASK(7, 0)
> +#define MC_SID_STREAMID_SECURITY_WRITE_ACCESS_DISABLED BIT(16)
> +#define MC_SID_STREAMID_SECURITY_OVERRIDE BIT(8)
> +
> static void tegra186_mc_program_sid(struct tegra_mc *mc)
> {
> unsigned int i;
> @@ -66,10 +71,77 @@ static int tegra186_mc_resume(struct tegra_mc *mc)
> return 0;
> }
>
> +static void tegra186_mc_client_sid_override(struct tegra_mc *mc,
> + const struct tegra_mc_client
> *client,
> + unsigned int sid)
> +{
> + u32 value, old;
> +
> + value = readl(mc->regs + client->regs.sid.security);
> + if ((value & MC_SID_STREAMID_SECURITY_OVERRIDE) == 0) {
> + /*
> + * If the secure firmware has locked this down the override
> + * for this memory client, there's nothing we can do here.
> + */
> + if (value & MC_SID_STREAMID_SECURITY_WRITE_ACCESS_DISABLED)
> + return;
> +
> + /*
> + * Otherwise, try to set the override itself. Typically the
> + * secure firmware will never have set this configuration.
> + * Instead, it will either have disabled write access to
> + * this field, or it will already have set an explicit
> + * override itself.
> + */
> + WARN_ON((value & MC_SID_STREAMID_SECURITY_OVERRIDE) == 0);
> +
> + value |= MC_SID_STREAMID_SECURITY_OVERRIDE;
> + writel(value, mc->regs + client->regs.sid.security);
> + }
> +
> + value = readl(mc->regs + client->regs.sid.override);
> + old = value & MC_SID_STREAMID_OVERRIDE_MASK;
> +
> + if (old != sid) {
> + dev_dbg(mc->dev, "overriding SID %x for %s with %x\n", old,
> + client->name, sid);
> + writel(sid, mc->regs + client->regs.sid.override);
> + }
> +}
> +
> +static int tegra186_mc_probe_device(struct tegra_mc *mc, struct device *dev)
> +{
> +#if IS_ENABLED(CONFIG_IOMMU_API)
Is this part really build-time dependent? I don't see here any uses of
IOMMU specific fields, so maybe this should be runtime choice based on
enabled interconnect devices?
> + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> + struct of_phandle_args args;
> + unsigned int i, index = 0;
> +
> + while (!of_parse_phandle_with_args(dev->of_node, "interconnects",
> "#interconnect-cells",
> +index, &args)) {
> + if (args.np == mc->dev->of_node && args.args_count != 0) {
> + for (i = 0; i < mc->soc->num_clients; i++) {
> + const struct tegra_mc_client *client =
> &mc->soc->clients[i];
> +
> + if (client->id == args.args[0]) {
> + u32 sid = fwspec->ids[0] &
> MC_SID_STREAMID_OVERRIDE_MASK;
> +
> + tegra186_mc_client_sid_override(mc,
> client, sid);
> + }
> + }
> + }
> +
> + index++;
> + }
> +#endif
> +
> + return 0;
> +}
Re: (subset) [PATCH v5 00/16] Clean up "mediatek,larb"
On Sat, 10 Apr 2021 17:11:12 +0800, Yong Wu wrote: > MediaTek IOMMU block diagram always like below: > > M4U > | > smi-common > | > - > | | ... > | | > larb1 larb2 > | | > vdec venc > > [...] Applied, thanks! [04/16] memory: mtk-smi: Add device-link between smi-larb and smi-common commit: 6ce2c05b21189eb17b3aa26720cc5841acf9dce8 Best regards, -- Krzysztof Kozlowski ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 04/16] memory: mtk-smi: Add device-link between smi-larb and smi-common
On 13/04/2021 08:04, Yong Wu wrote: > On Sat, 2021-04-10 at 14:40 +0200, Krzysztof Kozlowski wrote: >> On 10/04/2021 11:11, Yong Wu wrote: >>> Normally, If the smi-larb HW need work, we should enable the smi-common >>> HW power and clock firstly. >>> This patch adds device-link between the smi-larb dev and the smi-common >>> dev. then If pm_runtime_get_sync(smi-larb-dev), the pm_runtime_get_sync >>> (smi-common-dev) will be called automatically. >>> >>> Also, Add DL_FLAG_STATELESS to avoid the smi-common clocks be gated when >>> probe. >>> >>> CC: Matthias Brugger >>> Suggested-by: Tomasz Figa >>> Signed-off-by: Yong Wu >>> --- >>> drivers/memory/mtk-smi.c | 19 ++- >>> 1 file changed, 10 insertions(+), 9 deletions(-) >> >> I understood this is a dependency for other patches, so: >> Acked-by: Krzysztof Kozlowski >> >> If I am wrong and I can take it via memory tree, let me know. > > Hi Krzysztof, > > Thanks very much for your quickly review. > > I think it is ok if it go through memory tree. In the original patch, we > pm_runtime_get(smi-common-dev) in the smi-larb's pm resume callback. > This patch only use device-link do this. thus, this patch have no > function change. it only adjusts the SMI internal code flow. Hm, okay, I took it then for v5.13. > > In addition, [14/16] expects your Acked-by. and that one should be > merged with the others. Thanks for reminder. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 14/16] memory: mtk-smi: Get rid of mtk_smi_larb_get/put
On 10/04/2021 11:11, Yong Wu wrote: > After adding device_link between the iommu consumer and smi-larb, > the pm_runtime_get(_sync) of smi-larb and smi-common will be called > automatically. we can get rid of mtk_smi_larb_get/put. > > CC: Matthias Brugger > Signed-off-by: Yong Wu > Reviewed-by: Evan Green > --- > drivers/memory/mtk-smi.c | 14 -- > include/soc/mediatek/smi.h | 20 > 2 files changed, 34 deletions(-) > Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 04/16] memory: mtk-smi: Add device-link between smi-larb and smi-common
On 10/04/2021 11:11, Yong Wu wrote: > Normally, If the smi-larb HW need work, we should enable the smi-common > HW power and clock firstly. > This patch adds device-link between the smi-larb dev and the smi-common > dev. then If pm_runtime_get_sync(smi-larb-dev), the pm_runtime_get_sync > (smi-common-dev) will be called automatically. > > Also, Add DL_FLAG_STATELESS to avoid the smi-common clocks be gated when > probe. > > CC: Matthias Brugger > Suggested-by: Tomasz Figa > Signed-off-by: Yong Wu > --- > drivers/memory/mtk-smi.c | 19 ++- > 1 file changed, 10 insertions(+), 9 deletions(-) I understood this is a dependency for other patches, so: Acked-by: Krzysztof Kozlowski If I am wrong and I can take it via memory tree, let me know. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu: exynos: remove unneeded local variable initialization
The initialization of 'fault_addr' local variable is not needed as it is shortly after overwritten. Addresses-Coverity: Unused value Signed-off-by: Krzysztof Kozlowski --- drivers/iommu/exynos-iommu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index de324b4eedfe..8fa9a591fb96 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -407,7 +407,7 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id) struct sysmmu_drvdata *data = dev_id; const struct sysmmu_fault_info *finfo; unsigned int i, n, itype; - sysmmu_iova_t fault_addr = -1; + sysmmu_iova_t fault_addr; unsigned short reg_status, reg_clear; int ret = -ENOSYS; -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] memory: mtk-smi: Support SMI modular
On Tue, Jan 26, 2021 at 02:00:55PM +0800, Yong Wu wrote: > This patch mainly support SMI modular. Switch MTK_SMI to tristate, > and add module_exit/module_license. > > Signed-off-by: Yong Wu > --- > This patch rebase on the clean v5.11-rc1. > and this one: memory: mtk-smi: Use platform_register_drivers > https://lore.kernel.org/linux-arm-kernel/20210121062429.26504-2-yong...@mediatek.com/ > > change note: > a) squash the last two of v1 into one patch. > b) Remove module_alias > --- > drivers/memory/Kconfig | 2 +- > drivers/memory/mtk-smi.c | 9 + > include/soc/mediatek/smi.h | 2 +- > 3 files changed, 11 insertions(+), 2 deletions(-) Thanks, applied with slightly adjusted commit msg and subject. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/3] memory: mtk-smi: Add module_exit and module_license
On Mon, Jan 25, 2021 at 05:28:05PM +0800, Yong Wu wrote:
> On Mon, 2021-01-25 at 09:40 +0100, Krzysztof Kozlowski wrote:
> > On Mon, Jan 25, 2021 at 02:49:41PM +0800, Yong Wu wrote:
> > > On Fri, 2021-01-22 at 22:34 +0100, Krzysztof Kozlowski wrote:
> > > > On Thu, Jan 21, 2021 at 02:24:28PM +0800, Yong Wu wrote:
> > > > > The config MTK_SMI always depends on MTK_IOMMU which is built-in
> > > > > currently. Thus we don't have module_exit before. This patch adds
> > > > > module_exit and module_license. It is a preparing patch for supporting
> > > > > MTK_SMI could been built as a module.
> > > > >
> > > > > Signed-off-by: Yong Wu
> > > > > ---
> > > > > drivers/memory/mtk-smi.c | 10 ++
> > > > > 1 file changed, 10 insertions(+)
> > > > >
> > > > > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> > > > > index e2aebd2bfa8e..aa2a25abf04f 100644
> > > > > --- a/drivers/memory/mtk-smi.c
> > > > > +++ b/drivers/memory/mtk-smi.c
> > > > > @@ -597,3 +597,13 @@ static int __init mtk_smi_init(void)
> > > > > return platform_register_drivers(smidrivers,
> > > > > ARRAY_SIZE(smidrivers));
> > > > > }
> > > > > module_init(mtk_smi_init);
> > > > > +
> > > > > +static void __exit mtk_smi_exit(void)
> > > > > +{
> > > > > + platform_unregister_drivers(smidrivers, ARRAY_SIZE(smidrivers));
> > > > > +}
> > > > > +module_exit(mtk_smi_exit);
> > > > > +
> > > > > +MODULE_DESCRIPTION("MediaTek SMI driver");
> > > > > +MODULE_ALIAS("platform:MediaTek-SMI");
> > > >
> > > > Drivers do not use capital letters, so I have doubts whether this alias
> > > > is correct.
> > >
> > > I didn't care the upper/lower-case. I will change to lower case in next
> > > time.
> >
> > Then why do you need the alias? The name does not match driver name, so
> > what's the purpose of this alias/
>
> I think it is not so necessary for us. I will delete this line in next
> version.
>
> Only curious what's alias is fit in our case? normally it should be the
> file name: mtk-smi?
If autoloading of your module works, then remove it. The alias is
necessary for some cases when a device table is missing (e.g. platform
driver is matched via devicetree but not having the platform_device_id
table) or matching is done via different method (e.g. driver is matched
from MFD via devicetree compatible even though there is a
platform_device_id table).
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/3] memory: mtk-smi: Switch MTK_SMI to tristate
On Mon, Jan 25, 2021 at 02:49:44PM +0800, Yong Wu wrote: > On Fri, 2021-01-22 at 22:35 +0100, Krzysztof Kozlowski wrote: > > On Thu, Jan 21, 2021 at 02:24:29PM +0800, Yong Wu wrote: > > > This patch switches MTK_SMI to tristate. Support it could be 'm'. > > > > > > Meanwhile, Fix a build issue while MTK_SMI is built as module. > > > > s/Fix/fix. > > > > What error is being fixed here? How can I reproduce it? Aren't you just > > adjusting it to being buildable by module? > > Sorry, I didn't copy the fail log here. This is the build log: > > In file included from .../drivers/iommu/mtk_iommu.c:34:0: > .../drivers/iommu/mtk_iommu.h:84:28: error: array type has incomplete > element type 'struct mtk_smi_larb_iommu' > struct mtk_smi_larb_iommu larb_imu[MTK_LARB_NR_MAX]; > > Our iommu driver will use this structure. but it was contained by > "#ifdef CONFIG_MTK_SMI". thus I change it to "#if > IS_ENABLED(CONFIG_MTK_SMI)" > > If reproducing it, we should change mtk-iommu to module_init[1]. and > switch kconfig MTK_IOMMU to tristate, then change the CONFIG_MTK_IOMMU > to m. we could get the fail log. > > In this case, Should I squash this change into this patch? I though this > is a preparing patch and the fail is caused by MTK_SMI. thus I squash > that into this patch. or change it as a independent patch and send when > I change MTK_IOMMU to tristate? If I understand correctly, there is no error before this patch. In such case just don't mention the error to fix, because it is simply part of making things modular. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/3] memory: mtk-smi: Add module_exit and module_license
On Mon, Jan 25, 2021 at 02:49:41PM +0800, Yong Wu wrote:
> On Fri, 2021-01-22 at 22:34 +0100, Krzysztof Kozlowski wrote:
> > On Thu, Jan 21, 2021 at 02:24:28PM +0800, Yong Wu wrote:
> > > The config MTK_SMI always depends on MTK_IOMMU which is built-in
> > > currently. Thus we don't have module_exit before. This patch adds
> > > module_exit and module_license. It is a preparing patch for supporting
> > > MTK_SMI could been built as a module.
> > >
> > > Signed-off-by: Yong Wu
> > > ---
> > > drivers/memory/mtk-smi.c | 10 ++
> > > 1 file changed, 10 insertions(+)
> > >
> > > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> > > index e2aebd2bfa8e..aa2a25abf04f 100644
> > > --- a/drivers/memory/mtk-smi.c
> > > +++ b/drivers/memory/mtk-smi.c
> > > @@ -597,3 +597,13 @@ static int __init mtk_smi_init(void)
> > > return platform_register_drivers(smidrivers, ARRAY_SIZE(smidrivers));
> > > }
> > > module_init(mtk_smi_init);
> > > +
> > > +static void __exit mtk_smi_exit(void)
> > > +{
> > > + platform_unregister_drivers(smidrivers, ARRAY_SIZE(smidrivers));
> > > +}
> > > +module_exit(mtk_smi_exit);
> > > +
> > > +MODULE_DESCRIPTION("MediaTek SMI driver");
> > > +MODULE_ALIAS("platform:MediaTek-SMI");
> >
> > Drivers do not use capital letters, so I have doubts whether this alias
> > is correct.
>
> I didn't care the upper/lower-case. I will change to lower case in next
> time.
Then why do you need the alias? The name does not match driver name, so
what's the purpose of this alias/
>
> MODULE_ALIAS("platform:MediaTek-smi")
>
> >
> > Adding all these should be squashed with changing Kconfig into tristate.
> > It does not have sense on its own.
>
> Thanks very much for review.
>
> Only confirm: Squash whole this patch or only squash the MODULE_x into
> the next patch?
This entire patch 2/3 should be with 3/3.
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/3] memory: mtk-smi: Switch MTK_SMI to tristate
On Thu, Jan 21, 2021 at 02:24:29PM +0800, Yong Wu wrote: > This patch switches MTK_SMI to tristate. Support it could be 'm'. > > Meanwhile, Fix a build issue while MTK_SMI is built as module. s/Fix/fix. What error is being fixed here? How can I reproduce it? Aren't you just adjusting it to being buildable by module? Best regards, Krzysztof > > Signed-off-by: Yong Wu > --- > This patch has a little conflict with the mt8192 iommu patch which > delete the MTK_LARB_NR_MAX in smi.h(It's still reviewing). > This patch rebase on the clean v5.11-rc1. > --- > drivers/memory/Kconfig | 2 +- > include/soc/mediatek/smi.h | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig > index 3ea6913df176..d5f0f4680880 100644 > --- a/drivers/memory/Kconfig > +++ b/drivers/memory/Kconfig > @@ -173,7 +173,7 @@ config JZ4780_NEMC > memory devices such as NAND and SRAM. > > config MTK_SMI > - bool "Mediatek SoC Memory Controller driver" if COMPILE_TEST > + tristate "Mediatek SoC Memory Controller driver" if COMPILE_TEST > depends on ARCH_MEDIATEK || COMPILE_TEST > help > This driver is for the Memory Controller module in MediaTek SoCs, > diff --git a/include/soc/mediatek/smi.h b/include/soc/mediatek/smi.h > index 5a34b87d89e3..29e2fb8f33d6 100644 > --- a/include/soc/mediatek/smi.h > +++ b/include/soc/mediatek/smi.h > @@ -9,7 +9,7 @@ > #include > #include > > -#ifdef CONFIG_MTK_SMI > +#if IS_ENABLED(CONFIG_MTK_SMI) ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/3] memory: mtk-smi: Add module_exit and module_license
On Thu, Jan 21, 2021 at 02:24:28PM +0800, Yong Wu wrote:
> The config MTK_SMI always depends on MTK_IOMMU which is built-in
> currently. Thus we don't have module_exit before. This patch adds
> module_exit and module_license. It is a preparing patch for supporting
> MTK_SMI could been built as a module.
>
> Signed-off-by: Yong Wu
> ---
> drivers/memory/mtk-smi.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> index e2aebd2bfa8e..aa2a25abf04f 100644
> --- a/drivers/memory/mtk-smi.c
> +++ b/drivers/memory/mtk-smi.c
> @@ -597,3 +597,13 @@ static int __init mtk_smi_init(void)
> return platform_register_drivers(smidrivers, ARRAY_SIZE(smidrivers));
> }
> module_init(mtk_smi_init);
> +
> +static void __exit mtk_smi_exit(void)
> +{
> + platform_unregister_drivers(smidrivers, ARRAY_SIZE(smidrivers));
> +}
> +module_exit(mtk_smi_exit);
> +
> +MODULE_DESCRIPTION("MediaTek SMI driver");
> +MODULE_ALIAS("platform:MediaTek-SMI");
Drivers do not use capital letters, so I have doubts whether this alias
is correct.
Adding all these should be squashed with changing Kconfig into tristate.
It does not have sense on its own.
Best regards,
Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/3] memory: mtk-smi: Use platform_register_drivers
On Thu, Jan 21, 2021 at 02:24:27PM +0800, Yong Wu wrote: > In this file, we have 2 drivers, smi-common and smi-larb. > Use platform_register_drivers. > > Signed-off-by: Yong Wu > --- > drivers/memory/mtk-smi.c | 25 ++--- > 1 file changed, 6 insertions(+), 19 deletions(-) Thanks, applied. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 06/27] dt-bindings: mediatek: Add binding for mt8192 IOMMU
On Wed, Dec 09, 2020 at 04:00:41PM +0800, Yong Wu wrote: > This patch adds decriptions for mt8192 IOMMU and SMI. > > mt8192 also is MTK IOMMU gen2 which uses ARM Short-Descriptor translation > table format. The M4U-SMI HW diagram is as below: > > EMI >| > M4U >| > >SMI Common > >| > +---+--+--+--+---+ > | | | | .. | | > | | | | | | > larb0 larb1 larb2 larb4 .. larb19 larb20 > disp0 disp1 mdpvdec IPE IPE > > All the connections are HW fixed, SW can NOT adjust it. > > mt8192 M4U support 0~16GB iova range. we preassign different engines > into different iova ranges: > > domain-id module iova-range larbs >0 disp0 ~ 4G larb0/1 >1 vcodec 4G ~ 8G larb4/5/7 >2 cam/mdp 8G ~ 12G larb2/9/11/13/14/16/17/18/19/20 >3 CCU00x4000_ ~ 0x43ff_ larb13: port 9/10 >4 CCU10x4400_ ~ 0x47ff_ larb14: port 4/5 > > The iova range for CCU0/1(camera control unit) is HW requirement. > > Signed-off-by: Yong Wu > Reviewed-by: Rob Herring > --- > .../bindings/iommu/mediatek,iommu.yaml| 18 +- > include/dt-bindings/memory/mt8192-larb-port.h | 240 ++ > 2 files changed, 257 insertions(+), 1 deletion(-) > create mode 100644 include/dt-bindings/memory/mt8192-larb-port.h > Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 05/27] dt-bindings: memory: mediatek: Rename header guard for SMI header file
On Wed, Dec 09, 2020 at 04:00:40PM +0800, Yong Wu wrote: > Only rename the header guard for all the SoC larb port header file. > No funtional change. > > Suggested-by: Krzysztof Kozlowski > Signed-off-by: Yong Wu > --- Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 18/24] iommu/mediatek: Support master use iova over 32bit
On Wed, Nov 11, 2020 at 08:38:32PM +0800, Yong Wu wrote: > After extending v7s, our pagetable already support iova reach > 16GB(34bit). the master got the iova via dma_alloc_attrs may reach > 34bits, but its HW register still is 32bit. then how to set the > bit32/bit33 iova? this depend on a SMI larb setting(bank_sel). > > we separate whole 16GB iova to four banks: > bank: 0: 0~4G; 1: 4~8G; 2: 8-12G; 3: 12-16G; > The bank number is (iova >> 32). > > We will preassign which bank the larbs belong to. currently we don't > have a interface for master to adjust its bank number. > > Each a bank is a iova_region which is a independent iommu-domain. > the iova range for each iommu-domain can't cross 4G. > > Signed-off-by: Yong Wu > Acked-by: Krzysztof Kozlowski # memory part > --- > drivers/iommu/mtk_iommu.c | 12 +--- > drivers/memory/mtk-smi.c | 7 +++ > include/soc/mediatek/smi.h | 1 + > 3 files changed, 17 insertions(+), 3 deletions(-) > Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 06/24] iommu/mediatek: Use the common mtk-smi-larb-port.h
On Wed, Nov 11, 2020 at 08:38:20PM +0800, Yong Wu wrote: > Use the common larb-port header in the source code. > > Signed-off-by: Yong Wu > --- > drivers/iommu/mtk_iommu.c | 7 --- > drivers/iommu/mtk_iommu.h | 1 + > drivers/memory/mtk-smi.c | 1 + > include/soc/mediatek/smi.h | 2 -- > 4 files changed, 2 insertions(+), 9 deletions(-) > Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 05/24] dt-bindings: mediatek: Add binding for mt8192 IOMMU
On Wed, Nov 11, 2020 at 08:38:19PM +0800, Yong Wu wrote: > This patch adds decriptions for mt8192 IOMMU and SMI. > > mt8192 also is MTK IOMMU gen2 which uses ARM Short-Descriptor translation > table format. The M4U-SMI HW diagram is as below: > > EMI >| > M4U >| > >SMI Common > >| > +---+--+--+--+---+ > | | | | .. | | > | | | | | | > larb0 larb1 larb2 larb4 .. larb19 larb20 > disp0 disp1 mdpvdec IPE IPE > > All the connections are HW fixed, SW can NOT adjust it. > > mt8192 M4U support 0~16GB iova range. we preassign different engines > into different iova ranges: > > domain-id module iova-range larbs >0 disp0 ~ 4G larb0/1 >1 vcodec 4G ~ 8G larb4/5/7 >2 cam/mdp 8G ~ 12G larb2/9/11/13/14/16/17/18/19/20 >3 CCU00x4000_ ~ 0x43ff_ larb13: port 9/10 >4 CCU10x4400_ ~ 0x47ff_ larb14: port 4/5 > > The iova range for CCU0/1(camera control unit) is HW requirement. > > Signed-off-by: Yong Wu > Reviewed-by: Rob Herring > --- > .../bindings/iommu/mediatek,iommu.yaml| 18 +- > include/dt-bindings/memory/mt8192-larb-port.h | 240 ++ > 2 files changed, 257 insertions(+), 1 deletion(-) > create mode 100644 include/dt-bindings/memory/mt8192-larb-port.h > > diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > index ba6626347381..0f26fe14c8e2 100644 > --- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > +++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > @@ -76,6 +76,7 @@ properties: >- mediatek,mt8167-m4u # generation two >- mediatek,mt8173-m4u # generation two >- mediatek,mt8183-m4u # generation two > + - mediatek,mt8192-m4u # generation two > >- description: mt7623 generation one > items: > @@ -115,7 +116,11 @@ properties: >dt-binding/memory/mt6779-larb-port.h for mt6779, >dt-binding/memory/mt8167-larb-port.h for mt8167, >dt-binding/memory/mt8173-larb-port.h for mt8173, > - dt-binding/memory/mt8183-larb-port.h for mt8183. > + dt-binding/memory/mt8183-larb-port.h for mt8183, > + dt-binding/memory/mt8192-larb-port.h for mt8192. > + > + power-domains: > +maxItems: 1 > > required: >- compatible > @@ -133,11 +138,22 @@ allOf: >- mediatek,mt2701-m4u >- mediatek,mt2712-m4u >- mediatek,mt8173-m4u > + - mediatek,mt8192-m4u > > then: >required: > - clocks > > + - if: > + properties: > +compatible: > + enum: > +- mediatek,mt8192-m4u > + > +then: > + required: > +- power-domains > + > additionalProperties: false > > examples: > diff --git a/include/dt-bindings/memory/mt8192-larb-port.h > b/include/dt-bindings/memory/mt8192-larb-port.h > new file mode 100644 > index ..7437fa62654e > --- /dev/null > +++ b/include/dt-bindings/memory/mt8192-larb-port.h > @@ -0,0 +1,240 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright (c) 2020 MediaTek Inc. > + * > + * Author: Chao Hao > + * Author: Yong Wu > + */ > +#ifndef _DTS_IOMMU_PORT_MT8192_H_ > +#define _DTS_IOMMU_PORT_MT8192_H_ Not accurate header guard. Shoud be: _DT_BINDINGS_MEMORY_MT8192_LARB_PORT_H_ Probably you copied it from some other Mediatek headers - all of them have header guard pointing to different directory. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 04/24] dt-bindings: memory: mediatek: Add domain definition
On Wed, Nov 11, 2020 at 08:38:18PM +0800, Yong Wu wrote: > In the latest SoC, there are several HW IP require a sepecial iova > range, mainly CCU and VPU has this requirement. Take CCU as a example, > CCU require its iova locate in the range(0x4000_ ~ 0x43ff_). > > In this patch we add a domain definition for the special port. In the > example of CCU, If we preassign CCU port in domain1, then iommu driver > will prepare a independent iommu domain of the special iova range for it, > then the iova got from dma_alloc_attrs(ccu-dev) will locate in its special > range. > > This is a preparing patch for multi-domain support. > > Signed-off-by: Yong Wu > --- > include/dt-bindings/memory/mtk-smi-larb-port.h | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) > Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 03/24] dt-bindings: memory: mediatek: Extend LARB_NR_MAX to 32
On Wed, Nov 11, 2020 at 08:38:17PM +0800, Yong Wu wrote: > Extend the max larb number definition as mt8192 has larb_nr over 16. > > Signed-off-by: Yong Wu > Acked-by: Rob Herring > --- > Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml | 2 +- > include/dt-bindings/memory/mtk-smi-larb-port.h | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) > Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 02/24] dt-bindings: memory: mediatek: Add a common larb-port header file
On Wed, Nov 11, 2020 at 08:38:16PM +0800, Yong Wu wrote: > Put all the macros about smi larb/port togethers, this is a preparing > patch for extending LARB_NR and adding new dom-id support. > > Signed-off-by: Yong Wu > Acked-by: Rob Herring > --- > include/dt-bindings/memory/mt2712-larb-port.h | 2 +- > include/dt-bindings/memory/mt6779-larb-port.h | 2 +- > include/dt-bindings/memory/mt8167-larb-port.h | 2 +- > include/dt-bindings/memory/mt8173-larb-port.h | 2 +- > include/dt-bindings/memory/mt8183-larb-port.h | 2 +- > include/dt-bindings/memory/mtk-smi-larb-port.h | 15 +++ > 6 files changed, 20 insertions(+), 5 deletions(-) > create mode 100644 include/dt-bindings/memory/mtk-smi-larb-port.h > > diff --git a/include/dt-bindings/memory/mt2712-larb-port.h > b/include/dt-bindings/memory/mt2712-larb-port.h Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 3/3] memory: mtk-smi: Add mt8192 support
On Tue, Nov 03, 2020 at 01:42:00PM +0800, Yong Wu wrote: > Add mt8192 smi support. > > Signed-off-by: Yong Wu > --- > drivers/memory/mtk-smi.c | 19 +++ Thanks, applied. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 1/3] dt-bindings: memory: mediatek: Convert SMI to DT schema
On Tue, Nov 03, 2020 at 01:41:58PM +0800, Yong Wu wrote: > Convert MediaTek SMI to DT schema. > > Signed-off-by: Yong Wu > --- > .../mediatek,smi-common.txt | 50 --- > .../mediatek,smi-common.yaml | 140 ++ > .../memory-controllers/mediatek,smi-larb.txt | 50 --- > .../memory-controllers/mediatek,smi-larb.yaml | 130 > 4 files changed, 270 insertions(+), 100 deletions(-) > delete mode 100644 > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt > create mode 100644 > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml > delete mode 100644 > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt > create mode 100644 > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml Thanks, applied with Rob's tag. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 2/3] dt-bindings: memory: mediatek: Add mt8192 support
On Tue, Nov 03, 2020 at 01:41:59PM +0800, Yong Wu wrote: > Add mt8192 smi support in the bindings. > > Signed-off-by: Yong Wu > Reviewed-by: Rob Herring > --- > .../bindings/memory-controllers/mediatek,smi-common.yaml | 4 +++- > .../bindings/memory-controllers/mediatek,smi-larb.yaml| 2 ++ Thanks, applied. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/3] dt-bindings: memory: mediatek: Convert SMI to DT
On Mon, 2 Nov 2020 at 06:31, Yong Wu wrote: > > On Sat, 2020-10-31 at 12:36 +0100, Krzysztof Kozlowski wrote: > > On Fri, Oct 30, 2020 at 05:12:52PM +0800, Yong Wu wrote: > > > Convert MediaTek SMI to DT schema. > > > > > > CC: Fabien Parent > > > CC: Ming-Fan Chen > > > CC: Matthias Brugger > > > Signed-off-by: Yong Wu > > > --- > > > .../mediatek,smi-common.txt | 50 --- > > > .../mediatek,smi-common.yaml | 140 ++ > > > .../memory-controllers/mediatek,smi-larb.txt | 50 --- > > > .../memory-controllers/mediatek,smi-larb.yaml | 129 > > > 4 files changed, 269 insertions(+), 100 deletions(-) > > > delete mode 100644 > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt > > > > +Cc Honghui Zhang, > > As comment [1], Honghui's address is not valid now. I will act for him. > > > > > Your Ack is needed as you contributed descriptions to the bindings and > > work is being relicensed to GPL-2.0-only OR BSD-2-Clause. > > "GPL-2.0-only OR BSD-2-Clause" is required when we run check-patch. > > If I still use "GPL-2.0-only", then the contributors' Ack/SoB is not > needed, right? That would be one solution but I was thinking to proceed with only your agreement. You were the main contributor to these files. Honghui added a few descriptions. Other developers added only compatibles. Since we cannot reach Honghui, I would assume that your agreement (Sign-off) is enough. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/3] dt-bindings: memory: mediatek: Convert SMI to DT schema
On Fri, Oct 30, 2020 at 05:12:52PM +0800, Yong Wu wrote:
> Convert MediaTek SMI to DT schema.
>
> CC: Fabien Parent
> CC: Ming-Fan Chen
> CC: Matthias Brugger
> Signed-off-by: Yong Wu
> ---
> .../mediatek,smi-common.txt | 50 ---
> .../mediatek,smi-common.yaml | 140 ++
> .../memory-controllers/mediatek,smi-larb.txt | 50 ---
> .../memory-controllers/mediatek,smi-larb.yaml | 129
> 4 files changed, 269 insertions(+), 100 deletions(-)
> delete mode 100644
> Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
+Cc Honghui Zhang,
Your Ack is needed as you contributed descriptions to the bindings and
work is being relicensed to GPL-2.0-only OR BSD-2-Clause.
Best regards,
Krzysztof
> create mode 100644
> Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> delete mode 100644
> Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
> create mode 100644
> Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
>
> diff --git
> a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
>
> b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
> deleted file mode 100644
> index dbafffe3f41e..
> ---
> a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
> +++ /dev/null
> @@ -1,50 +0,0 @@
> -SMI (Smart Multimedia Interface) Common
> -
> -The hardware block diagram please check bindings/iommu/mediatek,iommu.txt
> -
> -Mediatek SMI have two generations of HW architecture, here is the list
> -which generation the SoCs use:
> -generation 1: mt2701 and mt7623.
> -generation 2: mt2712, mt6779, mt8167, mt8173 and mt8183.
> -
> -There's slight differences between the two SMI, for generation 2, the
> -register which control the iommu port is at each larb's register base. But
> -for generation 1, the register is at smi ao base(smi always on register
> -base). Besides that, the smi async clock should be prepared and enabled for
> -SMI generation 1 to transform the smi clock into emi clock domain, but that
> is
> -not needed for SMI generation 2.
> -
> -Required properties:
> -- compatible : must be one of :
> - "mediatek,mt2701-smi-common"
> - "mediatek,mt2712-smi-common"
> - "mediatek,mt6779-smi-common"
> - "mediatek,mt7623-smi-common", "mediatek,mt2701-smi-common"
> - "mediatek,mt8167-smi-common"
> - "mediatek,mt8173-smi-common"
> - "mediatek,mt8183-smi-common"
> -- reg : the register and size of the SMI block.
> -- power-domains : a phandle to the power domain of this local arbiter.
> -- clocks : Must contain an entry for each entry in clock-names.
> -- clock-names : must contain 3 entries for generation 1 smi HW and 2 entries
> - for generation 2 smi HW as follows:
> - - "apb" : Advanced Peripheral Bus clock, It's the clock for setting
> - the register.
> - - "smi" : It's the clock for transfer data and command.
> - They may be the same if both source clocks are the same.
> - - "async" : asynchronous clock, it help transform the smi clock into the
> emi
> - clock domain, this clock is only needed by generation 1 smi HW.
> - and these 2 option clocks for generation 2 smi HW:
> - - "gals0": the path0 clock of GALS(Global Async Local Sync).
> - - "gals1": the path1 clock of GALS(Global Async Local Sync).
> - Here is the list which has this GALS: mt6779 and mt8183.
> -
> -Example:
> - smi_common: smi@14022000 {
> - compatible = "mediatek,mt8173-smi-common";
> - reg = <0 0x14022000 0 0x1000>;
> - power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
> - clocks = <&mmsys CLK_MM_SMI_COMMON>,
> - <&mmsys CLK_MM_SMI_COMMON>;
> - clock-names = "apb", "smi";
> - };
> diff --git
> a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
>
> b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> new file mode 100644
> index ..e050a0c2aed6
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> @@ -0,0 +1,140 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (c) 2020 MediaTek Inc.
> +%YAML 1.2
> +---
> +$id:
> http://devicetree.org/schemas/memory-controllers/mediatek,smi-common.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SMI (Smart Multimedia Interface) Common
> +
> +maintainers:
> + - Yong Wu
> +
> +description: |+
> + The hardware block diagram please check bindings/iommu/mediatek,iommu.yaml
> +
> + MediaTek SMI have two generations of HW architecture, here is the list
> + which generation the SoCs use:
> + generation 1: mt2701 and mt7623.
> + generation 2: mt2712, mt6779, mt8167, mt8173 and mt8183.
> +
> + There's slight differen
Re: [PATCH v3 00/24] MT8192 IOMMU support
On Wed, Sep 30, 2020 at 03:06:23PM +0800, Yong Wu wrote: > This patch mainly adds support for mt8192 IOMMU and SMI. > > mt8192 also is MTK IOMMU gen2 which uses ARM Short-Descriptor translation > table format. The M4U-SMI HW diagram is as below: > > EMI >| > M4U >| > >SMI Common > >| > +---+--+--+--+---+ > | | | | .. | | > | | | | | | > larb0 larb1 larb2 larb4 .. larb19 larb20 > disp0 disp1 mdpvdec IPE IPE > > All the connections are HW fixed, SW can NOT adjust it. > > Comparing with the preview SoC, this patchset mainly adds two new functions: > a) add iova 34 bits support. > b) add multi domains support since several HW has the special iova > region requirement. > > this patchset depend on v5.9-rc1. Hi, I think there will be v4 of this, right? If yes, please also describe the dependencies between the patches. If the entire patchset is strictly ordered, then mention this as well. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 01/24] dt-bindings: iommu: mediatek: Convert IOMMU to DT schema
On Tue, 6 Oct 2020 at 06:27, Yong Wu wrote: > > On Fri, 2020-10-02 at 13:07 +0200, Krzysztof Kozlowski wrote: > > On Wed, Sep 30, 2020 at 03:06:24PM +0800, Yong Wu wrote: > > > Convert MediaTek IOMMU to DT schema. > > > > > > Signed-off-by: Yong Wu > > > --- > > > .../bindings/iommu/mediatek,iommu.txt | 103 > > > .../bindings/iommu/mediatek,iommu.yaml| 154 ++ > > > 2 files changed, 154 insertions(+), 103 deletions(-) > > > delete mode 100644 > > > Documentation/devicetree/bindings/iommu/mediatek,iommu.txt > > > create mode 100644 > > > Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > > > > > [...] > > > > +properties: > > > + compatible: > > > +oneOf: > > > + - enum: > > > + - mediatek,mt2701-m4u # mt2701 generation one HW > > > + - mediatek,mt2712-m4u # mt2712 generation two HW > > > + - mediatek,mt6779-m4u # mt6779 generation two HW > > > + - mediatek,mt8173-m4u # mt8173 generation two HW > > > + - mediatek,mt8183-m4u # mt8183 generation two HW > > > + > > > + - description: mt7623 generation one HW > > > +items: > > > + - const: mediatek,mt7623-m4u > > > + - const: mediatek,mt2701-m4u > > > + > > > + reg: > > > +maxItems: 1 > > > + > > > + interrupts: > > > +maxItems: 1 > > > + > > > + clocks: > > > +description: | > > > + bclk is optional. here is the list which require this bclk: > > > + mt2701, mt2712, mt7623 and mt8173. > > > > Similarly to my comment in other patch, this should be part of schema > > within 'if-then'. > > Thanks for the review. > > I will change like this: > > = > clocks: > items: > - description: bclk is the block clock. > > clock-names: > items: > - const: bclk > > required: > - compatible > - reg > - interrupts > - mediatek,larbs > - '#iommu-cells' > if: > properties: > compatible: > contains: > enum: > - mediatek,mt2701-m4u > - mediatek,mt2712-m4u > - mediatek,mt8173-m4u > > then: > required: >- clocks > == > > If this is not right, please tell me. > (dt_binding_check is ok.) Looks fine, except "if" should be part of some "allOf" block. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 02/24] dt-bindings: memory: mediatek: Convert SMI to DT schema
On Mon, 12 Oct 2020 at 14:02, Yong Wu wrote: > > On Mon, 2020-10-12 at 09:18 +0200, Krzysztof Kozlowski wrote: > > On Sat, Oct 10, 2020 at 02:18:11PM +0800, Yong Wu wrote: > > > On Tue, 2020-10-06 at 09:15 +0200, Krzysztof Kozlowski wrote: > > > > On Tue, 6 Oct 2020 at 06:27, Yong Wu wrote: > > > > > > > > > > On Fri, 2020-10-02 at 13:08 +0200, Krzysztof Kozlowski wrote: > > > > > > On Wed, Sep 30, 2020 at 03:06:25PM +0800, Yong Wu wrote: > > > > > > > Convert MediaTek SMI to DT schema. > > > > > > > > > > > > > > Signed-off-by: Yong Wu > > > > > > > --- > > > > > > > .../mediatek,smi-common.txt | 49 - > > > > > > > .../mediatek,smi-common.yaml | 100 > > > > > > > ++ > > > > > > > .../memory-controllers/mediatek,smi-larb.txt | 49 - > > > > > > > .../memory-controllers/mediatek,smi-larb.yaml | 91 > > > > > > > > > > > > > > 4 files changed, 191 insertions(+), 98 deletions(-) > > > > > > > delete mode 100644 > > > > > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt > > > > > > > create mode 100644 > > > > > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml > > > > > > > delete mode 100644 > > > > > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt > > > > > > > create mode 100644 > > > > > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml > > > > > ... > > > > > > > +properties: > > > > > > > + compatible: > > > > > > > +oneOf: > > > > > > > + - enum: > > > > > > > + - mediatek,mt2701-smi-common > > > > > > > + - mediatek,mt2712-smi-common > > > > > > > + - mediatek,mt6779-smi-common > > > > > > > + - mediatek,mt8173-smi-common > > > > > > > + - mediatek,mt8183-smi-common > > > > > > > + > > > > > > > + - description: for mt7623 > > > > > > > +items: > > > > > > > + - const: mediatek,mt7623-smi-common > > > > > > > + - const: mediatek,mt2701-smi-common > > > > > > > + > > > > > > > + reg: > > > > > > > +maxItems: 1 > > > > > > > + > > > > > > > + clocks: > > > > > > > +description: | > > > > > > > + apb and smi are mandatory. the async is only for > > > > > > > generation 1 smi HW. > > > > > > > + gals(global async local sync) also is optional, here is > > > > > > > the list which > > > > > > > + require gals: mt6779 and mt8183. > > > > > > > +minItems: 2 > > > > > > > +maxItems: 4 > > > > > > > +items: > > > > > > > + - description: apb is Advanced Peripheral Bus clock, It's > > > > > > > the clock for > > > > > > > + setting the register. > > > > > > > + - description: smi is the clock for transfer data and > > > > > > > command. > > > > > > > + - description: async is asynchronous clock, it help > > > > > > > transform the smi clock > > > > > > > + into the emi clock domain. > > > > > > > + - description: gals0 is the path0 clock of gals. > > > > > > > + - description: gals1 is the path1 clock of gals. > > > > > > > + > > > > > > > + clock-names: > > > > > > > +oneOf: > > > > > > > + - items: > > > > > > > + - const: apb > > > > > > > + - const: smi > > > > > > > + - items: > > > > > > > + - const: apb > > > > > > > + - const: smi > > > > > > > + - const: async > > > > > > > + - items: > > > > > > > + - const: apb > > > > > > > + - const: smi > > > > > > > + - const: gals0 > > > > > > > + - const: gals1 > > > > > > > > > > > > Similarly to my comment to other properties, this requirement per > > > > > > compatible should be part of the schema within 'if-then'. > > > > > > > > > > I'm not so familiar with this format. Do this has "if-then-'else > > > > > if'-then-else"? > > > > > > > > These are mutually exclusive conditions, so you can skip else: > > > > - if-then > > > > - if-then > > > > - if-then > > > > It will be more readable then stacking 'if' under 'else' > > > > > > Thanks. I will use something like this: > > > > > > anyOf: > > > > Then it should be oneOf as only one condition can be valid. > > I did do this at the beginning. But I get a warning log when > dt_binding_check. Mhmm, right, since "if-else" matches in either of arms, then oneOf will complain as it expects only one of items to match. Then just go with allOf. anyOf might match zero of items, so it would not catch actual errors, I think. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 02/24] dt-bindings: memory: mediatek: Convert SMI to DT schema
On Sat, Oct 10, 2020 at 02:18:11PM +0800, Yong Wu wrote: > On Tue, 2020-10-06 at 09:15 +0200, Krzysztof Kozlowski wrote: > > On Tue, 6 Oct 2020 at 06:27, Yong Wu wrote: > > > > > > On Fri, 2020-10-02 at 13:08 +0200, Krzysztof Kozlowski wrote: > > > > On Wed, Sep 30, 2020 at 03:06:25PM +0800, Yong Wu wrote: > > > > > Convert MediaTek SMI to DT schema. > > > > > > > > > > Signed-off-by: Yong Wu > > > > > --- > > > > > .../mediatek,smi-common.txt | 49 - > > > > > .../mediatek,smi-common.yaml | 100 > > > > > ++ > > > > > .../memory-controllers/mediatek,smi-larb.txt | 49 - > > > > > .../memory-controllers/mediatek,smi-larb.yaml | 91 > > > > > 4 files changed, 191 insertions(+), 98 deletions(-) > > > > > delete mode 100644 > > > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt > > > > > create mode 100644 > > > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml > > > > > delete mode 100644 > > > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt > > > > > create mode 100644 > > > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml > > > ... > > > > > +properties: > > > > > + compatible: > > > > > +oneOf: > > > > > + - enum: > > > > > + - mediatek,mt2701-smi-common > > > > > + - mediatek,mt2712-smi-common > > > > > + - mediatek,mt6779-smi-common > > > > > + - mediatek,mt8173-smi-common > > > > > + - mediatek,mt8183-smi-common > > > > > + > > > > > + - description: for mt7623 > > > > > +items: > > > > > + - const: mediatek,mt7623-smi-common > > > > > + - const: mediatek,mt2701-smi-common > > > > > + > > > > > + reg: > > > > > +maxItems: 1 > > > > > + > > > > > + clocks: > > > > > +description: | > > > > > + apb and smi are mandatory. the async is only for generation 1 > > > > > smi HW. > > > > > + gals(global async local sync) also is optional, here is the > > > > > list which > > > > > + require gals: mt6779 and mt8183. > > > > > +minItems: 2 > > > > > +maxItems: 4 > > > > > +items: > > > > > + - description: apb is Advanced Peripheral Bus clock, It's the > > > > > clock for > > > > > + setting the register. > > > > > + - description: smi is the clock for transfer data and command. > > > > > + - description: async is asynchronous clock, it help transform > > > > > the smi clock > > > > > + into the emi clock domain. > > > > > + - description: gals0 is the path0 clock of gals. > > > > > + - description: gals1 is the path1 clock of gals. > > > > > + > > > > > + clock-names: > > > > > +oneOf: > > > > > + - items: > > > > > + - const: apb > > > > > + - const: smi > > > > > + - items: > > > > > + - const: apb > > > > > + - const: smi > > > > > + - const: async > > > > > + - items: > > > > > + - const: apb > > > > > + - const: smi > > > > > + - const: gals0 > > > > > + - const: gals1 > > > > > > > > Similarly to my comment to other properties, this requirement per > > > > compatible should be part of the schema within 'if-then'. > > > > > > I'm not so familiar with this format. Do this has "if-then-'else > > > if'-then-else"? > > > > These are mutually exclusive conditions, so you can skip else: > > - if-then > > - if-then > > - if-then > > It will be more readable then stacking 'if' under 'else' > > Thanks. I will use something like this: > > anyOf: Then it should be oneOf as only one condition can be valid. Best regards, Krzysztof >- if: #gen1 hw > then: >use apb/smi/async clocks > >- if: #gen2 hw that has gals. > then: >use apb/smi/gals0/gals1 clocks > else: # gen2 hw that doesn't have gals. >use apb/smi clocks. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 06/24] dt-bindings: mediatek: Add binding for mt8192 IOMMU
On Tue, Oct 06, 2020 at 12:26:45PM +0800, Yong Wu wrote: > Hi Krzysztof, > > On Fri, 2020-10-02 at 13:10 +0200, Krzysztof Kozlowski wrote: > > On Wed, Sep 30, 2020 at 03:06:29PM +0800, Yong Wu wrote: > > > This patch adds decriptions for mt8192 IOMMU and SMI. > > > > > > mt8192 also is MTK IOMMU gen2 which uses ARM Short-Descriptor translation > > > table format. The M4U-SMI HW diagram is as below: > > > > > > EMI > > >| > > > M4U > > >| > > > > > >SMI Common > > > > > >| > > > +---+--+--+--+---+ > > > | | | | .. | | > > > | | | | | | > > > larb0 larb1 larb2 larb4 .. larb19 larb20 > > > disp0 disp1 mdpvdec IPE IPE > > > > > > All the connections are HW fixed, SW can NOT adjust it. > > > > > > mt8192 M4U support 0~16GB iova range. we preassign different engines > > > into different iova ranges: > > > > > > domain-id module iova-range larbs > > >0 disp0 ~ 4G larb0/1 > > >1 vcodec 4G ~ 8G larb4/5/7 > > >2 cam/mdp 8G ~ 12G > > > larb2/9/11/13/14/16/17/18/19/20 > > >3 CCU00x4000_ ~ 0x43ff_ larb13: port 9/10 > > >4 CCU10x4400_ ~ 0x47ff_ larb14: port 4/5 > > > > > > The iova range for CCU0/1(camera control unit) is HW requirement. > > > > > > Signed-off-by: Yong Wu > > > Reviewed-by: Rob Herring > > > --- > > > .../bindings/iommu/mediatek,iommu.yaml| 9 +- > > > .../mediatek,smi-common.yaml | 5 +- > > > .../memory-controllers/mediatek,smi-larb.yaml | 3 +- > > > include/dt-bindings/memory/mt8192-larb-port.h | 239 ++ > > > 4 files changed, 251 insertions(+), 5 deletions(-) > > > create mode 100644 include/dt-bindings/memory/mt8192-larb-port.h > > > > I see it depends on previous patches but does it have to be within one > > commit? Is it not bisectable? The memory changes/bindings could go via > > memory tree if this is split. > > Thanks for the view. > > I can split this into two patchset in next version, one is for iommu and > the other is for smi. > > Only the patch [18/24] change both the code(iommu and smi). I don't plan > to split it, and the smi patch[24/24] don't depend on it(won't > conflict). It got too late in the cycle, so I am not going to take the 24/24 now. > > since 18/24 also touch the smi code, I expect it could get Acked-by from > you or Matthias, then Joerg could take it. Sure. I acked it. Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 18/24] iommu/mediatek: Support master use iova over 32bit
On Wed, Sep 30, 2020 at 03:06:41PM +0800, Yong Wu wrote: > After extending v7s, our pagetable already support iova reach > 16GB(34bit). the master got the iova via dma_alloc_attrs may reach > 34bits, but its HW register still is 32bit. then how to set the > bit32/bit33 iova? this depend on a SMI larb setting(bank_sel). > > we separate whole 16GB iova to four banks: > bank: 0: 0~4G; 1: 4~8G; 2: 8-12G; 3: 12-16G; > The bank number is (iova >> 32). > > We will preassign which bank the larbs belong to. currently we don't > have a interface for master to adjust its bank number. > > Each a bank is a iova_region which is a independent iommu-domain. > the iova range for each iommu-domain can't cross 4G. > > Signed-off-by: Yong Wu > --- > drivers/iommu/mtk_iommu.c | 12 +--- > drivers/memory/mtk-smi.c | 7 +++ > include/soc/mediatek/smi.h | 1 + > 3 files changed, 17 insertions(+), 3 deletions(-) For the memory part: Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
