Re: [PATCH 1/2] mmc: mmci: Initial support to manage variant specific callbacks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux