Hi, John Youn <John.Youn at synopsys.com> writes: [...] >>>> + */ >>>> + if (of_device_is_compatible(np, "rockchip,rk3288-usb")) >>>> + hsotg->phy->ops->reset(hsotg->phy); >>>> + >>> >>> You should probably check for NULL before calling the reset() >>> callback. >> Sure. >>> >>> Also, I'd rather see a generic quirk property that you set for your >>> platform. >>> >>> Something like "phy_reset_on_wakeup_quirk". >> But Rob Herring want me to implied by the SoC specific compatible >> string. I agree with him. It is certainly bug in RK3288 platform. >> It is no found no the other platform. > > Ok, I missed that before. > > Based on the drivers I'm familiar with (like dwc3), you would > typically add a "quirk" anyways. > > Felipe, > > Do you have some policy or preference on this? if it's not a dwc2-generic feature, then let's do it via compatible flag, sure. What we don't want is for things like: if (is_compatible('synopsys') || is_compatible('rockchip') || is_compatible('foobar') ... ) For that, we'd be better of adding a generic quirk flag which several can use. -- balbi -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 800 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-rockchip/attachments/20160829/ed8d6add/attachment.sig>