Re: [PATCH for-next] qla2xxx: delete references to unused firmware files

2015-05-21 Thread Julian Calaby
Hi All,

On Thu, May 21, 2015 at 8:42 PM, Xose Vazquez Perez
 wrote:
> On 05/19/2015 05:51 PM, Himanshu Madhani wrote:
>
>> On 5/18/15, 6:50 PM, "Julian Calaby"  wrote:
>
>>> Do the devices these firmware files are for exist and is there any
>>> chance of the files being released?
>>
>> Yes. These devices are available and we read firmware from FLASH on these
>> adapters.
>> At times when debugging/triaging involves firmware, we use binaries that
>> would be useful for quick triaging.
>
> It looks like newer FW files were released on 4/29/2015 in:
> http://ldriver.qlogic.com/firmware/rpms/qlogic-firmware-8.01.00-1.noarch.rpm
>
> ql2400_fw.bin:  COPYRIGHT 2015 QLOGIC CORPORATION   ISP24xx Firmware   
> Version   8.01.00  $
> ql2500_fw.bin:  COPYRIGHT 2015 QLOGIC CORPORATION   ISP24xx Firmware   
> Version   8.01.00  $
> ql2600_fw.bin:  COPYRIGHT 2015 QLOGIC CORPORATION   ISP24xx Firmware   
> Version   8.01.00  $
> ql2700_fw.bin:  COPYRIGHT 2015 QLOGIC CORPORATION   ISP24xx Firmware   
> Version   8.01.00  $
> ql8300_fw.bin:  COPYRIGHT 2015 QLOGIC CORPORATION   ISP24xx Firmware   
> Version   8.01.00  $
>
>
> Could you please send meaningful files to linux-firmware.git ?
> And remove irrelevant code from qla_os.c/Kconfig

Assuming that these files are actually sent to linux-firmware.git, the
patch I just sent will hide the stuff for the missing firmware files
behind a new Kconfig symbol, this way it's there for Qlogic, however
hidden from people who don't have access to the files.

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 for-next] scsi: qla2xxx: Hide unavailable firmware

2015-05-21 Thread Julian Calaby
Hi Xose,

Adding you to CC.

On Fri, May 22, 2015 at 10:00 AM, Julian Calaby  wrote:
> Some qla2xxx devices have firmware stored in flash on the device,
> however for debugging and triage purposes, Qlogic staff like to
> be able to load known-good versions of these firmwares through
> request_firmware().
>
> These firmware files were never distributed and are unlikely to ever
> be released publically, so to hide these missing firmware files from
> scripts which check such things, (e.g. Debian's initramfs-tools) put
> them behind a new EXPERT Kconfig option.
>
> Cc: 
> Cc: James E.J. Bottomley 
> Cc: Linux Firmware Maintainers 
> Signed-off-by: Julian Calaby 
> ---
>  drivers/scsi/qla2xxx/Kconfig  | 25 +
>  drivers/scsi/qla2xxx/qla_os.c | 40 +++-
>  2 files changed, 52 insertions(+), 13 deletions(-)
>
> This is against linux-next @next-20150520 and has been compile
> tested only.
>
> diff --git a/drivers/scsi/qla2xxx/Kconfig b/drivers/scsi/qla2xxx/Kconfig
> index 33f60c9..31e9db4 100644
> --- a/drivers/scsi/qla2xxx/Kconfig
> +++ b/drivers/scsi/qla2xxx/Kconfig
> @@ -31,6 +31,31 @@ config SCSI_QLA_FC
>
> They are also included in the linux-firmware tree as well.
>
> +   This driver also supports some adapters with firmware stored
> +   onboard in flash.
> +
> +config SCSI_QLA_FC_TRIAGE
> +   bool "Firmware loading support for flash based adapters"
> +   depends on SCSI_QLA_FC
> +   depends on EXPERT
> +   default n
> +   ---help---
> +   Add firmware definitions for adapters with firmware stored
> +   onboard in flash.
> +
> +   This requires the following firmware files which are not
> +   distributed:
> +
> +   ISP   Firmware Filename
> +   ---
> +   81xx  ql8100_fw.bin
> +   82xx  ql8200_fw.bin
> +
> +   This option should only be enabled by Qlogic support staff
> +   as these firmware files are not available publically.
> +
> +   If unsure say N.
> +
>  config TCM_QLA2XXX
> tristate "TCM_QLA2XXX fabric module for Qlogic 2xxx series target 
> mode HBAs"
> depends on SCSI_QLA_FC && TARGET_CORE
> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> index 7462dd7..da98d83 100644
> --- a/drivers/scsi/qla2xxx/qla_os.c
> +++ b/drivers/scsi/qla2xxx/qla_os.c
> @@ -5284,11 +5284,14 @@ qla2x00_timer(scsi_qla_host_t *vha)
>  #define FW_ISP2322 3
>  #define FW_ISP24XX 4
>  #define FW_ISP25XX 5
> -#define FW_ISP81XX 6
> -#define FW_ISP82XX 7
> -#define FW_ISP2031 8
> -#define FW_ISP8031 9
> -#define FW_ISP27XX 10
> +#define FW_ISP2031 6
> +#define FW_ISP8031 7
> +#define FW_ISP27XX 8
> +
> +#ifdef CONFIG_SCSI_QLA_FC_TRIAGE
> +#define FW_ISP81XX 9
> +#define FW_ISP82XX 10
> +#endif
>
>  #define FW_FILE_ISP21XX"ql2100_fw.bin"
>  #define FW_FILE_ISP22XX"ql2200_fw.bin"
> @@ -5296,12 +5299,14 @@ qla2x00_timer(scsi_qla_host_t *vha)
>  #define FW_FILE_ISP2322"ql2322_fw.bin"
>  #define FW_FILE_ISP24XX"ql2400_fw.bin"
>  #define FW_FILE_ISP25XX"ql2500_fw.bin"
> -#define FW_FILE_ISP81XX"ql8100_fw.bin"
> -#define FW_FILE_ISP82XX"ql8200_fw.bin"
>  #define FW_FILE_ISP2031"ql2600_fw.bin"
>  #define FW_FILE_ISP8031"ql8300_fw.bin"
>  #define FW_FILE_ISP27XX"ql2700_fw.bin"
>
> +#ifdef CONFIG_SCSI_QLA_FC_TRIAGE
> +#define FW_FILE_ISP81XX"ql8100_fw.bin"
> +#define FW_FILE_ISP82XX"ql8200_fw.bin"
> +#endif
>
>  static DEFINE_MUTEX(qla_fw_lock);
>
> @@ -5312,11 +5317,13 @@ static struct fw_blob qla_fw_blobs[FW_BLOBS] = {
> { .name = FW_FILE_ISP2322, .segs = { 0x800, 0x1c000, 0x1e000, 0 }, },
> { .name = FW_FILE_ISP24XX, },
> { .name = FW_FILE_ISP25XX, },
> -   { .name = FW_FILE_ISP81XX, },
> -   { .name = FW_FILE_ISP82XX, },
> { .name = FW_FILE_ISP2031, },
> { .name = FW_FILE_ISP8031, },
> { .name = FW_FILE_ISP27XX, },
> +#ifdef CONFIG_SCSI_QLA_FC_TRIAGE
> +   { .name = FW_FILE_ISP81XX, },
> +   { .name = FW_FILE_ISP82XX, },
> +#endif
>  };
>
>  struct fw_blob *
> @@ -5337,16 +5344,18 @@ qla2x00_request_firmware(scsi_qla_host_t *vha)
> blob = &qla_fw_blobs[FW_ISP24XX];
> } else if (IS_QLA25XX(ha)) {
> blob = &qla_fw_blobs[FW_ISP25XX];
>

[PATCH for-next] scsi: qla2xxx: Hide unavailable firmware

2015-05-21 Thread Julian Calaby
Some qla2xxx devices have firmware stored in flash on the device,
however for debugging and triage purposes, Qlogic staff like to
be able to load known-good versions of these firmwares through
request_firmware().

These firmware files were never distributed and are unlikely to ever
be released publically, so to hide these missing firmware files from
scripts which check such things, (e.g. Debian's initramfs-tools) put
them behind a new EXPERT Kconfig option.

Cc: 
Cc: James E.J. Bottomley 
Cc: Linux Firmware Maintainers 
Signed-off-by: Julian Calaby 
---
 drivers/scsi/qla2xxx/Kconfig  | 25 +
 drivers/scsi/qla2xxx/qla_os.c | 40 +++-
 2 files changed, 52 insertions(+), 13 deletions(-)

This is against linux-next @next-20150520 and has been compile
tested only.

diff --git a/drivers/scsi/qla2xxx/Kconfig b/drivers/scsi/qla2xxx/Kconfig
index 33f60c9..31e9db4 100644
--- a/drivers/scsi/qla2xxx/Kconfig
+++ b/drivers/scsi/qla2xxx/Kconfig
@@ -31,6 +31,31 @@ config SCSI_QLA_FC
 
They are also included in the linux-firmware tree as well.
 
+   This driver also supports some adapters with firmware stored
+   onboard in flash.
+
+config SCSI_QLA_FC_TRIAGE
+   bool "Firmware loading support for flash based adapters"
+   depends on SCSI_QLA_FC
+   depends on EXPERT
+   default n
+   ---help---
+   Add firmware definitions for adapters with firmware stored
+   onboard in flash.
+
+   This requires the following firmware files which are not
+   distributed:
+
+   ISP   Firmware Filename
+   ---
+   81xx  ql8100_fw.bin
+   82xx  ql8200_fw.bin
+
+   This option should only be enabled by Qlogic support staff
+   as these firmware files are not available publically.
+
+   If unsure say N.
+
 config TCM_QLA2XXX
tristate "TCM_QLA2XXX fabric module for Qlogic 2xxx series target mode 
HBAs"
depends on SCSI_QLA_FC && TARGET_CORE
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 7462dd7..da98d83 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -5284,11 +5284,14 @@ qla2x00_timer(scsi_qla_host_t *vha)
 #define FW_ISP2322 3
 #define FW_ISP24XX 4
 #define FW_ISP25XX 5
-#define FW_ISP81XX 6
-#define FW_ISP82XX 7
-#define FW_ISP2031 8
-#define FW_ISP8031 9
-#define FW_ISP27XX 10
+#define FW_ISP2031 6
+#define FW_ISP8031 7
+#define FW_ISP27XX 8
+
+#ifdef CONFIG_SCSI_QLA_FC_TRIAGE
+#define FW_ISP81XX 9
+#define FW_ISP82XX 10
+#endif
 
 #define FW_FILE_ISP21XX"ql2100_fw.bin"
 #define FW_FILE_ISP22XX"ql2200_fw.bin"
@@ -5296,12 +5299,14 @@ qla2x00_timer(scsi_qla_host_t *vha)
 #define FW_FILE_ISP2322"ql2322_fw.bin"
 #define FW_FILE_ISP24XX"ql2400_fw.bin"
 #define FW_FILE_ISP25XX"ql2500_fw.bin"
-#define FW_FILE_ISP81XX"ql8100_fw.bin"
-#define FW_FILE_ISP82XX"ql8200_fw.bin"
 #define FW_FILE_ISP2031"ql2600_fw.bin"
 #define FW_FILE_ISP8031"ql8300_fw.bin"
 #define FW_FILE_ISP27XX"ql2700_fw.bin"
 
+#ifdef CONFIG_SCSI_QLA_FC_TRIAGE
+#define FW_FILE_ISP81XX"ql8100_fw.bin"
+#define FW_FILE_ISP82XX"ql8200_fw.bin"
+#endif
 
 static DEFINE_MUTEX(qla_fw_lock);
 
@@ -5312,11 +5317,13 @@ static struct fw_blob qla_fw_blobs[FW_BLOBS] = {
{ .name = FW_FILE_ISP2322, .segs = { 0x800, 0x1c000, 0x1e000, 0 }, },
{ .name = FW_FILE_ISP24XX, },
{ .name = FW_FILE_ISP25XX, },
-   { .name = FW_FILE_ISP81XX, },
-   { .name = FW_FILE_ISP82XX, },
{ .name = FW_FILE_ISP2031, },
{ .name = FW_FILE_ISP8031, },
{ .name = FW_FILE_ISP27XX, },
+#ifdef CONFIG_SCSI_QLA_FC_TRIAGE
+   { .name = FW_FILE_ISP81XX, },
+   { .name = FW_FILE_ISP82XX, },
+#endif
 };
 
 struct fw_blob *
@@ -5337,16 +5344,18 @@ qla2x00_request_firmware(scsi_qla_host_t *vha)
blob = &qla_fw_blobs[FW_ISP24XX];
} else if (IS_QLA25XX(ha)) {
blob = &qla_fw_blobs[FW_ISP25XX];
-   } else if (IS_QLA81XX(ha)) {
-   blob = &qla_fw_blobs[FW_ISP81XX];
-   } else if (IS_QLA82XX(ha)) {
-   blob = &qla_fw_blobs[FW_ISP82XX];
} else if (IS_QLA2031(ha)) {
blob = &qla_fw_blobs[FW_ISP2031];
} else if (IS_QLA8031(ha)) {
blob = &qla_fw_blobs[FW_ISP8031];
} else if (IS_QLA27XX(ha)) {
blob = &qla_fw_blobs[FW_ISP27XX];
+#ifdef CONFIG_SCSI_QLA_FC_TRIAGE
+   } else if (IS_QLA81XX(ha)) {
+   blob = &qla_fw_blobs[FW_ISP81XX];
+   } else if (IS_QLA82XX(ha)) {
+   blob = &qla_fw_blobs[FW_ISP82XX]

Re: [PATCH] csiostor: Fix backwards locking in the function __csio_unreg_rnode

2016-04-06 Thread Julian Calaby
Hi Bastien,

On Wed, Apr 6, 2016 at 7:19 AM, Bastien Philbert
 wrote:
> This fixes backwards locking in the function __csio_unreg_rnode to
> properly lock before the call to the function csio_unreg_rnode and
> not unlock with spin_unlock_irq as this would not allow the proper
> protection for concurrent access on the shared csio_hw structure
> pointer hw. In addition switch the locking after the critical region
> function call to properly unlock instead with spin_unlock_irq on
>
> Signed-off-by: Bastien Philbert 
> ---
>  drivers/scsi/csiostor/csio_rnode.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/csiostor/csio_rnode.c 
> b/drivers/scsi/csiostor/csio_rnode.c
> index e9c3b04..029a09e 100644
> --- a/drivers/scsi/csiostor/csio_rnode.c
> +++ b/drivers/scsi/csiostor/csio_rnode.c
> @@ -580,9 +580,9 @@ __csio_unreg_rnode(struct csio_rnode *rn)
> ln->last_scan_ntgts--;
> }
>
> -   spin_unlock_irq(&hw->lock);
> -   csio_unreg_rnode(rn);
> spin_lock_irq(&hw->lock);
> +   csio_unreg_rnode(rn);
> +   spin_unlock_irq(&hw->lock);

Are you _certain_ this is correct? This construct usually appears when
a function has a particular lock held, then needs to unlock it to call
some other function. Are you _certain_ that this isn't the case?

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] megaraid: add scsi_cmnd NULL check before use

