Messages in this thread | | | From | Peter Rosin <> | Subject | Re: [PATCH v2] drivers: mux: Add Generic regmap bitfield-based multiplexer in mmio-mux | Date | Thu, 21 Feb 2019 16:06:36 +0000 |
| |
On 2019-02-21 15:17, Pankaj Bansal wrote: > > >> -----Original Message----- >> From: Peter Rosin [mailto:peda@axentia.se] >> Sent: Thursday, 21 February, 2019 07:12 PM >> To: Pankaj Bansal <pankaj.bansal@nxp.com>; Leo Li <leoyang.li@nxp.com> >> Cc: linux-kernel@vger.kernel.org >> Subject: Re: [PATCH v2] drivers: mux: Add Generic regmap bitfield-based >> multiplexer in mmio-mux >> >> Hi! >> >> Much better, thanks! One nit below: >> >> On 2019-02-21 13:48, Pankaj Bansal wrote: >>> Generic register bitfield-based multiplexer that controls the >>> multiplexer producer defined under a parent node. >>> The driver corresponding to parent node provides register read/write >>> capabilities. >>> >>> Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com> >>> --- >>> >>> Notes: >>> V2: >>> - removed seperate driver regmap.c and added the regmap function in >> mmio.c >>> based on compatible field, the syscon or regmap function would be called >>> - Modified the KConfig as per Peter's comments >>> >>> drivers/mux/Kconfig | 12 ++++++------ drivers/mux/mmio.c | 6 >>> +++++- >>> 2 files changed, 11 insertions(+), 7 deletions(-) >>> >>> diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig index >>> 7659d6c5f718..e5c571fd232c 100644 >>> --- a/drivers/mux/Kconfig >>> +++ b/drivers/mux/Kconfig >>> @@ -46,14 +46,14 @@ config MUX_GPIO >>> be called mux-gpio. >>> >>> config MUX_MMIO >>> - tristate "MMIO register bitfield-controlled Multiplexer" >>> - depends on (OF && MFD_SYSCON) || COMPILE_TEST >>> + tristate "MMIO/Regmap register bitfield-controlled Multiplexer" >>> + depends on OF || COMPILE_TEST >>> help >>> - MMIO register bitfield-controlled Multiplexer controller. >>> + MMIO/Regmap register bitfield-controlled Multiplexer controller. >>> >>> - The driver builds multiplexer controllers for bitfields in a syscon >>> - register. For N bit wide bitfields, there will be 2^N possible >>> - multiplexer states. >>> + The driver builds multiplexer controllers for bitfields in either >>> + a syscon register or a driver regmap 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 mux-mmio. >>> diff --git a/drivers/mux/mmio.c b/drivers/mux/mmio.c index >>> 935ac44aa209..37fbcde7f1fc 100644 >>> --- a/drivers/mux/mmio.c >>> +++ b/drivers/mux/mmio.c >>> @@ -28,6 +28,7 @@ static const struct mux_control_ops mux_mmio_ops = { >>> >>> static const struct of_device_id mux_mmio_dt_ids[] = { >>> { .compatible = "mmio-mux", }, >>> + { .compatible = "reg-mux", }, >>> { /* sentinel */ } >>> }; >>> MODULE_DEVICE_TABLE(of, mux_mmio_dt_ids); @@ -43,7 +44,10 @@ >> static >>> int mux_mmio_probe(struct platform_device *pdev) >>> int ret; >>> int i; >>> >>> - regmap = syscon_node_to_regmap(np->parent); >>> + if (of_device_is_compatible(np, "mmio-mux")) >>> + regmap = syscon_node_to_regmap(np->parent); >>> + else >>> + regmap = dev_get_regmap(dev->parent, NULL); >> >> dev_get_regmap() returns NULL on failure, so I think you need something like: >> >> regmap = dev_get_regmap(dev->parent, NULL) ?: ERR_PTR(- >> EINVAL); >> >> Or perhaps -ENODEV, -ENOTSUPP or something, not sure? > > How about IS_ERR_OR_NULL just below this, where right now IS_ERR is being used ?
I discarded that since you then have to specialize the error handling to not just do "ret = PTR_ERR(regmap);"
But if you'd like to do something other than my minimal fix, then feel free to make a proposal...
Cheers, Peter
> >> >> Cheers, >> Peter >> >>> if (IS_ERR(regmap)) { >>> ret = PTR_ERR(regmap); >>> dev_err(dev, "failed to get regmap: %d\n", ret); >>> >
| |