Hi, Greg
Thanks a lot for code review.
>> + * the Intel MID (Langwell/Penwell) otg transceiver driver needs to interact
>> + * with device and host drivers to implement the USB OTG related feature. More
>> + * function members are added based on otg_transceiver data structure for this
>> + * purpose.
>> + */
>> +struct intel_mid_otg_xceiv {
>> + struct otg_transceiver otg;
>> + struct otg_hsm hsm;
>> +
>> + /* base address */
>> + void __iomem *base;
>> +
>> + /* ops to access ulpi */
>> + struct iotg_ulpi_access_ops ulpi_ops;
>> +
>> + /* atomic notifier for interrupt context */
>> + struct atomic_notifier_head iotg_notifier;
>> +
>> + /* start/stop USB Host function */
>> + int (*start_host)(struct intel_mid_otg_xceiv *iotg);
>> + int (*stop_host)(struct intel_mid_otg_xceiv *iotg);
>> +
>> + /* start/stop USB Peripheral function */
>> + int (*start_peripheral)(struct intel_mid_otg_xceiv *iotg);
>> + int (*stop_peripheral)(struct intel_mid_otg_xceiv *iotg);
>> +
>> + /* start/stop ADP sense/probe function */
>> + int (*set_adp_probe)(struct intel_mid_otg_xceiv *iotg,
>> + bool enabled, int dev);
>> + int (*set_adp_sense)(struct intel_mid_otg_xceiv *iotg,
>> + bool enabled);
>> +
>> +#ifdef CONFIG_PM
>> + /* suspend/resume USB host function */
>> + int (*suspend_host)(struct intel_mid_otg_xceiv *iotg,
>> + pm_message_t message);
>> + int (*resume_host)(struct intel_mid_otg_xceiv *iotg);
>> +
>> + int (*suspend_peripheral)(struct intel_mid_otg_xceiv *iotg,
>> + pm_message_t message);
>> + int (*resume_peripheral)(struct intel_mid_otg_xceiv *iotg);
>> +#endif
>> +
>> +};
>
>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.
>
>> +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.
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
