[PATCH v3 00/12] J-core J2 cpu and SoC peripherals support

[PATCH v3 00/12] J-core J2 cpu and SoC peripherals support

[Rich Felker]

The following patchset adds support for the J-core J2, an open-source
VHDL reimplementation of the SH-2 ISA, and drivers for the associated
SoC devices (interrupt controller, clocksource, and SPI).

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.

Rich Felker (12):
  of: add vendor prefix for J-Core
  of: add J-Core cpu bindings
  of: add J-Core interrupt controller bindings
  of: add J-Core timer bindings
  of: add J-Core SPI master bindings
  sh: add support for J-Core J2 processor
  sh: add AT_HWCAP flag for J-Core cas.l instruction
  irqchip: add J-Core AIC driver
  clocksource: add J-Core timer/clocksource driver
  spi: add driver for J-Core SPI controller
  sh: add defconfig for J-Core J2
  sh: add device tree source for J2 FPGA on Mimas v2 board

 .../bindings/interrupt-controller/jcore,aic.txt    |  29 +++
 Documentation/devicetree/bindings/jcore/cpus.txt   |  92 +++++++
 .../devicetree/bindings/spi/jcore,spi.txt          |  23 ++
 .../devicetree/bindings/timer/jcore,pit.txt        |  28 ++
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 arch/sh/Kconfig                                    |   8 +
 arch/sh/Makefile                                   |   1 +
 arch/sh/boot/dts/j2_mimas_v2.dts                   |  87 +++++++
 arch/sh/configs/j2_defconfig                       |  38 +++
 arch/sh/include/asm/processor.h                    |   2 +-
 arch/sh/include/uapi/asm/cpu-features.h            |   1 +
 arch/sh/kernel/cpu/init.c                          |   2 +-
 arch/sh/kernel/cpu/proc.c                          |   1 +
 arch/sh/kernel/cpu/sh2/entry.S                     |   5 +
 arch/sh/kernel/cpu/sh2/probe.c                     |  36 ++-
 arch/sh/mm/Makefile                                |   3 +-
 arch/sh/mm/cache-j2.c                              |  58 +++++
 arch/sh/mm/cache.c                                 |   6 +-
 drivers/clocksource/Kconfig                        |   8 +
 drivers/clocksource/Makefile                       |   1 +
 drivers/clocksource/jcore-pit.c                    | 282 +++++++++++++++++++++
 drivers/irqchip/Kconfig                            |   6 +
 drivers/irqchip/Makefile                           |   1 +
 drivers/irqchip/irq-jcore-aic.c                    |  95 +++++++
 drivers/spi/Kconfig                                |   4 +
 drivers/spi/Makefile                               |   1 +
 drivers/spi/spi-jcore.c                            | 209 +++++++++++++++
 27 files changed, 1023 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/jcore,aic.txt
 create mode 100644 Documentation/devicetree/bindings/jcore/cpus.txt
 create mode 100644 Documentation/devicetree/bindings/spi/jcore,spi.txt
 create mode 100644 Documentation/devicetree/bindings/timer/jcore,pit.txt
 create mode 100755 arch/sh/boot/dts/j2_mimas_v2.dts
 create mode 100644 arch/sh/configs/j2_defconfig
 create mode 100644 arch/sh/mm/cache-j2.c
 create mode 100644 drivers/clocksource/jcore-pit.c
 create mode 100644 drivers/irqchip/irq-jcore-aic.c
 create mode 100644 drivers/spi/spi-jcore.c

-- 
2.8.1

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 01/12] of: add vendor prefix for J-Core

