From: Sascha Hauer <s.hauer@pengutronix.de>
To: David Brandt <d.brandt@phytec.de>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH v2] commands: bootaux: New command bootaux for iMX
Date: Mon, 18 Mar 2019 11:15:48 +0100 [thread overview]
Message-ID: <20190318101548.7xrlmvgl75umyn2n@pengutronix.de> (raw)
In-Reply-To: <20190313144109.1066-1-d.brandt@phytec.de>
Hi David,
On Wed, Mar 13, 2019 at 03:41:09PM +0100, David Brandt wrote:
> This patch add the bootaux command which starts
> auxiliary cores of iMX processors, currently only
> iMX8MQ. This is based on the u-boot-imx command.
>
> The currently unnecessary parameter for the core
> to start is necessary for the iMX8QM and other
> processors having multiple auxiliary cores.
>
> Signed-off-by: David Brandt <d.brandt@phytec.de>
> ---
> Changes v1->v2:
> - removed ifdef around imx_auxiliary functions
> in arch/arm/mach-imx/imx8mq.c
> - cleaned up includes in commands/bootaux.c
> - clarified what ADDR is and what is done
> - core is now specified with -c option
> - added some checks and message to hint what
> could be wrong
> - use kstrtouint/kstrtoul
> ---
> arch/arm/mach-imx/imx8mq.c | 37 ++++++++++++
> arch/arm/mach-imx/include/mach/generic.h | 4 ++
> commands/Kconfig | 16 ++++++
> commands/Makefile | 1 +
> commands/bootaux.c | 73 ++++++++++++++++++++++++
> 5 files changed, 131 insertions(+)
> create mode 100644 commands/bootaux.c
>
> diff --git a/arch/arm/mach-imx/imx8mq.c b/arch/arm/mach-imx/imx8mq.c
> index 3f6b433a5..98c62b58d 100644
> --- a/arch/arm/mach-imx/imx8mq.c
> +++ b/arch/arm/mach-imx/imx8mq.c
> @@ -124,3 +124,40 @@ static int imx8mq_report_hdmi_firmware(void)
> return 0;
> }
> console_initcall(imx8mq_report_hdmi_firmware);
> +
> +#define FSL_SIP_SRC 0xC2000005
> +#define FSL_SIP_SRC_M4_START 0x00
> +#define FSL_SIP_SRC_M4_STARTED 0x01
> +int imx_auxiliary_core_up(unsigned int core_id, ulong boot_private_data)
> +{
> + u32 stack, pc;
> + struct arm_smccc_res res;
> +
> + if (core_id || !boot_private_data)
> + return -EINVAL;
> +
> + stack = *(u32 *)boot_private_data;
> + pc = *(u32 *)(boot_private_data + 4);
> +
> + /* Set the stack and pc to M4 bootROM */
> + writel(stack, MX8MQ_M4_BOOTROM_BASE_ADDR);
> + writel(pc, MX8MQ_M4_BOOTROM_BASE_ADDR + 4);
> +
> + /* Enable M4 */
> + arm_smccc_smc(FSL_SIP_SRC, FSL_SIP_SRC_M4_START,
> + 0, 0, 0, 0, 0, 0, &res);
> +
> + return res.a0;
> +}
I think this needs more work. First of all we have
imx_auxiliary_core_up(), but this function is i.MX8 specific, i.MX6/7
need other methods. Similar to this "bootaux" is a generic command name,
but here i.MX specific functions are called. The typical U-Boot way
would be to add #ifdefs to it until it works for other known cases and
until the code is completely unreadable. Fortunately we don't do U-Boot
here, so we'll need some kind of cpu core registration function and a
generic command could show a list of registered cores and start a
particular one.
That would fit well with the already existing devicetree binding for the
auxiliary i.MX cores, see bindings/remoteproc/imx-rproc.txt, compatible
"fsl,imx7d-cm4" and "fsl,imx6sx-cm4". The code for the aux cores could
then be simply written as regular drivers.
Also we should make sure that the memory area used for the aux core
isn't passed to Linux. I would expect some convenience here for the user
so that he doesn't have to adjust the devicetrees manually.
I agree with Oleksij here that remoteproc is the right approach for
doing this.
Overall I think these aux cores are a hot topic and we should do it
right rather than fast. Merging a trivial approach for it would just
give the impression that a SoC specific quick hack is the way to go
for others, but I'm quite sure we can do better.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
prev parent reply other threads:[~2019-03-18 10:16 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-11 13:41 [PATCH] " David Brandt
2019-03-11 23:47 ` Roland Hieber
2019-03-12 1:37 ` Andrey Smirnov
2019-03-12 6:03 ` Oleksij Rempel
2019-03-13 8:32 ` Sascha Hauer
2019-03-13 11:36 ` David Brandt
2019-03-13 14:41 ` [PATCH v2] " David Brandt
2019-03-18 10:15 ` Sascha Hauer [this message]
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=20190318101548.7xrlmvgl75umyn2n@pengutronix.de \
--to=s.hauer@pengutronix.de \
--cc=barebox@lists.infradead.org \
--cc=d.brandt@phytec.de \
/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