linux-kernel - Re: [PATCH V9 3/5] i2c: tegra: Add DMA support

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]

Date:   Sun, 3 Feb 2019 19:42:16 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Sowjanya Komatineni <skomatineni@...dia.com>,
        "thierry.reding@...il.com" <thierry.reding@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Mantravadi Karthik <mkarthik@...dia.com>,
        Shardar Mohammed <smohammed@...dia.com>,
        Timo Alho <talho@...dia.com>
Cc:     "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>
Subject: Re: [PATCH V9 3/5] i2c: tegra: Add DMA support

02.02.2019 21:43, Sowjanya Komatineni пишет:
> 
>>>>>> +static int tegra_i2c_init_dma(struct tegra_i2c_dev *i2c_dev) {
>>>>>> +	struct dma_chan *dma_chan;
>>>>>> +	u32 *dma_buf;
>>>>>> +	dma_addr_t dma_phys;
>>>>>> +	int err = 0;
>>>>>> +
>>>>>> +	if (!IS_ENABLED(CONFIG_TEGRA20_APB_DMA))
>>>>>> +		return -ENODEV;
>>>>>
>>>>> Driver shall not fail to probe if DMA driver is disabled, but to continue with the PIO-only mode available.
>>>>>
>>>>> Should be:
>>>>>
>>>>> 	if (!IS_ENABLED(CONFIG_TEGRA20_APB_DMA))
>>>>> 		return 0;
>>>>>
>>>> Except EPROBE_DEFER, anything else returned from tegra_i2c_init_dma 
>>>> (ENODEV/ENOMEM) is ignored in i2c_probe DMA mode decision is based on xfer size and availability of dma channel or can be changed based on valid dma buf to shorten the line.
>>>>
>>>
>>> Ah, sorry. I missed that, seems good then.
>>>
>>
>> BTW, it may be worthwhile to move out the error code handling into tegra_i2c_init_dma() for clarity. It also won't hurt to not ignore errors other than -ENODEV.
> 
> Either ways are same. To be more readable/clear on error types for diff cases and errors that we are ignoring, I am explicitly keeping error codecs (EPROBE_DEFER/ENODEV/ENOMEM) same way.
> Also in probe for readability checking with EPROBE_DEFER which explicitly indicating we keep deferring i2c probe rather than simple return.
> 

Okay, then please add a error message for the dma_chan request-failure, printing a message that tells error code for the case (or even regardless) of errno != -ENODEV.