Some platforms such as Spreadtrum platform, define a special method to
update bits of the registers instead of read-modify-write, which means
we should use a physical regmap bus to define the reg_update_bits()
operation instead of the MMIO regmap bus. Thus we can register a new
physical regmap bus into syscon core to support this.
Signed-off-by: Baolin Wang <[email protected]>
---
Changes from v1:
- Add WARN_ONCE() for seting bits and clearing bits at the same time.
- Remove the Spreadtrum SoC syscon driver, instead moving the regmap_bus
instance into syscon.c driver.
Changes from RFC v2:
- Drop regmap change, which was applied by Mark.
- Add more information about how to use set/clear.
- Add checking to ensure the platform is compatible with
using a new physical regmap bus.
Changes from RFC v1:
- Add new helper to registers a physical regmap bus instead of
using the MMIO bus.
---
drivers/mfd/syscon.c | 81 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 79 insertions(+), 2 deletions(-)
diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
index 3a97816d0cba..f85420d14ce3 100644
--- a/drivers/mfd/syscon.c
+++ b/drivers/mfd/syscon.c
@@ -40,6 +40,70 @@ static const struct regmap_config syscon_regmap_config = {
.reg_stride = 4,
};
+#if IS_ENABLED(CONFIG_ARCH_SPRD)
+#define SPRD_REG_SET_OFFSET 0x1000
+#define SPRD_REG_CLR_OFFSET 0x2000
+
+/*
+ * The Spreadtrum platform defines a special set/clear method to update
+ * registers' bits, which means it can write values to the register's SET
+ * address (offset is 0x1000) to set bits, and write values to the register's
+ * CLEAR address (offset is 0x2000) to clear bits.
+ *
+ * This set/clear method can help to remove the race of accessing the global
+ * registers between the multiple subsystems instead of using hardware
+ * spinlocks.
+ *
+ * Note: there is a potential risk when users want to set and clear bits
+ * at the same time, since the set/clear method will always do bits setting
+ * before bits clearing, which may cause some unexpected results if the
+ * operation sequence is strict. Thus we recommend that do not set and
+ * clear bits at the same time if you are not sure about the results.
+ */
+static int sprd_syscon_update_bits(void *context, unsigned int reg,
+ unsigned int mask, unsigned int val)
+{
+ void __iomem *base = context;
+ unsigned int set, clr;
+
+ set = val & mask;
+ clr = ~set & mask;
+
+ if (set)
+ writel(set, base + reg + SPRD_REG_SET_OFFSET);
+
+ if (clr)
+ writel(clr, base + reg + SPRD_REG_CLR_OFFSET);
+
+ WARN_ONCE(set && clr, "%s: non-atomic update", __func__);
+ return 0;
+}
+
+static int sprd_syscon_read(void *context, unsigned int reg, unsigned int *val)
+{
+ void __iomem *base = context;
+
+ *val = readl(base + reg);
+ return 0;
+}
+
+static int sprd_syscon_write(void *context, unsigned int reg, unsigned int val)
+{
+ void __iomem *base = context;
+
+ writel(val, base + reg);
+ return 0;
+}
+
+static struct regmap_bus sprd_syscon_regmap_bus = {
+ .fast_io = true,
+ .reg_write = sprd_syscon_write,
+ .reg_read = sprd_syscon_read,
+ .reg_update_bits = sprd_syscon_update_bits,
+ .val_format_endian_default = REGMAP_ENDIAN_LITTLE,
+};
+#endif
+
static struct syscon *of_syscon_register(struct device_node *np, bool check_clk)
{
struct clk *clk;
@@ -50,6 +114,7 @@ static struct syscon *of_syscon_register(struct device_node *np, bool check_clk)
int ret;
struct regmap_config syscon_config = syscon_regmap_config;
struct resource res;
+ bool use_phy_regmap_bus = false;
syscon = kzalloc(sizeof(*syscon), GFP_KERNEL);
if (!syscon)
@@ -106,14 +171,26 @@ static struct syscon *of_syscon_register(struct device_node *np, bool check_clk)
syscon_config.val_bits = reg_io_width * 8;
syscon_config.max_register = resource_size(&res) - reg_io_width;
- regmap = regmap_init_mmio(NULL, base, &syscon_config);
+ /*
+ * The Spreadtrum syscon need register a real physical regmap bus
+ * with new atomic bits updating operation instead of using
+ * read-modify-write.
+ */
+ if (IS_ENABLED(CONFIG_ARCH_SPRD) &&
+ of_device_is_compatible(np, "sprd,atomic-syscon")) {
+ use_phy_regmap_bus = true;
+ regmap = regmap_init(NULL, &sprd_syscon_regmap_bus, base,
+ &syscon_config);
+ } else {
+ regmap = regmap_init_mmio(NULL, base, &syscon_config);
+ }
if (IS_ERR(regmap)) {
pr_err("regmap init failed\n");
ret = PTR_ERR(regmap);
goto err_regmap;
}
- if (check_clk) {
+ if (!use_phy_regmap_bus && check_clk) {
clk = of_clk_get(np, 0);
if (IS_ERR(clk)) {
ret = PTR_ERR(clk);
--
2.17.1
Hi Baolin,
I love your patch! Yet something to improve:
[auto build test ERROR on ljones-mfd/for-mfd-next]
[also build test ERROR on arm-soc/for-next linus/master linux/master v5.7-rc2 next-20200420]
[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/Baolin-Wang/mfd-syscon-Add-Spreadtrum-physical-regmap-bus-support/20200421-035442
base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: x86_64-randconfig-f002-20200421 (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project a9b137f9ffba8cb25dfd7dd1fb613e8aac121b37)
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <[email protected]>
All errors (new ones prefixed by >>):
>> drivers/mfd/syscon.c:182:31: error: use of undeclared identifier 'sprd_syscon_regmap_bus'
regmap = regmap_init(NULL, &sprd_syscon_regmap_bus, base,
^
>> drivers/mfd/syscon.c:182:10: error: assigning to 'struct regmap *' from incompatible type 'void'
regmap = regmap_init(NULL, &sprd_syscon_regmap_bus, base,
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 errors generated.
vim +/sprd_syscon_regmap_bus +182 drivers/mfd/syscon.c
106
107 static struct syscon *of_syscon_register(struct device_node *np, bool check_clk)
108 {
109 struct clk *clk;
110 struct syscon *syscon;
111 struct regmap *regmap;
112 void __iomem *base;
113 u32 reg_io_width;
114 int ret;
115 struct regmap_config syscon_config = syscon_regmap_config;
116 struct resource res;
117 bool use_phy_regmap_bus = false;
118
119 syscon = kzalloc(sizeof(*syscon), GFP_KERNEL);
120 if (!syscon)
121 return ERR_PTR(-ENOMEM);
122
123 if (of_address_to_resource(np, 0, &res)) {
124 ret = -ENOMEM;
125 goto err_map;
126 }
127
128 base = ioremap(res.start, resource_size(&res));
129 if (!base) {
130 ret = -ENOMEM;
131 goto err_map;
132 }
133
134 /* Parse the device's DT node for an endianness specification */
135 if (of_property_read_bool(np, "big-endian"))
136 syscon_config.val_format_endian = REGMAP_ENDIAN_BIG;
137 else if (of_property_read_bool(np, "little-endian"))
138 syscon_config.val_format_endian = REGMAP_ENDIAN_LITTLE;
139 else if (of_property_read_bool(np, "native-endian"))
140 syscon_config.val_format_endian = REGMAP_ENDIAN_NATIVE;
141
142 /*
143 * search for reg-io-width property in DT. If it is not provided,
144 * default to 4 bytes. regmap_init_mmio will return an error if values
145 * are invalid so there is no need to check them here.
146 */
147 ret = of_property_read_u32(np, "reg-io-width", ®_io_width);
148 if (ret)
149 reg_io_width = 4;
150
151 ret = of_hwspin_lock_get_id(np, 0);
152 if (ret > 0 || (IS_ENABLED(CONFIG_HWSPINLOCK) && ret == 0)) {
153 syscon_config.use_hwlock = true;
154 syscon_config.hwlock_id = ret;
155 syscon_config.hwlock_mode = HWLOCK_IRQSTATE;
156 } else if (ret < 0) {
157 switch (ret) {
158 case -ENOENT:
159 /* Ignore missing hwlock, it's optional. */
160 break;
161 default:
162 pr_err("Failed to retrieve valid hwlock: %d\n", ret);
163 /* fall-through */
164 case -EPROBE_DEFER:
165 goto err_regmap;
166 }
167 }
168
169 syscon_config.name = of_node_full_name(np);
170 syscon_config.reg_stride = reg_io_width;
171 syscon_config.val_bits = reg_io_width * 8;
172 syscon_config.max_register = resource_size(&res) - reg_io_width;
173
174 /*
175 * The Spreadtrum syscon need register a real physical regmap bus
176 * with new atomic bits updating operation instead of using
177 * read-modify-write.
178 */
179 if (IS_ENABLED(CONFIG_ARCH_SPRD) &&
180 of_device_is_compatible(np, "sprd,atomic-syscon")) {
181 use_phy_regmap_bus = true;
> 182 regmap = regmap_init(NULL, &sprd_syscon_regmap_bus, base,
183 &syscon_config);
184 } else {
185 regmap = regmap_init_mmio(NULL, base, &syscon_config);
186 }
187 if (IS_ERR(regmap)) {
188 pr_err("regmap init failed\n");
189 ret = PTR_ERR(regmap);
190 goto err_regmap;
191 }
192
193 if (!use_phy_regmap_bus && check_clk) {
194 clk = of_clk_get(np, 0);
195 if (IS_ERR(clk)) {
196 ret = PTR_ERR(clk);
197 /* clock is optional */
198 if (ret != -ENOENT)
199 goto err_clk;
200 } else {
201 ret = regmap_mmio_attach_clk(regmap, clk);
202 if (ret)
203 goto err_attach;
204 }
205 }
206
207 syscon->regmap = regmap;
208 syscon->np = np;
209
210 spin_lock(&syscon_list_slock);
211 list_add_tail(&syscon->list, &syscon_list);
212 spin_unlock(&syscon_list_slock);
213
214 return syscon;
215
216 err_attach:
217 if (!IS_ERR(clk))
218 clk_put(clk);
219 err_clk:
220 regmap_exit(regmap);
221 err_regmap:
222 iounmap(base);
223 err_map:
224 kfree(syscon);
225 return ERR_PTR(ret);
226 }
227
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On Tue, Apr 21, 2020 at 6:03 PM kbuild test robot <[email protected]> wrote:
>
> Hi Baolin,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on ljones-mfd/for-mfd-next]
> [also build test ERROR on arm-soc/for-next linus/master linux/master v5.7-rc2 next-20200420]
> [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/Baolin-Wang/mfd-syscon-Add-Spreadtrum-physical-regmap-bus-support/20200421-035442
> base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
> config: x86_64-randconfig-f002-20200421 (attached as .config)
> compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project a9b137f9ffba8cb25dfd7dd1fb613e8aac121b37)
> reproduce:
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # install x86_64 cross compiling tool for clang build
> # apt-get install binutils-x86-64-linux-gnu
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kbuild test robot <[email protected]>
>
> All errors (new ones prefixed by >>):
>
> >> drivers/mfd/syscon.c:182:31: error: use of undeclared identifier 'sprd_syscon_regmap_bus'
> regmap = regmap_init(NULL, &sprd_syscon_regmap_bus, base,
> ^
> >> drivers/mfd/syscon.c:182:10: error: assigning to 'struct regmap *' from incompatible type 'void'
> regmap = regmap_init(NULL, &sprd_syscon_regmap_bus, base,
> ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 2 errors generated.
Ah, sorry, will fix the building errors in next version. Thanks for reporting.
>
> vim +/sprd_syscon_regmap_bus +182 drivers/mfd/syscon.c
>
> 106
> 107 static struct syscon *of_syscon_register(struct device_node *np, bool check_clk)
> 108 {
> 109 struct clk *clk;
> 110 struct syscon *syscon;
> 111 struct regmap *regmap;
> 112 void __iomem *base;
> 113 u32 reg_io_width;
> 114 int ret;
> 115 struct regmap_config syscon_config = syscon_regmap_config;
> 116 struct resource res;
> 117 bool use_phy_regmap_bus = false;
> 118
> 119 syscon = kzalloc(sizeof(*syscon), GFP_KERNEL);
> 120 if (!syscon)
> 121 return ERR_PTR(-ENOMEM);
> 122
> 123 if (of_address_to_resource(np, 0, &res)) {
> 124 ret = -ENOMEM;
> 125 goto err_map;
> 126 }
> 127
> 128 base = ioremap(res.start, resource_size(&res));
> 129 if (!base) {
> 130 ret = -ENOMEM;
> 131 goto err_map;
> 132 }
> 133
> 134 /* Parse the device's DT node for an endianness specification */
> 135 if (of_property_read_bool(np, "big-endian"))
> 136 syscon_config.val_format_endian = REGMAP_ENDIAN_BIG;
> 137 else if (of_property_read_bool(np, "little-endian"))
> 138 syscon_config.val_format_endian = REGMAP_ENDIAN_LITTLE;
> 139 else if (of_property_read_bool(np, "native-endian"))
> 140 syscon_config.val_format_endian = REGMAP_ENDIAN_NATIVE;
> 141
> 142 /*
> 143 * search for reg-io-width property in DT. If it is not provided,
> 144 * default to 4 bytes. regmap_init_mmio will return an error if values
> 145 * are invalid so there is no need to check them here.
> 146 */
> 147 ret = of_property_read_u32(np, "reg-io-width", ®_io_width);
> 148 if (ret)
> 149 reg_io_width = 4;
> 150
> 151 ret = of_hwspin_lock_get_id(np, 0);
> 152 if (ret > 0 || (IS_ENABLED(CONFIG_HWSPINLOCK) && ret == 0)) {
> 153 syscon_config.use_hwlock = true;
> 154 syscon_config.hwlock_id = ret;
> 155 syscon_config.hwlock_mode = HWLOCK_IRQSTATE;
> 156 } else if (ret < 0) {
> 157 switch (ret) {
> 158 case -ENOENT:
> 159 /* Ignore missing hwlock, it's optional. */
> 160 break;
> 161 default:
> 162 pr_err("Failed to retrieve valid hwlock: %d\n", ret);
> 163 /* fall-through */
> 164 case -EPROBE_DEFER:
> 165 goto err_regmap;
> 166 }
> 167 }
> 168
> 169 syscon_config.name = of_node_full_name(np);
> 170 syscon_config.reg_stride = reg_io_width;
> 171 syscon_config.val_bits = reg_io_width * 8;
> 172 syscon_config.max_register = resource_size(&res) - reg_io_width;
> 173
> 174 /*
> 175 * The Spreadtrum syscon need register a real physical regmap bus
> 176 * with new atomic bits updating operation instead of using
> 177 * read-modify-write.
> 178 */
> 179 if (IS_ENABLED(CONFIG_ARCH_SPRD) &&
> 180 of_device_is_compatible(np, "sprd,atomic-syscon")) {
> 181 use_phy_regmap_bus = true;
> > 182 regmap = regmap_init(NULL, &sprd_syscon_regmap_bus, base,
> 183 &syscon_config);
> 184 } else {
> 185 regmap = regmap_init_mmio(NULL, base, &syscon_config);
> 186 }
> 187 if (IS_ERR(regmap)) {
> 188 pr_err("regmap init failed\n");
> 189 ret = PTR_ERR(regmap);
> 190 goto err_regmap;
> 191 }
> 192
> 193 if (!use_phy_regmap_bus && check_clk) {
> 194 clk = of_clk_get(np, 0);
> 195 if (IS_ERR(clk)) {
> 196 ret = PTR_ERR(clk);
> 197 /* clock is optional */
> 198 if (ret != -ENOENT)
> 199 goto err_clk;
> 200 } else {
> 201 ret = regmap_mmio_attach_clk(regmap, clk);
> 202 if (ret)
> 203 goto err_attach;
> 204 }
> 205 }
> 206
> 207 syscon->regmap = regmap;
> 208 syscon->np = np;
> 209
> 210 spin_lock(&syscon_list_slock);
> 211 list_add_tail(&syscon->list, &syscon_list);
> 212 spin_unlock(&syscon_list_slock);
> 213
> 214 return syscon;
> 215
> 216 err_attach:
> 217 if (!IS_ERR(clk))
> 218 clk_put(clk);
> 219 err_clk:
> 220 regmap_exit(regmap);
> 221 err_regmap:
> 222 iounmap(base);
> 223 err_map:
> 224 kfree(syscon);
> 225 return ERR_PTR(ret);
> 226 }
> 227
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/[email protected]
--
Baolin Wang