From: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
To: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Bjorn Andersson <andersson@kernel.org>,
Rob Herring <robh+dt@kernel.org>,
Liviu Dudau <liviu.dudau@arm.com>,
Sudeep Holla <sudeep.holla@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 1/3] remoteproc: Add Arm remoteproc driver
Date: Thu, 7 Mar 2024 19:40:26 +0000 [thread overview]
Message-ID: <20240307194026.GA355455@e130802.arm.com> (raw)
In-Reply-To: <ZeYWKVpeFm1+4mlT@p14s>
Hi Mathieu,
> > + do {
> > + state_reg = readl(priv->reset_cfg.state_reg);
> > + *rst_ack = EXTSYS_RST_ST_RST_ACK(state_reg);
> > +
> > + if (*rst_ack == EXTSYS_RST_ACK_RESERVED) {
> > + dev_err(dev, "unexpected RST_ACK value: 0x%x\n",
> > + *rst_ack);
> > + return -EINVAL;
> > + }
> > +
> > + /* expected ACK value read */
> > + if ((*rst_ack & exp_ack) || (*rst_ack == exp_ack))
>
> I'm not sure why the second condition in this if() statement is needed. As far
> as I can tell the first condition will trigger and the second one won't be
> reached.
The second condition takes care of the following: exp_ack and *rst_ack are both 0.
This case happens when RST_REQ bit is cleared (meaning: No reset requested) and
we expect the RST_ACK to be 00 afterwards.
> > +/**
> > + * arm_rproc_load() - Load firmware to memory function for rproc_ops
> > + * @rproc: pointer to the remote processor object
> > + * @fw: pointer to the firmware
> > + *
> > + * Does nothing currently.
> > + *
> > + * Return:
> > + *
> > + * 0 for success.
> > + */
> > +static int arm_rproc_load(struct rproc *rproc, const struct firmware *fw)
> > +{
>
> What is the point of doing rproc_of_parse_firmware() if the firmware image is
> not loaded to memory? Does the remote processor have some kind of default ROM
> image to run if it doesn't find anything in memory?
Yes, the remote processor has a default FW image already loaded by default.
rproc_boot() [1] and _request_firmware() [2] fail if there is no FW file in the filesystem or a filename
provided.
Please correct me if I'm wrong.
[1]: https://elixir.bootlin.com/linux/v6.8-rc7/source/drivers/remoteproc/remoteproc_core.c#L1947
[2]: https://elixir.bootlin.com/linux/v6.8-rc7/source/drivers/base/firmware_loader/main.c#L863
> > +module_platform_driver(arm_rproc_driver);
> > +
>
> I am echoing Krzysztof view about how generic this driver name is. This has to
> be related to a family of processors or be made less generic in some way. Have
> a look at what TI did for their K3 lineup [1] - I would like to see the same
> thing done here.
Thank you, I'll take care of that and of all the other comments made.
Cheers,
Abdellatif
next prev parent reply other threads:[~2024-03-07 19:40 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-01 16:42 [PATCH 0/3] remoteproc: introduce Arm remoteproc support abdellatif.elkhlifi
2024-03-01 16:42 ` [PATCH 1/3] remoteproc: Add Arm remoteproc driver abdellatif.elkhlifi
2024-03-04 18:30 ` Rob Herring
2024-03-04 18:42 ` Mathieu Poirier
2024-03-07 19:40 ` Abdellatif El Khlifi [this message]
2024-03-08 16:44 ` Mathieu Poirier
2024-03-11 11:44 ` Abdellatif El Khlifi
2024-03-12 16:29 ` Mathieu Poirier
2024-03-12 17:32 ` Abdellatif El Khlifi
2024-03-13 16:25 ` Mathieu Poirier
2024-03-13 17:17 ` Abdellatif El Khlifi
2024-03-14 14:52 ` Mathieu Poirier
2024-03-14 14:59 ` Sudeep Holla
2024-03-14 15:16 ` Abdellatif El Khlifi
2024-03-14 15:24 ` Sudeep Holla
2024-03-14 16:29 ` Mathieu Poirier
2024-03-25 17:13 ` Abdellatif El Khlifi
2024-03-26 14:20 ` Mathieu Poirier
2024-03-26 17:14 ` Abdellatif El Khlifi
2024-03-01 16:42 ` [PATCH 2/3] arm64: dts: Add corstone1000 external system device node abdellatif.elkhlifi
2024-03-01 19:27 ` Krzysztof Kozlowski
2024-03-08 12:21 ` Sudeep Holla
2024-03-08 14:25 ` Abdellatif El Khlifi
2024-03-01 16:42 ` [PATCH 3/3] dt-bindings: remoteproc: Add Arm remoteproc abdellatif.elkhlifi
2024-03-01 19:30 ` Krzysztof Kozlowski
2024-03-08 12:29 ` Sudeep Holla
2024-03-08 13:54 ` Abdellatif El Khlifi
2024-03-13 19:59 ` Robin Murphy
2024-03-14 13:49 ` Abdellatif El Khlifi
2024-03-14 13:56 ` Krzysztof Kozlowski
2024-03-14 15:20 ` Abdellatif El Khlifi
2024-03-14 15:19 ` Sudeep Holla
2024-03-15 14:22 ` Abdellatif El Khlifi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240307194026.GA355455@e130802.arm.com \
--to=abdellatif.elkhlifi@arm.com \
--cc=Adam.Johnston@arm.com \
--cc=Drew.Reed@arm.com \
--cc=andersson@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-remoteproc@vger.kernel.org \
--cc=liviu.dudau@arm.com \
--cc=lpieralisi@kernel.org \
--cc=mathieu.poirier@linaro.org \
--cc=robh+dt@kernel.org \
--cc=sudeep.holla@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).