linux-kernel - Re: [PATCH v7 14/34] i2c: tegra: Clean up probe function

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: <20200917123755.GO3515672@ulmo>
Date:   Thu, 17 Sep 2020 14:37:55 +0200
From:   Thierry Reding <thierry.reding@...il.com>
To:     Dmitry Osipenko <digetx@...il.com>
Cc:     Jonathan Hunter <jonathanh@...dia.com>,
        Laxman Dewangan <ldewangan@...dia.com>,
        Wolfram Sang <wsa@...-dreams.de>,
        Michał Mirosław <mirq-linux@...e.qmqm.pl>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        linux-i2c@...r.kernel.org, linux-tegra@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 14/34] i2c: tegra: Clean up probe function

On Wed, Sep 09, 2020 at 01:39:46AM +0300, Dmitry Osipenko wrote:
> The driver's probe function code is a bit difficult to read. This patch
> reorders code of the probe function, forming groups of code that are easy
> to work with. The probe tear-down order now matches the driver-removal
> order.
> 
> Signed-off-by: Dmitry Osipenko <digetx@...il.com>
> ---
>  drivers/i2c/busses/i2c-tegra.c | 100 ++++++++++++++++-----------------
>  1 file changed, 49 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index 79d1cefdc901..7c91bbb3f95c 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -440,6 +440,9 @@ static int tegra_i2c_init_dma(struct tegra_i2c_dev *i2c_dev)
>  
>  	i2c_dev->tx_dma_chan = chan;
>  
> +	i2c_dev->dma_buf_size = i2c_dev->hw->quirks->max_write_len +
> +				I2C_PACKET_HEADER_SIZE;
> +
>  	dma_buf = dma_alloc_coherent(i2c_dev->dev, i2c_dev->dma_buf_size,
>  				     &dma_phys, GFP_KERNEL | __GFP_NOWARN);
>  	if (!dma_buf) {
> @@ -1693,34 +1696,42 @@ static int tegra_i2c_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct tegra_i2c_dev *i2c_dev;
>  	struct resource *res;
> -	void __iomem *base;
> -	phys_addr_t base_phys;
> -	int irq;
>  	int ret;
>  
> -	base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
> -	if (IS_ERR(base))
> -		return PTR_ERR(base);
> -
> -	base_phys = res->start;
> -
> -	irq = platform_get_irq(pdev, 0);
> -	if (irq < 0)
> -		return irq;
> -
>  	i2c_dev = devm_kzalloc(&pdev->dev, sizeof(*i2c_dev), GFP_KERNEL);
>  	if (!i2c_dev)
>  		return -ENOMEM;
>  
> -	i2c_dev->base = base;
> -	i2c_dev->base_phys = base_phys;
> -	i2c_dev->adapter.algo = &tegra_i2c_algo;
> -	i2c_dev->adapter.retries = 1;
> -	i2c_dev->adapter.timeout = 6 * HZ;
> -	i2c_dev->irq = irq;
> +	platform_set_drvdata(pdev, i2c_dev);
> +
> +	init_completion(&i2c_dev->msg_complete);
> +	init_completion(&i2c_dev->dma_complete);
> +
> +	i2c_dev->hw = of_device_get_match_data(&pdev->dev);
>  	i2c_dev->cont_id = pdev->id;
>  	i2c_dev->dev = &pdev->dev;
>  
> +	i2c_dev->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
> +	if (IS_ERR(i2c_dev->base))
> +		return PTR_ERR(i2c_dev->base);
> +
> +	i2c_dev->base_phys = res->start;
> +
> +	ret = platform_get_irq(pdev, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	i2c_dev->irq = ret;
> +
> +	/* interrupt will be enabled during of transfer time */
> +	irq_set_status_flags(i2c_dev->irq, IRQ_NOAUTOEN);
> +
> +	ret = devm_request_irq(&pdev->dev, i2c_dev->irq, tegra_i2c_isr,
> +			       IRQF_NO_SUSPEND, dev_name(&pdev->dev),
> +			       i2c_dev);
> +	if (ret)
> +		return ret;

Is it safe to install the interrupt handler at this point? What if,
perhaps because some bootloader didn't properly quiesce the I2C
controller, an interrupt triggers immediately after this?

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)