Re: [PATCH 0/5] Modify ida_* users to use ida_simple_*

[Tejun Heo]

On Thu, Oct 01, 2015 at 11:59:04AM -0700, Lee Duncan wrote:
> The ida index management routines are used in several
> driver modules to manage allocation and release of
> index values. Reviewing the way in which the
> ida routines were called, together with the small
> number of such clients, led to the belief that
> these users should all be able to share a simple
> built-in lock in the ida module by calling the
> ida_simple_*() functions instead of the non-simple
> versions. This means that ida does all the
> required locking so that clients don't have to
> manage that.

The whole series looks good to me.  Please feel free to add

Reviewed-by: Tejun Heo 

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] Modify ida_* users to use ida_simple_*

[James Bottomley]

On Mon, 2015-10-05 at 13:44 -0400, Tejun Heo wrote:
> On Thu, Oct 01, 2015 at 11:59:04AM -0700, Lee Duncan wrote:
> > The ida index management routines are used in several
> > driver modules to manage allocation and release of
> > index values. Reviewing the way in which the
> > ida routines were called, together with the small
> > number of such clients, led to the belief that
> > these users should all be able to share a simple
> > built-in lock in the ida module by calling the
> > ida_simple_*() functions instead of the non-simple
> > versions. This means that ida does all the
> > required locking so that clients don't have to
> > manage that.
> 
> The whole series looks good to me.  Please feel free to add

Since they're all independent, they can go via the correct trees without
adverse consequences.  It's probably me: 1/5; Jens 2-4/5; Greg 5/5

James


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH for v4.3-rc] scsi_dh: fix use-after-free when removing scsi device

[Junichi Nomura]

The commit 1bab0de0274f ("dm-mpath, scsi_dh: don't let dm detach device
handlers") removed reference counting of attached scsi device handler.
As a result, handler data is freed immediately via scsi_dh->detach()
in the context of scsi_remove_device() where activation request can be
still in flight.

This patch moves scsi_dh_handler_detach() to sdev releasing function,
scsi_device_dev_release_usercontext(), at that point the device
is already in quiesced state.

Fixes: 1bab0de0274f ("dm-mpath, scsi_dh: don't let dm detach device handlers")
Signed-off-by: Jun'ichi Nomura 
Acked-by: Christoph Hellwig 
---
 drivers/scsi/scsi_dh.c| 6 +-
 drivers/scsi/scsi_priv.h  | 2 ++
 drivers/scsi/scsi_sysfs.c | 2 ++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_dh.c b/drivers/scsi/scsi_dh.c
index edb044a..19bf9db 100644
--- a/drivers/scsi/scsi_dh.c
+++ b/drivers/scsi/scsi_dh.c
@@ -232,10 +232,14 @@ int scsi_dh_add_device(struct scsi_device *sdev)
return err;
 }
 
-void scsi_dh_remove_device(struct scsi_device *sdev)
+void scsi_dh_release_device(struct scsi_device *sdev)
 {
if (sdev->handler)
scsi_dh_handler_detach(sdev);
+}
+
+void scsi_dh_remove_device(struct scsi_device *sdev)
+{
device_remove_file(>sdev_gendev, _dh_state_attr);
 }
 
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index 644bb73..4d01cdb 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -173,9 +173,11 @@ extern struct async_domain scsi_sd_probe_domain;
 /* scsi_dh.c */
 #ifdef CONFIG_SCSI_DH
 int scsi_dh_add_device(struct scsi_device *sdev);
+void scsi_dh_release_device(struct scsi_device *sdev);
 void scsi_dh_remove_device(struct scsi_device *sdev);
 #else
 static inline int scsi_dh_add_device(struct scsi_device *sdev) { return 0; }
+static inline void scsi_dh_release_device(struct scsi_device *sdev) { }
 static inline void scsi_dh_remove_device(struct scsi_device *sdev) { }
 #endif
 
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index b89..dff8faf 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -399,6 +399,8 @@ static void scsi_device_dev_release_usercontext(struct 
work_struct *work)
 
sdev = container_of(work, struct scsi_device, ew.work);
 
+   scsi_dh_release_device(sdev);
+
parent = sdev->sdev_gendev.parent;
 
spin_lock_irqsave(sdev->host->host_lock, flags);
-- 
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 13/13] scsi: ufs: Add exynos ufs platform data

[Alim Akhtar]

CCing Rob Herring,

Hi Arnd,

On 10/01/2015 04:59 PM, Arnd Bergmann wrote:

On Thursday 01 October 2015 18:46:34 kbuild test robot wrote:

[auto build test results on v4.3-rc3 -- if it's inappropriate base, please 
ignore]

config: x86_64-allmodconfig (attached as .config)
reproduce:
 git checkout 6e153e3bf7c68b019e987c5a0ffadebd9c7d4fbb
 # save the attached .config to linux build tree
 make ARCH=x86_64

All error/warnings (new ones prefixed by >>):


ERROR: "ufs_hba_exynos_ops" [drivers/scsi/ufs/ufshcd-pltfrm.ko] undefined!





Ah, this seems to be a case of layering violation. It would be best to
restructure the code so that the exynos driver registers a platform_driver
by itself for the respective DT compatible string, and then calls
into the common code from its probe function, rather than having the
generic driver know about the specific backends.

That approach will also make the generic driver more scalable as we
add further chip-specific variations, and matches what we do in other
drivers.



Looks like some discussions on ufs variant driver probe method happened 
here [1] few months back.

[1]-> https://lkml.org/lkml/2015/6/3/180

And since ufshcd-pltfrm is already a platform_driver, so I just add a 
platform data for the variant driver.
I should have add a IS_ENABLED for it to avoid the compilation error for 
other ARCH.



