Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1559963

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

From Jan Kiszka <jan.kiszka@siemens.com>
Newsgroups linux.kernel
Subject Re: [PATCH 1/2] spi: pxa2xx: Prepare for edge-triggered interrupts
Date 2017-01-16 19:30 +0100
Message-ID <t0kcy-4hf-7@gated-at.bofh.it> (permalink)
References <t0bsB-6yd-11@gated-at.bofh.it> <t0bsD-6yd-57@gated-at.bofh.it> <t0bLX-6Q2-1@gated-at.bofh.it> <t0duq-85a-17@gated-at.bofh.it> <t0jJv-3Hq-9@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 2017-01-16 18:53, Andy Shevchenko wrote:
> On Mon, Jan 16, 2017 at 1:18 PM, Jan Kiszka <jan.kiszka@siemens.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.

OK, but I'll factor out only the handling block, ie. after the if.
That's more consistent.

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

Data-mined this from the list, both typical target group as well as
Jarkko's address that he tends to use. Will adjust.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

[PATCH 0/2] spi: pca2xx: Prepare for and enable MSI support Jan Kiszka <jan.kiszka@siemens.com> - 2017-01-16 10:10 +0100
  [PATCH 1/2] spi: pxa2xx: Prepare for edge-triggered interrupts Jan Kiszka <jan.kiszka@siemens.com> - 2017-01-16 10:10 +0100
    Re: [PATCH 1/2] spi: pxa2xx: Prepare for edge-triggered interrupts Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2017-01-16 10:30 +0100
      Re: [PATCH 1/2] spi: pxa2xx: Prepare for edge-triggered interrupts Jan Kiszka <jan.kiszka@siemens.com> - 2017-01-16 12:20 +0100
        Re: [PATCH 1/2] spi: pxa2xx: Prepare for edge-triggered interrupts Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-01-16 19:00 +0100
          Re: [PATCH 1/2] spi: pxa2xx: Prepare for edge-triggered interrupts Jan Kiszka <jan.kiszka@siemens.com> - 2017-01-16 19:30 +0100

csiph-web