2016-05-08 Thread Julian Calaby
Hi Petros,

On Mon, May 9, 2016 at 2:34 AM, Petros Koutoupis
 wrote:
> On Sun, 2016-05-08 at 22:22 +1000, Finn Thain wrote:
>> On Sun, 8 May 2016, Petros Koutoupis wrote:
>>
>> > >
>> > > That contains a tautology.
>> > >
>> >
>> > How so?
>>
>> if (x)
>>   /* ... */
>> else if (!x && (whatever))
>>   /* ... */
>>
>> --
>
> Thank you but I know the logic of what I wrote. A tautology
> will yield the same results no matter what the interpretation.
> That is not a tautology. The two conditionals in my case check
> different states and serve different purposes.

You're missing the point.

Execution will only reach the else branch if "!cmd_fusion->scmd",
hence checking that is unnecessary. Removing that test (and all the
unnecessary parentheses) will reduce the second test to:

else if (scsi_io_req->Function == MPI2_FUNCTION_SCSI_IO_REQUEST ||
scsi_io_req->Function == MEGASAS_MPI2_FUNCTION_LD_IO_REQUEST)

which is much cleaner.

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] scsi:stex.c Support Pegasus 3 product

2016-06-09 Thread Julian Calaby
stex_notifier);
> +

Adding the reboot notifier applies to all cards, so it should probably
be a separate patch.

