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

[Loh, Tien Hock]

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@xxxxxxx
> om> wrote:
> > 
> > Arnd, Greg,
> Please don't top-post.
OK.

> 
> > 
> > I checked the attributes returned by the soc attribute subsystem,
> > but
> > it seems that it is lacking something equivalent to timestamp in
> > the
> > Intel System ID controller. Do you think it is better to add a new
> > attribute (named timestamp) to soc or create a new sysfs entry like
> > what I did?
> It depends on how common and how important this attribute is.
> 
> - if it's not overly important, just drop it entirely.
> - if it's important enough that other SoCs are likely to have the
> same
> Â kind of information, make it a standard attribute
> - if this SoC is most likely the only one that will ever need it, but
> it has
> Â important uses, I'd make it a custom attribute
> 
> 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.

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.

> 
> ÂÂÂÂÂArnd

Thanks
Tien Hock