[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 <dalias@libc.org>
---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 86740d4..9c070b8 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -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

Re: [PATCH v3 01/12] of: add vendor prefix for J-Core

[Rob Herring]

On Wed, May 25, 2016 at 12:43 AM, Rich Felker <dalias@libc.org> wrote:
> 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 <dalias@libc.org>

Please add acks when posting subsequent versions.

> ---
>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index 86740d4..9c070b8 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -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
>
>

Re: [PATCH v3 01/12] of: add vendor prefix for J-Core

[Rich Felker]

On Wed, May 25, 2016 at 08:18:06AM -0500, Rob Herring wrote:
> On Wed, May 25, 2016 at 12:43 AM, Rich Felker <dalias@libc.org> wrote:
> > 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 <dalias@libc.org>
> 
> Please add acks when posting subsequent versions.
> 
> > ---
> >  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> > index 86740d4..9c070b8 100644
> > --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> > +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> > @@ -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

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

Rich

Re: [PATCH v3 01/12] of: add vendor prefix for J-Core

[Rich Felker]

On Wed, Jul 27, 2016 at 01:31:12AM -0400, Rich Felker wrote:
> On Wed, May 25, 2016 at 08:18:06AM -0500, Rob Herring wrote:
> > On Wed, May 25, 2016 at 12:43 AM, Rich Felker <dalias@libc.org> wrote:
> > > 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 <dalias@libc.org>
> > 
> > Please add acks when posting subsequent versions.
> > 
> > > ---
> > >  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> > > index 86740d4..9c070b8 100644
> > > --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> > > +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> > > @@ -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
> 
> 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.

Rich

Re: [PATCH v3 01/12] of: add vendor prefix for J-Core

[Rob Herring]

On Thu, Aug 04, 2016 at 06:27:57PM -0400, Rich Felker wrote:
> On Wed, Jul 27, 2016 at 01:31:12AM -0400, Rich Felker wrote:
> > On Wed, May 25, 2016 at 08:18:06AM -0500, Rob Herring wrote:
> > > On Wed, May 25, 2016 at 12:43 AM, Rich Felker <dalias-8zAoT0mYgF4@public.gmane.org> wrote:
> > > > 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 <dalias-8zAoT0mYgF4@public.gmane.org>
> > > 
> > > Please add acks when posting subsequent versions.
> > > 
> > > > ---
> > > >  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> > > > index 86740d4..9c070b8 100644
> > > > --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> > > > +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> > > > @@ -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
> > 
> > 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.

If the maintainer has acked it, you can take it normally.

However, for this cycle I've now applied a patch fixing the 
alphabetizing of this file which surely will cause merge conflicts. So 
I've now applied this.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 02/12] of: add J-Core cpu bindings

[Rich Felker]

Signed-off-by: Rich Felker <dalias@libc.org>
---
 Documentation/devicetree/bindings/jcore/cpus.txt | 92 ++++++++++++++++++++++++
 1 file changed, 92 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/jcore/cpus.txt

diff --git a/Documentation/devicetree/bindings/jcore/cpus.txt b/Documentation/devicetree/bindings/jcore/cpus.txt
new file mode 100644
index 0000000..9d77ec1
--- /dev/null
+++ b/Documentation/devicetree/bindings/jcore/cpus.txt
@@ -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.
+
+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
+  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.
+
+
+---------------------
+Cache controller node
+---------------------
+
+Required properties:
+
+- compatible: Must be "jcore,cache".
+
+- reg: A memory range for the cache controller registers.
+
+
+--------
+IPI node
+--------
+
+Device trees for SMP systems must have an IPI node representing the mechanism
+used for inter-processor interrupt generation.
+
+Required properties:
+
+- compatible: Must be "jcore,ipi-controller".
+
+- reg: A memory range used to IPI generation.
+
+- interrupts: An irq on which IPI will be received.
+
+
+----------
+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:
+
+- 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.
-- 
2.8.1

Re: [PATCH v3 02/12] of: add J-Core cpu bindings

[Mark Rutland]

On Wed, May 25, 2016 at 05:43:03AM +0000, Rich Felker wrote:
> Signed-off-by: Rich Felker <dalias-8zAoT0mYgF4@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/jcore/cpus.txt | 92 ++++++++++++++++++++++++
>  1 file changed, 92 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/jcore/cpus.txt
> 
> diff --git a/Documentation/devicetree/bindings/jcore/cpus.txt b/Documentation/devicetree/bindings/jcore/cpus.txt
> new file mode 100644
> index 0000000..9d77ec1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/jcore/cpus.txt
> @@ -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.
> +
> +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
> +  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).

> +---------------------
> +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).

> +--------
> +IPI node
> +--------
> +
> +Device trees for SMP systems must have an IPI node representing the mechanism
> +used for inter-processor interrupt generation.
> +
> +Required properties:
> +
> +- compatible: Must be "jcore,ipi-controller".
> +
> +- reg: A memory range used to IPI generation.
> +
> +- interrupts: An irq on which IPI will be received.

Likewise.

> +----------
> +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:
> +
> +- 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.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 02/12] of: add J-Core cpu bindings

[Rich Felker]

On Wed, May 25, 2016 at 11:22:15AM +0100, Mark Rutland wrote:
> > +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
> > +  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?

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

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

CPU native.

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

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

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

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

>   - Is the address permitted to be a device register?

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

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

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

Current models are nommu.

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

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

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

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

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

This depends on having a model that has these features.

>   - Should any registers be programmed with specific values?

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

> 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).
> 
> > +---------------------
> > +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).

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

> > +--------
> > +IPI node
> > +--------
> > +
> > +Device trees for SMP systems must have an IPI node representing the mechanism
> > +used for inter-processor interrupt generation.
> > +
> > +Required properties:
> > +
> > +- 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).

> > +----------
> > +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:
> > +
> > +- 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.

Rich

Re: [PATCH v3 02/12] of: add J-Core cpu bindings

[Geert Uytterhoeven]

Hi Rich,

On Thu, May 26, 2016 at 1:04 AM, Rich Felker <dalias@libc.org> wrote:
> 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.

Submitting a version now without SMP is indeed a good idea, and allows
to move forward.

Gr{oetje,eeting}s,

                        Geert

Re: [PATCH v3 02/12] of: add J-Core cpu bindings

