>> > >> >Why not just extend the internal structure so that not every OTG >> >controller has to define this type of structure? I think they should >> >all be the same, right? >> >> This data structure is only used for Intel MID Platform USB OTG controllers. >> So I did not extend the struct otg_transceiver for all OTG controllers. >> But I also think check if any member can be moved into the internal structure >> for common use will be one thing we can do in future development/improvement. > >Why not do that now? Hi Greg, Because driver development is still on going, for some reason, parts of the members are not well tested yet. : ( So I prefer to do it after that. > >> >> +static inline int >> >> +intel_mid_otg_register_notifier(struct intel_mid_otg_xceiv *iotg, >> >> + struct notifier_block *nb) >> >> +{ >> >> + return atomic_notifier_chain_register(&iotg->iotg_notifier, nb); >> >> +} >> >> + >> >> +static inline void >> >> +intel_mid_otg_unregister_notifier(struct intel_mid_otg_xceiv *iotg, >> >> + struct notifier_block *nb) >> >> +{ >> >> + atomic_notifier_chain_unregister(&iotg->iotg_notifier, nb); >> >> +} >> > >> >Are these really needed? >> >> I think it will make code a little bit more readable if such wrapped inline function >> used. > >No, it is obvious with the call to the atomic_notifier_chain_* function >what you are doing, don't wrap up things that don't need to be please. Hm... I did it using the same way as struct otg_transceiver. : ) Another point here from my side is that, it can reduce code changes in the future. As known, this common data structure is used in all Intel MID OTG Transceiver drivers. So if we want to change to another kind of notifier chain (blocking?) or want to do some other code update here, we can only did it in the wrapped function easily and no need to touch code in all related transceiver drivers. Thanks Hao -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html