linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] mfd: syscon: Add Spreadtrum physical regmap bus support
@ 2020-04-18  1:53 Baolin Wang
  2020-04-21 10:03 ` kbuild test robot
  0 siblings, 1 reply; 3+ messages in thread
From: Baolin Wang @ 2020-04-18  1:53 UTC (permalink / raw)
  To: lee.jones, arnd
  Cc: broonie, baolin.wang7, orsonzhai, zhang.lyra, linux-kernel

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 <baolin.wang7@gmail.com>
---
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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] mfd: syscon: Add Spreadtrum physical regmap bus support
  2020-04-18  1:53 [PATCH v2] mfd: syscon: Add Spreadtrum physical regmap bus support Baolin Wang
@ 2020-04-21 10:03 ` kbuild test robot
  2020-04-21 13:26   ` Baolin Wang
  0 siblings, 1 reply; 3+ messages in thread
From: kbuild test robot @ 2020-04-21 10:03 UTC (permalink / raw)
  To: Baolin Wang, lee.jones, arnd, broonie, orsonzhai, zhang.lyra,
	linux-kernel
  Cc: kbuild-all, clang-built-linux, broonie, baolin.wang7, orsonzhai,
	zhang.lyra

[-- Attachment #1: Type: text/plain, Size: 6001 bytes --]

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 <lkp@intel.com>

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", &reg_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/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30335 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] mfd: syscon: Add Spreadtrum physical regmap bus support
  2020-04-21 10:03 ` kbuild test robot
@ 2020-04-21 13:26   ` Baolin Wang
  0 siblings, 0 replies; 3+ messages in thread
From: Baolin Wang @ 2020-04-21 13:26 UTC (permalink / raw)
  To: kbuild test robot
  Cc: Lee Jones, Arnd Bergmann, Mark Brown, Orson Zhai, Chunyan Zhang,
	LKML, kbuild-all, clang-built-linux

On Tue, Apr 21, 2020 at 6:03 PM kbuild test robot <lkp@intel.com> 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 <lkp@intel.com>
>
> 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", &reg_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/kbuild-all@lists.01.org



-- 
Baolin Wang

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-04-21 13:26 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-18  1:53 [PATCH v2] mfd: syscon: Add Spreadtrum physical regmap bus support Baolin Wang
2020-04-21 10:03 ` kbuild test robot
2020-04-21 13:26   ` Baolin Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).