diff --git a/include/hddsupport.h b/include/hddsupport.h index 0304beb5e..133cb3d2d 100644 --- a/include/hddsupport.h +++ b/include/hddsupport.h @@ -66,6 +66,12 @@ typedef enum { HDD_LOADMODULES_STATUS_COUNT, } hdd_loadmodules_status; +// See hddLoadModulesReady() below the prototypes -- the single evaluate-once way to ask "is the ATA +// stack actually resident?". Callers used to test `hddLoadModules() >= 0`, which also accepted +// BUSYLOADING(2) -- returned for the whole session after a FAILED first load consumed the one-shot +// count -- so "nothing is loaded" read as success and the APA page sat silently empty under both +// Auto and Manual (Vapor). Pair with the retryable-failure change in hddLoadModules. + int hddCheck(void); u32 hddGetTotalSectors(void); int hddIs48bit(void); @@ -91,6 +97,17 @@ void hddVcdInvalidateCache(void); void hddInit(item_list_t *itemList); item_list_t *hddGetObject(int initOnly); int hddLoadModules(void); + +// Load (or confirm) the ATA stack and report residency, evaluating hddLoadModules EXACTLY ONCE. +// A function, not a macro taking the call as its argument: the earlier HDD_LOADMODULES_OK( +// hddLoadModules()) macro double-evaluated on any non-NOERROR first result -- and with the +// retryable-failure change, a FAILED load would have immediately re-run the whole load (second +// dev9 init + second error toast) inside one condition (CodeRabbit review of #241; vetted). +static inline int hddLoadModulesReady(void) +{ + int r = hddLoadModules(); + return r == HDD_LOADMODULES_STATUS_NOERROR || r == HDD_LOADMODULES_STATUS_ALREADYLOADED; +} void hddLoadSupportModules(void); void hddLaunchGame(item_list_t *itemList, int id, config_set_t *configSet); int hddIsPresent(); diff --git a/src/bdmsupport.c b/src/bdmsupport.c index 68c6e94be..5369ae3e9 100644 --- a/src/bdmsupport.c +++ b/src/bdmsupport.c @@ -353,7 +353,7 @@ static void bdmLoadBlockDeviceModules(void) // Only mark loaded on success -- a failure (no HDD/interface) must not // suppress future retries via bdmShouldQueueModuleLoad() (matches the // conditional pattern used for USB/MX4SIO and opl.c's hddLoadModules gate). - if (hddLoadModules() >= 0) + if (hddLoadModulesReady()) hddModLoaded = 1; } @@ -1660,7 +1660,7 @@ static int bdmEnsureTransportLoaded(int bdmType) } return iLinkModLoaded; case BDM_TYPE_ATA: - if (!hddModLoaded && hddLoadModules() >= 0) + if (!hddModLoaded && hddLoadModulesReady()) hddModLoaded = 1; return hddModLoaded; default: diff --git a/src/hddsupport.c b/src/hddsupport.c index f93a59bf3..5e11fbeec 100644 --- a/src/hddsupport.c +++ b/src/hddsupport.c @@ -224,6 +224,15 @@ int hddLoadModules(void) LOG("HDD: No HardDisk Drive detected.\n"); setErrorMessageWithCode(_STR_HDD_NOT_CONNECTED_ERROR, ERROR_HDD_IF_NOT_DETECTED); retStatus = HDD_LOADMODULES_STATUS_ERROR; + // Make the failure RETRYABLE. Leaving the count consumed turned one bad first probe into a + // whole-session poison: every later call took the else branch and returned BUSYLOADING(2), + // which the >= 0 caller tests read as SUCCESS while nothing was loaded -- so the APA page + // sat silently empty under BOTH Auto and Manual (Vapor's report; the drive lists fine in + // wOPL/upstream because their earliest callers run at tab entry, when the drive is ready -- + // our fork adds boot-time callers like bdmResolveBootDir's ATA escalation, seconds after + // power-on). sysInitDev9/sysLoadModuleBuffer are safe to re-run; a later caller (e.g. the + // HDD tab) now gets a real second attempt instead of a poisoned latch. + hddModulesLoadCount = 0; } else { retStatus = HDD_LOADMODULES_STATUS_NOERROR; hddModulesLoaded = 1; @@ -313,9 +322,17 @@ void hddLoadSupportModules(void) // Check if the drive contains MBR/GPT partition data before we load the APA/PFS modules. If the drive is not // APA then loading the APA irx modules can corrupt the drive as it will try to write APA partition data. - if (hddDetectNonSonyFileSystem() != 0) { + int nonSony = hddDetectNonSonyFileSystem(); + if (nonSony != 0) { // Drive is MBR/GPT style, or unknown, bail out or risk corrupting the drive. - LOG("HDDSUPPORT LoadSupportModules bailing out early...\n"); + LOG("HDDSUPPORT LoadSupportModules bailing out early (%d)...\n", nonSony); + // A PROBE FAILURE (-1: the xhdd0: partition-sector devctl errored -- drive not ready, module + // missing, transient bus fault) was completely SILENT: no error box, no list, an APA page that + // just looks like a dead drive for the whole session. Surface it. The 1 (genuine MBR/GPT/exFAT) + // branch stays silent on purpose -- that is the NORMAL coexistence case for BDM-HDD users and + // must not toast at every boot. + if (nonSony < 0) + setErrorMessageWithCode(_STR_HDD_NOT_CONNECTED_ERROR, ERROR_HDD_NOT_DETECTED); return; } diff --git a/src/opl.c b/src/opl.c index 18b9efc2f..e4d1866cd 100644 --- a/src/opl.c +++ b/src/opl.c @@ -1313,7 +1313,7 @@ static int checkLoadConfigBDMHDD(int types) int value; // Bounded wait so BDM-on-HDD can be detected without long black-screen stalls. - if (hddLoadModules() >= 0 && bdmHDDIsPresent(500)) { + if (hddLoadModulesReady() && bdmHDDIsPresent(500)) { if (bdmFindPartition(path, CONFIG_OPL_FILENAME, 0) || bdmFindPartition(path, CONFIG_OPL_FILENAME_LEGACY, 0)) { configEnd(); configInit(path); @@ -1580,6 +1580,12 @@ static void _loadConfig() configGetInt(configOPL, CONFIG_OPL_ENABLE_ART_TAR, &gEnableArtTar); configGetInt(configOPL, CONFIG_OPL_WIDESCREEN, &gWideScreen); configGetInt(configOPL, CONFIG_OPL_DEFAULT_GAME_VIEW, &gDefaultGameView); + // Clamp: an out-of-enum value (hand-edited/corrupt config) is UNRECOVERABLE on-console -- + // vcdViewActive() treats anything but BOTH as a lock, the L3 toggle goes inert and its hint + // is hidden, so every VCD-capable page silently serves one (possibly empty) list with no + // way back. The coverflow ints just below have always been clamped; this one was not. + if (gDefaultGameView < GAME_VIEW_BOTH || gDefaultGameView > GAME_VIEW_VCD) + gDefaultGameView = GAME_VIEW_BOTH; // A boot default-view locked to one type (VCD or ISO) must force the same one-shot // rescan the settings dialog does on a view change (gui.c). Without it, vcdViewActive() // short-circuits mmce/bdm/hdd/eth NeedsUpdate before the initial-scan trigger and the @@ -1895,7 +1901,7 @@ static int trySaveConfigBDMHDD(int types) char path[64]; // Bounded wait so save can target BDM-on-HDD without long stalls. - if (hddLoadModules() >= 0 && bdmHDDIsPresent(500)) { + if (hddLoadModulesReady() && bdmHDDIsPresent(500)) { if (bdmFindPartition(path, CONFIG_OPL_FILENAME, 1)) { configSetMove(path); return configWriteMulti(types);