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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 23 Dec 2011 21:43:54 +0100
From:	Francois Romieu <romieu@...zoreil.com>
To:	"Skidmore, Donald C" <donald.c.skidmore@...el.com>
Cc:	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"gospo@...hat.com" <gospo@...hat.com>,
	"sassmann@...hat.com" <sassmann@...hat.com>,
	"Waskiewicz Jr, Peter P" <peter.p.waskiewicz.jr@...el.com>
Subject: Re: [net-next 7/9] ixgbe: add support functions for gathering thermal data sensor

Skidmore, Donald C <donald.c.skidmore@...el.com> :
[...]
> >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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html