Message-ID: <CAA07jV9oc-AQQGN1sKHEgE09gVrG+Ng9Dx6f1TbcRrAbK6g8Sw@mail.gmail.com>
Date:	Thu, 16 Jul 2015 17:04:33 +1000
From:	Wendy Liang <sunnyliangjy@...il.com>
To:	Mark Rutland <mark.rutland@....com>
Cc:	Wendy Liang <wendy.liang@...inx.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"ohad@...ery.com" <ohad@...ery.com>, Jason Wu <j.wu@...inx.com>,
	"michal.simek@...inx.com" <michal.simek@...inx.com>,
	Wendy Liang <jliang@...inx.com>
Subject: Re: [RFC PATCH 2/2] Documentation: DTS: bindings: add ZynqMP RPU remoteproc

Hi Mark,

On Wed, Jul 15, 2015 at 10:50 PM, Mark Rutland <mark.rutland@....com> wrote:
>
> On Wed, Jul 15, 2015 at 01:22:55PM +0100, Wendy Liang wrote:
> > Add documentation about the ZynqMP RPU remoteproc driver
> > DTS bindings.
> >
> > Signed-off-by: Jason Wu <j.wu@...inx.com>
> > Signed-off-by: Wendy Liang <jliang@...inx.com>
> > ---
> >  .../bindings/remoteproc/zynqmp_r5_remoteproc.txt   | 49 ++++++++++++++++++++++
> >  1 file changed, 49 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/remoteproc/zynqmp_r5_remoteproc.txt
> >
> > diff --git a/Documentation/devicetree/bindings/remoteproc/zynqmp_r5_remoteproc.txt b/Documentation/devicetree/bindings/remoteproc/zynqmp_r5_remoteproc.txt
> > new file mode 100644
> > index 0000000..94c1d00
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/remoteproc/zynqmp_r5_remoteproc.txt
> > @@ -0,0 +1,49 @@
> > +Xilinx ARM Cortex A53-R5 remoteproc driver
> > +==========================================
> > +
> > +ZynqMP family of devices use two Cortex R5 processors to help with various
> > +low power / real time tasks.
> > +
> > +This driver requires specific ZynqMP hardware design.
> > +
> > +ZynqMP R5 RemoteProc Device Node:
> > +=================================
> > +A zynqmp_r5_remoteproc device node is used to represent a R5 IP instance
> > +within ZynqMP SoC.
> > +
> > +Required properties:
> > +--------------------
> > + - compatible : Should be "xlnx,zynqmp-r5-remoteproc-1.0"
> > + - reg : Address and length of the register set for the device. It
> > +        contains in the same order as described reg-names
> > + - reg-names: Contain the register set names. For direct control method,
> > +              ipi, rpu_base and apb_base must be provided
> > + - interrupts : Interrupt mapping for remoteproc IPI
> > + - interrupt-parent : Phandle for the interrupt controller
> > +
> > +Optional properties:
> > +--------------------
> > + - firmware : Default firmware image name "rproc-rpu-fw" which can be
> > +              loaded by remoteproc framework. This value can be override
> > +              by "firmware" module parameter.
>
> The binding shouldn't mention Linux specifics, to the stuff about
> modules should go.
>
> Why do you think you need the firmware name in the DT? It breaks
> layering (the DT has to know about the user's filesystem), so I'm not
> sure that makes much sense.


 Ok, We will remove it from the DTS.
 We use it in the DTS is because we want to have different remoteproc instance,
 and each of it can use a different firmware.
 We previously think it the simplest to put it to DTS.

>
>
> > + - bootmem : R5 boot device (valid string, ocm or tcm), default is ocm.
> > + - core_conf : R5 core configuration (valid string - split0 or split1 or
> > +               lock-step), default is lock-step.
>
> What do these mean, when would I choose each?

 bootmem: where the RPU boots, if bootmem is "ocm", it means it will
be configured
 as "HIVEC" and boots from 0xffff0000 which is in OCM in our ZynqMP SoC.
 If it is "tcm", it will be configured as "LOVEC", and boots form 0
which is TCM.
 Since it is confusing, I will rename it, and will use "hivec" and
"lovec" for the values.

 core_conf: it actually means the operation mode. We have R5-0 and R5-1,
 "split0" means RPU is running in split mode and the remoteproc is for R5-0,
 "lock-step" means RPU is running in lockstep mode.
>
>
> s/_/-/ in property names.

 Will change it
>
>
> > + - method : RPU and IPI control method - direct, smc, hvc, default is smc.
>
> Likewise.
>
> > + - ipi_dest_mask : IPI channel destination mask. This is used to clear
> > +         ipi SR and let the rpu know the kernel has completed the work.
> > +         default mask is "0x100"
>
> We shouldn't encode masks like this in DT.
>
> Could you elaborate on how this is used?


 We have IPI in our SoC, we don't have hardcoded IPI channel binding, though
 we have default channel binding settings.
 this property here is because in case user don't want to use default bindings
 and they have their own configuration settings.

 We will give number to each channel, and will change to do the
mapping in the driver,
 instead of using the mask directly in the DTS.

 Thanks & Regards,
 Wendy

>
>
> Thanks,
> Mark.
>
> > +
> > +Example:
> > +--------
> > +     zynqmp-r5-remoteproc@0 {
> > +             compatible = "xlnx,zynqmp-r5-remoteproc";
> > +             reg = <0x0 0x3ed00000 0x800000>, <0x0 0xff300000 0x100>, <0x0 0xff9a0000 0x400>, <0x0 0xff5e0000 0x400>;
> > +             reg-names = "shared", "ipi", "rpu_base", "apb_base";
> > +             core_conf = "split0";
> > +             method = "direct";
> > +             bootmem = "ocm";
> > +             firmware = "r5_image";
> > +             interrupt-parent = <&gic>;
> > +             interrupts = <0 49 4>;
> > +     } ;
> > --
> > 2.1.1
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@...ts.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists