| Message ID | 1450707546-15060-4-git-send-email-thomas.petazzoni@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Thomas, On lun., déc. 21 2015, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > In order to make the output of /proc/interrupts, use shorter names for ^ in order to make it what ?---| > the irq_chip registered by the irq-armada-370-xp driver. Using capital > letters also matches better what is done for the GIC driver, which > uses just "GIC" as the irq_chip->name. > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > --- > drivers/irqchip/irq-armada-370-xp.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c > index 304166b..31a183d 100644 > --- a/drivers/irqchip/irq-armada-370-xp.c > +++ b/drivers/irqchip/irq-armada-370-xp.c > @@ -117,7 +117,7 @@ static void armada_370_xp_irq_unmask(struct irq_data *d) > #ifdef CONFIG_PCI_MSI > > static struct irq_chip armada_370_xp_msi_irq_chip = { > - .name = "armada_370_xp_msi_irq", > + .name = "MSI MPIC", > .irq_enable = pci_msi_unmask_irq, > .irq_disable = pci_msi_mask_irq, > .irq_mask = pci_msi_mask_irq, > @@ -144,7 +144,7 @@ static int armada_370_xp_msi_set_affinity(struct irq_data *irq_data, > } > > static struct irq_chip armada_370_xp_msi_bottom_irq_chip = { > - .name = "MPIC MSI", > + .name = "MSI MPIC", > .irq_compose_msi_msg = armada_370_xp_compose_msi_msg, > .irq_set_affinity = armada_370_xp_msi_set_affinity, > }; > @@ -256,7 +256,7 @@ static int armada_xp_set_affinity(struct irq_data *d, > #endif > > static struct irq_chip armada_370_xp_irq_chip = { > - .name = "armada_370_xp_irq", > + .name = "MPIC", MPIC is the name also used by the power PC interrupt controller, so it would be confusing to use exactly the same name. What about calling it "MRVL MPIC" or "MVEBU MPIC"? the name remains short but it won't be confused with the power PC ones. Gregory
Gregory, Thanks for your feedback! On Wed, 23 Dec 2015 12:23:19 +0100, Gregory CLEMENT wrote: > > static struct irq_chip armada_370_xp_irq_chip = { > > - .name = "armada_370_xp_irq", > > + .name = "MPIC", > > MPIC is the name also used by the power PC interrupt controller, so it > would be confusing to use exactly the same name. Not really: on a given system, you won't have the PowerPC interrupt controller and the Marvell interrupt controller. For example, for the ARM GIC, /proc/interrupts only shows GIC-0, not "ARM GIC-0". > What about calling it "MRVL MPIC" or "MVEBU MPIC"? > the name remains short but it won't be confused with the power PC ones. Those names are still too long for a nice /proc/interrupts output: # cat /proc/interrupts CPU0 CPU1 17: 2878 2726 GIC-0 29 Edge twd 18: 0 0 MRVL MPIC 5 Level armada_370_xp_per_cpu_tick 21: 142 0 GIC-0 34 Level mv64xxx_i2c 22: 235 0 GIC-0 44 Level serial 37: 0 0 GIC-0 50 Level ehci_hcd:usb1 41: 0 0 GIC-0 53 Level f10a3800.rtc 42: 0 0 GIC-0 58 Level ahci-mvebu[f10a8000.sata] 43: 0 0 GIC-0 60 Level ahci-mvebu[f10e0000.sata] 44: 1040 0 GIC-0 57 Level mmc0 45: 0 0 GIC-0 48 Level xhci-hcd:usb2 46: 0 0 GIC-0 49 Level xhci-hcd:usb4 108: 2 0 GIC-0 54 Level f1060800.xor 109: 2 0 GIC-0 97 Level f1060900.xor 110: 2 0 MRVL MSI MPIC 524288 Edge eth0 Of course that's really a minor detail, but I don't think it's worth making those names longer than "MSI MPIC" and "MPIC". Thanks! Thomas
Hi Thomas, On mar., janv. 26 2016, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Gregory, > > Thanks for your feedback! > > On Wed, 23 Dec 2015 12:23:19 +0100, Gregory CLEMENT wrote: > >> > static struct irq_chip armada_370_xp_irq_chip = { >> > - .name = "armada_370_xp_irq", >> > + .name = "MPIC", >> >> MPIC is the name also used by the power PC interrupt controller, so it >> would be confusing to use exactly the same name. > > Not really: on a given system, you won't have the PowerPC interrupt > controller and the Marvell interrupt controller. For example, for the Do not underestimate the creativity of the hardware designers :) But I agree that it is very unlikely. > ARM GIC, /proc/interrupts only shows GIC-0, not "ARM GIC-0". > >> What about calling it "MRVL MPIC" or "MVEBU MPIC"? >> the name remains short but it won't be confused with the power PC ones. > > Those names are still too long for a nice /proc/interrupts output: > > # cat /proc/interrupts > CPU0 CPU1 > 17: 2878 2726 GIC-0 29 Edge twd > 18: 0 0 MRVL MPIC 5 Level armada_370_xp_per_cpu_tick > 21: 142 0 GIC-0 34 Level mv64xxx_i2c > 22: 235 0 GIC-0 44 Level serial > 37: 0 0 GIC-0 50 Level ehci_hcd:usb1 > 41: 0 0 GIC-0 53 Level f10a3800.rtc > 42: 0 0 GIC-0 58 Level ahci-mvebu[f10a8000.sata] > 43: 0 0 GIC-0 60 Level ahci-mvebu[f10e0000.sata] > 44: 1040 0 GIC-0 57 Level mmc0 > 45: 0 0 GIC-0 48 Level xhci-hcd:usb2 > 46: 0 0 GIC-0 49 Level xhci-hcd:usb4 > 108: 2 0 GIC-0 54 Level f1060800.xor > 109: 2 0 GIC-0 97 Level f1060900.xor > 110: 2 0 MRVL MSI MPIC 524288 Edge eth0 > OK with this output I see your point, so let's move to MPIC. Thanks, Gregory > Of course that's really a minor detail, but I don't think it's worth > making those names longer than "MSI MPIC" and "MPIC". > > Thanks! > > Thomas > -- > Thomas Petazzoni, CTO, Free Electrons > Embedded Linux, Kernel and Android engineering > http://free-electrons.com
diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c index 304166b..31a183d 100644 --- a/drivers/irqchip/irq-armada-370-xp.c +++ b/drivers/irqchip/irq-armada-370-xp.c @@ -117,7 +117,7 @@ static void armada_370_xp_irq_unmask(struct irq_data *d) #ifdef CONFIG_PCI_MSI static struct irq_chip armada_370_xp_msi_irq_chip = { - .name = "armada_370_xp_msi_irq", + .name = "MSI MPIC", .irq_enable = pci_msi_unmask_irq, .irq_disable = pci_msi_mask_irq, .irq_mask = pci_msi_mask_irq, @@ -144,7 +144,7 @@ static int armada_370_xp_msi_set_affinity(struct irq_data *irq_data, } static struct irq_chip armada_370_xp_msi_bottom_irq_chip = { - .name = "MPIC MSI", + .name = "MSI MPIC", .irq_compose_msi_msg = armada_370_xp_compose_msi_msg, .irq_set_affinity = armada_370_xp_msi_set_affinity, }; @@ -256,7 +256,7 @@ static int armada_xp_set_affinity(struct irq_data *d, #endif static struct irq_chip armada_370_xp_irq_chip = { - .name = "armada_370_xp_irq", + .name = "MPIC", .irq_mask = armada_370_xp_irq_mask, .irq_mask_ack = armada_370_xp_irq_mask, .irq_unmask = armada_370_xp_irq_unmask,
In order to make the output of /proc/interrupts, use shorter names for the irq_chip registered by the irq-armada-370-xp driver. Using capital letters also matches better what is done for the GIC driver, which uses just "GIC" as the irq_chip->name. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- drivers/irqchip/irq-armada-370-xp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)