On 13 July 2018 at 17:12, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote: > On Fri, Jul 13, 2018 at 1:15 PM Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote: > >> To be able to better support different mmci variants, we need to be able to >> use variant specific callbacks, rather than continue to sprinkle the code >> with additional variant data. To move in this direction, let's add an >> optional ->init() callback to the variant data struct, which variants shall >> use to assign the mmci_host_ops pointer. >> >> Using an ->init() callback enables us to partition the code between >> different files. To allow separate mmci variant files to implement the >> variant specifics, let's also move the definition of the struct >> variant_data to the common mmci header file. >> >> Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx> > > Good idea! Thanks! > >> + if (variant->init) >> + variant->init(host); > (...) >> +struct variant_data { > (...) >> + void (*init)(struct mmci_host *host); >> +}; >> + >> +/* mmci variant callbacks */ >> +struct mmci_host_ops { >> +}; > > Why not have the .init() callback inside the struct mmci_host_ops vtable? The idea is to let the ->init() callback to be assigned at compilation time and couple it with the amba_id. In that way, we can keep having one common driver and thus one ->probe() function. Moving the ->init() callback to the mmci_host_ops structure, doesn't really makes sense to me, as in principle that would requires us to have separate drivers, which during probe then would call common mmci library function to register their ops. We can do this as well, but I didn't want to move that far in these early steps. What do you think? > > If you think it's hairy to dereference twice like mmci->variant->ops(foo) > you can always copy the pointer mmci->ops = variant->ops and > be done with it. Sure, but as stated above, that was not the reason. > > It's easy to just assign it directly in the variant data that way: > > variant_foo = { > (...) > .ops = { > .init = ... > Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html