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


Groups > linux.kernel > #1277679

Re: [PATCH 01/14] DEVICETREE: Add bindings for PIC32 interrupt controller

From Joshua Henderson <joshua.henderson@microchip.com>
Newsgroups linux.kernel
Subject Re: [PATCH 01/14] DEVICETREE: Add bindings for PIC32 interrupt controller
Date 2015-11-25 19:30 +0100
Message-ID <qyMZk-1sg-15@gated-at.bofh.it> (permalink)
References <qx44h-682-3@gated-at.bofh.it> <qx4dY-6bp-23@gated-at.bofh.it> <qxngC-1ZX-15@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 11/21/2015 1:47 PM, Arnd Bergmann wrote:
> On Friday 20 November 2015 17:17:13 Joshua Henderson wrote:
> 
>> +Example
>> +-------
>> +
>> +evic: interrupt-controller@1f810000 {
>> +        compatible = "microchip,evic-v2";
>> +        interrupt-controller;
>> +        #interrupt-cells = <3>;
>> +        reg = <0x1f810000 0x1000>;
>> +        device_type="evic-v2";
>> +};
> 
> This is not a correct use of device_type. Just drop that property.

Ack.

> 
>> diff --git a/include/dt-bindings/interrupt-controller/microchip,pic32mz-evic.h b/include/dt-bindings/interrupt-controller/microchip,pic32mz-evic.h
>> new file mode 100644
>> index 0000000..2c466b8
>> --- /dev/null
>> +++ b/include/dt-bindings/interrupt-controller/microchip,pic32mz-evic.h
>> @@ -0,0 +1,238 @@
>> +/*
>> + * This header provides constants for the MICROCHIP PIC32 EVIC.
>> + */
>> +
>> +#ifndef _DT_BINDINGS_INTERRUPT_CONTROLLER_MICROCHIP_EVIC_H
>> +#define _DT_BINDINGS_INTERRUPT_CONTROLLER_MICROCHIP_EVIC_H
>> +
>> +#include <dt-bindings/interrupt-controller/irq.h>
>> +
>> +/* Hardware interrupt number */
>> +#define CORE_TIMER_INTERRUPT 0
>> +#define CORE_SOFTWARE_INTERRUPT_0 1
>> +#define CORE_SOFTWARE_INTERRUPT_1 2
>> +#define EXTERNAL_INTERRUPT_0 3
>> +#define TIMER1 4
> 
> A header file like this is just going to make everyone's life
> miserable. Try to remove as much as possible here: normally
> you can just use the numbers from the data sheet that match
> the actual hardware registers, and put them into the dts file.
> 

Agreed.  Removing these defines along with removing the priorities from the bindings as suggested makes sense.  With doing that, this header file becomes pointless and it will be dropped.

>> +/* Interrupt priority bits */
>> +#define PRI_0	0	/* Note:This priority disables the interrupt! */
>> +#define PRI_1	1
>> +#define PRI_2	2
>> +#define PRI_3	3
>> +#define PRI_4	4
>> +#define PRI_5	5
>> +#define PRI_6	6
>> +#define PRI_7	7
> 
>> +/* Interrupt subpriority bits */
>> +#define SUB_PRI_0	0
>> +#define SUB_PRI_1	1
>> +#define SUB_PRI_2	2
>> +#define SUB_PRI_3	3
> 
> These are obviously silly and should be removed/
> 

Ack.

>> +#define PRI_MASK	0x7	/* 3 bit priority mask */
>> +#define SUBPRI_MASK	0x3	/* 2 bit subpriority mask */
>> +#define INT_MASK	0x1F	/* 5 bit pri and subpri mask */
>> +#define NR_EXT_IRQS	5	/* 5 external interrupts sources */
>> +
>> +#define MICROCHIP_EVIC_MIN_PRIORITY 0
>> +#define MICROCHIP_EVIC_MAX_PRIORITY INT_MASK
>> +
>> +#define INT_PRI(pri, subpri)	\
>> +	(((pri & PRI_MASK) << 2) | (subpri & SUBPRI_MASK))
>> +
>> +#define DEFINE_INT(irq, pri) { irq, pri }
>> +
>> +#define DEFAULT_INT_PRI INT_PRI(2, 0)
> 
> Is it required to have a specific priority configured for each line?
> If these are software selectable, it's probably better to not put
> them into DT in the first place.
> 
> If you absolutely need them, I would suggest using two separate cells
> for pri and subpri so you can avoid the macro.
> 

These priorities are hardware priorities that arbitrate pending interrupts to the CPU.  These are indeed software configurable and we can agree that DT is probably not the best place to put this configuration in light of this.  We'll default to something sane instead.  They will be removed from the binding.

Josh

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


Thread

