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

Groups > linux.kernel > #1292687 > unrolled thread

[PATCH v4 00/19] Adding plat-eznps to ARC

Back to article view | Back to linux.kernel

Contents

  [PATCH v4 00/19] Adding plat-eznps to ARC Noam Camus <noamc@ezchip.com> - 2015-12-16 02:20 +0100
    Re: [PATCH v4 05/19] irqchip: add nps Internal and external irqchips Marc Zyngier <marc.zyngier@arm.com> - 2015-12-16 10:40 +0100
      RE: [PATCH v4 05/19] irqchip: add nps Internal and external irqchips Noam Camus <noamc@ezchip.com> - 2015-12-18 11:40 +0100
        Re: [PATCH v4 05/19] irqchip: add nps Internal and external irqchips Marc Zyngier <marc.zyngier@arm.com> - 2015-12-18 12:30 +0100
          RE: [PATCH v4 05/19] irqchip: add nps Internal and external irqchips Noam Camus <noamc@ezchip.com> - 2015-12-18 15:30 +0100
            Re: [PATCH v4 05/19] irqchip: add nps Internal and external irqchips Marc Zyngier <marc.zyngier@arm.com> - 2015-12-18 17:40 +0100
              RE: [PATCH v4 05/19] irqchip: add nps Internal and external irqchips Noam Camus <noamc@ezchip.com> - 2015-12-19 10:40 +0100
              Re: [PATCH v4 05/19] irqchip: add nps Internal and external irqchips Vineet Gupta <Vineet.Gupta1@synopsys.com> - 2015-12-30 12:40 +0100
    Re: [PATCH v4 04/19] clocksource: Add NPS400 timers driver Daniel Lezcano <daniel.lezcano@linaro.org> - 2015-12-17 14:20 +0100

#1292687 — [PATCH v4 00/19] Adding plat-eznps to ARC

From: Noam Camus <noamc@ezchip.com>

v4:
1) irqchip -- use irq_domain_add_linear()
2) clocksource -- use of_clk_get()
3) New header at include/soc use by platform and drivers
4) update DTS file for above fixed drivers

v3:
1)	irqchip: use MACROS instead of structures to decribe
	registers.
2)	clocksource: use 32bit counter and avoid 2 halfs read
	of 64bit dance.

v2:
1)	Remove out of tree platform include path
2)	Move atomic/bitop/cmpxchg for platform to end.
	Remove macro duplication.
	Fix some bad implementation.
3)	define cpu_relax_lowlatency() for platform.
4)	rename init_irq_cpu() to init_per_cpu()
	reorder call to init_per_cpu() for secondary
	use it instead of init_cpu_smp().
5)	set res_service to call stext
6)	fix build failure for CTOP_AUX_BASE at assembly code
7)	Use ilog2 for mtm_init_nat()
8)	Add CLKSRC_NPS option to Kconfig
	change nps_clksrc_read() to be more readable.

General summay:
This set introduce new platform to ARC architecture.
Platform name called "eznps" for working with EZchip NPS400
Network Proccessor.
NPS400 is targeted to service "fast path" network applications.

NPS400 got mesh of 256 extended ARC cores (AKA CTOP), each core
got 16 HW threads. This is basically SMT core where at any point of
time only one HW thread is active.
Each core have HW scheduler that round robin between eligible HW
threads. Totaly, kernel sees 4096 CPUs which I belive is a high record.
There is no cache coherency between cores so generic user applications
and kernel do not use D$.

Cores got special memory mappings for huge pages (8MB).
Mapping is static and should provide application enough memory without
any "TLB miss". This mapping is on top of TLB mapping.

This is a basic set that will later be followed with additional
set of patches with all advanced features.

Many thanks to all people helping to make this happen.

Regards,
Noam Camus

