Hi Krzysztof, On 6/14/19 3:47 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 >> +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 >> + tristate "ARM EXYNOS5422 Dynamic Memory Controller driver" >> + depends on ARCH_EXYNOS >> + select DDR >> + select PM_DEVFREQ >> + select DEVFREQ_GOV_SIMPLE_ONDEMAND >> + select DEVFREQ_GOV_USERSPACE >> + select PM_DEVFREQ_EVENT >> + select PM_OPP >> + help >> + This adds driver for Exynos5422 DMC (Dynamic Memory Controller). >> + The driver provides support for Dynamic Voltage and Frequency Scaling in >> + DMC and DRAM. It also supports changing timings of DRAM running with >> + different frequency. The timings are calculated based on DT memory >> + information. >> + >> + >> if SAMSUNG_MC >> >> config EXYNOS_SROM >> diff --git a/drivers/memory/samsung/Makefile b/drivers/memory/samsung/Makefile >> index 00587be66211..4f6e4383bab7 100644 >> --- a/drivers/memory/samsung/Makefile >> +++ b/drivers/memory/samsung/Makefile >> @@ -1,2 +1,3 @@ >> # SPDX-License-Identifier: GPL-2.0 >> +obj-$(CONFIG_ARM_EXYNOS5422_DMC) += exynos5422-dmc.o >> obj-$(CONFIG_EXYNOS_SROM) += exynos-srom.o >> diff --git a/drivers/memory/samsung/exynos5422-dmc.c b/drivers/memory/samsung/exynos5422-dmc.c >> new file mode 100644 >> index 000000000000..b397efe0da57 >> --- /dev/null >> +++ b/drivers/memory/samsung/exynos5422-dmc.c >> @@ -0,0 +1,1262 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (c) 2019 Samsung Electronics Co., Ltd. >> + * Author: Lukasz Luba <l.luba@xxxxxxxxxxxxxxxxxxx> >> + */ >> + >> +#include <linux/clk.h> >> +#include <linux/devfreq.h> >> +#include <linux/devfreq-event.h> >> +#include <linux/device.h> >> +#include <linux/io.h> >> +#include <linux/mfd/syscon.h> >> +#include <linux/module.h> >> +#include <linux/of_device.h> >> +#include <linux/pm_opp.h> >> +#include <linux/platform_device.h> >> +#include <linux/regmap.h> >> +#include <linux/regulator/consumer.h> >> +#include <linux/slab.h> >> +#include <memory/jedec_ddr.h> >> +#include "../of_memory.h" >> + >> +#define EXYNOS5_DREXI_TIMINGAREF (0x0030) >> +#define EXYNOS5_DREXI_TIMINGROW0 (0x0034) >> +#define EXYNOS5_DREXI_TIMINGDATA0 (0x0038) >> +#define EXYNOS5_DREXI_TIMINGPOWER0 (0x003C) >> +#define EXYNOS5_DREXI_TIMINGROW1 (0x00E4) >> +#define EXYNOS5_DREXI_TIMINGDATA1 (0x00E8) >> +#define EXYNOS5_DREXI_TIMINGPOWER1 (0x00EC) >> +#define CDREX_PAUSE (0x2091c) >> +#define CDREX_LPDDR3PHY_CON3 (0x20a20) >> +#define EXYNOS5_TIMING_SET_SWI (1UL << 28) >> +#define USE_MX_MSPLL_TIMINGS (1) >> +#define USE_BPLL_TIMINGS (0) >> +#define EXYNOS5_AREF_NORMAL (0x2e) >> + >> +/** >> + * struct dmc_opp_table - Operating level desciption >> + * >> + * Covers frequency and voltage settings of the DMC operating mode. >> + */ >> +struct dmc_opp_table { >> + u32 freq_hz; >> + u32 volt_uv; >> +}; >> + >> +/** >> + * struct exynos5_dmc - main structure describing DMC device >> + * >> + * The main structure for the Dynamic Memory Controller which covers clocks, >> + * memory regions, HW information, parameters and current operating mode. >> + */ >> +struct exynos5_dmc { >> + struct device *dev; >> + struct devfreq *df; >> + struct devfreq_simple_ondemand_data gov_data; >> + void __iomem *base_drexi0; >> + void __iomem *base_drexi1; >> + struct regmap *clk_regmap; >> + struct mutex lock; >> + unsigned long curr_rate; >> + unsigned long curr_volt; >> + unsigned long bypass_rate; >> + struct dmc_opp_table *opp; >> + struct dmc_opp_table opp_bypass; >> + int opp_count; >> + u32 timings_arr_size; >> + u32 *timing_row; >> + u32 *timing_data; >> + u32 *timing_power; >> + const struct lpddr3_timings *timings; >> + const struct lpddr3_min_tck *min_tck; >> + u32 bypass_timing_row; >> + u32 bypass_timing_data; >> + u32 bypass_timing_power; >> + struct regulator *vdd_mif; >> + struct clk *fout_spll; >> + struct clk *fout_bpll; >> + struct clk *mout_spll; >> + struct clk *mout_bpll; >> + struct clk *mout_mclk_cdrex; >> + struct clk *mout_mx_mspll_ccore; >> + struct clk *mx_mspll_ccore_phy; >> + struct clk *mout_mx_mspll_ccore_phy; >> + struct devfreq_event_dev **counter; >> + int num_counters; >> +}; >> + >> +#define TIMING_FIELD(t_name, t_bit_beg, t_bit_end) \ >> + { .name = t_name, .bit_beg = t_bit_beg, .bit_end = t_bit_end } >> + >> +#define TIMING_VAL(timing_array, id, t_val) \ >> +({ \ >> + u32 __val; \ >> + __val = t_val << timing_array[id].bit_beg; \ >> + __val; \ >> +}) >> + >> +#define TIMING_VAL2REG(timing, t_val) \ >> +({ \ >> + u32 __val; \ >> + __val = t_val << timing->bit_beg; \ >> + __val; \ >> +}) >> + >> +#define TIMING_REG2VAL(reg, timing) \ > > It seems that only some of these defines are used. Please clean them up. Indeed, they were used in previous version which had more features and debug options. So TIMING_REG2VAL and TIMING_VAL will be cleaned > You have also a lot of checkpatch --strict suggestions: > CHECK: Macro argument 'reg' may be better as '(reg)' to avoid > precedence issues > which seems to be valid. Will not be valid since it is in TIMING_REG2VAL macro which was used for debugfs information only. > > While at it please also fix few other --strict errors: > CHECK: Please don't use multiple blank lines > CHECK: Alignment should match open parenthesis > CHECK: Prefer using the BIT macro > CHECK: struct mutex definition without comment I wouldn't call them 'errors' but will do a cleanup. Thank you for the review. Regards, Lukasz > > Best regards, > Krzysztof > >