[PATCH 00/14] Initial Microchip PIC32MZDA Support Joshua Henderson <joshua.henderson@microchip.com> - 2015-11-21 01:20 +0100
  [PATCH 09/14] DEVICETREE: Add bindings for PIC32 usart driver Joshua Henderson <joshua.henderson@microchip.com> - 2015-11-21 01:20 +0100
    Re: [PATCH 09/14] DEVICETREE: Add bindings for PIC32 usart driver Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-11-21 16:30 +0100
    Re: [PATCH 09/14] DEVICETREE: Add bindings for PIC32 usart driver Rob Herring <robh@kernel.org> - 2015-11-22 23:00 +0100
  [PATCH 07/14] DEVICETREE: Add bindings for PIC32 pin control and GPIO Joshua Henderson <joshua.henderson@microchip.com> - 2015-11-21 01:20 +0100
    Re: [PATCH 07/14] DEVICETREE: Add bindings for PIC32 pin control and  GPIO Rob Herring <robh@kernel.org> - 2015-11-22 22:50 +0100
  [PATCH 12/14] DEVICETREE: Add bindings for PIC32 SDHC host controller Joshua Henderson <joshua.henderson@microchip.com> - 2015-11-21 01:20 +0100
    Re: [PATCH 12/14] DEVICETREE: Add bindings for PIC32 SDHC host  controller Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-11-21 16:30 +0100
      Re: [PATCH 12/14] DEVICETREE: Add bindings for PIC32 SDHC host  controller Joshua Henderson <joshua.henderson@microchip.com> - 2015-11-26 05:50 +0100
    Re: [PATCH 12/14] DEVICETREE: Add bindings for PIC32 SDHC host  controller Rob Herring <robh@kernel.org> - 2015-11-22 23:00 +0100
      Re: [PATCH 12/14] DEVICETREE: Add bindings for PIC32 SDHC host  controller Joshua Henderson <joshua.henderson@microchip.com> - 2015-11-26 06:20 +0100
  [PATCH 05/14] DEVICETREE: Add bindings for PIC32/MZDA platforms Joshua Henderson <joshua.henderson@microchip.com> - 2015-11-21 01:30 +0100
    Re: [PATCH 05/14] DEVICETREE: Add bindings for PIC32/MZDA platforms Rob Herring <robh@kernel.org> - 2015-11-22 22:40 +0100
  [PATCH 11/14] mmc: sdhci-pic32: Add PIC32 SDHC host controller driver Joshua Henderson <joshua.henderson@microchip.com> - 2015-11-21 01:30 +0100
    Re: [PATCH 11/14] mmc: sdhci-pic32: Add PIC32 SDHC host controller driver Ulf Hansson <ulf.hansson@linaro.org> - 2015-11-25 17:00 +0100
      Re: [PATCH 11/14] mmc: sdhci-pic32: Add PIC32 SDHC host controller driver Ulf Hansson <ulf.hansson@linaro.org> - 2015-11-25 17:40 +0100
      RE: [PATCH 11/14] mmc: sdhci-pic32: Add PIC32 SDHC host controller  driver <Paul.Thacker@microchip.com> - 2015-11-29 21:30 +0100
  [PATCH 13/14] MIPS: dts: Add initial DTS for the PIC32MZDA Starter Kit Joshua Henderson <joshua.henderson@microchip.com> - 2015-11-21 01:30 +0100
  [PATCH 03/14] DEVICETREE: Add PIC32 clock binding documentation Joshua Henderson <joshua.henderson@microchip.com> - 2015-11-21 01:30 +0100
    Re: [PATCH 03/14] DEVICETREE: Add PIC32 clock binding documentation Arnd Bergmann <arnd@arndb.de> - 2015-11-21 21:50 +0100
      Re: [PATCH 03/14] DEVICETREE: Add PIC32 clock binding documentation Joshua Henderson <joshua.henderson@microchip.com> - 2015-11-26 05:10 +0100
    Re: [PATCH 03/14] DEVICETREE: Add PIC32 clock binding documentation Rob Herring <robh@kernel.org> - 2015-11-22 22:40 +0100
      Re: [PATCH 03/14] DEVICETREE: Add PIC32 clock binding documentation Joshua Henderson <joshua.henderson@microchip.com> - 2015-11-26 06:40 +0100
  [PATCH 02/14] irqchip: irq-pic32-evic: Add support for PIC32 interrupt controller Joshua Henderson <joshua.henderson@microchip.com> - 2015-11-21 01:30 +0100
    Re: [PATCH 02/14] irqchip: irq-pic32-evic: Add support for PIC32  interrupt controller Marc Zyngier <marc.zyngier@arm.com> - 2015-11-22 12:50 +0100
      Re: [PATCH 02/14] irqchip: irq-pic32-evic: Add support for PIC32  interrupt controller Joshua Henderson <joshua.henderson@microchip.com> - 2015-11-25 19:30 +0100
  [PATCH 10/14] serial: pic32_uart: Add PIC32 uart driver Joshua Henderson <joshua.henderson@microchip.com> - 2015-11-21 01:30 +0100
  [PATCH 01/14] DEVICETREE: Add bindings for PIC32 interrupt controller Joshua Henderson <joshua.henderson@microchip.com> - 2015-11-21 01:30 +0100
    Re: [PATCH 01/14] DEVICETREE: Add bindings for PIC32 interrupt controller Arnd Bergmann <arnd@arndb.de> - 2015-11-21 21:50 +0100
      Re: [PATCH 01/14] DEVICETREE: Add bindings for PIC32 interrupt  controller Joshua Henderson <joshua.henderson@microchip.com> - 2015-11-25 19:30 +0100
    Re: [PATCH 01/14] DEVICETREE: Add bindings for PIC32 interrupt  controller Rob Herring <robh@kernel.org> - 2015-11-22 22:20 +0100
      Re: [PATCH 01/14] DEVICETREE: Add bindings for PIC32 interrupt  controller Joshua Henderson <joshua.henderson@microchip.com> - 2015-11-26 06:10 +0100
  [PATCH 14/14] MIPS: pic32mzda: Add initial PIC32MZDA Starter Kit defconfig Joshua Henderson <joshua.henderson@microchip.com> - 2015-11-21 01:30 +0100
  Re: [PATCH 06/14] MIPS: Add support for PIC32MZDA platform Alban <albeu@free.fr> - 2015-11-21 13:40 +0100
    Re: [PATCH 06/14] MIPS: Add support for PIC32MZDA platform Joshua Henderson <joshua.henderson@microchip.com> - 2015-11-22 17:30 +0100
  Re: [PATCH 08/14] pinctrl: Add PIC32 pin control driver Linus Walleij <linus.walleij@linaro.org> - 2015-11-30 14:50 +0100

csiph-web