J-core J2 cpu and SoC peripherals support

The J-Core project (j-core.org) produces open source cpu and SoC peripheral cores synthesizable as FPGA bitstreams or ASICs.

Signed-off-by Rich Felker <[email protected]>

---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

Documentation/devicetree/bindings/vendor-prefixes.txt +1 -1

index 86740d4..9c070b8 100644
@@ -126,6 +126,7 @@ invensense  InvenSense Inc.
 isee   ISEE 2007 S.L.
 isil   Intersil
 issi   Integrated Silicon Solutions Inc.
+jcore  J-Core.org
 jedec  JEDEC Solid State Technology Association
 karo   Ka-Ro electronics GmbH
 keymile        Keymile GmbH
--

2.8.1

Quoted 4 lines · Rich Felker

The J-Core project (j-core.org) produces open source cpu and SoC peripheral cores synthesizable as FPGA bitstreams or ASICs.

Signed-off-by Rich Felker <[email protected]>

Please add acks when posting subsequent versions.

Quoted 4 lines

---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

Quoted 13 lines

Documentation/devicetree/bindings/vendor-prefixes.txt +1 -1

index 86740d4..9c070b8 100644
@@ -126,6 +126,7 @@ invensense  InvenSense Inc.
 isee   ISEE 2007 S.L.
 isil   Intersil
 issi   Integrated Silicon Solutions Inc.
+jcore  J-Core.org
 jedec  JEDEC Solid State Technology Association
 karo   Ka-Ro electronics GmbH
 keymile        Keymile GmbH
--

Quoted 1 line

2.8.1

Quoted 21 lines · Rob Herring

Quoted 4 lines · Rich Felker

The J-Core project (j-core.org) produces open source cpu and SoC peripheral cores synthesizable as FPGA bitstreams or ASICs.

Signed-off-by Rich Felker <[email protected]>

Please add acks when posting subsequent versions.

Quoted 13 lines

---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

Documentation/devicetree/bindings/vendor-prefixes.txt +1 -0

index 86740d4..9c070b8 100644
@@ -126,6 +126,7 @@ invensense  InvenSense Inc.
 isee   ISEE 2007 S.L.
 isil   Intersil
 issi   Integrated Silicon Solutions Inc.
+jcore  J-Core.org

Quoted 5 lines

Quoted 5 lines

Documentation/devicetree/bindings/vendor-prefixes.txt +0 -1

 jedec  JEDEC Solid State Technology Association
 karo   Ka-Ro electronics GmbH
 keymile        Keymile GmbH
--

2.8.1

Since this was previously acked, can I include it in my tree for upstreaming? Or do you want me to submit the patch again?

Quoted 22 lines · Rich Felker

Quoted 21 lines · Rob Herring

Quoted 4 lines · Rich Felker

The J-Core project (j-core.org) produces open source cpu and SoC peripheral cores synthesizable as FPGA bitstreams or ASICs.

Signed-off-by Rich Felker <[email protected]>

Please add acks when posting subsequent versions.

Quoted 13 lines

---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

Documentation/devicetree/bindings/vendor-prefixes.txt +1 -0

index 86740d4..9c070b8 100644
@@ -126,6 +126,7 @@ invensense  InvenSense Inc.
 isee   ISEE 2007 S.L.
 isil   Intersil
 issi   Integrated Silicon Solutions Inc.
+jcore  J-Core.org

Quoted 8 lines

Quoted 5 lines

Quoted 5 lines

Documentation/devicetree/bindings/vendor-prefixes.txt +0 -1

 jedec  JEDEC Solid State Technology Association
 karo   Ka-Ro electronics GmbH
 keymile        Keymile GmbH
--

2.8.1

Since this was previously acked, can I include it in my tree for upstreaming? Or do you want me to submit the patch again?

Ping. Should this go upstream via me or someone else? It doesn't correspond to any specific driver subsystem.

Signed-off-by Rich Felker <[email protected]>

---
 Documentation/devicetree/bindings/jcore/cpus.txt | 92 ++++++++++++++++++++++++
 1 file changed, 92 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/jcore/cpus.txt

Documentation/devicetree/bindings/jcore/cpus.txt new file +39 -0

new file mode 100644
index 0000000..9d77ec1
@@ -0,0 +1,92 @@
+===================
+J-Core cpu bindings
+===================
+
+The J-Core processors are open source CPU cores that can be built as FPGA
+soft cores or ASICs. The device tree is also responsible for describing the
+cache controls and, for SMP configurations, all details of the SMP method,
+as documented below.
+
+
+---------------------
+Top-level "cpus" node
+---------------------
+
+Required properties:
+
+- #address-cells: Must be 1.
+
+- #size-cells: Must be 0.
+
+Optional properties:
+
+- enable-method: Required only for SMP systems. If present, must be
+  "jcore,spin-table".
+
+
+--------------------
+Individual cpu nodes
+--------------------
+
+Required properties:
+
+- device_type: Must be "cpu".
+
+- compatible: Must be "jcore,j2".
+
+- reg: Must be 0 on uniprocessor systems, or the sequential, zero-based
+  hardware cpu id on SMP systems.
+

From my experience with the arm64 spin-table, I would *strongly*

+--------

Documentation/devicetree/bindings/jcore/cpus.txt new file +3 -0

+IPI node
+--------
+

+----------

Documentation/devicetree/bindings/jcore/cpus.txt new file +3 -0

+CPUID node
+----------
+

Quoted 25 lines

Documentation/devicetree/bindings/jcore/cpus.txt new file +25 -0

+Optional properties:
+
+- enable-method: Required only for SMP systems. If present, must be
+  "jcore,spin-table".
+
+
+--------------------
+Individual cpu nodes
+--------------------
+
+Required properties:
+
+- device_type: Must be "cpu".
+
+- compatible: Must be "jcore,j2".
+
+- reg: Must be 0 on uniprocessor systems, or the sequential, zero-based
+  hardware cpu id on SMP systems.
+
+Optional properties:
+
+- clock-frequency: Clock frequency of the cpu in Hz.
+
+- cpu-release-addr: Necessary only for secondary processors on SMP systems
+  using the "jcore,spin-table" enable method. If present, must consist of

Documentation/devicetree/bindings/jcore/cpus.txt line 49

Quoted 4 lines

Documentation/devicetree/bindings/jcore/cpus.txt new file +4 -0

+  two cells containing physical addresses. The first cell contains an
+  address which, when written, unblocks the secondary cpu. The second cell
+  contains an address from which the cpu will read its initial program
+  counter when unblocked.

I take it this follows the example of the arm64 spin-table rather than the ePAPR spin-table, given the lack of an ePAPR reference or struct definition.

From my experience with the arm64 spin-table, I would *strongly* recommend that you tighten this up, and define things more thoroughly, before a variety of FW/bootloader implementations appear. e.g.

* What initial/invalid value should the location contain? Is zero a valid address that you might want to jump a secondary CPU to?

* How must the value be written?
- Which endianness?

- With a single store? Or is there a more involved sequence to prevent the secondary CPU from seeing a torn value?

* Must CPUs have a unique cpu-release-addr? I would *strongly* recommend that they do.

- Is any minimal padding required around cpu-release-addrs? e.g. can FW or bootlaoder put data in the same cacheline-aligned region and exepct the OS not to corrupt this?

* How should the OS map the region (i.e. which MMU/cache attributes)?

- Is the address permitted to be a device register?