[Mark Rutland]

On Wed, May 25, 2016 at 07:04:08PM -0400, Rich Felker wrote:
> On Wed, May 25, 2016 at 11:22:15AM +0100, Mark Rutland wrote:
> > > +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
> > > +  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?

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

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

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

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

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

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

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

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

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

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

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

> > * What state should the CPU be in when it branches to the provided
> >   address?
> >   - Must the MMU be off?
> 
> Current models are nommu.
> 
> >   - 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.

Ok.

> 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.
> 
> >   - 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.
> 
> >   - 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.
> 
> >   - 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.
> 
> >   - Should debug logic (e.g. breakpoint and watchpoints) be placed into
> >     a quiescent state?
> 
> This depends on having a model that has these features.
> 
> >   - Should any registers be programmed with specific values?
> 
> No, beyond perhaps things like control registers (IMASK).
> 
> > 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).
> > 
> > > +---------------------
> > > +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).
> 
> 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.)

Ok, this is simpler than I had anticipated.

> > > +--------
> > > +IPI node
> > > +--------
> > > +
> > > +Device trees for SMP systems must have an IPI node representing the mechanism
> > > +used for inter-processor interrupt generation.
> > > +
> > > +Required properties:
> > > +
> > > +- 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).
> 
> > > +----------
> > > +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:
> > > +
> > > +- 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. 

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.

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

Thanks,
Mark.

Re: [PATCH v3 02/12] of: add J-Core cpu bindings

[Rich Felker]

On Thu, May 26, 2016 at 11:38:29AM +0100, Mark Rutland wrote:
> On Wed, May 25, 2016 at 07:04:08PM -0400, Rich Felker wrote:
> > On Wed, May 25, 2016 at 11:22:15AM +0100, Mark Rutland wrote:
> > > > +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
> > > > +  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?
> 
> No, the "jcore,spin-table" name is fine.
> 
> > > 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. 
> 
> Ok, so this is effectively a device register, rather than a location in
> "real" memory.

Yes, I re-checked and it actually works as a device register. Sorry
for the confusion. I think what happened is that I modeled the
binding/kernel with the intent that it could just as well be in normal
memory with the cpu spinning 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.
> 
> Ok. Is this second address also a device register, or might this be in
> "real" memory?

In practice it's real memory, but aside from possible constraints on
how it's written I don't think it matters a lot.

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

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

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

I'm not sure. If so, native word (32-bit) would be the right size to
specify, but it's possible that any size works.

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

Yes.

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

I agree, but from a practical standpoint I think it's irrelevant. The
secondary cpu(s) should start with either empty cache or just some
boot rom code in their caches (and current architecture does not even
cache sram/"rom"). Flushing would be a good idea just to be safe but
it's going to be an effective no-op.

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

The only configuration possible via the current binding is
enabling/disabling the cache. If it's enabled, it needs to be used for
icache flush when new code is written, and for dcache flush
before/after DMA. (Note that there is no DMA binding yet and no DMA
driver; these are coming later.)

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

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.

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

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.

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

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.

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

Yes. For FPGA implementations it's just part of the FPGA bitstream and
you upgrade it the same way you load a new bitstream onto the FPGA.

Rich

Re: [PATCH v3 02/12] of: add J-Core cpu bindings

[Mark Rutland]

On Thu, May 26, 2016 at 11:33:23AM -0400, Rich Felker wrote:
> On Thu, May 26, 2016 at 11:38:29AM +0100, Mark Rutland wrote:
> > On Wed, May 25, 2016 at 07:04:08PM -0400, Rich Felker wrote:
> > > On Wed, May 25, 2016 at 11:22:15AM +0100, Mark Rutland wrote:
> > > > * 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.
> 
> > 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.

Sure. If endianness isn't runtime switchable, and there is no near-term
plan to add that, then my concerns do not apply.

[...]

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

Yes, this is strictly more correct than my wording.

Given the lack of MMU or cache configration beynd on/off, it doesn't
sound like there are any arch-specific memory attribute rules to
document.

[...]

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

Sorry for that; in retrospect I probably should have sent the boot
protocol comments as a separate reply.

[...]

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

That sounds good to me.

Thanks,
Mark,
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 02/12] of: add J-Core cpu bindings

[Rob Landley]

On 05/25/2016 06:04 PM, Rich Felker wrote:
> On Wed, May 25, 2016 at 11:22:15AM +0100, Mark Rutland wrote:
>> * What state should the CPU be in when it branches to the provided
>>   address?
>>   - Must the MMU be off?
> 
> Current models are nommu.

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

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

There are hundreds of todo items. At the moment we're just trying to get
basic board support in for more or less the design we demoed a year ago
at https://lwn.net/Articles/647636/

