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

Groups > linux.kernel > #1264817 > unrolled thread

Re: [PATCH v2 04/19] irqchip: add nps Internal and external irqchips

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 04/19] irqchip: add nps Internal and external  irqchips Thomas Gleixner <tglx@linutronix.de> - 2015-11-07 12:40 +0100
    Re: [PATCH v2 04/19] irqchip: add nps Internal and external irqchips Noam Camus <noamc@ezchip.com> - 2015-11-07 22:00 +0100
      Re: [PATCH v2 04/19] irqchip: add nps Internal and external  irqchips Thomas Gleixner <tglx@linutronix.de> - 2015-11-08 01:00 +0100

#1264817 — Re: [PATCH v2 04/19] irqchip: add nps Internal and external irqchips

On Sat, 7 Nov 2015, Noam Camus wrote:
> +#define NPS_MSU_EN_CFG	0x80
> +
> +/* Messaging and Scheduling Unit:
> + * Provides message management for a CPU cluster.
> + */
> +static void __init eznps_configure_msu(void)
> +{
> +	int cpu;
> +	struct nps_host_reg_msu_en_cfg {
> +		union {
> +			struct {
> +				u32     __reserved1:11,
> +				rtc_en:1, ipc_en:1, gim_1_en:1,
> +				gim_0_en:1, ipi_en:1, buff_e_rls_bmuw:1,
> +				buff_e_alc_bmuw:1, buff_i_rls_bmuw:1,
> +				buff_i_alc_bmuw:1, buff_e_rls_bmue:1,
> +				buff_e_alc_bmue:1, buff_i_rls_bmue:1,
> +				buff_i_alc_bmue:1, __reserved2:1,
> +				buff_e_pre_en:1, buff_i_pre_en:1,
> +				pmuw_ja_en:1, pmue_ja_en:1,
> +				pmuw_nj_en:1, pmue_nj_en:1, msu_en:1;
> +			};
> +			u32 value;
> +		};
> +	};
> +	struct nps_host_reg_msu_en_cfg msu_en_cfg = {.value = 0};
> +
> +	msu_en_cfg.msu_en = 1;
> +	msu_en_cfg.ipi_en = 1;
> +	msu_en_cfg.gim_0_en = 1;
> +	msu_en_cfg.gim_1_en = 1;

Yuck. What's wrong with:

#define GIM_1_EN	(1 << 13)
#define GIM_0_EN	(1 << 14)
#define IPI_EN		(1 << 15)
#define MSU_EN		(1 << 31)

	u32 val = GIM_1_EN | GIM_0_EN | IPI_EN | MSU_EN;

Hmm?

> +/* Global Interrupt Manager:
> + * Configures and manages up to 64 interrupts from peripherals,
> + * 16 interrupts from CPUs (virtual interrupts) and ECC interrupts.
> + * Receives the interrupts and transmits them to relevant CPU.
> + */
> +static void __init eznps_configure_gim(void)
> +{
> +	u32 reg_value;
> +	u32 gim_int_lines;
> +	struct nps_host_reg_gim_p_int_dst gim_p_int_dst = {.value = 0};
> +
> +	gim_int_lines = NPS_GIM_UART_LINE;
> +	gim_int_lines |= NPS_GIM_DBG_LAN_TX_DONE_LINE;
> +	gim_int_lines |= NPS_GIM_DBG_LAN_RX_RDY_LINE;
> +
> +	/*
> +	 * IRQ polarity
> +	 * low or high level
> +	 * negative or positive edge
> +	 */
> +	reg_value = ioread32be(REG_GIM_P_INT_POL_0);
> +	reg_value &= ~gim_int_lines;
> +	iowrite32be(reg_value, REG_GIM_P_INT_POL_0);
> +
> +	/* IRQ type level or edge */
> +	reg_value = ioread32be(REG_GIM_P_INT_SENS_0);
> +	reg_value |= NPS_GIM_DBG_LAN_TX_DONE_LINE;
> +	iowrite32be(reg_value, REG_GIM_P_INT_SENS_0);
> +
> +	/*
> +	 * GIM interrupt select type for
> +	 * dbg_lan TX and RX interrupts
> +	 * should be type 1
> +	 * type 0 = IRQ line 6
> +	 * type 1 = IRQ line 7
> +	 */
> +	gim_p_int_dst.is = 1;

More magic structs to set a single bit, right?

> +/*
> + * NPS400 core includes a Interrupt Controller (IC) support.
> + * All cores can deactivate level irqs at first level control
> + * at cores mesh layer called MTM.
> + * For devices out side chip e.g. uart, network there is another
> + * level called Global Interrupt Manager (GIM).
> + * This second level can control level and edge interrupt.
> + */
> +
> +static void nps400_irq_mask(struct irq_data *data)
> +{
> +	unsigned int ienb;
> +
> +	ienb = read_aux_reg(AUX_IENABLE);
> +	ienb &= ~(1 << data->irq);

You should not rely on data->irq ever. It's the Linux interrupt number
and it does not necessarily have a 1:1 mapping to the hardware
interrupt number. Its working for legacy domains, but there
data->hwirq is set up for you as well.

> +	write_aux_reg(AUX_IENABLE, ienb);

I can see how that works for per cpu interrupts, but what happens if
two cpus run that concurrent for two different interrupts?

Thanks,

	tglx
--
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/

[toc] | [next] | [standalone]

#1264934 — Re: [PATCH v2 04/19] irqchip: add nps Internal and external irqchips

>From: Thomas Gleixner <tglx@linutronix.de>
>Sent: Saturday, November 7, 2015 1:38 PM

>> +     /*
>> +      * GIM interrupt select type for
>> +      * dbg_lan TX and RX interrupts
>> +      * should be type 1
>> +      * type 0 = IRQ line 6
>> +      * type 1 = IRQ line 7
>> +      */
>> +     gim_p_int_dst.is = 1;

>More magic structs to set a single bit, right?
I will replace all such magic with macros.

>> +     ienb &= ~(1 << data->irq);

>You should not rely on data->irq ever. It's the Linux interrupt number
>and it does not necessarily have a 1:1 mapping to the hardware
>nterrupt number. Its working for legacy domains, but there
>data->hwirq is set up for you as well.
Thanks, I will use data->hwirq instead of data->irq.


>> +     write_aux_reg(AUX_IENABLE, ienb);

>I can see how that works for per cpu interrupts, but what happens if
>two cpus run that concurrent for two different interrupts?

Each CPU got its own HW copy of auxiliary register IENABLE, so concurrent access won't be a trouble.
 
-Noam--
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/

[toc] | [prev] | [next] | [standalone]

#1264952

Noam,

On Sat, 7 Nov 2015, Noam Camus wrote:
> >From: Thomas Gleixner <tglx@linutronix.de>
> >> +     write_aux_reg(AUX_IENABLE, ienb);
> 
> >I can see how that works for per cpu interrupts, but what happens if
> >two cpus run that concurrent for two different interrupts?
> 
> Each CPU got its own HW copy of auxiliary register IENABLE, so
> concurrent access won't be a trouble.

Please put a comment into the code explaining it.

Thanks,

	tglx
--
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/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web