Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1559963
| 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 |
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 | Next — Previous in thread | Find similar | Unroll 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