> host = scsi_host_alloc(&driver_template, sizeof(struct st_hba));
>
> if (!host) {
> @@ -1736,28 +1870,29 @@ static void stex_hba_stop(struct st_hba *hba, int 
> st_sleep_mic)
>
> spin_lock_irqsave(hba->host->host_lock, flags);
>
> -   if (hba->cardtype == st_yel && hba->supports_pm == 1)
> -   {
> -   if(st_sleep_mic == ST_NOTHANDLED)
> -   {
> +   if ((hba->cardtype == st_yel && hba->supports_pm == 1)
> +   || (hba->cardtype == st_P3 && hba->supports_pm == 1)) {

if ((hba->cardtype == st_yel || hba->cardtype == st_P3) &&
hba->supports_pm == 1) {

is simpler.

> +   if (st_sleep_mic == ST_NOTHANDLED) {
> spin_unlock_irqrestore(hba->host->host_lock, flags);
> return;
> }
> }
> req = hba->alloc_rq(hba);
> -   if (hba->cardtype == st_yel) {
> +   if (hba->cardtype == st_yel || hba->cardtype == st_P3) {
> msg_h = (struct st_msg_header *)req - 1;
> memset(msg_h, 0, hba->rq_size);
> } else
> memset(req, 0, hba->rq_size);
>
> -   if ((hba->cardtype == st_yosemite || hba->cardtype == st_yel)
> +   if ((hba->cardtype == st_yosemite || hba->cardtype == st_yel
> +   || hba->cardtype == st_P3)
> && st_sleep_mic == ST_IGNORED) {
> req->cdb[0] = MGT_CMD;
> req->cdb[1] = MGT_CMD_SIGNATURE;
> req->cdb[2] = CTLR_CONFIG_CMD;
> req->cdb[3] = CTLR_SHUTDOWN;
> -   } else if (hba->cardtype == st_yel && st_sleep_mic != ST_IGNORED) {
> +   } else if ((hba->cardtype == st_yel || hba->cardtype == st_P3)
> +   && st_sleep_mic != ST_IGNORED) {

Er, this will never get run.

We have:

if (hba->cardtype == st_yosemite || hba->cardtype == st_yel ||
hba->cardtype == st_P3) {
// stuff
} else if ((hba->cardtype == st_yel || hba->cardtype == st_P3) &&
st_sleep_mic != ST_IGNORED) {
// stuff
}

Should the two branches of the if statement be reversed or should the
first one be written like:

if (hba->cardtype == st_yosemite || ((hba->cardtype == st_yel ||
hba->cardtype == st_P3) && st_sleep_mic == ST_IGNORED)) {

> req->cdb[0] = MGT_CMD;
> req->cdb[1] = MGT_CMD_SIGNATURE;
> req->cdb[2] = CTLR_CONFIG_CMD;
> @@ -1768,16 +1903,14 @@ static void stex_hba_stop(struct st_hba *hba, int 
> st_sleep_mic)
> req->cdb[1] = CTLR_POWER_STATE_CHANGE;
> req->cdb[2] = CTLR_POWER_SAVING;
> }
> -
> hba->ccb[tag].cmd = NULL;
> hba->ccb[tag].sg_count = 0;
> hba->ccb[tag].sense_bufflen = 0;
> hba->ccb[tag].sense_buffer = NULL;
> hba->ccb[tag].req_type = PASSTHRU_REQ_TYPE;
> -
> hba->send(hba, req, tag);
> -   spin_unlock_irqrestore(hba->host->host_lock, flags);
>
> +   spin_unlock_irqrestore(hba->host->host_lock, flags);

More unrelated whitespace changes.

> before = jiffies;
> while (hba->ccb[tag].req_type & PASSTHRU_REQ_TYPE) {
> if (time_after(jiffies, before + ST_INTERNAL_TIMEOUT * HZ)) {
> @@ -1821,24 +1954,29 @@ static void stex_remove(struct pci_dev *pdev)
> scsi_host_put(hba->host);
>
> pci_disable_device(pdev);
> +
> +   unregister_reboot_notifier(&stex_notifier);

Again, not P3 specific.

>  }
>
>  static void stex_shutdown(struct pci_dev *pdev)
>  {
> struct st_hba *hba = pci_get_drvdata(pdev);
> -
> -   if (hba->supports_pm == 0)
> +   if (hba->supports_pm == 0) {
> stex_hba_stop(hba, ST_IGNORED);
> -   else
> +   } else if (hba->supports_pm == 1 && S6flag) {
> +   unregister_reboot_notifier(&stex_notifier);
> +   stex_hba_stop(hba, ST_S6);
> +   } else

Also not P3 specific.

> stex_hba_stop(hba, ST_S5);
>  }
>
> -static int stex_choice_sleep_mic(pm_message_t state)
> +static int stex_choice_sleep_mic(struct st_hba *hba, pm_message_t state)
>  {
> switch (state.event) {
> case PM_EVENT_SUSPEND:
> return ST_S3;
> case PM_EVENT_HIBERNATE:
> +   hba->msi_lock = 0;
> return ST_S4;
> default:
> return ST_NOTHANDLED;
> @@ -1864,6 +2003,13 @@ static int stex_resume(struct pci_dev *pdev)
> stex_handshake(hba);
> return 0;
>  }
> +
> +static int stex_halt(struct notifier_block *nb, unsigned long event, void 
> *buf)
> +{
> +   S6flag = 1;
> +   return NOTIFY_OK;
> +}
> +

And again.

Why is this needed?

>  MODULE_DEVICE_TABLE(pci, stex_pci_tbl);
>
>  static struct pci_driver stex_pci_driver = {
> --
> 1.9.1
>
> --
> 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

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 3/8] [SCSI] dc395x: use NULL instead of 0

2013-08-07 Thread Julian Calaby
Hi Jingoo,

On Wed, Aug 7, 2013 at 5:45 PM, Jingoo Han  wrote:
> On Wednesday, August 07, 2013 3:50 PM, Oliver Neukum wrote:
>> On Wed, 2013-08-07 at 12:55 +0900, Jingoo Han wrote:
>>
>> > @@ -4183,15 +4183,17 @@ static void check_eeprom(struct NvRamType *eeprom, 
>> > unsigned long io_port)
>> >  */
>> > dprintkl(KERN_WARNING,
>> > "EEProm checksum error: using default values and 
>> > options.\n");
>> > -   eeprom->sub_vendor_id[0] = (u8)PCI_VENDOR_ID_TEKRAM;
>> > +   eeprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & 0xff);
>>
>> Hi,
>>
>> if you are fixing these issues please use the proper macros for
>> conversion of endianness.
>
> Then, do you mean the following? :)
>
> -   prom->sub_vendor_id[0] = (u8)PCI_VENDOR_ID_TEKRAM;
> +   eprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & 
> le16_to_cpu(0xff));

No.

The issue is that the driver is doing things like this:

eeprom->member[0] = (u8)(CONSTANT & 0xff);
eeprom->member[1] = (u8)(CONSTANT >> 8);

Which is exactly the same as code along the lines of:

eeprom->member = cpu_to_le16(CONSTANT);

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/
--
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 3/8] [SCSI] dc395x: use NULL instead of 0

2013-08-07 Thread Julian Calaby
Hi Jingoo,

On Wed, Aug 7, 2013 at 6:36 PM, Jingoo Han  wrote:
>
>
>> -Original Message-----
>> From: Julian Calaby [mailto:julian.cal...@gmail.com]
>> Sent: Wednesday, August 07, 2013 5:21 PM
>> To: Jingoo Han
>> Cc: Oliver Neukum; James Bottomley; Ali Akcaagac; Jamie Lenehan; 
>> dc3...@twibble.org; James Bottomley;
>> linux-scsi
>> Subject: Re: [PATCH 3/8] [SCSI] dc395x: use NULL instead of 0
>>
>> Hi Jingoo,
>>
>> On Wed, Aug 7, 2013 at 5:45 PM, Jingoo Han  wrote:
>> > On Wednesday, August 07, 2013 3:50 PM, Oliver Neukum wrote:
>> >> On Wed, 2013-08-07 at 12:55 +0900, Jingoo Han wrote:
>> >>
>> >> > @@ -4183,15 +4183,17 @@ static void check_eeprom(struct NvRamType 
>> >> > *eeprom, unsigned long io_port)
>> >> >  */
>> >> > dprintkl(KERN_WARNING,
>> >> > "EEProm checksum error: using default values and 
>> >> > options.\n");
>> >> > -   eeprom->sub_vendor_id[0] = (u8)PCI_VENDOR_ID_TEKRAM;
>> >> > +   eeprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & 
>> >> > 0xff);
>> >>
>> >> Hi,
>> >>
>> >> if you are fixing these issues please use the proper macros for
>> >> conversion of endianness.
>> >
>> > Then, do you mean the following? :)
>> >
>> > -   prom->sub_vendor_id[0] = (u8)PCI_VENDOR_ID_TEKRAM;
>> > +   eprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & 
>> > le16_to_cpu(0xff));
>>
>> No.
>>
>> The issue is that the driver is doing things like this:
>>
>> eeprom->member[0] = (u8)(CONSTANT & 0xff);
>> eeprom->member[1] = (u8)(CONSTANT >> 8);
>>
>> Which is exactly the same as code along the lines of:
>>
>> eeprom->member = cpu_to_le16(CONSTANT);
>
> However, when I compile the following, it makes build error.
>
> -   eeprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & 0xff);
> -   eeprom->sub_vendor_id[1] = (u8)(PCI_VENDOR_ID_TEKRAM >> 8);
> +   eeprom->sub_vendor_id = cpu_to_le16(PCI_VENDOR_ID_TEKRAM);

Of course it does.

I said code along the lines of. You'll need to do more than just what
I suggested, including changing the definition of the eeprom struct
and fixing any other places where it's used / set.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/
--
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/4] Fix for crash when bfa_itnim is NULL

2015-01-29 Thread Julian Calaby
Hi Anil,

On Thu, Jan 29, 2015 at 7:55 PM,   wrote:
> From: Anil Gurumurthy 
>
> Signed-off-by: Sudarsana Kalluru 
> Signed-off-by: Anil Gurumurthy 
> ---
>  drivers/scsi/bfa/bfa_fcs_lport.c |2 +-
>  drivers/scsi/bfa/bfad_im.c   |   26 ++
>  2 files changed, 27 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/scsi/bfa/bfa_fcs_lport.c 
> b/drivers/scsi/bfa/bfa_fcs_lport.c
> index d823792..4631630 100644
> --- a/drivers/scsi/bfa/bfa_fcs_lport.c
> +++ b/drivers/scsi/bfa/bfa_fcs_lport.c
> @@ -2654,7 +2654,7 @@ bfa_fcs_fdmi_get_hbaattr(struct bfa_fcs_lport_fdmi_s 
> *fdmi,
>
> strncpy(hba_attr->node_sym_name.symname,
> port->port_cfg.node_sym_name.symname, BFA_SYMNAME_MAXLEN);
> -   strcpy(hba_attr->vendor_info, "BROCADE");
> +   strcpy(hba_attr->vendor_info, "QLogic");

Shouldn't this be in the next patch?

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 v2 1/9] snic: snic module infrastructure

2015-03-11 Thread Julian Calaby
Hi,

On Thu, Mar 12, 2015 at 4:01 AM, Narsimhulu Musini  wrote:
> snic_main.c contains module load and unload, global driver context,
> PCI Registration, PCI probe and remove, SCSI ML registration functionality.
>
> snic.h contains snic structure definition, snic global context, and
> prototypes.
>
> snic_os.h contains OS specific interfaces.
>
> snic_attrs.c contains device attributes to list snic state, link state,
> and driver version under /sys/class/scsi_host/host/
>
> v2
> Added Compile time macro for debugfs dependent functionality.

Your changelog should be below the "---" and before the diffstat.

> Signed-off-by: Narsimhulu Musini 
> Signed-off-by: Sesidhar Baddela 
> ---

I.e. here.

>  drivers/scsi/snic/snic.h   |  421 +
>  drivers/scsi/snic/snic_attrs.c |   80 
>  drivers/scsi/snic/snic_main.c  | 1022 
> 
>  drivers/scsi/snic/snic_os.h|   81 
>  4 files changed, 1604 insertions(+)
>  create mode 100644 drivers/scsi/snic/snic.h
>  create mode 100644 drivers/scsi/snic/snic_attrs.c
>  create mode 100644 drivers/scsi/snic/snic_main.c
>  create mode 100644 drivers/scsi/snic/snic_os.h

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 v2 1/9] snic: snic module infrastructure

2015-03-11 Thread Julian Calaby
Hi simha,

On Thu, Mar 12, 2015 at 12:37 PM, Narsimhulu Musini (nmusini)
 wrote:
> Hi,
>
>   Thanks for pointing the change log location. Do you suggest to resubmit the 
> entire patch series with v2. If you find some time, Could you please review 
> the patch.

Firstly, please don't top post (i.e. post above the reply) and please
don't take discussions off list. (always reply-to-all)

I can almost guarantee that you'll get more comments, so wait for
other people to review it first.

I don't know the SCSI code well enough to do a useful technical
review, so I only review for basic style issues.

Thanks,

Julian Calaby


> Thanks
> simha
>
>
> On 12-Mar-2015, at 6:24 am, Julian Calaby  wrote:
>
>> Hi,
>>
>> On Thu, Mar 12, 2015 at 4:01 AM, Narsimhulu Musini  wrote:
>>> snic_main.c contains module load and unload, global driver context,
>>> PCI Registration, PCI probe and remove, SCSI ML registration functionality.
>>>
>>> snic.h contains snic structure definition, snic global context, and
>>> prototypes.
>>>
>>> snic_os.h contains OS specific interfaces.
>>>
>>> snic_attrs.c contains device attributes to list snic state, link state,
>>> and driver version under /sys/class/scsi_host/host/
>>>
>>> v2
>>> Added Compile time macro for debugfs dependent functionality.
>>
>> Your changelog should be below the "---" and before the diffstat.
>>
>>> Signed-off-by: Narsimhulu Musini 
>>> Signed-off-by: Sesidhar Baddela 
>>> ---
>>
>> I.e. here.
>>
>>> drivers/scsi/snic/snic.h   |  421 +
>>> drivers/scsi/snic/snic_attrs.c |   80 
>>> drivers/scsi/snic/snic_main.c  | 1022 
>>> 
>>> drivers/scsi/snic/snic_os.h|   81 
>>> 4 files changed, 1604 insertions(+)
>>> create mode 100644 drivers/scsi/snic/snic.h
>>> create mode 100644 drivers/scsi/snic/snic_attrs.c
>>> create mode 100644 drivers/scsi/snic/snic_main.c
>>> create mode 100644 drivers/scsi/snic/snic_os.h
>>
>> Thanks,
>>
>> --
>> Julian Calaby
>>
>> Email: julian.cal...@gmail.com
>> Profile: http://www.google.com/profiles/julian.calaby/
>



-- 
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 v13 1/9] scsi: sr: support runtime pm

2013-01-20 Thread Julian Calaby
Hi Alan,

On Sun, Jan 20, 2013 at 5:46 AM, Alan Stern  wrote:
> On Sat, 19 Jan 2013, Aaron Lu wrote:
>> > closed.  Do we want to drop support for that kind of behavior?
>>
>> I don't think we should drop such support.
>> And the safest way to avoid such break is we refine the suspend
>> condition for ODD, and using what ZPODD defined condition isn't that
>> bad to me:
>> - for tray type, no media inside and tray close;
>> - for slot type, no media inside.
>> While whether tray is closed or not may not be that important, but at
>> least we should make sure there is no media inside.
>>
>> Thoughts?
>
> That sounds reasonable to me, at least as a first step.  If people want
> their CD drive to suspend, they can eject the disc.

Stupid question: does the kernel know if a CD has audio tracks?

(I'm assuming that nobody will access a data track without mounting it
or holding the device open)

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/
--
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 v13 1/9] scsi: sr: support runtime pm

2013-01-21 Thread Julian Calaby
Hi Aaron,

On Mon, Jan 21, 2013 at 7:14 PM, Aaron Lu  wrote:
> On Mon, Jan 21, 2013 at 02:31:50PM +1100, Julian Calaby wrote:
>> Hi Alan,
>>
>> On Sun, Jan 20, 2013 at 5:46 AM, Alan Stern  
>> wrote:
>> > On Sat, 19 Jan 2013, Aaron Lu wrote:
>> >> > closed.  Do we want to drop support for that kind of behavior?
>> >>
>> >> I don't think we should drop such support.
>> >> And the safest way to avoid such break is we refine the suspend
>> >> condition for ODD, and using what ZPODD defined condition isn't that
>> >> bad to me:
>> >> - for tray type, no media inside and tray close;
>> >> - for slot type, no media inside.
>> >> While whether tray is closed or not may not be that important, but at
>> >> least we should make sure there is no media inside.
>> >>
>> >> Thoughts?
>> >
>> > That sounds reasonable to me, at least as a first step.  If people want
>> > their CD drive to suspend, they can eject the disc.
>>
>> Stupid question: does the kernel know if a CD has audio tracks?
>
> Yes, cdrom module knows that, but the block driver(e.g. sr) doesn't.
> See cdrom_count_tracks in cdrom.c.
>
> May I know if you have an use case that you want to runtime suspend the
> cd drive with a disc inside? That would help us to refine the runtime
> suspend condition.

The discussion seemed to be restricting when the drive would be
suspended to only occasions where the drive is empty and, where
applicable, closed. I'll admit that I hadn't followed the discussion
enough to know what the restrictions were before that, but this seemed
to be a further restriction, therefore I assumed that you were
originally planning to be able to suspend the drive with a disk
inside.

I asked my question in the hope of someone setting up the compromise:
"we could suspend the drive only if the drive is empty and closed, or
a data disc is inside and nobody's using it."

