| Message ID | 20220117070510.17642-3-yong.wu@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Clean up "mediatek,larb" | expand |
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? Regards, Mauro Em Mon, 17 Jan 2022 15:04:59 +0800 Yong Wu <yong.wu@mediatek.com> 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@public-files.de> > Tested-by: Frank Wunderlich <frank-w@public-files.de> # BPI-R2/MT7623 > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > Acked-by: Joerg Roedel <jroedel@suse.de> > Acked-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > --- > 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
Em Fri, 28 Jan 2022 13:40:55 +0100 Mauro Carvalho Chehab <mchehab@kernel.org> 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@xs4all.nl/ 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@mediatek.com> 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@public-files.de> > > Tested-by: Frank Wunderlich <frank-w@public-files.de> # BPI-R2/MT7623 > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > > Acked-by: Joerg Roedel <jroedel@suse.de> > > Acked-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > > --- > > 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
On 28/01/2022 13:45, Mauro Carvalho Chehab wrote: > Em Fri, 28 Jan 2022 13:40:55 +0100 > Mauro Carvalho Chehab <mchehab@kernel.org> 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@xs4all.nl/ > 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@mediatek.com> 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@public-files.de> >>> Tested-by: Frank Wunderlich <frank-w@public-files.de> # BPI-R2/MT7623 >>> Signed-off-by: Yong Wu <yong.wu@mediatek.com> >>> Acked-by: Joerg Roedel <jroedel@suse.de> >>> Acked-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> >>> --- >>> 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
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)) {
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@public-files.de> Tested-by: Frank Wunderlich <frank-w@public-files.de> # BPI-R2/MT7623 Signed-off-by: Yong Wu <yong.wu@mediatek.com> Acked-by: Joerg Roedel <jroedel@suse.de> Acked-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> --- drivers/iommu/mtk_iommu_v1.c | 9 +++++++++ 1 file changed, 9 insertions(+)