Messages in this thread | | | From | Baolin Wang <> | Date | Mon, 27 Apr 2020 15:23:38 +0800 | Subject | Re: [PATCH v3] mfd: syscon: Add Spreadtrum physical regmap bus support |
| |
Hi Arnd and Lee,
On Tue, Apr 21, 2020 at 10:13 PM Baolin Wang <baolin.wang7@gmail.com> wrote: > > 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>
Do you have any comments for this patch? Thanks.
> --- > Changes from v2: > - Fix building errors without enabling CONFIG_ARCH_SPRD. > > 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 | 83 ++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 81 insertions(+), 2 deletions(-) > > diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c > index 3a97816d0cba..ca91b7770e1a 100644 > --- a/drivers/mfd/syscon.c > +++ b/drivers/mfd/syscon.c > @@ -40,6 +40,72 @@ 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; > +} > +#endif > + > +static struct regmap_bus sprd_syscon_regmap_bus = { > +#if IS_ENABLED(CONFIG_ARCH_SPRD) > + .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 +116,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 +173,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 >
-- Baolin Wang
| |