]>
Commit | Line | Data |
---|---|---|
37554d48 SL |
1 | From aac4c49cf718182439225690a841d6a706aa8c1b Mon Sep 17 00:00:00 2001 |
2 | From: Martin Blumenstingl <martin.blumenstingl@googlemail.com> | |
3 | Date: Mon, 1 Apr 2019 19:57:48 +0200 | |
4 | Subject: pwm: meson: Use the spin-lock only to protect register modifications | |
5 | MIME-Version: 1.0 | |
6 | Content-Type: text/plain; charset=UTF-8 | |
7 | Content-Transfer-Encoding: 8bit | |
8 | ||
9 | [ Upstream commit f173747fffdf037c791405ab4f1ec0eb392fc48e ] | |
10 | ||
11 | Holding the spin-lock for all of the code in meson_pwm_apply() can | |
12 | result in a "BUG: scheduling while atomic". This can happen because | |
13 | clk_get_rate() (which is called from meson_pwm_calc()) may sleep. | |
14 | Only hold the spin-lock when modifying registers to solve this. | |
15 | ||
16 | The reason why we need a spin-lock in the driver is because the | |
17 | REG_MISC_AB register is shared between the two channels provided by one | |
18 | PWM controller. The only functions where REG_MISC_AB is modified are | |
19 | meson_pwm_enable() and meson_pwm_disable() so the register reads/writes | |
20 | in there need to be protected by the spin-lock. | |
21 | ||
22 | The original code also used the spin-lock to protect the values in | |
23 | struct meson_pwm_channel. This could be necessary if two consumers can | |
24 | use the same PWM channel. However, PWM core doesn't allow this so we | |
25 | don't need to protect the values in struct meson_pwm_channel with a | |
26 | lock. | |
27 | ||
28 | Fixes: 211ed630753d2f ("pwm: Add support for Meson PWM Controller") | |
29 | Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> | |
30 | Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> | |
31 | Reviewed-by: Neil Armstrong <narmstrong@baylibre.com> | |
32 | Signed-off-by: Thierry Reding <thierry.reding@gmail.com> | |
33 | Signed-off-by: Sasha Levin <sashal@kernel.org> | |
34 | --- | |
35 | drivers/pwm/pwm-meson.c | 25 +++++++++++++++++-------- | |
36 | 1 file changed, 17 insertions(+), 8 deletions(-) | |
37 | ||
38 | diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c | |
39 | index c1ed641b3e26..f6e738ad7bd9 100644 | |
40 | --- a/drivers/pwm/pwm-meson.c | |
41 | +++ b/drivers/pwm/pwm-meson.c | |
42 | @@ -111,6 +111,10 @@ struct meson_pwm { | |
43 | const struct meson_pwm_data *data; | |
44 | void __iomem *base; | |
45 | u8 inverter_mask; | |
46 | + /* | |
47 | + * Protects register (write) access to the REG_MISC_AB register | |
48 | + * that is shared between the two PWMs. | |
49 | + */ | |
50 | spinlock_t lock; | |
51 | }; | |
52 | ||
53 | @@ -235,6 +239,7 @@ static void meson_pwm_enable(struct meson_pwm *meson, | |
54 | { | |
55 | u32 value, clk_shift, clk_enable, enable; | |
56 | unsigned int offset; | |
57 | + unsigned long flags; | |
58 | ||
59 | switch (id) { | |
60 | case 0: | |
61 | @@ -255,6 +260,8 @@ static void meson_pwm_enable(struct meson_pwm *meson, | |
62 | return; | |
63 | } | |
64 | ||
65 | + spin_lock_irqsave(&meson->lock, flags); | |
66 | + | |
67 | value = readl(meson->base + REG_MISC_AB); | |
68 | value &= ~(MISC_CLK_DIV_MASK << clk_shift); | |
69 | value |= channel->pre_div << clk_shift; | |
70 | @@ -267,11 +274,14 @@ static void meson_pwm_enable(struct meson_pwm *meson, | |
71 | value = readl(meson->base + REG_MISC_AB); | |
72 | value |= enable; | |
73 | writel(value, meson->base + REG_MISC_AB); | |
74 | + | |
75 | + spin_unlock_irqrestore(&meson->lock, flags); | |
76 | } | |
77 | ||
78 | static void meson_pwm_disable(struct meson_pwm *meson, unsigned int id) | |
79 | { | |
80 | u32 value, enable; | |
81 | + unsigned long flags; | |
82 | ||
83 | switch (id) { | |
84 | case 0: | |
85 | @@ -286,9 +296,13 @@ static void meson_pwm_disable(struct meson_pwm *meson, unsigned int id) | |
86 | return; | |
87 | } | |
88 | ||
89 | + spin_lock_irqsave(&meson->lock, flags); | |
90 | + | |
91 | value = readl(meson->base + REG_MISC_AB); | |
92 | value &= ~enable; | |
93 | writel(value, meson->base + REG_MISC_AB); | |
94 | + | |
95 | + spin_unlock_irqrestore(&meson->lock, flags); | |
96 | } | |
97 | ||
98 | static int meson_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, | |
99 | @@ -296,19 +310,16 @@ static int meson_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, | |
100 | { | |
101 | struct meson_pwm_channel *channel = pwm_get_chip_data(pwm); | |
102 | struct meson_pwm *meson = to_meson_pwm(chip); | |
103 | - unsigned long flags; | |
104 | int err = 0; | |
105 | ||
106 | if (!state) | |
107 | return -EINVAL; | |
108 | ||
109 | - spin_lock_irqsave(&meson->lock, flags); | |
110 | - | |
111 | if (!state->enabled) { | |
112 | meson_pwm_disable(meson, pwm->hwpwm); | |
113 | channel->state.enabled = false; | |
114 | ||
115 | - goto unlock; | |
116 | + return 0; | |
117 | } | |
118 | ||
119 | if (state->period != channel->state.period || | |
120 | @@ -329,7 +340,7 @@ static int meson_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, | |
121 | err = meson_pwm_calc(meson, channel, pwm->hwpwm, | |
122 | state->duty_cycle, state->period); | |
123 | if (err < 0) | |
124 | - goto unlock; | |
125 | + return err; | |
126 | ||
127 | channel->state.polarity = state->polarity; | |
128 | channel->state.period = state->period; | |
129 | @@ -341,9 +352,7 @@ static int meson_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, | |
130 | channel->state.enabled = true; | |
131 | } | |
132 | ||
133 | -unlock: | |
134 | - spin_unlock_irqrestore(&meson->lock, flags); | |
135 | - return err; | |
136 | + return 0; | |
137 | } | |
138 | ||
139 | static void meson_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, | |
140 | -- | |
141 | 2.20.1 | |
142 |