On Thu, Apr 30, 2020 at 02:09:45PM +0200, Oleksij Rempel wrote: > > > > > @@ -119,7 +123,12 @@ static int linkmodes_fill_reply(struct sk_buff > > > > > *skb, > > > > > } > > > > > > > > > > if (nla_put_u32(skb, ETHTOOL_A_LINKMODES_SPEED, > > > > > lsettings->speed) || > > > > > - nla_put_u8(skb, ETHTOOL_A_LINKMODES_DUPLEX, > > > > > lsettings->duplex)) > > > > > + nla_put_u8(skb, ETHTOOL_A_LINKMODES_DUPLEX, > > > > > lsettings->duplex) || > > > > > + nla_put_u8(skb, ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG, > > > > > + lsettings->master_slave_cfg) || > > > > > + nla_put_u8(skb, ETHTOOL_A_LINKMODES_MASTER_SLAVE_STATE, > > > > > + lsettings->master_slave_state)) > > > > > + > > > > > return -EMSGSIZE; > > > > > > > > From the two handlers you introduced, it seems we only get CFG_UNKNOWN > > > > or STATE_UNKNOWN if driver or device does not support the feature at all > > > > so it would be IMHO more appropriate to omit the attribute in such case. > > > > > > STATE_UNKNOWN is returned if link is not active. > > > > How about distinguishing the two cases then? Omitting both if CFG is > > CFG_UNKNOWN (i.e. driver does not support the feature) and sending > > STATE=STATE_UNKNOWN to userspace only if we know it's a meaningful value > > actually reported by the driver? > > Hm... after thinking about it, we should keep state and config > separately. The TJA1102 PHY do not provide actual state. Even no error > related to Master/Master conflict. I reworked the code to have > unsupported and unknown values. In case we know, that state or config > is not supported, it will not be exported to the user space.
Sounds good to me, splitting "unsupported" and "unknown" states will be probably the best option. Michal