The roadmap page we've posted is a summary of a summary, it doesn't even
mention the DSP designs (plural) we've talked about at some length
internally. There's a DMA engine we're not using yet. We've got various
other things like ethernet support that the cheap $50 introductory board
we're targeting as an entry point hasn't got connectors for. And we
didn't do anything for the vga or sound connectors on that board because
"boot to a shell prompt on serial prompt" was our first target. (It runs
out of initramfs, but since you need the sdcard to load linux from, and
thus the bootloader had to have an sdcard driver, it seemed a shame NOT
to have an sdcard driver in linux too.)

Other than that? Intentionally minimalist first pass.

>> 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).
>>
>>> +---------------------
>>> +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?

The icache and dcache are 8k each, have 32 byte cache lines, and they
cache (addr>>5)&255 so reading from + or - 8192 bytes will evict that
cache line due to aliasing.

The icache and dcache each have an enable and flush bit in the
processor's control register. (So 4 bits total controlling the cache, I
think? I'd have to dig up Niishi-san's slides to check.)

Each processor has its own set of control registers. Back when SMP was
first implemented there was a lot of talk among the engineers about
separating the register stuff out, because right now processor stuff and
SOC I/O devices are kinda mashed together. But "release early, release
often" won out over endlessly polishing the same stuff in-house before
showing it to anybody else. We WANT other people to suggest fixes, but
we also want basic support for what's already been working for a year
and a half to go into the vanilla kernel at some point.

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

During my most recent trip to Japan I sat down with the engineer who
wrote the dcache and dram controller and passed on his explanation of
them to somebody on the j-core mailing list, about halfway through this
message:

  http://lists.j-core.org/pipermail/j-core/2016-April/000038.html

I've been meaning to cut that out and put it on a web page on
j-core.org, but have been busy with other things. That post also points
at comments in the VHDL source that implement the features.

That is, alas, the level of documentation we're talking about at the
moment. Better is on the todo list. In the meantime you can RTFS if you
understand vhdl, or ask the engineers on the j-core list or #j-core
freenode channel. The instruction set is based on an existing
architecture and the other SOC features in the initial release are as
minimal as we could get them and still be useful. (We've got a lot more
peripherals implemented than this release includes.)

We thought getting working code into the kernel should be a high
priority, but apparently everything has to be done before anything can
be done?

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

The first dache-only Linux support I did last year worked by reading an
aliased cache line from sram to evict individual cache lines, and it
turned out to have no detectable speed advantage over just blanking the
whole thing. Then doing the same "flush by aliasing" trick with icache
would have required an 8k jump table and we just went "no" and
implemented the flush bits instead, so almost all that code went away.

When we implement L2 cache for j-core, we can start caring about
granularity again, but http://j-core.org/roadmap.html has scaling _down_
into arduino country scheduled before scheduling up into MMU and 64-bit
territory so... not this year. :)

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

In general if they haven't needed it yet, they haven't done it yet.
Doesn't mean we haven't thought about it, or even sketched out multiple
possible paths forward and decided on our favorite and our preferred
fallback approach. (I have more photographs of whiteboards on my phone
than anyone really needs. Some of them I could even tell you what the
subject was.)

But for the initial "here's how people other than us can try this out on
a $50 FPGA board from india" board support, going down the rathole of
every possible direction of future expansion as a prerequisite for "one
release of one processor on one brand of board" introductory support
seems counterproductive. Or is it just me?

If you want to critique the hardware design on the j-core mailing list,
I'll happily point the appropriate in-house engineers at the thread. In
the meantime, Rich submitted a patch to support the SOC we've had for a
year now.

Rob

Re: [PATCH v3 02/12] of: add J-Core cpu bindings

[Afzal Mohammed]

Hi,

On Thu, May 26, 2016 at 04:44:02PM -0500, Rob Landley wrote:

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

According to hearsay, thou shall be called Buzz Aldrin, Blackfin is
Neil Armstrong.

Regards
afzal
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 05/12] of: add J-Core SPI master bindings

[Rich Felker]

Signed-off-by: Rich Felker <dalias@libc.org>
---
 .../devicetree/bindings/spi/jcore,spi.txt          | 23 ++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/jcore,spi.txt

diff --git a/Documentation/devicetree/bindings/spi/jcore,spi.txt b/Documentation/devicetree/bindings/spi/jcore,spi.txt
new file mode 100644
index 0000000..7bff8c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/jcore,spi.txt
@@ -0,0 +1,23 @@
+J-Core SPI master
+
+Required properties:
+
+- compatible: Must be "jcore,spi2".
+
+- reg: Memory region for registers.
+
+- #address-cells: Must be 1.
+
+- #size-cells: Must be 0.
+
+See spi-bus.txt for additional properties not specific to this device.
+
+Example:
+
+spi@40 {
+	compatible = "jcore,spi2";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	reg = < 0x40 0x8 >;
+	spi-max-frequency = <12500000>;
+}
-- 
2.8.1

Re: [PATCH v3 05/12] of: add J-Core SPI master bindings

[Rob Herring]

