Arnd, Greg,
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?
Thanks
Tien Hock
On Kha, 2017-02-23 at 16:15 +0800, Loh, Tien Hock wrote:
> OK then I'll move it into driver/soc/ in that case.
>
> On Kha, 2017-02-23 at 09:05 +0100, Arnd Bergmann wrote:
> >
> > On Thu, Feb 23, 2017 at 6:58 AM, Loh, Tien Hock <tien.hock.loh@inte
> > l.
> > com> wrote:
> > >
> > >
> > > On Rab, 2017-02-15 at 20:51 +0100, Arnd Bergmann wrote:
> > > >
> > > >
> > > > On Wed, Feb 15, 2017 at 6:17 PM, Greg KH <gregkh@linuxfoundatio
> > > > n.
> > > > org>
> > > > wrote:
> > > > >
> > > > >
> > > > >
> > > > > On Wed, Feb 15, 2017 at 07:09:41PM +0800, thloh wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > From: "Loh, Tien Hock" <[email protected]>
> > > > > >
> > > > > > This patch is to add Altera System ID driver.
> > > > > > User can obtain the system ID and timestamp of the system
> > > > > > by
> > > > > > reading the sysfs entry.
> > > > > >
> > > > > > Usage:
> > > > > > cat /sys/bus/platform/devices/[addr].sysid/sysid/id
> > > > > > cat /sys/bus/platform/devices/[addr].sysid/sysid/timestamp
> > > > > If you add new sysfs attributes, you need to also add a
> > > > > Documentation/ABI/ description as well.
> > > > Maybe we could pretend that this is for a SoC and use the
> > > > standard
> > > > soc_device
> > > > attributes as well as moving the driver into drivers/soc/?>
> > > > Sorry
> > > > for the late reply.
> > > This driver can currently be used by ARM and Nios II, so moving
> > > it
> > > into
> > > drivers/soc might not be the best idea.
> > Why not? drivers/soc/ was specifically introduced for stuff that is
> > used on
> > some SoC but across more than one architecture (otherwise it would
> > be
> > in arch/foo/). This seems to fit perfectly.
> >
> > Arnd
On Wed, Mar 1, 2017 at 8:23 AM, Loh, Tien Hock <[email protected]> wrote:
> Arnd, Greg,
Please don't top-post.
> 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?
Arnd
On Rab, 2017-03-01 at 10:01 +0100, Arnd Bergmann wrote:
> On Wed, Mar 1, 2017 at 8:23 AM, Loh, Tien Hock <[email protected]
> 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
On Wed, Mar 1, 2017 at 11:42 AM, Loh, Tien Hock <[email protected]> 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 <[email protected]
>> 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/
> 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
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