Personally, providing nobody's using it, and relevant state is stored,
I can't see any reason why you can't suspend a drive with a disc in
it.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/
--
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 PATCH 76/71] ncr5380: Enable PDMA for DTC chips

2015-12-03 Thread Julian Calaby
Hi Finn, Ondrej,

One small question:

On Fri, Dec 4, 2015 at 10:03 AM, Ondrej Zary  wrote:
> Add I/O register mapping for DTC chips and enable PDMA mode.
>
> These chips have 16-bit wide HOST BUFFER register (counter register at
> offset 0x0d increments by 2 on each HOST BUFFER read).
>
> Large PIO transfers crash at least the DTCT-436P chip (all reads result
> in 0xFF) so this patch actually makes it work.
>
> The chip also crashes when we bang the C400 host status register too
> heavily after PDMA write - a small udelay is needed.
>
> Signed-off-by: Ondrej Zary 
> ---
> # hdparm -t --direct /dev/sdb
>
> /dev/sdb:
>  Timing O_DIRECT disk reads:   4 MB in  3.78 seconds =   1.06 MB/sec
>
>
>  drivers/scsi/NCR5380.h   |1 +
>  drivers/scsi/g_NCR5380.c |   47 
> +++---
>  2 files changed, 25 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c
> index fae4332..04f6c29 100644
> --- a/drivers/scsi/g_NCR5380.c
> +++ b/drivers/scsi/g_NCR5380.c
> @@ -415,7 +415,8 @@ static int __init generic_NCR5380_detect(struct 
> scsi_host_template *tpnt)
> hostdata->c400_blk_cnt = 1;
> hostdata->c400_host_buf = 4;
> }
> -   if (overrides[current_override].board == BOARD_NCR53C400A) {
> +   if (overrides[current_override].board == BOARD_NCR53C400A ||
> +   overrides[current_override].board == BOARD_DTC3181E) {

These if statements are starting to get a bit long, would it make
sense to replace them with a flag or equivalent?

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: [RFC PATCH 76/71] ncr5380: Enable PDMA for DTC chips

2015-12-04 Thread Julian Calaby
Hi Finn,

On Fri, Dec 4, 2015 at 7:38 PM, Finn Thain  wrote:
>
> On Fri, 4 Dec 2015, Julian Calaby wrote:
>
>> > -   if (overrides[current_override].board == BOARD_NCR53C400A) 
>> > {
>> > +   if (overrides[current_override].board == BOARD_NCR53C400A 
>> > ||
>> > +   overrides[current_override].board == BOARD_DTC3181E) {
>>
>> These if statements are starting to get a bit long, would it make
>> sense to replace them with a flag or equivalent?
>
> To what end? Shorter lines? As in,

Pretty much, each expression is quite long and they seem to be growing
fairly rapidly as you and Ondrej discover similar boards.

>
> if (board_is_ncr53c400a || board_is_dtc3181e) {
> /* ... */
> }
>
> I suppose that could be an improvement if new flags would entirely replace
> the override.board struct member and the existing switch statement,
>
> switch (overrides[current_override].board) {
> /* ... */
> }
>
> Or maybe you meant testing a new flag something like this,
>
> if (hostdata->ncr53c400_compatible) {
> /* ... */
> }
>
> If your concern is the Don't Repeat Yourself rule, I'm not sure that new
> flag would get tested more than once (?) And it would still have to be
> assigned using an "objectionably" long expression, e.g.
>
> hostdata->ncr53c400_compatible =
> overrides[current_override].board == BOARD_NCR53C400 ||
> overrides[current_override].board == BOARD_NCR53C400A ||
> overrides[current_override].board == BOARD_DTC3181E;
>
> Rather than add new flags, perhaps a 'switch' statement instead of an 'if'
> statement would be shorter (if the size of the expression is the problem).

I think switch statements would be cleaner in this particular
instance. I was thinking something like:

if (somthing->flags & NCR53C400_COMPATIBLE) {
/* ... */
}

but if it's only ever going to be used once, then it's pretty
pointless and switch statements are cleaner.

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: [RFC PATCH 76/71] ncr5380: Enable PDMA for DTC chips

2015-12-04 Thread Julian Calaby
Hi Finn,

On Sat, Dec 5, 2015 at 1:12 PM, Finn Thain  wrote:
>
> On Sat, 5 Dec 2015, Julian Calaby wrote:
>
>> Hi Finn,
>>
>> On Fri, Dec 4, 2015 at 7:38 PM, Finn Thain  
>> wrote:
>> >
>> > On Fri, 4 Dec 2015, Julian Calaby wrote:
>> >
>> >> > -   if (overrides[current_override].board == 
>> >> > BOARD_NCR53C400A) {
>> >> > +   if (overrides[current_override].board == 
>> >> > BOARD_NCR53C400A ||
>> >> > +   overrides[current_override].board == 
>> >> > BOARD_DTC3181E) {
>> >>
>> >> These if statements are starting to get a bit long, would it make
>> >> sense to replace them with a flag or equivalent?
>> >
>> > To what end? Shorter lines? As in,
>>
>> Pretty much, each expression is quite long and they seem to be growing
>> fairly rapidly as you and Ondrej discover similar boards.
>
> Each BOARD_* macro actually refers to a whole category of devices. No new
> boards, devices or categories of devices have been discovered.
>
> Ondrej is enabling and/or fixing PDMA functionality for three existing
> device categories, for which the driver already has a nominally compatible
> PDMA implementation.

I meant discovering boards which are similar.

Either way, I'm not sure it matters that much.

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 size problem

2015-12-17 Thread Julian Calaby
Hi Joao,

On Fri, Dec 18, 2015 at 3:04 AM, Joao Pinto  wrote:
> Hi,
> I have a patch ready to send which contains a new UFS driver, but the problem 
> is
> that the patch size is ~200K and my company' email security blocks it.
> Is it possible to send the driver in a tarball?

The usual practise in this case is to split it into patches per file
(I assume it's broken down into a few separate files to implement
different bits of functionality) sending the bits that link it into
the build system (Kconfig and Makefiles) last.

Alternatively, if you have a non-work account, you could send it
through that with appropriate From headers at the top of the email.

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: How to set DPOFUA=0 ?

2015-12-29 Thread Julian Calaby
Hi,

On Tue, Dec 29, 2015 at 10:30 PM, U.Mutlu  wrote:
> After this nigtmare-ish experience I would suggest the debian maintainers
> to trash the default jessie kernel 3.16 and replace it with the above
> 4.2 kernel; otherwise unexperienced users will wonder why their new SSD etc.
> isn't working in debian/ubuntu & co., as happened to me... :-(

Have you reported this as a bug against Jessie's kernel package?

I don't believe the Debian kernel maintainers read this mailing list.

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] add support for DWC UFS Host Controller

2016-02-01 Thread Julian Calaby
 UIC_ARG_MIB_SEL(TX_REFCLKFREQ,
> +   SELIND_LN0_TX), 0x01);
> +   if (ret)
> +   goto out;
> +
> +#ifdef CONFIG_SCSI_UFS_DWC_MPHY_TC_GEN2

Furthermore, the ARM developers are moving towards having single
kernels that support multiple different hardware platforms based on
the devicetree loaded at boot.

It's expected that a single kernel might have drivers to support
multiple different types of UFS hardware from multiple vendors.

Consequently, as the GEN2 hardware needs extra stuff, this stuff
should be enabled either by:
1. detecting it at runtime
2. adding a second compatible string and checking it where needed
3. using a separate driver with a different compatible string

[snip]

> @@ -5645,8 +6449,16 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem 
> *mmio_base, unsigned int irq)
>  */
> ufshcd_set_ufs_dev_poweroff(hba);
>
> +#ifndef CONFIG_SCSI_UFS_DWC_HOOKS
> async_schedule(ufshcd_async_scan, hba);
> -
> +#else
> +   /* Synopsys DWC Core + MPHY Test Chip needs a specific init routine */
> +   err = ufshcd_dwc_host_configuration(hba);
> +   if (err)
> +   dev_err(dev, "DWC host configuration failed\n");
> +   else
> +   dev_info(dev, "DWC host configuration successful\n");
> +#endif

This initialisation should be in your hardware specific code.

> return 0;
>
>  out_remove_scsi_host:
> diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
> index 0ae0967..9bf67fb 100644
> --- a/drivers/scsi/ufs/ufshci.h
> +++ b/drivers/scsi/ufs/ufshci.h
> @@ -72,8 +72,28 @@ enum {
> REG_UIC_COMMAND_ARG_1   = 0x94,
> REG_UIC_COMMAND_ARG_2   = 0x98,
> REG_UIC_COMMAND_ARG_3   = 0x9C,
> +
> +/* DWC UFS HC specific Registers */
> +#ifdef CONFIG_SCSI_UFS_DWC_HOOKS
> +   DWC_UFS_REG_HCLKDIV = 0xFC,
> +#endif

All the DWC specific registers and datatypes should be in a separate header.

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] add support for DWC UFS Host Controller

2016-02-02 Thread Julian Calaby
Hi Joao,

On Tue, Feb 2, 2016 at 9:22 PM, Joao Pinto  wrote:
> Hi Julian,
>
> Thanks for the review. My comments are below.
>
> On 2/2/2016 1:00 AM, Julian Calaby wrote:
>> Hi Joao,
>>
>> On Mon, Feb 1, 2016 at 11:47 PM, Joao Pinto  wrote:
>>> diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c
>>> index d15eaa4..0ee6c62 100644
>>> --- a/drivers/scsi/ufs/ufshcd-pci.c
>>> +++ b/drivers/scsi/ufs/ufshcd-pci.c
>>> @@ -167,6 +167,8 @@ static const struct dev_pm_ops ufshcd_pci_pm_ops = {
>>>
>>>  static const struct pci_device_id ufshcd_pci_tbl[] = {
>>> { PCI_VENDOR_ID_SAMSUNG, 0xC00C, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
>>> +   { PCI_VENDOR_ID_SYNOPSYS, 0xB101, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
>>> +   { PCI_VENDOR_ID_SYNOPSYS, 0xB102, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
>>
>> Listing these here implies that the devices these lines match are
>> "normal" PCI UFSHCD devices that don't require any special handling
>> whatsoever. Is that correct?
>
> Yes they are normal PCI UFSHCD devices.
>
>>
>> If they do require special handling, then you need to put them in a
>> separate driver, e.g. ufs-dwc-pci.c
>
> Both ufs-dwc and pci driver must execute the same init sequence to correctly
> kick-off the hardware. That's why the common code is in the ufshcd.c.
> Maybe create a ufshcd-dwc-quirks.c with the dwc specififc code would be 
> better.
> This way it could be used by ufs-dwc platform driver and pci.
> Since dwc hardware uses a specific init routine would it be better to have a
> ufs-dwc-pci and ufs-dwc calling the dwc specific init routine?

What you suggested below, i.e. having a ufshcd-dwc.c file containing
the common stuff then having separate platform and PCI drivers sounds
like the best option.

>>> { } /* terminate list */
>>>  };
>>>
>>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>>> index 85cd256..05d309d 100644
>>> --- a/drivers/scsi/ufs/ufshcd.c
>>> +++ b/drivers/scsi/ufs/ufshcd.c
>>> @@ -5521,6 +5541,790 @@ static struct devfreq_dev_profile 
>>> ufs_devfreq_profile = {
>>> .get_dev_status = ufshcd_devfreq_get_dev_status,
>>>  };
>>>
>>> +#ifdef CONFIG_SCSI_UFS_DWC_HOOKS
>>
>> This doesn't look right.
>>
>> The driver should be structured like this:
>>
>>  - ufs-dwc: contains everything that is specific to your hardware.
>>  - ufshcd: contains everything that is common to multiple types of
>> ufshcd from different vendors
>>
>> Your "hooks" here look like they're doing stuff that is specific to
>> the Designware hardware. They should not be in this file as it's for
>> hardware type independent code.
>>
>> If you need to do something special at some point in the common code,
>> then this should be exposed as an op in struct ufs_hba_variant_ops
>> which is then implemented in your device-specific code.
>
> Yes I agree that maybe the ufs core drive is not the perfect spot for specific
> vendor code. But DWC HW can be using pci or platform and so it has to share
> common code and that's why I put it in the ufshcd.
>
> I think creating a ufshcd-dwc.c would be better to share code between ufs-dwc
> and ufs-dwc-pci. Agree?

Agreed.

>>> +/**
>>> + * ufshcd_dwc_program_clk_div()
>>> + * This function programs the clk divider value. This value is needed to
>>> + * provide 1 microsecond tick to unipro layer.
>>> + * @hba: Private Structure pointer
>>> + * @divider_val: clock divider value to be programmed
>>> + *
>>> + */
>>> +void ufshcd_dwc_program_clk_div(struct ufs_hba *hba, u32 divider_val)
>>> +{
>>> +   ufshcd_writel(hba, divider_val, DWC_UFS_REG_HCLKDIV);
>>> +}
>>> +
>>> +/**
>>> + * ufshcd_dwc_link_is_up()
>>> + * Check if link is up
>>> + * @hba: private structure poitner
>>> + *
>>> + * Returns 0 on success, non-zero value on failure
>>> + */
>>> +int ufshcd_dwc_link_is_up(struct ufs_hba *hba)
>>> +{
>>> +   int dme_result = 0;
>>> +
>>> +   ufshcd_dme_get(hba, UIC_ARG_MIB(VS_POWERSTATE), &dme_result);
>>> +
>>> +   if (dme_result == UFSHCD_LINK_IS_UP) {
>>> +   ufshcd_set_link_active(hba);
>>> +   return 0;
>>> +   }
>>> +
>>> +   return 1;
>>> +}
>>> +
>>> +/**
>>> + * ufshcd

Re: [PATCH] add support for DWC UFS Host Controller

2016-02-02 Thread Julian Calaby
Hi Joao,

On Tue, Feb 2, 2016 at 10:47 PM, Joao Pinto  wrote:
>
> Hi Julian,
> I am already changing the architecture and I will send a v2 soon.
> Thanks for the review.

Awesome, I look forward to it.

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] add support for DWC UFS Host Controller

2016-02-02 Thread Julian Calaby
Hi Joao,

On Wed, Feb 3, 2016 at 1:47 AM, Joao Pinto  wrote:
> Hi,
> In order to make a ufs-dwc-pci glue driver I will need to create a "pci driver
> lib" like we have already for platform (ufshcd-pltfm.c). Should I call the
> samsung glue driver ufs-samsung-pci.c which will use common pci functions 
> from a
> ufshcd-pci.c? Agree?

That sounds sensible.

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 3/7] scsi: Drop runtime PM usage count after host is added

2016-02-18 Thread Julian Calaby
Hi Mika,

On Thu, Feb 18, 2016 at 7:54 PM, Mika Westerberg
 wrote:
> Runtime PM of the SCSI host is already handled by calls to
> scsi_autopm_get_host() and scsi_autopm_put_host() from appropriate places
> whenever the host needs to be powered on. This works fine when there is
> device connected to the host as once it runtime suspends the host will too.
>
> However, if there is no device connected the host is never runtime
> suspended (the usage counter is always 0).
>
> Allow runtime suspend of host even if it has no devices connected by
> calling scsi_autopm_put_host() at the end of scsi_add_host_with_dma(). We
> temporarily increase runtime PM usage counter first so call to
> scsi_autopm_put_host() will result idle request to be scheduled for the
> device.
>
> Signed-off-by: Mika Westerberg 
> ---
>  drivers/scsi/hosts.c | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
> index 82ac1cd818ac..e46bf4d152a0 100644
> --- a/drivers/scsi/hosts.c
> +++ b/drivers/scsi/hosts.c
> @@ -250,6 +250,12 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, 
> struct device *dev,
> if (error)
> goto out_destroy_freelist;
>
> +   /*
> +* Increase usage count temporarily here so that calling
> +* scsi_autopm_put_host() will trigger runtime idle if there is
> +* nothing else preventing suspending the device.
> +*/
> +   pm_runtime_get_noresume(&shost->shost_gendev);
> pm_runtime_set_active(&shost->shost_gendev);
> pm_runtime_enable(&shost->shost_gendev);
> device_enable_async_suspend(&shost->shost_gendev);
> @@ -290,6 +296,7 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, 
> struct device *dev,
> goto out_destroy_host;
>
> scsi_proc_host_add(shost);
> +   scsi_autopm_put_host(shost);

Would it be cleaner to export the code that runs when the usage
counter decrements and call it 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: RFC: reduce CONFIG_SCSI_CONSTANTS impact by 4k

2015-10-05 Thread 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: RFC: reduce CONFIG_SCSI_CONSTANTS impact by 4k

2015-10-06 Thread Julian Calaby
Hi Rasmus,

On Wed, Oct 7, 2015 at 2:39 AM, Rasmus Villemoes
 wrote:
> On Tue, Oct 06 2015, Julian Calaby  wrote:
>
>> Hi Rasmus,
>>
>>>
>>> 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.
>
> Well, no, I account for the nul bytes where I define the table (the
> comment actually says as much). sizeof("foo") is 4. Since
> additional_text ends up pointing to a string containing
>
> "foo" "\0" "xyzzy" "\0" "..." "\0"
>
> aka
>
> "foo\0xyzzy\0...\0"
>
> this is the right amount to skip. As I said in the cover letter, I did
> test this (so that I'd at least catch silly off-by-ones), and I do get
> the right strings out.

Ah, that makes sense. It just didn't look right.

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 for-next] scsi: qla2xxx: Hide unavailable firmware

2015-10-07 Thread Julian Calaby
Hi Xose,

On Thu, Oct 8, 2015 at 2:13 AM, Xose Vazquez Perez
 wrote:
> On Fri, May 22, 2015 at 10:00 AM, Julian Calaby  
> wrote:
>
>> Some qla2xxx devices have firmware stored in flash on the device,
>> however for debugging and triage purposes, Qlogic staff like to
>> be able to load known-good versions of these firmwares through
>> request_firmware().
>>
>> These firmware files were never distributed and are unlikely to ever
>> be released publically, so to hide these missing firmware files from
>> scripts which check such things, (e.g. Debian's initramfs-tools) put
>> them behind a new EXPERT Kconfig option.
>
>
> What is state of this patch ?

Apparently nobody cared, either from qLogic or linux-scsi.

I'm not overly fussed whether it goes in or not, it was more a point
in the discussion that proceeded it, however it does solve the
problems in the discussion that preceded it.

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 for-next] scsi: qla2xxx: Hide unavailable firmware

2015-10-08 Thread Julian Calaby
Hi James,

On Fri, Oct 9, 2015 at 3:17 AM, James Bottomley
 wrote:
> On Thu, 2015-10-08 at 15:46 +, Himanshu Madhani wrote:
>>
>> On 10/7/15, 4:41 PM, "Julian Calaby"  wrote:
>>
>> >Hi Xose,
>> >
>> >On Thu, Oct 8, 2015 at 2:13 AM, Xose Vazquez Perez
>> > wrote:
>> >> On Fri, May 22, 2015 at 10:00 AM, Julian Calaby
>> >> wrote:
>> >>
>> >>> Some qla2xxx devices have firmware stored in flash on the device,
>> >>> however for debugging and triage purposes, Qlogic staff like to
>> >>> be able to load known-good versions of these firmwares through
>> >>> request_firmware().
>> >>>
>> >>> These firmware files were never distributed and are unlikely to ever
>> >>> be released publically, so to hide these missing firmware files from
>> >>> scripts which check such things, (e.g. Debian's initramfs-tools) put
>> >>> them behind a new EXPERT Kconfig option.
>> >>
>> >>
>> >> What is state of this patch ?
>> >
>> >Apparently nobody cared, either from qLogic or linux-scsi.
>> >
>> >I'm not overly fussed whether it goes in or not, it was more a point
>> >in the discussion that proceeded it, however it does solve the
>> >problems in the discussion that preceded it.
>>
>> This patch Looks good.
>>
>> Acked-By: Himanshu Madhani 
>
> Actually, this isn't helpful.  You now add another option over which the
> distributions have to make a choice.  Is this interface necessary and
> useful?  If yes, then it should be compiled in and if not, just remove
> it ... don't do death by 1000 Kconfig options.

The original issue here was that the qla2xxx driver specifies two
firmware files which are not publicly available.

They aren't publicly available because the hardware that uses them
always stores it's firmware in flash.

The firmware files are specified in the driver because qLogic support
likes to be able to load a known-good firmware file from disk for
debugging faulty hardware.

Some distributions (Debian is a good example) will complain vocally
about missing firmware files when installing kernels or producing
initramfs images.

This patch was intended as an ugly compromise between all of this.

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 13/25] scsi: hisi_sas: add path from phyup irq to SAS framework

2015-10-12 Thread Julian Calaby
Hi kbuild test robot people,

On Tue, Oct 13, 2015 at 9:03 AM, kbuild test robot  wrote:
> Hi John,
>
> [auto build test WARNING on scsi/for-next -- if it's inappropriate base, 
> please suggest rules for selecting the more suitable base]
>
> url:
> https://github.com/0day-ci/linux/commits/John-Garry/HiSilicon-SAS-driver/20151012-231929
> reproduce:
> # apt-get install sparse
> make ARCH=x86_64 allmodconfig
> make C=1 CF=-D__CHECK_ENDIAN__
>
>
> sparse warnings: (new ones prefixed by >>)

Your script seems to have lost the actual warnings here.

> Please review and possibly fold the followup patch.

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/2] qla2xxx: Remove unavailable firmware files

2015-10-14 Thread Julian Calaby
Hi Himanshu,

On Thu, Oct 15, 2015 at 2:57 AM, Himanshu Madhani
 wrote:
> Remove firmware binary names for the ISPs, which
> are not submitted to linux-firmware
>
> Signed-off-by: Himanshu Madhani 
> Signed-off-by: Giridhar Malavali 

For what it's worth, this looks good to me.

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 v2] qla2xxx: Remove unavailable firmware files

2015-11-17 Thread Julian Calaby
Hi Himanshu,

On Wed, Nov 18, 2015 at 7:44 AM, Himanshu Madhani
 wrote:
> Remove firmware binary names for the ISPs, which
> are not submitted to linux-firmware
>
> Signed-off-by: Himanshu Madhani 
> Signed-off-by: Giridhar Malavali 
> Reviewed-by: Julian Calaby 

The patch looks good so for what it's worth:

Reviewed-by: Julian Calaby 

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] SMP request handlers need to update resid

2007-12-19 Thread Julian Calaby
On Dec 20, 2007 1:44 PM, FUJITA Tomonori <[EMAIL PROTECTED]> wrote:
> --- a/drivers/message/fusion/mptsas.c
> +++ b/drivers/message/fusion/mptsas.c
> @@ -1343,6 +1343,8 @@ static int mptsas_smp_handler(struct Scsi_Host *shost, 
> struct sas_rphy *rphy,
> smprep = (SmpPassthroughReply_t *)ioc->sas_mgmt.reply;
> memcpy(req->sense, smprep, sizeof(*smprep));
> req->sense_len = sizeof(*smprep);
> +   req->data_len = 0;
> +   rsp->data_len -= smprep->ResponseDataLength;

Would

+   rsp->data_len = -smprep->ResponseDataLength;

not work?

Thanks,

-- 

Julian Calaby

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


Re: [PATCH] SMP request handlers need to update resid

2007-12-19 Thread Julian Calaby
On Dec 20, 2007 2:20 PM, FUJITA Tomonori <[EMAIL PROTECTED]> wrote:
>
> On Thu, 20 Dec 2007 14:13:15 +1100
> "Julian Calaby" <[EMAIL PROTECTED]> wrote:
>
> > On Dec 20, 2007 1:44 PM, FUJITA Tomonori <[EMAIL PROTECTED]> wrote:
> > > --- a/drivers/message/fusion/mptsas.c
> > > +++ b/drivers/message/fusion/mptsas.c
> > > @@ -1343,6 +1343,8 @@ static int mptsas_smp_handler(struct Scsi_Host 
> > > *shost, struct sas_rphy *rphy,
> > > smprep = (SmpPassthroughReply_t *)ioc->sas_mgmt.reply;
> > > memcpy(req->sense, smprep, sizeof(*smprep));
> > > req->sense_len = sizeof(*smprep);
> > > +   req->data_len = 0;
> > > +   rsp->data_len -= smprep->ResponseDataLength;
> >
> > Would
> >
> > +   rsp->data_len = -smprep->ResponseDataLength;
> >
> > not work?

Oh ! I'm totally blind! they're completely different variables! Argh!

Sorry for the noise,

-- 

Julian Calaby

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


Re: [PATCH ver3 1/5] scsi_error: code cleanup before refactoring of scsi_send_eh_cmnd()

2007-09-11 Thread Julian Calaby
(added CCs - that's what you get for sending emails after 5.)

On 9/11/07, Boaz Harrosh <[EMAIL PROTECTED]> wrote:
>
>  static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
> -int cmnd_size, int timeout, int copy_sense)
> +int cmnd_size, int timeout, unsigned sense_bytes)

Shouldn't that be unsigned _int_?

Thanks,

--

Julian Calaby

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


Re: aic79xx problems

2007-09-20 Thread Julian Calaby
On 9/21/07, Stefan Boresch <[EMAIL PROTECTED]> wrote:
>
> I will try, but a very dumb question first. If I have a kernel that
> crashes because of a disk problem ..., how do I get to save the output?
> (Never worked with a serial console, but there might be a first time
> for everything ...) But maybe there is an obvious solution that I overlook?
>
> Or should I just try to watch and look for certain messages (ordering of
> messages), which given the huge amount of output seems a bit daunting ...

Ok, in response to your dumb question, an answer from another dumb user: (me)

Firstly, serial consoles aren't that difficult to set up - all you
need is a null modem cable and suitable software on the other end.
Then ensure that it's built in and pass "console=ttyS0" (or
"console=ttyS1" if you're using serial port #2) when you boot.

There is a network console - but I'm not sure whether that is useful
for boot messages.

Hope that helps =)

--

Julian Calaby

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


Re: What still uses the block layer?

2007-10-15 Thread Julian Calaby
On 10/15/07, Rob Landley <[EMAIL PROTECTED]> wrote:
> I note that the eth0 and eth1 names are dynamically assigned on a first come
> first serve basis (like scsi).  This never causes me a problem because the
> driver loading order is constant, and once you figure out that eth0 is
> gigabit and eth1 is the 80211g it _stays_ that way across reboots, reliably.
> Yeah, it's a heuristic.  Hands up everybody relying on such a heuristic in
> the real world.

Umm, not quite, from my experiences with pre-production wireless
drivers, (another story, another time) fancy stuff is being done in
udev to make sure that your gigabit card is always assigned to eth0.

-- 

Julian Calaby

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


Re: What still uses the block layer?

2007-10-15 Thread Julian Calaby
[adding back CCs which were dropped because I'm stupid - sorry!]

On 10/16/07, Rob Landley <[EMAIL PROTECTED]> wrote:
> On Monday 15 October 2007 5:27:55 am Julian Calaby wrote:
> > On 10/15/07, Rob Landley <[EMAIL PROTECTED]> wrote:
> > > On Monday 15 October 2007 4:06:20 am Julian Calaby wrote:
> > > > On 10/15/07, Rob Landley <[EMAIL PROTECTED]> wrote:
> > > > > I note that the eth0 and eth1 names are dynamically assigned on a
> > > > > first come first serve basis (like scsi).  This never causes me a
> > > > > problem because the driver loading order is constant, and once you
> > > > > figure out that eth0 is gigabit and eth1 is the 80211g it _stays_
> > > > > that way across reboots, reliably. Yeah, it's a heuristic.  Hands up
> > > > > everybody relying on such a heuristic in the real world.
> > > >
> > > > Umm, not quite, from my experiences with pre-production wireless
> > > > drivers, (another story, another time) fancy stuff is being done in
> > > > udev to make sure that your gigabit card is always assigned to eth0.
> > >
> > > I remember building a 2.4 kernel, statically linking in all the drivers,
> > > and getting the ethernet devices showing up in a reliable order for
> > > years.  Where does the need for fancy stuff come in?
> >
> > I remember that too. In fact, I have had no issues with network card
> > enumeration order, outside my own inexperience and stupidity.
> >
> > However, this sort of thing is needed now because of the various types
> > of hotpluggable networking devices, e.g. USB 802.11 cards, USB
> > ethernet cards, PCMCIA, etc.
>
> I thought the strategy was just to scan the hotpluggable busses after the
> non-hotpluggable busses.

My (practical) experience is that I couldn't guarantee which card was
which. (I remember once where it changed over a kernel re-compile) So
my solution, before Debian's persistent naming scheme appeared, was to
check it after every new kernel and make sure my config matched up
with the names of the physical interfaces.

> > And yes, PCMCIA worked fine for ages, but
> > usually you'd never have more than one PCMCIA network card.
>
> Still don't, but presumably the slots are scanned in a reliable order so if
> the cards are always present on bootup in the same slots, they'd stay in that
> order.

Well, yes and no. My gut feeling is that it's probed like PCI cards
are. They're initialised when the drivers are loaded, and not before,
as such, there are no guarantees which card will be initialised first.
- and anyway, what happens if you plug them in in a different order?

> > Personally, I use 2 different usb network cards, and I'm quite
> > comforted to know that the 802.11a one is always wlan0, and the
> > 802.11b/g one is always wlan1.
>
> So if I have a USB 100baseT adapter, and I boot with it plugged in, it'll
> potentially come before my built-in wireless card due to ordering based on
> device type?

Ok, firstly the 100baseT adapter will be named something like ethX,
the wireless card will most likely be named something like wlanX.

Now let's say your laptop has a built in ethernet card.

So, we'll assume a modular kernel, with the module "usbnet" for the
usb card and "e100" for the onboard card:

If the "usbnet" module is loaded first, then initially, according to
the kernel, the usb card will be eth0 and the built in one eth1.

Now let's assume that, on the PCI bus, the USB controller is in a
lower slot number than the network card. (highly likely, given that
the network card is most likely external to the chipset of the laptop)
It's pretty likely that the USB controller will have it's module
loaded first, before the built in network card. At this point, it'll
send out hotplug events for all it's children (root hubs, etc.) and
eventually an event will be sent out for the usb network card. Now, at
this point, it's impossible to say which one will claim eth0 first.

Now, in my case, with my two wireless cards, what happens if I plug
the 802.11b/g one in first? If this fancy renaming didn't happen, it'd
end up with the name wlan0 and, hence, try to connect to the network
which the 802.11a one is supposed to connect to.

This is not a good thing.

I also have to make the point that this has been happening all over
the kernel, well before I started using it. Video4Linux and DVB
devices can be USB, and the order the /dev/videoX nodes appear in is
determined by the plugging order. IRDA cards, sound cards, usb
devices, framebuffers, mice, keyboards, loopback devices, etc. all
have the same "is

Re: [PATCH 00/22] Add and use pci_zalloc_consistent

2014-06-23 Thread Julian Calaby
Hi Joe,

On Tue, Jun 24, 2014 at 5:13 AM, Joe Perches  wrote:
> On Mon, 2014-06-23 at 10:25 -0700, Luis R. Rodriguez wrote:
>> On Mon, Jun 23, 2014 at 06:41:28AM -0700, Joe Perches wrote:
>> > Adding the helper reduces object code size as well as overall
>> > source size line count.
>> >
>> > It's also consistent with all the various zalloc mechanisms
>> > in the kernel.
>> >
>> > Done with a simple cocci script and some typing.
>>
>> Awesome, any chance you can paste in the SmPL? Also any chance
>> we can get this added to a make coccicheck so that maintainers
>> moving forward can use that to ensure that no new code is
>> added that uses the old school API?
>
> Not many of these are recent.
>
> Arnd Bergmann reasonably suggested that the pci_alloc_consistent
> api be converted the the more widely used dma_alloc_coherent.
>
> https://lkml.org/lkml/2014/6/23/513
>
>> Shouldn't these drivers just use the normal dma-mapping API now?
>
> and I replied:
>
> https://lkml.org/lkml/2014/6/23/525
>
>> Maybe.  I wouldn't mind.
>> They do seem to have a trivial bit of unnecessary overhead for
>> hwdev == NULL ? NULL : &hwdev->dev
>
> Anyway, here's the little script.
> I'm not sure it's worthwhile to add it though.
>
> $ cat ./scripts/coccinelle/api/alloc/pci_zalloc_consistent.cocci
> ///
> /// Use pci_zalloc_consistent rather than
> /// pci_alloc_consistent followed by memset with 0
> ///
> /// This considers some simple cases that are common and easy to validate
> /// Note in particular that there are no ...s in the rule, so all of the
> /// matched code has to be contiguous
> ///
> /// Blatantly cribbed from: scripts/coccinelle/api/alloc/kzalloc-simple.cocci
>
> @@
> type T, T2;
> expression x;
> expression E1,E2,E3;
> statement S;
> @@
>
> - x = (T)pci_alloc_consistent(E1,E2,E3);
> + x = pci_zalloc_consistent(E1,E2,E3);
>   if ((x==NULL) || ...) S
> - memset((T2)x,0,E2);

I don't know much about SmPL, but wouldn't having that if statement
there reduce your matches?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/
--
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] zfcp: Stop system after memory corruption

2007-05-07 Thread Julian Calaby

On 5/8/07, Swen Schillig <[EMAIL PROTECTED]> wrote:

From: Christof Schmitt <[EMAIL PROTECTED]>

[snip]

-   ZFCP_LOG_NORMAL("bug: unexpected inbound "
-   "packet on adapter %s "
-   "(reqid=0x%lx, "
-   "first_element=%d, "
-   "elements_processed=%d)\n",
-   
zfcp_get_busid_by_adapter(adapter),
-   (unsigned long) buffere->addr,
-   first_element,
-   elements_processed);
-   ZFCP_LOG_NORMAL("hex dump of inbound buffer "
-   "at address %p "
-   "(buffer_index=%d, "
-   "buffere_index=%d)\n", buffer,
-   buffer_index, buffere_index);
-   ZFCP_HEX_DUMP(ZFCP_LOG_LEVEL_NORMAL,
- (char *) buffer, SBAL_SIZE);


As a minor quibble, may I suggest that these lines belong just above
the panic introduced above? I assume that they'd be useful for
debugging should this situation occur.

If I'm completely mis-understanding the situation, please disregard this email.

Thanks,

--

Julian Calaby

Email: [EMAIL PROTECTED]

(please note that I'm only subscribed to linux-scsi if replying)
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] zfcp: Stop system after memory corruption

2007-05-08 Thread Julian Calaby

On 5/8/07, Christof Schmitt <[EMAIL PROTECTED]> wrote:

On Tue, May 08, 2007 at 01:56:07AM +1000, Julian Calaby wrote:
> >From: Christof Schmitt <[EMAIL PROTECTED]>
> [snip]
> >-   ZFCP_LOG_NORMAL("bug: unexpected inbound "
> >-   "packet on adapter %s "
> >-   "(reqid=0x%lx, "
> >-   "first_element=%d, "
> >-   "elements_processed=%d)\n",
[...]
> As a minor quibble, may I suggest that these lines belong just above
> the panic introduced above? I assume that they'd be useful for
> debugging should this situation occur.

If this problem occurs, the administrator of the Linux system will be
advised to dump the Linux system and to also capture the logs from the
FCP adapter. The memory dump already contains the information about the
unexpected packet and also information about the last requests
sent to the FCP adapter. So, the messages do not provide additional
debug data, and i removed them to simplify the code.


Ah!

Thanks,

--

Julian Calaby

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


Re: scsi target, likely GPL violation

2012-11-11 Thread Julian Calaby
Hi Lawrence,

On Mon, Nov 12, 2012 at 9:13 AM, Lawrence Rosen  wrote:
> Alan Cox wrote:
>> So either your work is truely not derivative of the kernel (which I find
>> wildly improbable) or you have a problem and since you are aware
>> of the complaints publically I guess probably a triple damages sized
>> problem. But that's one for your lawyers and whatever opinion they
>> have on the subject.
>
> Hi Alan and others,
>
> I've been advising Rising Tide Systems (RTS) in this matter. Please let me
> reassure you that RTS is acting on advice of counsel.

It's nice to hear from legal counsel on this matter.

I don't think that the *usage* of the kernel APIs is the biggest issue
here. There are many examples where proprietary code uses these APIs
and is not violating the GPL.

As I see it, one of the main concerns is because the proprietary and
in-kernel target systems are, from what I understand, quite similar,
there is the possibility that GPL licensed contributions to the
in-kernel target code may have "leaked" into to the proprietary code.
That said, proving this is a very difficult problem, but the question
must still be asked:

Can Rising Tide Systems assure us that there is no GPL licensed code
within their proprietary target code?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/
--
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/4] Fix performance burning or extracting audio etc. from multiple optical drives.

2014-11-26 Thread Julian Calaby
Hi Tim,

On Thu, Nov 27, 2014 at 2:33 AM, Tim Small  wrote:
> On 25/11/14 16:30, Jens Axboe wrote:
>
>> do we really need to do paride here?
>
> I did consider this, but I made the change there too on the basis that:
>
> . paride has received a few commits this year (and is listed as being
> maintained)
> . The change is trivial
> . It fixes a performance regression which was introduced during the BKL
> removal (mutex being retained by sleeping processes).
>
> I'm happy to drop it, if you prefer.
>
>> Patches 2-4 have identical subjects, and no commit message...
>
> Sorry about that, will fix it with next version.
>
> Having just seen this thread from 2013:
>
> http://permalink.gmane.org/gmane.linux.scsi/79483
>
> I decided to exercise the eject code path a bit more by triggering
> simultaneous eject commands on all 11 optical drives in my test box,
> followed by simultaneous close-tray commands, repeatedly.
>
> I haven't been able to reproduce the error reported in that email, but
> from observing the behaviour of the drives it looks like access to pata
> drives is being serialising elsewhere, so the issue in that link may
> have been fixed?
>
> Unfortunately running these tests did eventually make all further
> attempts to open /dev/sr* block on my test box.
>
> I've stared at the code for a while, but not making any headway
> currently, except that a blocking blk_execute_rq (called by
> test_unit_ready) is then causing all over cdrom open/close calls to
> block (because sr_mutex is held by sr_block_open(), and in turn calls
> check_disk_change... scsi_test_unit_ready).
>
> How do I work out why blk_execute_rq is blocking?

As you're playing with locks, I assume you're running with LOCKDEP
enabled? If not, that might tell you what's going on.

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 0/20] fix misspelling of current function in string

2014-12-07 Thread Julian Calaby
Hi Julia,

On Mon, Dec 8, 2014 at 6:20 AM, Julia Lawall  wrote:
> These patches replace what appears to be a reference to the name of the
> current function but is misspelled in some way by either the name of the
> function itself, or by %s and then __func__ in an argument list.

Would there be any value in doing this for _all_ cases where the
function name is written in a format string?

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 0/20] fix misspelling of current function in string

2014-12-09 Thread Julian Calaby
Hi Julia,

On Mon, Dec 8, 2014 at 5:43 PM, Julia Lawall  wrote:
> On Mon, 8 Dec 2014, Julian Calaby wrote:
>
>> Hi Julia,
>>
>> On Mon, Dec 8, 2014 at 6:20 AM, Julia Lawall  wrote:
>> > These patches replace what appears to be a reference to the name of the
>> > current function but is misspelled in some way by either the name of the
>> > function itself, or by %s and then __func__ in an argument list.
>>
>> Would there be any value in doing this for _all_ cases where the
>> function name is written in a format string?
>
> Probably.  But there are a lot of them.  Even for the misspellings, I have
> only don about 1/3 of the cases.
>
> On the other hand, the misspelling have to be checked carefully, because a
> misspelling of one thing could be the correct spelling of the thing thst
> was actually intended.
>
> Joe, however, points out that a lot of these prints are just for function
> tracing, and could be removed.  I worked on another semantic patch that
> tries to do that.  It might be better to remove those prints completely,
> rather than sending one patch to transform them and then one patch to
> remove them after that.  That is why for this series I did only the ones
> where there was actually a problem.

Ok, that makes sense.

Either way though, this is a really interesting application of the
semantic patching. Nice work!

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] scsi:stex.c Support Pegasus 3 product

2016-06-13 Thread Julian Calaby
Hi Charles,

On Mon, Jun 13, 2016 at 9:40 PM, Charles Chiou  wrote:
> Hi Julian,
>
>
> On 06/10/2016 08:10 AM, Julian Calaby wrote:
>>
>> Hi Charles,
>>
>> On Mon, Jun 6, 2016 at 5:53 PM, Charles Chiou 
>> wrote:
>>>
>>> From: Charles 
>>>
>>> Pegasus series is a RAID support product by using Thunderbolt technology.
>>>
>>> The newest product, Pegasus 3 is support Thunderbolt 3 technology with
>>> another chip.
>>>
>>> 1.Change driver version.
>>>
>>> 2.Add Pegasus 3 VID, DID and define it's device address.
>>>
>>> 3.Pegasus 3 use msi interrupt, so stex_request_irq P3 type enable msi.
>>>
>>> 4.For hibernation, use msi_lock in stex_ss_handshake to prevent msi
>>> register write again when handshaking.
>>>
>>> 5.Pegasus 3 don't need read() as flush.
>>>
>>> 6.In stex_ss_intr & stex_abort, P3 only clear interrupt register when
>>> getting vendor defined interrupt.
>>>
>>> 7.Add reboot notifier and register it in stex_probe for all supported
>>> device.
>>>
>>> 8.For all supported device in restart flow, we get a callback from
>>> notifier and set S6flag for stex_shutdown & stex_hba_stop to send restart
>>> command to FW.
>>>
>>> Signed-off-by: Charles 
>>> Signed-off-by: Paul 
>>> ---
>>>   drivers/scsi/stex.c | 282
>>> +++-
>>>   1 file changed, 214 insertions(+), 68 deletions(-)
>>>
>>> diff --git a/drivers/scsi/stex.c b/drivers/scsi/stex.c
>>> index 5b23175..9de2de2 100644
>>> --- a/drivers/scsi/stex.c
>>> +++ b/drivers/scsi/stex.c
>>> @@ -87,7 +95,7 @@ enum {
>>>  MU_STATE_STOP   = 5,
>>>  MU_STATE_NOCONNECT  = 6,
>>>
>>> -   MU_MAX_DELAY= 120,
>>> +   MU_MAX_DELAY= 50,
>>
>>
>> This won't cause problems for older adapters, right?
>
>
> Correct.
>
>>
>>>  MU_HANDSHAKE_SIGNATURE  = 0x5555,
>>>  MU_HANDSHAKE_SIGNATURE_HALF = 0x5a5a,
>>>  MU_HARD_RESET_WAIT  = 3,
>>> @@ -540,11 +556,15 @@ stex_ss_send_cmd(struct st_hba *hba, struct req_msg
>>> *req, u16 tag)
>>>
>>>  ++hba->req_head;
>>>  hba->req_head %= hba->rq_count+1;
>>> -
>>> -   writel((addr >> 16) >> 16, hba->mmio_base + YH2I_REQ_HI);
>>> -   readl(hba->mmio_base + YH2I_REQ_HI); /* flush */
>>> -   writel(addr, hba->mmio_base + YH2I_REQ);
>>> -   readl(hba->mmio_base + YH2I_REQ); /* flush */
>>> +   if (hba->cardtype == st_P3) {
>>> +   writel((addr >> 16) >> 16, hba->mmio_base + YH2I_REQ_HI);
>>> +   writel(addr, hba->mmio_base + YH2I_REQ);
>>> +   } else {
>>> +   writel((addr >> 16) >> 16, hba->mmio_base + YH2I_REQ_HI);
>>> +   readl(hba->mmio_base + YH2I_REQ_HI); /* flush */
>>> +   writel(addr, hba->mmio_base + YH2I_REQ);
>>> +   readl(hba->mmio_base + YH2I_REQ); /* flush */
>>> +   }
>>
>>
>> The first writel() lines in each branch of the if statement are
>> identical, so they could be outside of it.
>
>
> I'll revise it in next patch.

On second thought, don't worry about doing this, keep the two
(slightly) different sets of code separate.

>>
>> Would it make sense to add a helper that does the readl() flush only
>> for non-st_P3? This could be a function pointer in the hba structure
>> which shouldn't slow stuff down.
>>
>
> Would you means to register another function pointer in struct "struct
> st_card_info" then point to hba strucrure for non-st_P3?
>
> struct st_card_info {
> struct req_msg * (*alloc_rq) (struct st_hba *);
> int (*map_sg)(struct st_hba *, struct req_msg *, struct st_ccb *);
> void (*send) (struct st_hba *, struct req_msg *, u16);
> unsigned int max_id;
> unsigned int max_lun;
> unsigned int max_channel;
> u16 rq_count;
> u16 rq_size;
> u16 sts_count;
> };

Again, on second thought, don't worry about it.

>>>   }
>>>
>>>   st

Re: [PATCH] VMW_PVSCSI: Change to update maintainer details (name, email)

2016-06-16 Thread Julian Calaby
Hi Jim,

On Fri, Jun 17, 2016 at 11:05 AM, Jim Gill  wrote:
> From 6a076cc00ec12c6f9cba58ee7e4c3dec49e1e7e5 Mon Sep 17 00:00:00 2001
> From: Jim Gill 
> Date: Thu, 16 Jun 2016 14:10:43 -0700
> Subject: [PATCH] VMW_PVSCSI: Change to update maintainer details (name,
> email)
>
> Signed-off-by: Jim Gill 
> ---
>  drivers/scsi/vmw_pvscsi.c | 2 +-
>  drivers/scsi/vmw_pvscsi.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/vmw_pvscsi.c b/drivers/scsi/vmw_pvscsi.c
> index 6164634..4a0d3cd 100644
> --- a/drivers/scsi/vmw_pvscsi.c
> +++ b/drivers/scsi/vmw_pvscsi.c
> @@ -17,7 +17,7 @@
>   * along with this program; if not, write to the Free Software
>   * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301
> USA.
>   *
> - * Maintained by: Arvind Kumar 
> + * Maintained by: Jim Gill 

Shouldn't you update MAINTAINERs too? And isn't having this
information in these files redundant?

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] VMW_PVSCSI: Change to update maintainer details (name, email)