Noam Camus (18):
  Documentation: Add EZchip vendor to binding list
  soc: Support for EZchip SoC
  ARC: [plat-eznps] define IPI_IRQ
  clocksource: Add NPS400 timers driver
  irqchip: add nps Internal and external irqchips
  ARC: Set vmalloc size from configuration
  ARC: rwlock: disable interrupts in !LLSC variant
  ARC: rename smp operation init_irq_cpu() to init_per_cpu()
  ARC: Mark secondary cpu online only after all HW setup is done
  ARC: Add clock from device tree to time_init()
  ARC: [plat-eznps] Add eznps board defconfig and dts
  ARC: [plat-eznps] Add eznps platform
  ARC: [plat-eznps] Use dedicated user stack top
  ARC: [plat-eznps] Use dedicated atomic/bitops/cmpxchg
  ARC: [plat-eznps] Use dedicated SMP barriers
  ARC: [plat-eznps] Use dedicated identity auxiliary register.
  ARC: [plat-eznps] Use dedicated COMMAND_LINE_SIZE
  ARC: Add eznps platform to Kconfig and Makefile

Tal Zilcer (1):
  ARC: [plat-eznps] Use dedicated cpu_relax()

 Documentation/devicetree/bindings/arc/eznps.txt    |    7 +
 .../interrupt-controller/ezchip,nps400-ic.txt      |   17 ++
 .../bindings/timer/ezchip,nps400-timer.txt         |   15 ++
 .../devicetree/bindings/vendor-prefixes.txt        |    1 +
 MAINTAINERS                                        |    6 +
 arch/arc/Kconfig                                   |    9 +
 arch/arc/Makefile                                  |    5 +
 arch/arc/boot/dts/eznps.dts                        |   93 +++++++++
 arch/arc/configs/nps_defconfig                     |   85 +++++++++
 arch/arc/include/asm/atomic.h                      |   79 ++++++++-
 arch/arc/include/asm/barrier.h                     |    8 +
 arch/arc/include/asm/bitops.h                      |   54 ++++++
 arch/arc/include/asm/cmpxchg.h                     |   87 +++++++--
 arch/arc/include/asm/entry-compact.h               |    8 +
 arch/arc/include/asm/irq.h                         |    4 +
 arch/arc/include/asm/pgtable.h                     |    2 +-
 arch/arc/include/asm/processor.h                   |   36 +++-
 arch/arc/include/asm/setup.h                       |    4 +
 arch/arc/include/asm/smp.h                         |    4 +-
 arch/arc/include/asm/spinlock.h                    |   14 ++
 arch/arc/kernel/ctx_sw.c                           |   13 ++
 arch/arc/kernel/irq.c                              |    4 +-
 arch/arc/kernel/mcip.c                             |    2 +-
 arch/arc/kernel/smp.c                              |   14 +-
 arch/arc/kernel/time.c                             |   10 +-
 arch/arc/mm/tlb.c                                  |   12 ++
 arch/arc/plat-eznps/Kconfig                        |   34 ++++
 arch/arc/plat-eznps/Makefile                       |    7 +
 arch/arc/plat-eznps/entry.S                        |   75 ++++++++
 arch/arc/plat-eznps/include/plat/ctop.h            |  198 ++++++++++++++++++++
 arch/arc/plat-eznps/include/plat/mtm.h             |   60 ++++++
 arch/arc/plat-eznps/include/plat/smp.h             |   26 +++
 arch/arc/plat-eznps/mtm.c                          |  133 +++++++++++++
 arch/arc/plat-eznps/platform.c                     |  102 ++++++++++
 arch/arc/plat-eznps/smp.c                          |  156 +++++++++++++++
 drivers/clocksource/Kconfig                        |    7 +
 drivers/clocksource/Makefile                       |    1 +
 drivers/clocksource/timer-nps.c                    |   68 +++++++
 drivers/irqchip/Makefile                           |    1 +
 drivers/irqchip/irq-eznps.c                        |  131 +++++++++++++
 include/soc/nps/common.h                           |  123 ++++++++++++
 41 files changed, 1678 insertions(+), 37 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arc/eznps.txt
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/ezchip,nps400-ic.txt
 create mode 100644 Documentation/devicetree/bindings/timer/ezchip,nps400-timer.txt
 create mode 100644 arch/arc/boot/dts/eznps.dts
 create mode 100644 arch/arc/configs/nps_defconfig
 create mode 100644 arch/arc/plat-eznps/Kconfig
 create mode 100644 arch/arc/plat-eznps/Makefile
 create mode 100644 arch/arc/plat-eznps/entry.S
 create mode 100644 arch/arc/plat-eznps/include/plat/ctop.h
 create mode 100644 arch/arc/plat-eznps/include/plat/mtm.h
 create mode 100644 arch/arc/plat-eznps/include/plat/smp.h
 create mode 100644 arch/arc/plat-eznps/mtm.c
 create mode 100644 arch/arc/plat-eznps/platform.c
 create mode 100644 arch/arc/plat-eznps/smp.c
 create mode 100644 drivers/clocksource/timer-nps.c
 create mode 100644 drivers/irqchip/irq-eznps.c
 create mode 100644 include/soc/nps/common.h