On Wed, May 25, 2016 at 05:43:03AM +0000, Rich Felker wrote:
> Signed-off-by: Rich Felker <dalias@libc.org>
> ---
>  .../devicetree/bindings/spi/jcore,spi.txt          | 23 ++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/jcore,spi.txt

Acked-by: Rob Herring <robh@kernel.org>

[PATCH v3 03/12] of: add J-Core interrupt controller bindings

[Rich Felker]

Signed-off-by: Rich Felker <dalias@libc.org>
---
 .../bindings/interrupt-controller/jcore,aic.txt    | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/jcore,aic.txt

diff --git a/Documentation/devicetree/bindings/interrupt-controller/jcore,aic.txt b/Documentation/devicetree/bindings/interrupt-controller/jcore,aic.txt
new file mode 100644
index 0000000..5dc99b9
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/jcore,aic.txt
@@ -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:
+
+- cpu-offset : For SMP, the offset to the per-cpu memory region for
+  configuration, to be scaled by the cpu number.
+
+
+Example:
+
+aic: interrupt-controller@200 {
+	compatible = "jcore,aic2";
+	reg = < 0x200 0x10 >;
+	interrupt-controller;
+	#interrupt-cells = <1>;
+};
-- 
2.8.1

Re: [PATCH v3 03/12] of: add J-Core interrupt controller bindings

[Mark Rutland]

On Wed, May 25, 2016 at 05:43:03AM +0000, Rich Felker wrote:
> Signed-off-by: Rich Felker <dalias@libc.org>
> ---
>  .../bindings/interrupt-controller/jcore,aic.txt    | 29 ++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/jcore,aic.txt
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/jcore,aic.txt b/Documentation/devicetree/bindings/interrupt-controller/jcore,aic.txt
> new file mode 100644
> index 0000000..5dc99b9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/jcore,aic.txt
> @@ -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:
> +
> +- 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.

Thanks,
Mark.

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

Re: [PATCH v3 03/12] of: add J-Core interrupt controller bindings

[Rich Felker]

On Wed, May 25, 2016 at 11:25:04AM +0100, Mark Rutland wrote:
> On Wed, May 25, 2016 at 05:43:03AM +0000, Rich Felker wrote:
> > Signed-off-by: Rich Felker <dalias@libc.org>
> > ---
> >  .../bindings/interrupt-controller/jcore,aic.txt    | 29 ++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/jcore,aic.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/jcore,aic.txt b/Documentation/devicetree/bindings/interrupt-controller/jcore,aic.txt
> > new file mode 100644
> > index 0000000..5dc99b9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/jcore,aic.txt
> > @@ -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:
> > +
> > +- 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.

OK. The current arch/sh SMP framework only has nominal support for hw
cpuid != logical cpuid; it's not actually used/usable right now. But
the DT binding spec should be clear on this anyway.

Rich

[PATCH v3 12/12] sh: add device tree source for J2 FPGA on Mimas v2 board

[Rich Felker]

Signed-off-by: Rich Felker <dalias@libc.org>
---
 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

diff --git a/arch/sh/boot/dts/j2_mimas_v2.dts b/arch/sh/boot/dts/j2_mimas_v2.dts
new file mode 100755
index 0000000..4a66cda
--- /dev/null
+++ b/arch/sh/boot/dts/j2_mimas_v2.dts
@@ -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>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu@0 {
+			device_type = "cpu";
+			compatible = "jcore,j2";
+			reg = < 0 >;
+			clock-frequency = < 50000000 >;
+		};
+	};
+
+	memory@10000000 {
+		device_type = "memory";
+		reg = < 0x10000000 0x4000000 >;
+	};
+
+	chosen {
+		stdout-path = "/soc@abcd0000/serial@100";
+	};
+
+	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 >;
+		};
+	};
+};
-- 
2.8.1

Re: [PATCH v3 12/12] sh: add device tree source for J2 FPGA on Mimas v2 board

[Mark Rutland]

On Wed, May 25, 2016 at 05:43:03AM +0000, Rich Felker wrote:
> Signed-off-by: Rich Felker <dalias-8zAoT0mYgF4@public.gmane.org>
> ---
>  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
> 
> diff --git a/arch/sh/boot/dts/j2_mimas_v2.dts b/arch/sh/boot/dts/j2_mimas_v2.dts
> new file mode 100755
> index 0000000..4a66cda
> --- /dev/null
> +++ b/arch/sh/boot/dts/j2_mimas_v2.dts
> @@ -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>;
> +
> +	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.

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

Likewise.

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


	aliases {
		serial0 = &uart0;
	};
	
	chosen {
		stdout-path = "serial0:19200n8";
	};

	...

	soc@abcd0000 {
		...
		uart0: serial@100 {
			...
		};
	};

Note that using stdout-path is preferable to current-speed or other
UART-specific bindings.

Thanks,
Mark.

> +
> +	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 >;
> +		};
> +	};
> +};
> -- 
> 2.8.1
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 12/12] sh: add device tree source for J2 FPGA on Mimas v2 board