2016-06-16 Thread Julian Calaby
Hi Arvind,

On Fri, Jun 17, 2016 at 12:03 PM, Arvind Kumar  wrote:
> Hi Julian,
>
> Thanks for spotting that. We will update that too.
>
> I don't really see it as redundant rather as a quick reference instead of 
> digging out the MAINTAINERS file and then search for vmw_pvscsi.c file. 
> Specially for those readers who might not even know about the MAINTAINERS 
> file.

You can use the get_maintainers script in the scripts directory to
quickly get information from MAINTAINERS, i.e.:

./scripts/get_maintainers.pl -f drivers/scsi/vmw_pvscsi.c

or

./scripts/get_maintainers.pl patchfile.patch

Thanks,

Julian Calaby


>
> Thanks!
> Arvind
> ________
> From: Julian Calaby 
> Sent: Thursday, June 16, 2016 6:48 PM
> To: Jim Gill
> Cc: j...@linux.vnet.ibm.com; Martin K. Petersen; Arvind Kumar; 
> pv-driv...@vmware.com; linux-scsi; linux-ker...@vger.kernel.org
> Subject: Re: [PATCH] VMW_PVSCSI: Change to update maintainer details (name, 
> email)
>
> Hi Jim,
>
> On Fri, Jun 17, 2016 at 11:05 AM, Jim Gill  wrote:
>> From 6a076cc00ec12c6f9cba58ee7e4c3dec49e1e7e5 Mon Sep 17 00:00:00 2001
>> From: Jim Gill 
>> Date: Thu, 16 Jun 2016 14:10:43 -0700
>> Subject: [PATCH] VMW_PVSCSI: Change to update maintainer details (name,
>> email)
>>
>> Signed-off-by: Jim Gill 
>> ---
>>  drivers/scsi/vmw_pvscsi.c | 2 +-
>>  drivers/scsi/vmw_pvscsi.h | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/scsi/vmw_pvscsi.c b/drivers/scsi/vmw_pvscsi.c
>> index 6164634..4a0d3cd 100644
>> --- a/drivers/scsi/vmw_pvscsi.c
>> +++ b/drivers/scsi/vmw_pvscsi.c
>> @@ -17,7 +17,7 @@
>>   * along with this program; if not, write to the Free Software
>>   * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301
>> USA.
>>   *
>> - * Maintained by: Arvind Kumar 
>> + * Maintained by: Jim Gill 
>
> Shouldn't you update MAINTAINERs too? And isn't having this
> information in these files redundant?
>
> Thanks,
>
> --
> Julian Calaby
>
> Email: julian.cal...@gmail.com
> Profile: 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__www.google.com_profiles_julian.calaby_&d=CwIBaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=bUMaNc7nC9xbXtaMJrOvIIPNpPH0chY2kdRsskQn6GY&m=JibRIMgSBQLffjc5PofQnpJ0k_gawmvafvIKpsEaMzk&s=CeVpY1FwvY9Qlb0VDeQUXeeL3kuVkcIKTZIC7exS6SM&e=



