Re: [PATCH 3/3] dt-bindings: remoteproc: Add Arm remoteproc

From: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
To: Sudeep Holla <sudeep.holla@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>,
	Bjorn Andersson <andersson@kernel.org>,
	Sudeep Holla <sudeep.holla@arm.com>,
	Mathieu Poirier <mathieu.poirier@linaro.org>,
	Rob Herring <robh+dt@kernel.org>,
	Liviu Dudau <liviu.dudau@arm.com>,
	Lorenzo Pieralisi <lpieralisi@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Drew.Reed@arm.com, Adam.Johnston@arm.com,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-remoteproc@vger.kernel.org
Subject: Re: [PATCH 3/3] dt-bindings: remoteproc: Add Arm remoteproc
Date: Fri, 15 Mar 2024 14:22:22 +0000	[thread overview]
Message-ID: <20240315142222.GA38748@e130802.arm.com> (raw)
In-Reply-To: <ZfMVcQsmgQUXXcef@bogus>

Hi Sudeep,

On Thu, Mar 14, 2024 at 03:19:13PM +0000, Sudeep Holla wrote:
> > The plan for the driver is as follows:
> >
> >     Step 1: provide a foundation driver capable of turning the core on/off
> >     Step 2: provide mailbox support for comms
> >     Step 3: provide FW reload capability
> >
> > Steps 2 & 3 are waiting for a HW update so the Cortex-A35 (running Linux) can
> > share memory with the remote core.
> >
> 
> Honestly, I would prefer to know the overall design before pushing any partial
> solution. If you know the final complete solution, present the same with
> the complete device tree binding for better understanding and review.

Sounds good to me. I'll make the binding as complete as possible.

> Agreed, but it is part of a bigger block with other functionality in place.
> MFD/syscon might be better way to use these registers. You never know in
> future you might want to use another set of 2-4 registers with a different
> functionality in another driver.
> 
> > It makes sense to me to use a mapped region of 8 bytes for both registers rather
> > than individual registers (since they are consecutive).
> 
> Not exactly. Are you sure, Linux will not have to use another other registers
> in that block ? Will you keep creating such (random if I may call it so)
> bindings for a smaller sets of register under "Host Base System Control
> registers".
> 
> I would see if it makes sense to put together a single binding for
> this "Host Base System Control" register(not sure what exactly that means).
> Use MFD/regmap you access parts of this block. The remoteproc driver can
> then be semi-generic(meaning applicable to group of similar platforms)
> based on the platform compatible and use this regmap to provide the
> functionality needed.

I like the idea of using syscon/regmap to represent the "Host Base System Control registers"
area. Thank you for suggesting that.

I think syscon is the way to go (rather than MFD). With syscon we can use
the generic syscon driver that converts a set of MMIO registers to a regmap,
allowing it to be accessed from multiple device drivers.
In our case these MMIO registers will be the "Host Base System Control registers".

remoteproc will be a child node under sysctrl node.

Cheers,
Abdellatif