Re: [PATCH 1/1] drivers/misc: Add Intel System ID driver

From: "Loh, Tien Hock" <tien.hock.loh@intel.com>
To: "arnd@arndb.de" <arnd@arndb.de>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Nguyen, Dinh" <dinh.nguyen@intel.com>,
	"thloh85@gmail.com" <thloh85@gmail.com>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"Gerlach, Matthew" <matthew.gerlach@intel.com>
Subject: Re: [PATCH 1/1] drivers/misc: Add Intel System ID driver
Date: Thu, 2 Mar 2017 02:17:36 +0000	[thread overview]
Message-ID: <1488362309.4010.23.camel@intel.com> (raw)
In-Reply-To: <CAK8P3a1Xiq2YrkxY9T-+-DsHzMSj5Dk7E-MQ4AF-7+zZBDrdpA@mail.gmail.com>

On Rab, 2017-03-01 at 12:34 +0100, Arnd Bergmann wrote:
> On Wed, Mar 1, 2017 at 11:42 AM, Loh, Tien Hock <tien.hock.loh@intel.
> com> wrote:
> > 
> > On Rab, 2017-03-01 at 10:01 +0100, Arnd Bergmann wrote:
> > > 
> > > On Wed, Mar 1, 2017 at 8:23 AM, Loh, Tien Hock <tien.hock.loh@int
> > > el.c
> > > Another option would be to fold the timestamp into the revision
> > > attribute,
> > > but whether that is a reasonable place for it would in turn
> > > depend on
> > > what the timestamp signifies.
> > > 
> > > Can you explain what the timestamp is used for? Does it identify
> > > the
> > > time that the hardware revision was made, the time that a
> > > software
> > > was built which was loaded into it, or something else?
> > > What kind of user space application would need this information?
> > I just checked, and it seems like we can't put this into soc
> > subsystem.
> > In FPGA, we now can do partial reconfiguration, which
> > "reconfigures"
> > the hardware to have an updated sysid and timestamp value, and the
> > base
> > address of the Intel System ID may also be changed. This would
> > require
> > the driver to be a module that will be removed, probed again. The
> > soc
> > subsystem doesn't seem to be a suitable place to add this driver.
> Ah, I had not realized this is for fpga_manager.
> 
> Why not put the attributes into /sys/class/fpga_manager/*/ then
> along with the other attributes that exist there? That way, we have
> an interface that works for all users of drivers/fpga/
> 

Well, this is not only for fpga_manager, but often time used to ensure
that the correct hardware is programmed into the FPGA by checking the
sysid after fpga_manager reconfigures the hardware. Systems without
fpga_manager can still use sysid and timestamp to ensure the hardware
is as expected value.

What do you think of /sys/class/fpga_sysid/*/id and
/sys/class/fpga_sysid/*/timestamp?

> > 
> > A note on the timestamp, in the old days this is used to check that
> > the
> > BSP is using the correct FPGA hardware. I believe in Linux we
> > should do
> > the same in the driver, and if it not, the driver should print a
> > warning. The timestamp's print is not exactly needed. I'll add the
> > feature into the driver in the next patch.
> Ok.
> 
>      Arnd