[Rich Felker]

On Wed, May 25, 2016 at 11:33:50AM +0100, Mark Rutland wrote:
> On Wed, May 25, 2016 at 05:43:03AM +0000, Rich Felker wrote:
> > Signed-off-by: Rich Felker <dalias@libc.org>
> > ---
> >  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
> > 
> > diff --git a/arch/sh/boot/dts/j2_mimas_v2.dts b/arch/sh/boot/dts/j2_mimas_v2.dts
> > new file mode 100755
> > index 0000000..4a66cda
> > --- /dev/null
> > +++ b/arch/sh/boot/dts/j2_mimas_v2.dts
> > @@ -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>;
> > +
> > +	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.
> 
> > +		};
> > +	};
> > +
> > +	memory@10000000 {
> > +		device_type = "memory";
> > +		reg = < 0x10000000 0x4000000 >;
> > +	};
> 
> Likewise.

OK, I'll change both of these.

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

Rich

Re: [PATCH v3 12/12] sh: add device tree source for J2 FPGA on Mimas v2 board

[Mark Rutland]

On Wed, May 25, 2016 at 07:15:25PM -0400, Rich Felker wrote:
> On Wed, May 25, 2016 at 11:33:50AM +0100, Mark Rutland wrote:
> > On Wed, May 25, 2016 at 05:43:03AM +0000, Rich Felker wrote:
> > > Signed-off-by: Rich Felker <dalias@libc.org>
> > > ---
> > >  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
> > > 
> > > diff --git a/arch/sh/boot/dts/j2_mimas_v2.dts b/arch/sh/boot/dts/j2_mimas_v2.dts
> > > new file mode 100755
> > > index 0000000..4a66cda
> > > --- /dev/null
> > > +++ b/arch/sh/boot/dts/j2_mimas_v2.dts
> > > @@ -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>;
> > > +
> > > +	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.
> > 
> > > +		};
> > > +	};
> > > +
> > > +	memory@10000000 {
> > > +		device_type = "memory";
> > > +		reg = < 0x10000000 0x4000000 >;
> > > +	};
> > 
> > Likewise.
> 
> OK, I'll change both of these.
> 
> > > +
> > > +	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.

Ah, ok. I was not aware of this.

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

Please do.

Thanks,
Mark.

[PATCH v3 04/12] of: add J-Core timer bindings

[Rich Felker]

Signed-off-by: Rich Felker <dalias@libc.org>
---
 .../devicetree/bindings/timer/jcore,pit.txt        | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/jcore,pit.txt

diff --git a/Documentation/devicetree/bindings/timer/jcore,pit.txt b/Documentation/devicetree/bindings/timer/jcore,pit.txt
new file mode 100644
index 0000000..96c6815
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/jcore,pit.txt
@@ -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:
+
+timer@200 {
+	compatible = "jcore,pit";
+	reg = < 0x200 0x30 >;
+	cpu-offset = < 0x300 >;
+	interrupts = < 0x48 >;
+};
-- 
2.8.1

Re: [PATCH v3 04/12] of: add J-Core timer bindings

[Rob Herring]

On Wed, May 25, 2016 at 05:43:03AM +0000, Rich Felker wrote:
> Signed-off-by: Rich Felker <dalias@libc.org>
> ---
>  .../devicetree/bindings/timer/jcore,pit.txt        | 28 ++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/timer/jcore,pit.txt
> 
> diff --git a/Documentation/devicetree/bindings/timer/jcore,pit.txt b/Documentation/devicetree/bindings/timer/jcore,pit.txt
> new file mode 100644
> index 0000000..96c6815
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/jcore,pit.txt
> @@ -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:
> +
> +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.

Rob

Re: [PATCH v3 04/12] of: add J-Core timer bindings

[Rich Felker]

On Wed, Jun 01, 2016 at 08:58:52AM -0500, Rob Herring wrote:
> On Wed, May 25, 2016 at 05:43:03AM +0000, Rich Felker wrote:
> > Signed-off-by: Rich Felker <dalias@libc.org>
> > ---
> >  .../devicetree/bindings/timer/jcore,pit.txt        | 28 ++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/timer/jcore,pit.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/timer/jcore,pit.txt b/Documentation/devicetree/bindings/timer/jcore,pit.txt
> > new file mode 100644
> > index 0000000..96c6815
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/timer/jcore,pit.txt
> > @@ -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:
> > +
> > +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.

Rich

Re: [PATCH v3 04/12] of: add J-Core timer bindings

[Rich Felker]