--
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]

#1292847 — Re: [PATCH v4 05/19] irqchip: add nps Internal and external irqchips

On 16/12/15 01:10, Noam Camus wrote:
> From: Noam Camus <noamc@ezchip.com>
> 
> Adding EZchip NPS400 support.
> NPS internal interrupts are internally handled at
> Multi Thread Manager (MTM) that is signaled for deactivating
> an interrupt.
> External interrupts is handled also at Global Interrupt
> Controller (GIC) e.g. serial and network devices.
> 
> Signed-off-by: Noam Camus <noamc@ezchip.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  .../interrupt-controller/ezchip,nps400-ic.txt      |   17 +++
>  drivers/irqchip/Makefile                           |    1 +
>  drivers/irqchip/irq-eznps.c                        |  131 ++++++++++++++++++++
>  3 files changed, 149 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/ezchip,nps400-ic.txt
>  create mode 100644 drivers/irqchip/irq-eznps.c
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/ezchip,nps400-ic.txt b/Documentation/devicetree/bindings/interrupt-controller/ezchip,nps400-ic.txt
> new file mode 100644
> index 0000000..888b2b9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/ezchip,nps400-ic.txt
> @@ -0,0 +1,17 @@
> +EZchip NPS Interrupt Controller
> +
> +Required properties:
> +
> +- compatible : should be "ezchip,nps400-ic"
> +- interrupt-controller : Identifies the node as an interrupt controller
> +- #interrupt-cells : Specifies the number of cells needed to encode an
> +  interrupt source. The value shall be 1.
> +
> +
> +Example:
> +
> +intc: interrupt-controller {
> +	compatible = "ezchip,nps400-ic";
> +	interrupt-controller;
> +	#interrupt-cells = <1>;
> +};
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index 177f78f..b95b954 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -55,3 +55,4 @@ obj-$(CONFIG_RENESAS_H8S_INTC)		+= irq-renesas-h8s.o
>  obj-$(CONFIG_ARCH_SA1100)		+= irq-sa11x0.o
>  obj-$(CONFIG_INGENIC_IRQ)		+= irq-ingenic.o
>  obj-$(CONFIG_IMX_GPCV2)			+= irq-imx-gpcv2.o
> +obj-$(CONFIG_ARC_PLAT_EZNPS)		+= irq-eznps.o
> diff --git a/drivers/irqchip/irq-eznps.c b/drivers/irqchip/irq-eznps.c
> new file mode 100644
> index 0000000..fc6d59a
> --- /dev/null
> +++ b/drivers/irqchip/irq-eznps.c
> @@ -0,0 +1,131 @@
> +/*
> + * Copyright(c) 2015 EZchip Technologies.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * The full GNU General Public License is included in this distribution in
> + * the file called "COPYING".
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/irqdomain.h>
> +#include <linux/irqchip.h>
> +#include <asm/irq.h>
> +
> +/*
> + * 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.
> + *
> + * NOTE: AUX_IENABLE and CTOP_AUX_IACK are auxiliary registers
> + * with private HW copy per CPU.
> + */
> +
> +static void nps400_irq_mask(struct irq_data *data)
> +{
> +	unsigned int ienb;
> +
> +	ienb = read_aux_reg(AUX_IENABLE);
> +	ienb &= ~(1 << data->hwirq);
> +	write_aux_reg(AUX_IENABLE, ienb);
> +}
> +
> +static void nps400_irq_unmask(struct irq_data *data)
> +{
> +	unsigned int ienb;
> +
> +	ienb = read_aux_reg(AUX_IENABLE);
> +	ienb |= (1 << data->hwirq);
> +	write_aux_reg(AUX_IENABLE, ienb);
> +}
> +
> +static void nps400_irq_eoi_global(struct irq_data *data)
> +{
> +	write_aux_reg(CTOP_AUX_IACK, 1 << data->hwirq);
> +
> +	/* Don't ack before all device access is done */
> +	mb();
> +
> +	__asm__ __volatile__ (
> +	"       .word %0\n"
> +	:
> +	: "i"(CTOP_INST_RSPI_GIC_0_R12)
> +	: "memory");
> +}
> +
> +static void nps400_irq_eoi(struct irq_data *data)
> +{
> +	write_aux_reg(CTOP_AUX_IACK, 1 << data->hwirq);
> +}
> +
> +static struct irq_chip nps400_irq_chip_fasteoi = {
> +	.name		= "NPS400 IC Global",
> +	.irq_mask	= nps400_irq_mask,
> +	.irq_unmask	= nps400_irq_unmask,
> +	.irq_eoi	= nps400_irq_eoi_global,
> +};
> +
> +static struct irq_chip nps400_irq_chip_percpu = {
> +	.name		= "NPS400 IC",
> +	.irq_mask	= nps400_irq_mask,
> +	.irq_unmask	= nps400_irq_unmask,
> +	.irq_eoi	= nps400_irq_eoi,
> +};
> +
> +static int nps400_irq_map(struct irq_domain *d, unsigned int virq,
> +			  irq_hw_number_t hw)
> +{
> +	switch (hw) {
> +	case TIMER0_IRQ:
> +#if defined(CONFIG_SMP)
> +	case IPI_IRQ:
> +#endif
> +		irq_set_chip_and_handler(virq, &nps400_irq_chip_percpu,
> +					 handle_percpu_irq);
> +	break;
> +	default:
> +		irq_set_chip_and_handler(virq, &nps400_irq_chip_fasteoi,
> +					 handle_fasteoi_irq);
> +	break;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct irq_domain_ops nps400_irq_ops = {
> +	.xlate = irq_domain_xlate_onecell,
> +	.map = nps400_irq_map,
> +};
> +
> +static struct irq_domain *nps400_root_domain;
> +
> +static int __init nps400_of_init(struct device_node *node,
> +				 struct device_node *parent)
> +{
> +	if (parent)
> +		panic("DeviceTree incore ic not a root irq controller\n");
> +
> +	nps400_root_domain = irq_domain_add_linear(node, NR_CPU_IRQS,
> +						   &nps400_irq_ops, NULL);
> +
> +	if (!nps400_root_domain)
> +		panic("nps400 root irq domain not avail\n");
> +
> +	/* with this we don't need to export nps400_root_domain */
> +	irq_set_default_host(nps400_root_domain);

Why do you need this? Devices should have their interrupt-parent
pointing to this node, and irq_find_host should sort it out. I must be
missing some information (only being CC'd on this single patch).

> +
> +	return 0;
> +}
> +IRQCHIP_DECLARE(ezchip_nps400_ic, "ezchip,nps400-ic", nps400_of_init);
> 

Another thing I'm not seeing here is where is the interrupt actually
taken. This code only contains the EOI part, but not the ACK side, as
well as the reverse lookup hwirq -> irq). Where is that code?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...
--
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]

#1294650 — RE: [PATCH v4 05/19] irqchip: add nps Internal and external irqchips

From: Marc Zyngier [mailto:marc.zyngier@arm.com] 
Sent: Wednesday, December 16, 2015 11:31 AM

>> +static int __init nps400_of_init(struct device_node *node,
>> +				 struct device_node *parent)
>> +{
>> +	if (parent)
>> +		panic("DeviceTree incore ic not a root irq controller\n");
>> +
>> +	nps400_root_domain = irq_domain_add_linear(node, NR_CPU_IRQS,
>> +						   &nps400_irq_ops, NULL);
>> +
>> +	if (!nps400_root_domain)
>> +		panic("nps400 root irq domain not avail\n");
>> +
>> +	/* with this we don't need to export nps400_root_domain */
>> +	irq_set_default_host(nps400_root_domain);

>Why do you need this? Devices should have their interrupt-parent pointing to this node, and irq_find_host should sort it >out. I must be missing some information (only being CC'd on this single patch).
Sorry, I will CC you by my next version, in the meantime please refer to:
https://lkml.org/lkml/2015/12/15/864

I need this for my per CPU irqs such timer and IPI which do not come from some external device but from CPUs.
For these IRQs I am calling to irq_create_mapping() from my platform at arch/arc and at that point I got no irqdomain and using irq_find_host() is not good since I got no device_node (at most I can have DT root).
Is there device_node for DT root? 
Please advise what to do?

>> +
>> +	return 0;
>> +}
>> +IRQCHIP_DECLARE(ezchip_nps400_ic, "ezchip,nps400-ic", 
>> +nps400_of_init);
>> 

>Another thing I'm not seeing here is where is the interrupt actually taken. This code only contains the EOI part, but not the ACK side, as well as the reverse lookup hwirq -> irq). Where is that code?

ACK is an optional handler and is not needed by my platform.
I will add comment that since my IRQs are EOI based I do not need an ACK.

I do not understand reverse lookup remark, where is it missing?
Could you point me to an example for such reverse lookup? 

Regards,
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]

#1294691 — Re: [PATCH v4 05/19] irqchip: add nps Internal and external irqchips

On 18/12/15 10:37, Noam Camus wrote:
> From: Marc Zyngier [mailto:marc.zyngier@arm.com] 
> Sent: Wednesday, December 16, 2015 11:31 AM
> 
>>> +static int __init nps400_of_init(struct device_node *node,
>>> +				 struct device_node *parent)
>>> +{
>>> +	if (parent)
>>> +		panic("DeviceTree incore ic not a root irq controller\n");
>>> +
>>> +	nps400_root_domain = irq_domain_add_linear(node, NR_CPU_IRQS,
>>> +						   &nps400_irq_ops, NULL);
>>> +
>>> +	if (!nps400_root_domain)
>>> +		panic("nps400 root irq domain not avail\n");
>>> +
>>> +	/* with this we don't need to export nps400_root_domain */
>>> +	irq_set_default_host(nps400_root_domain);
> 
>> Why do you need this? Devices should have their interrupt-parent pointing to this node, and irq_find_host should sort it >out. I must be missing some information (only being CC'd on this single patch).
> Sorry, I will CC you by my next version, in the meantime please refer to:
> https://lkml.org/lkml/2015/12/15/864
> 
> I need this for my per CPU irqs such timer and IPI which do not come
> from some external device but from CPUs. For these IRQs I am calling
> to irq_create_mapping() from my platform at arch/arc and at that
> point I got no irqdomain and using irq_find_host() is not good since
> I got no device_node (at most I can have DT root).

That's a problem. You should never do that for your timer (doing a
request_irq will do the right thing, and that's what your timer driver
already does).

As for initializing your IPIs, they are usually outside of the IRQ
space, so you should handle them separately (and get your irqchip to
initialize them).

> Is there device_node for DT root? 
> Please advise what to do?
> 
>>> +
>>> +	return 0;
>>> +}
>>> +IRQCHIP_DECLARE(ezchip_nps400_ic, "ezchip,nps400-ic", 
>>> +nps400_of_init);
>>>
> 
>> Another thing I'm not seeing here is where is the interrupt actually taken. This code only contains the EOI part, but not the ACK side, as well as the reverse lookup hwirq -> irq). Where is that code?
> 
> ACK is an optional handler and is not needed by my platform.
> I will add comment that since my IRQs are EOI based I do not need an ACK.

What I'm talking about is not the irq_ack method that the irqchip can
provide, but the action your perform on your interrupt controller to
extract the hwirq number and find out what corresponding Linux interrupt
has fired.

> 
> I do not understand reverse lookup remark, where is it missing?
> Could you point me to an example for such reverse lookup? 

See for example:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/irqchip/irq-sun4i.c#n136

This is the function (sun4i_handle_irq) that is executed almost
immediately after the IRQ is asserted. The CPU reads the hwirq from the
interrupt controller, and use handle_domain_irq() to call the
corresponding handler (by doing a lookup in the domain).

I couldn't find out in your platform code where you are doing that.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...
--
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]

