Quoting Andy Shevchenko (2024-09-04 06:00:57) > On Tue, Sep 03, 2024 at 06:20:14PM -0400, Stephen Boyd wrote: > > Quoting Andy Shevchenko (2024-09-02 04:35:46) > > > On Sat, Aug 31, 2024 at 09:06:44PM -0700, Stephen Boyd wrote: > > > > > + adev->dev.of_node = of_node_get(parent->of_node); > > > > > > device_set_node() ? > > > > Or device_set_of_node_from_dev()? > > This is quite unclear to me. The second one bumps the reference count IIRC > for no reason (in usual cases). Also only few drivers use that, I would hear > what OF people can tell about this API and its usage scope. Sure, but to be equivalent to the existing code from which this has been copied it should use device_set_of_node_from_dev(). Seems fine to just use that. > > > > +static int dp_lane_to_typec_lane(enum dp_lane lane) > > > > +{ > > > > + switch (lane) { > > > > + case DP_ML0: > > > > + return USB_SSTX2; > > > > + case DP_ML1: > > > > + return USB_SSRX2; > > > > + case DP_ML2: > > > > + return USB_SSTX1; > > > > + case DP_ML3: > > > > + return USB_SSRX1; > > > > + } > > > > > > > + return -EINVAL; > > > > > > Hmm... This can be simply made as default case. > > > > And then the enum is always "covered" and the compiler doesn't complain > > about missing cases (I don't think we have -Wswitch-enum)? Seems worse. > > Hmm... You mean if I remove one of the above cases I will get the warning? > Yes.