Re: [PATCH v2 01/30] hw/arm/raspi: Use BCM2708 machine type with pre Device Tree kernels

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

Hi Zoltan,

On 2/6/20 1:21 PM, BALATON Zoltan wrote:
> On Thu, 6 Feb 2020, Philippe Mathieu-Daudé wrote:
>> When booting without device tree, the Linux kernels uses the $R1
>> register to determine the machine type. The list of values is
>> registered at [1].
>>
>> There are two entries for the Raspberry Pi:
>>
>> - https://www.arm.linux.org.uk/developer/machines/list.php?mid=3138
>>  name: MACH_TYPE_BCM2708
>>  value: 0xc42 (3138)
>>  status: Active, not mainlined
>>  date: 15 Oct 2010
>>
>> - https://www.arm.linux.org.uk/developer/machines/list.php?mid=4828
>>  name: MACH_TYPE_BCM2835
>>  value: 4828
>>  status: Active, mainlined
>>  date: 6 Dec 2013
>>
>> QEMU always used the non-mainlined type MACH_TYPE_BCM2708.
>> The value 0xc43 is registered to 'MX51_GGC' (processor i.MX51), and
>> 0xc44 to 'Western Digital Sharespace NAS' (processor Marvell 88F5182).
>>
>> The Raspberry Pi foundation bootloader only sets the BCM2708 machine
>> type, see [2] or [3]:
>>
>> 133 9:
>> 134     mov r0, #0
>> 135     ldr r1, =3138       @ BCM2708 machine id
>> 136     ldr r2, atags       @ ATAGS
>> 137     bx  r4
>>
>> U-Boot only uses MACH_TYPE_BCM2708 (see [4]):
>>
>> 25 /*
>> 26  * 2835 is a SKU in a series for which the 2708 is the first or 
>> primary SoC,
>> 27  * so 2708 has historically been used rather than a dedicated 2835 ID.
>> 28  *
>> 29  * We don't define a machine type for bcm2709/bcm2836 since the RPi 
>> Foundation
>> 30  * chose to use someone else's previously registered machine ID 
>> (3139, MX51_GGC)
>> 31  * rather than obtaining a valid ID:-/
>> 32  *
>> 33  * For the bcm2837, hopefully a machine type is not needed, since 
>> everything
>> 34  * is DT.
>> 35  */
>>
>> While the definition MACH_BCM2709 with value 0xc43 was introduced in
>> a commit described "Add 2709 platform for Raspberry Pi 2" out of the
>> mainline Linux kernel, it does not seem used, and the platform is
>> introduced with Device Tree support anyway (see [5] and [6]).
>>
>> Remove the unused values (0xc43 introduced in commit 1df7d1f9303aef
>> "raspi: add raspberry pi 2 machine" and 0xc44 in commit bade58166f4
>> "raspi: Raspberry Pi 3 support"), keeping only MACH_TYPE_BCM2708.
>>
>> [1] https://www.arm.linux.org.uk/developer/machines/
>> [2] 
>> https://github.com/raspberrypi/tools/blob/920c7ed2e/armstubs/armstub7.S#L135 
>>
>> [3] 
>> https://github.com/raspberrypi/tools/blob/49719d554/armstubs/armstub7.S#L64 
>>
>> [4] 
>> https://gitlab.denx.de/u-boot/u-boot/blob/v2015.04/include/configs/rpi-common.h#L18 
>>
>> [5] 
>> https://github.com/raspberrypi/linux/commit/d9fac63adac#diff-6722037d79570df5b392a49e0e006573R526 
>>
>> [6] 
>> http://lists.infradead.org/pipermail/linux-rpi-kernel/2015-February/001268.html 
>>
>>
>> Cc: Zoltán Baldaszti <bztemail@gmail.com>
>> Cc: Pekka Enberg <penberg@iki.fi>
>> Cc: Stephen Warren <swarren@nvidia.com>
>> Cc: Kshitij Soni <kshitij.soni@broadcom.com>
>> Cc: Michael Chan <michael.chan@broadcom.com>
>> Cc: Andrew Baumann <Andrew.Baumann@microsoft.com>
>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> hw/arm/raspi.c | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
>> index 3996f6c63a..ef76a27f33 100644
>> --- a/hw/arm/raspi.c
>> +++ b/hw/arm/raspi.c
>> @@ -29,8 +29,7 @@
>> #define FIRMWARE_ADDR_3 0x80000 /* Pi 3 loads kernel.img here by 
>> default */
>> #define SPINTABLE_ADDR  0xd8 /* Pi 3 bootloader spintable */
>>
>> -/* Table of Linux board IDs for different Pi versions */
>> -static const int raspi_boardid[] = {[1] = 0xc42, [2] = 0xc43, [3] = 
>> 0xc44};
> 
> Maybe worth a comment about why using this number (short, one line 
> version of commit message).

You mean something like:

/* Registered machine type:
  * https://www.arm.linux.org.uk/developer/machines/list.php?mid=3138
  */

> 
> Regards,
> BALATON Zoltan
> 
>> +#define MACH_TYPE_BCM2708   3138 /* Linux board IDs */
>>
>> typedef struct RasPiState {
>>     BCM283XState soc;
>> @@ -116,7 +115,7 @@ static void setup_boot(MachineState *machine, int 
>> version, size_t ram_size)
>>     static struct arm_boot_info binfo;
>>     int r;
>>
>> -    binfo.board_id = raspi_boardid[version];
>> +    binfo.board_id = MACH_TYPE_BCM2708;
>>     binfo.ram_size = ram_size;
>>     binfo.nb_cpus = machine->smp.cpus;
>>
>>