#1294883 — RE: [PATCH v4 05/19] irqchip: add nps Internal and external irqchips

>From: Marc Zyngier [mailto:marc.zyngier@arm.com] 
>Sent: Friday, December 18, 2015 1:21 PM

>> I need this for my per CPU irqs such timer and IPI which do not come 
>> from some external device but from CPUs. For these IRQs I am calling 
>> to irq_create_mapping() from my platform at arch/arc and at that point 
>> I got no irqdomain and using irq_find_host() is not good since I got 
>> no device_node (at most I can have DT root).

>That's a problem. You should never do that for your timer (doing a request_irq will do the right thing, and that's what your timer driver already does).

Please be more specific, from all that I wrote what is the problem?
When I use request_irq() it fail without the mapping and mapping fail without a domain.
Never do what?
What should I do then?

>As for initializing your IPIs, they are usually outside of the IRQ space, so you should handle them separately (and get your irqchip to initialize them).
I am handling all my IRQs within same irqchip, which is the only one I have. So I am not sure what you expect here.
Please be more elaborate.

>> 
>> Another thing I'm not seeing here is where is the interrupt actually taken. This code only contains the EOI part, but not the ACK side, as well as the reverse lookup hwirq -> irq). Where is that code?
>> 
>> ACK is an optional handler and is not needed by my platform.
>> I will add comment that since my IRQs are EOI based I do not need an ACK.

