2019-02-21 12:49:22

by Pankaj Bansal

[permalink] [raw]
Subject: [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 <[email protected]>
---

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



2019-02-21 13:43:10

by Peter Rosin

[permalink] [raw]
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 <[email protected]>
> ---
>
> 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);
>

2019-02-21 14:18:08

by Pankaj Bansal

[permalink] [raw]
Subject: RE: [PATCH v2] drivers: mux: Add Generic regmap bitfield-based multiplexer in mmio-mux



> -----Original Message-----
> From: Peter Rosin [mailto:[email protected]]
> Sent: Thursday, 21 February, 2019 07:12 PM
> To: Pankaj Bansal <[email protected]>; Leo Li <[email protected]>
> Cc: [email protected]
> 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 <[email protected]>
> > ---
> >
> > 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);
> >

2019-02-21 16:07:35

by Peter Rosin

[permalink] [raw]
Subject: 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:[email protected]]
>> Sent: Thursday, 21 February, 2019 07:12 PM
>> To: Pankaj Bansal <[email protected]>; Leo Li <[email protected]>
>> Cc: [email protected]
>> 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 <[email protected]>
>>> ---
>>>
>>> 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);
>>>
>