Re: [PATCHv5 1/3] mfd: sec-core: Add support for S2MPU02 device

[Lee Jones]

On Mon, 16 Jun 2014, Chanwoo Choi wrote:

> Add support for Samsung S2MPU02 PMIC device to the MFD sec-core driver.
> The S2MPU02 device includes PMIC/RTC/Clock devices.
> 
> Signed-off-by: Chanwoo Choi <cw00.choi@xxxxxxxxxxx>
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@xxxxxxxxxxx>
> ---
>  drivers/mfd/sec-core.c           | 19 +++++++++
>  drivers/mfd/sec-irq.c            | 88 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/samsung/core.h |  1 +
>  include/linux/mfd/samsung/irq.h  | 24 +++++++++++
>  4 files changed, 132 insertions(+)
> 
> diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
> index be06d0a..5e8784b 100644
> --- a/drivers/mfd/sec-core.c
> +++ b/drivers/mfd/sec-core.c
> @@ -89,6 +89,17 @@ static const struct mfd_cell s2mpa01_devs[] = {
>  	},
>  };
>  
> +static const struct mfd_cell s2mpu02_devs[] = {
> +	{
> +		.name = "s2mpu02-pmic",
> +	}, {
> +		.name = "s2mpu02-rtc",

Make these two one liners, so:

  { .name = "s2mpu02-pmic" },
  { .name = "s2mpu02-rtc" ),

> +	}, {
> +		.name = "s2mpu02-clk",
> +		.of_compatible = "samsung,s2mpu02-clk",
> +	}
> +};
> +
>  #ifdef CONFIG_OF
>  static const struct of_device_id sec_dt_match[] = {
>  	{	.compatible = "samsung,s5m8767-pmic",
> @@ -103,6 +114,9 @@ static const struct of_device_id sec_dt_match[] = {
>  		.compatible = "samsung,s2mpa01-pmic",
>  		.data = (void *)S2MPA01,
>  	}, {
> +		.compatible = "samsung,s2mpu02-pmic",
> +		.data = (void *)S2MPU02,
> +	}, {
>  		/* Sentinel */
>  	},
>  };
> @@ -342,6 +356,11 @@ static int sec_pmic_probe(struct i2c_client *i2c,
>  		ret = mfd_add_devices(sec_pmic->dev, -1, s2mps14_devs,
>  				      ARRAY_SIZE(s2mps14_devs), NULL, 0, NULL);
>  		break;
> +	case S2MPU02:
> +		ret = mfd_add_devices(sec_pmic->dev, -1, s2mpu02_devs,
> +				      ARRAY_SIZE(s2mpu02_devs), NULL, 0, NULL);
> +		break;

This is a bit bonkers.  Instead of passing S2MPU02, why don't you pass
a new container which holds s2mpu02_devs and s2mpu02_irq_chip, then
you won't need new mfd_add_devices() and regmap_add_irq_chip() calls
for every device you want to support.

[...]

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org â Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/