>What I'm talking about is not the irq_ack method that the irqchip can provide, but the action your perform on your interrupt controller to extract the hwirq number and find out what corresponding Linux interrupt has fired.

>> 
>> I do not understand reverse lookup remark, where is it missing?
>> Could you point me to an example for such reverse lookup? 

>See for example:

>https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/irqchip/irq-sun4i.c#n136

>This is the function (sun4i_handle_irq) that is executed almost immediately after the IRQ is asserted. The CPU reads the hwirq from the interrupt controller, and use handle_domain_irq() to call the corresponding handler (by doing a lookup in the domain).

>I couldn't find out in your platform code where you are doing that.

OK, this is seem much like exclusively ARM stuff.
Note that I am working with ARC (seem alike) here and we do not define CONFIG_HANDLE_DOMAIN_IRQ and do not implement set_handle_irq().

So for ARC this reverse mapping is something we can leave without (maybe because we are kind of a legacy domain).

Note that this patch is part of a set I introduce to ARC as a new platform.
You can view my tree and the patch set at:
https://github.com/EZchip/linux/commits/EZ-arc-for-next-basic-reviewd-DO-NOT-COMMENT

-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]

#1295011 — Re: [PATCH v4 05/19] irqchip: add nps Internal and external irqchips

On 18/12/15 14:29, Noam Camus wrote:
>> From: Marc Zyngier [mailto:marc.zyngier@arm.com] 
>> Sent: Friday, December 18, 2015 1:21 PM
> 
>>> I need this for my per CPU irqs such timer and IPI which do not come 
>>> from some external device but from CPUs. For these IRQs I am calling 
>>> to irq_create_mapping() from my platform at arch/arc and at that point 
>>> I got no irqdomain and using irq_find_host() is not good since I got 
>>> no device_node (at most I can have DT root).
> 
>> That's a problem. You should never do that for your timer (doing a request_irq will do the right thing, and that's what your timer driver already does).
> 
> Please be more specific, from all that I wrote what is the problem?

