| Message ID | 20230207-iommu-support-v2-10-60d5fa00e4e5@baylibre.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add IOMMU support to MT8365 SoC | expand |
On 05/04/2023 10:06, Alexandre Mergnat wrote: > Add MT8365 SMI common support. > > Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> > --- Why the driver patch is after DTS? Driver code cannot be mixed with the DTS on branches/repos, so such ordering suggest your patchset is not bisectable. Best regards, Krzysztof
Ok, I will move the driver patch before the DTS patches in the next version. Regards, Alexandre Le mer. 5 avr. 2023 à 11:43, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> a écrit : > > On 05/04/2023 10:06, Alexandre Mergnat wrote: > > Add MT8365 SMI common support. > > > > Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> > > --- > > Why the driver patch is after DTS? Driver code cannot be mixed with the > DTS on branches/repos, so such ordering suggest your patchset is not > bisectable. > > Best regards, > Krzysztof >
On 05/04/2023 11:53, Alexandre Mergnat wrote: > Ok, I will move the driver patch before the DTS patches in the next version. > Or do not send it together at all, which might solve your dependency problem. According to your cover letter I cannot take the memory controller bits, so I am waiting for dependencies to hit the mainline. Alternatively I will need pull request with stable tag. Best regards, Krzysztof
On 05/04/2023 13:45, Krzysztof Kozlowski wrote: > On 05/04/2023 11:53, Alexandre Mergnat wrote: >> Ok, I will move the driver patch before the DTS patches in the next version. >> > Or do not send it together at all, which might solve your dependency > problem. According to your cover letter I cannot take the memory > controller bits, so I am waiting for dependencies to hit the mainline. > Alternatively I will need pull request with stable tag. > Ok, I prefer send the driver patch in another serie. That will solve the dependency with the DTS a least. Regards, Alexandre
On 05/04/2023 15:06, Alexandre Mergnat wrote: > > On 05/04/2023 13:45, Krzysztof Kozlowski wrote: >> On 05/04/2023 11:53, Alexandre Mergnat wrote: >>> Ok, I will move the driver patch before the DTS patches in the next version. >>> >> Or do not send it together at all, which might solve your dependency >> problem. According to your cover letter I cannot take the memory >> controller bits, so I am waiting for dependencies to hit the mainline. >> Alternatively I will need pull request with stable tag. >> > Ok, I prefer send the driver patch in another serie. That will solve the > dependency with the DTS a least. What dependency? Why do you have dependencies between drivers and DTS? That's a no-go. Best regards, Krzysztof
On 05/04/2023 15:54, Krzysztof Kozlowski wrote: > On 05/04/2023 15:06, Alexandre Mergnat wrote: >> On 05/04/2023 13:45, Krzysztof Kozlowski wrote: >>> On 05/04/2023 11:53, Alexandre Mergnat wrote: >>>> Ok, I will move the driver patch before the DTS patches in the next version. >>>> >>> Or do not send it together at all, which might solve your dependency >>> problem. According to your cover letter I cannot take the memory >>> controller bits, so I am waiting for dependencies to hit the mainline. >>> Alternatively I will need pull request with stable tag. >>> >> Ok, I prefer send the driver patch in another serie. That will solve the >> dependency with the DTS a least. > What dependency? Why do you have dependencies between drivers and DTS? > That's a no-go. I probably do something wrong but, that start with this comment [1]: > I guess we should add a independent "mediatek,mt8365-smi-common". Then I have added the mt8365 compatible support in the driver instead of using the mt8186 which already supported and used in the v1. I change the binding and DTS to use "mediatek,mt8365-smi-common" only (no more "mediatek,mt8186-smi-common"). Maybe "dependency isn't the good word to use in that case. Except for the patch order in the serie (or send the driver in another one), everything is fine or there are others wrong things ? [1]: https://lore.kernel.org/all/488d226a8a66bc6f5b96063b2816b59dd065ad3f.camel@mediatek.com/ Regards, Alexandre
On 05/04/2023 16:34, Alexandre Mergnat wrote: > > On 05/04/2023 15:54, Krzysztof Kozlowski wrote: >> On 05/04/2023 15:06, Alexandre Mergnat wrote: >>> On 05/04/2023 13:45, Krzysztof Kozlowski wrote: >>>> On 05/04/2023 11:53, Alexandre Mergnat wrote: >>>>> Ok, I will move the driver patch before the DTS patches in the next version. >>>>> >>>> Or do not send it together at all, which might solve your dependency >>>> problem. According to your cover letter I cannot take the memory >>>> controller bits, so I am waiting for dependencies to hit the mainline. >>>> Alternatively I will need pull request with stable tag. >>>> >>> Ok, I prefer send the driver patch in another serie. That will solve the >>> dependency with the DTS a least. >> What dependency? Why do you have dependencies between drivers and DTS? >> That's a no-go. > I probably do something wrong but, that start with this comment [1]: > >> I guess we should add a independent "mediatek,mt8365-smi-common". > > Then I have added the mt8365 compatible support in the driver instead of using the mt8186 which already supported and used in the v1. > I change the binding and DTS to use "mediatek,mt8365-smi-common" only (no more "mediatek,mt8186-smi-common"). > Maybe "dependency isn't the good word to use in that case. I do not see patch changing existing compatible. Which one is it? I don't know what is your meaning of dependency then. For all of us, dependency means one patch must be applied after another patch. So is this the case here? If yes, then why? > Except for the patch order in the serie (or send the driver in another one), everything is fine or there are others wrong things ? If this is the question to me, then I am not the maintainer of your platform. I am taking only memory controller bits, which look fine and I would have already apply them if not the dependency trouble. Soon the window for applying will close, BTW. We are almost at RC6. Best regards, Krzysztof
On Wed, 2023-04-05 at 10:06 +0200, Alexandre Mergnat wrote: > > Add MT8365 SMI common support. > > Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> > --- > drivers/memory/mtk-smi.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c > index 5a9754442bc7..477b5d1ffd46 100644 > --- a/drivers/memory/mtk-smi.c > +++ b/drivers/memory/mtk-smi.c > @@ -713,6 +713,12 @@ static const struct mtk_smi_common_plat > mtk_smi_sub_common_mt8195 = { > .has_gals = true, > }; > > +static const struct mtk_smi_common_plat mtk_smi_common_mt8365 = { > + .type = MTK_SMI_GEN2, > + .has_gals = true, mt8365 doesn't have "gals". Please remove this line, then the code is ok for me. Reviewed-by: Yong Wu <yong.wu@mediatek.com> Thanks. > + .bus_sel = F_MMU1_LARB(2) | F_MMU1_LARB(4), > +}; > + > static const struct of_device_id mtk_smi_common_of_ids[] = { > {.compatible = "mediatek,mt2701-smi-common", .data = > &mtk_smi_common_gen1}, > {.compatible = "mediatek,mt2712-smi-common", .data = > &mtk_smi_common_gen2}, > @@ -728,6 +734,7 @@ static const struct of_device_id > mtk_smi_common_of_ids[] = { > {.compatible = "mediatek,mt8195-smi-common-vdo", .data = > &mtk_smi_common_mt8195_vdo}, > {.compatible = "mediatek,mt8195-smi-common-vpp", .data = > &mtk_smi_common_mt8195_vpp}, > {.compatible = "mediatek,mt8195-smi-sub-common", .data = > &mtk_smi_sub_common_mt8195}, > + {.compatible = "mediatek,mt8365-smi-common", .data = > &mtk_smi_common_mt8365}, > {} > }; > > > -- > 2.25.1 >
Le jeu. 6 avr. 2023 à 09:50, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> a écrit : > > On 05/04/2023 16:34, Alexandre Mergnat wrote: > > > > On 05/04/2023 15:54, Krzysztof Kozlowski wrote: > >> On 05/04/2023 15:06, Alexandre Mergnat wrote: > >>> On 05/04/2023 13:45, Krzysztof Kozlowski wrote: > >>>> On 05/04/2023 11:53, Alexandre Mergnat wrote: > >>>>> Ok, I will move the driver patch before the DTS patches in the next version. > >>>>> > >>>> Or do not send it together at all, which might solve your dependency > >>>> problem. According to your cover letter I cannot take the memory > >>>> controller bits, so I am waiting for dependencies to hit the mainline. > >>>> Alternatively I will need pull request with stable tag. > >>>> > >>> Ok, I prefer send the driver patch in another serie. That will solve the > >>> dependency with the DTS a least. > >> What dependency? Why do you have dependencies between drivers and DTS? > >> That's a no-go. > > I probably do something wrong but, that start with this comment [1]: > > > >> I guess we should add a independent "mediatek,mt8365-smi-common". > > > > Then I have added the mt8365 compatible support in the driver instead of using the mt8186 which already supported and used in the v1. > > I change the binding and DTS to use "mediatek,mt8365-smi-common" only (no more "mediatek,mt8186-smi-common"). > > Maybe "dependency isn't the good word to use in that case. > > I do not see patch changing existing compatible. Which one is it? I was talking between the v1 and v2 of this serie > > I don't know what is your meaning of dependency then. For all of us, > dependency means one patch must be applied after another patch. So is > this the case here? If yes, then why? I don't think so. I think you start talking dependency in your 2nd reply about driver/DTS, but this is only related change. > > > Except for the patch order in the serie (or send the driver in another one), everything is fine or there are others wrong things ? > > If this is the question to me, then I am not the maintainer of your > platform. I am taking only memory controller bits, which look fine and I > would have already apply them if not the dependency trouble. Soon the > window for applying will close, BTW. We are almost at RC6. The dependency is between the power series and the DTS change in this series as explained in the cover letter thread (PATCH 00/10). IMHO, you can take the bindings without worry. I don't see possible regression. This series seems pretty simple, one part adds MT8365 SMI common support in the bindings, the mt8365-evk DTS and the mtk-smi driver, and there are no exotic stuffs. To conclude: I'm ok to send this driver patch in another series and put it as a dependency of the iommu series to answer your first comment: > >>>>>Driver code cannot be mixed with the DTS on branches/repos, so such ordering suggest your patchset is not bisectable. Regards, Alexandre
diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c index 5a9754442bc7..477b5d1ffd46 100644 --- a/drivers/memory/mtk-smi.c +++ b/drivers/memory/mtk-smi.c @@ -713,6 +713,12 @@ static const struct mtk_smi_common_plat mtk_smi_sub_common_mt8195 = { .has_gals = true, }; +static const struct mtk_smi_common_plat mtk_smi_common_mt8365 = { + .type = MTK_SMI_GEN2, + .has_gals = true, + .bus_sel = F_MMU1_LARB(2) | F_MMU1_LARB(4), +}; + static const struct of_device_id mtk_smi_common_of_ids[] = { {.compatible = "mediatek,mt2701-smi-common", .data = &mtk_smi_common_gen1}, {.compatible = "mediatek,mt2712-smi-common", .data = &mtk_smi_common_gen2}, @@ -728,6 +734,7 @@ static const struct of_device_id mtk_smi_common_of_ids[] = { {.compatible = "mediatek,mt8195-smi-common-vdo", .data = &mtk_smi_common_mt8195_vdo}, {.compatible = "mediatek,mt8195-smi-common-vpp", .data = &mtk_smi_common_mt8195_vpp}, {.compatible = "mediatek,mt8195-smi-sub-common", .data = &mtk_smi_sub_common_mt8195}, + {.compatible = "mediatek,mt8365-smi-common", .data = &mtk_smi_common_mt8365}, {} };
Add MT8365 SMI common support. Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> --- drivers/memory/mtk-smi.c | 7 +++++++ 1 file changed, 7 insertions(+)