Hi Vitaly, On Mon, Aug 2, 2010 at 7:25 PM, Vitaly Wool <vitalywool@xxxxxxxxx> wrote: > On Mon, Aug 2, 2010 at 5:54 PM, Ohad Ben-Cohen <ohad@xxxxxxxxxx> wrote: >> SPI is using these spi_board_info tables to populate the SPI device >> trees. These tables are registered early at the board-specific init >> code, and are later used by SPI core to populate the devices when the >> SPI master controller is registered. >> >> SDIO doesn't normally needs any kind of hard coded data: most devices >> are dynamically probed and populated. >> >> On rare cases like the wl1271, we have some platform-specific data we >> need to deliver the SDIO function driver (i.e. the irq info in this >> case). Since the device is hardwired to a specific controller, it does >> make some sense to pass this private data from the controller's info >> in the board files, through the host driver, and make it accessible >> through the specific host instance that drives this controller. >> >> Btw, if our problem was be broader (e.g., needs to supply private data >> for non-hardwired devices), then I agree that a more complex >> mechanism, such as the one you suggest, would be needed. But currently >> the problem is very simple and the solution is even simpler: just add >> 1 private pointer to the host. >> >> Hope you find this reasonable, > > no, actually I don't. I think this is a hack that intrudes into the > area it completely doesn't belong to. > > In fact, one can have 2 views on this problem: either this is a fairly > generic problem we need to address, or this is a very specific corner > case. > Your solution will be treated as a hack in both cases. > > If we consider it a generic problem, then we need to find a generic > solution, which is the board_info solution, for instance. FWIW, I2C > also uses this approach now. > > If we consider it to be a corner case, let's just add a dummy > platform_device like it's done in WL1251 implementation and keep the > MMC subsystem clean. Let's start by defining the problem exactly: SDIO devices that are hardwired to a specific controller and have some platform data that needs to be available to the SDIO function driver. It doesn't get more generic than that - it's not needed with common fully-enumerable SDIO devices (at least I'm not aware of such need), hence a generic mechanism of maintaining a global pile of platform data pointers, which can be queried with the device and vendor ID, really sounds like an overkill. But it's not specific to the wl1271 device - I prefer to support this at the MMC level, so we wouldn't have to add an extra platform device for every SDIO function driver that needs to cope with such issue (we already have two drivers - wl1271_sdio.c and wl1251_sdio.c); Adding an extra platform device for every driver is just too much dummy code (that btw adds a well-hidden race condition between the two probe calls), which can be nicely eliminated if we just introduce this new per-host private data pointer. So we have a SDIO device hardwired to a specific controller, that is driven by a specific host controller driver instance. This patch allows this specific host instance to carry platform data for the device that is hardwired to it. The platform data would be available only to SDIO function driver instances of that specific host controller. The solution is generic enough to support all SDIO function drivers with similar needs, and it's extremely simple. I'm honestly trying to understand your concerns, but I'm afraid that just saying "it's a hack" is not too informative. Can you please explain what do you think is technically wrong with the proposed solution ? is it not general enough for other use cases ? will it break something ? Thanks, Ohad. > > Thanks, > Vitaly > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
