[PATCH 1/4] scsi: core: constify pointer to scsi_host_template

[PATCH 1/4] scsi: core: constify pointer to scsi_host_template

[Krzysztof Kozlowski]

Several pointers to 'struct scsi_host_template' do not modify it, so
made them const for safety.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/scsi/hosts.c      |  2 +-
 drivers/scsi/scsi_error.c | 17 +++++++++--------
 drivers/scsi/scsi_proc.c  |  2 +-
 drivers/scsi/scsi_sysfs.c |  6 +++---
 4 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index f69b77cbf538..65616a01761a 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -209,7 +209,7 @@ EXPORT_SYMBOL(scsi_remove_host);
 int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev,
 			   struct device *dma_dev)
 {
-	struct scsi_host_template *sht = shost->hostt;
+	const struct scsi_host_template *sht = shost->hostt;
 	int error = -EINVAL;
 
 	shost_printk(KERN_INFO, shost, "%s\n",
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index cdaca13ac1f1..0bca4108aae2 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -58,7 +58,7 @@
 #define HOST_RESET_SETTLE_TIME  (10)
 
 static int scsi_eh_try_stu(struct scsi_cmnd *scmd);
-static enum scsi_disposition scsi_try_to_abort_cmd(struct scsi_host_template *,
+static enum scsi_disposition scsi_try_to_abort_cmd(const struct scsi_host_template *,
 						   struct scsi_cmnd *);
 
 void scsi_eh_wakeup(struct Scsi_Host *shost)
@@ -692,7 +692,7 @@ EXPORT_SYMBOL_GPL(scsi_check_sense);
 
 static void scsi_handle_queue_ramp_up(struct scsi_device *sdev)
 {
-	struct scsi_host_template *sht = sdev->host->hostt;
+	const struct scsi_host_template *sht = sdev->host->hostt;
 	struct scsi_device *tmp_sdev;
 
 	if (!sht->track_queue_depth ||
@@ -724,7 +724,7 @@ static void scsi_handle_queue_ramp_up(struct scsi_device *sdev)
 
 static void scsi_handle_queue_full(struct scsi_device *sdev)
 {
-	struct scsi_host_template *sht = sdev->host->hostt;
+	const struct scsi_host_template *sht = sdev->host->hostt;
 	struct scsi_device *tmp_sdev;
 
 	if (!sht->track_queue_depth)
@@ -833,7 +833,7 @@ static enum scsi_disposition scsi_try_host_reset(struct scsi_cmnd *scmd)
 	unsigned long flags;
 	enum scsi_disposition rtn;
 	struct Scsi_Host *host = scmd->device->host;
-	struct scsi_host_template *hostt = host->hostt;
+	const struct scsi_host_template *hostt = host->hostt;
 
 	SCSI_LOG_ERROR_RECOVERY(3,
 		shost_printk(KERN_INFO, host, "Snd Host RST\n"));
@@ -863,7 +863,7 @@ static enum scsi_disposition scsi_try_bus_reset(struct scsi_cmnd *scmd)
 	unsigned long flags;
 	enum scsi_disposition rtn;
 	struct Scsi_Host *host = scmd->device->host;
-	struct scsi_host_template *hostt = host->hostt;
+	const struct scsi_host_template *hostt = host->hostt;
 
 	SCSI_LOG_ERROR_RECOVERY(3, scmd_printk(KERN_INFO, scmd,
 		"%s: Snd Bus RST\n", __func__));
@@ -905,7 +905,7 @@ static enum scsi_disposition scsi_try_target_reset(struct scsi_cmnd *scmd)
 	unsigned long flags;
 	enum scsi_disposition rtn;
 	struct Scsi_Host *host = scmd->device->host;
-	struct scsi_host_template *hostt = host->hostt;
+	const struct scsi_host_template *hostt = host->hostt;
 
 	if (!hostt->eh_target_reset_handler)
 		return FAILED;
@@ -934,7 +934,7 @@ static enum scsi_disposition scsi_try_target_reset(struct scsi_cmnd *scmd)
 static enum scsi_disposition scsi_try_bus_device_reset(struct scsi_cmnd *scmd)
 {
 	enum scsi_disposition rtn;
-	struct scsi_host_template *hostt = scmd->device->host->hostt;
+	const struct scsi_host_template *hostt = scmd->device->host->hostt;
 
 	if (!hostt->eh_device_reset_handler)
 		return FAILED;
@@ -963,7 +963,8 @@ static enum scsi_disposition scsi_try_bus_device_reset(struct scsi_cmnd *scmd)
  *    link down on FibreChannel)
  */
 static enum scsi_disposition
-scsi_try_to_abort_cmd(struct scsi_host_template *hostt, struct scsi_cmnd *scmd)
+scsi_try_to_abort_cmd(const struct scsi_host_template *hostt,
+		      struct scsi_cmnd *scmd)
 {
 	if (!hostt->eh_abort_handler)
 		return FAILED;
diff --git a/drivers/scsi/scsi_proc.c b/drivers/scsi/scsi_proc.c
index 95aee1ad1383..6f023a2f951b 100644
--- a/drivers/scsi/scsi_proc.c
+++ b/drivers/scsi/scsi_proc.c
@@ -137,7 +137,7 @@ void scsi_proc_hostdir_rm(struct scsi_host_template *sht)
  */
 void scsi_proc_host_add(struct Scsi_Host *shost)
 {
-	struct scsi_host_template *sht = shost->hostt;
+	const struct scsi_host_template *sht = shost->hostt;
 	struct proc_dir_entry *p;
 	char name[10];
 
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index dc6872e352bd..cdc6e1ab8ce7 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -296,7 +296,7 @@ store_host_reset(struct device *dev, struct device_attribute *attr,
 		const char *buf, size_t count)
 {
 	struct Scsi_Host *shost = class_to_shost(dev);
-	struct scsi_host_template *sht = shost->hostt;
+	const struct scsi_host_template *sht = shost->hostt;
 	int ret = -EINVAL;
 	int type;
 
@@ -1017,7 +1017,7 @@ sdev_store_queue_depth(struct device *dev, struct device_attribute *attr,
 {
 	int depth, retval;
 	struct scsi_device *sdev = to_scsi_device(dev);
-	struct scsi_host_template *sht = sdev->host->hostt;
+	const struct scsi_host_template *sht = sdev->host->hostt;
 
 	if (!sht->change_queue_depth)
 		return -EINVAL;
@@ -1584,7 +1584,7 @@ void scsi_sysfs_device_initialize(struct scsi_device *sdev)
 {
 	unsigned long flags;
 	struct Scsi_Host *shost = sdev->host;
-	struct scsi_host_template *hostt = shost->hostt;
+	const struct scsi_host_template *hostt = shost->hostt;
 	struct scsi_target  *starget = sdev->sdev_target;
 
 	device_initialize(&sdev->sdev_gendev);
-- 
2.32.0

[PATCH 2/4] scsi: core: fix white-spaces

[Krzysztof Kozlowski]

Remove trailing white-spaces and correct mixed-up indentation.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/scsi/scsi_debug.c         |  2 +-
 drivers/scsi/scsi_priv.h          |  4 +--
 drivers/scsi/scsi_proc.c          | 14 ++++-----
 drivers/scsi/scsi_scan.c          | 10 +++----
 drivers/scsi/scsi_sysfs.c         |  4 +--
 drivers/scsi/scsi_transport_spi.c | 49 +++++++++++++++----------------
 drivers/scsi/scsicam.c            |  6 ++--
 include/scsi/scsi_cmnd.h          |  2 +-
 include/scsi/scsi_device.h        | 10 +++----
 include/scsi/scsi_host.h          | 13 ++++----
 include/scsi/scsi_ioctl.h         |  2 +-
 include/scsi/scsi_transport.h     |  2 +-
 include/scsi/scsi_transport_spi.h |  2 +-
 include/scsi/scsicam.h            |  2 +-
 include/scsi/sg.h                 |  2 +-
 15 files changed, 61 insertions(+), 63 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index c607755cce00..e227e2fc98ba 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1602,7 +1602,7 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 		u32 len;
 		char lu_id_str[6];
 		int host_no = devip->sdbg_host->shost->host_no;
-		
+
 		port_group_id = (((host_no + 1) & 0x7f) << 8) +
 		    (devip->channel & 0x7f);
 		if (sdebug_vpd_use_hostno == 0)
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index 5c4786310a31..8bc7353883ee 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -37,7 +37,7 @@ void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd);
 void scsi_log_send(struct scsi_cmnd *cmd);
 void scsi_log_completion(struct scsi_cmnd *cmd, int disposition);
 #else
-static inline void scsi_log_send(struct scsi_cmnd *cmd) 
+static inline void scsi_log_send(struct scsi_cmnd *cmd)
 	{ };
 static inline void scsi_log_completion(struct scsi_cmnd *cmd, int disposition)
 	{ };
@@ -183,7 +183,7 @@ struct bsg_device *scsi_bsg_register_queue(struct scsi_device *sdev);
 
 extern int scsi_device_max_queue_depth(struct scsi_device *sdev);
 
-/* 
+/*
  * internal scsi timeout functions: for use by mid-layer and transport
  * classes.
  */
diff --git a/drivers/scsi/scsi_proc.c b/drivers/scsi/scsi_proc.c
index 6f023a2f951b..fce8ae470be4 100644
--- a/drivers/scsi/scsi_proc.c
+++ b/drivers/scsi/scsi_proc.c
@@ -4,13 +4,13 @@
  *
  * The functions in this file provide an interface between
  * the PROC file system and the SCSI device drivers
- * It is mainly used for debugging, statistics and to pass 
+ * It is mainly used for debugging, statistics and to pass
  * information directly to the lowlevel driver.
  *
- * (c) 1995 Michael Neuffer neuffer@goofy.zdv.uni-mainz.de 
+ * (c) 1995 Michael Neuffer neuffer@goofy.zdv.uni-mainz.de
  * Version: 0.99.8   last change: 95/09/13
- * 
- * generic command parser provided by: 
+ *
+ * generic command parser provided by:
  * Andreas Heilwagen <crashcar@informatik.uni-koblenz.de>
  *
  * generic_proc_info() support of xxxx_info() by:
@@ -52,7 +52,7 @@ static ssize_t proc_scsi_host_write(struct file *file, const char __user *buf,
 	struct Scsi_Host *shost = pde_data(file_inode(file));
 	ssize_t ret = -ENOMEM;
 	char *page;
-    
+
 	if (count > PROC_BLOCK_SIZE)
 		return -EOVERFLOW;
 
@@ -106,7 +106,7 @@ void scsi_proc_hostdir_add(struct scsi_host_template *sht)
 	mutex_lock(&global_host_template_mutex);
 	if (!sht->present++) {
 		sht->proc_dir = proc_mkdir(sht->proc_name, proc_scsi);
-        	if (!sht->proc_dir)
+		if (!sht->proc_dir)
 			printk(KERN_ERR "%s: proc_mkdir failed for %s\n",
 			       __func__, sht->proc_name);
 	}
@@ -361,7 +361,7 @@ static ssize_t proc_scsi_write(struct file *file, const char __user *buf,
 	}
 
 	/*
-	 * convert success returns so that we return the 
+	 * convert success returns so that we return the
 	 * number of bytes consumed.
 	 */
 	if (!err)
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 2ef78083f1ef..d8c272cd9544 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -108,7 +108,7 @@ MODULE_PARM_DESC(scan, "sync, async, manual, or none. "
 static unsigned int scsi_inq_timeout = SCSI_TIMEOUT/HZ + 18;
 
 module_param_named(inq_timeout, scsi_inq_timeout, uint, S_IRUGO|S_IWUSR);
-MODULE_PARM_DESC(inq_timeout, 
+MODULE_PARM_DESC(inq_timeout,
 		 "Timeout (in seconds) waiting for devices to answer INQUIRY."
 		 " Default is 20. Some devices may need more; most need less.");
 
@@ -687,7 +687,7 @@ static int scsi_probe_lun(struct scsi_device *sdev, unsigned char *inq_result,
 			 * not-ready to ready transition [asc/ascq=0x28/0x0]
 			 * or power-on, reset [asc/ascq=0x29/0x0], continue.
 			 * INQUIRY should not yield UNIT_ATTENTION
-			 * but many buggy devices do so anyway. 
+			 * but many buggy devices do so anyway.
 			 */
 			if (scsi_status_is_check_condition(result) &&
 			    scsi_sense_valid(&sshdr)) {
@@ -933,7 +933,7 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
 	 * Don't set the device offline here; rather let the upper
 	 * level drivers eval the PQ to decide whether they should
 	 * attach. So remove ((inq_result[0] >> 5) & 7) == 1 check.
-	 */ 
+	 */
 
 	sdev->inq_periph_qual = (inq_result[0] >> 5) & 7;
 	sdev->lockable = sdev->removable;
@@ -1088,7 +1088,7 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
 }
 
 #ifdef CONFIG_SCSI_LOGGING
-/** 
+/**
  * scsi_inq_str - print INQUIRY data from min to max index, strip trailing whitespace
  * @buf:   Output buffer with at least end-first+1 bytes of space
  * @inq:   Inquiry buffer (input)
@@ -1588,7 +1588,7 @@ EXPORT_SYMBOL(__scsi_add_device);
 int scsi_add_device(struct Scsi_Host *host, uint channel,
 		    uint target, u64 lun)
 {
-	struct scsi_device *sdev = 
+	struct scsi_device *sdev =
 		__scsi_add_device(host, channel, target, lun, NULL);
 	if (IS_ERR(sdev))
 		return PTR_ERR(sdev);
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index cdc6e1ab8ce7..139ec8f4a4fc 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -660,7 +660,7 @@ static int scsi_sdev_check_buf_bit(const char *buf)
 			return 1;
 		else if (buf[0] == '0')
 			return 0;
-		else 
+		else
 			return -EINVAL;
 	} else
 		return -EINVAL;
@@ -876,7 +876,7 @@ store_queue_type_field(struct device *dev, struct device_attribute *attr,
 
 	if (!sdev->tagged_supported)
 		return -EINVAL;
-		
+
 	sdev_printk(KERN_INFO, sdev,
 		    "ignoring write to deprecated queue_type attribute");
 	return count;
diff --git a/drivers/scsi/scsi_transport_spi.c b/drivers/scsi/scsi_transport_spi.c
index bd72c38d7bfc..51d163200f41 100644
--- a/drivers/scsi/scsi_transport_spi.c
+++ b/drivers/scsi/scsi_transport_spi.c
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
-/* 
+/*
  *  Parallel SCSI (SPI) transport specific attributes exported to sysfs.
  *
  *  Copyright (c) 2003 Silicon Graphics, Inc.  All rights reserved.
@@ -33,7 +33,7 @@
 
 #define DV_LOOPS	3
 #define DV_TIMEOUT	(10*HZ)
-#define DV_RETRIES	3	/* should only need at most 
+#define DV_RETRIES	3	/* should only need at most
 				 * two cc/ua clears */
 
 /* Our blacklist flags */
@@ -651,7 +651,7 @@ spi_dv_device_echo_buffer(struct scsi_device *sdev, u8 *buffer,
 		 * 0xffff (test b) */
 		for ( ; j < min(len, k + 32); j += 2) {
 			u16 *word = (u16 *)&buffer[j];
-			
+
 			*word = (j & 0x02) ? 0x0000 : 0xffff;
 		}
 		k = j;
@@ -667,7 +667,7 @@ spi_dv_device_echo_buffer(struct scsi_device *sdev, u8 *buffer,
 		for ( ; j < min(len, k + 32); j += 4) {
 			u32 *word = (unsigned int *)&buffer[j];
 			u32 roll = (pattern & 0x80000000) ? 1 : 0;
-			
+
 			*word = pattern;
 			pattern = (pattern << 1) | roll;
 		}
@@ -724,7 +724,7 @@ spi_dv_device_compare_inquiry(struct scsi_device *sdev, u8 *buffer,
 
 		result = spi_execute(sdev, spi_inquiry, DMA_FROM_DEVICE,
 				     ptr, len, NULL);
-		
+
 		if(result || !scsi_device_online(sdev)) {
 			scsi_device_set_state(sdev, SDEV_QUIESCE);
 			return SPI_COMPARE_FAILURE;
@@ -747,12 +747,12 @@ spi_dv_device_compare_inquiry(struct scsi_device *sdev, u8 *buffer,
 
 static enum spi_compare_returns
 spi_dv_retrain(struct scsi_device *sdev, u8 *buffer, u8 *ptr,
-	       enum spi_compare_returns 
+	       enum spi_compare_returns
 	       (*compare_fn)(struct scsi_device *, u8 *, u8 *, int))
 {
 	struct spi_internal *i = to_spi_internal(sdev->host->transportt);
 	struct scsi_target *starget = sdev->sdev_target;
-	int period = 0, prevperiod = 0; 
+	int period = 0, prevperiod = 0;
 	enum spi_compare_returns retval;
 
 
@@ -808,11 +808,11 @@ spi_dv_device_get_echo_buffer(struct scsi_device *sdev, u8 *buffer)
 {
 	int l, result;
 
-	/* first off do a test unit ready.  This can error out 
+	/* first off do a test unit ready.  This can error out
 	 * because of reservations or some other reason.  If it
 	 * fails, the device won't let us write to the echo buffer
 	 * so just return failure */
-	
+
 	static const char spi_test_unit_ready[] = {
 		TEST_UNIT_READY, 0, 0, 0, 0, 0
 	};
@@ -821,14 +821,13 @@ spi_dv_device_get_echo_buffer(struct scsi_device *sdev, u8 *buffer)
 		READ_BUFFER, 0x0b, 0, 0, 0, 0, 0, 0, 4, 0
 	};
 
-	
-	/* We send a set of three TURs to clear any outstanding 
+	/* We send a set of three TURs to clear any outstanding
 	 * unit attention conditions if they exist (Otherwise the
 	 * buffer tests won't be happy).  If the TUR still fails
 	 * (reservation conflict, device not ready, etc) just
 	 * skip the write tests */
 	for (l = 0; ; l++) {
-		result = spi_execute(sdev, spi_test_unit_ready, DMA_NONE, 
+		result = spi_execute(sdev, spi_test_unit_ready, DMA_NONE,
 				     NULL, 0, NULL);
 
 		if(result) {
@@ -840,7 +839,7 @@ spi_dv_device_get_echo_buffer(struct scsi_device *sdev, u8 *buffer)
 		}
 	}
 
-	result = spi_execute(sdev, spi_read_buffer_descriptor, 
+	result = spi_execute(sdev, spi_read_buffer_descriptor,
 			     DMA_FROM_DEVICE, buffer, 4, NULL);
 
 	if (result)
@@ -1222,7 +1221,7 @@ EXPORT_SYMBOL_GPL(spi_populate_ppr_msg);
  * @cmd:	pointer to the scsi command for the tag
  *
  * Notes:
- *	designed to create the correct type of tag message for the 
+ *	designed to create the correct type of tag message for the
  *	particular request.  Returns the size of the tag message.
  *	May return 0 if TCQ is disabled for this device.
  **/
@@ -1231,7 +1230,7 @@ int spi_populate_tag_msg(unsigned char *msg, struct scsi_cmnd *cmd)
         if (cmd->flags & SCMD_TAGGED) {
 		*msg++ = SIMPLE_QUEUE_TAG;
 		*msg++ = scsi_cmd_to_rq(cmd)->tag;
-        	return 2;
+		return 2;
 	}
 
 	return 0;
@@ -1241,10 +1240,10 @@ EXPORT_SYMBOL_GPL(spi_populate_tag_msg);
 #ifdef CONFIG_SCSI_CONSTANTS
 static const char * const one_byte_msgs[] = {
 /* 0x00 */ "Task Complete", NULL /* Extended Message */, "Save Pointers",
-/* 0x03 */ "Restore Pointers", "Disconnect", "Initiator Error", 
+/* 0x03 */ "Restore Pointers", "Disconnect", "Initiator Error",
 /* 0x06 */ "Abort Task Set", "Message Reject", "Nop", "Message Parity Error",
 /* 0x0a */ "Linked Command Complete", "Linked Command Complete w/flag",
-/* 0x0c */ "Target Reset", "Abort Task", "Clear Task Set", 
+/* 0x0c */ "Target Reset", "Abort Task", "Clear Task Set",
 /* 0x0f */ "Initiate Recovery", "Release Recovery",
 /* 0x11 */ "Terminate Process", "Continue Task", "Target Transfer Disable",
 /* 0x14 */ NULL, NULL, "Clear ACA", "LUN Reset"
@@ -1290,8 +1289,8 @@ int spi_print_msg(const unsigned char *msg)
 		if (len == 2)
 			len += 256;
 		if (msg[2] < ARRAY_SIZE(extended_msgs))
-			printk ("%s ", extended_msgs[msg[2]]); 
-		else 
+			printk ("%s ", extended_msgs[msg[2]]);
+		else
 			printk ("Extended Message, reserved code (0x%02x) ",
 				(int) msg[2]);
 		switch (msg[2]) {
@@ -1312,7 +1311,7 @@ int spi_print_msg(const unsigned char *msg)
 			print_ptr(msg, 7, "in");
 			break;
 		default:
-		for (i = 2; i < len; ++i) 
+		for (i = 2; i < len; ++i)
 			printk("%02x ", msg[i]);
 		}
 	/* Identify */
@@ -1332,13 +1331,13 @@ int spi_print_msg(const unsigned char *msg)
 	/* Two byte */
 	} else if (msg[0] <= 0x2f) {
 		if ((msg[0] - 0x20) < ARRAY_SIZE(two_byte_msgs))
-			printk("%s %02x ", two_byte_msgs[msg[0] - 0x20], 
+			printk("%s %02x ", two_byte_msgs[msg[0] - 0x20],
 				msg[1]);
-		else 
-			printk("reserved two byte (%02x %02x) ", 
+		else
+			printk("reserved two byte (%02x %02x) ",
 				msg[0], msg[1]);
 		len = 2;
-	} else 
+	} else
 		printk("reserved ");
 	return len;
 }
@@ -1366,7 +1365,7 @@ int spi_print_msg(const unsigned char *msg)
 	} else if (msg[0] <= 0x2f) {
 		printk("%02x %02x", msg[0], msg[1]);
 		len = 2;
-	} else 
+	} else
 		printk("%02x ", msg[0]);
 	return len;
 }
diff --git a/drivers/scsi/scsicam.c b/drivers/scsi/scsicam.c
index acdc0aceca5e..30385c176c68 100644
--- a/drivers/scsi/scsicam.c
+++ b/drivers/scsi/scsicam.c
@@ -3,7 +3,7 @@
  * scsicam.c - SCSI CAM support functions, use for HDIO_GETGEO, etc.
  *
  * Copyright 1993, 1994 Drew Eckhardt
- *      Visionary Computing 
+ *      Visionary Computing
  *      (Unix and Linux consulting and custom programming)
  *      drew@Colorado.EDU
  *      +1 (303) 786-7975
@@ -160,7 +160,7 @@ EXPORT_SYMBOL(scsi_partsize);
  * Information technology -
  * SCSI-2 Common access method
  * transport and SCSI interface module
- * 
+ *
  * ANNEX A :
  *
  * setsize() converts a read capacity value to int 13h
@@ -170,7 +170,7 @@ EXPORT_SYMBOL(scsi_partsize);
  * will not fit in 4 bits (or 6 bits). This algorithm also
  * minimizes the number of sectors that will be unused at the end
  * of the disk while allowing for very large disks to be
- * accommodated. This algorithm does not use physical geometry. 
+ * accommodated. This algorithm does not use physical geometry.
  */
 
 static int setsize(unsigned long capacity, unsigned int *cyls, unsigned int *hds,
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 1e80e70dfa92..410a2ceefc52 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -105,7 +105,7 @@ struct scsi_cmnd {
 
 	unsigned transfersize;	/* How much we are guaranteed to
 				   transfer with each SCSI transfer
-				   (ie, between disconnect / 
+				   (ie, between disconnect /
 				   reconnects.   Probably == sector
 				   size */
 	unsigned resid_len;	/* residual count */
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 57e3e239a1fc..cbbd69902f61 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -42,7 +42,7 @@ enum scsi_device_state {
 				 * All commands allowed */
 	SDEV_CANCEL,		/* beginning to delete device
 				 * Only error handler commands allowed */
-	SDEV_DEL,		/* device deleted 
+	SDEV_DEL,		/* device deleted
 				 * no commands allowed */
 	SDEV_QUIESCE,		/* Device quiescent.  No block commands
 				 * will be accepted, only specials (which
@@ -126,14 +126,14 @@ struct scsi_device {
 
 	unsigned int id, channel;
 	u64 lun;
-	unsigned int manufacturer;	/* Manufacturer of device, for using 
+	unsigned int manufacturer;	/* Manufacturer of device, for using
 					 * vendor-specific cmd's */
 	unsigned sector_size;	/* size in bytes */
 
 	void *hostdata;		/* available to low-level driver */
 	unsigned char type;
 	char scsi_level;
-	char inq_periph_qual;	/* PQ from INQUIRY data */	
+	char inq_periph_qual;	/* PQ from INQUIRY data */
 	struct mutex inquiry_mutex;
 	unsigned char inquiry_len;	/* valid bytes in 'inquiry' */
 	unsigned char * inquiry;	/* INQUIRY response data */
@@ -158,7 +158,7 @@ struct scsi_device {
 	unsigned busy:1;	/* Used to prevent races */
 	unsigned lockable:1;	/* Able to prevent media removal */
 	unsigned locked:1;      /* Media removal disabled */
-	unsigned borken:1;	/* Tell the Seagate driver to be 
+	unsigned borken:1;	/* Tell the Seagate driver to be
 				 * painfully slow on this device */
 	unsigned disconnect:1;	/* can disconnect */
 	unsigned soft_reset:1;	/* Uses soft reset option */
@@ -167,7 +167,7 @@ struct scsi_device {
 	unsigned ppr:1;		/* Device supports PPR messages */
 	unsigned tagged_supported:1;	/* Supports SCSI-II tagged queuing */
 	unsigned simple_tags:1;	/* simple queue tag messages are enabled */
-	unsigned was_reset:1;	/* There was a bus reset on the bus for 
+	unsigned was_reset:1;	/* There was a bus reset on the bus for
 				 * this device */
 	unsigned expecting_cc_ua:1; /* Expecting a CHECK_CONDITION/UNIT_ATTN
 				     * because we did a bus reset. */
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 667d889b92b5..44ff28d8fd0d 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -178,7 +178,7 @@ struct scsi_host_template {
 	 * this function, it *must* perform the task of setting the queue
 	 * depth on the device.  All other tasks are optional and depend
 	 * on what the driver supports and various implementation details.
-	 * 
+	 *
 	 * Things currently recommended to be handled at this time include:
 	 *
 	 * 1.  Setting the device queue depth.  Proper setting of this is
@@ -207,7 +207,7 @@ struct scsi_host_template {
 	 * has ceased the mid layer calls this point so that the low level
 	 * driver may completely detach itself from the scsi device and vice
 	 * versa.  The low level driver is responsible for freeing any memory
-	 * it allocated in the slave_alloc or slave_configure calls. 
+	 * it allocated in the slave_alloc or slave_configure calls.
 	 *
 	 * Status: OPTIONAL
 	 */
@@ -466,7 +466,7 @@ struct scsi_host_template {
 	/*
 	 * Default value for the blocking.  If the queue is empty,
 	 * host_blocked counts down in the request_fn until it restarts
-	 * host operations as zero is reached.  
+	 * host operations as zero is reached.
 	 *
 	 * FIXME: This should probably be a value in the template
 	 */
@@ -540,7 +540,7 @@ struct Scsi_Host {
 	 */
 	struct list_head	__devices;
 	struct list_head	__targets;
-	
+
 	struct list_head	starved_list;
 
 	spinlock_t		default_lock;
@@ -565,7 +565,7 @@ struct Scsi_Host {
 	unsigned int host_failed;	   /* commands that failed.
 					      protected by host_lock */
 	unsigned int host_eh_scheduled;    /* EH scheduled without command */
-    
+
 	unsigned int host_no;  /* Used for IOCTL_GET_IDLUN, /proc/scsi et al. */
 
 	/* next two fields are used to bound the time spent in error handling */
@@ -627,7 +627,7 @@ struct Scsi_Host {
 	 * time being.
 	 */
 	unsigned host_self_blocked:1;
-    
+
 	/*
 	 * Host uses correct SCSI ordering not PC ordering. The bit is
 	 * set for the minority of drivers whose authors actually read
@@ -682,7 +682,6 @@ struct Scsi_Host {
 	unsigned char n_io_port;
 	unsigned char dma_channel;
 	unsigned int  irq;
-	
 
 	enum scsi_host_state shost_state;
 
diff --git a/include/scsi/scsi_ioctl.h b/include/scsi/scsi_ioctl.h
index beac64e38b87..c3ab331c809b 100644
--- a/include/scsi/scsi_ioctl.h
+++ b/include/scsi/scsi_ioctl.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 #ifndef _SCSI_IOCTL_H
-#define _SCSI_IOCTL_H 
+#define _SCSI_IOCTL_H
 
 #define SCSI_IOCTL_SEND_COMMAND 1
 #define SCSI_IOCTL_TEST_UNIT_READY 2
diff --git a/include/scsi/scsi_transport.h b/include/scsi/scsi_transport.h
index a0458bda3148..1a7b92f8b423 100644
--- a/include/scsi/scsi_transport.h
+++ b/include/scsi/scsi_transport.h
@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
-/* 
+/*
  *  Transport specific attributes.
  *
  *  Copyright (c) 2003 Silicon Graphics, Inc.  All rights reserved.
diff --git a/include/scsi/scsi_transport_spi.h b/include/scsi/scsi_transport_spi.h
index 78324502b1c9..d967bc6f1836 100644
--- a/include/scsi/scsi_transport_spi.h
+++ b/include/scsi/scsi_transport_spi.h
@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: GPL-2.0-or-later */
-/* 
+/*
  *  Parallel SCSI (SPI) transport specific attributes exported to sysfs.
  *
  *  Copyright (c) 2003 Silicon Graphics, Inc.  All rights reserved.
diff --git a/include/scsi/scsicam.h b/include/scsi/scsicam.h
index 08edd603e521..3a37512c06ec 100644
--- a/include/scsi/scsicam.h
+++ b/include/scsi/scsicam.h
@@ -3,7 +3,7 @@
  * scsicam.h - SCSI CAM support functions, use for HDIO_GETGEO, etc.
  *
  * Copyright 1993, 1994 Drew Eckhardt
- *      Visionary Computing 
+ *      Visionary Computing
  *      (Unix and Linux consulting and custom programming)
  *      drew@Colorado.EDU
  *	+1 (303) 786-7975
diff --git a/include/scsi/sg.h b/include/scsi/sg.h
index 068e35d36557..e16160b5b407 100644
--- a/include/scsi/sg.h
+++ b/include/scsi/sg.h
@@ -244,7 +244,7 @@ typedef struct sg_req_info { /* used by SG_GET_REQUEST_TABLE ioctl() */
 #define SG_GET_KEEP_ORPHAN 0x2288
 
 /* yields scsi midlevel's access_count for this SCSI device */
-#define SG_GET_ACCESS_COUNT 0x2289  
+#define SG_GET_ACCESS_COUNT 0x2289
 
 
 #define SG_SCATTER_SZ (8 * 4096)
-- 
2.32.0

Re: [PATCH 2/4] scsi: core: fix white-spaces

[Krzysztof Kozlowski]

On 08/04/2022 12:30, Krzysztof Kozlowski wrote:
> Remove trailing white-spaces and correct mixed-up indentation.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  drivers/scsi/scsi_debug.c         |  2 +-
>  drivers/scsi/scsi_priv.h          |  4 +--
>  drivers/scsi/scsi_proc.c          | 14 ++++-----
>  drivers/scsi/scsi_scan.c          | 10 +++----
>  drivers/scsi/scsi_sysfs.c         |  4 +--
>  drivers/scsi/scsi_transport_spi.c | 49 +++++++++++++++----------------
>  drivers/scsi/scsicam.c            |  6 ++--
>  include/scsi/scsi_cmnd.h          |  2 +-
>  include/scsi/scsi_device.h        | 10 +++----
>  include/scsi/scsi_host.h          | 13 ++++----
>  include/scsi/scsi_ioctl.h         |  2 +-
>  include/scsi/scsi_transport.h     |  2 +-
>  include/scsi/scsi_transport_spi.h |  2 +-
>  include/scsi/scsicam.h            |  2 +-
>  include/scsi/sg.h                 |  2 +-
>  15 files changed, 61 insertions(+), 63 deletions(-)

Hi folks!

I understand that patch #1 in the series is discussed/needs followup,
but what about patches 2-4? Could you pick them up?

Best regards,
Krzysztof

[PATCH 3/4] scsi: ufs: ufshcd-pltfrm: constify pointed data

[Krzysztof Kozlowski]

Constify pointers to data which is not modified for code safety.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/scsi/ufs/ufshcd-pltfrm.c | 10 +++++-----
 drivers/scsi/ufs/ufshcd-pltfrm.h |  4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index 87975d1a21c8..3548897c3692 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -25,7 +25,7 @@ static int ufshcd_parse_clock_info(struct ufs_hba *hba)
 	int i;
 	struct device *dev = hba->dev;
 	struct device_node *np = dev->of_node;
-	char *name;
+	const char *name;
 	u32 *clkfreq = NULL;
 	struct ufs_clk_info *clki;
 	int len = 0;
@@ -78,8 +78,8 @@ static int ufshcd_parse_clock_info(struct ufs_hba *hba)
 	}
 
 	for (i = 0; i < sz; i += 2) {
-		ret = of_property_read_string_index(np,
-				"clock-names", i/2, (const char **)&name);
+		ret = of_property_read_string_index(np,	"clock-names", i/2,
+						    &name);
 		if (ret)
 			goto out;
 
@@ -207,8 +207,8 @@ static void ufshcd_init_lanes_per_dir(struct ufs_hba *hba)
  *
  * Returns 0 on success, non-zero value on failure
  */
-int ufshcd_get_pwr_dev_param(struct ufs_dev_params *pltfrm_param,
-			     struct ufs_pa_layer_attr *dev_max,
+int ufshcd_get_pwr_dev_param(const struct ufs_dev_params *pltfrm_param,
+			     const struct ufs_pa_layer_attr *dev_max,
 			     struct ufs_pa_layer_attr *agreed_pwr)
 {
 	int min_pltfrm_gear;
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.h b/drivers/scsi/ufs/ufshcd-pltfrm.h
index c33e28ac6ef6..0f0c2942a5be 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.h
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.h
@@ -25,8 +25,8 @@ struct ufs_dev_params {
 	u32 desired_working_mode;
 };
 
-int ufshcd_get_pwr_dev_param(struct ufs_dev_params *dev_param,
-			     struct ufs_pa_layer_attr *dev_max,
+int ufshcd_get_pwr_dev_param(const struct ufs_dev_params *dev_param,
+			     const struct ufs_pa_layer_attr *dev_max,
 			     struct ufs_pa_layer_attr *agreed_pwr);
 void ufshcd_init_pwr_dev_param(struct ufs_dev_params *dev_param);
 int ufshcd_pltfrm_init(struct platform_device *pdev,
-- 
2.32.0

[PATCH 4/4] scsi: ufs: ufshcd: constify pointed data

[Krzysztof Kozlowski]

Constify arrays and pointers to data which is not modified for code
safety.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/scsi/ufs/ufshcd.c | 48 ++++++++++++++++++++-------------------
 drivers/scsi/ufs/ufshcd.h |  9 ++++----
 2 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 3f9caafa91bf..5bfa62fa288a 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -162,7 +162,7 @@ enum {
 #define ufshcd_clear_eh_in_progress(h) \
 	((h)->eh_flags &= ~UFSHCD_EH_IN_PROGRESS)
 
-struct ufs_pm_lvl_states ufs_pm_lvl_states[] = {
+const struct ufs_pm_lvl_states ufs_pm_lvl_states[] = {
 	[UFS_PM_LVL_0] = {UFS_ACTIVE_PWR_MODE, UIC_LINK_ACTIVE_STATE},
 	[UFS_PM_LVL_1] = {UFS_ACTIVE_PWR_MODE, UIC_LINK_HIBERN8_STATE},
 	[UFS_PM_LVL_2] = {UFS_SLEEP_PWR_MODE, UIC_LINK_ACTIVE_STATE},
@@ -204,7 +204,7 @@ ufs_get_desired_pm_lvl_for_dev_link_state(enum ufs_dev_pwr_mode dev_state,
 	return UFS_PM_LVL_0;
 }
 
-static struct ufs_dev_fix ufs_fixups[] = {
+static const struct ufs_dev_fix ufs_fixups[] = {
 	/* UFS cards deviations table */
 	UFS_FIX(UFS_VENDOR_MICRON, UFS_ANY_MODEL,
 		UFS_DEVICE_QUIRK_DELAY_BEFORE_LPM |
@@ -343,7 +343,7 @@ static void ufshcd_add_tm_upiu_trace(struct ufs_hba *hba, unsigned int tag,
 }
 
 static void ufshcd_add_uic_command_trace(struct ufs_hba *hba,
-					 struct uic_command *ucmd,
+					 const struct uic_command *ucmd,
 					 enum ufs_trace_str_t str_t)
 {
 	u32 cmd;
@@ -423,11 +423,11 @@ static void ufshcd_print_clk_freqs(struct ufs_hba *hba)
 }
 
 static void ufshcd_print_evt(struct ufs_hba *hba, u32 id,
-			     char *err_name)
+			     const char *err_name)
 {
 	int i;
 	bool found = false;
-	struct ufs_event_hist *e;
+	const struct ufs_event_hist *e;
 
 	if (id >= UFS_EVT_CNT)
 		return;
@@ -477,7 +477,7 @@ static void ufshcd_print_evt_hist(struct ufs_hba *hba)
 static
 void ufshcd_print_trs(struct ufs_hba *hba, unsigned long bitmap, bool pr_prdt)
 {
-	struct ufshcd_lrb *lrbp;
+	const struct ufshcd_lrb *lrbp;
 	int prdt_length;
 	int tag;
 
@@ -533,7 +533,7 @@ static void ufshcd_print_tmrs(struct ufs_hba *hba, unsigned long bitmap)
 
 static void ufshcd_print_host_state(struct ufs_hba *hba)
 {
-	struct scsi_device *sdev_ufs = hba->sdev_ufs_device;
+	const struct scsi_device *sdev_ufs = hba->sdev_ufs_device;
 
 	dev_err(hba->dev, "UFS Host state=%d\n", hba->ufshcd_state);
 	dev_err(hba->dev, "outstanding reqs=0x%lx tasks=0x%lx\n",
@@ -1083,7 +1083,7 @@ static bool ufshcd_is_devfreq_scaling_required(struct ufs_hba *hba,
  */
 static u32 ufshcd_pending_cmds(struct ufs_hba *hba)
 {
-	struct scsi_device *sdev;
+	const struct scsi_device *sdev;
 	u32 pending = 0;
 
 	lockdep_assert_held(hba->host->host_lock);
@@ -2046,14 +2046,15 @@ static inline int ufshcd_monitor_opcode2dir(u8 opcode)
 static inline bool ufshcd_should_inform_monitor(struct ufs_hba *hba,
 						struct ufshcd_lrb *lrbp)
 {
-	struct ufs_hba_monitor *m = &hba->monitor;
+	const struct ufs_hba_monitor *m = &hba->monitor;
 
 	return (m->enabled && lrbp && lrbp->cmd &&
 		(!m->chunk_size || m->chunk_size == lrbp->cmd->sdb.length) &&
 		ktime_before(hba->monitor.enabled_ts, lrbp->issue_time_stamp));
 }
 
-static void ufshcd_start_monitor(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
+static void ufshcd_start_monitor(struct ufs_hba *hba,
+				 const struct ufshcd_lrb *lrbp)
 {
 	int dir = ufshcd_monitor_opcode2dir(*lrbp->cmd->cmnd);
 	unsigned long flags;
@@ -2064,14 +2065,14 @@ static void ufshcd_start_monitor(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 	spin_unlock_irqrestore(hba->host->host_lock, flags);
 }
 
-static void ufshcd_update_monitor(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
+static void ufshcd_update_monitor(struct ufs_hba *hba, const struct ufshcd_lrb *lrbp)
 {
 	int dir = ufshcd_monitor_opcode2dir(*lrbp->cmd->cmnd);
 	unsigned long flags;
 
 	spin_lock_irqsave(hba->host->host_lock, flags);
 	if (dir >= 0 && hba->monitor.nr_queued[dir] > 0) {
-		struct request *req = scsi_cmd_to_rq(lrbp->cmd);
+		const struct request *req = scsi_cmd_to_rq(lrbp->cmd);
 		struct ufs_hba_monitor *m = &hba->monitor;
 		ktime_t now, inc, lat;
 
@@ -4888,7 +4889,7 @@ static int ufshcd_get_lu_wp(struct ufs_hba *hba,
  *
  */
 static inline void ufshcd_get_lu_power_on_wp_status(struct ufs_hba *hba,
-						    struct scsi_device *sdev)
+						    const struct scsi_device *sdev)
 {
 	if (hba->dev_info.f_power_on_wp_en &&
 	    !hba->dev_info.is_lu_power_on_wp) {
@@ -5407,8 +5408,8 @@ int ufshcd_write_ee_control(struct ufs_hba *hba)
 	return err;
 }
 
-int ufshcd_update_ee_control(struct ufs_hba *hba, u16 *mask, u16 *other_mask,
-			     u16 set, u16 clr)
+int ufshcd_update_ee_control(struct ufs_hba *hba, u16 *mask,
+			     const u16 *other_mask, u16 set, u16 clr)
 {
 	u16 new_mask, ee_ctrl_mask;
 	int err = 0;
@@ -7343,7 +7344,8 @@ static int ufshcd_eh_host_reset_handler(struct scsi_cmnd *cmd)
  *
  * Returns calculated max ICC level for specific regulator
  */
-static u32 ufshcd_get_max_icc_level(int sup_curr_uA, u32 start_scan, char *buff)
+static u32 ufshcd_get_max_icc_level(int sup_curr_uA, u32 start_scan,
+				    const char *buff)
 {
 	int i;
 	int curr_uA;
@@ -7390,7 +7392,7 @@ static u32 ufshcd_get_max_icc_level(int sup_curr_uA, u32 start_scan, char *buff)
  * Returns calculated ICC level
  */
 static u32 ufshcd_find_max_sup_active_icc_level(struct ufs_hba *hba,
-							u8 *desc_buf, int len)
+						const u8 *desc_buf, int len)
 {
 	u32 icc_level = 0;
 
@@ -7540,7 +7542,7 @@ static int ufshcd_scsi_add_wlus(struct ufs_hba *hba)
 	return ret;
 }
 
-static void ufshcd_wb_probe(struct ufs_hba *hba, u8 *desc_buf)
+static void ufshcd_wb_probe(struct ufs_hba *hba, const u8 *desc_buf)
 {
 	struct ufs_dev_info *dev_info = &hba->dev_info;
 	u8 lun;
@@ -7611,7 +7613,7 @@ static void ufshcd_wb_probe(struct ufs_hba *hba, u8 *desc_buf)
 	hba->caps &= ~UFSHCD_CAP_WB_EN;
 }
 
-static void ufshcd_temp_notif_probe(struct ufs_hba *hba, u8 *desc_buf)
+static void ufshcd_temp_notif_probe(struct ufs_hba *hba, const u8 *desc_buf)
 {
 	struct ufs_dev_info *dev_info = &hba->dev_info;
 	u32 ext_ufs_feature;
@@ -7634,9 +7636,9 @@ static void ufshcd_temp_notif_probe(struct ufs_hba *hba, u8 *desc_buf)
 	}
 }
 
-void ufshcd_fixup_dev_quirks(struct ufs_hba *hba, struct ufs_dev_fix *fixups)
+void ufshcd_fixup_dev_quirks(struct ufs_hba *hba, const struct ufs_dev_fix *fixups)
 {
-	struct ufs_dev_fix *f;
+	const struct ufs_dev_fix *f;
 	struct ufs_dev_info *dev_info = &hba->dev_info;
 
 	if (!fixups)
@@ -7844,7 +7846,7 @@ static int ufshcd_quirk_tune_host_pa_tactivate(struct ufs_hba *hba)
 	u32 granularity, peer_granularity;
 	u32 pa_tactivate, peer_pa_tactivate;
 	u32 pa_tactivate_us, peer_pa_tactivate_us;
-	u8 gran_to_us_table[] = {1, 4, 8, 16, 32, 100};
+	const u8 gran_to_us_table[] = {1, 4, 8, 16, 32, 100};
 
 	ret = ufshcd_dme_get(hba, UIC_ARG_MIB(PA_GRANULARITY),
 				  &granularity);
@@ -7956,7 +7958,7 @@ static int ufshcd_device_geo_params_init(struct ufs_hba *hba)
 	return err;
 }
 
-static struct ufs_ref_clk ufs_ref_clk_freqs[] = {
+static const struct ufs_ref_clk ufs_ref_clk_freqs[] = {
 	{19200000, REF_CLK_FREQ_19_2_MHZ},
 	{26000000, REF_CLK_FREQ_26_MHZ},
 	{38400000, REF_CLK_FREQ_38_4_MHZ},
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 94f545be183a..1a8f7b8977e6 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -1185,7 +1185,8 @@ int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode,
 
 void ufshcd_auto_hibern8_enable(struct ufs_hba *hba);
 void ufshcd_auto_hibern8_update(struct ufs_hba *hba, u32 ahit);
-void ufshcd_fixup_dev_quirks(struct ufs_hba *hba, struct ufs_dev_fix *fixups);
+void ufshcd_fixup_dev_quirks(struct ufs_hba *hba,
+			     const struct ufs_dev_fix *fixups);
 #define SD_ASCII_STD true
 #define SD_RAW false
 int ufshcd_read_string_desc(struct ufs_hba *hba, u8 desc_index,
@@ -1371,7 +1372,7 @@ static inline void ufshcd_vops_config_scaling_param(struct ufs_hba *hba,
 		hba->vops->config_scaling_param(hba, profile, data);
 }
 
-extern struct ufs_pm_lvl_states ufs_pm_lvl_states[];
+extern const struct ufs_pm_lvl_states ufs_pm_lvl_states[];
 
 /*
  * ufshcd_scsi_to_upiu_lun - maps scsi LUN to UPIU LUN
@@ -1393,8 +1394,8 @@ int ufshcd_dump_regs(struct ufs_hba *hba, size_t offset, size_t len,
 
 int __ufshcd_write_ee_control(struct ufs_hba *hba, u32 ee_ctrl_mask);
 int ufshcd_write_ee_control(struct ufs_hba *hba);
-int ufshcd_update_ee_control(struct ufs_hba *hba, u16 *mask, u16 *other_mask,
-			     u16 set, u16 clr);
+int ufshcd_update_ee_control(struct ufs_hba *hba, u16 *mask,
+			     const u16 *other_mask, u16 set, u16 clr);
 
 static inline int ufshcd_update_ee_drv_mask(struct ufs_hba *hba,
 					    u16 set, u16 clr)
-- 
2.32.0

Re: [PATCH 4/4] scsi: ufs: ufshcd: constify pointed data

[Bart Van Assche]

On 4/8/22 03:30, Krzysztof Kozlowski wrote:
> Constify arrays and pointers to data which is not modified for code
> safety.

This patch includes changes that have already been posted. See also 
https://lore.kernel.org/linux-scsi/20220331223424.1054715-1-bvanassche@acm.org/T/#m47f9489f1976039d07bfa4ec766b06d3f2d73239

Thanks,

Bart.

Re: [PATCH 1/4] scsi: core: constify pointer to scsi_host_template

[John Garry]

On 08/04/2022 11:30, Krzysztof Kozlowski wrote:
> Several pointers to 'struct scsi_host_template' do not modify it, so
> made them const for safety.
> 

Is this standard practice? What is so special here?

Thanks,
john


> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>   drivers/scsi/hosts.c      |  2 +-
>   drivers/scsi/scsi_error.c | 17 +++++++++--------
>   drivers/scsi/scsi_proc.c  |  2 +-
>   drivers/scsi/scsi_sysfs.c |  6 +++---
>   4 files changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
> index f69b77cbf538..65616a01761a 100644
> --- a/drivers/scsi/hosts.c
> +++ b/drivers/scsi/hosts.c
> @@ -209,7 +209,7 @@ EXPORT_SYMBOL(scsi_remove_host);
>   int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev,
>   			   struct device *dma_dev)
>   {
> -	struct scsi_host_template *sht = shost->hostt;
> +	const struct scsi_host_template *sht = shost->hostt;
>   	int error = -EINVAL;
>   
>   	shost_printk(KERN_INFO, shost, "%s\n",
> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c

Re: [PATCH 1/4] scsi: core: constify pointer to scsi_host_template

[Krzysztof Kozlowski]

On 08/04/2022 14:14, John Garry wrote:
> On 08/04/2022 11:30, Krzysztof Kozlowski wrote:
>> Several pointers to 'struct scsi_host_template' do not modify it, so
>> made them const for safety.
>>
> 
> Is this standard practice? What is so special here?

This is standard practice and there is nothing special here. Pointers to
const are preferred because:
1. They add safety if data is actually const. This is not yet the case,
but scsi_host_template allocation could be made const with some effort.
2. The more const variables, the easier function contents and its impact
is to understand. This is actually the biggest benefit when dealing with
code touching different structures.

In general, constifying is a common practice everywhere in the kernel.

Best regards,
Krzysztof

Re: [PATCH 1/4] scsi: core: constify pointer to scsi_host_template

[John Garry]

On 08/04/2022 13:32, Krzysztof Kozlowski wrote:
> On 08/04/2022 14:14, John Garry wrote:
>> On 08/04/2022 11:30, Krzysztof Kozlowski wrote:
>>> Several pointers to 'struct scsi_host_template' do not modify it, so
>>> made them const for safety.
>>>
>> Is this standard practice? What is so special here?
> This is standard practice and there is nothing special here. Pointers to
> const are preferred because:
> 1. They add safety if data is actually const. This is not yet the case,
> but scsi_host_template allocation could be made const with some effort.

To me this seems better, but I think that some drivers might modify 
their scsi_host_template (so not possible)

> 2. The more const variables, the easier function contents and its impact
> is to understand. This is actually the biggest benefit when dealing with
> code touching different structures.
> 
> In general, constifying is a common practice everywhere in the kernel.

Thanks,
John

Re: [PATCH 1/4] scsi: core: constify pointer to scsi_host_template

[Ewan D. Milne]

On Fri, 2022-04-08 at 13:57 +0100, John Garry wrote:
> On 08/04/2022 13:32, Krzysztof Kozlowski wrote:
> > On 08/04/2022 14:14, John Garry wrote:
> > > On 08/04/2022 11:30, Krzysztof Kozlowski wrote:
> > > > Several pointers to 'struct scsi_host_template' do not modify it, so
> > > > made them const for safety.
> > > > 
> > > Is this standard practice? What is so special here?
> > This is standard practice and there is nothing special here. Pointers to
> > const are preferred because:
> > 1. They add safety if data is actually const. This is not yet the case,
> > but scsi_host_template allocation could be made const with some effort.

This seems unlikely, because some drivers, e.g. vmw_pvscsi and scsi_debug,
modify the scsi_host_template based on things like module parameters.

> 
> To me this seems better, but I think that some drivers might modify 
> their scsi_host_template (so not possible)

Several drivers modify scsi_host_template, e.g. .can_queue, .cmd_per_lun

There is also code in lpfc_create_port() that initializes a scsi_host_template
that is embedded in the lpfc_hba struct.  I don't think it gets modified after
scsi_add_host() but it seems like driver maintainers might expect to be able
to do so, in general.

-Ewan

> 
> > 2. The more const variables, the easier function contents and its impact
> > is to understand. This is actually the biggest benefit when dealing with
> > code touching different structures.
> > 
> > In general, constifying is a common practice everywhere in the kernel.
> 
> Thanks,
> John
> 

Re: [PATCH 1/4] scsi: core: constify pointer to scsi_host_template

[John Garry]

On 08/04/2022 20:31, Ewan D. Milne wrote:
> On Fri, 2022-04-08 at 13:57 +0100, John Garry wrote:
>> On 08/04/2022 13:32, Krzysztof Kozlowski wrote:
>>> On 08/04/2022 14:14, John Garry wrote:
>>>> On 08/04/2022 11:30, Krzysztof Kozlowski wrote:
>>>>> Several pointers to 'struct scsi_host_template' do not modify it, so
>>>>> made them const for safety.
>>>>>
>>>> Is this standard practice? What is so special here?
>>> This is standard practice and there is nothing special here. Pointers to
>>> const are preferred because:
>>> 1. They add safety if data is actually const. This is not yet the case,
>>> but scsi_host_template allocation could be made const with some effort.
> 
> This seems unlikely, because some drivers, e.g. vmw_pvscsi and scsi_debug,
> modify the scsi_host_template based on things like module parameters.
>

The standard flow is:

shost = scsi_host_alloc(sht, )

// modify shost, like
shost->cmd_per_lun = 5;

scsi_add_host(shost)

Is there some reason for which those two drivers can't follow that?

>>
>> To me this seems better, but I think that some drivers might modify
>> their scsi_host_template (so not possible)
> 
> Several drivers modify scsi_host_template, e.g. .can_queue, .cmd_per_lun
> 
> There is also code in lpfc_create_port() that initializes a scsi_host_template
> that is embedded in the lpfc_hba struct.  I don't think it gets modified after
> scsi_add_host() but it seems like driver maintainers might expect to be able
> to do so, in general.
> 

Even so, I don't see why other drivers cannot declare their 
scsi_host_template as const. C would have no problem with sht not be 
being const for this:

struct Scsi_Host *scsi_host_alloc(const struct scsi_host_template *sht, )

thanks,
John

Re: [PATCH 1/4] scsi: core: constify pointer to scsi_host_template

[Christoph Hellwig]

On Tue, Apr 12, 2022 at 08:57:39AM +0100, John Garry wrote:
> > 
> 
> The standard flow is:
> 
> shost = scsi_host_alloc(sht, )
> 
> // modify shost, like
> shost->cmd_per_lun = 5;
> 
> scsi_add_host(shost)
> 
> Is there some reason for which those two drivers can't follow that?

I think they should.  Method tables should not be mutable data.

Re: [PATCH 1/4] scsi: core: constify pointer to scsi_host_template

[John Garry]

On 20/04/2022 08:03, Christoph Hellwig wrote:
>> The standard flow is:
>>
>> shost = scsi_host_alloc(sht, )
>>
>> // modify shost, like
>> shost->cmd_per_lun = 5;
>>
>> scsi_add_host(shost)
>>
>> Is there some reason for which those two drivers can't follow that?
> I think they should.  Method tables should not be mutable data.
> .

Hi Krzysztof,

Do you have any interest in going further with your work and trying to 
change all SCSI driver instances of scsi_host_template to be const? I am 
not sure if it has been attempted before...

Thanks,
John

Re: [PATCH 1/4] scsi: core: constify pointer to scsi_host_template

[Krzysztof Kozlowski]

On 25/04/2022 10:58, John Garry wrote:
> On 20/04/2022 08:03, Christoph Hellwig wrote:
>>> The standard flow is:
>>>
>>> shost = scsi_host_alloc(sht, )
>>>
>>> // modify shost, like
>>> shost->cmd_per_lun = 5;
>>>
>>> scsi_add_host(shost)
>>>
>>> Is there some reason for which those two drivers can't follow that?
>> I think they should.  Method tables should not be mutable data.
>> .
> 
> Hi Krzysztof,
> 
> Do you have any interest in going further with your work and trying to 
> change all SCSI driver instances of scsi_host_template to be const? I am 
> not sure if it has been attempted before...

I can work on this, but what about the SCSI core modifying the template?
For example scsi_proc_hostdir_rm(): 'present' and 'proc_dir' members.
Where should they be stored? Should they be moved to the Scsi_Host?


Best regards,
Krzysztof

Re: [PATCH 1/4] scsi: core: constify pointer to scsi_host_template

[John Garry]

On 25/04/2022 10:22, Krzysztof Kozlowski wrote:
> On 25/04/2022 10:58, John Garry wrote:
>> On 20/04/2022 08:03, Christoph Hellwig wrote:
>>>> The standard flow is:
>>>>
>>>> shost = scsi_host_alloc(sht, )
>>>>
>>>> // modify shost, like
>>>> shost->cmd_per_lun = 5;
>>>>
>>>> scsi_add_host(shost)
>>>>
>>>> Is there some reason for which those two drivers can't follow that?
>>> I think they should.  Method tables should not be mutable data.
>>> .
>>
>> Hi Krzysztof,
>>
>> Do you have any interest in going further with your work and trying to
>> change all SCSI driver instances of scsi_host_template to be const? I am
>> not sure if it has been attempted before...
> 
> I can work on this, but what about the SCSI core modifying the template?

I hope that this isn't a can of worms...

> For example scsi_proc_hostdir_rm(): 'present' and 'proc_dir' members.
> Where should they be stored? Should they be moved to the Scsi_Host?
> 

I don't think scsi_Host is appropriate as this is per-scsi host 
template, unless you see a way to do it that way. Alternatively we could 
keep a separate list of registered sht, like this:

struct sht_proc_dir {
	int cnt;
	struct list_head list;
	struct proc_dir_entry *proc_dir;
	struct scsi_host_template *sht;
};
static LIST_HEAD(sht_proc_dir_list);

void scsi_proc_hostdir_add(struct scsi_host_template *sht)
{
	struct sht_proc_dir *dir;

	if (!sht->show_info)
		return;

	mutex_lock(&global_host_template_mutex);
	list_for_each_entry(dir, &sht_proc_dir_list, list) {
		if (dir->sht == sht) {
			dir->cnt++;
			goto out;
		}
	}
	dir = kzalloc(sizeof(*dir), GFP_KERNEL);
	if (!dir)
		goto out;

	dir->proc_dir = proc_mkdir(sht->proc_name, proc_scsi);
	if (!dir->proc_dir) {
		printk(KERN_ERR "%s: proc_mkdir failed for %s\n",
			       __func__, sht->proc_name);
		kfree(dir);
		goto out;
	}

	dir->cnt++;
	list_add_tail(&dir->list, &sht_proc_dir_list);
out:
	mutex_unlock(&global_host_template_mutex);
}

and so on..

--->8---

Thanks,
John

Re: [PATCH 1/4] scsi: core: constify pointer to scsi_host_template

[Bart Van Assche]

On 4/25/22 06:04, John Garry wrote:
> On 25/04/2022 10:22, Krzysztof Kozlowski wrote:
>> For example scsi_proc_hostdir_rm(): 'present' and 'proc_dir' members.
>> Where should they be stored? Should they be moved to the Scsi_Host?
>>
> 
> I don't think scsi_Host is appropriate as this is per-scsi host 
> template, unless you see a way to do it that way. Alternatively we could 
> keep a separate list of registered sht, like this:
> 
> struct sht_proc_dir {
>      int cnt;
>      struct list_head list;
>      struct proc_dir_entry *proc_dir;
>      struct scsi_host_template *sht;
> };
> static LIST_HEAD(sht_proc_dir_list);
> 
> void scsi_proc_hostdir_add(struct scsi_host_template *sht)
> {
>      struct sht_proc_dir *dir;
> 
>      if (!sht->show_info)
>          return;
> 
>      mutex_lock(&global_host_template_mutex);
>      list_for_each_entry(dir, &sht_proc_dir_list, list) {
>          if (dir->sht == sht) {
>              dir->cnt++;
>              goto out;
>          }
>      }
>      dir = kzalloc(sizeof(*dir), GFP_KERNEL);
>      if (!dir)
>          goto out;
> 
>      dir->proc_dir = proc_mkdir(sht->proc_name, proc_scsi);
>      if (!dir->proc_dir) {
>          printk(KERN_ERR "%s: proc_mkdir failed for %s\n",
>                     __func__, sht->proc_name);
>          kfree(dir);
>          goto out;
>      }
> 
>      dir->cnt++;
>      list_add_tail(&dir->list, &sht_proc_dir_list);
> out:
>      mutex_unlock(&global_host_template_mutex);
> }

How about removing scsi_proc_hostdir_add(), scsi_proc_hostdir_rm() and 
all other code that creates files or directories under /proc/scsi? There 
should be corresponding entries in sysfs for all /proc/scsi entries. 
Some tools in sg3_utils use that directory so sg3_utils will have to be 
updated.

Thanks,

Bart.

Re: [PATCH 1/4] scsi: core: constify pointer to scsi_host_template

[Douglas Gilbert]

On 2022-04-25 21:16, Bart Van Assche wrote:
> On 4/25/22 06:04, John Garry wrote:
>> On 25/04/2022 10:22, Krzysztof Kozlowski wrote:
>>> For example scsi_proc_hostdir_rm(): 'present' and 'proc_dir' members.
>>> Where should they be stored? Should they be moved to the Scsi_Host?
>>>
>>
>> I don't think scsi_Host is appropriate as this is per-scsi host template, 
>> unless you see a way to do it that way. Alternatively we could keep a separate 
>> list of registered sht, like this:
>>
>> struct sht_proc_dir {
>>      int cnt;
>>      struct list_head list;
>>      struct proc_dir_entry *proc_dir;
>>      struct scsi_host_template *sht;
>> };
>> static LIST_HEAD(sht_proc_dir_list);
>>
>> void scsi_proc_hostdir_add(struct scsi_host_template *sht)
>> {
>>      struct sht_proc_dir *dir;
>>
>>      if (!sht->show_info)
>>          return;
>>
>>      mutex_lock(&global_host_template_mutex);
>>      list_for_each_entry(dir, &sht_proc_dir_list, list) {
>>          if (dir->sht == sht) {
>>              dir->cnt++;
>>              goto out;
>>          }
>>      }
>>      dir = kzalloc(sizeof(*dir), GFP_KERNEL);
>>      if (!dir)
>>          goto out;
>>
>>      dir->proc_dir = proc_mkdir(sht->proc_name, proc_scsi);
>>      if (!dir->proc_dir) {
>>          printk(KERN_ERR "%s: proc_mkdir failed for %s\n",
>>                     __func__, sht->proc_name);
>>          kfree(dir);
>>          goto out;
>>      }
>>
>>      dir->cnt++;
>>      list_add_tail(&dir->list, &sht_proc_dir_list);
>> out:
>>      mutex_unlock(&global_host_template_mutex);
>> }
> 
> How about removing scsi_proc_hostdir_add(), scsi_proc_hostdir_rm() and all other 
> code that creates files or directories under /proc/scsi? There should be 
> corresponding entries in sysfs for all /proc/scsi entries. Some tools in 
> sg3_utils use that directory so sg3_utils will have to be updated.

... breaking this:

~$ cat /proc/scsi/scsi

Attached devices:

Host: scsi3 Channel: 00 Id: 00 Lun: 00

   Vendor: IBM-207x Model: HUSMM8020ASS20   Rev: J4B6

   Type:   Direct-Access                    ANSI  SCSI revision: 06

Host: scsi3 Channel: 00 Id: 01 Lun: 00

   Vendor: IBM-207x Model: HUSMM8020ASS20   Rev: J4B6

   Type:   Direct-Access                    ANSI  SCSI revision: 06

Host: scsi3 Channel: 00 Id: 02 Lun: 00

   Vendor: SEAGATE  Model: ST200FM0073      Rev: 0007

   Type:   Direct-Access                    ANSI  SCSI revision: 06
...

A deprecation notice would be helpful, then removal after a few kernel
cycles. Yes, lsscsi can give that output:

$ lsscsi -c

Attached devices:

Host: scsi2 Channel: 00 Target: 00 Lun: 00

   Vendor: SEAGATE  Model: ST200FM0073      Rev: 0007

   Type:   Direct-Access                    ANSI SCSI revision: 06

Host: scsi2 Channel: 00 Target: 01 Lun: 00

   Vendor: WDC      Model: WSH722020AL5204  Rev: C421

   Type:   Zoned Block                      ANSI SCSI revision: 07

Host: scsi2 Channel: 00 Target: 02 Lun: 00

   Vendor: Areca Te Model: ARC-802801.37.69 Rev: 0137

   Type:   Enclosure                        ANSI SCSI revision: 05
...

[Hmmm, in a different order.]

However no distribution that I'm aware of includes lsscsi in its installation.
[Most recent example: Ubuntu 22.04]
Linux is not alone ... in FreeBSD how do you list SCSI devices in your
system? Answer: as root you invoke 'camcontrol devlist', it's so obvious.

Perhaps the Linux kernel could have a deprecation process which uses inotify
or similar to notice accesses to /proc/scsi/scsi (say) and print out
a helpful response along the lines" "this is no longer supported, try using
the lsscsi utility".

Doug Gilbert

Re: [PATCH 1/4] scsi: core: constify pointer to scsi_host_template

[Bart Van Assche]

On 4/25/22 18:54, Douglas Gilbert wrote:
> On 2022-04-25 21:16, Bart Van Assche wrote:
>> How about removing scsi_proc_hostdir_add(), scsi_proc_hostdir_rm() and 
>> all other code that creates files or directories under /proc/scsi? 
>> There should be corresponding entries in sysfs for all /proc/scsi 
>> entries. Some tools in sg3_utils use that directory so sg3_utils will 
>> have to be updated.
> 
> ... breaking this:
> 
> ~$ cat /proc/scsi/scsi
> 
> Attached devices:
> 
> Host: scsi3 Channel: 00 Id: 00 Lun: 00
> 
>    Vendor: IBM-207x Model: HUSMM8020ASS20   Rev: J4B6
> 
>    Type:   Direct-Access                    ANSI  SCSI revision: 06
> 
> Host: scsi3 Channel: 00 Id: 01 Lun: 00
> 
>    Vendor: IBM-207x Model: HUSMM8020ASS20   Rev: J4B6
> 
>    Type:   Direct-Access                    ANSI  SCSI revision: 06
> 
> Host: scsi3 Channel: 00 Id: 02 Lun: 00
> 
>    Vendor: SEAGATE  Model: ST200FM0073      Rev: 0007
> 
>    Type:   Direct-Access                    ANSI  SCSI revision: 06
> ...
> 
> A deprecation notice would be helpful, then removal after a few kernel
> cycles.

Agreed with the deprecation notice + delayed removal, but is anyone 
using cat /proc/scsi/scsi?

> Yes, lsscsi can give that output:
> 
> $ lsscsi -c
> 
> Attached devices:
> 
> Host: scsi2 Channel: 00 Target: 00 Lun: 00
> 
>    Vendor: SEAGATE  Model: ST200FM0073      Rev: 0007
> 
>    Type:   Direct-Access                    ANSI SCSI revision: 06
> 
> Host: scsi2 Channel: 00 Target: 01 Lun: 00
> 
>    Vendor: WDC      Model: WSH722020AL5204  Rev: C421
> 
>    Type:   Zoned Block                      ANSI SCSI revision: 07
> 
> Host: scsi2 Channel: 00 Target: 02 Lun: 00
> 
>    Vendor: Areca Te Model: ARC-802801.37.69 Rev: 0137
> 
>    Type:   Enclosure                        ANSI SCSI revision: 05
> ...
> 
> [Hmmm, in a different order.]
> 
> However no distribution that I'm aware of includes lsscsi in its 
> installation.
> [Most recent example: Ubuntu 22.04]

Hmm ... are you sure? Last time I looked into this an lsscsi package was 
available for every distro I tried (RHEL, SLES, Debian and openSUSE). 
See also 
https://packages.debian.org/search?searchon=contents&keywords=lsscsi&mode=path&suite=stable&arch=any.

Are there other utilities in sg3_utils that would break if the 
/proc/scsi directory would be removed?

$ cd sg3_utils && git grep /proc/scsi | wc -l
51

Thanks,

Bart.

Re: [PATCH 1/4] scsi: core: constify pointer to scsi_host_template

[Douglas Gilbert]

On 2022-04-26 00:13, Bart Van Assche wrote:
> On 4/25/22 18:54, Douglas Gilbert wrote:
>> On 2022-04-25 21:16, Bart Van Assche wrote:
>>> How about removing scsi_proc_hostdir_add(), scsi_proc_hostdir_rm() and all 
>>> other code that creates files or directories under /proc/scsi? There should 
>>> be corresponding entries in sysfs for all /proc/scsi entries. Some tools in 
>>> sg3_utils use that directory so sg3_utils will have to be updated.
>>
>> ... breaking this:
>>
>> ~$ cat /proc/scsi/scsi
>>
>> Attached devices:
>>
>> Host: scsi3 Channel: 00 Id: 00 Lun: 00
>>    Vendor: IBM-207x Model: HUSMM8020ASS20   Rev: J4B6
>>    Type:   Direct-Access                    ANSI  SCSI revision: 06
>> Host: scsi3 Channel: 00 Id: 01 Lun: 00
>>    Vendor: IBM-207x Model: HUSMM8020ASS20   Rev: J4B6
>>    Type:   Direct-Access                    ANSI  SCSI revision: 06
>> Host: scsi3 Channel: 00 Id: 02 Lun: 00
>>    Vendor: SEAGATE  Model: ST200FM0073      Rev: 0007
>>    Type:   Direct-Access                    ANSI  SCSI revision: 06
>> ...
>>
>> A deprecation notice would be helpful, then removal after a few kernel
>> cycles.
> 
> Agreed with the deprecation notice + delayed removal, but is anyone using cat 
> /proc/scsi/scsi?
> 
>> Yes, lsscsi can give that output:
>>
>> $ lsscsi -c
>> Attached devices:
>> Host: scsi2 Channel: 00 Target: 00 Lun: 00
>>    Vendor: SEAGATE  Model: ST200FM0073      Rev: 0007
>>    Type:   Direct-Access                    ANSI SCSI revision: 06
>> Host: scsi2 Channel: 00 Target: 01 Lun: 00
>>    Vendor: WDC      Model: WSH722020AL5204  Rev: C421
>>    Type:   Zoned Block                      ANSI SCSI revision: 07
>> Host: scsi2 Channel: 00 Target: 02 Lun: 00
>>    Vendor: Areca Te Model: ARC-802801.37.69 Rev: 0137
>>    Type:   Enclosure                        ANSI SCSI revision: 05
>> ...
>>
>> [Hmmm, in a different order.]
>>
>> However no distribution that I'm aware of includes lsscsi in its installation.
>> [Most recent example: Ubuntu 22.04]
> 
> Hmm ... are you sure? Last time I looked into this an lsscsi package was 
> available for every distro I tried (RHEL, SLES, Debian and openSUSE). See also 
> https://packages.debian.org/search?searchon=contents&keywords=lsscsi&mode=path&suite=stable&arch=any.

I was talking about the _initial_ installation. When I install new versions
of Fedora or Ubuntu, or play with a "live" CD (usually a USB stick) one
of the first things I do is get a terminal and then invoke 'lsscsi'.
Invariably that second step fails. And on a "live" USB stick you can install
lsscsi but the next time you use it, lsscsi is gone because those "live"
USB sticks hardly ever have persistent storage set up. [Why not? ..
typically the rest of the storage on such a USB stick is un-utilized.]

> Are there other utilities in sg3_utils that would break if the /proc/scsi 
> directory would be removed?
> 
> $ cd sg3_utils && git grep /proc/scsi | wc -l
> 51

Most of those are in the scripts/rescan-scsi-bus.sh which, judging from the
number of patches and additions it gets, has quite a bit of use out there.
The rest are in my dd variants that are mainly setting /proc/scsi/sg/allow_dio
which has no effect in my sg driver rewrite.

Doug Gilbert

Re: [PATCH 1/4] scsi: core: constify pointer to scsi_host_template

[Krzysztof Kozlowski]

On 25/04/2022 15:04, John Garry wrote:
> 
>> For example scsi_proc_hostdir_rm(): 'present' and 'proc_dir' members.
>> Where should they be stored? Should they be moved to the Scsi_Host?
>>
> 
> I don't think scsi_Host is appropriate as this is per-scsi host 
> template, unless you see a way to do it that way. Alternatively we could 
> keep a separate list of registered sht, like this:
> 
> struct sht_proc_dir {
> 	int cnt;
> 	struct list_head list;
> 	struct proc_dir_entry *proc_dir;
> 	struct scsi_host_template *sht;
> };
> static LIST_HEAD(sht_proc_dir_list);

Hi everyone,

It took me some time to get back to this topic. I moved the proc_dir out
of SHT, how John proposed. Patches do not look that bad:
The commit:
https://github.com/krzk/linux/commit/157eb2ee8867afbae9dac3836e4c0bedb542e5c1

Branch:
https://github.com/krzk/linux/commits/n/qcom-ufs-opp-cleanups-v2

However this does not solve the problem. The SHT has "module" which gets
incremented/decremented. Exactly like in case of other drivers
(driver->owner).

I started moving the SHT->module to a new field scsi_host->owner and
trying to use the parent's driver (so PCI, USB) owner.
I am not sure if it is correct approach, so before implementing such big
change affecting multiple subsystems (USB, ATA, SCSI) - can you share
ideas/opinion?

The Work-in-Progress looks like this (last commit):
https://github.com/krzk/linux/commit/17609caecd53df20f631703ea084a70e7735b5d7


Best regards,
Krzysztof

Re: [PATCH 1/4] scsi: core: constify pointer to scsi_host_template

[John Garry]

On 06/05/2022 17:42, Krzysztof Kozlowski wrote:
>> I don't think scsi_Host is appropriate as this is per-scsi host
>> template, unless you see a way to do it that way. Alternatively we could
>> keep a separate list of registered sht, like this:
>>
>> struct sht_proc_dir {
>> 	int cnt;
>> 	struct list_head list;
>> 	struct proc_dir_entry *proc_dir;
>> 	struct scsi_host_template *sht;
>> };
>> static LIST_HEAD(sht_proc_dir_list);
> Hi everyone,
> 

Hi Krzysztof,

> It took me some time to get back to this topic. I moved the proc_dir out
> of SHT, how John proposed. Patches do not look that bad:
> The commit:
> https://github.com/krzk/linux/commit/157eb2ee8867afbae9dac3836e4c0bedb542e5c1
> 

For some reason I cannot fetch your git due to "error: RPC failed ..." 
which I think is a timeout. I seem to have this problem recently 
whenever a linux.git clone has branches based on linux-next.git . Maybe 
a git config issue for me...

> Branch:
> https://github.com/krzk/linux/commits/n/qcom-ufs-opp-cleanups-v2
> 
> However this does not solve the problem. The SHT has "module" which gets
> incremented/decremented. Exactly like in case of other drivers
> (driver->owner).

Ah, I missed that this could be a problem. So we have this member to 
stop the SCSI host driver being removed when we have disks mounted, etc.

But isn't scsi_host_template.module just a pointer to the local driver 
module data (and that data gets incremented/decremented)? I am looking 
at the THIS_MODULE definition in export.h:

extern stuct module __this_module;
#define THIS_MODULE(&__this_module)

However I do see scsi_device_dev_release(), which does:

sdp->host->hostt->module = NULL

I am not sure how necessary that really is. I would need to check further.

Did you see if there other places which set hostt->module dynamically?

> 
> I started moving the SHT->module to a new field scsi_host->owner and
> trying to use the parent's driver (so PCI, USB) owner.
> I am not sure if it is correct approach, so before implementing such big
> change affecting multiple subsystems (USB, ATA, SCSI) - can you share
> ideas/opinion?
> 
> The Work-in-Progress looks like this (last commit):
> https://github.com/krzk/linux/commit/17609caecd53df20f631703ea084a70e7735b5d7

Thanks,
John

Re: [PATCH 1/4] scsi: core: constify pointer to scsi_host_template

[Krzysztof Kozlowski]

On 09/05/2022 13:28, John Garry wrote:
> 
> For some reason I cannot fetch your git due to "error: RPC failed ..." 
> which I think is a timeout. I seem to have this problem recently 
> whenever a linux.git clone has branches based on linux-next.git . Maybe 
> a git config issue for me...

Just to be sure - the link was not a git remote, but direct link for a
web browser. The repo is:
https://github.com/krzk/linux.git
branch: n/qcom-ufs-opp-cleanups-v2

>> However this does not solve the problem. The SHT has "module" which gets
>> incremented/decremented. Exactly like in case of other drivers
>> (driver->owner).
> 
> Ah, I missed that this could be a problem. So we have this member to 
> stop the SCSI host driver being removed when we have disks mounted, etc.
> 
> But isn't scsi_host_template.module just a pointer to the local driver 
> module data (and that data gets incremented/decremented)? I am looking 
> at the THIS_MODULE definition in export.h:

Yes, it is just a pointer, just like 'struct device_driver.owner' is.

> 
> extern stuct module __this_module;
> #define THIS_MODULE(&__this_module)
> 
> However I do see scsi_device_dev_release(), which does:
> 
> sdp->host->hostt->module = NULL
> 
> I am not sure how necessary that really is. I would need to check further.

> 
> Did you see if there other places which set hostt->module dynamically?

I think all SHT set it statically. Then it is being dynamically
incremented when needed and in scsi_device_dev_release() is being
nullified (as you mentioned above).

I guess this SHT->module is actually duplicating what most of drivers
(e.g. PCI, platform, USB) are doing. If I understand correctly, the
Scsi_Host is instantiated by the probe of a driver (pci_driver,
virtio_driver etc), therefore the SHT->module could be simply stored in
Scsi_Host.


>>
>> I started moving the SHT->module to a new field scsi_host->owner and
>> trying to use the parent's driver (so PCI, USB) owner.
>> I am not sure if it is correct approach, so before implementing such big
>> change affecting multiple subsystems (USB, ATA, SCSI) - can you share
>> ideas/opinion?
>>
>> The Work-in-Progress looks like this (last commit):
>> https://github.com/krzk/linux/commit/17609caecd53df20f631703ea084a70e7735b5d7



Best regards,
Krzysztof

Re: [PATCH 1/4] scsi: core: constify pointer to scsi_host_template

[John Garry]

On 09/05/2022 14:20, Krzysztof Kozlowski wrote:
> On 09/05/2022 13:28, John Garry wrote:
>>
>> For some reason I cannot fetch your git due to "error: RPC failed ..."
>> which I think is a timeout. I seem to have this problem recently
>> whenever a linux.git clone has branches based on linux-next.git . Maybe
>> a git config issue for me...
> 
> Just to be sure - the link was not a git remote, but direct link for a
> web browser. The repo is:
> https://github.com/krzk/linux.git
> branch: n/qcom-ufs-opp-cleanups-v2
> 

OK, I'll double check. Regardless I'll sort my own IT issues :)

>>> However this does not solve the problem. The SHT has "module" which gets
>>> incremented/decremented. Exactly like in case of other drivers
>>> (driver->owner).
>>
>> Ah, I missed that this could be a problem. So we have this member to
>> stop the SCSI host driver being removed when we have disks mounted, etc.
>>
>> But isn't scsi_host_template.module just a pointer to the local driver
>> module data (and that data gets incremented/decremented)? I am looking
>> at the THIS_MODULE definition in export.h:
> 
> Yes, it is just a pointer, just like 'struct device_driver.owner' is.
> 
>>
>> extern stuct module __this_module;
>> #define THIS_MODULE(&__this_module)
>>
>> However I do see scsi_device_dev_release(), which does:
>>
>> sdp->host->hostt->module = NULL
>>
>> I am not sure how necessary that really is. I would need to check further.
> 
>>
>> Did you see if there other places which set hostt->module dynamically?
> 
> I think all SHT set it statically. 

Incidentally I did notice that the ata ahci driver does not set sht->module.

> Then it is being dynamically
> incremented when needed and in scsi_device_dev_release() is being
> nullified (as you mentioned above).
> 
> I guess this SHT->module is actually duplicating what most of drivers
> (e.g. PCI, platform, USB) are doing. If I understand correctly, the
> Scsi_Host is instantiated by the probe of a driver (pci_driver,
> virtio_driver etc), therefore the SHT->module could be simply stored in
> Scsi_Host.

If you check scsi_device_dev_release(), we try to do a 'get' - if it 
fails, then we nullify hostt->module. I think that is important as then 
we call execute_in_process_context(), whose worker does the 'put'. 
However, the 'put' will get upset if the refcnt was 0, which it would be 
if the earlier 'get' fails - hence the nullify is to avoid that 
possibility. So whatever you do needs to handle that. Details are in 
f2b85040

Thanks,
john

Re: [PATCH 1/4] scsi: core: constify pointer to scsi_host_template

[Christoph Hellwig]

On Mon, May 09, 2022 at 03:50:33PM +0100, John Garry wrote:
> If you check scsi_device_dev_release(), we try to do a 'get' - if it fails,
> then we nullify hostt->module. I think that is important as then we call
> execute_in_process_context(), whose worker does the 'put'. However, the
> 'put' will get upset if the refcnt was 0, which it would be if the earlier
> 'get' fails - hence the nullify is to avoid that possibility. So whatever
> you do needs to handle that. Details are in f2b85040

Yikes, that code is completely and utterly buggy and does not account
for all the cases why try_module_get can fail.  I think we always have
a reference here and could use __module_get, but what we have is
certainly unsafe and a good reason why the host template should be
constifyed.