Re: [PATCH 05/35] ARM: davinci: drop irq defines from default_priorites

Sekhar Nori
2019-02-06T14:52:04Z

On 06/02/19 7:02 PM, Bartosz Golaszewski wrote:
> śr., 6 lut 2019 o 14:03 Sekhar Nori <nsekhar@ti.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@baylibre.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@baylibre.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