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

[Peter Rosin]

On 2019-02-24 09:27, 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@xxxxxxx>
> ---
> 
> Notes:
>     V3:
>     - Added the patch in series with device tree binding patch
>     - Added the NULL return handling for regmap
>     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  | 10 +++++++---
>  2 files changed, 13 insertions(+), 9 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..cc02155e4644 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,9 +44,12 @@ static int mux_mmio_probe(struct platform_device *pdev)
>  	int ret;
>  	int i;
>  
> -	regmap = syscon_node_to_regmap(np->parent);
> -	if (IS_ERR(regmap)) {
> -		ret = PTR_ERR(regmap);
> +	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_OR_NULL(regmap)) {
> +		ret = PTR_ERR_OR_ZERO(regmap) ? PTR_ERR(regmap) : -ENODEV;

The above is not correct, this should be better (untested):

		ret = PTR_ERR(regmap) ?: -ENODEV;

Cheers,
Peter

>  		dev_err(dev, "failed to get regmap: %d\n", ret);
>  		return ret;
>  	}
>