[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20180318224736.GK5626@tuxbook-pro>
Date: Sun, 18 Mar 2018 15:47:36 -0700
From: Bjorn Andersson <bjorn.andersson@...aro.org>
To: Anup Patel <anup@...infault.org>
Cc: Ohad Ben-Cohen <ohad@...ery.com>, linux-remoteproc@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH RESEND] rpmsg: Add driver_override device attribute for
rpmsg_device
On Wed 10 Jan 05:17 PST 2018, Anup Patel wrote:
> This patch adds "driver_override" device attribute for rpmsg_device which
> will allow users to explicitly specify the rpmsg_driver to be used via
> sysfs entry.
>
> The "driver_override" device attribute implemented here is very similar
> to "driver_override" implemented for platform, pci, and amba bus types.
>
> One important use-case of "driver_override" device attribute is to force
> use of rpmsg_chrdev driver for certain rpmsg_device instances.
>
I assume you mean specifically for the case where you want to prevent
some kernel driver to probe for some given channel?
The intention with rpmsg_char is that you through the rpmsg_ctrlX
interface create and destroy endpoints dynamically, so you wouldn't need
to use this mechanism to bind some specific channel to rpmsg_char.
That said, this does make sense for completeness sake.
[..]
> diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
> index dffa3aa..9a25e42 100644
> --- a/drivers/rpmsg/rpmsg_core.c
> +++ b/drivers/rpmsg/rpmsg_core.c
> @@ -321,11 +321,11 @@ struct device *rpmsg_find_device(struct device *parent,
> }
> EXPORT_SYMBOL(rpmsg_find_device);
>
> -/* sysfs show configuration fields */
> +/* sysfs configuration fields */
> #define rpmsg_show_attr(field, path, format_string) \
> static ssize_t \
> field##_show(struct device *dev, \
> - struct device_attribute *attr, char *buf) \
> + struct device_attribute *attr, char *buf) \
Seems unnecessary.
> { \
> struct rpmsg_device *rpdev = to_rpmsg_device(dev); \
> \
> @@ -333,11 +333,52 @@ field##_show(struct device *dev, \
> } \
> static DEVICE_ATTR_RO(field);
>
> +#define rpmsg_string_attr(field, path) \
"path" is an odd name for these, I think it's a "member".
> +static ssize_t \
> +field##_store(struct device *dev, \
> + struct device_attribute *attr, const char *buf, size_t sz)\
field##_store(struct device *dev, struct device_attribute *attr, \
const char *buf, size_t sz) \
Is prettier
> +{ \
> + struct rpmsg_device *rpdev = to_rpmsg_device(dev); \
> + char *new, *old, *cp; \
> + \
> + new = kstrndup(buf, sz, GFP_KERNEL); \
> + if (!new) \
> + return -ENOMEM; \
> + \
> + cp = strchr(new, '\n'); \
> + if (cp) \
> + *cp = '\0'; \
I prefer
new[strcspn(new, "\n")] = '\0';
Regards,
Bjorn
Powered by blists - more mailing lists