Date:	Mon, 9 Jan 2012 18:49:43 +0000
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	"Waskiewicz Jr, Peter P" <peter.p.waskiewicz.jr@...el.com>
CC:	Michal Miroslaw <mirqus@...il.com>,
	"Skidmore, Donald C" <donald.c.skidmore@...el.com>,
	"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>
Subject: RE: [net-next 8/9] ixgbe: add interface to export thermal data

On Mon, 2012-01-09 at 18:08 +0000, Waskiewicz Jr, Peter P wrote:
> > -----Original Message-----
> > From: netdev-owner@...r.kernel.org [mailto:netdev-
> > owner@...r.kernel.org] On Behalf Of Ben Hutchings
> > Sent: Friday, January 06, 2012 10:25 AM
> > To: Waskiewicz Jr, Peter P
> > Cc: Michal Miroslaw; Skidmore, Donald C; Kirsher, Jeffrey T;
> > davem@...emloft.net; netdev@...r.kernel.org; gospo@...hat.com;
> > sassmann@...hat.com
> > Subject: RE: [net-next 8/9] ixgbe: add interface to export thermal data
> > 
> > On Fri, 2012-01-06 at 18:20 +0000, Waskiewicz Jr, Peter P wrote:
> > > > -----Original Message-----
> > > > From: Ben Hutchings [mailto:bhutchings@...arflare.com]
> > > > Sent: Friday, January 06, 2012 10:09 AM
> > > > To: Waskiewicz Jr, Peter P
> > > > Cc: Michal Miroslaw; Skidmore, Donald C; Kirsher, Jeffrey T;
> > > > davem@...emloft.net; netdev@...r.kernel.org; gospo@...hat.com;
> > > > sassmann@...hat.com
> > > > Subject: RE: [net-next 8/9] ixgbe: add interface to export thermal
> > > > data
> > > >
> > > > On Fri, 2012-01-06 at 17:55 +0000, Waskiewicz Jr, Peter P wrote:
> > > > > > -----Original Message-----
> > > > > > From: Michał Mirosław [mailto:mirqus@...il.com]
> > > > > > Sent: Thursday, January 05, 2012 3:51 PM
> > > > > > To: Skidmore, Donald C
> > > > > > Cc: Ben Hutchings; Kirsher, Jeffrey T; davem@...emloft.net;
> > > > > > netdev@...r.kernel.org; gospo@...hat.com;
> > sassmann@...hat.com;
> > > > > > Waskiewicz Jr, Peter P
> > > > > > Subject: Re: [net-next 8/9] ixgbe: add interface to export
> > > > > > thermal data
> > > > > >
> > > > > > Drivers outside of drivers/hwmon just select HWMON in Kconfig.
> > > > > > This adds a 3kB .c file to the kernel build for the first one that needs it.
> > > > >
> > > > > This is where we run into issues though.  We can put this
> > > > > dependency in, but customers don't pull upstream kernels, they
> > > > > rely on the OSV's to distribute updates.  The customer doesn't
> > > > > want HWMON, and if their kernel doesn't ship with HWMON support
> > for ixgbe, then we're sunk.
> > > >
> > > > So if !IS_ENABLED(CONFIG_HWMON), don't try to create an hwmon
> > > > device, but create the attributes anyway.
> > >
> > > And if CONFIG_HWMON is not enabled, are we then responsible for
> > > creating the hwmon area in sysfs to link in our attributes?  That
> > > would make even less sense to include HWMON attributes if HWMON isn't
> > > built in.
> > 
> > No, the attributes actually appear under the parent device.
> 
> Ok, so how we have it now in the current patches.

Only with different names.

> > > > > The point is what we're trying to implement, based on what our
> > customers'
> > > > > requirements are, is something completely different than what
> > > > > HWMON is providing.  Trying to wedge HWMON onto this framework
> > > > > we're trying to provide just overcomplicates the entire thing we're trying
> > to provide.
> > > > > It's a generic interface to generic data in our drivers,
> > > > [...]
> > > >
> > > > It sounds like you want to provide a "generic" ixgbe interface on
> > > > different operating systems.  But that is not a valid argument for an in-
> > tree driver.
> > >
> > > No, we're trying to provide a generic ixgbe interface on Linux
> > > systems.  The reality is different OSV's pull our driver from upstream
> > > to build their distros,
> > 
> > Yes we know.
> > 
> > [...]
> > > The reality is these attributes of our thermal sensors are about 5% of
> > > the data we need to export.
> > [...]
>
> > And I think all you need to do is (1) give these particular attributes the right
> > names and (2) create a child hwmon device if possible.  Is that so hard?
> 
> Yes, it is.  The sensors that we have defined for this SKU are defined
> by the customers who this board was built for, so it fits with their
> BMC.  We don't have much control over the names.

You should expose the names as '<type><index>_label' attributes.

> Again, I'm failing to see why you are requiring us to put in this
> support when we don't benefit at all from it, we don't need it, and
> don't want it.  Perhaps you can add the HWMON support to ixgbe when
> you get around to adding the same support to your Solarflare driver
> that hasn't been completed yet?

I already did that for the SFC4000 boards, using existing hwmon drivers.
I'm attaching the implementation for SFC9000-family boards (though it's
not immediately applicable as it needs an update to the firmware
protocol header).  Oddly enough I'm not inclined to spend time on your
boards!  But maybe this can serve as an example of how to expose sensors
that are discoverd at run-time.

> We'd be happy to accept those patches.  Please don't hold us to a
> standard you won't hold your own driver to.

I'm not the one proposing to add a driver-specific interface for
something that's already standardised!

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

View attachment "0001-sfc-Add-hardware-monitor-for-sensors-managed-by-firm.patch" of type "text/x-patch" (16611 bytes)

Powered by blists - more mailing lists