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