Re: [PATCH v3 25/25] bus: mhi: ep: Add uevent support for module autoloading

[Alex Elder <elder@linaro.org>]

On 2/12/22 12:21 PM, Manivannan Sadhasivam wrote:
> Add uevent support to MHI endpoint bus so that the client drivers can be
> autoloaded by udev when the MHI endpoint devices gets created. The client
> drivers are expected to provide MODULE_DEVICE_TABLE with the MHI id_table
> struct so that the alias can be exported.
> 
> The MHI endpoint reused the mhi_device_id structure of the MHI bus.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Looks OK to me.

Reviewed-by: Alex Elder <elder@linaro.org>


Next time I review this, I think I'll review the code
in its entirety (i.e., with the entire series applied
rather than in steps).  At that point I'm sure it will
be nearly perfect.

					-Alex

> ---
>   drivers/bus/mhi/ep/main.c       |  9 +++++++++
>   include/linux/mod_devicetable.h |  2 ++
>   scripts/mod/file2alias.c        | 10 ++++++++++
>   3 files changed, 21 insertions(+)
> 
> diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
> index 315409705b91..8889382ee8d0 100644
> --- a/drivers/bus/mhi/ep/main.c
> +++ b/drivers/bus/mhi/ep/main.c
> @@ -1546,6 +1546,14 @@ void mhi_ep_driver_unregister(struct mhi_ep_driver *mhi_drv)
>   }
>   EXPORT_SYMBOL_GPL(mhi_ep_driver_unregister);
>   
> +static int mhi_ep_uevent(struct device *dev, struct kobj_uevent_env *env)
> +{
> +	struct mhi_ep_device *mhi_dev = to_mhi_ep_device(dev);
> +
> +	return add_uevent_var(env, "MODALIAS=" MHI_EP_DEVICE_MODALIAS_FMT,
> +					mhi_dev->name);
> +}
> +
>   static int mhi_ep_match(struct device *dev, struct device_driver *drv)
>   {
>   	struct mhi_ep_device *mhi_dev = to_mhi_ep_device(dev);
> @@ -1572,6 +1580,7 @@ struct bus_type mhi_ep_bus_type = {
>   	.name = "mhi_ep",
>   	.dev_name = "mhi_ep",
>   	.match = mhi_ep_match,
> +	.uevent = mhi_ep_uevent,
>   };
>   
>   static int __init mhi_ep_init(void)
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index 4bb71979a8fd..0cff19bd72bf 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -835,6 +835,8 @@ struct wmi_device_id {
>   #define MHI_DEVICE_MODALIAS_FMT "mhi:%s"
>   #define MHI_NAME_SIZE 32
>   
> +#define MHI_EP_DEVICE_MODALIAS_FMT "mhi_ep:%s"
> +
>   /**
>    * struct mhi_device_id - MHI device identification
>    * @chan: MHI channel name
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> index 5258247d78ac..d9d6a31446ea 100644
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -1391,6 +1391,15 @@ static int do_mhi_entry(const char *filename, void *symval, char *alias)
>   	return 1;
>   }
>   
> +/* Looks like: mhi_ep:S */
> +static int do_mhi_ep_entry(const char *filename, void *symval, char *alias)
> +{
> +	DEF_FIELD_ADDR(symval, mhi_device_id, chan);
> +	sprintf(alias, MHI_EP_DEVICE_MODALIAS_FMT, *chan);
> +
> +	return 1;
> +}
> +
>   /* Looks like: ishtp:{guid} */
>   static int do_ishtp_entry(const char *filename, void *symval, char *alias)
>   {
> @@ -1519,6 +1528,7 @@ static const struct devtable devtable[] = {
>   	{"tee", SIZE_tee_client_device_id, do_tee_entry},
>   	{"wmi", SIZE_wmi_device_id, do_wmi_entry},
>   	{"mhi", SIZE_mhi_device_id, do_mhi_entry},
> +	{"mhi_ep", SIZE_mhi_device_id, do_mhi_ep_entry},
>   	{"auxiliary", SIZE_auxiliary_device_id, do_auxiliary_entry},
>   	{"ssam", SIZE_ssam_device_id, do_ssam_entry},
>   	{"dfl", SIZE_dfl_device_id, do_dfl_entry},