Re: [PATCH 4.19.y-cip 15/23] usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller


Pavel Machek
 

Hi!

+static enum usb_role hd3ss3220_get_attached_state(struct hd3ss3220
+*hd3ss3220) {
+ unsigned int reg_val;
+ enum usb_role attached_state;
+ int ret;
+
+ ret = regmap_read(hd3ss3220->regmap,
HD3SS3220_REG_CN_STAT_CTRL,
+ &reg_val);
+ if (ret < 0)
+ return ret;
This function claims to return "enum usb_role", but here it returns errno
from regmap_read.
######### HERE

+static int hd3ss3220_dr_set(const struct typec_capability *cap,
+ enum typec_data_role role)
+{
...
+ ret = hd3ss3220_set_source_pref(hd3ss3220, pref);
+ usleep_range(10, 100);
Would udelay() make more sense here? Are your CPUs / timer subsystem so
fast that sleeping for 10usec is possible and reasonable to do?
I think Biju followed the overall indications from:
Documentation/timers-howto.txt (.rst)

According to the documentation the inflection point is at 10us, so this is kind
of a grey area. Just to put it in context, the RZ/G2E has 2x1.2Ghz ARM Cortex A53.
Therefore this is kind of a high-performance embedded platform. At least, we
like to advertise it like that 😊

So, I am inclined to keep this as it is if you agree.
It is not a big deal to keep it. What about the "returning -errno but
return type is enum" issue above? (Marked with "HERE").

Best regards,
Pavel

--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Join cip-dev@lists.cip-project.org to automatically receive all group messages.