On Mon, Dec 30, 2019 at 05:26:00PM +0300, Heikki Krogerus wrote:
> For now the read-writable attribute files have made a check
> in their store callback function to see does the underlying
> port interface support changing the value or not, and when
> it didn't, the callbacks returned -EOPNOTSUPP. From user
> perspective that is not ideal, as there is no way to know is
> changing the value possible beforehand.
>
> Instead of returning -EOPNOTSUPP, making the attribute file
> read-only when the operation is not supported.
As fun as this is, if someone then changes the permission on the sysfs
file, trying to write to them will now crash the kernel, right?
So I think you need to leave this check in:
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 08923637cd88..3abcfa09ecdf 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -373,12 +373,9 @@ static ssize_t active_store(struct device *dev, struct device_attribute *attr,
> }
> }
>
> - /* Note: If there is no driver, the mode will not be entered */
> - if (adev->ops && adev->ops->activate) {
> - ret = adev->ops->activate(adev, enter);
> - if (ret)
> - return ret;
> - }
> + ret = adev->ops->activate(adev, enter);
> + if (ret)
> + return ret;
>
> return size;
> }
We had to go add this type of check to some subsystems recently that had
this same problem, I don't want to have to go and add it back for this
one as well :)
thanks,
greg k-h
