[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <529c73bb-849c-434f-888c-0b010d280424@alliedtelesis.co.nz>
Date: Sun, 30 Jun 2024 22:39:04 +0000
From: Chris Packham <Chris.Packham@...iedtelesis.co.nz>
To: Rob Herring <robh@...nel.org>
CC: "tglx@...utronix.de" <tglx@...utronix.de>, "krzk+dt@...nel.org"
<krzk+dt@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
"tsbogend@...ha.franken.de" <tsbogend@...ha.franken.de>,
"daniel.lezcano@...aro.org" <daniel.lezcano@...aro.org>,
"paulburton@...nel.org" <paulburton@...nel.org>, "peterz@...radead.org"
<peterz@...radead.org>, "mail@...ger-koblitz.de" <mail@...ger-koblitz.de>,
"bert@...t.com" <bert@...t.com>, "john@...ozen.org" <john@...ozen.org>,
"sander@...nheule.net" <sander@...nheule.net>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "devicetree@...r.kernel.org"
<devicetree@...r.kernel.org>, "linux-mips@...r.kernel.org"
<linux-mips@...r.kernel.org>, "kabel@...nel.org" <kabel@...nel.org>,
"ericwouds@...il.com" <ericwouds@...il.com>
Subject: Re: [PATCH v3 8/9] mips: generic: add fdt fixup for Realtek reference
board
On 28/06/24 05:48, Rob Herring wrote:
> On Wed, Jun 26, 2024 at 10:33 PM Chris Packham
> <chris.packham@...iedtelesis.co.nz> wrote:
>> The bootloader used on the Realtek RTL9302C boards is an ancient vendor
>> fork of U-Boot that doesn't understand device trees. So to run a modern
>> kernel it is necessary use one of the APPENDED_DTB options.
>>
>> When appending the DTB the inintrd information, if present, needs to be
>> inserted into the /chosen device tree node. The bootloader provides the
>> initrd start/size via the firmware environment. Add a fdt fixup that
>> will update the device tree with the initrd information.
> Is this really specific to this board/soc?
Based on my sample size of 1 I can't say. I thought this was a quirk of
the realtek SDK version of U-Boot because it's based on something old
and dt unaware. I did think about putting this code in a more generic
place but in the end I went with something less invasive figuring it
could be expanded on later if needed.
> I think there are lots of MIPS boards in this state.
If anyone can confirm that I'd be happy to make the workaround kick in
if the fw_env has an initrd_start in it. I'm not sure if there is any
risk in that approach. I don't know how other mips boards are currently
booting using the generic board support.
> The code to handle all the possible
> combinations of bootloader handoff information and sources of DTB is
> quite the mess. Just for DTB source you have bootloader DTB, appended
> DTB, or built-in DTB (and there's even logic if you have multiple of
> those). Contrast that to arm32 ('the zoo"), where you have 2 choices:
> bootloader DTB or appended DTB with legacy bootloader parameters
> transferred to DTB. All the uglyness is contained and the kernel boot
> deals with 1 possibility. </rant>
>
>> Signed-off-by: Chris Packham <chris.packham@...iedtelesis.co.nz>
>> ---
>>
>> Notes:
>> Changes in v3:
>> - None
>> Changes in v2:
>> - update compatible string
>>
>> arch/mips/generic/Makefile | 1 +
>> arch/mips/generic/board-realtek.c | 81 +++++++++++++++++++++++++++++++
>> 2 files changed, 82 insertions(+)
>> create mode 100644 arch/mips/generic/board-realtek.c
>>
>> diff --git a/arch/mips/generic/Makefile b/arch/mips/generic/Makefile
>> index 56011d738441..ea0e4ad5e600 100644
>> --- a/arch/mips/generic/Makefile
>> +++ b/arch/mips/generic/Makefile
>> @@ -13,3 +13,4 @@ obj-$(CONFIG_LEGACY_BOARD_SEAD3) += board-sead3.o
>> obj-$(CONFIG_LEGACY_BOARD_OCELOT) += board-ocelot.o
>> obj-$(CONFIG_MACH_INGENIC) += board-ingenic.o
>> obj-$(CONFIG_VIRT_BOARD_RANCHU) += board-ranchu.o
>> +obj-$(CONFIG_MACH_REALTEK_RTL) += board-realtek.o
>> diff --git a/arch/mips/generic/board-realtek.c b/arch/mips/generic/board-realtek.c
>> new file mode 100644
>> index 000000000000..cd83fbf1968c
>> --- /dev/null
>> +++ b/arch/mips/generic/board-realtek.c
>> @@ -0,0 +1,81 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
> Kernel license by default is GPL-2.0-only. Why do something different?
Copy and paste from the existing arch/mips/generic/board-*.c. I'm fine
with making it GPL-2.0-only
>> +/*
>> + * Copyright (C) 2024 Allied Telesis
>> + */
>> +
>> +#include <linux/errno.h>
>> +#include <linux/libfdt.h>
>> +#include <linux/of_address.h>
> You aren't using this header.
Ack.
>> +#include <linux/types.h>
>> +
>> +#include <asm/fw/fw.h>
>> +#include <asm/machine.h>
>> +
>> +static __init int realtek_add_initrd(void *fdt)
>> +{
>> + int node, err;
>> + u32 start, size;
>> +
>> + node = fdt_path_offset(fdt, "/chosen");
>> + if (node < 0) {
>> + pr_err("/chosen node not found\n");
>> + return -ENOENT;
>> + }
>> +
>> + start = fw_getenvl("initrd_start");
>> + size = fw_getenvl("initrd_size");
>> +
>> + if (start == 0 && size == 0)
>> + return 0;
>> +
>> + pr_info("Adding initrd info from environment\n");
>> +
>> + err = fdt_setprop_u32(fdt, node, "linux,initrd-start", start);
>> + if (err) {
>> + pr_err("unable to set initrd-start: %d\n", err);
>> + return err;
>> + }
>> +
>> + err = fdt_setprop_u32(fdt, node, "linux,initrd-end", start + size);
>> + if (err) {
>> + pr_err("unable to set initrd-end: %d\n", err);
>> + return err;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct mips_fdt_fixup realtek_fdt_fixups[] __initconst = {
>> + { realtek_add_initrd, "add initrd" },
>> + {},
>> +};
>> +
>> +static __init const void *realtek_fixup_fdt(const void *fdt, const void *match_data)
>> +{
>> + static unsigned char fdt_buf[16 << 10] __initdata;
>> + int err;
>> +
>> + if (fdt_check_header(fdt))
>> + panic("Corrupt DT");
>> +
>> + fw_init_cmdline();
>> +
>> + err = apply_mips_fdt_fixups(fdt_buf, sizeof(fdt_buf), fdt, realtek_fdt_fixups);
>> + if (err)
>> + panic("Unable to fixup FDT: %d", err);
>> +
>> + return fdt_buf;
>> +
>> +}
>> +
>> +static const struct of_device_id realtek_of_match[] __initconst = {
>> + {
>> + .compatible = "realtek,rtl9302",
>> + },
>> + {}
>> +};
>> +
>> +MIPS_MACHINE(realtek) = {
>> + .matches = realtek_of_match,
>> + .fixup_fdt = realtek_fixup_fdt,
>> +};
>> --
>> 2.45.2
>>
>>
Powered by blists - more mailing lists