Re: [PATCH] sm501: Add device property

[Lee Jones <lee.jones@linaro.org>]

On Tue, 28 Jun 2016, Yoshinori Sato wrote:

> This driver have configuration parameter "device" in platform_data.
> But don't have it in devicetree.
> 
> This patch add "device" configuration to devicetree.

This is really ugly.

1. Where are you documenting the binding?
2. Just because it's in platform data, it doesn't mean it lives in DT
3. Does this code even work?
   Won't private_platdata get freed when you leave probe()?
4. Where is 'devices' consumed?

> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> ---
>  drivers/mfd/sm501.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/mfd/sm501.c b/drivers/mfd/sm501.c
> index 65cd0d2..e2e3f9b 100644
> --- a/drivers/mfd/sm501.c
> +++ b/drivers/mfd/sm501.c
> @@ -21,6 +21,7 @@
>  #include <linux/pci.h>
>  #include <linux/i2c-gpio.h>
>  #include <linux/slab.h>
> +#include <linux/of.h>
>  
>  #include <linux/sm501.h>
>  #include <linux/sm501-regs.h>
> @@ -1377,6 +1378,8 @@ static int sm501_plat_probe(struct platform_device *dev)
>  {
>  	struct sm501_devdata *sm;
>  	int ret;
> +	struct sm501_platdata private_platdata;
> +	struct sm501_initdata private_initdata;
>  
>  	sm = kzalloc(sizeof(struct sm501_devdata), GFP_KERNEL);
>  	if (sm == NULL) {
> @@ -1388,6 +1391,12 @@ static int sm501_plat_probe(struct platform_device *dev)
>  	sm->dev = &dev->dev;
>  	sm->pdev_id = dev->id;
>  	sm->platdata = dev_get_platdata(&dev->dev);
> +	if (!sm->platdata) {
> +		of_property_read_u32(dev->dev.of_node, "smi,devices",
> +				     (u32 *)&private_initdata.devices);
> +		private_platdata.init = &private_initdata;
> +		sm->platdata = &private_platdata;
> +	}
>  
>  	ret = platform_get_irq(dev, 0);
>  	if (ret < 0) {

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog