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

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

[Pankaj Bansal]

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);
 	if (IS_ERR(regmap)) {
 		ret = PTR_ERR(regmap);
 		dev_err(dev, "failed to get regmap: %d\n", ret);
-- 
2.17.1

Re: [PATCH v2] drivers: mux: Add Generic regmap bitfield-based multiplexer in mmio-mux

[Peter Rosin]

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?

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

[Pankaj Bansal]

> -----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 ?

> 
> 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

[Peter Rosin]

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);
>>>
>