Thanks!!


Arnd


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFT v3] eata: Convert eata driver as normal PCI and platform device drivers

[Arthur Marsh]

Jiang Liu wrote on 03/10/15 17:41:


If I do a normal boot which includes eata being loaded, the disk
attached to the DPT2044W controller having its filesystems checked and
mounted, then attempt a kexec reboot, I get the reboot pausing after the
"synchronizing SCSI cache" messages as before.

If I un-mount the filesystems on the disk attached to the DPT2044W
controller after start-up and try a reboot I get the same problem.

If I do modprobe -r eata after un-mounting the filesystems on the disk
attached to the DPT2044W controller after a start-up kexec *works fine*.

Hi Arthur,
The above results suggest that we need to shutdown eata
controller for kexec. So could you please try to apply the attached
patch upon the previous two patches?
Thanks!
Gerry



To clarify, if the eata driver gets loaded once and stays loaded, at a 
kexec reboot attempt the "Synchronising SCSI cache" message is missing 
for the SCSI disk attached to the controller using the eata driver and 
eventually other error messages appear as seen in screen images that I 
have previously posted.


If the eata driver is loaded, unloaded via modprobe -r, then reloaded, a 
kexec reboot shows 2 "Synchronising SCSI cache" messages for the SCSI 
disk attached to the controller using the eata driver and the kexec 
reboot is successful.



Arthur.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/2] scsi: reduce CONFIG_SCSI_CONSTANTS=y impact by 8k

[Rasmus Villemoes]

This reduces the impact of choosing CONFIG_SCSI_CONSTANTS by about 8KB.

2dd951ecd511 ("scsi: Conditionally compile in constants.c") updated
the Kconfig help text from 12KB to 75KB. The 12K predated git so was
certainly outdated. But I'm not sure where the 75K comes from; using
size(1) on a defconfig (with/without this config option) vmlinux shows
a difference of about 47K, and 39K after these patches are applied. In
any case, I've left the Kconfig text alone, since I'm not sure I'm
counting the same way the 75K was computed (I'm fairly certain of the
8K delta, however).

Tested with a trivial module calling scsi_extd_sense_format with a few
random known codes and comparing the result to the expected value.

Rasmus Villemoes (2):
  scsi: move Additional Sense Codes to separate file
  scsi: reduce CONFIG_SCSI_CONSTANTS=y impact by 8k

 drivers/scsi/constants.c   | 860 ++---
 drivers/scsi/sense_codes.h | 835 +++
 2 files changed, 857 insertions(+), 838 deletions(-)
 create mode 100644 drivers/scsi/sense_codes.h

-- 
2.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] scsi: reduce CONFIG_SCSI_CONSTANTS=y impact by 8k

[Rasmus Villemoes]

On 64 bit, struct error_info has 6 bytes of padding, which amounts to
over 4k of wasted space in the additional[] array. We could easily get
rid of that by instead using separate arrays for the codes and the
pointers. However, we can do even better than that and save an
additional 6 bytes per entry: In the table, just store the sizeof()
the corresponding string literal. The cumulative sum of these is then
the appropriate offset into additional_text, which is built from the
concatenation (with '\0's inbetween) of the strings.

$ scripts/bloat-o-meter /tmp/vmlinux vmlinux
add/remove: 0/0 grow/shrink: 1/1 up/down: 24/-8488 (-8464)
function old new   delta
scsi_extd_sense_format   136 160 +24
additional 113122824   -8488

