Attention is currently required from: Patrick Georgi, xin hua wang, Evgeny Zinoviev.
Patch set 16:Code-Review +1
45 comments:
File Documentation/mainboard/apple/macbookpro8_1.md:
nit: 8 MiB
Patch Set #16, Line 55: ## Untested
I imagine macOS is untested?
Patch Set #16, Line 57: Thunderbolt
Probably has issues because the thunderbolt controller needs some initialisation
File src/mainboard/apple/macbookpro8_1/Kconfig:
Patch Set #16, Line 16: select GFX_GMA_INTERNAL_IS_LVDS
Please drop
Patch Set #16, Line 21: string
Remove the type as per CB:56553
Patch Set #16, Line 22: apple/macbookpro8_1
This now needs to be surrounded in double quotes
Patch Set #16, Line 25: string
Remove the type as per CB:56554
Patch Set #16, Line 38: default 28
Um, gpio.c configures this as an input.
config MAX_CPUS
int
default 8
Remove as per CB:41839
File src/mainboard/apple/macbookpro8_1/acpi/ec.asl:
/*
* This file is part of the coreboot project.
*
* Copyright (c) 2019-2020 Evgeny Zinoviev <me@ch1p.io>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; version 2 of the License.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*/
Please use SPDX license headers
File src/mainboard/apple/macbookpro8_1/acpi/platform.asl:
/*
* This file is part of the coreboot project.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; version 2 of the License.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*/
Please use SPDX license headers
File src/mainboard/apple/macbookpro8_1/acpi_tables.c:
Make this comment a C-style comment, so that it is consistent with the other comments
Done
File src/mainboard/apple/macbookpro8_1/acpi_tables.c:
/*
* This file is part of the coreboot project.
*
* Copyright (C) 2008-2009 coresystems GmbH
* Copyright (C) 2014 Vladimir Serbinenko
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License as
* published by the Free Software Foundation; version 2 of
* the License.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*/
Please use SPDX license headers
Patch Set #16, Line 18: #include <southbridge/intel/bd82x6x/nvs.h>
Needs update
Patch Set #16, Line 20: global_nvs_t
Needs update
gnvs->tcrt = 100;
gnvs->tpsv = 90;
I don't think these are needed.
File src/mainboard/apple/macbookpro8_1/cmos.default:
Patch Set #16, Line 3: Enable
`Disable` would make more sense
File src/mainboard/apple/macbookpro8_1/cmos.layout:
Please indent with tabs.
##
## This file is part of the coreboot project.
##
## This program is free software; you can redistribute it and/or modify
## it under the terms of the GNU General Public License as published by
## the Free Software Foundation; version 2 of the License.
##
## This program is distributed in the hope that it will be useful,
## but WITHOUT ANY WARRANTY; without even the implied warranty of
## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
## GNU General Public License for more details.
##
Please use SPDX license headers
# -----------------------------------------------------------------
# Status Register A
# -----------------------------------------------------------------
# Status Register B
# -----------------------------------------------------------------
# Status Register C
#96 4 r 0 status_c_rsvd
#100 1 r 0 uf_flag
#101 1 r 0 af_flag
#102 1 r 0 pf_flag
#103 1 r 0 irqf_flag
# -----------------------------------------------------------------
# Status Register D
#104 7 r 0 status_d_rsvd
#111 1 r 0 valid_cmos_ram
# -----------------------------------------------------------------
# Diagnostic Status Register
#112 8 r 0 diag_rsvd1
Please remove
Patch Set #16, Line 38: #120 264 r 0 unused
Please remove
Patch Set #16, Line 44: #390 2 r 0 unused?
Please remove
Patch Set #16, Line 46: # -----------------------------------------------------------------
Where are the CMOS options for `gfx_uma_size`, among other things?
Patch Set #16, Line 48: #392 3 r 0 unused
Please remove
Patch Set #16, Line 50: #399 1 r 0 unused
Please remove
# SandyBridge MRC Scrambler Seed values
896 32 r 0 mrc_scrambler_seed
928 32 r 0 mrc_scrambler_seed_s3
960 16 r 0 mrc_scrambler_seed_chk
Only used with MRC.bin
File src/mainboard/apple/macbookpro8_1/devicetree.cb:
Patch Set #8, Line 105: device pci 1a.7 on
Device gone.
this is the find_debugport for ft232h logging but the port is not the ones on the side. does anyone have the schematics to see if there is test points that one can tap into to access this? thanks
Please don't derail comment threads. You could've asked your question in a new comment thread.
File src/mainboard/apple/macbookpro8_1/devicetree.cb:
PANEL_PORT_LVDS
register "c1_acpower" = "1"
register "c1_battery" = "1"
register "c2_acpower" = "3"
register "c2_battery" = "3"
register "c3_acpower" = "5"
register "c3_battery" = "5"
Needs to be updated:
register "acpi_c1" = "1"
register "acpi_c2" = "3"
register "acpi_c3" = "5"
Patch Set #16, Line 32: subsystemid 0x8086 0x7270 inherit
I think you can move this inside the southbridge chip, put `subsystemid 0x106b 0x00db inherit` here and drop all `subsystemid 0x106b 0x00db` lines.
device pci 01.0 on
subsystemid 0x106b 0x00db
end
Is there anything connected here? Or is this only on so that the thunderbolt root port function is discoverable? (multifunction PCI devices must implement function 0).
Patch Set #16, Line 46: register "c2_latency" = "0x0065"
Remove as per CB:55212
Patch Set #16, Line 52: register "pcie_port_coalesce" = "1"
Doesn't seem to be needed
Patch Set #16, Line 74: device pci 1f.0 on end # LPC bridge
No EC chip?
File src/mainboard/apple/macbookpro8_1/dsdt.asl:
/*
* This file is part of the coreboot project.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; version 2 of the License.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*/
Please use SPDX license headers
Patch Set #16, Line 18: 0x02, // DSDT revision: ACPI 2.0 and up
ACPI_DSDT_REV_2,
Patch Set #16, Line 21: // OEM revision
autoport nonsense, please remove
File src/mainboard/apple/macbookpro8_1/early_init.c:
/*
* This file is part of the coreboot project.
*
* Copyright (C) 2008-2009 coresystems GmbH
* Copyright (C) 2014 Vladimir Serbinenko
* Copyright (C) 2019-2020 Evgeny Zinoviev <me@ch1p.io>
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License as
* published by the Free Software Foundation; version 2 of
* the License.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*/
Please use SPDX license headers
void mainboard_early_init(int s3resume)
{
}
Can be removed
File src/mainboard/apple/macbookpro8_1/gma-mainboard.ads:
--
-- This file is part of the coreboot project.
--
-- This program is free software; you can redistribute it and/or modify
-- it under the terms of the GNU General Public License as published by
-- the Free Software Foundation; either version 2 of the License, or
-- (at your option) any later version.
--
-- This program is distributed in the hope that it will be useful,
-- but WITHOUT ANY WARRANTY; without even the implied warranty of
-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-- GNU General Public License for more details.
--
Please use SPDX license headers
Patch Set #16, Line 31: Internal
LVDS
File src/mainboard/apple/macbookpro8_1/gpio.c:
/*
* This file is part of the coreboot project.
*
* Copyright (C) 2008-2009 coresystems GmbH
* Copyright (C) 2014 Vladimir Serbinenko
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License as
* published by the Free Software Foundation; version 2 of
* the License.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*/
Please use SPDX license headers
File src/mainboard/apple/macbookpro8_1/hda_verb.c:
Patch Set #8, Line 21: Cirrus
Yes.
Please expand the comment, thanks
File src/mainboard/apple/macbookpro8_1/mainboard.c:
Patch Set #8, Line 24: RCBA32(0x38c8) = 0x00002005;
What does this do?
Is gone
File src/mainboard/apple/macbookpro8_1/mainboard.c:
/*
* This file is part of the coreboot project.
*
* Copyright (C) 2019 Evgeny Zinoviev <me@ch1p.io>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; version 2 of the License.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*/
Please use SPDX license headers
To view, visit change 33151. To unsubscribe, or for help writing mail filters, visit settings.