Re: [net-next 7/9] ixgbe: add support functions for gathering thermal data sensor

Skidmore, Donald C [off-list ref] :
[...]

quoted

While defined, IXGBE_NOT_IMPLEMENTED is currently unused in both davem-
next
and the remaining patches of this series. Can't you remove it completely
and
use a standard error code, say -EOPNOTSUPP ?

Previously we have only been using EOPNOTSUPP and it's like as a return
value seen outside the driver particularly to user space.  Looking through
a few other device drivers (in no way an extensive search) they 'seem' to
be following that model too.  Still that said I'm not aware of this being
a BKM it's just what I noticed. 

Your call. As long as the code does not end cluttered with:

	rc = (status != MY_OWN_PRIVATE_ERROR_STATUS) ? 0 : -Esomething;

[...]

if (((ets_cfg & IXGBE_ETS_TYPE_MASK) >> IXGBE_ETS_TYPE_SHIFT) != IXGBE_ETS_TYPE_EMC) {

Or are you talking about just moving the "!=" to the previous line?

Just the "!=".

You may include the shift in the definition of IXGBE_ETS_TYPE_EMC itself
so that it disappears here.

[...]

While I like this idea and it would remove some duplicate code, I need
to use ets_cfg in ixgbe_init_thermal_sensor_thresh_generic() to get the
low_thresh_delta.

You are right, I missed it.

This makes the support function a little less useful as it would either
need to return two values (num_sensors and low_thresh_delta) or just the
ets_cfs itself.   For the latter case the support function would just be
making two reads and we would still need the local variables. Still might
be worth it though.

Something like a stack allocated struct ets { u16 cfg; u16 sensor; } may
help.

-- 
Ueimor