On Wed, Jun 01, 2016 at 01:53:07PM -0400, Rich Felker wrote:
> On Wed, Jun 01, 2016 at 08:58:52AM -0500, Rob Herring wrote:
> > On Wed, May 25, 2016 at 05:43:03AM +0000, Rich Felker wrote:
> > > Signed-off-by: Rich Felker <dalias@libc.org>
> > > ---
> > >  .../devicetree/bindings/timer/jcore,pit.txt        | 28 ++++++++++++++++++++++
> > >  1 file changed, 28 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/timer/jcore,pit.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/timer/jcore,pit.txt b/Documentation/devicetree/bindings/timer/jcore,pit.txt
> > > new file mode 100644
> > > index 0000000..96c6815
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/timer/jcore,pit.txt
> > > @@ -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:
> > > +
> > > +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.

As a bit more background: the current mappings at 0xabcd0200 and
0xabcd0500 seem to have been chosen to maintain address-level
compatibility with existing non-smp builds (0xabcd0200) while avoiding
stepping on unrelated nearby mmio ranges -- uarts 1 and 2 were at
0xabcd0300 and 0xabcd0400 on systems with multiple uarts. Eventually
these can all be moved arbitrarily and put in a more logical
arrangement once everything is using device tree, but for the time
being, compatibility with non-DT kernels is still needed.

Rich

Re: [PATCH v3 04/12] of: add J-Core timer bindings

[Rob Herring]

On Wed, Jun 1, 2016 at 12:53 PM, Rich Felker <dalias@libc.org> wrote:
> On Wed, Jun 01, 2016 at 08:58:52AM -0500, Rob Herring wrote:
>> On Wed, May 25, 2016 at 05:43:03AM +0000, Rich Felker wrote:
>> > Signed-off-by: Rich Felker <dalias@libc.org>
>> > ---
>> >  .../devicetree/bindings/timer/jcore,pit.txt        | 28 ++++++++++++++++++++++
>> >  1 file changed, 28 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/timer/jcore,pit.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/timer/jcore,pit.txt b/Documentation/devicetree/bindings/timer/jcore,pit.txt
>> > new file mode 100644
>> > index 0000000..96c6815
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/timer/jcore,pit.txt
>> > @@ -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:
>> > +
>> > +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?

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.

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

Rob

Re: [PATCH v3 04/12] of: add J-Core timer bindings

[Rich Felker]

On Wed, Jun 01, 2016 at 05:36:19PM -0500, Rob Herring wrote:
> On Wed, Jun 1, 2016 at 12:53 PM, Rich Felker <dalias@libc.org> wrote:
> > On Wed, Jun 01, 2016 at 08:58:52AM -0500, Rob Herring wrote:
> >> On Wed, May 25, 2016 at 05:43:03AM +0000, Rich Felker wrote:
> >> > Signed-off-by: Rich Felker <dalias@libc.org>
> >> > ---
> >> >  .../devicetree/bindings/timer/jcore,pit.txt        | 28 ++++++++++++++++++++++
> >> >  1 file changed, 28 insertions(+)
> >> >  create mode 100644 Documentation/devicetree/bindings/timer/jcore,pit.txt
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/timer/jcore,pit.txt b/Documentation/devicetree/bindings/timer/jcore,pit.txt
> >> > new file mode 100644
> >> > index 0000000..96c6815
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/timer/jcore,pit.txt
> >> > @@ -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:
> >> > +
> >> > +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?
> 
> 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?

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

Rich

Re: [PATCH v3 04/12] of: add J-Core timer bindings

[Rob Herring]

On Wed, Jun 01, 2016 at 09:34:07PM -0400, Rich Felker wrote:
> On Wed, Jun 01, 2016 at 05:36:19PM -0500, Rob Herring wrote:
> > On Wed, Jun 1, 2016 at 12:53 PM, Rich Felker <dalias@libc.org> wrote:
> > > On Wed, Jun 01, 2016 at 08:58:52AM -0500, Rob Herring wrote:
> > >> On Wed, May 25, 2016 at 05:43:03AM +0000, Rich Felker wrote:
> > >> > Signed-off-by: Rich Felker <dalias@libc.org>
> > >> > ---
> > >> >  .../devicetree/bindings/timer/jcore,pit.txt        | 28 ++++++++++++++++++++++
> > >> >  1 file changed, 28 insertions(+)
> > >> >  create mode 100644 Documentation/devicetree/bindings/timer/jcore,pit.txt
> > >> >
> > >> > diff --git a/Documentation/devicetree/bindings/timer/jcore,pit.txt b/Documentation/devicetree/bindings/timer/jcore,pit.txt
> > >> > new file mode 100644
> > >> > index 0000000..96c6815
> > >> > --- /dev/null
> > >> > +++ b/Documentation/devicetree/bindings/timer/jcore,pit.txt
> > >> > @@ -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:
> > >> > +
> > >> > +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?
> > 
> > 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?

Maybe the memory mapped ARM arch timer. I'm not too sure off hand. It is 
quite common for devices to have multiple reg addresses.

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

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.

Rob

Re: [PATCH v3 04/12] of: add J-Core timer bindings

[Rich Felker]

On Thu, Jun 02, 2016 at 05:44:25PM -0500, Rob Herring wrote:
> > > > 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.
> 
> 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?

