[haiku-commits] Re: haiku: hrev52174 - src/system/kernel/device_manager

  • From: Adrien Destugues <pulkomandy@xxxxxxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Mon, 6 Aug 2018 08:25:08 +0200

On Sun, Aug 05, 2018 at 09:57:58PM -0400, waddlesplash wrote:

hrev52174 adds 1 changeset to branch 'master'
old head: 8d86b84d18535654da5b693537802d6ca115efe0
new head: daad9a3c1cf5fa4884b0adf4e66ce9b03dc3e0b4
overview: 
https://git.haiku-os.org/haiku/log/?qt=range&q=daad9a3c1cf5+%5E8d86b84d1853

----------------------------------------------------------------------------

daad9a3c1cf5: kernel: Revert MMC bus changes to device_manager.
  
  This broke booting on most (all?) systems.

And I thought I requested a second review on this as I spent too much
time looking at the code…

@@ -1896,8 +1885,7 @@ device_node::Probe(const char* devicePath, uint32 
updateCycle)
                      // Check if this node matches the device path
                      // TODO: maybe make this extendible via settings file?
                      if (!strcmp(devicePath, "disk")) {
-                             matches = type == PCI_mass_storage
-                                     && (type == PCI_base_peripheral || 
subType == PCI_sd_host);
+                             matches = type == PCI_mass_storage;
                      } else if (!strcmp(devicePath, "audio")) {
                              matches = type == PCI_multimedia
                                      && (subType == PCI_audio || subType == 
PCI_hd_audio);



This condition was always false. I think what we wanted is:

                              matches = type == PCI_mass_storage
                                      || (type == PCI_base_peripheral && 
subType == PCI_sd_host);

Looks like a copypaste error from the multimedia one, which tests a
different thing.

I had raised a concern about this change in a previous version of the
patch, however. As far as I can tell, SD host is not itself a "disk"
module. It is another bus, on which we may find MMC "disks", but also
other things (SDIO cards which may provides webcams, wifi adapters,
etc). So it seems weird to have it added here.

I think the other parts of the change are fine and should be
reintroduced?

-- 
Adrien.

Other related posts:

  1. Home
  2. haiku-commits
  3. Archive
  4. 08-2018