| Message ID | 20200930003013.31289-2-nicoleotsuka@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iommu/tegra-smmu: Add PCI support | expand |
30.09.2020 03:30, Nicolin Chen пишет: ... > int tegra_mc_write_emem_configuration(struct tegra_mc *mc, unsigned long rate); > unsigned int tegra_mc_get_emem_device_count(struct tegra_mc *mc); > +struct tegra_mc *tegra_get_memory_controller(void); > > #endif /* __SOC_TEGRA_MC_H__ */ > This will conflict with the tegra20-devfreq driver, you should change it as well.
On Wed, Sep 30, 2020 at 08:12:10AM +0300, Dmitry Osipenko wrote: > 30.09.2020 03:30, Nicolin Chen пишет: > ... > > int tegra_mc_write_emem_configuration(struct tegra_mc *mc, unsigned long rate); > > unsigned int tegra_mc_get_emem_device_count(struct tegra_mc *mc); > > +struct tegra_mc *tegra_get_memory_controller(void); > > > > #endif /* __SOC_TEGRA_MC_H__ */ > > > > This will conflict with the tegra20-devfreq driver, you should change it > as well. Will remove that in v3. Thanks
30.09.2020 08:44, Nicolin Chen пишет: > On Wed, Sep 30, 2020 at 08:12:10AM +0300, Dmitry Osipenko wrote: >> 30.09.2020 03:30, Nicolin Chen пишет: >> ... >>> int tegra_mc_write_emem_configuration(struct tegra_mc *mc, unsigned long rate); >>> unsigned int tegra_mc_get_emem_device_count(struct tegra_mc *mc); >>> +struct tegra_mc *tegra_get_memory_controller(void); >>> >>> #endif /* __SOC_TEGRA_MC_H__ */ >>> >> >> This will conflict with the tegra20-devfreq driver, you should change it >> as well. > > Will remove that in v3. > > Thanks > Please also consider to add a resource-managed variant, similar to what I did here: https://github.com/grate-driver/linux/commit/2105e7664063772d72fefe9696bdab0b688b9de2 I was going to use it in the next iteration of the memory interconnect series. But now it also will be good if you could add the devm variant to yours SMMU patchset since you're already about to touch the tegra20-devfreq driver. I'll then rebase my patches on top of yours patch.
On Wed, Sep 30, 2020 at 09:32:20AM +0300, Dmitry Osipenko wrote: > 30.09.2020 08:44, Nicolin Chen пишет: > > On Wed, Sep 30, 2020 at 08:12:10AM +0300, Dmitry Osipenko wrote: > >> 30.09.2020 03:30, Nicolin Chen пишет: > >> ... > >>> int tegra_mc_write_emem_configuration(struct tegra_mc *mc, unsigned long rate); > >>> unsigned int tegra_mc_get_emem_device_count(struct tegra_mc *mc); > >>> +struct tegra_mc *tegra_get_memory_controller(void); > >>> > >>> #endif /* __SOC_TEGRA_MC_H__ */ > >>> > >> > >> This will conflict with the tegra20-devfreq driver, you should change it > >> as well. > > > > Will remove that in v3. > > > > Thanks > > > > Please also consider to add a resource-managed variant, similar to what > I did here: > > https://github.com/grate-driver/linux/commit/2105e7664063772d72fefe9696bdab0b688b9de2 > > I was going to use it in the next iteration of the memory interconnect > series. > > But now it also will be good if you could add the devm variant to yours > SMMU patchset since you're already about to touch the tegra20-devfreq > driver. I'll then rebase my patches on top of yours patch. Just saw this reply. Yea, I think this'd be better. Thanks
On Wed, 30 Sep 2020 at 02:35, Nicolin Chen <nicoleotsuka@gmail.com> wrote: > > This can be used by both tegra-smmu and tegra20-devfreq drivers. > > Suggested-by: Dmitry Osipenko <digetx@gmail.com> > Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> > --- > > Changelog > v1->v2 > * N/A > > drivers/memory/tegra/mc.c | 23 +++++++++++++++++++++++ > include/soc/tegra/mc.h | 1 + > 2 files changed, 24 insertions(+) > > diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c > index ec8403557ed4..09352ad66dcc 100644 > --- a/drivers/memory/tegra/mc.c > +++ b/drivers/memory/tegra/mc.c > @@ -42,6 +42,29 @@ static const struct of_device_id tegra_mc_of_match[] = { > }; > MODULE_DEVICE_TABLE(of, tegra_mc_of_match); > +struct tegra_mc *tegra_get_memory_controller(void) > +{ Add kerneldoc and mention dropping of reference to the device after use. Best regards, Krzysztof > + struct platform_device *pdev; > + struct device_node *np; > + struct tegra_mc *mc; > + > + np = of_find_matching_node_and_match(NULL, tegra_mc_of_match, NULL); > + if (!np) > + return ERR_PTR(-ENOENT); > + > + pdev = of_find_device_by_node(np); > + of_node_put(np); > + if (!pdev) > + return ERR_PTR(-ENODEV); > + > + mc = platform_get_drvdata(pdev); > + if (!mc) > + return ERR_PTR(-EPROBE_DEFER); > + > + return mc; > +} > +EXPORT_SYMBOL_GPL(tegra_get_memory_controller); > + > static int tegra_mc_block_dma_common(struct tegra_mc *mc, > const struct tegra_mc_reset *rst) > { > diff --git a/include/soc/tegra/mc.h b/include/soc/tegra/mc.h > index 1238e35653d1..c72718fd589f 100644 > --- a/include/soc/tegra/mc.h > +++ b/include/soc/tegra/mc.h > @@ -183,5 +183,6 @@ struct tegra_mc { > > int tegra_mc_write_emem_configuration(struct tegra_mc *mc, unsigned long rate); > unsigned int tegra_mc_get_emem_device_count(struct tegra_mc *mc); > +struct tegra_mc *tegra_get_memory_controller(void); > > #endif /* __SOC_TEGRA_MC_H__ */ > -- > 2.17.1 >
Hi Krzysztof, On Wed, Sep 30, 2020 at 09:21:39AM +0200, Krzysztof Kozlowski wrote: > On Wed, 30 Sep 2020 at 02:35, Nicolin Chen <nicoleotsuka@gmail.com> wrote: > > > > This can be used by both tegra-smmu and tegra20-devfreq drivers. > > > > Suggested-by: Dmitry Osipenko <digetx@gmail.com> > > Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> > > --- > > > > Changelog > > v1->v2 > > * N/A > > > > drivers/memory/tegra/mc.c | 23 +++++++++++++++++++++++ > > include/soc/tegra/mc.h | 1 + > > 2 files changed, 24 insertions(+) > > > > diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c > > index ec8403557ed4..09352ad66dcc 100644 > > --- a/drivers/memory/tegra/mc.c > > +++ b/drivers/memory/tegra/mc.c > > @@ -42,6 +42,29 @@ static const struct of_device_id tegra_mc_of_match[] = { > > }; > > MODULE_DEVICE_TABLE(of, tegra_mc_of_match); > > > +struct tegra_mc *tegra_get_memory_controller(void) > > +{ > > Add kerneldoc and mention dropping of reference to the device after use. I am abort to use Dmitry's devm_ one in my next version: https://github.com/grate-driver/linux/commit/2105e7664063772d72fefe9696bdab0b688b9de2 Could I just skip the kerneldoc part? Otherwise, would you please tell me which kerneldoc file I should update? Thanks
30.09.2020 09:38, Nicolin Chen пишет: > On Wed, Sep 30, 2020 at 09:32:20AM +0300, Dmitry Osipenko wrote: >> 30.09.2020 08:44, Nicolin Chen пишет: >>> On Wed, Sep 30, 2020 at 08:12:10AM +0300, Dmitry Osipenko wrote: >>>> 30.09.2020 03:30, Nicolin Chen пишет: >>>> ... >>>>> int tegra_mc_write_emem_configuration(struct tegra_mc *mc, unsigned long rate); >>>>> unsigned int tegra_mc_get_emem_device_count(struct tegra_mc *mc); >>>>> +struct tegra_mc *tegra_get_memory_controller(void); >>>>> >>>>> #endif /* __SOC_TEGRA_MC_H__ */ >>>>> >>>> >>>> This will conflict with the tegra20-devfreq driver, you should change it >>>> as well. >>> >>> Will remove that in v3. >>> >>> Thanks >>> >> >> Please also consider to add a resource-managed variant, similar to what >> I did here: >> >> https://github.com/grate-driver/linux/commit/2105e7664063772d72fefe9696bdab0b688b9de2 >> >> I was going to use it in the next iteration of the memory interconnect >> series. >> >> But now it also will be good if you could add the devm variant to yours >> SMMU patchset since you're already about to touch the tegra20-devfreq >> driver. I'll then rebase my patches on top of yours patch. > > Just saw this reply. Yea, I think this'd be better. Thanks > Please don't forget to add a stub for !MC as well since devfreq drivers use COMPILE_TEST and don't directly depend on the MC driver.
On Wed, 30 Sep 2020 at 09:31, Nicolin Chen <nicoleotsuka@gmail.com> wrote: > > Hi Krzysztof, > > On Wed, Sep 30, 2020 at 09:21:39AM +0200, Krzysztof Kozlowski wrote: > > On Wed, 30 Sep 2020 at 02:35, Nicolin Chen <nicoleotsuka@gmail.com> wrote: > > > > > > This can be used by both tegra-smmu and tegra20-devfreq drivers. > > > > > > Suggested-by: Dmitry Osipenko <digetx@gmail.com> > > > Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> > > > --- > > > > > > Changelog > > > v1->v2 > > > * N/A > > > > > > drivers/memory/tegra/mc.c | 23 +++++++++++++++++++++++ > > > include/soc/tegra/mc.h | 1 + > > > 2 files changed, 24 insertions(+) > > > > > > diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c > > > index ec8403557ed4..09352ad66dcc 100644 > > > --- a/drivers/memory/tegra/mc.c > > > +++ b/drivers/memory/tegra/mc.c > > > @@ -42,6 +42,29 @@ static const struct of_device_id tegra_mc_of_match[] = { > > > }; > > > MODULE_DEVICE_TABLE(of, tegra_mc_of_match); > > > > > +struct tegra_mc *tegra_get_memory_controller(void) > > > +{ > > > > Add kerneldoc and mention dropping of reference to the device after use. > > I am abort to use Dmitry's devm_ one in my next version: > https://github.com/grate-driver/linux/commit/2105e7664063772d72fefe9696bdab0b688b9de2 > > Could I just skip the kerneldoc part? Otherwise, would you please > tell me which kerneldoc file I should update? His version is almost the same as yours so it does not matter - you declare an exported function, so you need to document it. kerneldoc goes to the C file. https://elixir.bootlin.com/linux/latest/source/Documentation/doc-guide/kernel-doc.rst Best regards, Krzysztof
diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c index ec8403557ed4..09352ad66dcc 100644 --- a/drivers/memory/tegra/mc.c +++ b/drivers/memory/tegra/mc.c @@ -42,6 +42,29 @@ static const struct of_device_id tegra_mc_of_match[] = { }; MODULE_DEVICE_TABLE(of, tegra_mc_of_match); +struct tegra_mc *tegra_get_memory_controller(void) +{ + struct platform_device *pdev; + struct device_node *np; + struct tegra_mc *mc; + + np = of_find_matching_node_and_match(NULL, tegra_mc_of_match, NULL); + if (!np) + return ERR_PTR(-ENOENT); + + pdev = of_find_device_by_node(np); + of_node_put(np); + if (!pdev) + return ERR_PTR(-ENODEV); + + mc = platform_get_drvdata(pdev); + if (!mc) + return ERR_PTR(-EPROBE_DEFER); + + return mc; +} +EXPORT_SYMBOL_GPL(tegra_get_memory_controller); + static int tegra_mc_block_dma_common(struct tegra_mc *mc, const struct tegra_mc_reset *rst) { diff --git a/include/soc/tegra/mc.h b/include/soc/tegra/mc.h index 1238e35653d1..c72718fd589f 100644 --- a/include/soc/tegra/mc.h +++ b/include/soc/tegra/mc.h @@ -183,5 +183,6 @@ struct tegra_mc { int tegra_mc_write_emem_configuration(struct tegra_mc *mc, unsigned long rate); unsigned int tegra_mc_get_emem_device_count(struct tegra_mc *mc); +struct tegra_mc *tegra_get_memory_controller(void); #endif /* __SOC_TEGRA_MC_H__ */
This can be used by both tegra-smmu and tegra20-devfreq drivers. Suggested-by: Dmitry Osipenko <digetx@gmail.com> Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> --- Changelog v1->v2 * N/A drivers/memory/tegra/mc.c | 23 +++++++++++++++++++++++ include/soc/tegra/mc.h | 1 + 2 files changed, 24 insertions(+)