[PATCH linux dev-4.7] drivers: fsi: scom: Add reset functionality and sysfs interface

Jeremy Kerr jk at ozlabs.org
Tue Feb 21 14:40:02 AEDT 2017


Hi Eddie,

>>> +static ssize_t store_reset_register(struct device *dev,
>>> +                    struct device_attribute *attr,
>>> +                    const char *buf, size_t count)
>>> +{
>>> +    int rc;
>>> +    struct fsi_device *fsi_dev = to_fsi_dev(dev);
>>> +
>>> +    rc = scom_full_reset(fsi_dev);
>>> +
>>> +    return rc ? rc : count;
>>> +}
>>> +DEVICE_ATTR(reset, 0200, NULL, store_reset_register);
>>> +
>> How does this interact with the scom read/write operations? Having
>> different interfaces access methods (file for one, sysfs interface for
>> another)for both seems like an invitation for trouble - would this make
>> more sense as an ioctl on the existing fd?
> 
> It would, but I was under the impression ioctls were frowned upon for
> some reason?

The downside of an ioctl is that it needs a new definition in kernel
headers (ie, the ioctl number and any structures required for
input/output data).

However, I think it's entirely suitable in this case - we have the usual
flow of data (the scoms) over read()/write(), and control operations
(reset) using an ioctl.

However, that assumes that the reset is something that a user may want
to issue in normal operation, rather than a debug or diagnosis event. Is
that the case here?

Cheers,


Jeremy


More information about the openbmc mailing list