Calling irq_create_mapping out of the blue like you do it here:

+static void eznps_init_per_cpu(int cpu)
+{
+	/* Create mapping for all per cpu IRQs */
+	if (cpu == 0) {
+		irq_create_mapping(NULL, TIMER0_IRQ);
+		irq_create_mapping(NULL, IPI_IRQ);
+	}

is simply not acceptable.

> When I use request_irq() it fail without the mapping and mapping fail without a domain.

Grmbl...

That's not completely surprising:

+		timer {
+			compatible = "ezchip,nps400-timer";
+			clocks = <&sysclk>;
+			clock-names="sysclk";
+		};

Where is the interrupt?

> Never do what?
> What should I do then?

Maybe you should start by looking how the other architectures have
solved that exact problem at least half a dozen time.

> 
>> As for initializing your IPIs, they are usually outside of the IRQ
>> space, so you should handle them separately (and get your irqchip
>> to initialize them).
> I am handling all my IRQs within same irqchip, which is the only one 
> I have. So I am not sure what you expect here. Please be more 
> elaborate.

Do not create a mapping for IPIs. Full stop. Handle them independently
from your normal IRQs.

>>>
>>> Another thing I'm not seeing here is where is the interrupt actually taken. This code only contains the EOI part, but not the ACK side, as well as the reverse lookup hwirq -> irq). Where is that code?
>>>
>>> ACK is an optional handler and is not needed by my platform.
>>> I will add comment that since my IRQs are EOI based I do not need an ACK.
> 
>> What I'm talking about is not the irq_ack method that the irqchip can provide, but the action your perform on your interrupt controller to extract the hwirq number and find out what corresponding Linux interrupt has fired.
> 
>>>
>>> I do not understand reverse lookup remark, where is it missing?
>>> Could you point me to an example for such reverse lookup? 
> 
>> See for example:
> 
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/irqchip/irq-sun4i.c#n136
> 
>> This is the function (sun4i_handle_irq) that is executed almost immediately after the IRQ is asserted. The CPU reads the hwirq from the interrupt controller, and use handle_domain_irq() to call the corresponding handler (by doing a lookup in the domain).
> 
>> I couldn't find out in your platform code where you are doing that.
> 
> OK, this is seem much like exclusively ARM stuff.

No, this is not. Can you please stop looking at the surface of things
and start taking an interest in how things actually *work*? Almost
*nothing* in the interrupt handling code is architecture specific.

> Note that I am working with ARC (seem alike) here and we do not
> define CONFIG_HANDLE_DOMAIN_IRQ and do not implement
> set_handle_irq().
> 
> So for ARC this reverse mapping is something we can leave without
> (maybe because we are kind of a legacy domain).

Yeah, I just located the crap: arch_do_IRQ() happily takes a hwirq (the
vector number), and uses that as a Linux IRQ. This looks a lot like ARM
pre-DT, about 10 years ago.

Well, time to meet the 21st century. If you intend to use DT, please fix
your arch port. Otherwise, just hardcode everything in your platform and
don't pretend to support device tree.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...
--
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]

