On 2019-02-18 11:20, Pankaj Bansal wrote: > Hi Peter, > >> -----Original Message----- >> From: Peter Rosin [mailto:p...@axentia.se] >> Sent: Monday, 18 February, 2019 03:17 PM >> To: Pankaj Bansal <pankaj.ban...@nxp.com>; Leo Li <leoyang...@nxp.com>; >> linux-kernel@vger.kernel.org; Philipp Zabel <p.za...@pengutronix.de> >> Subject: Re: [PATCH] drivers: mux: Generic register bitfield-based >> multiplexer >> driver >> >> Hi! >> >> On 2019-02-18 06:40, Pankaj Bansal wrote: >>> Generic register bitfield-based multiplexer driver that controls the >>> multiplexer producer defined under a parent node. >>> The driver corresponding to parent node provides register read/write >>> capabilities. >> >> This driver is just a rename of drivers/mux/mmio.c with a one-liner on top. >> And >> there's a license change too. That's obnoxious. Please keep this as GPL v2. >> Not >> that I /think/ Philipp nor Pengutronix cares deeply, but what do I know? >> Changing the license as you copy the code is simply not all right. > > My Apologies. I will fix it as I send V2. > >> >> Anyway, I would prefer if you could extend drivers/mux/mmio.c to support both >> compatibles, and using the compatible to select if >> >> regmap = syscon_node_to_regmap(np->parent); >> >> or >> >> regmap = dev_get_regmap(dev->parent, NULL); >> >> is called to get to the desired regmap. > > This can be done. The name mmio.c however suggests that mux is controlled by > a Memory mapped device. > IMO, if the generic regmap API is to be added to it, the name needs to > changed. Any suggestions ?
Just keep the driver name as is, there is no shortage of drivers supporting more than one thing... Cheers, Peter >> >> Philipp, you don't object to extending the mmio driver, right? >> >> Or are there more differences that I failed to notice? > > Nope, it's the only difference. > >> >> Cheers, >> Peter >> >>> >>> Signed-off-by: Pankaj Bansal <pankaj.ban...@nxp.com> >>> --- >>> >>> Notes: >>> Dependencies: >>> - >>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat >>> >> chwork.ozlabs.org%2Fpatch%2F1043790%2F&data=02%7C01%7Cpankaj.b >> ansa >>> >> l%40nxp.com%7C3b8a1e8520ce4345105108d695861210%7C686ea1d3bc2b4c6f >> a92cd >>> >> 99c5c301635%7C0%7C0%7C636860800396857042&sdata=5e%2BhyasYwf >> uc%2Fbr >>> 1u6WKlKybYipz5c4ndBeLZDflHqk%3D&reserved=0 >>> >>> drivers/mux/Kconfig | 13 ++++ >>> drivers/mux/Makefile | 2 + >>> drivers/mux/regmap.c | 139 >> +++++++++++++++++++++++++++++++++++++++++ >>> 3 files changed, 154 insertions(+) >>> >>> diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig index >>> 7659d6c5f718..a412d0955258 100644 >>> --- a/drivers/mux/Kconfig >>> +++ b/drivers/mux/Kconfig >>> @@ -58,4 +58,17 @@ config MUX_MMIO >>> To compile the driver as a module, choose M here: the module will >>> be called mux-mmio. >>> >>> +config MUX_REGMAP >>> + tristate "Regmap register bitfield-controlled Multiplexer" >>> + depends on (OF && REGMAP) || COMPILE_TEST >>> + help >>> + Regmap register bitfield-controlled Multiplexer controller. >>> + >>> + The driver builds multiplexer controllers for bitfields in a regmap >>> + device register. For N bit wide bitfields, there will be 2^N possible >>> + multiplexer states. >>> + >>> + To compile the driver as a module, choose M here: the module will >>> + be called regmap-mmio. >>> + >>> endmenu >>> diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile index >>> 6e9fa47daf56..ae1bafac0cbd 100644 >>> --- a/drivers/mux/Makefile >>> +++ b/drivers/mux/Makefile >>> @@ -8,9 +8,11 @@ mux-adg792a-objs := adg792a.o >>> mux-adgs1408-objs := adgs1408.o >>> mux-gpio-objs := gpio.o >>> mux-mmio-objs := mmio.o >>> +mux-regmap-objs := regmap.o >>> >>> obj-$(CONFIG_MULTIPLEXER) += mux-core.o >>> obj-$(CONFIG_MUX_ADG792A) += mux-adg792a.o >>> obj-$(CONFIG_MUX_ADGS1408) += mux-adgs1408.o >>> obj-$(CONFIG_MUX_GPIO) += mux-gpio.o >>> obj-$(CONFIG_MUX_MMIO) += mux-mmio.o >>> +obj-$(CONFIG_MUX_REGMAP) += mux-regmap.o >>> diff --git a/drivers/mux/regmap.c b/drivers/mux/regmap.c new file mode >>> 100644 index 000000000000..c2156302929a >>> --- /dev/null >>> +++ b/drivers/mux/regmap.c >>> @@ -0,0 +1,139 @@ >>> +// SPDX-License-Identifier: GPL-2.0+ >>> +/* >>> + * Regmap register bitfield-controlled multiplexer driver >>> + * >>> + * Based on drivers/mux/mmio.c >>> + * >>> + * Copyright 2019 NXP >>> + */ >>> + >>> +#include <linux/bitops.h> >>> +#include <linux/err.h> >>> +#include <linux/module.h> >>> +#include <linux/mux/driver.h> >>> +#include <linux/of_platform.h> >>> +#include <linux/platform_device.h> >>> +#include <linux/property.h> >>> +#include <linux/regmap.h> >>> + >>> +static int mux_regmap_set(struct mux_control *mux, int state) { >>> + struct regmap_field **fields = mux_chip_priv(mux->chip); >>> + >>> + return regmap_field_write(fields[mux_control_get_index(mux)], >>> +state); } >>> + >>> +static const struct mux_control_ops mux_regmap_ops = { >>> + .set = mux_regmap_set, >>> +}; >>> + >>> +static const struct of_device_id mux_regmap_dt_ids[] = { >>> + { .compatible = "reg-mux", }, >>> + { /* sentinel */ } >>> +}; >>> +MODULE_DEVICE_TABLE(of, mux_regmap_dt_ids); >>> + >>> +static int mux_regmap_probe(struct platform_device *pdev) { >>> + struct device *dev = &pdev->dev; >>> + struct device_node *np = dev->of_node; >>> + struct regmap_field **fields; >>> + struct mux_chip *mux_chip; >>> + struct regmap *regmap; >>> + int num_fields; >>> + int ret; >>> + int i; >>> + >>> + regmap = dev_get_regmap(dev->parent, NULL); >>> + if (IS_ERR(regmap)) { >>> + ret = PTR_ERR(regmap); >>> + dev_err(dev, "failed to get regmap: %d\n", ret); >>> + return ret; >>> + } >>> + >>> + ret = of_property_count_u32_elems(np, "mux-reg-masks"); >>> + if (ret == 0 || ret % 2) >>> + ret = -EINVAL; >>> + if (ret < 0) { >>> + dev_err(dev, "mux-reg-masks property missing or invalid: %d\n", >>> + ret); >>> + return ret; >>> + } >>> + num_fields = ret / 2; >>> + >>> + mux_chip = devm_mux_chip_alloc(dev, num_fields, num_fields * >>> + sizeof(*fields)); >>> + if (IS_ERR(mux_chip)) >>> + return PTR_ERR(mux_chip); >>> + >>> + fields = mux_chip_priv(mux_chip); >>> + >>> + for (i = 0; i < num_fields; i++) { >>> + struct mux_control *mux = &mux_chip->mux[i]; >>> + struct reg_field field; >>> + s32 idle_state = MUX_IDLE_AS_IS; >>> + u32 reg, mask; >>> + int bits; >>> + >>> + ret = of_property_read_u32_index(np, "mux-reg-masks", >>> + 2 * i, ®); >>> + if (!ret) >>> + ret = of_property_read_u32_index(np, "mux-reg- >> masks", >>> + 2 * i + 1, &mask); >>> + if (ret < 0) { >>> + dev_err(dev, "bitfield %d: failed to read mux-reg-masks >> property: %d\n", >>> + i, ret); >>> + return ret; >>> + } >>> + >>> + field.reg = reg; >>> + field.msb = fls(mask) - 1; >>> + field.lsb = ffs(mask) - 1; >>> + >>> + if (mask != GENMASK(field.msb, field.lsb)) { >>> + dev_err(dev, "bitfield %d: invalid mask 0x%x\n", >>> + i, mask); >>> + return -EINVAL; >>> + } >>> + >>> + fields[i] = devm_regmap_field_alloc(dev, regmap, field); >>> + if (IS_ERR(fields[i])) { >>> + ret = PTR_ERR(fields[i]); >>> + dev_err(dev, "bitfield %d: failed allocate: %d\n", >>> + i, ret); >>> + return ret; >>> + } >>> + >>> + bits = 1 + field.msb - field.lsb; >>> + mux->states = 1 << bits; >>> + >>> + of_property_read_u32_index(np, "idle-states", i, >>> + (u32 *)&idle_state); >>> + if (idle_state != MUX_IDLE_AS_IS) { >>> + if (idle_state < 0 || idle_state >= mux->states) { >>> + dev_err(dev, "bitfield: %d: out of range idle >> state %d\n", >>> + i, idle_state); >>> + return -EINVAL; >>> + } >>> + >>> + mux->idle_state = idle_state; >>> + } >>> + } >>> + >>> + mux_chip->ops = &mux_regmap_ops; >>> + >>> + return devm_mux_chip_register(dev, mux_chip); } >>> + >>> +static struct platform_driver mux_regmap_driver = { >>> + .driver = { >>> + .name = "regmap-mux", >>> + .of_match_table = of_match_ptr(mux_regmap_dt_ids), >>> + }, >>> + .probe = mux_regmap_probe, >>> +}; >>> +module_platform_driver(mux_regmap_driver); >>> + >>> +MODULE_DESCRIPTION("Regmap register bitfield-controlled multiplexer >>> +driver"); MODULE_AUTHOR("Pankaj Bansal <pankaj.ban...@nxp.com>"); >>> +MODULE_LICENSE("GPL"); >>> >