-- 
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] VMW_PVSCSI: Change to update maintainer details (name, email)

2016-06-16 Thread Julian Calaby
Hi Joe,

On Fri, Jun 17, 2016 at 12:33 PM, Joe Perches  wrote:
> On Fri, 2016-06-17 at 12:18 +1000, Julian Calaby wrote:
>> ./scripts/get_maintainers.pl -f drivers/scsi/vmw_pvscsi.c
>
> just fyi:  the script name is not plural

Thanks, must have typo'd it between running it and typing it into the email.

> $ ./scripts/get_maintainer.pl  -f drivers/scsi/vmw_pvscsi.c
> Arvind Kumar  (maintainer:VMware PVSCSI driver)
> VMware PV-Drivers  (maintainer:VMware PVSCSI driver)
> "James E.J. Bottomley"  (maintainer:SCSI SUBSYSTEM)
> "Martin K. Petersen"  (maintainer:SCSI SUBSYSTEM)
> linux-scsi@vger.kernel.org (open list:VMware PVSCSI driver)
> linux-ker...@vger.kernel.org (open list)
>
> and another fyi:
>
> get_maintainer.pl also has a rarely used "--file-emails" option to
> scan for what appears to be email addresses in specific files.
>
> $ ./scripts/get_maintainer.pl  -f --file-emails drivers/scsi/vmw_pvscsi.c
> Arvind Kumar  (maintainer:VMware PVSCSI driver,in 
> file)
> VMware PV-Drivers  (maintainer:VMware PVSCSI driver)
> "James E.J. Bottomley"  (maintainer:SCSI SUBSYSTEM)
> "Martin K. Petersen"  (maintainer:SCSI SUBSYSTEM)
> linux-scsi@vger.kernel.org (open list:VMware PVSCSI driver)
> linux-ker...@vger.kernel.org (open list)
>
> note the "in file" after Arvind's name

