linux-kernel - Re: [PATCH V1] input: use device managed memory in da9052 touchscreen driver

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: <20140109191236.GA28020@core.coreip.homeip.net>
Date:	Thu, 9 Jan 2014 11:12:36 -0800
From:	Dmitry Torokhov <dmitry.torokhov@...il.com>
To:	Anthony Olech <anthony.olech.opensource@...semi.com>
Cc:	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
	Huqiu Liu <liuhq11@...ls.tsinghua.edu.cn>,
	David Dajun Chen <david.chen@...semi.com>
Subject: Re: [PATCH V1] input: use device managed memory in da9052
 touchscreen driver

Hi Anthony,

On Thu, Jan 09, 2014 at 12:51:37PM +0000, Anthony Olech wrote:
> The touchscreen component driver for the da9052/3 Dialog PMICs
> is changed to use device managed memory allocation.
> 
> This results in simpler error paths as failures in the probe()
> function do not require explicit calls to free the devm_...
> allocated memory.
> The allocation functions used in this driver are:
>     devm_kzalloc()
>     devm_input_allocate_device()
>     devm_request_threaded_irq()
> 
> Suggested-by: Huqiu Liu <liuhq11@...ls.tsinghua.edu.cn>
> Signed-off-by: Anthony Olech <anthony.olech.opensource@...semi.com>
> ---
> This patch is relative to linux-next repository tag next-20140109
> 
> Many thanks to Huqiu Liu who instigated this patch.
> 
>  drivers/input/touchscreen/da9052_tsi.c |   62 ++++++++++++++++----------------
>  1 file changed, 30 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/da9052_tsi.c b/drivers/input/touchscreen/da9052_tsi.c
> index ab64d58..dcc4cf1 100644
> --- a/drivers/input/touchscreen/da9052_tsi.c
> +++ b/drivers/input/touchscreen/da9052_tsi.c
> @@ -233,18 +233,30 @@ static int da9052_ts_probe(struct platform_device *pdev)
>  	struct da9052_tsi *tsi;
>  	struct input_dev *input_dev;
>  	int error;
> +	int pdown_irq;
> +	int ready_irq;
>  
>  	da9052 = dev_get_drvdata(pdev->dev.parent);
>  	if (!da9052)
>  		return -EINVAL;
>  
> -	tsi = kzalloc(sizeof(struct da9052_tsi), GFP_KERNEL);
> -	input_dev = input_allocate_device();
> -	if (!tsi || !input_dev) {
> -		error = -ENOMEM;
> -		goto err_free_mem;
> -	}
> +	pdown_irq = regmap_irq_get_virq(da9052->irq_data, DA9052_IRQ_PENDOWN);
> +	if (pdown_irq < 0)
> +		return pdown_irq;
...
>  
> -	error = da9052_request_irq(tsi->da9052, DA9052_IRQ_PENDOWN,
> -				"pendown-irq", da9052_ts_pendwn_irq, tsi);
> +	error = devm_request_threaded_irq(&pdev->dev, pdown_irq,
> +				NULL, da9052_ts_pendwn_irq,
> +				IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> +				"pendown-irq", tsi);

I am uncomfortable with the touchscreen portion of this driver ignoring
the framework of it's MFD core and mixing native IRQ management with the
ones done through the core.

What would happen if somebody changes da9052_request_irq() to do some
thing more than it is doing now so that your open-coded duplicate of the
same in da9052_ts_probe() is no longer equivalent? Or
da9052_disable_irq() no longer works correctly with IRQs allocated by
this sub-module?

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/