Signed-off-by: Rasmus Villemoes 
---
 drivers/scsi/constants.c   | 25 +
 drivers/scsi/sense_codes.h |  2 --
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 47aaccd5e68e..ccd34b0481cd 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -292,17 +292,31 @@ bool scsi_opcode_sa_name(int opcode, int service_action,
 
 struct error_info {
unsigned short code12;  /* 0x0302 looks better than 0x03,0x02 */
-   const char * text;
+   unsigned short size;
 };
 
 
+/*
+ * There are 700+ entries in this table. To save space, we don't store
+ * (code, pointer) pairs, which would make sizeof(struct
+ * error_info)==16 on 64 bits. Rather, the second element just stores
+ * the size (including \0) of the corresponding string, and we use the
+ * sum of these to get the appropriate offset into additional_text
+ * defined below. This approach saves 12 bytes per entry.
+ */
 static const struct error_info additional[] =
 {
-#define SENSE_CODE(c, s) {c, s},
+#define SENSE_CODE(c, s) {c, sizeof(s)},
 #include "sense_codes.h"
 #undef SENSE_CODE
 };
 
+static const char *additional_text =
+#define SENSE_CODE(c, s) s "\0"
+#include "sense_codes.h"
+#undef SENSE_CODE
+   ;
+
 struct error_info2 {
unsigned char code1, code2_min, code2_max;
const char * str;
@@ -364,11 +378,14 @@ scsi_extd_sense_format(unsigned char asc, unsigned char 
ascq, const char **fmt)
 {
int i;
unsigned short code = ((asc << 8) | ascq);
+   unsigned offset = 0;
 
*fmt = NULL;
-   for (i = 0; additional[i].text; i++)
+   for (i = 0; i < ARRAY_SIZE(additional); i++) {
if (additional[i].code12 == code)
-   return additional[i].text;
+   return additional_text + offset;
+   offset += additional[i].size;
+   }
for (i = 0; additional2[i].fmt; i++) {
if (additional2[i].code1 == asc &&
ascq >= additional2[i].code2_min &&
diff --git a/drivers/scsi/sense_codes.h b/drivers/scsi/sense_codes.h
index 54b3939d6309..da84d53b3379 100644
--- a/drivers/scsi/sense_codes.h
+++ b/drivers/scsi/sense_codes.h
@@ -833,5 +833,3 @@ SENSE_CODE(0x746E, "External data encryption control 
timeout")
 SENSE_CODE(0x746F, "External data encryption control error")
 SENSE_CODE(0x7471, "Logical unit access not authorized")
 SENSE_CODE(0x7479, "Security conflict in translated device")
-
-SENSE_CODE(0, NULL)
-- 
2.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] scsi: move Additional Sense Codes to separate file

[Rasmus Villemoes]

This is a purely mechanical move of the list of additional sense codes
to a separate file, in preparation for reducing the impact of choosing
CONFIG_SCSI_CONSTANTS=y by about 8k..

Signed-off-by: Rasmus Villemoes 
---
 drivers/scsi/constants.c   | 839 +
 drivers/scsi/sense_codes.h | 837 
 2 files changed, 840 insertions(+), 836 deletions(-)
 create mode 100644 drivers/scsi/sense_codes.h

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index fa09d4be2b53..47aaccd5e68e 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -295,845 +295,12 @@ struct error_info {
const char * text;
 };
 
-/*
- * The canonical list of T10 Additional Sense Codes is available at:
- * http://www.t10.org/lists/asc-num.txt [most recent: 20141221]
- */
 
 static const struct error_info additional[] =
 {
-   {0x, "No additional sense information"},
-   {0x0001, "Filemark detected"},
-   {0x0002, "End-of-partition/medium detected"},
-   {0x0003, "Setmark detected"},
-   {0x0004, "Beginning-of-partition/medium detected"},
-   {0x0005, "End-of-data detected"},
-   {0x0006, "I/O process terminated"},
-   {0x0007, "Programmable early warning detected"},
-   {0x0011, "Audio play operation in progress"},
-   {0x0012, "Audio play operation paused"},
-   {0x0013, "Audio play operation successfully completed"},
-   {0x0014, "Audio play operation stopped due to error"},
-   {0x0015, "No current audio status to return"},
-   {0x0016, "Operation in progress"},
-   {0x0017, "Cleaning requested"},
-   {0x0018, "Erase operation in progress"},
-   {0x0019, "Locate operation in progress"},
-   {0x001A, "Rewind operation in progress"},
-   {0x001B, "Set capacity operation in progress"},
-   {0x001C, "Verify operation in progress"},
-   {0x001D, "ATA pass through information available"},
-   {0x001E, "Conflicting SA creation request"},
-   {0x001F, "Logical unit transitioning to another power condition"},
-   {0x0020, "Extended copy information available"},
-   {0x0021, "Atomic command aborted due to ACA"},
-
-   {0x0100, "No index/sector signal"},
-
-   {0x0200, "No seek complete"},
-
-   {0x0300, "Peripheral device write fault"},
-   {0x0301, "No write current"},
-   {0x0302, "Excessive write errors"},
-
-   {0x0400, "Logical unit not ready, cause not reportable"},
-   {0x0401, "Logical unit is in process of becoming ready"},
-   {0x0402, "Logical unit not ready, initializing command required"},
-   {0x0403, "Logical unit not ready, manual intervention required"},
-   {0x0404, "Logical unit not ready, format in progress"},
-   {0x0405, "Logical unit not ready, rebuild in progress"},
-   {0x0406, "Logical unit not ready, recalculation in progress"},
-   {0x0407, "Logical unit not ready, operation in progress"},
-   {0x0408, "Logical unit not ready, long write in progress"},
-   {0x0409, "Logical unit not ready, self-test in progress"},
-   {0x040A, "Logical unit not accessible, asymmetric access state "
-"transition"},
-   {0x040B, "Logical unit not accessible, target port in standby state"},
-   {0x040C, "Logical unit not accessible, target port in unavailable "
-"state"},
-   {0x040D, "Logical unit not ready, structure check required"},
-   {0x040E, "Logical unit not ready, security session in progress"},
-   {0x0410, "Logical unit not ready, auxiliary memory not accessible"},
-   {0x0411, "Logical unit not ready, notify (enable spinup) required"},
-   {0x0412, "Logical unit not ready, offline"},
-   {0x0413, "Logical unit not ready, SA creation in progress"},
-   {0x0414, "Logical unit not ready, space allocation in progress"},
-   {0x0415, "Logical unit not ready, robotics disabled"},
-   {0x0416, "Logical unit not ready, configuration required"},
-   {0x0417, "Logical unit not ready, calibration required"},
-   {0x0418, "Logical unit not ready, a door is open"},
-   {0x0419, "Logical unit not ready, operating in sequential mode"},
-   {0x041A, "Logical unit not ready, start stop unit command in "
-"progress"},
-   {0x041B, "Logical unit not ready, sanitize in progress"},
-   {0x041C, "Logical unit not ready, additional power use not yet "
-"granted"},
-   {0x041D, "Logical unit not ready, configuration in progress"},
-   {0x041E, "Logical unit not ready, microcode activation required"},
-   {0x041F, "Logical unit not ready, microcode download required"},
-   {0x0420, "Logical unit not ready, logical unit reset required"},
-   {0x0421, "Logical unit not ready, hard reset required"},
-   {0x0422, "Logical unit not ready, power cycle required"},
-
-   {0x0500, "Logical unit does not respond to selection"},

Re: [PATCH] 3w-9xxx: don't unmap bounce buffered commands

[adam radford]

On Sat, Oct 3, 2015 at 10:16 AM, Christoph Hellwig  wrote:
> 3w controller don't dma map small single SGL entry commands but instead
> bounce buffer them.  Add a helper to identify these commands and don't
> call scsi_dma_unmap for them.
>
> Based on an earlier patch from James Bottomley.
>
> Fixes: 118c85 ("3w-9xxx: fix command completion race")
> Reported-by: T??th Attila 
> Tested-by: T??th Attila 
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/scsi/3w-9xxx.c | 28 +---
>  1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
> index add419d..a56a7b2 100644
> --- a/drivers/scsi/3w-9xxx.c
> +++ b/drivers/scsi/3w-9xxx.c
> @@ -212,6 +212,17 @@ static const struct file_operations twa_fops = {
> .llseek = noop_llseek,
>  };
>
> +/*
> + * The controllers use an inline buffer instead of a mapped SGL for small,
> + * single entry buffers.  Note that we treat a zero-length transfer like
> + * a mapped SGL.
> + */
> +static bool twa_command_mapped(struct scsi_cmnd *cmd)
> +{
> +   return scsi_sg_count(cmd) != 1 ||
> +   scsi_bufflen(cmd) >= TW_MIN_SGL_LENGTH;
> +}
> +
>  /* This function will complete an aen request from the isr */
>  static int twa_aen_complete(TW_Device_Extension *tw_dev, int request_id)
>  {
> @@ -1339,7 +1350,8 @@ static irqreturn_t twa_interrupt(int irq, void 
> *dev_instance)
> }
>
> /* Now complete the io */
> -   scsi_dma_unmap(cmd);
> +   if (twa_command_mapped(cmd))
> +   scsi_dma_unmap(cmd);
> cmd->scsi_done(cmd);
> tw_dev->state[request_id] = TW_S_COMPLETED;
> twa_free_request_id(tw_dev, request_id);
> @@ -1582,7 +1594,8 @@ static int 
> twa_reset_device_extension(TW_Device_Extension *tw_dev)
> struct scsi_cmnd *cmd = tw_dev->srb[i];
>
> cmd->result = (DID_RESET << 16);
> -   scsi_dma_unmap(cmd);
> +   if (twa_command_mapped(cmd))
> +   scsi_dma_unmap(cmd);
> cmd->scsi_done(cmd);
> }
> }
> @@ -1765,12 +1778,14 @@ static int twa_scsi_queue_lck(struct scsi_cmnd 
> *SCpnt, void (*done)(struct scsi_
> retval = twa_scsiop_execute_scsi(tw_dev, request_id, NULL, 0, NULL);
> switch (retval) {
> case SCSI_MLQUEUE_HOST_BUSY:
> -   scsi_dma_unmap(SCpnt);
> +   if (twa_command_mapped(SCpnt))
> +   scsi_dma_unmap(SCpnt);
> twa_free_request_id(tw_dev, request_id);
> break;
> case 1:
> SCpnt->result = (DID_ERROR << 16);
> -   scsi_dma_unmap(SCpnt);
> +   if (twa_command_mapped(SCpnt))
> +   scsi_dma_unmap(SCpnt);
> done(SCpnt);
> tw_dev->state[request_id] = TW_S_COMPLETED;
> twa_free_request_id(tw_dev, request_id);
> @@ -1831,8 +1846,7 @@ static int twa_scsiop_execute_scsi(TW_Device_Extension 
> *tw_dev, int request_id,
> /* Map sglist from scsi layer to cmd packet */
>
> if (scsi_sg_count(srb)) {
> -   if ((scsi_sg_count(srb) == 1) &&
> -   (scsi_bufflen(srb) < TW_MIN_SGL_LENGTH)) {
> +   if (!twa_command_mapped(srb)) {
> if (srb->sc_data_direction == DMA_TO_DEVICE ||
> srb->sc_data_direction == 
> DMA_BIDIRECTIONAL)
> scsi_sg_copy_to_buffer(srb,
> @@ -1905,7 +1919,7 @@ static void 
> twa_scsiop_execute_scsi_complete(TW_Device_Extension *tw_dev, int re
>  {
> struct scsi_cmnd *cmd = tw_dev->srb[request_id];
>
> -   if (scsi_bufflen(cmd) < TW_MIN_SGL_LENGTH &&
> +   if (!twa_command_mapped(cmd) &&
> (cmd->sc_data_direction == DMA_FROM_DEVICE ||
>  cmd->sc_data_direction == DMA_BIDIRECTIONAL)) {
> if (scsi_sg_count(cmd) == 1) {
> --
> 1.9.1
>

Christoph/James,

Thanks for fixing this.  The patch looks good to me.

Acked-by: Adam Radford 

-Adam
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RFC: reduce CONFIG_SCSI_CONSTANTS impact by 4k

[Julian Calaby]

Hi Rasmus,

On Sun, Oct 4, 2015 at 9:09 AM, Rasmus Villemoes
 wrote:
> Subject: [PATCH 2/2] scsi: reduce CONFIG_SCSI_CONSTANTS=y impact by 8k
>
> On 64 bit, struct error_info has 6 bytes of padding, which amounts to
> over 4k of wasted space in the additional[] array. We could easily get
> rid of that by instead using separate arrays for the codes and the
> pointers. However, we can do even better than that and save an
> additional 6 bytes per entry: In the table, just store the sizeof()
> the corresponding string literal. The cumulative sum of these is then
> the appropriate offset into additional_text, which is built from the
> concatenation (with '\0's inbetween) of the strings.
>
> $ scripts/bloat-o-meter /tmp/vmlinux vmlinux
> add/remove: 0/0 grow/shrink: 1/1 up/down: 24/-8488 (-8464)
> function old new   delta
> scsi_extd_sense_format   136 160 +24
> additional 113122824   -8488

Quick question:

> Signed-off-by: Rasmus Villemoes 
> ---
>  drivers/scsi/constants.c   | 25 +
>  drivers/scsi/sense_codes.h |  2 --
>  2 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
> index 47aaccd5e68e..ccd34b0481cd 100644
> --- a/drivers/scsi/constants.c
> +++ b/drivers/scsi/constants.c
> @@ -292,17 +292,31 @@ bool scsi_opcode_sa_name(int opcode, int service_action,
>
>  struct error_info {
> unsigned short code12;  /* 0x0302 looks better than 0x03,0x02 */
> -   const char * text;
> +   unsigned short size;
>  };
>
>
> +/*
> + * There are 700+ entries in this table. To save space, we don't store
> + * (code, pointer) pairs, which would make sizeof(struct
> + * error_info)==16 on 64 bits. Rather, the second element just stores
> + * the size (including \0) of the corresponding string, and we use the
> + * sum of these to get the appropriate offset into additional_text
> + * defined below. This approach saves 12 bytes per entry.
> + */
>  static const struct error_info additional[] =
>  {
> -#define SENSE_CODE(c, s) {c, s},
> +#define SENSE_CODE(c, s) {c, sizeof(s)},
>  #include "sense_codes.h"
>  #undef SENSE_CODE
>  };
>
> +static const char *additional_text =
> +#define SENSE_CODE(c, s) s "\0"
> +#include "sense_codes.h"
> +#undef SENSE_CODE
> +   ;
> +
>  struct error_info2 {
> unsigned char code1, code2_min, code2_max;
> const char * str;
> @@ -364,11 +378,14 @@ scsi_extd_sense_format(unsigned char asc, unsigned char 
> ascq, const char **fmt)
>  {
> int i;
> unsigned short code = ((asc << 8) | ascq);
> +   unsigned offset = 0;
>
> *fmt = NULL;
> -   for (i = 0; additional[i].text; i++)
> +   for (i = 0; i < ARRAY_SIZE(additional); i++) {
> if (additional[i].code12 == code)
> -   return additional[i].text;
> +   return additional_text + offset;
> +   offset += additional[i].size;

You don't seem to be accounting for the null bytes here.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/1] storvsc: Don't set the SRB_FLAGS_QUEUE_ACTION_ENABLE flag

[KY Srinivasan]

> -Original Message-
> From: James Bottomley [mailto:james.bottom...@hansenpartnership.com]
> Sent: Monday, August 31, 2015 7:02 AM
> To: KY Srinivasan 
> Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org;
> de...@linuxdriverproject.org; oher...@suse.com;
> jbottom...@parallels.com; h...@infradead.org; linux-scsi@vger.kernel.org;
> a...@canonical.com; vkuzn...@redhat.com; jasow...@redhat.com;
> sta...@vger.kernel.org
> Subject: Re: [PATCH 1/1] storvsc: Don't set the
> SRB_FLAGS_QUEUE_ACTION_ENABLE flag
> 
> On Mon, 2015-08-31 at 08:21 -0700, K. Y. Srinivasan wrote:
> > Don't set the SRB_FLAGS_QUEUE_ACTION_ENABLE flag since we are not
> specifying
> > tags.
> 
> What's the actual problem description this causes?

James,

Should I resend this patch. I think I provided the clarification you had sought.

Regards,

K. Y
> 
> James
> 
> 
> > Signed-off-by: K. Y. Srinivasan 
> > Cc: sta...@vger.kernel.org
> > ---
> >  drivers/scsi/storvsc_drv.c |3 +--
> >  1 files changed, 1 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> > index 40c43ae..ad8c4bc 100644
> > --- a/drivers/scsi/storvsc_drv.c
> > +++ b/drivers/scsi/storvsc_drv.c
> > @@ -1647,8 +1647,7 @@ static int storvsc_queuecommand(struct
> Scsi_Host *host, struct scsi_cmnd *scmnd)
> > vm_srb->win8_extension.time_out_value = 60;
> >
> > vm_srb->win8_extension.srb_flags |=
> > -   (SRB_FLAGS_QUEUE_ACTION_ENABLE |
> > -   SRB_FLAGS_DISABLE_SYNCH_TRANSFER);
> > +   SRB_FLAGS_DISABLE_SYNCH_TRANSFER;
> >
> > /* Build the SRB */
> > switch (scmnd->sc_data_direction) {
> 
> 

N�r��yb�X��ǧv�^�)޺{.n�+{���"�{ay�ʇڙ�,j��f���h���z��w���
���j:+v���w�j�mzZ+�ݢj"��!�i

[PATCHv2 0/1] Update SCSI hosts to use idr for host number mgmt

[Lee Duncan]

This patch updates the SCSI hosts module to use the idr
index-management routines to manage its host_no index instead
of using an ATOMIC integer. This means that host numbers
can now be reclaimed and re-used.

It also updates the hosts module to use the idr routine idr_find()
to lookup hosts based on the host number, hopefully speeding
up said lookup.

After noticing that my idr calling sequences where very close
to those in other modules, I considered creating some idr helper
functions (and using them), but because idr usage almost always
requires the caller to manage their own locks, I gave up on
this approach (as suggested by Tejon -- thank you).

Lee Duncan (1):
  SCSI: update hosts module to use idr index management

 drivers/scsi/hosts.c | 60 +---
 1 file changed, 29 insertions(+), 31 deletions(-)

-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 1/1] SCSI: update hosts module to use idr index management

[Lee Duncan]

Update the SCSI hosts module to use idr to manage
its host_no index instead of an ATOMIC integer. This
also allows using idr_find() to look up the SCSI
host structure given the host number.

This means that the SCSI host number will now
be reclaimable.

Signed-off-by: Lee Duncan 
---
 drivers/scsi/hosts.c | 60 +---
 1 file changed, 29 insertions(+), 31 deletions(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 8bb173e01084..9dc8ff971f5a 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -33,7 +33,7 @@
 #include 
 #include 
 #include 
-
+#include 
 #include 
 #include 
 #include 
@@ -41,8 +41,8 @@
 #include "scsi_priv.h"
 #include "scsi_logging.h"
 
-
-static atomic_t scsi_host_next_hn = ATOMIC_INIT(0);/* host_no for next new 
host */
+static DEFINE_IDR(host_index_idr);
+static DEFINE_SPINLOCK(host_index_lock);
 
 
 static void scsi_host_cls_release(struct device *dev)
@@ -337,6 +337,10 @@ static void scsi_host_dev_release(struct device *dev)
 
kfree(shost->shost_data);
 
+   spin_lock(_index_lock);
+   idr_remove(_index_idr, shost->host_no);
+   spin_unlock(_index_lock);
+
if (parent)
put_device(parent);
kfree(shost);
@@ -370,6 +374,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template 
*sht, int privsize)
 {
struct Scsi_Host *shost;
gfp_t gfp_mask = GFP_KERNEL;
+   int index;
 
if (sht->unchecked_isa_dma && privsize)
gfp_mask |= __GFP_DMA;
@@ -388,11 +393,15 @@ struct Scsi_Host *scsi_host_alloc(struct 
scsi_host_template *sht, int privsize)
init_waitqueue_head(>host_wait);
mutex_init(>scan_mutex);
 
-   /*
-* subtract one because we increment first then return, but we need to
-* know what the next host number was before increment
-*/
-   shost->host_no = atomic_inc_return(_host_next_hn) - 1;
+   idr_preload(GFP_KERNEL);
+   spin_lock(_index_lock);
+   index = idr_alloc(_index_idr, shost, 0, 0, GFP_NOWAIT);
+   spin_unlock(_index_lock);
+   idr_preload_end();
+   if (index < 0)
+   goto fail_kfree;
+   shost->host_no = index;
+
shost->dma_channel = 0xff;
 
/* These three are default values which can be overridden */
@@ -477,7 +486,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template 
*sht, int privsize)
shost_printk(KERN_WARNING, shost,
"error handler thread failed to spawn, error = %ld\n",
PTR_ERR(shost->ehandler));
-   goto fail_kfree;
+   goto fail_idr_remove;
}
 
shost->tmf_work_q = alloc_workqueue("scsi_tmf_%d",
@@ -493,6 +502,10 @@ struct Scsi_Host *scsi_host_alloc(struct 
scsi_host_template *sht, int privsize)
 
  fail_kthread:
kthread_stop(shost->ehandler);
+ fail_idr_remove:
+   spin_lock(_index_lock);
+   idr_remove(_index_idr, shost->host_no);
+   spin_unlock(_index_lock);
  fail_kfree:
kfree(shost);
return NULL;
@@ -522,37 +535,21 @@ void scsi_unregister(struct Scsi_Host *shost)
 }
 EXPORT_SYMBOL(scsi_unregister);
 
-static int __scsi_host_match(struct device *dev, const void *data)
-{
-   struct Scsi_Host *p;
-   const unsigned short *hostnum = data;
-
-   p = class_to_shost(dev);
-   return p->host_no == *hostnum;
-}
-
 /**
  * scsi_host_lookup - get a reference to a Scsi_Host by host no
  * @hostnum:   host number to locate
  *
  * Return value:
  * A pointer to located Scsi_Host or NULL.
- *
- * The caller must do a scsi_host_put() to drop the reference
- * that scsi_host_get() took. The put_device() below dropped
- * the reference from class_find_device().
  **/
 struct Scsi_Host *scsi_host_lookup(unsigned short hostnum)
 {
-   struct device *cdev;
-   struct Scsi_Host *shost = NULL;
-
-   cdev = class_find_device(_class, NULL, ,
-__scsi_host_match);
-   if (cdev) {
-   shost = scsi_host_get(class_to_shost(cdev));
-   put_device(cdev);
-   }
+   struct Scsi_Host *shost;
+
+   spin_lock(_index_lock);
+   shost = idr_find(_index_idr, hostnum);
+   spin_unlock(_index_lock);
+
return shost;
 }
 EXPORT_SYMBOL(scsi_host_lookup);
@@ -588,6 +585,7 @@ int scsi_init_hosts(void)
 void scsi_exit_hosts(void)
 {
class_unregister(_class);
+   idr_destroy(_index_idr);
 }
 
 int scsi_is_host_device(const struct device *dev)
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 33/34] cxlflash: Fix to avoid leaving dangling interrupt resources

[Andrew Donnellan]

On 02/10/15 01:58, Matthew R. Ochs wrote:

When running with an unsupported AFU, the cxlflash driver fails
the probe. When the driver is removed, the following Oops is
encountered on a show_interrupts() thread:

Call Trace:
[c01fba5a7a10] [0003] 0x3 (unreliable)
[c01fba5a7a60] [c053dcf4] vsnprintf+0x204/0x4c0
[c01fba5a7ae0] [c030045c] seq_vprintf+0x5c/0xd0
[c01fba5a7b20] [c030051c] seq_printf+0x4c/0x60
[c01fba5a7b50] [c013e140] show_interrupts+0x370/0x4f0
[c01fba5a7c10] [c02ff898] seq_read+0xe8/0x530
[c01fba5a7ca0] [c035d5c0] proc_reg_read+0xb0/0x110
[c01fba5a7cf0] [c02ca74c] __vfs_read+0x6c/0x180
[c01fba5a7d90] [c02cb464] vfs_read+0xa4/0x1c0
[c01fba5a7de0] [c02cc51c] SyS_read+0x6c/0x110
[c01fba5a7e30] [c0009204] system_call+0x38/0xb4

The Oops is due to not cleaning up correctly on the unsupported
AFU error path, leaving various allocated and registered resources.
In this case, interrupts are in a semi-allocated/registered state,
which the show_interrupts() thread attempts to use.

To fix, the cleanup logic in init_afu() is consolidated to error
gates at the bottom of the function and the appropriate goto is
added to each error path. As a mini side fix while refactoring
in this routine, the else statement following the AFU version
evaluation is eliminated as it is not needed.

Signed-off-by: Matthew R. Ochs 


Reviewed-by: Andrew Donnellan 

--
Andrew Donnellan  Software Engineer, OzLabs
andrew.donnel...@au1.ibm.com  Australia Development Lab, Canberra
+61 2 6201 8874 (work)IBM Australia Limited

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Inheritance

[Stephen Woolfe]

I am Stephen Woolfe,the Legal Attorney to late Engr. Mark with assets value 
placed at £10.9 Million.Contact me for more information.Email: 
stephenwoo...@qq.com
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] isci: fix two comment typos

[Artur Paszkiewicz]

On 10/04/2015 10:53 AM, Geliang Tang wrote:
> Just fix two typos in the code comment.
> 
> Signed-off-by: Geliang Tang 
> ---
>  drivers/scsi/isci/request.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/isci/request.c b/drivers/scsi/isci/request.c
> index cfd0084..8fe106f 100644
> --- a/drivers/scsi/isci/request.c
> +++ b/drivers/scsi/isci/request.c
> @@ -3306,7 +3306,7 @@ sci_io_request_construct_smp(struct device *dev,
>   * @ireq: This parameter points to the isci_request allocated in the
>   *request construct function.
>   *
> - * SCI_SUCCESS on successfull completion, or specific failure code.
> + * SCI_SUCCESS on successful completion, or specific failure code.
>   */
>  static enum sci_status isci_smp_request_build(struct isci_request *ireq)
>  {
> @@ -3332,7 +3332,7 @@ static enum sci_status isci_smp_request_build(struct 
> isci_request *ireq)
>   * @sci_device: This parameter is the handle for the sci core's remote device
>   *object that is the destination for this request.
>   *
> - * SCI_SUCCESS on successfull completion, or specific failure code.
> + * SCI_SUCCESS on successful completion, or specific failure code.
>   */
>  static enum sci_status isci_io_request_build(struct isci_host *ihost,
>struct isci_request *request,
> 

Acked-by: Artur Paszkiewicz 

Thanks,
Artur

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/3] scsi: drop unlikely behind BUG_ON()

[Geliang Tang]

(1) For !CONFIG_BUG cases, the bug call is a no-op, so we couldn't care
less and the change is ok.

(2) ppc and mips, which HAVE_ARCH_BUG_ON, do not rely on branch predictions
as it seems to be pointless[1] and thus callers should not be trying to
push an optimization in the first place.

(3) For CONFIG_BUG and !HAVE_ARCH_BUG_ON cases, BUG_ON() contains an
unlikely compiler flag already.

Hence, we can drop unlikely behind BUG_ON().

[1] http://lkml.iu.edu/hypermail/linux/kernel/1101.3/02289.html

Signed-off-by: Geliang Tang 
Reviewed-by: Bart Van Assche 
---
Changes in v2:
 - Just rewrite the commit log.
---
 drivers/scsi/scsi_lib.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index f570b48..3b5faab 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1164,8 +1164,8 @@ int scsi_init_io(struct scsi_cmnd *cmd)
 
count = blk_rq_map_integrity_sg(rq->q, rq->bio,
prot_sdb->table.sgl);
-   BUG_ON(unlikely(count > ivecs));
-   BUG_ON(unlikely(count > queue_max_integrity_segments(rq->q)));
+   BUG_ON(count > ivecs);
+   BUG_ON(count > queue_max_integrity_segments(rq->q));
 
cmd->prot_sdb = prot_sdb;
cmd->prot_sdb->table.nents = count;
-- 
2.5.0


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 13/13] scsi: ufs: Add exynos ufs platform data

[Alim Akhtar]

Hi Rob,

On Mon, Oct 5, 2015 at 7:41 PM, Rob Herring  wrote:
> On Mon, Oct 5, 2015 at 4:06 AM, Arnd Bergmann  wrote:
>> On Monday 05 October 2015 13:44:29 Alim Akhtar wrote:
>>> CCing Rob Herring,
>>>
>>> Hi Arnd,
>>>
>>> On 10/01/2015 04:59 PM, Arnd Bergmann wrote:
>>> > On Thursday 01 October 2015 18:46:34 kbuild test robot wrote:
>>> >> [auto build test results on v4.3-rc3 -- if it's inappropriate base, 
>>> >> please ignore]
>>> >>
>>> >> config: x86_64-allmodconfig (attached as .config)
>>> >> reproduce:
>>> >>  git checkout 6e153e3bf7c68b019e987c5a0ffadebd9c7d4fbb
>>> >>  # save the attached .config to linux build tree
>>> >>  make ARCH=x86_64
>>> >>
>>> >> All error/warnings (new ones prefixed by >>):
>>> >>
>>>  ERROR: "ufs_hba_exynos_ops" [drivers/scsi/ufs/ufshcd-pltfrm.ko] 
>>>  undefined!
>>> >>
>>> >>
>>> >
>>> > Ah, this seems to be a case of layering violation. It would be best to
>>> > restructure the code so that the exynos driver registers a platform_driver
>>> > by itself for the respective DT compatible string, and then calls
>>> > into the common code from its probe function, rather than having the
>>> > generic driver know about the specific backends.
>>> >
>>> > That approach will also make the generic driver more scalable as we
>>> > add further chip-specific variations, and matches what we do in other
>>> > drivers.
>>> >
>>>
>>> Looks like some discussions on ufs variant driver probe method happened
>>> here [1] few months back.
>>> [1]-> https://lkml.org/lkml/2015/6/3/180
>>
>> Hmm, too bad we didn't catch it then, it's much more work to fix now.
>
> What you suggested is what is being implemented[1]. It is not merged
> yet. The core is a library and the platform specific parts create the
> driver.
>
> Rob
>
> [1] https://lkml.org/lkml/2015/9/2/364

Thanks for the pointer...let me have a look. At least now we have
another variant to test it out.

> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Regards,
Alim
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] scsi: move Additional Sense Codes to separate file

[Bart Van Assche]

On 10/05/15 02:26, Rasmus Villemoes wrote:

-   {0x041A, "Logical unit not ready, start stop unit command in "
-"progress"},
-   {0x041B, "Logical unit not ready, sanitize in progress"},
-   {0x041C, "Logical unit not ready, additional power use not yet "
-"granted"},


Please convert these multi-line strings into single line string 
constants such that users can look up these easily with grep.


> +
> +SENSE_CODE(0, NULL)

The above looks confusing to me. Please leave this out and add { 0, NULL 
} at the end of the additional[] array instead.


Thanks,

Bart.

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] scsi: reduce CONFIG_SCSI_CONSTANTS=y impact by 8k

[Bart Van Assche]

On 10/05/15 02:26, Rasmus Villemoes wrote:

  struct error_info {
unsigned short code12;  /* 0x0302 looks better than 0x03,0x02 */
-   const char * text;
+   unsigned short size;
  };


Had you considered to use the type uint16_t instead of unsigned short ?

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 13/13] scsi: ufs: Add exynos ufs platform data

[Arnd Bergmann]

On Monday 05 October 2015 09:11:33 Rob Herring wrote:
> On Mon, Oct 5, 2015 at 4:06 AM, Arnd Bergmann  wrote:
> > On Monday 05 October 2015 13:44:29 Alim Akhtar wrote:
> >>
> >> On 10/01/2015 04:59 PM, Arnd Bergmann wrote:
> >> > On Thursday 01 October 2015 18:46:34 kbuild test robot wrote:
> >> > Ah, this seems to be a case of layering violation. It would be best to
> >> > restructure the code so that the exynos driver registers a 
> >> > platform_driver
> >> > by itself for the respective DT compatible string, and then calls
> >> > into the common code from its probe function, rather than having the
> >> > generic driver know about the specific backends.
> >> >
> >> > That approach will also make the generic driver more scalable as we
> >> > add further chip-specific variations, and matches what we do in other
> >> > drivers.
> >> >
> >>
> >> Looks like some discussions on ufs variant driver probe method happened
> >> here [1] few months back.
> >> [1]-> https://lkml.org/lkml/2015/6/3/180
> >
> > Hmm, too bad we didn't catch it then, it's much more work to fix now.
> 
> What you suggested is what is being implemented[1]. It is not merged
> yet. The core is a library and the platform specific parts create the
> driver.
> 
> Rob
> 
> [1] https://lkml.org/lkml/2015/9/2/364

Ah, good. Sorry for the misunderstanding on my side.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 13/13] scsi: ufs: Add exynos ufs platform data

[Rob Herring]

On Mon, Oct 5, 2015 at 4:06 AM, Arnd Bergmann  wrote:
> On Monday 05 October 2015 13:44:29 Alim Akhtar wrote:
>> CCing Rob Herring,
>>
>> Hi Arnd,
>>
>> On 10/01/2015 04:59 PM, Arnd Bergmann wrote:
>> > On Thursday 01 October 2015 18:46:34 kbuild test robot wrote:
>> >> [auto build test results on v4.3-rc3 -- if it's inappropriate base, 
>> >> please ignore]
>> >>
>> >> config: x86_64-allmodconfig (attached as .config)
>> >> reproduce:
>> >>  git checkout 6e153e3bf7c68b019e987c5a0ffadebd9c7d4fbb
>> >>  # save the attached .config to linux build tree
>> >>  make ARCH=x86_64
>> >>
>> >> All error/warnings (new ones prefixed by >>):
>> >>
>>  ERROR: "ufs_hba_exynos_ops" [drivers/scsi/ufs/ufshcd-pltfrm.ko] 
>>  undefined!
>> >>
>> >>
>> >
>> > Ah, this seems to be a case of layering violation. It would be best to
>> > restructure the code so that the exynos driver registers a platform_driver
>> > by itself for the respective DT compatible string, and then calls
>> > into the common code from its probe function, rather than having the
>> > generic driver know about the specific backends.
>> >
>> > That approach will also make the generic driver more scalable as we
>> > add further chip-specific variations, and matches what we do in other
>> > drivers.
>> >
>>
>> Looks like some discussions on ufs variant driver probe method happened
>> here [1] few months back.
>> [1]-> https://lkml.org/lkml/2015/6/3/180
>
> Hmm, too bad we didn't catch it then, it's much more work to fix now.

What you suggested is what is being implemented[1]. It is not merged
yet. The core is a library and the platform specific parts create the
driver.

Rob

[1] https://lkml.org/lkml/2015/9/2/364
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html