Rich
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 04/12] of: add J-Core timer bindings

[Rich Felker]

On Thu, Jun 23, 2016 at 05:16:08PM -0400, Rich Felker wrote:
> On Thu, Jun 02, 2016 at 05:44:25PM -0500, Rob Herring wrote:
> > > > > 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.
> > 
> > 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?

Ping.

I want to submit this again for the upcoming merge window and need to
know whether you still want me to pursue different approaches.

Rich

Re: [PATCH v3 00/12] J-core J2 cpu and SoC peripherals support

[Geert Uytterhoeven]

Hi Rich,

On Wed, May 25, 2016 at 7:43 AM, Rich Felker <dalias-8zAoT0mYgF4@public.gmane.org> wrote:
> 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

Drivers outside arch/sh/ should go through the respective maintainer's tree,
unless that maintainer allows you to take it by giving his Acked-by.
The same for DT bindings for such drivers.

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

Please also add a changelog, so people who reviewed your previous series
before know what to skip and what to focus on.
Please add collected Acked-by and Reviewed-by tags when reposting.

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 00/12] J-core J2 cpu and SoC peripherals support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1292 bytes --]

On Wed, May 25, 2016 at 05:43:02AM +0000, Rich Felker wrote:

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

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.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v3 00/12] J-core J2 cpu and SoC peripherals support

[Rich Felker]

On Wed, May 25, 2016 at 10:54:44AM +0100, Mark Brown wrote:
> On Wed, May 25, 2016 at 05:43:02AM +0000, Rich Felker wrote:
> 
> > 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):

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

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.

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

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

Rich

Re: [PATCH v3 00/12] J-core J2 cpu and SoC peripherals support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 6430 bytes --]

On Wed, May 25, 2016 at 06:24:41PM -0400, Rich Felker wrote:
> On Wed, May 25, 2016 at 10:54:44AM +0100, Mark Brown wrote:
> > On Wed, May 25, 2016 at 05:43:02AM +0000, Rich Felker wrote:

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

Given that you started off with "my intent is to include as much as
possible" and the general apparent lack of clarity about the process
it's really not sufficiently obvious to me based on your message that
this is clear to you.  The presentation of your message especially in
the context of the prior discussion suggests that it is expected for
things to go in at this point which haven't even been in -next yet and
that this is all perfectly normal, it is really not clear enough to me
reading it that you are looking for acks but instead sounds like you
might possibly intend to try to send anything that doesn't get
explicitly nacked.

It would really have helped to have some explicit mention of the fact
that you understand that what you are asking for is unusual and some
discussion of why you think it should still go in.  The best and
clearest thing to do would have been to post the series you were
considering sending as one series and everything else separately.  This
is one of the reasons why it was recommended to you that you should
split things up, it helps make things clear - the normal thing would be
that a series like this would be what you were planning to send.
Failing that another thing that'd have helped would be an explicit
mention of the bits you knew weren't going to be included in any pull
request.

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

Since you were still talking about sending pull requests for this code
during this mergen window after the previous thread I want to be as sure
as I can be that you do understand the process and remove any hint of
ambiguity.

Note that you should not expect that people are going to send you an
explicit message about when they intend to review things and usually a
week is considered the lowest bound for chasing on things that aren't
urgent.

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

You're not a new contributor posting some patches here, you are talking
about sending pull requests as the architecture maintainer.  That's
rather different.  

If you were just sending patches that would be fine and not at all
disruptive, that is a perfectly normal part of the workflow, but that's
not what's happening here.  I'm actually one of the people who's more
gung ho about applying things - I do tend to apply patches right up to
the wire and will carry on reviewing and applying new code through the
merge window (I looked at your first version after all) but only fixes
will get queued up for Linus, anything else that sits on topic branches
until after the merge window.

> 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

Even if people aren't going to run the code it's buildable on other
architectures (as it should be so we can compile test things, do static
analysis and so on) and breaking the build or even introducing new
warnings during the merge window isn't helpful.  People build and test
things like all*config as a matter of routine and if those get broken
then that takes people's time can mask other issues.

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

Like I say it's the bit where you're talking about sending pull requests
that's really flagging up here.

Most people's changes are important for them and only affect some
specific subset of platforms or systems which often aren't widely used
outside a given company but we still expect their changes to be in -next
before the merge window.  It's a lot easier for everyone to just follow
that rule, we have time based releases and things like -next available
so it is really not the end of the world to wait for one release, doing
this is fairer, minimises the risk of disruption for everyone and it
saves effort with evaluating the different bits of special pleading or
trying to rush to get reviews done quickly.

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

To repeat what was said last time the code and binding need to be
reviewed together - they will generally be merged together.  This means
that you need to copy subsystem maintainers on bindings for their
subsystem along with the code, as a rule you should send the binding to
at least everyone you send the code to.  Sending things to the lists
alone is not enough to ensure they will be seen with the code using
them.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]