Re: [PATCH v4 1/3] dt-bindings: soc: add bindings for Intel HPS Copy Engine

[Rob Herring <robh@kernel.org>]

On Tue, May 10, 2022 at 03:23:23PM -0700, matthew.gerlach@linux.intel.com wrote:
> 
> 
> On Tue, 10 May 2022, Rob Herring wrote:
> 
> > On Sun, May 08, 2022 at 07:26:22AM -0700, matthew.gerlach@linux.intel.com wrote:
> > > From: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > > 
> > > Add device tree bindings documentation for the Intel Hard
> > > Processor System (HPS) Copy Engine.
> > > 
> > > Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > > ---
> > > v4:
> > >   - move from soc to soc/intel/
> > > 
> > > v3:
> > >   - remove unused label
> > >   - move from misc to soc
> > >   - remove 0x from #address-cells/#size-cells values
> > >   - change hps_cp_eng@0 to dma-controller@0
> > >   - remote inaccurate 'items:' tag
> > > ---
> > >  .../soc/intel/intel,hps-copy-engine.yaml      | 51 +++++++++++++++++++
> > >  1 file changed, 51 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/soc/intel/intel,hps-copy-engine.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/soc/intel/intel,hps-copy-engine.yaml b/Documentation/devicetree/bindings/soc/intel/intel,hps-copy-engine.yaml
> > > new file mode 100644
> > > index 000000000000..8634865015cd
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/soc/intel/intel,hps-copy-engine.yaml
> > > @@ -0,0 +1,51 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +# Copyright (C) 2022, Intel Corporation
> > > +%YAML 1.2
> > > +---
> > > +$id: "http://devicetree.org/schemas/soc/intel/intel,hps-copy-engine.yaml#"
> > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > > +
> > > +title: Intel HPS Copy Engine
> > > +
> > > +maintainers:
> > > +  - Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > > +
> > > +description: |
> > > +  The Intel Hard Processor System (HPS) Copy Engine is an IP block used to copy
> > > +  a bootable image from host memory to HPS DDR.  Additionally, there is a
> > > +  register the HPS can use to indicate the state of booting the copied image as
> > > +  well as a keep-a-live indication to the host.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: intel,hps-copy-engine
> > > +
> > > +  '#dma-cells':
> > > +    const: 1
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    bus@80000000 {
> > > +        compatible = "simple-bus";
> > > +        reg = <0x80000000 0x60000000>,
> > > +              <0xf9000000 0x00100000>;
> > > +        reg-names = "axi_h2f", "axi_h2f_lw";
> > 
> > A simple-bus doesn't have regs because it is simple. If you have
> > registers, then you need a specific compatible. You can have
> > 'simple-bus' as a fallback if the bus is completely setup by firmware
> > and the OS never needs to configure/manage it.
> 
> The hardware I'm trying to describe above is the connection from the HPS/SOC
> to the FPGA.  There are two ranges of physical addresses with this
> connection referred to as the "HPS to FPGA bridge" and the "Lightweight HPS
> to FPGA bridge".  Device tree subnodes of bus@80000000 are IP blocks in the
> FPGA.  The IP blocks may be connected to one or both of the
> physical address ranges.  Since these physical address ranges are not registers
> of the bus@80000000, the field names, reg and reg-names, are probably wrong.
> Should the reg field above really be ranges?

Yes, sound likes it should be.

Rob