> -----Original Message----- > From: Peter Rosin [mailto:p...@axentia.se] > Sent: Thursday, 21 February, 2019 07:12 PM > To: Pankaj Bansal <pankaj.ban...@nxp.com>; Leo Li <leoyang...@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.ban...@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 ? > > Cheers, > Peter > > > if (IS_ERR(regmap)) { > > ret = PTR_ERR(regmap); > > dev_err(dev, "failed to get regmap: %d\n", ret); > >