linux-kernel - Re: [PATCH 1/2] spi: pxa2xx: Prepare for edge-triggered interrupts

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:   Mon, 16 Jan 2017 19:53:46 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Jan Kiszka <jan.kiszka@...mens.com>
Cc:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        linux-arm Mailing List <linux-arm-kernel@...ts.infradead.org>,
        Daniel Mack <daniel@...que.org>,
        Haojian Zhuang <haojian.zhuang@...il.com>,
        Robert Jarzmik <robert.jarzmik@...e.fr>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Jarkko Nikula <jarkko.nikula@...mer.com>,
        Sascha Weisenberger <sascha.weisenberger@...mens.com>
Subject: Re: [PATCH 1/2] spi: pxa2xx: Prepare for edge-triggered interrupts

On Mon, Jan 16, 2017 at 1:18 PM, Jan Kiszka <jan.kiszka@...mens.com> wrote:
> On 2017-01-16 10:24, Andy Shevchenko wrote:
>> On Mon, 2017-01-16 at 10:05 +0100, Jan Kiszka wrote:
>>> When using the a device with edge-triggered interrupts, such as MSIs,
>>> the interrupt handler has to ensure that there is a point in time
>>> during
>>> its execution where all interrupts sources are silent so that a new
>>> event can trigger a new interrupt again.
>>>
>>> This is achieved here by looping over SSSR evaluation. We need to take
>>> into account that SSCR1 may be changed by the transfer handler, thus
>>> we
>>> need to redo the mask calculation, at least regarding the volatile
>>> interrupt enable bit (TIE).
>>
>> Could you split this to two patches, one just move the code under
>> question to a helper function (no functional change), the other does
>> what you state in commit message here?
>
> IMHO, factoring out some helper called from the loop in ssp_int won't be
> a natural split due to the large number of local variables being shared
> here. But maybe I'm not seeing the design you have in mind, so please
> propose a useful helper function signature.

At least everything starting from if (!...) {} can be a helper with
only one parameter. Something like:

static int handle_bad_msg(struct driver_data *drv_data)
{
  if (...)
    return 0;

  ...handle it...
  return 1;
}

Let's start from above.

P.S. Btw, you totally missed SPI list/maintainers. And you are using
wrong Jarkko's address.

-- 
With Best Regards,
Andy Shevchenko