Re: [PATCH v10 02/13] iommu/mediatek-v1: Free the existed fwspec if the master dev already has

[Matthias Brugger <matthias.bgg@xxxxxxxxx>]

On 28/01/2022 13:45, Mauro Carvalho Chehab wrote:
> Em Fri, 28 Jan 2022 13:40:55 +0100
> Mauro Carvalho Chehab <mchehab@xxxxxxxxxx> escreveu:
>
> > Hi Matthias/Yong,
> >
> > Are you ok if this patch gets merged via the media tree together with the
> > remaining series, or do you prefer to apply it via SoC tree instead?
>
> Same questions for other patches touching files outside drivers/media
> on this pull request:
>
> 	https://patchwork.kernel.org/project/linux-mediatek/patch/7af52d61-47c7-581d-62ed-76a7f8315b16@xxxxxxxxx/

Looks good to me.

Please let me know once you accepted the pull request and I'll queue the DTS 
related changes from this series.

Regards,
Matthias

> Like those:
> 	0004-0013-iommu-mediatek-v1-Free-the-existed-fwspec-if-the-mas.patch
> 	0005-0013-iommu-mediatek-Return-ENODEV-if-the-device-is-NULL.patch
> 	0006-0013-iommu-mediatek-Add-probe_defer-for-smi-larb.patch
> 	0007-0013-iommu-mediatek-Add-device_link-between-the-consumer-.patch
>
> Regards,
> Mauro
>
> > Regards,
> > Mauro
> >
> >
> > Em Mon, 17 Jan 2022 15:04:59 +0800
> > Yong Wu <yong.wu@xxxxxxxxxxxx> escreveu:
> >
> > > When the iommu master device enters of_iommu_xlate, the ops may be
> > > NULL(iommu dev is defered), then it will initialize the fwspec here:
> > >
> > > [<c0c9c5bc>] (dev_iommu_fwspec_set) from [<c06bda80>]
> > > (iommu_fwspec_init+0xbc/0xd4)
> > > [<c06bd9c4>] (iommu_fwspec_init) from [<c06c0db4>]
> > > (of_iommu_xlate+0x7c/0x12c)
> > > [<c06c0d38>] (of_iommu_xlate) from [<c06c10e8>]
> > > (of_iommu_configure+0x144/0x1e8)
> > >
> > > BUT the mtk_iommu_v1.c only supports arm32, the probing flow still is a bit
> > > weird. We always expect create the fwspec internally. otherwise it will
> > > enter here and return fail.
> > >
> > > static int mtk_iommu_create_mapping(struct device *dev,
> > > 				    struct of_phandle_args *args)
> > > {
> > >          ...
> > > 	if (!fwspec) {
> > > 	        ....
> > > 	} else if (dev_iommu_fwspec_get(dev)->ops != &mtk_iommu_ops) {
> > >                  >>>>>>>>>>Enter here. return fail.<<<<<<<<<<<<
> > > 		return -EINVAL;
> > > 	}
> > > 	...
> > > }
> > >
> > > Thus, Free the existed fwspec if the master device already has fwspec.
> > >
> > > This issue is reported at:
> > > https://lore.kernel.org/linux-mediatek/trinity-7d9ebdc9-4849-4d93-bfb5-429dcb4ee449-1626253158870@3c-app-gmx-bs01/
> > >
> > > Reported-by: Frank Wunderlich <frank-w@xxxxxxxxxxxxxxx>
> > > Tested-by: Frank Wunderlich <frank-w@xxxxxxxxxxxxxxx> # BPI-R2/MT7623
> > > Signed-off-by: Yong Wu <yong.wu@xxxxxxxxxxxx>
> > > Acked-by: Joerg Roedel <jroedel@xxxxxxx>
> > > Acked-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>
> > > ---
> > >   drivers/iommu/mtk_iommu_v1.c | 9 +++++++++
> > >   1 file changed, 9 insertions(+)
> > >
> > > diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
> > > index be22fcf988ce..1467ba1e4417 100644
> > > --- a/drivers/iommu/mtk_iommu_v1.c
> > > +++ b/drivers/iommu/mtk_iommu_v1.c
> > > @@ -425,6 +425,15 @@ static struct iommu_device *mtk_iommu_probe_device(struct device *dev)
> > >   	struct mtk_iommu_data *data;
> > >   	int err, idx = 0;
> > >   
> > > +	/*
> > > +	 * In the deferred case, free the existed fwspec.
> > > +	 * Always initialize the fwspec internally.
> > > +	 */
> > > +	if (fwspec) {
> > > +		iommu_fwspec_free(dev);
> > > +		fwspec = dev_iommu_fwspec_get(dev);
> > > +	}
> > > +
> > >   	while (!of_parse_phandle_with_args(dev->of_node, "iommus",
> > >   					   "#iommu-cells",
> > >   					   idx, &iommu_spec)) {
> >
> >
> >
> > Thanks,
> > Mauro
>
>
>
> Thanks,
> Mauro