Didn't know this, however my point stands: the maintainer line in the
file is redundant if we find maintainers through MAINTAINERS or the
get_maintainer.pl script.

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] VMW_PVSCSI: Change to update maintainer details (name, email)

2016-06-16 Thread Julian Calaby
Hi Joe,

On Fri, Jun 17, 2016 at 1:04 PM, Joe Perches  wrote:
> On Fri, 2016-06-17 at 12:44 +1000, Julian Calaby wrote:
>> Hi Joe,
>
> rehi Julian.

(I always put a salutation on my emails and always finish them with
"Thanks," =) )

>> On Fri, Jun 17, 2016 at 12:33 PM, Joe Perches  wrote:
> []
>> > get_maintainer.pl also has a rarely used "--file-emails" option to
>> > scan for what appears to be email addresses in specific files.
>> >
>> > $ ./scripts/get_maintainer.pl  -f --file-emails drivers/scsi/vmw_pvscsi.c
>> > Arvind Kumar  (maintainer:VMware PVSCSI driver,in 
>> > file)
>> > VMware PV-Drivers  (maintainer:VMware PVSCSI driver)
>> > "James E.J. Bottomley"  (maintainer:SCSI 
>> > SUBSYSTEM)
>> > "Martin K. Petersen"  (maintainer:SCSI 
>> > SUBSYSTEM)
>> > linux-scsi@vger.kernel.org (open list:VMware PVSCSI driver)
>> > linux-ker...@vger.kernel.org (open list)
>> >
>> > note the "in file" after Arvind's name
>> Didn't know this, however my point stands: the maintainer line in the
>> file is redundant if we find maintainers through MAINTAINERS or the
>> get_maintainer.pl script.
>
> Yes, I'm not suggesting anything else.  Jim's name
> should appear in the MAINTAINERS file somewhere.
>
> The question to me is whether or not Jim Gill is
> taking over the maintainership of the entire
> VMware PVSCSI driver or just a few files of it.

