2019-09-18 06:21:58

by Sam Shih

[permalink] [raw]
Subject: [RESEND, PATCH v7 04/11] pwm: mediatek: allocate the clks array dynamically

Instead of using fixed size of arrays, allocate the memory for them
based on the information we get from the DT.

Also remove the check for num_pwms, due to dynamically allocate pwm
should not cause array index out of bound.

Signed-off-by: Ryder Lee <[email protected]>
Signed-off-by: Sam Shih <[email protected]>
Reviewed-by: Uwe Kleine-König <[email protected]>
---
Changes since v6:
- Add a Reviewed-by tag

Changes since v5:
- Follow reviewers's comments
Make the changes of allocate the clks array dynamically as a single patch

Changes since v4:
- Follow reviewers's comments
1. use pc->soc->has_clks to check clocks exist or not.
2. Add error message when probe() unable to get clks
- Fixes bug when SoC is old mips which has no complex clock tree.
if clocks not exist, use the new property from DT to apply period caculation;
otherwise, use clk_get_rate to get clock frequency and apply period caculation.

---
drivers/pwm/pwm-mediatek.c | 84 +++++++++++++++++++-------------------
1 file changed, 42 insertions(+), 42 deletions(-)

diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
index 07e843aeddb1..71bfab7e2e19 100644
--- a/drivers/pwm/pwm-mediatek.c
+++ b/drivers/pwm/pwm-mediatek.c
@@ -35,25 +35,6 @@

#define PWM_CLK_DIV_MAX 7

-enum {
- MTK_CLK_MAIN = 0,
- MTK_CLK_TOP,
- MTK_CLK_PWM1,
- MTK_CLK_PWM2,
- MTK_CLK_PWM3,
- MTK_CLK_PWM4,
- MTK_CLK_PWM5,
- MTK_CLK_PWM6,
- MTK_CLK_PWM7,
- MTK_CLK_PWM8,
- MTK_CLK_MAX,
-};
-
-static const char * const mtk_pwm_clk_name[MTK_CLK_MAX] = {
- "main", "top", "pwm1", "pwm2", "pwm3", "pwm4", "pwm5", "pwm6", "pwm7",
- "pwm8"
-};
-
struct mtk_pwm_platform_data {
unsigned int fallback_npwms;
bool pwm45_fixup;
@@ -63,12 +44,17 @@ struct mtk_pwm_platform_data {
* struct mtk_pwm_chip - struct representing PWM chip
* @chip: linux PWM chip representation
* @regs: base address of PWM chip
- * @clks: list of clocks
+ * @clk_top: the top clock generator
+ * @clk_main: the clock used by PWM core
+ * @clk_pwms: the clock used by each PWM channel
+ * @clk_freq: the fix clock frequency of legacy MIPS SoC
*/
struct mtk_pwm_chip {
struct pwm_chip chip;
void __iomem *regs;
- struct clk *clks[MTK_CLK_MAX];
+ struct clk *clk_top;
+ struct clk *clk_main;
+ struct clk **clk_pwms;
const struct mtk_pwm_platform_data *soc;
};

@@ -86,24 +72,24 @@ static int mtk_pwm_clk_enable(struct pwm_chip *chip, struct pwm_device *pwm)
struct mtk_pwm_chip *pc = to_mtk_pwm_chip(chip);
int ret;

- ret = clk_prepare_enable(pc->clks[MTK_CLK_TOP]);
+ ret = clk_prepare_enable(pc->clk_top);
if (ret < 0)
return ret;

- ret = clk_prepare_enable(pc->clks[MTK_CLK_MAIN]);
+ ret = clk_prepare_enable(pc->clk_main);
if (ret < 0)
goto disable_clk_top;

- ret = clk_prepare_enable(pc->clks[MTK_CLK_PWM1 + pwm->hwpwm]);
+ ret = clk_prepare_enable(pc->clk_pwms[pwm->hwpwm]);
if (ret < 0)
goto disable_clk_main;

return 0;

disable_clk_main:
- clk_disable_unprepare(pc->clks[MTK_CLK_MAIN]);
+ clk_disable_unprepare(pc->clk_main);
disable_clk_top:
- clk_disable_unprepare(pc->clks[MTK_CLK_TOP]);
+ clk_disable_unprepare(pc->clk_top);

return ret;
}
@@ -112,9 +98,9 @@ static void mtk_pwm_clk_disable(struct pwm_chip *chip, struct pwm_device *pwm)
{
struct mtk_pwm_chip *pc = to_mtk_pwm_chip(chip);

- clk_disable_unprepare(pc->clks[MTK_CLK_PWM1 + pwm->hwpwm]);
- clk_disable_unprepare(pc->clks[MTK_CLK_MAIN]);
- clk_disable_unprepare(pc->clks[MTK_CLK_TOP]);
+ clk_disable_unprepare(pc->clk_pwms[pwm->hwpwm]);
+ clk_disable_unprepare(pc->clk_main);
+ clk_disable_unprepare(pc->clk_top);
}

static inline u32 mtk_pwm_readl(struct mtk_pwm_chip *chip, unsigned int num,
@@ -222,7 +208,7 @@ static int mtk_pwm_probe(struct platform_device *pdev)
struct device_node *np = pdev->dev.of_node;
struct mtk_pwm_chip *pc;
struct resource *res;
- unsigned int i, npwms;
+ unsigned int npwms;
int ret;

pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL);
@@ -248,21 +234,35 @@ static int mtk_pwm_probe(struct platform_device *pdev)
}
}