#1295337 — RE: [PATCH v4 05/19] irqchip: add nps Internal and external irqchips

>From: Marc Zyngier [mailto:marc.zyngier@arm.com] 
>Sent: Friday, December 18, 2015 6:31 PM

>> Note that I am working with ARC (seem alike) here and we do not define 
>> CONFIG_HANDLE_DOMAIN_IRQ and do not implement set_handle_irq().
>> 
>> So for ARC this reverse mapping is something we can leave without 
>> (maybe because we are kind of a legacy domain).

>Yeah, I just located the crap: arch_do_IRQ() happily takes a hwirq (the vector number), and uses that as a Linux IRQ. This looks a lot like ARM pre-DT, about 10 years ago.

>Well, time to meet the 21st century. If you intend to use DT, please fix your arch port. Otherwise, just hardcode everything in your platform and don't pretend to support device tree.

Thank you very much. 
I have the complete picture now.

Will discuss with ARC maintainer and see what best way to do this change. 

-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]

#1299330 — Re: [PATCH v4 05/19] irqchip: add nps Internal and external irqchips

On Friday 18 December 2015 10:01 PM, Marc Zyngier wrote:
> On 18/12/15 14:29, Noam Camus wrote:
>>> From: Marc Zyngier [mailto:marc.zyngier@arm.com] 
>>> Sent: Friday, December 18, 2015 1:21 PM
>>
>>>> I need this for my per CPU irqs such timer and IPI which do not come 
>>>> from some external device but from CPUs. For these IRQs I am calling 
>>>> to irq_create_mapping() from my platform at arch/arc and at that point 
>>>> I got no irqdomain and using irq_find_host() is not good since I got 
>>>> no device_node (at most I can have DT root).
>>
>>> That's a problem. You should never do that for your timer (doing a request_irq will do the right thing, and that's what your timer driver already does).
>>
>> Please be more specific, from all that I wrote what is the problem?
> 
> Calling irq_create_mapping out of the blue like you do it here:
> 
> +static void eznps_init_per_cpu(int cpu)
> +{
> +	/* Create mapping for all per cpu IRQs */
> +	if (cpu == 0) {
> +		irq_create_mapping(NULL, TIMER0_IRQ);
> +		irq_create_mapping(NULL, IPI_IRQ);
> +	}
> 
> is simply not acceptable.
> 
>> When I use request_irq() it fail without the mapping and mapping fail without a domain.
> 
> Grmbl...
> 
> That's not completely surprising:
> 
> +		timer {
> +			compatible = "ezchip,nps400-timer";
> +			clocks = <&sysclk>;
> +			clock-names="sysclk";
> +		};
> 
> Where is the interrupt?
> 
>> Never do what?
>> What should I do then?
> 
> Maybe you should start by looking how the other architectures have
> solved that exact problem at least half a dozen time.
> 
>>
>>> As for initializing your IPIs, they are usually outside of the IRQ
>>> space, so you should handle them separately (and get your irqchip
>>> to initialize them).
>> I am handling all my IRQs within same irqchip, which is the only one 
>> I have. So I am not sure what you expect here. Please be more 
>> elaborate.
> 
> Do not create a mapping for IPIs. Full stop. Handle them independently
> from your normal IRQs.
> 
>>>>
>>>> Another thing I'm not seeing here is where is the interrupt actually taken. This code only contains the EOI part, but not the ACK side, as well as the reverse lookup hwirq -> irq). Where is that code?
>>>>
>>>> ACK is an optional handler and is not needed by my platform.
>>>> I will add comment that since my IRQs are EOI based I do not need an ACK.
>>
>>> What I'm talking about is not the irq_ack method that the irqchip can provide, but the action your perform on your interrupt controller to extract the hwirq number and find out what corresponding Linux interrupt has fired.
>>
>>>>
>>>> I do not understand reverse lookup remark, where is it missing?
>>>> Could you point me to an example for such reverse lookup? 
>>
>>> See for example:
>>
>>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/irqchip/irq-sun4i.c#n136
>>
>>> This is the function (sun4i_handle_irq) that is executed almost immediately after the IRQ is asserted. The CPU reads the hwirq from the interrupt controller, and use handle_domain_irq() to call the corresponding handler (by doing a lookup in the domain).
>>
>>> I couldn't find out in your platform code where you are doing that.
>>
>> OK, this is seem much like exclusively ARM stuff.
> 
> No, this is not. Can you please stop looking at the surface of things
> and start taking an interest in how things actually *work*? Almost
> *nothing* in the interrupt handling code is architecture specific.
> 
>> Note that I am working with ARC (seem alike) here and we do not
>> define CONFIG_HANDLE_DOMAIN_IRQ and do not implement
>> set_handle_irq().
>>
>> So for ARC this reverse mapping is something we can leave without
>> (maybe because we are kind of a legacy domain).
> 
> Yeah, I just located the crap: arch_do_IRQ() happily takes a hwirq (the
> vector number), and uses that as a Linux IRQ. This looks a lot like ARM
> pre-DT, about 10 years ago.
> 
> Well, time to meet the 21st century. If you intend to use DT, please fix
> your arch port. Otherwise, just hardcode everything in your platform and
> don't pretend to support device tree.

Hi Marc,

Taking your rant in a positive stride and I'm all up for making this as
nice/modern as possible. I don't have issues with enabling
CONFIG_HANDLE_DOMAIN_IRQ for ARC (although it might add a few cycles o/h to each ISR)

However currently (4.4-rcX) it is only enabled for arm/arm64/openrisc and from the
looks of it in drivers/irqchip, only ARM based SoCs use the handle_domain_irq()
calls by plugging into ARM top level handler.

Why is that not a problem for other arches like PPC/MIPS which also use DT
heavily. Or perhaps they are also subtly broken as ARC is !

What am I missing ?

Thx,
-Vineet
--
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]

#1293880 — Re: [PATCH v4 04/19] clocksource: Add NPS400 timers driver

On 12/16/2015 02:10 AM, Noam Camus wrote:
> From: Noam Camus <noamc@ezchip.com>
>
> Add internal tick generator which is shared by all cores.
> Each cluster of cores view it through dedicated address.
> This is used for SMP system where all CPUs synced by same
> clock source.
>
> Signed-off-by: Noam Camus <noamc@ezchip.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: John Stultz <john.stultz@linaro.org>
> Acked-by: Vineet Gupta <vgupta@synopsys.com>

[ ... ]

> +	clk_prepare_enable(clk);
> +	rate = clk_get_rate(clk);

With the return value checked:

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Thanks !

   -- Daniel


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

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