[RFC PATCH] nvme-core: add helper to get nvme_ctrl from device

[RFC PATCH] nvme-core: add helper to get nvme_ctrl from device

[Chaitanya Kulkarni]

Just like we have other helpers to_nvmet_port(), to_nvmet_ns(),
ana_groups_to_port() and to_nvmet_ns(), add a helper to get nvme_ctr
from struct device.

Note that this removes the bunch of nvme_ctrl declaration code that is
needed to get the nvme_ctrl from struct device in the host/core.c.
Followwing is the diff :-

drivers/nvme/host/core.c | 69 +++++++++++++++-------------------------

Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
 drivers/nvme/host/core.c | 69 +++++++++++++++-------------------------
 1 file changed, 26 insertions(+), 43 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 518c759346f0..a957da110ee1 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -126,6 +126,11 @@ static void nvme_remove_invalid_namespaces(struct nvme_ctrl *ctrl,
 static void nvme_update_keep_alive(struct nvme_ctrl *ctrl,
 				   struct nvme_command *cmd);
 
+static struct nvme_ctrl *dev_to_nvme_ctrl(struct device *dev)
+{
+	return dev_get_drvdata(dev);
+}
+
 void nvme_queue_scan(struct nvme_ctrl *ctrl)
 {
 	/*
@@ -3386,10 +3391,9 @@ static ssize_t nvme_sysfs_reset(struct device *dev,
 				struct device_attribute *attr, const char *buf,
 				size_t count)
 {
-	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
 	int ret;
 
-	ret = nvme_reset_ctrl_sync(ctrl);
+	ret = nvme_reset_ctrl_sync(dev_to_nvme_ctrl(dev));
 	if (ret < 0)
 		return ret;
 	return count;
@@ -3400,9 +3404,7 @@ static ssize_t nvme_sysfs_rescan(struct device *dev,
 				struct device_attribute *attr, const char *buf,
 				size_t count)
 {
-	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
-
-	nvme_queue_scan(ctrl);
+	nvme_queue_scan(dev_to_nvme_ctrl(dev));
 	return count;
 }
 static DEVICE_ATTR(rescan_controller, S_IWUSR, NULL, nvme_sysfs_rescan);
@@ -3557,8 +3559,7 @@ nvme_show_str_function(firmware_rev);
 static ssize_t  field##_show(struct device *dev,				\
 			    struct device_attribute *attr, char *buf)		\
 {										\
-        struct nvme_ctrl *ctrl = dev_get_drvdata(dev);				\
-        return sysfs_emit(buf, "%d\n", ctrl->field);				\
+        return sysfs_emit(buf, "%d\n", dev_to_nvme_ctrl(dev)->field);		\
 }										\
 static DEVICE_ATTR(field, S_IRUGO, field##_show, NULL);
 
@@ -3572,10 +3573,8 @@ static ssize_t nvme_sysfs_delete(struct device *dev,
 				struct device_attribute *attr, const char *buf,
 				size_t count)
 {
-	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
-
 	if (device_remove_file_self(dev, attr))
-		nvme_delete_ctrl_sync(ctrl);
+		nvme_delete_ctrl_sync(dev_to_nvme_ctrl(dev));
 	return count;
 }
 static DEVICE_ATTR(delete_controller, S_IWUSR, NULL, nvme_sysfs_delete);
@@ -3584,9 +3583,7 @@ static ssize_t nvme_sysfs_show_transport(struct device *dev,
 					 struct device_attribute *attr,
 					 char *buf)
 {
-	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
-
-	return sysfs_emit(buf, "%s\n", ctrl->ops->name);
+	return sysfs_emit(buf, "%s\n", dev_to_nvme_ctrl(dev)->ops->name);
 }
 static DEVICE_ATTR(transport, S_IRUGO, nvme_sysfs_show_transport, NULL);
 
@@ -3618,9 +3615,7 @@ static ssize_t nvme_sysfs_show_subsysnqn(struct device *dev,
 					 struct device_attribute *attr,
 					 char *buf)
 {
-	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
-
-	return sysfs_emit(buf, "%s\n", ctrl->subsys->subnqn);
+	return sysfs_emit(buf, "%s\n", dev_to_nvme_ctrl(dev)->subsys->subnqn);
 }
 static DEVICE_ATTR(subsysnqn, S_IRUGO, nvme_sysfs_show_subsysnqn, NULL);
 
@@ -3628,9 +3623,7 @@ static ssize_t nvme_sysfs_show_hostnqn(struct device *dev,
 					struct device_attribute *attr,
 					char *buf)
 {
-	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
-
-	return sysfs_emit(buf, "%s\n", ctrl->opts->host->nqn);
+	return sysfs_emit(buf, "%s\n", dev_to_nvme_ctrl(dev)->opts->host->nqn);
 }
 static DEVICE_ATTR(hostnqn, S_IRUGO, nvme_sysfs_show_hostnqn, NULL);
 
@@ -3638,9 +3631,7 @@ static ssize_t nvme_sysfs_show_hostid(struct device *dev,
 					struct device_attribute *attr,
 					char *buf)
 {
-	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
-
-	return sysfs_emit(buf, "%pU\n", &ctrl->opts->host->id);
+	return sysfs_emit(buf, "%pU\n", &dev_to_nvme_ctrl(dev)->opts->host->id);
 }
 static DEVICE_ATTR(hostid, S_IRUGO, nvme_sysfs_show_hostid, NULL);
 
@@ -3657,10 +3648,9 @@ static DEVICE_ATTR(address, S_IRUGO, nvme_sysfs_show_address, NULL);
 static ssize_t nvme_ctrl_loss_tmo_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
-	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
-	struct nvmf_ctrl_options *opts = ctrl->opts;
+	struct nvmf_ctrl_options *opts = dev_to_nvme_ctrl(dev)->opts;
 
-	if (ctrl->opts->max_reconnects == -1)
+	if (dev_to_nvme_ctrl(dev)->opts->max_reconnects == -1)
 		return sysfs_emit(buf, "off\n");
 	return sysfs_emit(buf, "%d\n",
 			  opts->max_reconnects * opts->reconnect_delay);
@@ -3669,8 +3659,7 @@ static ssize_t nvme_ctrl_loss_tmo_show(struct device *dev,
 static ssize_t nvme_ctrl_loss_tmo_store(struct device *dev,
 		struct device_attribute *attr, const char *buf, size_t count)
 {
-	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
-	struct nvmf_ctrl_options *opts = ctrl->opts;
+	struct nvmf_ctrl_options *opts = dev_to_nvme_ctrl(dev)->opts;
 	int ctrl_loss_tmo, err;
 
 	err = kstrtoint(buf, 10, &ctrl_loss_tmo);
@@ -3690,17 +3679,15 @@ static DEVICE_ATTR(ctrl_loss_tmo, S_IRUGO | S_IWUSR,
 static ssize_t nvme_ctrl_reconnect_delay_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
-	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
-
-	if (ctrl->opts->reconnect_delay == -1)
+	if (dev_to_nvme_ctrl(dev)->opts->reconnect_delay == -1)
 		return sysfs_emit(buf, "off\n");
-	return sysfs_emit(buf, "%d\n", ctrl->opts->reconnect_delay);
+	return sysfs_emit(buf, "%d\n",
+			  dev_to_nvme_ctrl(dev)->opts->reconnect_delay);
 }
 
 static ssize_t nvme_ctrl_reconnect_delay_store(struct device *dev,
 		struct device_attribute *attr, const char *buf, size_t count)
 {
-	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
 	unsigned int v;
 	int err;
 
@@ -3708,7 +3695,7 @@ static ssize_t nvme_ctrl_reconnect_delay_store(struct device *dev,
 	if (err)
 		return err;
 
-	ctrl->opts->reconnect_delay = v;
+	dev_to_nvme_ctrl(dev)->opts->reconnect_delay = v;
 	return count;
 }
 static DEVICE_ATTR(reconnect_delay, S_IRUGO | S_IWUSR,
@@ -3717,18 +3704,16 @@ static DEVICE_ATTR(reconnect_delay, S_IRUGO | S_IWUSR,
 static ssize_t nvme_ctrl_fast_io_fail_tmo_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
-	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
-
-	if (ctrl->opts->fast_io_fail_tmo == -1)
+	if (dev_to_nvme_ctrl(dev)->opts->fast_io_fail_tmo == -1)
 		return sysfs_emit(buf, "off\n");
-	return sysfs_emit(buf, "%d\n", ctrl->opts->fast_io_fail_tmo);
+	return sysfs_emit(buf, "%d\n",
+			  dev_to_nvme_ctrl(dev)->opts->fast_io_fail_tmo);
 }
 
 static ssize_t nvme_ctrl_fast_io_fail_tmo_store(struct device *dev,
 		struct device_attribute *attr, const char *buf, size_t count)
 {
-	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
-	struct nvmf_ctrl_options *opts = ctrl->opts;
+	struct nvmf_ctrl_options *opts = dev_to_nvme_ctrl(dev)->opts;
 	int fast_io_fail_tmo, err;
 
 	err = kstrtoint(buf, 10, &fast_io_fail_tmo);
@@ -3782,8 +3767,7 @@ static DEVICE_ATTR_RO(dctype);
 static ssize_t nvme_ctrl_dhchap_secret_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
-	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
-	struct nvmf_ctrl_options *opts = ctrl->opts;
+	struct nvmf_ctrl_options *opts = dev_to_nvme_ctrl(dev)->opts;
 
 	if (!opts->dhchap_secret)
 		return sysfs_emit(buf, "none\n");
@@ -3836,8 +3820,7 @@ static DEVICE_ATTR(dhchap_secret, S_IRUGO | S_IWUSR,
 static ssize_t nvme_ctrl_dhchap_ctrl_secret_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
-	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
-	struct nvmf_ctrl_options *opts = ctrl->opts;
+	struct nvmf_ctrl_options *opts = dev_to_nvme_ctrl(dev)->opts;
 
 	if (!opts->dhchap_ctrl_secret)
 		return sysfs_emit(buf, "none\n");
-- 
2.40.0

Re: [RFC PATCH] nvme-core: add helper to get nvme_ctrl from device

[Sagi Grimberg]

> Just like we have other helpers to_nvmet_port(), to_nvmet_ns(),
> ana_groups_to_port() and to_nvmet_ns(), add a helper to get nvme_ctr
> from struct device.

I don't think this is very useful.

> Note that this removes the bunch of nvme_ctrl declaration code that is
> needed to get the nvme_ctrl from struct device in the host/core.c.
> Followwing is the diff :-

It doesn't make it easier on my eyes...

> 
> drivers/nvme/host/core.c | 69 +++++++++++++++-------------------------
> 
> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
> ---
>   drivers/nvme/host/core.c | 69 +++++++++++++++-------------------------
>   1 file changed, 26 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 518c759346f0..a957da110ee1 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -126,6 +126,11 @@ static void nvme_remove_invalid_namespaces(struct nvme_ctrl *ctrl,
>   static void nvme_update_keep_alive(struct nvme_ctrl *ctrl,
>   				   struct nvme_command *cmd);
>   
> +static struct nvme_ctrl *dev_to_nvme_ctrl(struct device *dev)
> +{
> +	return dev_get_drvdata(dev);
> +}
> +
>   void nvme_queue_scan(struct nvme_ctrl *ctrl)
>   {
>   	/*
> @@ -3386,10 +3391,9 @@ static ssize_t nvme_sysfs_reset(struct device *dev,
>   				struct device_attribute *attr, const char *buf,
>   				size_t count)
>   {
> -	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
>   	int ret;
>   
> -	ret = nvme_reset_ctrl_sync(ctrl);
> +	ret = nvme_reset_ctrl_sync(dev_to_nvme_ctrl(dev));
>   	if (ret < 0)
>   		return ret;
>   	return count;
> @@ -3400,9 +3404,7 @@ static ssize_t nvme_sysfs_rescan(struct device *dev,
>   				struct device_attribute *attr, const char *buf,
>   				size_t count)
>   {
> -	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> -
> -	nvme_queue_scan(ctrl);
> +	nvme_queue_scan(dev_to_nvme_ctrl(dev));
>   	return count;
>   }
>   static DEVICE_ATTR(rescan_controller, S_IWUSR, NULL, nvme_sysfs_rescan);
> @@ -3557,8 +3559,7 @@ nvme_show_str_function(firmware_rev);
>   static ssize_t  field##_show(struct device *dev,				\
>   			    struct device_attribute *attr, char *buf)		\
>   {										\
> -        struct nvme_ctrl *ctrl = dev_get_drvdata(dev);				\
> -        return sysfs_emit(buf, "%d\n", ctrl->field);				\
> +        return sysfs_emit(buf, "%d\n", dev_to_nvme_ctrl(dev)->field);		\
>   }										\
>   static DEVICE_ATTR(field, S_IRUGO, field##_show, NULL);
>   
> @@ -3572,10 +3573,8 @@ static ssize_t nvme_sysfs_delete(struct device *dev,
>   				struct device_attribute *attr, const char *buf,
>   				size_t count)
>   {
> -	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> -
>   	if (device_remove_file_self(dev, attr))
> -		nvme_delete_ctrl_sync(ctrl);
> +		nvme_delete_ctrl_sync(dev_to_nvme_ctrl(dev));
>   	return count;
>   }
>   static DEVICE_ATTR(delete_controller, S_IWUSR, NULL, nvme_sysfs_delete);
> @@ -3584,9 +3583,7 @@ static ssize_t nvme_sysfs_show_transport(struct device *dev,
>   					 struct device_attribute *attr,
>   					 char *buf)
>   {
> -	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> -
> -	return sysfs_emit(buf, "%s\n", ctrl->ops->name);
> +	return sysfs_emit(buf, "%s\n", dev_to_nvme_ctrl(dev)->ops->name);
>   }
>   static DEVICE_ATTR(transport, S_IRUGO, nvme_sysfs_show_transport, NULL);
>   
> @@ -3618,9 +3615,7 @@ static ssize_t nvme_sysfs_show_subsysnqn(struct device *dev,
>   					 struct device_attribute *attr,
>   					 char *buf)
>   {
> -	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> -
> -	return sysfs_emit(buf, "%s\n", ctrl->subsys->subnqn);
> +	return sysfs_emit(buf, "%s\n", dev_to_nvme_ctrl(dev)->subsys->subnqn);
>   }
>   static DEVICE_ATTR(subsysnqn, S_IRUGO, nvme_sysfs_show_subsysnqn, NULL);
>   
> @@ -3628,9 +3623,7 @@ static ssize_t nvme_sysfs_show_hostnqn(struct device *dev,
>   					struct device_attribute *attr,
>   					char *buf)
>   {
> -	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> -
> -	return sysfs_emit(buf, "%s\n", ctrl->opts->host->nqn);
> +	return sysfs_emit(buf, "%s\n", dev_to_nvme_ctrl(dev)->opts->host->nqn);
>   }
>   static DEVICE_ATTR(hostnqn, S_IRUGO, nvme_sysfs_show_hostnqn, NULL);
>   
> @@ -3638,9 +3631,7 @@ static ssize_t nvme_sysfs_show_hostid(struct device *dev,
>   					struct device_attribute *attr,
>   					char *buf)
>   {
> -	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> -
> -	return sysfs_emit(buf, "%pU\n", &ctrl->opts->host->id);
> +	return sysfs_emit(buf, "%pU\n", &dev_to_nvme_ctrl(dev)->opts->host->id);
>   }
>   static DEVICE_ATTR(hostid, S_IRUGO, nvme_sysfs_show_hostid, NULL);
>   
> @@ -3657,10 +3648,9 @@ static DEVICE_ATTR(address, S_IRUGO, nvme_sysfs_show_address, NULL);
>   static ssize_t nvme_ctrl_loss_tmo_show(struct device *dev,
>   		struct device_attribute *attr, char *buf)
>   {
> -	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> -	struct nvmf_ctrl_options *opts = ctrl->opts;
> +	struct nvmf_ctrl_options *opts = dev_to_nvme_ctrl(dev)->opts;
>   
> -	if (ctrl->opts->max_reconnects == -1)
> +	if (dev_to_nvme_ctrl(dev)->opts->max_reconnects == -1)
>   		return sysfs_emit(buf, "off\n");
>   	return sysfs_emit(buf, "%d\n",
>   			  opts->max_reconnects * opts->reconnect_delay);
> @@ -3669,8 +3659,7 @@ static ssize_t nvme_ctrl_loss_tmo_show(struct device *dev,
>   static ssize_t nvme_ctrl_loss_tmo_store(struct device *dev,
>   		struct device_attribute *attr, const char *buf, size_t count)
>   {
> -	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> -	struct nvmf_ctrl_options *opts = ctrl->opts;
> +	struct nvmf_ctrl_options *opts = dev_to_nvme_ctrl(dev)->opts;
>   	int ctrl_loss_tmo, err;
>   
>   	err = kstrtoint(buf, 10, &ctrl_loss_tmo);
> @@ -3690,17 +3679,15 @@ static DEVICE_ATTR(ctrl_loss_tmo, S_IRUGO | S_IWUSR,
>   static ssize_t nvme_ctrl_reconnect_delay_show(struct device *dev,
>   		struct device_attribute *attr, char *buf)
>   {
> -	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> -
> -	if (ctrl->opts->reconnect_delay == -1)
> +	if (dev_to_nvme_ctrl(dev)->opts->reconnect_delay == -1)
>   		return sysfs_emit(buf, "off\n");
> -	return sysfs_emit(buf, "%d\n", ctrl->opts->reconnect_delay);
> +	return sysfs_emit(buf, "%d\n",
> +			  dev_to_nvme_ctrl(dev)->opts->reconnect_delay);
>   }
>   
>   static ssize_t nvme_ctrl_reconnect_delay_store(struct device *dev,
>   		struct device_attribute *attr, const char *buf, size_t count)
>   {
> -	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
>   	unsigned int v;
>   	int err;
>   
> @@ -3708,7 +3695,7 @@ static ssize_t nvme_ctrl_reconnect_delay_store(struct device *dev,
>   	if (err)
>   		return err;
>   
> -	ctrl->opts->reconnect_delay = v;
> +	dev_to_nvme_ctrl(dev)->opts->reconnect_delay = v;
>   	return count;
>   }
>   static DEVICE_ATTR(reconnect_delay, S_IRUGO | S_IWUSR,
> @@ -3717,18 +3704,16 @@ static DEVICE_ATTR(reconnect_delay, S_IRUGO | S_IWUSR,
>   static ssize_t nvme_ctrl_fast_io_fail_tmo_show(struct device *dev,
>   		struct device_attribute *attr, char *buf)
>   {
> -	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> -
> -	if (ctrl->opts->fast_io_fail_tmo == -1)
> +	if (dev_to_nvme_ctrl(dev)->opts->fast_io_fail_tmo == -1)
>   		return sysfs_emit(buf, "off\n");
> -	return sysfs_emit(buf, "%d\n", ctrl->opts->fast_io_fail_tmo);
> +	return sysfs_emit(buf, "%d\n",
> +			  dev_to_nvme_ctrl(dev)->opts->fast_io_fail_tmo);
>   }
>   
>   static ssize_t nvme_ctrl_fast_io_fail_tmo_store(struct device *dev,
>   		struct device_attribute *attr, const char *buf, size_t count)
>   {
> -	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> -	struct nvmf_ctrl_options *opts = ctrl->opts;
> +	struct nvmf_ctrl_options *opts = dev_to_nvme_ctrl(dev)->opts;
>   	int fast_io_fail_tmo, err;
>   
>   	err = kstrtoint(buf, 10, &fast_io_fail_tmo);
> @@ -3782,8 +3767,7 @@ static DEVICE_ATTR_RO(dctype);
>   static ssize_t nvme_ctrl_dhchap_secret_show(struct device *dev,
>   		struct device_attribute *attr, char *buf)
>   {
> -	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> -	struct nvmf_ctrl_options *opts = ctrl->opts;
> +	struct nvmf_ctrl_options *opts = dev_to_nvme_ctrl(dev)->opts;
>   
>   	if (!opts->dhchap_secret)
>   		return sysfs_emit(buf, "none\n");
> @@ -3836,8 +3820,7 @@ static DEVICE_ATTR(dhchap_secret, S_IRUGO | S_IWUSR,
>   static ssize_t nvme_ctrl_dhchap_ctrl_secret_show(struct device *dev,
>   		struct device_attribute *attr, char *buf)
>   {
> -	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> -	struct nvmf_ctrl_options *opts = ctrl->opts;
> +	struct nvmf_ctrl_options *opts = dev_to_nvme_ctrl(dev)->opts;
>   
>   	if (!opts->dhchap_ctrl_secret)
>   		return sysfs_emit(buf, "none\n");

Re: [RFC PATCH] nvme-core: add helper to get nvme_ctrl from device

[Chaitanya Kulkarni]

Hi Sagi,

On 5/17/23 00:12, Sagi Grimberg wrote:
>
>> Just like we have other helpers to_nvmet_port(), to_nvmet_ns(),
>> ana_groups_to_port() and to_nvmet_ns(), add a helper to get nvme_ctr
>> from struct device.
>
> I don't think this is very useful.

it's the same pattern we have used everywhere in the nvme code,
I did not invent anything new here ...

>> Note that this removes the bunch of nvme_ctrl declaration code that is
>> needed to get the nvme_ctrl from struct device in the host/core.c.
>> Followwing is the diff :-
>
> It doesn't make it easier on my eyes...
>

I failed to understand how this is different from examples mentioned above
which are part of existing code reducing ~20 lines in current code and
future code for a variable declaration that is only accessed once ?

-ck

[1] diffstat:-

   drivers/nvme/host/core.c | 69 +++++++++++++++-------------------------
   1 file changed, 26 insertions(+), 43 deletions(-)

Re: [RFC PATCH] nvme-core: add helper to get nvme_ctrl from device

[Sagi Grimberg]

> Hi Sagi,
> 
> On 5/17/23 00:12, Sagi Grimberg wrote:
>>
>>> Just like we have other helpers to_nvmet_port(), to_nvmet_ns(),
>>> ana_groups_to_port() and to_nvmet_ns(), add a helper to get nvme_ctr
>>> from struct device.
>>
>> I don't think this is very useful.
> 
> it's the same pattern we have used everywhere in the nvme code,
> I did not invent anything new here ...

Still I don't find it very useful. get drvdata from a device is probably
the most common helper in device-driver territory, not sure why we want
to add a helper on top of it.

>>> Note that this removes the bunch of nvme_ctrl declaration code that is
>>> needed to get the nvme_ctrl from struct device in the host/core.c.
>>> Followwing is the diff :-
>>
>> It doesn't make it easier on my eyes...
>>
> 
> I failed to understand how this is different from examples mentioned above
> which are part of existing code reducing ~20 lines in current code and
> future code for a variable declaration that is only accessed once ?

It adds an accessor on top of an accessor. The reduction of declaration
code does not by definition make the code simpler. and personally I
don't think that making bar->x->y foo_to_bar(foo)->x->y is necessarily
an improvement.

If others think its worth while to change, I won't object, I just said
that I don't think its very useful.