As I see it, he's taking over maintainership of all of it: it's only
files are drivers/scsi/vmw_pvscsi.[ch] AFAIK.

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: QLogicPTI Hangs on SPARC64

2016-08-08 Thread Julian Calaby
Hi Alex,

On Mon, Aug 8, 2016 at 2:23 PM, Alex McWhirter  wrote:
> This is a bug i've been playing with for a while now, but i think i've
> narrowed it down about as far as i can without additional help. I
> belive we are hitting this bit of code, as i have seen the message
> before in previous panics.

You might get more help from the linux-scsi list (CC'd)

>
> toss_command:
> printk(KERN_EMERG "qlogicpti%d: request queue overflow\n",
>qpti->qpti_id);
>
> /* Unfortunately, unless you use the new EH code, which
>  * we don't, the midlayer will ignore the return value,
>  * which is insane.  We pick up the pieces like this.
>  */
> Cmnd->result = DID_BUS_BUSY;
> done(Cmnd);
> return 1;
>
> Correct me if i'm wrong, but i don't how we're pickuping up any peices
> here. Something went wrong and SCSI requests built up to an
> unmanageable point and we just say the bus is busy? Granted i'm not
> really sure how you would pick up any peices in that case unless you
> set the bus busy before the queue were to overflow and just try to wait
> it out.
>
> Take a look at the iostat information below. iostat was configured to
> refresh every second, the bottom was cut off during the panic.
>
> iostat log > http://pastebin.com/ea96AucT
>
> From this you can see that sdc was the first drive to stop responding.
> it's r/s and w/s drop to zero but the util% stays at 100. Shortly
> after, the request queue overflows and sets the whole bus to busy which
> can be seen in the last portion of the log (which didn't finish as the
> system panic'd). All of the disks on that bus have subsequently
> followed suite with sdc because the bus is essnstially screeching to a
> halt.
>
> Below you will find the kernel panic.
>
> kernel panic > http://pastebin.com/n9agfz1z
>
> Again, correct me if i'm wrong, but it would seem that any pointers
> pointing towards the request queue are now invalid as the queue has
> overflown.
>
> Below is just some conjecture on my part.
>
> Should the correct behaviour here not be to fail the disk that is
> holding up the rest of the bus? From what i see, it is quite likely sdc
> is bad so i will be replacing it, however having the whole system panic
> because of a bad disk seems counter intuitive. I realise this is quite
> an old driver, and may have been written before we had ways of dealing
> with these types of issues. Or perhaps even, it's a a hardware
> limitation that prevents up from pinpointing what is acutally no longer
> responding on the bus?
> --
> To unsubscribe from this list: send the line "unsubscribe sparclinux" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
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: Obtain file's inode in ufshcd driver

2018-06-18 Thread Julian Calaby
Hi Roman,

On Fri, Jun 1, 2018 at 4:25 AM Roman Storozhenko
 wrote:
>
> Hello everybody.
>
> I am modifying ufshcd driver:
> https://elixir.bootlin.com/linux/v3.8/source/drivers/scsi/ufs/ufshcd.c
>
> I do a read of a file on ext4 filesystem and want to obtain the file's
> inode. But it seems that there are no structures contatin any
> references to inode at this level of abstraction.

I'm no expert, I'm just answering so you actually have an answer, not
because I have any good knowledge in this area.

This is by design - by the time the data gets to the hardware the
associated file is both irrelevant and unobtainable: the read or write
might be metadata or housekeeping, the filesystem might be
log-structured so it's reading or writing bits of 5 different files,
the upper layer might be a swap partition or something more exotic, it
might be part of a disk check, there might be software RAID between
the filesystem and hardware, etc.

Why do you need this data and what are you trying to accomplish here?

> I looked into scsi_cmd structure and all the structures it references
> to. Also I used ftrace to find on which level of abstraction inode
> disappear but without any luck. Could you please say to me where I
> could find the lowest level where inode is still present and the best
> way how to pass it throughout linux io stack to the ufshcd driver?
> Frankly speaking I need not struct inode as a whole but just its
> number, so using reserved or unused fields of existing structures is
> appropriate.

The inode number is part of the filesystem so it wouldn't appear below
that layer.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/