linux-kernel - Re: [PATCH] sm501: Add device property

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160628074908.GI6720@dell>
Date:	Tue, 28 Jun 2016 08:49:08 +0100
From:	Lee Jones <lee.jones@...aro.org>
To:	Yoshinori Sato <ysato@...rs.sourceforge.jp>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] sm501: Add device property

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@...rs.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