On 18 January 2017 at 10:25, Arend Van Spriel <arend.vanspriel@xxxxxxxxxxxx> wrote: > On 17-1-2017 20:23, Rafał Miłecki wrote: >> From: Rafał Miłecki <rafal@xxxxxxxxxx> >> >> This change intends to make init/attach process easier to follow. >> >> What driver were doing in brcmf_bus_start wasn't bus specific at all and >> function brcmf_bus_stop wasn't undoing things done there. It seems >> brcmf_detach was actually undoing things done in both: brcmf_attach and >> brcmf_bus_start. >> >> To me it makes more sense to call these two function in a similar way as >> they both handle some part of attaching process. It also makes >> brcmf_detach more meaningful. > > To me this is all bike-shedding and I have two options 1) what's in a > name and let it pass, or 2) explain the naming. Let's try option 2). It > seems your expectation was different because of the name > brcmf_*bus*_start(). The function brcmf_attach() is called by > bus-specific code, ie. sdio, pcie, or usb, to instantiate the common > driver structures and essential common facilities, eg. debugfs, and > brcmf_bus_start() is called by bus-specific code when everything is > ready for use. For PCIe there is nothing between those calls but for > SDIO there are several steps before the party can start, hence the name. Sorry you find this cleanup try this way. If you still have some patience for this /silly/ stuff, I've one more question. So as you noticed (and confirmed my note) both: brcmf_attach and brcmf_bus_start are called from bus specific code. They initialize some *common* stuff. How did you come with the "brcmf_bus_start" name then? 1) It's called after bus got initialized (started?) 2) It's initializes common stuff 3) It doesn't execute bus specific code My point is "brcmf_bus_start" name doesn't seem to make much sense. If you have any better suggestion than "brcmf_bus_start" and "brcmf_attach_phase2", I'll be happy to use it. What could it be? brcmf_attach_after_bus_started would be more accurate but too long. -- Rafał