* Where can this memory live?
- Should it be abesnt from any memory node? To which padding?
- Should the memory be described with a memreserve? If so, what are your architecture-specific memreserve semantics (i.e. which MMU/cache attributes are permitted for a memreserve'd region)?

* What state should the CPU be in when it branches to the provided address?
- Must the MMU be off?

- What state must any cache be in?
Should FW perform any implementation defined coherency and cache management prior to branching?

- Must the CPU be in a particular endianness?

- Which exceptions must be masked?

- Are interrupts permitted to be pending?

- Should debug logic (e.g. breakpoint and watchpoints) be placed into a quiescent state?

- Should any registers be programmed with specific values?

At some point, you are likely to want CPU hotplug and/or cpuidle. We didn't provision the arm64 spin-table for either of these and never extended it, but you may want to put in place some discoverability now to allow future OSs to use that new support while allowing current OSs to retain functional (e.g. not requiring a new enable-method string).

Documentation/devicetree/bindings/jcore/cpus.txt line 60

Quoted 9 lines

+---------------------

Documentation/devicetree/bindings/jcore/cpus.txt new file +8 -0

+Cache controller node
+---------------------
+
+Required properties:
+
+- compatible: Must be "jcore,cache".
+
+- reg: A memory range for the cache controller registers.

There is a well-defined memory map for the cache controller?

If so, please refer to documentation for it here (either in this section, or the top of this document if common with other elements described herein).

Quoted 8 lines

+--------

Documentation/devicetree/bindings/jcore/cpus.txt new file +7 -0

+IPI node
+--------
+
+Device trees for SMP systems must have an IPI node representing the mechanism
+used for inter-processor interrupt generation.
+
+Required properties:

Documentation/devicetree/bindings/jcore/cpus.txt line 76

Quoted 6 lines

Documentation/devicetree/bindings/jcore/cpus.txt new file +6 -0

+
+- compatible: Must be "jcore,ipi-controller".
+
+- reg: A memory range used to IPI generation.
+
+- interrupts: An irq on which IPI will be received.

Likewise.

Quoted 8 lines

+----------

Documentation/devicetree/bindings/jcore/cpus.txt new file +7 -0

+CPUID node
+----------
+
+Device trees for SMP systems must have a CPUID node representing the mechanism
+used to identify the current processor on which execution is taking place.
+
+Required properties:

Documentation/devicetree/bindings/jcore/cpus.txt line 92

Quoted 6 lines

Documentation/devicetree/bindings/jcore/cpus.txt new file +6 -0

+
+- compatible: Must be "jcore,cpuid-mmio".
+
+- reg: A memory range containing a single 32-bit mmio register which produces
+  the current cpu id (matching the "reg" property of the cpu performing the
+  read) when read.

Likewise.

If you really do want a lot more detail for SMP-related bindings, I could consider submitting a version with SMP omitted for now (since the kernel patches submitted at this point don't include SMP) and do the addition of SMP as a separate patch later. But with the launch of open-hardware boards capable of running SMP J2 systems (see
https://twitter.com/jcoreeng/status/730330848306700288) near, I'd like to be getting bindings we can use stabilized so that we're properly including DTB in the boot rom and not relying on external DTB files or linking DTB in kernel.

Quoted 25 lines · Mark Rutland

Quoted 25 lines

Documentation/devicetree/bindings/jcore/cpus.txt new file +25 -0

+Optional properties:
+
+- enable-method: Required only for SMP systems. If present, must be
+  "jcore,spin-table".
+
+
+--------------------
+Individual cpu nodes
+--------------------
+
+Required properties:
+
+- device_type: Must be "cpu".
+
+- compatible: Must be "jcore,j2".
+
+- reg: Must be 0 on uniprocessor systems, or the sequential, zero-based
+  hardware cpu id on SMP systems.
+
+Optional properties:
+
+- clock-frequency: Clock frequency of the cpu in Hz.
+
+- cpu-release-addr: Necessary only for secondary processors on SMP systems
+  using the "jcore,spin-table" enable method. If present, must consist of

Quoted 8 lines

Documentation/devicetree/bindings/jcore/cpus.txt line 49

Quoted 4 lines

Documentation/devicetree/bindings/jcore/cpus.txt new file +4 -0

+  two cells containing physical addresses. The first cell contains an
+  address which, when written, unblocks the secondary cpu. The second cell
+  contains an address from which the cpu will read its initial program
+  counter when unblocked.

I take it this follows the example of the arm64 spin-table rather than the ePAPR spin-table, given the lack of an ePAPR reference or struct definition.

Indeed, I wasn't aware of the ePAPR spec for it. Would you prefer that we use a different name or something?

Quoted 6 lines

From my experience with the arm64 spin-table, I would *strongly* recommend that you tighten this up, and define things more thoroughly, before a variety of FW/bootloader implementations appear. e.g.

* What initial/invalid value should the location contain? Is zero a valid address that you might want to jump a secondary CPU to?

I believe the hardware is implemented such that just the appearance of the address for write on the bus unblocks the secondary cpu waiting on it.

As for the address to jump to, this is provided by the kernel and, for Linux purposes, is always _stext. As I understand the mechanism, the actual initial PC for secondary cpus is in rom, and the code there is responsible for loading the application-desired (i.e. kernel-desired) initial PC and jumping to it.

Quoted 2 lines

* How must the value be written?
- Which endianness?

CPU native.

Quoted 2 lines

- With a single store? Or is there a more involved sequence to prevent the secondary CPU from seeing a torn value?

The start address is just a physical ram address (internal sram) and how it's written does not matter, because it's only read once the release write occurs.

Quoted 2 lines

* Must CPUs have a unique cpu-release-addr? I would *strongly* recommend that they do.

There is currently no spec or implementation with more than one secondary cpu.

Quoted 3 lines

- Is any minimal padding required around cpu-release-addrs? e.g. can FW or bootlaoder put data in the same cacheline-aligned region and exepct the OS not to corrupt this?

The word-sized memory (for J2, 32-bit) at the address is what's being addressed. There is no implicit license for the kernel to clobber other nearby data.

Quoted 1 line

* How should the OS map the region (i.e. which MMU/cache attributes)?

For J2, there is no mmu. All these specs need extension for future models with mmu, because the properties of the mmu should be described in the DT.

Quoted 1 line

- Is the address permitted to be a device register?

I don't see a strong reason to disallow it. Do you?

Quoted 5 lines

* Where can this memory live?
- Should it be abesnt from any memory node? To which padding?
- Should the memory be described with a memreserve? If so, what are your architecture-specific memreserve semantics (i.e. which MMU/cache attributes are permitted for a memreserve'd region)?

If it's in the memory regions exposed by the DT to the kernel, it should be reserved, I think. In practice it's not.

Quoted 3 lines

* What state should the CPU be in when it branches to the provided address?
- Must the MMU be off?

Current models are nommu.

Quoted 3 lines

- What state must any cache be in?
Should FW perform any implementation defined coherency and cache management prior to branching?

The current dcache implementation is fully coherent, but we want to relax that If this changes the hw/fw should ensure that the secondary cpu being started does not see stale dcache.

The icache requires explicit flush so secondary should start with it off (as it does now) or ensure that it's flushed before jumping to kernel-provided start address.

Quoted 1 line

- Must the CPU be in a particular endianness?

There are not any switchable-endian J-Core cpus. If such a thing is added it would make sense to describe this.

Quoted 1 line

- Which exceptions must be masked?

In practice I think it's the same as how cpu0 starts. I suspect that's with everything masked, but I'm not sure right off.

Quoted 1 line

- Are interrupts permitted to be pending?

In practice they won't be for current implementations, but I don't have an answer that can forsee the future.

Quoted 2 lines

- Should debug logic (e.g. breakpoint and watchpoints) be placed into a quiescent state?

This depends on having a model that has these features.

Quoted 1 line

- Should any registers be programmed with specific values?

No, beyond perhaps things like control registers (IMASK).

Quoted 18 lines

At some point, you are likely to want CPU hotplug and/or cpuidle. We didn't provision the arm64 spin-table for either of these and never extended it, but you may want to put in place some discoverability now to allow future OSs to use that new support while allowing current OSs to retain functional (e.g. not requiring a new enable-method string).

Documentation/devicetree/bindings/jcore/cpus.txt line 60

Quoted 9 lines

+---------------------

Documentation/devicetree/bindings/jcore/cpus.txt new file +8 -0

+Cache controller node
+---------------------
+
+Required properties:
+
+- compatible: Must be "jcore,cache".
+
+- reg: A memory range for the cache controller registers.

There is a well-defined memory map for the cache controller?

Quoted 3 lines

If so, please refer to documentation for it here (either in this section, or the top of this document if common with other elements described herein).

The current version "jcore,cache" has a single 32-bit control register per cpu that can be used to enable/disable/flush icache and/or dcache. There is no finer-grained control. If/when we do larger caches in the future where it makes sense, there will be a new binding for it. (For example it may make sense to do one that matches the original SH memory-mapped cache interface.)

Quoted 8 lines

Quoted 8 lines

+--------

Documentation/devicetree/bindings/jcore/cpus.txt new file +7 -0

+IPI node
+--------
+
+Device trees for SMP systems must have an IPI node representing the mechanism
+used for inter-processor interrupt generation.
+
+Required properties:

Quoted 8 lines

Documentation/devicetree/bindings/jcore/cpus.txt line 76

Quoted 6 lines

Documentation/devicetree/bindings/jcore/cpus.txt new file +6 -0

+
+- compatible: Must be "jcore,ipi-controller".
+
+- reg: A memory range used to IPI generation.
+
+- interrupts: An irq on which IPI will be received.

Likewise.

It's the same (actually even the same memory range, though I didn't see a sense in requiring that; there's also an IPI-generate bit).

Quoted 8 lines

Quoted 8 lines

+----------

Documentation/devicetree/bindings/jcore/cpus.txt new file +7 -0

+CPUID node
+----------
+
+Device trees for SMP systems must have a CPUID node representing the mechanism
+used to identify the current processor on which execution is taking place.
+
+Required properties:

Quoted 8 lines

Documentation/devicetree/bindings/jcore/cpus.txt line 92

Quoted 6 lines

Documentation/devicetree/bindings/jcore/cpus.txt new file +6 -0

+
+- compatible: Must be "jcore,cpuid-mmio".
+
+- reg: A memory range containing a single 32-bit mmio register which produces
+  the current cpu id (matching the "reg" property of the cpu performing the
+  read) when read.

Likewise.

One general question I have about all of your comments -- is the DT binding file really supposed to amount to a hardware programming manual, fully specifying all of the programming interfaces? I don't see that in other binding files, and it feels like what you're asking for is beyond the scope of just specifying the bindings.

If you really do want a lot more detail for SMP-related bindings, I could consider submitting a version with SMP omitted for now (since the kernel patches submitted at this point don't include SMP) and do the addition of SMP as a separate patch later. But with the launch of open-hardware boards capable of running SMP J2 systems (see
https://twitter.com/jcoreeng/status/730330848306700288) near, I'd like to be getting bindings we can use stabilized so that we're properly including DTB in the boot rom and not relying on external DTB files or linking DTB in kernel.

Quoted 35 lines · Rich Felker

Quoted 25 lines · Mark Rutland

Quoted 25 lines

Documentation/devicetree/bindings/jcore/cpus.txt new file +25 -0

+Optional properties:
+
+- enable-method: Required only for SMP systems. If present, must be
+  "jcore,spin-table".
+
+
+--------------------
+Individual cpu nodes
+--------------------
+
+Required properties:
+
+- device_type: Must be "cpu".
+
+- compatible: Must be "jcore,j2".
+
+- reg: Must be 0 on uniprocessor systems, or the sequential, zero-based
+  hardware cpu id on SMP systems.
+
+Optional properties:
+
+- clock-frequency: Clock frequency of the cpu in Hz.
+
+- cpu-release-addr: Necessary only for secondary processors on SMP systems
+  using the "jcore,spin-table" enable method. If present, must consist of

Quoted 8 lines

Documentation/devicetree/bindings/jcore/cpus.txt line 49

Quoted 4 lines

Documentation/devicetree/bindings/jcore/cpus.txt new file +4 -0

+  two cells containing physical addresses. The first cell contains an
+  address which, when written, unblocks the secondary cpu. The second cell
+  contains an address from which the cpu will read its initial program
+  counter when unblocked.

I take it this follows the example of the arm64 spin-table rather than the ePAPR spin-table, given the lack of an ePAPR reference or struct definition.

Quoted 2 lines

Indeed, I wasn't aware of the ePAPR spec for it. Would you prefer that we use a different name or something?

No, the "jcore,spin-table" name is fine.

Quoted 7 lines

Quoted 6 lines

From my experience with the arm64 spin-table, I would *strongly* recommend that you tighten this up, and define things more thoroughly, before a variety of FW/bootloader implementations appear. e.g.

* What initial/invalid value should the location contain? Is zero a valid address that you might want to jump a secondary CPU to?

Quoted 3 lines

I believe the hardware is implemented such that just the appearance of the address for write on the bus unblocks the secondary cpu waiting on it.

Ok, so this is effectively a device register, rather than a location in "real" memory.

Quoted 5 lines

As for the address to jump to, this is provided by the kernel and, for Linux purposes, is always _stext. As I understand the mechanism, the actual initial PC for secondary cpus is in rom, and the code there is responsible for loading the application-desired (i.e. kernel-desired) initial PC and jumping to it.

Ok. Is this second address also a device register, or might this be in "real" memory?

Quoted 4 lines

Quoted 2 lines

* How must the value be written?
- Which endianness?

CPU native.

Ok. I take it that a CPU's endianness cannot be switched onthe fly, then? Or does the hardware backing the release-addr register handle arbitrary endianness dynamically?

If you want to reuse the same HW block across configurations where CPU endianness differs, it may make sense to define an endianness regardless, to simplify integration concerns.

Quoted 6 lines

Quoted 2 lines

- With a single store? Or is there a more involved sequence to prevent the secondary CPU from seeing a torn value?

The start address is just a physical ram address (internal sram) and how it's written does not matter, because it's only read once the release write occurs.

Sure. I had initially mis-read the documentation and applied my understanding of the arm64 spin-table sequence (which only has a single write for both purposes).

For the actual release write are there any constraints? e.g. value, size of access?

Quoted 5 lines

Quoted 2 lines

* Must CPUs have a unique cpu-release-addr? I would *strongly* recommend that they do.

There is currently no spec or implementation with more than one secondary cpu.

Ok. Please bear the above in mind if/when implementations with more than two secondary CPUs are conceivable.

Quoted 7 lines

Quoted 3 lines

- Is any minimal padding required around cpu-release-addrs? e.g. can FW or bootlaoder put data in the same cacheline-aligned region and exepct the OS not to corrupt this?

The word-sized memory (for J2, 32-bit) at the address is what's being addressed. There is no implicit license for the kernel to clobber other nearby data.

My concern was that if your memory system is not fully coherent, and the CPU has a cacheable mapping of the initial program counter field, it would need to perform a cache clean to ensure visibility of that field. If the cache line for that region were stale, that would clobber data in the same cache line (e.g. something owned/used by FW).

Per your comments below that doesn't matter now but may in future.

Quoted 5 lines

Quoted 1 line

* How should the OS map the region (i.e. which MMU/cache attributes)?

For J2, there is no mmu. All these specs need extension for future models with mmu, because the properties of the mmu should be described in the DT.

I was going by the fact you had a binding for a cache, which I assumed was SW configurable. If that's not the case, then my questions about caches and MMU attributes do not apply for the timebeing.

Quoted 3 lines

Quoted 1 line

- Is the address permitted to be a device register?

I don't see a strong reason to disallow it. Do you?

So long as you can guarantee that OS does not have a cacheable mapping of this region, and the size of the access wis well-defined, I do not see a reason to disallow it.

Quoted 8 lines

Quoted 5 lines

* Where can this memory live?
- Should it be abesnt from any memory node? To which padding?
- Should the memory be described with a memreserve? If so, what are your architecture-specific memreserve semantics (i.e. which MMU/cache attributes are permitted for a memreserve'd region)?

If it's in the memory regions exposed by the DT to the kernel, it should be reserved, I think. In practice it's not.

Ok. This should be documented (as we do for the arm64 spin-table).

Perhaps that is not a major problem if the OS never pokes the release register.

If you do /memreserve/ the region rather than carving it out of memory nodes, you will also need to define semantics for memreserve. Typically memreserve meaans that the OS should not perform any stores to the region, but is permitted to map it with some architecture-specific cacheable attributes.

Quoted 5 lines

One general question I have about all of your comments -- is the DT binding file really supposed to amount to a hardware programming manual, fully specifying all of the programming interfaces? I don't see that in other binding files, and it feels like what you're asking for is beyond the scope of just specifying the bindings.

The binding file is not intended to be a full HW description, but where possible relevant documentation should be referred to, in case there is some ambiguity.

My questions about SMP are largely orthogonal to DT; I simply have experience in dealing with that for arm64, and am aware of some of the pain points that were not immediately obvious.

Quoted 9 lines

If you really do want a lot more detail for SMP-related bindings, I could consider submitting a version with SMP omitted for now (since the kernel patches submitted at this point don't include SMP) and do the addition of SMP as a separate patch later. But with the launch of open-hardware boards capable of running SMP J2 systems (see
https://twitter.com/jcoreeng/status/730330848306700288) near, I'd like to be getting bindings we can use stabilized so that we're properly including DTB in the boot rom and not relying on external DTB files or linking DTB in kernel.

I would argue that the SMP bindings can be added at the same point as the code. If there's any chance that something may change, having the bindings in the kernel early gives a potentially misleading impression of stability.

I assume that you have the facility to upgrade the boot ROM?

Quoted 10 lines · Mark Rutland

Quoted 5 lines · Rich Felker

Quoted 2 lines · Mark Rutland

* How must the value be written?
- Which endianness?

CPU native.

Ok. I take it that a CPU's endianness cannot be switched onthe fly, then? Or does the hardware backing the release-addr register handle arbitrary endianness dynamically?

No, it's not switched on the fly.

Quoted 3 lines

If you want to reuse the same HW block across configurations where CPU endianness differs, it may make sense to define an endianness regardless, to simplify integration concerns.

The existing cpus are all big-endian, but I believe at one point there was talk that it's easy to get a little-endian version if you want. In any case the value is to be read by the cpu, so cpu endianness (i.e. no endianness, just a value) is the only thing that makes sense to specify. Adding a fixed endian spec independent of cpu endianness just complicates both hardware and kernel implementation and its only benefit seems to be supporting runtime-switchable chips where the entry-point code has to select the endianness to match the rest of the kernel.

Quoted 5 lines

If you do /memreserve/ the region rather than carving it out of memory nodes, you will also need to define semantics for memreserve. Typically memreserve meaans that the OS should not perform any stores to the region, but is permitted to map it with some architecture-specific cacheable attributes.

My interpretation of memreserve is just that it marks memory ranges that the kernel cannot use for allocatable memory for its own purposes, despite otherwise possibly lying in the range of a "memory" node. I would not interpret it as excluding accesses by drivers that were told to use specific addresses in the "reserved" range as part of their DT bindings.

Quoted 3 lines

My questions about SMP are largely orthogonal to DT; I simply have experience in dealing with that for arm64, and am aware of some of the pain points that were not immediately obvious.

OK, thanks for clarifying that. This is actually really helpful feedback to have but I wasn't sure if you wanted me to consider it part of what needs to be done for DT binding or for consideration in designing and documenting the SMP architecture.

OK. I'll strip it down to just the parts that are relevant for non-SMP and submit the patch adding SMP bindings along with the SMP kernel patches.

Quoted 3 lines · Mark Rutland

* What state should the CPU be in when it branches to the provided address?
- Must the MMU be off?

Current models are nommu.

Quoted 1 line

At some point, you are likely to want CPU hotplug and/or cpuidle.

Quoted 6 lines

We
didn't provision the arm64 spin-table for either of these and never extended it, but you may want to put in place some discoverability now to allow future OSs to use that new support while allowing current OSs to retain functional (e.g. not requiring a new enable-method string).

Quoted 11 lines

Documentation/devicetree/bindings/jcore/cpus.txt line 60

Quoted 9 lines

+---------------------

Documentation/devicetree/bindings/jcore/cpus.txt new file +8 -0

+Cache controller node
+---------------------
+
+Required properties:
+
+- compatible: Must be "jcore,cache".
+
+- reg: A memory range for the cache controller registers.

There is a well-defined memory map for the cache controller?

Quoted 3 lines

If so, please refer to documentation for it here (either in this section, or the top of this document if common with other elements described herein).

The current version "jcore,cache" has a single 32-bit control register per cpu that can be used to enable/disable/flush icache and/or dcache. There is no finer-grained control. If/when we do larger caches in the future where it makes sense, there will be a new binding for it. (For example it may make sense to do one that matches the original SH memory-mapped cache interface.)

Quoted 5 lines

Documentation/devicetree/bindings/jcore/cpus.txt line 92

Quoted 3 lines

+- reg: A memory range containing a single 32-bit mmio register which produces

Documentation/devicetree/bindings/jcore/cpus.txt new file +2 -0

+  the current cpu id (matching the "reg" property of the cpu performing the
+  read) when read.

Likewise.

One general question I have about all of your comments -- is the DT binding file really supposed to amount to a hardware programming manual, fully specifying all of the programming interfaces? I don't see that in other binding files, and it feels like what you're asking for is beyond the scope of just specifying the bindings.

As far as I know, we're the first nommu SMP implementation in Linux.

Signed-off-by Rich Felker <[email protected]>

---
 .../bindings/interrupt-controller/jcore,aic.txt    | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/jcore,aic.txt

Documentation/devicetree/bindings/interrupt-controller/jcore,aic.txt new file +9 -0

new file mode 100644
index 0000000..5dc99b9
@@ -0,0 +1,29 @@
+J-Core Advanced Interrupt Controller
+
+Required properties:
+
+- compatible : Should be "jcore,aic1" for the (obsolete) first-generation aic
+  with 8 interrupt lines with programmable priorities, or "jcore,aic2" for
+  the "aic2" core with 64 interrupts.
+
+- reg : Memory region for configuration.

Documentation/devicetree/bindings/interrupt-controller/jcore,aic.txt new file +10 -0

+
+
+Example:
+
+aic: interrupt-controller@200 {
+	compatible = "jcore,aic2";
+	reg = < 0x200 0x10 >;
+	interrupt-controller;
+	#interrupt-cells = <1>;
+};

Signature · 1 line

2.8.1

Quoted 28 lines · Rich Felker

Signed-off-by Rich Felker <[email protected]>

---
 .../bindings/interrupt-controller/jcore,aic.txt    | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/jcore,aic.txt

Documentation/devicetree/bindings/interrupt-controller/jcore,aic.txt new file +16 -0

new file mode 100644
index 0000000..5dc99b9
@@ -0,0 +1,29 @@
+J-Core Advanced Interrupt Controller
+
+Required properties:
+
+- compatible : Should be "jcore,aic1" for the (obsolete) first-generation aic
+  with 8 interrupt lines with programmable priorities, or "jcore,aic2" for
+  the "aic2" core with 64 interrupts.
+
+- reg : Memory region for configuration.
+
+- 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.
+
+Optional properties:

Documentation/devicetree/bindings/interrupt-controller/jcore,aic.txt line 19

Quoted 3 lines

Documentation/devicetree/bindings/interrupt-controller/jcore,aic.txt new file +3 -0

+
+- cpu-offset : For SMP, the offset to the per-cpu memory region for
+  configuration, to be scaled by the cpu number.

I take is that "cpu number" means the "sequential, zero-based hardware cpu id" defined in patch 2. I would recommend that you explicitly mention that (e.g. here say "hardware cpu id" rather than "cpu number"), so as to not have this confused with Linux logical IDs.

Signed-off-by Rich Felker <[email protected]>

---
 .../devicetree/bindings/timer/jcore,pit.txt        | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/jcore,pit.txt

Documentation/devicetree/bindings/timer/jcore,pit.txt new file +16 -0

new file mode 100644
index 0000000..96c6815
@@ -0,0 +1,28 @@
+J-Core Programmable Interval Timer and Clocksource
+
+Required properties:
+
+- compatible: Must be "jcore,pit".
+
+- reg: Memory region for timer/clocksource registers.
+
+- interrupts: An interrupt to assign for the timer. The actual pit
+  core is integrated with the aic and allows the timer interrupt
+  assignment to be programmed by software, but this property is
+  required in order to reserve an interrupt number that doesn't
+  conflict with other devices.
+
+Optional properties:
+

Quoted 33 lines · Rich Felker

Signed-off-by Rich Felker <[email protected]>

---
 .../devicetree/bindings/timer/jcore,pit.txt        | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/jcore,pit.txt

Documentation/devicetree/bindings/timer/jcore,pit.txt new file +21 -0

new file mode 100644
index 0000000..96c6815
@@ -0,0 +1,28 @@
+J-Core Programmable Interval Timer and Clocksource
+
+Required properties:
+
+- compatible: Must be "jcore,pit".
+
+- reg: Memory region for timer/clocksource registers.
+
+- interrupts: An interrupt to assign for the timer. The actual pit
+  core is integrated with the aic and allows the timer interrupt
+  assignment to be programmed by software, but this property is
+  required in order to reserve an interrupt number that doesn't
+  conflict with other devices.
+
+Optional properties:
+
+- cpu-offset: For SMP, the per-cpu offset to the local timer
+  programming memory range.
+
+
+Example:

Documentation/devicetree/bindings/timer/jcore,pit.txt line 26

Quoted 5 lines

Documentation/devicetree/bindings/timer/jcore,pit.txt new file +5 -0

+
+timer@200 {
+	compatible = "jcore,pit";
+	reg = < 0x200 0x30 >;
+	cpu-offset = < 0x300 >;

This is outside the reg range. Perhaps reg should include each range of per cpu registers.

Quoted 34 lines · Rob Herring

Quoted 33 lines · Rich Felker

Signed-off-by Rich Felker <[email protected]>

---
 .../devicetree/bindings/timer/jcore,pit.txt        | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/jcore,pit.txt

Documentation/devicetree/bindings/timer/jcore,pit.txt new file +21 -0

new file mode 100644
index 0000000..96c6815
@@ -0,0 +1,28 @@
+J-Core Programmable Interval Timer and Clocksource
+
+Required properties:
+
+- compatible: Must be "jcore,pit".
+
+- reg: Memory region for timer/clocksource registers.
+
+- interrupts: An interrupt to assign for the timer. The actual pit
+  core is integrated with the aic and allows the timer interrupt
+  assignment to be programmed by software, but this property is
+  required in order to reserve an interrupt number that doesn't
+  conflict with other devices.
+
+Optional properties:
+
+- cpu-offset: For SMP, the per-cpu offset to the local timer
+  programming memory range.
+
+
+Example:

Quoted 8 lines

Documentation/devicetree/bindings/timer/jcore,pit.txt line 26

Quoted 5 lines

Documentation/devicetree/bindings/timer/jcore,pit.txt new file +5 -0

+
+timer@200 {
+	compatible = "jcore,pit";
+	reg = < 0x200 0x30 >;
+	cpu-offset = < 0x300 >;

This is outside the reg range. Perhaps reg should include each range of per cpu registers.

In the hardware, each timer instance is mapped independently so there's no fundamental reason they need to be mapped sufficiently close that it would make sense for a single virtual mapping to cover them all. This doesn't matter for nommu but it would with mmu in the future. In the driver I've updated it to ioremap each percpu instance separately (as its own memory range) using the cpu-offset applied to the range obtained from "reg". Is this acceptable (in which case I suppose the binding needs to be documented that "reg" just covers the cpu0 instance's range)? Do you think it would be preferable to have multiple "reg" ranges indexed by cpu instead of cpu-offset?

In theory it would even be possible to just require a DT node per cpulocal timer, but I didn't see a good way to make the bindings represent the relationship to cpus or to make the driver handle irqs correctly for such a setup, so I'd need a viable proposal for how that could be done to even consider such an approach.

Quoted 34 lines · Rob Herring

Quoted 33 lines · Rich Felker

Signed-off-by Rich Felker <[email protected]>

---
 .../devicetree/bindings/timer/jcore,pit.txt        | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/jcore,pit.txt

Documentation/devicetree/bindings/timer/jcore,pit.txt new file +21 -0

new file mode 100644
index 0000000..96c6815
@@ -0,0 +1,28 @@
+J-Core Programmable Interval Timer and Clocksource
+
+Required properties:
+
+- compatible: Must be "jcore,pit".
+
+- reg: Memory region for timer/clocksource registers.
+
+- interrupts: An interrupt to assign for the timer. The actual pit
+  core is integrated with the aic and allows the timer interrupt
+  assignment to be programmed by software, but this property is
+  required in order to reserve an interrupt number that doesn't
+  conflict with other devices.
+
+Optional properties:
+
+- cpu-offset: For SMP, the per-cpu offset to the local timer
+  programming memory range.
+
+
+Example:

Quoted 8 lines

Documentation/devicetree/bindings/timer/jcore,pit.txt line 26

Quoted 5 lines

Documentation/devicetree/bindings/timer/jcore,pit.txt new file +5 -0

+
+timer@200 {
+   compatible = "jcore,pit";
+   reg = < 0x200 0x30 >;
+   cpu-offset = < 0x300 >;

This is outside the reg range. Perhaps reg should include each range of per cpu registers.

In the hardware, each timer instance is mapped independently so there's no fundamental reason they need to be mapped sufficiently close that it would make sense for a single virtual mapping to cover them all. This doesn't matter for nommu but it would with mmu in the future. In the driver I've updated it to ioremap each percpu instance separately (as its own memory range) using the cpu-offset applied to the range obtained from "reg". Is this acceptable (in which case I suppose the binding needs to be documented that "reg" just covers the cpu0 instance's range)? Do you think it would be preferable to have multiple "reg" ranges indexed by cpu instead of cpu-offset?

In theory it would even be possible to just require a DT node per cpulocal timer, but I didn't see a good way to make the bindings represent the relationship to cpus or to make the driver handle irqs correctly for such a setup, so I'd need a viable proposal for how that could be done to even consider such an approach.

Quoted 44 lines · Rich Felker

Quoted 34 lines · Rob Herring

Quoted 33 lines · Rich Felker

Signed-off-by Rich Felker <[email protected]>

---
 .../devicetree/bindings/timer/jcore,pit.txt        | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/jcore,pit.txt

Documentation/devicetree/bindings/timer/jcore,pit.txt new file +21 -0

new file mode 100644
index 0000000..96c6815
@@ -0,0 +1,28 @@
+J-Core Programmable Interval Timer and Clocksource
+
+Required properties:
+
+- compatible: Must be "jcore,pit".
+
+- reg: Memory region for timer/clocksource registers.
+
+- interrupts: An interrupt to assign for the timer. The actual pit
+  core is integrated with the aic and allows the timer interrupt
+  assignment to be programmed by software, but this property is
+  required in order to reserve an interrupt number that doesn't
+  conflict with other devices.
+
+Optional properties:
+
+- cpu-offset: For SMP, the per-cpu offset to the local timer
+  programming memory range.
+
+
+Example:

Quoted 8 lines

Documentation/devicetree/bindings/timer/jcore,pit.txt line 26

Quoted 5 lines

Documentation/devicetree/bindings/timer/jcore,pit.txt new file +5 -0

+
+timer@200 {
+   compatible = "jcore,pit";
+   reg = < 0x200 0x30 >;
+   cpu-offset = < 0x300 >;

This is outside the reg range. Perhaps reg should include each range of per cpu registers.

Quoted 10 lines

In the hardware, each timer instance is mapped independently so there's no fundamental reason they need to be mapped sufficiently close that it would make sense for a single virtual mapping to cover them all. This doesn't matter for nommu but it would with mmu in the future. In the driver I've updated it to ioremap each percpu instance separately (as its own memory range) using the cpu-offset applied to the range obtained from "reg". Is this acceptable (in which case I suppose the binding needs to be documented that "reg" just covers the cpu0 instance's range)? Do you think it would be preferable to have multiple "reg" ranges indexed by cpu instead of cpu-offset?

Yes. Either reg should cover the full range of addresses or you should have multiple reg values with one for each cpu. I prefer the latter as it doesn't create a custom property for expressing register addresses.

Quoted 5 lines

In theory it would even be possible to just require a DT node per cpulocal timer, but I didn't see a good way to make the bindings represent the relationship to cpus or to make the driver handle irqs correctly for such a setup, so I'd need a viable proposal for how that could be done to even consider such an approach.

Yeah, there's not really a standard way to map per cpu blocks to cpus. We could, but doesn't really seem necessary here.

For the irqs, percpu irqs doesn't help you?

Quoted 56 lines · Rob Herring

Quoted 44 lines · Rich Felker

Quoted 34 lines · Rob Herring

Quoted 33 lines · Rich Felker

Signed-off-by Rich Felker <[email protected]>

---
 .../devicetree/bindings/timer/jcore,pit.txt        | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/jcore,pit.txt

Documentation/devicetree/bindings/timer/jcore,pit.txt new file +21 -0

new file mode 100644
index 0000000..96c6815
@@ -0,0 +1,28 @@
+J-Core Programmable Interval Timer and Clocksource
+
+Required properties:
+
+- compatible: Must be "jcore,pit".
+
+- reg: Memory region for timer/clocksource registers.
+
+- interrupts: An interrupt to assign for the timer. The actual pit
+  core is integrated with the aic and allows the timer interrupt
+  assignment to be programmed by software, but this property is
+  required in order to reserve an interrupt number that doesn't
+  conflict with other devices.
+
+Optional properties:
+
+- cpu-offset: For SMP, the per-cpu offset to the local timer
+  programming memory range.
+
+
+Example:

Quoted 8 lines

Documentation/devicetree/bindings/timer/jcore,pit.txt line 26

Quoted 5 lines

Documentation/devicetree/bindings/timer/jcore,pit.txt new file +5 -0

+
+timer@200 {
+   compatible = "jcore,pit";
+   reg = < 0x200 0x30 >;
+   cpu-offset = < 0x300 >;

This is outside the reg range. Perhaps reg should include each range of per cpu registers.

Quoted 10 lines

In the hardware, each timer instance is mapped independently so there's no fundamental reason they need to be mapped sufficiently close that it would make sense for a single virtual mapping to cover them all. This doesn't matter for nommu but it would with mmu in the future. In the driver I've updated it to ioremap each percpu instance separately (as its own memory range) using the cpu-offset applied to the range obtained from "reg". Is this acceptable (in which case I suppose the binding needs to be documented that "reg" just covers the cpu0 instance's range)? Do you think it would be preferable to have multiple "reg" ranges indexed by cpu instead of cpu-offset?

Quoted 3 lines

Yes. Either reg should cover the full range of addresses or you should have multiple reg values with one for each cpu. I prefer the latter as it doesn't create a custom property for expressing register addresses.

OK, let's lean towards going with the latter then. It's more flexible and allows any placement of the maps; the offset per cpu does not have to be constant. It also makes the driver code simpler since of_iomap can be used directly rather than requiring of_address_to_resource, physical address arithmetic arithmetic, and ioremap on the results. Is there any precedent for using multiple reg ranges for percpu register instances like this? Anything else they've been used for?

Quoted 11 lines

Quoted 6 lines

In theory it would even be possible to just require a DT node per cpulocal timer, but I didn't see a good way to make the bindings represent the relationship to cpus or to make the driver handle irqs correctly for such a setup, so I'd need a viable proposal for how that could be done to even consider such an approach.

Yeah, there's not really a standard way to map per cpu blocks to cpus. We could, but doesn't really seem necessary here.

For the irqs, percpu irqs doesn't help you?

What I mean is that, if there were a separate device node and driver instance per cpu, they'd all want to register the same irq just to handle it on their own cpu, so we'd have a lot of spurious handlers running. The right way to model this, I think, would be as a virtual irqchip that's the irq parent of all the timer nodes, and that multiplexes the real irq to one virq per cpu (where the current cpu id becomes the irq number in its irq domain). But that's a lot of virtual infrastructure just for the sake of modelling each percpu timer as its own DT node and I don't think it makes sense to do it that way.

Quoted 11 lines

Quoted 10 lines

Quoted 5 lines

In theory it would even be possible to just require a DT node per cpulocal timer, but I didn't see a good way to make the bindings represent the relationship to cpus or to make the driver handle irqs correctly for such a setup, so I'd need a viable proposal for how that could be done to even consider such an approach.

Yeah, there's not really a standard way to map per cpu blocks to cpus. We could, but doesn't really seem necessary here.

For the irqs, percpu irqs doesn't help you?

Quoted 9 lines

What I mean is that, if there were a separate device node and driver instance per cpu, they'd all want to register the same irq just to handle it on their own cpu, so we'd have a lot of spurious handlers running. The right way to model this, I think, would be as a virtual irqchip that's the irq parent of all the timer nodes, and that multiplexes the real irq to one virq per cpu (where the current cpu id becomes the irq number in its irq domain). But that's a lot of virtual infrastructure just for the sake of modelling each percpu timer as its own DT node and I don't think it makes sense to do it that way.

I would have thought your interrupt controller did all this. On the ARM GIC for example, you have the same irq number but there is a per cpu interface and really N (== # cpus) physical irq lines.

Quoted 21 lines · Rob Herring

Quoted 11 lines

Quoted 10 lines

Quoted 5 lines

In theory it would even be possible to just require a DT node per cpulocal timer, but I didn't see a good way to make the bindings represent the relationship to cpus or to make the driver handle irqs correctly for such a setup, so I'd need a viable proposal for how that could be done to even consider such an approach.

Yeah, there's not really a standard way to map per cpu blocks to cpus. We could, but doesn't really seem necessary here.

For the irqs, percpu irqs doesn't help you?

Quoted 9 lines

What I mean is that, if there were a separate device node and driver instance per cpu, they'd all want to register the same irq just to handle it on their own cpu, so we'd have a lot of spurious handlers running. The right way to model this, I think, would be as a virtual irqchip that's the irq parent of all the timer nodes, and that multiplexes the real irq to one virq per cpu (where the current cpu id becomes the irq number in its irq domain). But that's a lot of virtual infrastructure just for the sake of modelling each percpu timer as its own DT node and I don't think it makes sense to do it that way.

Quoted 3 lines

I would have thought your interrupt controller did all this. On the ARM GIC for example, you have the same irq number but there is a per cpu interface and really N (== # cpus) physical irq lines.

I've looked at the ARM GIC code and bindings and I don't see where the per-cpu interrupt interfaces are modelled with multiple interrupt controller nodes or irq domains. It looks to me like it just uses a single interrupt controller/domain with percpu irq. Does that match your understanding?

Signed-off-by Rich Felker <[email protected]>

---
 .../devicetree/bindings/spi/jcore,spi.txt          | 23 ++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/jcore,spi.txt

There are two versions of the J-Core interrupt controller in use, aic1 which generates interrupts with programmable priorities, but only supports 8 irq lines and maps them to cpu traps in the range 17 to 24, and aic2 which uses traps in the range 64-127 and supports up to 128 irqs, with priorities dependent on the interrupt number. The Linux driver does not make use of priorities anyway.

For simplicity, there is no aic1-specific logic in the driver beyond setting the priority register, which is necessary for interrupts to work at all. Eventually aic1 will likely be phased out, but it's currently in use in deployments and all released bitstream binaries.

Signed-off-by Rich Felker <[email protected]>

---

This version fixes a missing "static" reported in the previous version of the patch. I missed Marc Zyngier's reply to the v2 when preparing the v3 patch series, but I'm still not sure exactly what, if any, changes should come out of that discussion, and they'll probably mostly in the area of comments or changelog.

 drivers/irqchip/Kconfig         |  6 +++
 drivers/irqchip/Makefile        |  1 +
 drivers/irqchip/irq-jcore-aic.c | 95 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 102 insertions(+)
 create mode 100644 drivers/irqchip/irq-jcore-aic.c

drivers/irqchip/Kconfig +6 -0

index 3e12479..3cb37d6 100644
@@ -149,6 +149,12 @@ config PIC32_EVIC
 	select GENERIC_IRQ_CHIP
 	select IRQ_DOMAIN
 
+config JCORE_AIC
+	bool "J-Core integrated AIC"
+	select IRQ_DOMAIN
+	help
+	  Support for the J-Core integrated AIC.
+
 config RENESAS_INTC_IRQPIN
 	bool
 	select IRQ_DOMAIN

drivers/irqchip/Makefile +1 -0

index b03cfcb..5a1f1bf 100644
@@ -37,6 +37,7 @@ obj-$(CONFIG_I8259)			+= irq-i8259.o
 obj-$(CONFIG_IMGPDC_IRQ)		+= irq-imgpdc.o
 obj-$(CONFIG_IRQ_MIPS_CPU)		+= irq-mips-cpu.o
 obj-$(CONFIG_SIRF_IRQ)			+= irq-sirfsoc.o
+obj-$(CONFIG_JCORE_AIC)			+= irq-jcore-aic.o
 obj-$(CONFIG_RENESAS_INTC_IRQPIN)	+= irq-renesas-intc-irqpin.o
 obj-$(CONFIG_RENESAS_IRQC)		+= irq-renesas-irqc.o
 obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o

drivers/irqchip/irq-jcore-aic.c new file +95 -0

new file mode 100644
index 0000000..1348cdb
@@ -0,0 +1,95 @@
+/*
+ * J-Core SoC AIC driver
+ *
+ * Copyright (C) 2015-2016 Smart Energy Instruments, Inc.
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#include <linux/irq.h>
+#include <linux/io.h>
+#include <linux/irqchip.h>
+#include <linux/irqdomain.h>
+#include <linux/module.h>
+#include <linux/cpu.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+
+#define AIC1_INTPRI 8
+
+static struct aic_data {
+	unsigned char __iomem *base;
+	u32 cpu_offset;
+	struct irq_chip chip;
+	struct irq_domain *domain;
+	struct notifier_block nb;
+} aic_data;
+
+static int aic_irqdomain_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hwirq)
+{
+	struct aic_data *aic = d->host_data;
+
+	irq_set_chip_data(irq, aic);
+	irq_set_chip_and_handler(irq, &aic->chip, handle_simple_irq);
+	irq_set_probe(irq);
+
+	return 0;
+}
+
+static const struct irq_domain_ops aic_irqdomain_ops = {
+	.map = aic_irqdomain_map,
+	.xlate = irq_domain_xlate_onecell,
+};
+
+static void noop(struct irq_data *data)
+{
+}
+
+static void aic1_localenable(struct aic_data *aic)
+{
+	unsigned cpu = smp_processor_id();
+	pr_info("Local AIC enable on cpu %u\n", cpu);
+	writel(0xffffffff, aic->base + cpu * aic->cpu_offset + AIC1_INTPRI);
+}
+
+static int aic1_cpu_notify(struct notifier_block *self, unsigned long action, void *hcpu)
+{
+	switch (action & ~CPU_TASKS_FROZEN) {
+	case CPU_STARTING:
+		aic1_localenable(container_of(self, struct aic_data, nb));
+		break;
+	}
+	return NOTIFY_OK;
+}
+
+int __init aic_irq_of_init(struct device_node *node, struct device_node *parent)
+{
+	struct aic_data *aic = &aic_data;
+
+	aic->base = of_iomap(node, 0);
+	of_property_read_u32(node, "cpu-offset", &aic->cpu_offset);
+
+	pr_info("Initializing J-Core AIC at %p\n", aic->base);
+
+	if (of_device_is_compatible(node, "jcore,aic1")) {
+		/* For aic1, need to enabled zero-priority-by-default irqs */
+		aic->nb.notifier_call = aic1_cpu_notify;
+		register_cpu_notifier(&aic->nb);
+		aic1_localenable(aic);
+	}
+
+	aic->chip.name = node->name;
+	aic->chip.irq_mask = noop;
+	aic->chip.irq_unmask = noop;
+
+	aic->domain = irq_domain_add_linear(node, 128, &aic_irqdomain_ops, aic);
+	irq_create_strict_mappings(aic->domain, 16, 16, 112);
+
+	return 0;
+}
+
+IRQCHIP_DECLARE(jcore_aic2, "jcore,aic2", aic_irq_of_init);
+IRQCHIP_DECLARE(jcore_aic1, "jcore,aic1", aic_irq_of_init);

Signature · 7 lines

2.8.1


--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 11 lines · Rich Felker

There are two versions of the J-Core interrupt controller in use, aic1 which generates interrupts with programmable priorities, but only supports 8 irq lines and maps them to cpu traps in the range 17 to 24, and aic2 which uses traps in the range 64-127 and supports up to 128 irqs, with priorities dependent on the interrupt number. The Linux driver does not make use of priorities anyway.

For simplicity, there is no aic1-specific logic in the driver beyond setting the priority register, which is necessary for interrupts to work at all. Eventually aic1 will likely be phased out, but it's currently in use in deployments and all released bitstream binaries.

Any comments on changes I should make in resubmitting this for the 4.8 merge window? I could somewhat restrict the possible irqs, but aic1 allows the pit to be programmed to generate any trap number you want, despite the hard-wired interrupts being limited to the range 17-24.

It might make sense to do something to allow percpu irq requests, but I couldn't figure out how to make them work and the current timer driver (that needs per-cpu irq delivery) just requests its irq through normal request_irq and lets the hardware do the right thing.

There are two versions of the J-Core interrupt controller in use, aic1 which generates interrupts with programmable priorities, but only supports 8 irq lines and maps them to cpu traps in the range 17 to 24, and aic2 which uses traps in the range 64-127 and supports up to 128 irqs, with priorities dependent on the interrupt number. The Linux driver does not make use of priorities anyway.

For simplicity, there is no aic1-specific logic in the driver beyond setting the priority register, which is necessary for interrupts to work at all. Eventually aic1 will likely be phased out, but it's currently in use in deployments and all released bitstream binaries.

Signed-off-by Rich Felker <[email protected]>

---

This version fixes a missing "static" reported in the previous version of the patch. I missed Marc Zyngier's reply to the v2 when preparing the v3 patch series, but I'm still not sure exactly what, if any, changes should come out of that discussion, and they'll probably mostly in the area of comments or changelog.

 drivers/irqchip/Kconfig         |  6 +++
 drivers/irqchip/Makefile        |  1 +
 drivers/irqchip/irq-jcore-aic.c | 95 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 102 insertions(+)
 create mode 100644 drivers/irqchip/irq-jcore-aic.c

drivers/irqchip/Kconfig +6 -0

index 3e12479..3cb37d6 100644
@@ -149,6 +149,12 @@ config PIC32_EVIC
        select GENERIC_IRQ_CHIP
        select IRQ_DOMAIN
 
+config JCORE_AIC
+       bool "J-Core integrated AIC"
+       select IRQ_DOMAIN
+       help
+         Support for the J-Core integrated AIC.
+
 config RENESAS_INTC_IRQPIN
        bool
        select IRQ_DOMAIN

drivers/irqchip/Makefile +1 -0

index b03cfcb..5a1f1bf 100644
@@ -37,6 +37,7 @@ obj-$(CONFIG_I8259)                   += irq-i8259.o
 obj-$(CONFIG_IMGPDC_IRQ)               += irq-imgpdc.o
 obj-$(CONFIG_IRQ_MIPS_CPU)             += irq-mips-cpu.o
 obj-$(CONFIG_SIRF_IRQ)                 += irq-sirfsoc.o
+obj-$(CONFIG_JCORE_AIC)                        += irq-jcore-aic.o
 obj-$(CONFIG_RENESAS_INTC_IRQPIN)      += irq-renesas-intc-irqpin.o
 obj-$(CONFIG_RENESAS_IRQC)             += irq-renesas-irqc.o
 obj-$(CONFIG_VERSATILE_FPGA_IRQ)       += irq-versatile-fpga.o

drivers/irqchip/irq-jcore-aic.c new file +5 -0

new file mode 100644
index 0000000..1348cdb
@@ -0,0 +1,95 @@
+/*
+ * J-Core SoC AIC driver
+ *
+ * Copyright (C) 2015-2016 Smart Energy Instruments, Inc.
+ *

Quoted 12 lines

+++ b/drivers/irqchip/irq-jcore-aic.c

drivers/irqchip/irq-jcore-aic.c new file +10 -0

@@ -0,0 +1,95 @@
+/*
+ * J-Core SoC AIC driver
+ *
+ * Copyright (C) 2015-2016 Smart Energy Instruments, Inc.
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+

drivers/irqchip/irq-jcore-aic.c line 15

Quoted 5 lines

drivers/irqchip/irq-jcore-aic.c new file +5 -0

+#include <linux/irq.h>
+#include <linux/io.h>
+#include <linux/irqchip.h>
+#include <linux/irqdomain.h>
+#include <linux/module.h>

Please don't use module.h or MODULE_* in code that is controlled by a bool Kconfig.

There are two versions of the J-Core interrupt controller in use, aic1 which generates interrupts with programmable priorities, but only supports 8 irq lines and maps them to cpu traps in the range 17 to 24, and aic2 which uses traps in the range 64-127 and supports up to 128 irqs, with priorities dependent on the interrupt number. The Linux driver does not make use of priorities anyway.

For simplicity, there is no aic1-specific logic in the driver beyond setting the priority register, which is necessary for interrupts to work at all. Eventually aic1 will likely be phased out, but it's currently in use in deployments and all released bitstream binaries.

Signed-off-by Rich Felker <[email protected]>

---

This version fixes a missing "static" reported in the previous version of the patch. I missed Marc Zyngier's reply to the v2 when preparing the v3 patch series, but I'm still not sure exactly what, if any, changes should come out of that discussion, and they'll probably mostly in the area of comments or changelog.

 drivers/irqchip/Kconfig         |  6 +++
 drivers/irqchip/Makefile        |  1 +
 drivers/irqchip/irq-jcore-aic.c | 95 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 102 insertions(+)
 create mode 100644 drivers/irqchip/irq-jcore-aic.c

drivers/irqchip/Kconfig +6 -0

index 3e12479..3cb37d6 100644
@@ -149,6 +149,12 @@ config PIC32_EVIC
 	select GENERIC_IRQ_CHIP
 	select IRQ_DOMAIN
 
+config JCORE_AIC
+	bool "J-Core integrated AIC"
+	select IRQ_DOMAIN
+	help
+	  Support for the J-Core integrated AIC.
+
 config RENESAS_INTC_IRQPIN
 	bool
 	select IRQ_DOMAIN

drivers/irqchip/Makefile +1 -0

index b03cfcb..5a1f1bf 100644
@@ -37,6 +37,7 @@ obj-$(CONFIG_I8259)			+= irq-i8259.o
 obj-$(CONFIG_IMGPDC_IRQ)		+= irq-imgpdc.o
 obj-$(CONFIG_IRQ_MIPS_CPU)		+= irq-mips-cpu.o
 obj-$(CONFIG_SIRF_IRQ)			+= irq-sirfsoc.o
+obj-$(CONFIG_JCORE_AIC)			+= irq-jcore-aic.o
 obj-$(CONFIG_RENESAS_INTC_IRQPIN)	+= irq-renesas-intc-irqpin.o
 obj-$(CONFIG_RENESAS_IRQC)		+= irq-renesas-irqc.o
 obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o

drivers/irqchip/irq-jcore-aic.c new file +14 -0

new file mode 100644
index 0000000..1348cdb
@@ -0,0 +1,95 @@
+/*
+ * J-Core SoC AIC driver
+ *
+ * Copyright (C) 2015-2016 Smart Energy Instruments, Inc.
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#include <linux/irq.h>
+#include <linux/io.h>
+#include <linux/irqchip.h>
+#include <linux/irqdomain.h>

drivers/irqchip/irq-jcore-aic.c new file +32 -0

+	u32 cpu_offset;
+	struct irq_chip chip;
+	struct irq_domain *domain;
+	struct notifier_block nb;
+} aic_data;
+
+static int aic_irqdomain_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hwirq)
+{
+	struct aic_data *aic = d->host_data;
+
+	irq_set_chip_data(irq, aic);
+	irq_set_chip_and_handler(irq, &aic->chip, handle_simple_irq);
+	irq_set_probe(irq);
+
+	return 0;
+}
+
+static const struct irq_domain_ops aic_irqdomain_ops = {
+	.map = aic_irqdomain_map,
+	.xlate = irq_domain_xlate_onecell,
+};
+
+static void noop(struct irq_data *data)
+{
+}
+
+static void aic1_localenable(struct aic_data *aic)
+{
+	unsigned cpu = smp_processor_id();
+	pr_info("Local AIC enable on cpu %u\n", cpu);
+	writel(0xffffffff, aic->base + cpu * aic->cpu_offset + AIC1_INTPRI);
+}

drivers/irqchip/irq-jcore-aic.c new file +19 -0

+
+int __init aic_irq_of_init(struct device_node *node, struct device_node *parent)
+{
+	struct aic_data *aic = &aic_data;
+
+	aic->base = of_iomap(node, 0);
+	of_property_read_u32(node, "cpu-offset", &aic->cpu_offset);
+
+	pr_info("Initializing J-Core AIC at %p\n", aic->base);
+
+	if (of_device_is_compatible(node, "jcore,aic1")) {
+		/* For aic1, need to enabled zero-priority-by-default irqs */
+		aic->nb.notifier_call = aic1_cpu_notify;
+		register_cpu_notifier(&aic->nb);
+		aic1_localenable(aic);
+	}
+
+	aic->chip.name = node->name;
+	aic->chip.irq_mask = noop;

Quoted 32 lines

drivers/irqchip/irq-jcore-aic.c new file +32 -0

+	u32 cpu_offset;
+	struct irq_chip chip;
+	struct irq_domain *domain;
+	struct notifier_block nb;
+} aic_data;
+
+static int aic_irqdomain_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hwirq)
+{
+	struct aic_data *aic = d->host_data;
+
+	irq_set_chip_data(irq, aic);
+	irq_set_chip_and_handler(irq, &aic->chip, handle_simple_irq);
+	irq_set_probe(irq);
+
+	return 0;
+}
+
+static const struct irq_domain_ops aic_irqdomain_ops = {
+	.map = aic_irqdomain_map,
+	.xlate = irq_domain_xlate_onecell,
+};
+
+static void noop(struct irq_data *data)
+{
+}
+
+static void aic1_localenable(struct aic_data *aic)
+{
+	unsigned cpu = smp_processor_id();
+	pr_info("Local AIC enable on cpu %u\n", cpu);
+	writel(0xffffffff, aic->base + cpu * aic->cpu_offset + AIC1_INTPRI);
+}

drivers/irqchip/irq-jcore-aic.c line 66

Quoted 10 lines

drivers/irqchip/irq-jcore-aic.c new file +10 -0

+
+static int aic1_cpu_notify(struct notifier_block *self, unsigned long action, void *hcpu)
+{
+	switch (action & ~CPU_TASKS_FROZEN) {
+	case CPU_STARTING:
+		aic1_localenable(container_of(self, struct aic_data, nb));
+		break;
+	}
+	return NOTIFY_OK;
+}

Please take a look at the series posted by Anna-Maria Gleixner for recent changes to the cpu hp state machine.

https://lkml.kernel.org/r/[email protected]

changes based on ml discussion of the v2 patch. The chipselect change has not been made yet, except for rewriting the current logic to be more clear. If the decision is that it's needed, I can do it easily, but it needs testing still.

Quoted 4 lines · Rich Felker

changes based on ml discussion of the v2 patch. The chipselect change has not been made yet, except for rewriting the current logic to be more clear. If the decision is that it's needed, I can do it easily, but it needs testing still.

You need to fix that, it's not good to just decide to ignore review comments. It is astonishing that you are doing this while expecting to bypass the normal process for merging things. I'm not going to look at this version, please ensure you have addressed all the feedback from the previous review before any new version you submit.

Would you prefer I continue to submit this driver with new versions of the patch series, or separate it out for further review on its own?

Signed-off-by Rich Felker <[email protected]>

---
 arch/sh/boot/dts/j2_mimas_v2.dts | 87 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 87 insertions(+)
 create mode 100755 arch/sh/boot/dts/j2_mimas_v2.dts

arch/sh/boot/dts/j2_mimas_v2.dts new file +10 -0

new file mode 100755
index 0000000..4a66cda
@@ -0,0 +1,87 @@
+/dts-v1/;
+
+/ {
+	compatible = "jcore,j2-soc";
+	model = "J2 FPGA SoC on Mimas v2 board";
+
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	interrupt-parent = <&aic>;

arch/sh/boot/dts/j2_mimas_v2.dts new file +56 -0

+
+	soc@abcd0000 {
+		compatible = "simple-bus";
+		ranges = <0 0xabcd0000 0x100000>;
+
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		aic: interrupt-controller@200 {
+			compatible = "jcore,aic1";
+			reg = < 0x200 0x10 >;
+			interrupt-controller;
+			#interrupt-cells = <1>;
+		};
+
+		cache-controller@c0 {
+			compatible = "jcore,cache";
+			reg = < 0xc0 4 >;
+		};
+
+		timer@200 {
+			compatible = "jcore,pit";
+			reg = < 0x200 0x30 >;
+			interrupts = < 0x48 >;
+		};
+
+		spi@40 {
+			compatible = "jcore,spi2";
+
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			spi-max-frequency = <12500000>;
+
+			reg = < 0x40 0x8 >;
+
+			sdcard@0 {
+				compatible = "mmc-spi-slot";
+				reg = <0>;
+				spi-max-frequency = <12500000>;
+				voltage-ranges = <3200 3400>;
+				mode = <0>;
+			};
+		};
+
+		serial@100 {
+			clock-frequency = <125000000>;
+			compatible = "xlnx,xps-uartlite-1.00.a";
+			current-speed = <19200>;
+			device_type = "serial";
+			interrupts = < 0x12 >;
+			port-number = <0>;
+			reg = < 0x100 0x10 >;
+		};
+	};
+};

Signature · 1 line

2.8.1

Quoted 22 lines · Rich Felker

Signed-off-by Rich Felker <[email protected]>

---
 arch/sh/boot/dts/j2_mimas_v2.dts | 87 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 87 insertions(+)
 create mode 100755 arch/sh/boot/dts/j2_mimas_v2.dts

arch/sh/boot/dts/j2_mimas_v2.dts new file +10 -0

new file mode 100755
index 0000000..4a66cda
@@ -0,0 +1,87 @@
+/dts-v1/;
+
+/ {
+	compatible = "jcore,j2-soc";
+	model = "J2 FPGA SoC on Mimas v2 board";
+
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	interrupt-parent = <&aic>;

arch/sh/boot/dts/j2_mimas_v2.dts line 20

Quoted 10 lines

arch/sh/boot/dts/j2_mimas_v2.dts new file +10 -0

+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu@0 {
+			device_type = "cpu";
+			compatible = "jcore,j2";
+			reg = < 0 >;
+			clock-frequency = < 50000000 >;

Nit: please remove the spacing around the '<' and '>' here. If nothing else, it's inconsistent with the rest of this file.

arch/sh/boot/dts/j2_mimas_v2.dts line 27

Quoted 7 lines

arch/sh/boot/dts/j2_mimas_v2.dts new file +7 -0

+		};
+	};
+
+	memory@10000000 {
+		device_type = "memory";
+		reg = < 0x10000000 0x4000000 >;
+	};

Likewise.

arch/sh/boot/dts/j2_mimas_v2.dts line 31

Quoted 4 lines

arch/sh/boot/dts/j2_mimas_v2.dts new file +4 -0

+
+	chosen {
+		stdout-path = "/soc@abcd0000/serial@100";
+	};

Please use a label for the serial node, have an alias, and describe the pre-configured rate per the stdout-path binding, e.g.

Quoted 37 lines · Mark Rutland

Quoted 22 lines · Rich Felker

Signed-off-by Rich Felker <[email protected]>

---
 arch/sh/boot/dts/j2_mimas_v2.dts | 87 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 87 insertions(+)
 create mode 100755 arch/sh/boot/dts/j2_mimas_v2.dts

arch/sh/boot/dts/j2_mimas_v2.dts new file +10 -0

new file mode 100755
index 0000000..4a66cda
@@ -0,0 +1,87 @@
+/dts-v1/;
+
+/ {
+	compatible = "jcore,j2-soc";
+	model = "J2 FPGA SoC on Mimas v2 board";
+
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	interrupt-parent = <&aic>;

arch/sh/boot/dts/j2_mimas_v2.dts line 20

Quoted 10 lines

arch/sh/boot/dts/j2_mimas_v2.dts new file +10 -0

+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu@0 {
+			device_type = "cpu";
+			compatible = "jcore,j2";
+			reg = < 0 >;
+			clock-frequency = < 50000000 >;

Nit: please remove the spacing around the '<' and '>' here. If nothing else, it's inconsistent with the rest of this file.

Quoted 9 lines

arch/sh/boot/dts/j2_mimas_v2.dts line 27

Quoted 7 lines

arch/sh/boot/dts/j2_mimas_v2.dts new file +7 -0

+		};
+	};
+
+	memory@10000000 {
+		device_type = "memory";
+		reg = < 0x10000000 0x4000000 >;
+	};

Likewise.

OK, I'll change both of these.

Quoted 7 lines

arch/sh/boot/dts/j2_mimas_v2.dts line 31

Quoted 4 lines

arch/sh/boot/dts/j2_mimas_v2.dts new file +4 -0

+
+	chosen {
+		stdout-path = "/soc@abcd0000/serial@100";
+	};

Please use a label for the serial node, have an alias, and describe the pre-configured rate per the stdout-path binding, e.g.

Per Documentation/devicetree/bindings/xilinx.txt, current-speed is a required property for "xlnx,xps-uartlite-1.00.a". Note that uartlite does not actually have a programmable baud rate; the property is instead being used to describe the hardware-provided rate.

BTW our uartlite is not actually derived from Xilinx's IP core, just interface-compatible with it, so I'd actually like to add a "jcore,uartlite" binding too in order to express this, with "xlnx,xps-uartlite-1.00.a" as the fallback compatible-tag. I'll propose that as a separate patch later.

As arch/sh co-maintainer my intent is to include as much as possible in my pull request for the linux-sh tree. If there are parts outside of arch/sh that can be included in this, please let me know. I'm not clear yet on what the right path to upstream is for the clocksource and irq drivers that are currently only useful/interesting for one arch, or for the DT binding patches. Even if some drivers are delayed

going upstream, I would really like to get DT bindings acked and ideally merged, because we want to go ahead with moving the DTB into J2 boot rom where it belongs, and that should only happen with stable bindings.

As arch/sh co-maintainer my intent is to include as much as possible in my pull request for the linux-sh tree. If there are parts outside of arch/sh that can be included in this, please let me know. I'm not

arch, or for the DT binding patches. Even if some drivers are delayed going upstream, I would really like to get DT bindings acked and ideally merged, because we want to go ahead with moving the DTB into J2 boot rom where it belongs, and that should only happen with stable

Quoted 3 lines · Rich Felker

As arch/sh co-maintainer my intent is to include as much as possible in my pull request for the linux-sh tree. If there are parts outside of arch/sh that can be included in this, please let me know. I'm not

Do *not* include the SPI driver, you shouldn't be including any drivers unless it's been explicitly discussed with the subsystem maintainers.

Quoted 4 lines

clear yet on what the right path to upstream is for the clocksource and irq drivers that are currently only useful/interesting for one arch, or for the DT binding patches. Even if some drivers are delayed [...]

Quite aside from the fact that like Geert says drivers are expected to go through the subsystem trees to repeat what I said last time it wasn't posted until after the merge window and we're now a few days before the end of the merge window and a new version is being posted. The turnaround times you are demanding on review are unreasonable - people get busy, have holidays and so on - and you really need to pay attention to what people are telling you about the process or you're just going to annoy people.

Quoted 4 lines

arch, or for the DT binding patches. Even if some drivers are delayed going upstream, I would really like to get DT bindings acked and ideally merged, because we want to go ahead with moving the DTB into J2 boot rom where it belongs, and that should only happen with stable

If you want people to review DT bindings you're going to need to submit them.

Quoted 8 lines · Mark Brown

Quoted 3 lines · Rich Felker

As arch/sh co-maintainer my intent is to include as much as possible in my pull request for the linux-sh tree. If there are parts outside of arch/sh that can be included in this, please let me know. I'm not

Do *not* include the SPI driver, you shouldn't be including any drivers unless it's been explicitly discussed with the subsystem maintainers.

See the "please let me know". I thought this was plenty clear that I was asking for permission for including things outside of arch/sh, and that short of getting an ack, the default permission is no. You also snipped the part of my message that mentioned the specific subsystems I was asking about (which were non-SPI because you already made quite a point about not taking the SPI driver):

Quoted 5 lines

end of the merge window and a new version is being posted. The turnaround times you are demanding on review are unreasonable - people get busy, have holidays and so on - and you really need to pay attention to what people are telling you about the process or you're just going to annoy people.

If you can't review and ack the code on short notice, that's fine; just say so. There's no need to be overerly hostile about it. I've

gotten arch/sh patches during the merge window before and I try to be polite with the contributor and ask if there's something seriously broken that would be improved by my making an effort to check it at the last minute, or it if can happily wait until next time.

Being that the driver in question here is for a new platform that was not previously supported upstream and has zero chance of breaking anything else, and that its inclusion would be a big plus for users of

the platform, I don't see any reason for you making such a big deal out of it unless enforcing policy for its own sake makes you feel good, but I have better things to do than argue about it.

Quoted 2 lines

If you want people to review DT bindings you're going to need to submit them.

I have, twice now, and I Cc'd the the linux-spi list too on v3 for the spi binding. Rob Herring acked it.