On Fri, 14 Jun 2019 at 15:41, Lukasz Luba <l.luba@xxxxxxxxxxxxxxxxxxx> wrote: > > Hi Krzysztof, > > On 6/14/19 2:58 PM, Krzysztof Kozlowski wrote: > > On Fri, 14 Jun 2019 at 11:53, Lukasz Luba <l.luba@xxxxxxxxxxxxxxxxxxx> wrote: > >> > >> This patch adds driver for Exynos5422 Dynamic Memory Controller. > >> The driver provides support for dynamic frequency and voltage scaling for > >> DMC and DRAM. It supports changing timings of DRAM running with different > >> frequency. There is also an algorithm to calculate timigns based on > >> memory description provided in DT. > >> The patch also contains needed MAINTAINERS file update. > >> > >> Signed-off-by: Lukasz Luba <l.luba@xxxxxxxxxxxxxxxxxxx> > >> --- > >> MAINTAINERS | 8 + > >> drivers/memory/samsung/Kconfig | 17 + > >> drivers/memory/samsung/Makefile | 1 + > >> drivers/memory/samsung/exynos5422-dmc.c | 1262 +++++++++++++++++++++++ > >> 4 files changed, 1288 insertions(+) > >> create mode 100644 drivers/memory/samsung/exynos5422-dmc.c > >> > >> diff --git a/MAINTAINERS b/MAINTAINERS > >> index 57f496cff999..6ffccfd95351 100644 > >> --- a/MAINTAINERS > >> +++ b/MAINTAINERS > >> @@ -3470,6 +3470,14 @@ S: Maintained > >> F: drivers/devfreq/exynos-bus.c > >> F: Documentation/devicetree/bindings/devfreq/exynos-bus.txt > >> > >> +DMC FREQUENCY DRIVER FOR SAMSUNG EXYNOS5422 > > > > Eh, more comments from my side. > > "For the hard of thinking, this list is meant to remain in alphabetical order." > OK > > > >> +M: Lukasz Luba <l.luba@xxxxxxxxxxxxxxxxxxx> > >> +L: linux-pm@xxxxxxxxxxxxxxx > >> +L: linux-samsung-soc@xxxxxxxxxxxxxxx > >> +S: Maintained > >> +F: drivers/memory/samsung/exynos5422-dmc.c > >> +F: Documentation/devicetree/bindings/memory-controllers/exynos5422-dmc.txt > >> + > >> BUSLOGIC SCSI DRIVER > >> M: Khalid Aziz <khalid@xxxxxxxxxxxxxx> > >> L: linux-scsi@xxxxxxxxxxxxxxx > >> diff --git a/drivers/memory/samsung/Kconfig b/drivers/memory/samsung/Kconfig > >> index 79ce7ea58903..c93baa029654 100644 > >> --- a/drivers/memory/samsung/Kconfig > >> +++ b/drivers/memory/samsung/Kconfig > >> @@ -5,6 +5,23 @@ config SAMSUNG_MC > >> Support for the Memory Controller (MC) devices found on > >> Samsung Exynos SoCs. > >> > >> +config ARM_EXYNOS5422_DMC > > > > Why you added prefix of ARM to CONFIG think none of other Exynos > > Kconfig options follow such convention (except devfreq). > In the previous versions the driver was in drivers/devfreq/, > that's why they have this prefix. > > > >> + tristate "ARM EXYNOS5422 Dynamic Memory Controller driver" > >> + depends on ARCH_EXYNOS > >> + select DDR > > > > In general select should be used only for non-visible symbols and DDR > > is visible. Use depends. > OK > > > >> + select PM_DEVFREQ > > > > This definitely cannot be select. You do not select entire subsystem > > because some similar driver was chosen. > So I will use depends int this case > > > >> + select DEVFREQ_GOV_SIMPLE_ONDEMAND > >> + select DEVFREQ_GOV_USERSPACE > > > > I think only simple_ondemand is needed. Is userspace governor > > necessary for working? Or actually maybe both could be skipped? > > > Actually we can skip both governors from here. > > >> + select PM_DEVFREQ_EVENT > > > > Again, depends. > OK > > There are these 4 options which the driver depends on: > depends on ARCH_EXYNOS > depends on DDR > depends on PM_DEVFREQ > depends on PM_DEVFREQ_EVENT > > Should I merged them into one, two lines? like below: > a) > depends on (ARCH_EXYNOS && DDR && PM_DEVFREQ && PM_DEVFREQ_EVENT) > b) grouped into two sets > depends on (ARCH_EXYNOS && DDR) > depends on (PM_DEVFREQ && PM_DEVFREQ_EVENT) > c) grouped by pm_devfreq only > depends on ARCH_EXYNOS > depends on DDR > depends on (PM_DEVFREQ && PM_DEVFREQ_EVENT) I like the third option... although I would be also happy to see COMPILE_TEST (probably with IOMEM). You need to check which dependencies are still necessary for COMPILE_TEST. Best regards, Krzysztof