Re: [PATCH v2] drivers: mux: Add Generic regmap bitfield-based multiplexer in mmio-mux
On 2019-02-21 15:17, Pankaj Bansal wrote: > > >> -Original Message- >> From: Peter Rosin [mailto:p...@axentia.se] >> Sent: Thursday, 21 February, 2019 07:12 PM >> To: Pankaj Bansal ; Leo Li >> 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 >>> --- >>> >>> 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); >>> >
RE: [PATCH v2] drivers: mux: Add Generic regmap bitfield-based multiplexer in mmio-mux
> -Original Message- > From: Peter Rosin [mailto:p...@axentia.se] > Sent: Thursday, 21 February, 2019 07:12 PM > To: Pankaj Bansal ; Leo Li > 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 > > --- > > > > 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); > >
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 > --- > > 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? Cheers, Peter > if (IS_ERR(regmap)) { > ret = PTR_ERR(regmap); > dev_err(dev, "failed to get regmap: %d\n", ret); >
[PATCH v2] drivers: mux: Add Generic regmap bitfield-based multiplexer in mmio-mux
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 --- 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); if (IS_ERR(regmap)) { ret = PTR_ERR(regmap); dev_err(dev, "failed to get regmap: %d\n", ret); -- 2.17.1