- /* MAIN + TOP + NPWM < MTK_CLK_MAX */
- if ((npwms + 2) > MTK_CLK_MAX) {
- dev_warn(&pdev->dev, "number of PWMs is larger than %d\n",
- MTK_CLK_MAX - 2);
- npwms = MTK_CLK_MAX - 2;
+ int i;
+
+ pc->clk_pwms = devm_kcalloc(&pdev->dev, npwms,
+ sizeof(*pc->clk_pwms), GFP_KERNEL);
+ if (!pc->clk_pwms)
+ return -ENOMEM;
+
+ pc->clk_top = devm_clk_get(&pdev->dev, "top");
+ if (IS_ERR(pc->clk_top)) {
+ dev_err(&pdev->dev, "clock: top fail: %ld\n",
+ PTR_ERR(pc->clk_top));
+ return PTR_ERR(pc->clk_top);
+ }
+
+ pc->clk_main = devm_clk_get(&pdev->dev, "main");
+ if (IS_ERR(pc->clk_main)) {
+ dev_err(&pdev->dev, "clock: main fail: %ld\n",
+ PTR_ERR(pc->clk_main));
+ return PTR_ERR(pc->clk_main);
}
+ for (i = 0; i < npwms; i++) {
+ char name[8];

- for (i = 0; i < npwms + 2 ; i++) {
- pc->clks[i] = devm_clk_get(&pdev->dev,
- mtk_pwm_clk_name[i]);
- if (IS_ERR(pc->clks[i])) {
+ snprintf(name, sizeof(name), "pwm%d", i + 1);
+ pc->clk_pwms[i] = devm_clk_get(&pdev->dev, name);
+ if (IS_ERR(pc->clk_pwms[i])) {
dev_err(&pdev->dev, "clock: %s fail: %ld\n",
- mtk_pwm_clk_name[i],
- PTR_ERR(pc->clks[i]));
- return PTR_ERR(pc->clks[i]);
+ name, PTR_ERR(pc->clk_pwms[i]));
+ return PTR_ERR(pc->clk_pwms[i]);
}
}

--
2.17.1


2019-09-18 09:04:49

by kernel test robot

[permalink] [raw]
Subject: Re: [RESEND, PATCH v7 04/11] pwm: mediatek: allocate the clks array dynamically

Hi Sam,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[cannot apply to v5.3 next-20190917]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Sam-Shih/Add-mt7629-and-fix-mt7628-pwm/20190918-140213
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=sparc64

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>

All warnings (new ones prefixed by >>):

drivers//pwm/pwm-mediatek.c: In function 'mtk_pwm_config':
drivers//pwm/pwm-mediatek.c:123:22: error: 'struct mtk_pwm_chip' has no member named 'clks'
struct clk *clk = pc->clks[MTK_CLK_PWM1 + pwm->hwpwm];
^~
drivers//pwm/pwm-mediatek.c:123:29: error: 'MTK_CLK_PWM1' undeclared (first use in this function)
struct clk *clk = pc->clks[MTK_CLK_PWM1 + pwm->hwpwm];
^~~~~~~~~~~~
drivers//pwm/pwm-mediatek.c:123:29: note: each undeclared identifier is reported only once for each function it appears in
drivers//pwm/pwm-mediatek.c: In function 'mtk_pwm_probe':
>> drivers//pwm/pwm-mediatek.c:237:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
int i;
^~~

vim +237 drivers//pwm/pwm-mediatek.c

118
119 static int mtk_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
120 int duty_ns, int period_ns)
121 {
122 struct mtk_pwm_chip *pc = to_mtk_pwm_chip(chip);
> 123 struct clk *clk = pc->clks[MTK_CLK_PWM1 + pwm->hwpwm];
124 u32 clkdiv = 0, cnt_period, cnt_duty, reg_width = PWMDWIDTH,
125 reg_thres = PWMTHRES;
126 u64 resolution;
127 int ret;
128
129 ret = mtk_pwm_clk_enable(chip, pwm);
130 if (ret < 0)
131 return ret;
132
133 /* Using resolution in picosecond gets accuracy higher */
134 resolution = (u64)NSEC_PER_SEC * 1000;
135 do_div(resolution, clk_get_rate(clk));
136
137 cnt_period = DIV_ROUND_CLOSEST_ULL((u64)period_ns * 1000, resolution);
138 while (cnt_period > 8191) {
139 resolution *= 2;
140 clkdiv++;
141 cnt_period = DIV_ROUND_CLOSEST_ULL((u64)period_ns * 1000,
142 resolution);
143 }
144
145 if (clkdiv > PWM_CLK_DIV_MAX) {
146 mtk_pwm_clk_disable(chip, pwm);
147 dev_err(chip->dev, "period %d not supported\n", period_ns);
148 return -EINVAL;
149 }
150
151 if (pc->soc->pwm45_fixup && pwm->hwpwm > 2) {
152 /*
153 * PWM[4,5] has distinct offset for PWMDWIDTH and PWMTHRES
154 * from the other PWMs on MT7623.
155 */
156 reg_width = PWM45DWIDTH_FIXUP;
157 reg_thres = PWM45THRES_FIXUP;
158 }
159
160 cnt_duty = DIV_ROUND_CLOSEST_ULL((u64)duty_ns * 1000, resolution);
161 mtk_pwm_writel(pc, pwm->hwpwm, PWMCON, BIT(15) | clkdiv);
162 mtk_pwm_writel(pc, pwm->hwpwm, reg_width, cnt_period);
163 mtk_pwm_writel(pc, pwm->hwpwm, reg_thres, cnt_duty);
164
165 mtk_pwm_clk_disable(chip, pwm);
166
167 return 0;
168 }
169
170 static int mtk_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
171 {
172 struct mtk_pwm_chip *pc = to_mtk_pwm_chip(chip);
173 u32 value;
174 int ret;
175
176 ret = mtk_pwm_clk_enable(chip, pwm);
177 if (ret < 0)
178 return ret;
179
180 value = readl(pc->regs);
181 value |= BIT(pwm->hwpwm);
182 writel(value, pc->regs);
183
184 return 0;
185 }
186
187 static void mtk_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
188 {
189 struct mtk_pwm_chip *pc = to_mtk_pwm_chip(chip);
190 u32 value;
191
192 value = readl(pc->regs);
193 value &= ~BIT(pwm->hwpwm);
194 writel(value, pc->regs);
195
196 mtk_pwm_clk_disable(chip, pwm);
197 }
198
199 static const struct pwm_ops mtk_pwm_ops = {
200 .config = mtk_pwm_config,
201 .enable = mtk_pwm_enable,
202 .disable = mtk_pwm_disable,
203 .owner = THIS_MODULE,
204 };
205
206 static int mtk_pwm_probe(struct platform_device *pdev)
207 {
208 struct device_node *np = pdev->dev.of_node;
209 struct mtk_pwm_chip *pc;
210 struct resource *res;
211 unsigned int npwms;
212 int ret;
213
214 pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL);
215 if (!pc)
216 return -ENOMEM;
217
218 pc->soc = of_device_get_match_data(&pdev->dev);
219
220 res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
221 pc->regs = devm_ioremap_resource(&pdev->dev, res);
222 if (IS_ERR(pc->regs))
223 return PTR_ERR(pc->regs);
224
225 ret = of_property_read_u32(np, "num-pwms", &npwms);
226 if (ret < 0) {
227 /* It's deprecated, we should specify num_pwms via DT now. */
228 if (pc->soc->fallback_npwms) {
229 npwms = pc->soc->fallback_npwms;
230 dev_warn(&pdev->dev, "DT is outdated, please update it\n");
231 } else {
232 dev_err(&pdev->dev, "failed to get number of PWMs\n");
233 return ret;
234 }
235 }
236
> 237 int i;
238
239 pc->clk_pwms = devm_kcalloc(&pdev->dev, npwms,
240 sizeof(*pc->clk_pwms), GFP_KERNEL);
241 if (!pc->clk_pwms)
242 return -ENOMEM;
243
244 pc->clk_top = devm_clk_get(&pdev->dev, "top");
245 if (IS_ERR(pc->clk_top)) {
246 dev_err(&pdev->dev, "clock: top fail: %ld\n",
247 PTR_ERR(pc->clk_top));
248 return PTR_ERR(pc->clk_top);
249 }
250
251 pc->clk_main = devm_clk_get(&pdev->dev, "main");
252 if (IS_ERR(pc->clk_main)) {
253 dev_err(&pdev->dev, "clock: main fail: %ld\n",
254 PTR_ERR(pc->clk_main));
255 return PTR_ERR(pc->clk_main);
256 }
257 for (i = 0; i < npwms; i++) {
258 char name[8];
259
260 snprintf(name, sizeof(name), "pwm%d", i + 1);
261 pc->clk_pwms[i] = devm_clk_get(&pdev->dev, name);
262 if (IS_ERR(pc->clk_pwms[i])) {
263 dev_err(&pdev->dev, "clock: %s fail: %ld\n",
264 name, PTR_ERR(pc->clk_pwms[i]));
265 return PTR_ERR(pc->clk_pwms[i]);
266 }
267 }
268
269 platform_set_drvdata(pdev, pc);
270
271 pc->chip.dev = &pdev->dev;
272 pc->chip.ops = &mtk_pwm_ops;
273 pc->chip.base = -1;
274 pc->chip.npwm = npwms;
275
276 ret = pwmchip_add(&pc->chip);
277 if (ret < 0) {
278 dev_err(&pdev->dev, "pwmchip_add() failed: %d\n", ret);
279 return ret;
280 }
281
282 return 0;
283 }
284

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (7.23 kB)
.config.gz (57.43 kB)
Download all attachments