linux-kernel - Re: [PATCH 05/35] ARM: davinci: drop irq defines from default

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:   Wed, 6 Feb 2019 20:20:50 +0530
From:   Sekhar Nori <nsekhar@...com>
To:     Bartosz Golaszewski <bgolaszewski@...libre.com>
CC:     David Lechner <david@...hnology.com>,
        Bartosz Golaszewski <brgl@...ev.pl>,
        Kevin Hilman <khilman@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Jason Cooper <jason@...edaemon.net>,
        Marc Zyngier <marc.zyngier@....com>,
        LKML <linux-kernel@...r.kernel.org>,
        arm-soc <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 05/35] ARM: davinci: drop irq defines from
 default_priorites

On 06/02/19 7:02 PM, Bartosz Golaszewski wrote:
> śr., 6 lut 2019 o 14:03 Sekhar Nori <nsekhar@...com> napisał(a):
>>
>> On 05/02/19 3:51 AM, David Lechner wrote:
>>> On 1/31/19 7:38 AM, Bartosz Golaszewski wrote:
>>>> From: Bartosz Golaszewski <bgolaszewski@...libre.com>
>>>>
>>>> In order to select SPARSE_IRQ we need to make the interrupt numbers
>>>> dynamic (at least at build-time for the top-level controller). The
>>>> interrupt numbers are used as array indexes for irq priorities.
>>>>
>>>> Drop the defines and just initialize the arrays in a linear manner.
>>>>
>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@...libre.com>
>>>> ---
>>>
>>> ...
>>>
>>>> -static u8 dm355_default_priorities[DAVINCI_N_AINTC_IRQ] = {
>>>> -    [IRQ_DM355_CCDC_VDINT0]        = 2,
>>>> -    [IRQ_DM355_CCDC_VDINT1]        = 6,
>>>> -    [IRQ_DM355_CCDC_VDINT2]        = 6,
>>>> -    [IRQ_DM355_IPIPE_HST]        = 6,
>>>> -    [IRQ_DM355_H3AINT]        = 6,
>>>> -    [IRQ_DM355_IPIPE_SDR]        = 6,
>>>> -    [IRQ_DM355_IPIPEIFINT]        = 6,
>>>> -    [IRQ_DM355_OSDINT]        = 7,
>>>> -    [IRQ_DM355_VENCINT]        = 6,
>>>> -    [IRQ_ASQINT]            = 6,
>>>> -    [IRQ_IMXINT]            = 6,
>>>> -    [IRQ_USBINT]            = 4,
>>>> -    [IRQ_DM355_RTOINT]        = 4,
>>>> -    [IRQ_DM355_UARTINT2]        = 7,
>>>> -    [IRQ_DM355_TINT6]        = 7,
>>>> -    [IRQ_CCINT0]            = 5,    /* dma */
>>>> -    [IRQ_CCERRINT]            = 5,    /* dma */
>>>> -    [IRQ_TCERRINT0]            = 5,    /* dma */
>>>> -    [IRQ_TCERRINT]            = 5,    /* dma */
>>>> -    [IRQ_DM355_SPINT2_1]        = 7,
>>>> -    [IRQ_DM355_TINT7]        = 4,
>>>> -    [IRQ_DM355_SDIOINT0]        = 7,
>>>> -    [IRQ_MBXINT]            = 7,
>>>> -    [IRQ_MBRINT]            = 7,
>>>> -    [IRQ_MMCINT]            = 7,
>>>> -    [IRQ_DM355_MMCINT1]        = 7,
>>>> -    [IRQ_DM355_PWMINT3]        = 7,
>>>> -    [IRQ_DDRINT]            = 7,
>>>> -    [IRQ_AEMIFINT]            = 7,
>>>> -    [IRQ_DM355_SDIOINT1]        = 4,
>>>> -    [IRQ_TINT0_TINT12]        = 2,    /* clockevent */
>>>> -    [IRQ_TINT0_TINT34]        = 2,    /* clocksource */
>>>> -    [IRQ_TINT1_TINT12]        = 7,    /* DSP timer */
>>>> -    [IRQ_TINT1_TINT34]        = 7,    /* system tick */
>>>> -    [IRQ_PWMINT0]            = 7,
>>>> -    [IRQ_PWMINT1]            = 7,
>>>> -    [IRQ_PWMINT2]            = 7,
>>>> -    [IRQ_I2C]            = 3,
>>>> -    [IRQ_UARTINT0]            = 3,
>>>> -    [IRQ_UARTINT1]            = 3,
>>>> -    [IRQ_DM355_SPINT0_0]        = 3,
>>>> -    [IRQ_DM355_SPINT0_1]        = 3,
>>>> -    [IRQ_DM355_GPIO0]        = 3,
>>>> -    [IRQ_DM355_GPIO1]        = 7,
>>>> -    [IRQ_DM355_GPIO2]        = 4,
>>>> -    [IRQ_DM355_GPIO3]        = 4,
>>>> -    [IRQ_DM355_GPIO4]        = 7,
>>>> -    [IRQ_DM355_GPIO5]        = 7,
>>>> -    [IRQ_DM355_GPIO6]        = 7,
>>>> -    [IRQ_DM355_GPIO7]        = 7,
>>>> -    [IRQ_DM355_GPIO8]        = 7,
>>>> -    [IRQ_DM355_GPIO9]        = 7,
>>>> -    [IRQ_DM355_GPIOBNK0]        = 7,
>>>> -    [IRQ_DM355_GPIOBNK1]        = 7,
>>>> -    [IRQ_DM355_GPIOBNK2]        = 7,
>>>> -    [IRQ_DM355_GPIOBNK3]        = 7,
>>>> -    [IRQ_DM355_GPIOBNK4]        = 7,
>>>> -    [IRQ_DM355_GPIOBNK5]        = 7,
>>>> -    [IRQ_DM355_GPIOBNK6]        = 7,
>>>> -    [IRQ_COMMTX]            = 7,
>>>> -    [IRQ_COMMRX]            = 7,
>>>> -    [IRQ_EMUINT]            = 7,
>>>> +static u8 dm355_aintc_prios[] = {
>>>> +    2, 6, 6, 6, 6, 6, 6, 7,
>>>> +    6, 6, 6, 4, 4, 7, 7, 5,
>>>> +    5, 5, 5, 7, 4, 7, 7, 7,
>>>> +    7, 7, 7, 7, 7, 4, 2, 2,
>>>> +    7, 7, 7, 7, 7, 3, 3, 3,
>>>> +    3, 3, 3, 7, 4, 4, 7, 7,
>>>> +    7, 7, 7, 7, 7, 7, 7, 7,
>>>> +    7, 7, 7, 7, 7, 7, 0, 0,
>>>>   };
>>>
>>> Hmm... this makes it harder to see what is going on here.
>>> You can no longer see which priority corresponds to which
>>> IRQ without consulting a manual.
>>
>> I agree with David here. The interrupt numbers are dynamic, but the
>> interrupt number offset from hardware point-of-view is fixed. So can
>> these macros be re-purposed to represent the hardware offset (eventually
>> you would pass them to DAVINCI_INTC_IRQ())?
>>
>> Thanks,
>> Sekhar
> 
> Should we keep the mach/irqs.h header then? While working on patches
> for supporting the multi_v5_defconfig build I noticed the mach/*
> headers tend to cause build problems in certain drivers that use them.
> Most machines have gotten rid of them. Should we maybe create a local
> header in mach-davinci/?

Yes, we should have a local header in mach-davinci. It should not be in
include/mach/ so it should not be exposed to drivers.

Thanks,
Sekhar