Re: [PATCH 38/42] isdn: replace ->proc_fops with ->proc_show

[Paul Bolle]

Hi Christoph,

(I don't think the patches of this series ever hit the ISDN related addresses
still found in MAINTAINERS. And now I might be a bit late.) 

Christoph Hellwig schreef op wo 16-05-2018 om 11:43 [+0200]:
> diff --git a/drivers/isdn/gigaset/capi.c b/drivers/isdn/gigaset/capi.c
> index ccec7778cad2..dac5cd35e901 100644
> --- a/drivers/isdn/gigaset/capi.c
> +++ b/drivers/isdn/gigaset/capi.c
> @@ -2437,19 +2437,6 @@ static int gigaset_proc_show(struct seq_file *m, void 
> *v)
>   return 0;
>  }
>  
> -static int gigaset_proc_open(struct inode *inode, struct file *file)
> -{
> - return single_open(file, gigaset_proc_show, PDE_DATA(inode));
> -}
> -
> -static const struct file_operations gigaset_proc_fops = {
> - .owner  = THIS_MODULE,
> - .open   = gigaset_proc_open,
> - .read   = seq_read,
> - .llseek = seq_lseek,
> - .release= single_release,
> -};
> -
>  /**
>   * gigaset_isdn_regdev() - register device to LL
>   * @cs:  device descriptor structure.
> @@ -2478,8 +2465,7 @@ int gigaset_isdn_regdev(struct cardstate *cs, const 
> char *isdnid)
>   iif->ctr.register_appl = gigaset_register_appl;
>   iif->ctr.release_appl  = gigaset_release_appl;
>   iif->ctr.send_message  = gigaset_send_message;
> - iif->ctr.procinfo  = gigaset_procinfo;

Is this intentional? You didn't touch the procinfo method in the other ISDN
drivers, as far as I can see.

(If it was intentional, gigaset_procinfo() can of course be removed.)

> - iif->ctr.proc_fops = _proc_fops;
> + iif->ctr.proc_show = gigaset_proc_show,
>   INIT_LIST_HEAD(>appls);
>   skb_queue_head_init(>sendqueue);
>   atomic_set(>sendqlen, 0);

Thanks,


Paul Bolle

Re: [PATCH v2 4/4] scsi: ufs: probe and init of variant driver from the platform device

[Paul Bolle]

On Wed, 2015-06-03 at 12:37 +0300, Yaniv Gardi wrote:
 --- a/drivers/scsi/ufs/ufs-qcom.c
 +++ b/drivers/scsi/ufs/ufs-qcom.c

  EXPORT_SYMBOL(ufs_hba_qcom_vops);

Nothing uses this export. It's still a (static) symbol that is not
included in any header. I think this export serves no purpose. Am I
missing something subtle here?

 +/**
 + * ufs_qcom_probe - probe routine of the driver
 + * @pdev: pointer to Platform device handle
 + *
 + * Always return 0
 + */
 +static int ufs_qcom_probe(struct platform_device *pdev)
 +{
 + dev_set_drvdata(pdev-dev, (void *)ufs_hba_qcom_vops);

(Cast to void * should not be needed.)

 + return 0;
 +}
 +
 +/**
 + * ufs_qcom_remove - set driver_data of the device to NULL
 + * @pdev: pointer to platform device handle
 + *
 + * Always return 0
 + */
 +static int ufs_qcom_remove(struct platform_device *pdev)
 +{
 + dev_set_drvdata(pdev-dev, NULL);
 + return 0;
 +}
 +
 +static const struct of_device_id ufs_qcom_of_match[] = {
 + { .compatible = qcom,ufs_variant},
 + {},
 +};
 +
 +static struct platform_driver ufs_qcom_pltform = {
 + .probe  = ufs_qcom_probe,
 + .remove = ufs_qcom_remove,
 + .driver = {
 + .name   = ufs_qcom,
 + .owner  = THIS_MODULE,
 + .of_match_table = of_match_ptr(ufs_qcom_of_match),
 + },
 +};
 +module_platform_driver(ufs_qcom_pltform);

 --- a/drivers/scsi/ufs/ufshcd-pltfrm.c
 +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c

 + struct device_node *node = pdev-dev.of_node;
 + struct device_node *ufs_variant_node;
 + struct platform_device *ufs_variant_pdev;
 
 - hba-vops = get_variant_ops(pdev-dev);
 + err = of_platform_populate(node, NULL, NULL, pdev-dev);
 + if (err)
 + dev_err(pdev-dev,
 + %s: of_platform_populate() failed\n, __func__);
 +
 + ufs_variant_node = of_get_next_available_child(node, NULL);
 +
 + if (!ufs_variant_node) {
 + dev_dbg(pdev-dev, failed to find ufs_variant_node child\n);
 + } else {
 + ufs_variant_pdev = of_find_device_by_node(ufs_variant_node);
 +
 + if (ufs_variant_pdev)
 + hba-vops = (struct ufs_hba_variant_ops *)
 + dev_get_drvdata(ufs_variant_pdev-dev);

(Another cast that I think is not needed.)

 + }

If I scanned this correctly, the dev_set_drvdata() and dev_get_drvdata()
pair adds an actual user of ufs_hba_qcom_vops. So that ends the obvious
issue I think the code currently has. And I gladly defer to the scsi
people to determine whether that is done the right way.

Thanks,


Paul Bolle

--
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 4/4] scsi: ufs: probe and init of variant driver from the platform device

[Paul Bolle]

On Thu, 2015-06-04 at 16:07 +0200, Paul Bolle wrote:
 On Wed, 2015-06-03 at 12:37 +0300, Yaniv Gardi wrote:
  +static int ufs_qcom_probe(struct platform_device *pdev)
  +{
  +   dev_set_drvdata(pdev-dev, (void *)ufs_hba_qcom_vops);
 
 (Cast to void * should not be needed.)

Only if ufs_hba_qcom_vops wasn't a _const_ struct ufs_hba_variant_ops,
that is. Never mind.


Paul Bolle

--
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 v1 3/3] scsi: ufs-qcom: update configuration option of SCSI_UFS_QCOM component

[Paul Bolle]

On Wed, 2015-05-20 at 10:22 +0200, Paul Bolle wrote:
 By the way, as far as I can see, this (new) module can only be loaded
 manually (or via scripts). Is that what people want?

This comment wasn't well thought through. So I hand another look at the
code of usf-qcom.

I noticed that the single thing ufs-qcom exports is struct
ufs_hba_qcom_vops. But that's unused in next-20150520:
$ git grep -nw ufs_hba_qcom_vops
drivers/scsi/ufs/ufs-qcom.c:999: * struct ufs_hba_qcom_vops - UFS QCOM 
specific variant operations
drivers/scsi/ufs/ufs-qcom.c:1004:static const struct ufs_hba_variant_ops 
ufs_hba_qcom_vops = {
drivers/scsi/ufs/ufs-qcom.c:1016:EXPORT_SYMBOL(ufs_hba_qcom_vops);

So it's not used by code outside of ufs-qcom.c. Probably because it
can't actually be used by outside code. It's not mentioned in any public
header and it's even static!

Am I missing something obvious here? Because ufs-qcom currently looks
pointless to me, and I actually see little reason to even have it in the
mainline tree. 


Paul Bolle

--
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 v1 3/3] scsi: ufs-qcom: update configuration option of SCSI_UFS_QCOM component

[Paul Bolle]

On Thu, 2015-05-21 at 10:09 +, yga...@codeaurora.org wrote:
  On Wed, 2015-05-20 at 10:22 +0200, Paul Bolle wrote:
  Am I missing something obvious here? Because ufs-qcom currently looks
  pointless to me, and I actually see little reason to even have it in the
  mainline tree.
 
 
 we haven't uploaded yet the patch that binds qcom vops to the driver, but
 we will soon.

Perhaps you could make that patch part of v2 of this series. I see
little point in this series without that patch. Perhaps someone else
still cares about it, but I'm not looking at it anymore.

Thanks,


Paul Bolle

--
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 v1 3/3] scsi: ufs-qcom: update configuration option of SCSI_UFS_QCOM component

[Paul Bolle]

On Tue, 2015-05-19 at 16:47 +0300, Yaniv Gardi wrote:
 This change is required in order to be able to build the component
 as a module.
 
 Signed-off-by: Yaniv Gardi yga...@codeaurora.org

 --- a/drivers/scsi/ufs/Kconfig
 +++ b/drivers/scsi/ufs/Kconfig
 
  config SCSI_UFS_QCOM
 - bool QCOM specific hooks to UFS controller platform driver
 + tristate QCOM specific hooks to UFS controller platform driver
   depends on SCSI_UFSHCD_PLATFORM  ARCH_QCOM
   select PHY_QCOM_UFS
   help

As far as I can see, in next-20150519, drivers/scsi/ufs/ufs-qcom.c lacks
the required module specific boilerplate for this to be useful. Is that
boilerplate added in another series?

Thanks,


Paul Bolle

--
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 v1 1/3] phy: qcom-ufs: fix build error when the driver is built as a module

[Paul Bolle]

On Tue, 2015-05-19 at 16:47 +0300, Yaniv Gardi wrote:
 Export the following functions in order to avoid build errors
 when the driver is compiled as a module:

Where the driver actually means something like ufs-qcom.c, or
SCSI_UFS_QCOM, or QCOM specific hooks to UFS controller platform
driver, right?

 ERROR: ufs_qcom_phy_disable_ref_clk [drivers/scsi/ufs/ufs-qcom.ko] 
 undefined!
 ERROR: ufs_qcom_phy_enable_ref_clk [drivers/scsi/ufs/ufs-qcom.ko] undefined!
 ERROR: ufs_qcom_phy_is_pcs_ready [drivers/scsi/ufs/ufs-qcom.ko] undefined!
 ERROR: ufs_qcom_phy_disable_iface_clk [drivers/scsi/ufs/ufs-qcom.ko] 
 undefined!
 ERROR: ufs_qcom_phy_start_serdes [drivers/scsi/ufs/ufs-qcom.ko] undefined!
 ERROR: ufs_qcom_phy_calibrate_phy [drivers/scsi/ufs/ufs-qcom.ko] undefined!
 ERROR: ufs_qcom_phy_enable_dev_ref_clk [drivers/scsi/ufs/ufs-qcom.ko] 
 undefined!
 ERROR: ufs_qcom_phy_set_tx_lane_enable [drivers/scsi/ufs/ufs-qcom.ko] 
 undefined!
 ERROR: ufs_qcom_phy_disable_dev_ref_clk [drivers/scsi/ufs/ufs-qcom.ko] 
 undefined!
 ERROR: ufs_qcom_phy_save_controller_version [drivers/scsi/ufs/ufs-qcom.ko] 
 undefined!
 ERROR: ufs_qcom_phy_enable_iface_clk [drivers/scsi/ufs/ufs-qcom.ko] 
 undefined!
 make[1]: *** [__modpost] Error 1

Thanks,


Paul Bolle

--
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] drivers/scsi/arm/acornscsi.c: rename CONFIG_ACORNSCSI_CONSTANTS

[Paul Bolle]

On Tue, 2015-04-28 at 16:15 +0200, Valentin Rothberg wrote:
 CONFIG_ACORNSCSI_CONSTANTS is a file local CPP identifier and thereby
 violates the naming convention of Kconfig options in Make and CPP
 syntax; only Kconfig options should carry the 'CONFIG_' prefix.
 
 This patch removes the 'CONFIG_' prefix to apply to this convention and
 to make static analysis tools happy.

Will the Erlangen bot still spot ACORNSCSI_CONSTANTS as a potential
issue?


Paul Bolle

--
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] drivers/scsi/arm/acornscsi.c: rename CONFIG_ACORNSCSI_CONSTANTS

[Paul Bolle]

Hi Valentin,

On Tue, 2015-04-28 at 21:26 +0200, Valentin Rothberg wrote:
 On Tue, Apr 28, 2015 at 9:10 PM, Paul Bolle pebo...@tiscali.nl wrote:
  Will the Erlangen bot still spot ACORNSCSI_CONSTANTS as a potential
  issue?
 
 No, undertaker-checkpatch won't complain about this.  There are
 thousands of such cases (i.e., without CONFIG_ prefix) around in the
 code (mostly #ifdef DEBUG).  But most of them are intentionally dead
 or related to debugging, so they are ignored to avoid having false
 positives.

Well, in a few years time, once undertaker-checkpatch has stomped out
most of the faux Kconfig preprocessor checks, that might be an area to
cover too. Or is that issue, ie pointless preprocessor checks, harder
than one might naively think?

Thanks,


Paul Bolle

--
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 v7 5/5] scsi: ufs-qcom-ice: add Inline Crypto Engine (ICE) support for UFS

[Paul Bolle]

Yaniv,

On Thu, 2015-01-15 at 16:32 +0200, Yaniv Gardi wrote:
 From: Yaniv Gardi yga...@qti.qualcomm.com
 
 In-order to enhance storage encryption performance,
 an Inline Cryptographic Engine is introduced to UFS.
 This patch adds in-line encryption capabilities to the UFS
 driver.
 
 Signed-off-by: Yaniv Gardi yga...@codeaurora.org

This patch became commit 8805ccd069b7 (ufs-qcom-ice: add Inline Crypto
Engine (ICE) support for UFS) in today's linux-next (ie,
next-20150123). I noticed because a script I use to check linux-next
spotted a problem with it.

 ---
  drivers/scsi/ufs/Kconfig|  12 +
  drivers/scsi/ufs/Makefile   |   1 +
  drivers/scsi/ufs/ufs-qcom-ice.c | 520 
 
  drivers/scsi/ufs/ufs-qcom-ice.h | 113 +
  drivers/scsi/ufs/ufs-qcom.c |  56 -
  drivers/scsi/ufs/ufs-qcom.h |  25 ++
  6 files changed, 726 insertions(+), 1 deletion(-)
  create mode 100644 drivers/scsi/ufs/ufs-qcom-ice.c
  create mode 100644 drivers/scsi/ufs/ufs-qcom-ice.h
 
 diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
 index 8a1f4b3..ecf34ed 100644
 --- a/drivers/scsi/ufs/Kconfig
 +++ b/drivers/scsi/ufs/Kconfig
 @@ -83,3 +83,15 @@ config SCSI_UFS_QCOM
  
 Select this if you have UFS controller on QCOM chipset.
 If unsure, say N.
 +
 +config SCSI_UFS_QCOM_ICE
 + bool QCOM specific hooks to Inline Crypto Engine for UFS driver
 + depends on SCSI_UFS_QCOM  CRYPTO_DEV_QCOM_ICE

There's currently no Kconfig symbol CRYPTO_DEV_QCOM_ICE in linux-next.
So SCSI_UFS_QCOM_ICE can not be set and these in-line encryption
capabilities can not be enabled.

 + help
 +   This selects the QCOM specific additions to support Inline Crypto
 +   Engine (ICE).
 +   ICE accelerates the crypto operations and maintains the high UFS
 +   performance.
 +
 +   Select this if you have ICE supported for UFS on QCOM chipset.
 +   If unsure, say N.
 diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
 index 8303bcc..31adca5 100644
 --- a/drivers/scsi/ufs/Makefile
 +++ b/drivers/scsi/ufs/Makefile
 @@ -1,5 +1,6 @@
  # UFSHCD makefile
  obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
 +obj-$(CONFIG_SCSI_UFS_QCOM_ICE) += ufs-qcom-ice.o
  obj-$(CONFIG_SCSI_UFSHCD) += ufshcd.o
  obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
  obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o
 diff --git a/drivers/scsi/ufs/ufs-qcom-ice.c b/drivers/scsi/ufs/ufs-qcom-ice.c
 new file mode 100644
 index 000..9202b73
 --- /dev/null
 +++ b/drivers/scsi/ufs/ufs-qcom-ice.c
 @@ -0,0 +1,520 @@
 +/* Copyright (c) 2014-2015, The Linux Foundation. All rights reserved.
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 and
 + * only version 2 as published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + */
 +
 +#include linux/of.h
 +#include linux/async.h
 +#include linux/blkdev.h
 +#include crypto/ice.h

This header is not included in linux-next so manually building
ufs-qcom-ice.o isn't possible either.

I assume a series to add CRYPTO_DEV_QCOM_ICE and crypto/ice.h (and
whatever else is needed to build this) is queued somewhere. Is that
correct?


Paul Bolle

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

[PATCH] tgt: remove SCSI_TGT and SCSI_FC_TGT_ATTRS

[Paul Bolle]

The Kconfig symbols SCSI_TGT and SCSI_FC_TGT_ATTRS are unused since
commit 73685a458f2e (tgt: removal). Setting them has no effect. Remove
these symbols.

Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
Tested by running make oldconfig on a Fedora 20 based .config.
That .config has CONFIG_SCSI_TGT set to 'm' and CONFIG_SCSI_FC_TGT_ATTRS
set to 'y'. This patch only makes both symbols disappear from
the .config, which is what we want. Please shout if that's not enough
testing in your opinion.

Note that the commit explanation references a linux-next commit ID. If
that patch gets rebased before entering mainline that won't be a ID
found in the mainline repository anymore!

 drivers/scsi/Kconfig | 14 --
 1 file changed, 14 deletions(-)

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 1fa07e4221f7..18a3358eb1d4 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -40,13 +40,6 @@ config SCSI_DMA
bool
default n
 
-config SCSI_TGT
-   tristate SCSI target support
-   depends on SCSI
-   ---help---
- If you want to use SCSI target mode drivers enable this option.
- If you choose M, the module will be called scsi_tgt.
-
 config SCSI_NETLINK
bool
default n
@@ -271,13 +264,6 @@ config SCSI_FC_ATTRS
  each attached FiberChannel device to sysfs, say Y.
  Otherwise, say N.
 
-config SCSI_FC_TGT_ATTRS
-   bool SCSI target support for FiberChannel Transport Attributes
-   depends on SCSI_FC_ATTRS
-   depends on SCSI_TGT = y || SCSI_TGT = SCSI_FC_ATTRS
-   help
-   If you want to use SCSI target mode drivers enable this option.
-
 config SCSI_ISCSI_ATTRS
tristate iSCSI Transport Attributes
depends on SCSI  NET
-- 
1.9.3

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

scsi: SCSI_FC_TGT_ATTRS

[Paul Bolle]

0) Commit 73685a458f2e (tgt: removal) landed in next-20140714. It
states that the CONFIG_SCSI_TGT, CONFIG_SCSI_SRP_TGT_ATTRS and
CONFIG_SCSI_FC_TGT_ATTRS kbuild variable[s] [...] are no longer needed.
It removed the Kconfig entry for SCSI_SRP_TGT_ATTRS, but it left the
entries for SCSI_TGT and SCSI_FC_TGT_ATTRS untouched.

1) Both SCSI_TGT and SCSI_FC_TGT_ATTRS appear to be unused after that
commit. Is a patch to remove their Kconfig entries queued somewhere?


Paul Bolle

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

[PATCH] ncr53c8xx: remove remnants of immediate arbitration

[Paul Bolle]

The code for immediate arbitration was removed from (what looks like a
predecessor of) the ncr53c8xx code in v2.6.0. Remove its remnants now.

Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
Untested.

 Documentation/scsi/ncr53c8xx.txt | 52 
 drivers/scsi/ncr53c8xx.c | 12 --
 drivers/scsi/ncr53c8xx.h |  8 ---
 3 files changed, 72 deletions(-)

diff --git a/Documentation/scsi/ncr53c8xx.txt b/Documentation/scsi/ncr53c8xx.txt
index 1d508dcbf859..49c7b723225b 100644
--- a/Documentation/scsi/ncr53c8xx.txt
+++ b/Documentation/scsi/ncr53c8xx.txt
@@ -52,12 +52,10 @@ Written by Gerard Roudier groud...@free.fr
  10.2.19 Check SCSI BUS 
  10.2.20 Exclude a host from being attached
  10.2.21 Suggest a default SCSI id for hosts
- 10.2.22 Enable use of IMMEDIATE ARBITRATION
   10.3 Advised boot setup commands
   10.4 PCI configuration fix-up boot option
   10.5 Serial NVRAM support boot option
   10.6 SCSI BUS checking boot option
-  10.7 IMMEDIATE ARBITRATION boot option
 11. Some constants and flags of the ncr53c8xx.h header file
 12. Installation
 13. Architecture dependent features
@@ -843,16 +841,6 @@ port address 0x1400.
 try to deduce the value previously set in the hardware and use value 
 7 if the hardware value is zero.
 
-10.2.22 Enable use of IMMEDIATE ARBITRATION
-(only supported by the sym53c8xx driver. See 10.7 for more details)
-iarb:0do not use this feature.
-iarb:#x   use this feature according to bit fields as follow:
-
-bit 0 (1) : enable IARB each time the initiator has been reselected 
-when it arbitrated for the SCSI BUS.
-(#x  4) : maximum number of successive settings of IARB if the initiator 
-win arbitration and it has other commands to send to a device.
-
 Boot fail safe
 safe:y load the following assumed fail safe initial setup
 
@@ -876,7 +864,6 @@ Boot fail safe
   differential support from BIOS settings  diff:1
   irq mode from BIOS settings  irqm:1
   SCSI BUS check   do not attach on error  buschk:1
-  immediate arbitrationdisablediarb:0
 
 10.3 Advised boot setup commands
 
@@ -1005,45 +992,6 @@ Unfortunately, the following common SCSI BUS problems are 
not detected:
 On the other hand, either bad cabling, broken devices, not conformant 
 devices, ... may cause a SCSI signal to be wrong when te driver reads it.
 
-10.7 IMMEDIATE ARBITRATION boot option
-
-This option is only supported by the SYM53C8XX driver (not by the NCR53C8XX).
-
-SYMBIOS 53C8XX chips are able to arbitrate for the SCSI BUS as soon as they 
-have detected an expected disconnection (BUS FREE PHASE). For this process 
-to be started, bit 1 of SCNTL1 IO register must be set when the chip is 
-connected to the SCSI BUS.
-
-When this feature has been enabled for the current connection, the chip has 
-every chance to win arbitration if only devices with lower priority are 
-competing for the SCSI BUS. By the way, when the chip is using SCSI id 7, 
-then it will for sure win the next SCSI BUS arbitration.
-
-Since, there is no way to know what devices are trying to arbitrate for the 
-BUS, using this feature can be extremely unfair. So, you are not advised
-to enable it, or at most enable this feature for the case the chip lost 
-the previous arbitration (boot option 'iarb:1').
-
-This feature has the following advantages:
-
-a) Allow the initiator with ID 7 to win arbitration when it wants so.
-b) Overlap at least 4 micro-seconds of arbitration time with the execution 
-   of SCRIPTS that deal with the end of the current connection and that 
-   starts the next job.
-
-Hmmm... But (a) may just prevent other devices from reselecting the initiator, 
-and delay data transfers or status/completions, and (b) may just waste 
-SCSI BUS bandwidth if the SCRIPTS execution lasts more than 4 micro-seconds.
-
-The use of IARB needs the SCSI_NCR_IARB_SUPPORT option to have been defined 
-at compile time and the 'iarb' boot option to have been set to a non zero 
-value at boot time. It is not that useful for real work, but can be used 
-to stress SCSI devices or for some applications that can gain advantage of 
-it. By the way, if you experience badnesses like 'unexpected disconnections', 
-'bad reselections', etc... when using IARB on heavy IO load, you should not 
-be surprised, because force-feeding anything and blocking its arse at the 
-same time cannot work for a long time. :-))
-
 
 11. Some constants and flags of the ncr53c8xx.h header file
 
diff --git a/drivers/scsi/ncr53c8xx.c b/drivers/scsi/ncr53c8xx.c
index 7d014b11df62..9661a2c9e13c 100644
--- a/drivers/scsi/ncr53c8xx.c
+++ b/drivers/scsi/ncr53c8xx.c
@@ -615,10 +615,6 @@ static struct ncr_driver_setup
 #define OPT_EXCLUDE24
 #define OPT_HOST_ID25

[PATCH] ncr53c8xx: remove ancient configuration macros

[Paul Bolle]

It has not been possible to set CONFIG_SCSI_NCR53C8XX_FORCE_SYNC_NEGO,
CONFIG_SCSI_NCR53C8XX_DISABLE_MPARITY_CHECK, and
CONFIG_SCSI_NCR53C8XX_DISABLE_PARITY_CHECK through the configuration
system since v2.1.20. Remove these ancient macros.

To enable (or disable) the functionality they covered one should edit a
header file and set related preprocessor macros. Since this is actually
a documented feature those related macros remain available.

Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
Untested.

Note that the documentation still contains
insmod ncr53c8xx

Besides insmod being outdated, the modules that use ncr53c8xx's code
appear to be NCR_Q720_mod.ko and zalon7xx.ko. So the documentation needs
probably to be updated to reflect this.

 Documentation/scsi/ncr53c8xx.txt |  5 -
 drivers/scsi/ncr53c8xx.h | 12 
 2 files changed, 17 deletions(-)

diff --git a/Documentation/scsi/ncr53c8xx.txt b/Documentation/scsi/ncr53c8xx.txt
index 49c7b723225b..df4a1b97e659 100644
--- a/Documentation/scsi/ncr53c8xx.txt
+++ b/Documentation/scsi/ncr53c8xx.txt
@@ -580,11 +580,6 @@ CONFIG_SCSI_NCR53C8XX_SYNC(default answer: 5)
 This frequency can be changed later with the setsync control command.
 0 means asynchronous data transfers.
 
-CONFIG_SCSI_NCR53C8XX_FORCE_SYNC_NEGO (default answer: n)
-Force synchronous negotiation for all SCSI-2 devices.
-Some SCSI-2 devices do not report this feature in byte 7 of inquiry 
-response but do support it properly (TAMARACK scanners for example).
-
 CONFIG_SCSI_NCR53C8XX_NO_DISCONNECT   (default and only reasonable answer: n)
 If you suspect a device of yours does not properly support disconnections,
 you can answer y. Then, all SCSI devices will never disconnect the bus 
diff --git a/drivers/scsi/ncr53c8xx.h b/drivers/scsi/ncr53c8xx.h
index e34ec5a2ea5e..8043635982bc 100644
--- a/drivers/scsi/ncr53c8xx.h
+++ b/drivers/scsi/ncr53c8xx.h
@@ -148,29 +148,17 @@
 /*
  * Force synchronous negotiation for all targets
  */
-#ifdef CONFIG_SCSI_NCR53C8XX_FORCE_SYNC_NEGO
-#define SCSI_NCR_SETUP_FORCE_SYNC_NEGO (1)
-#else
 #define SCSI_NCR_SETUP_FORCE_SYNC_NEGO (0)
-#endif
 
 /*
  * Disable master parity checking (flawed hardwares need that)
  */
-#ifdef CONFIG_SCSI_NCR53C8XX_DISABLE_MPARITY_CHECK
-#define SCSI_NCR_SETUP_MASTER_PARITY   (0)
-#else
 #define SCSI_NCR_SETUP_MASTER_PARITY   (1)
-#endif
 
 /*
  * Disable scsi parity checking (flawed devices may need that)
  */
-#ifdef CONFIG_SCSI_NCR53C8XX_DISABLE_PARITY_CHECK
-#define SCSI_NCR_SETUP_SCSI_PARITY (0)
-#else
 #define SCSI_NCR_SETUP_SCSI_PARITY (1)
-#endif
 
 /*
  * Settle time after reset at boot-up
-- 
1.9.3

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

Re: [PATCH] acornscsi: remove linked command support

[Paul Bolle]

On Sun, 2014-05-25 at 11:42 +0400, James Bottomley wrote:
 On Sat, 2014-05-24 at 15:16 +0200, Paul Bolle wrote:
  On Sat, 2014-05-24 at 16:13 +0400, James Bottomley wrote:
   Wait, no, that's not a good idea.  We leave obsolete drivers to bitrot.
   Particularly we try not to touch them unless we have to because there
   might be a few people still using them and the more we tamper, the
   greater the risk that something gets broken.
  
  Which is also a way to notice whether people still use those obsolete
  drivers.
 
 Really, no.  We don't deliberately break old drivers to see if anyone
 notices.  Usually the feedback loop is months to years for the long tail
 to notice and by that time fixing the problem becomes a real pain if you
 allow driver churn.

Of course I wasn't advocating deliberately breaking old drivers. But
it's easy to get annoyed by that short remark. It would have been better
if I hadn't made it.

Especially because I didn't also point out, as Cristoph did, that the
code I want removed doesn't get compiled. So removing it can, by
definition I'd say, not break that old driver.

 We keep old drivers that compile until there's a problem caused usually
 by something like API changes.
 
   On that principle, since
   there's no real reason to remove the code,
  
  (Unless one carries the hope that any check, treewide, for a CONFIG_*
  macro can be linked to a proper Kconfig symbol.)
 
 The check can be fixed.  If you look at what Fengguang Wu does, he has a
 list of expected problems which he diffs.  Don't churn the tree to match
 the checker, make the checker match the tree.

Sure. See my recent patch to scripts/headers_check.pl, which does just
that. But before one special cases a certain hit for a checker one needs
to know that this hit really can't or won't be fixed. And in order to
know that one needs to at least try to fix it first.

   it should stay ... until the
   whole driver bitrots to the extent that we can no-longer compile it.
  
  I've run into this depreciation policy before. With aic7xxx_old (which I
  eventually convinced Fedora to disable, a few relases before it was
  removed from the tree). With aic94xx (which I also convinced Fedora to
  disable). I also tried multiple times to shut up advansys' compile
  time[1]. It seems SCSI might risk not to notice their bitrot, because
  eventually everybody stops compiling these obsolete drivers, leaving
  SCSI without feedback on their state.
 
 Why would we care?  If it compiles that's fine, it's not causing a
 problem and it might just be useful to somebody.

Fair point: having code that no one uses doesn't cost a lot.

 The time obsolete drivers cause problems is tree or subsystem wide API
 changes.  Then we look at the amount of work required and sometimes
 remove them or do hack fixes. 
 
  Anyhow, SCSI seems to be the only subsystem that uses this subcategory
  of not-really-maintained drivers. Other subsystems appear to allow
  anything to be fixed, even trivialities, which are what I tend to fix,
  and only stop allowing fixes if the drivers involved are going to be
  removed anyway. But maybe I just never ran into that category in other
  subsystems.
 
 Try ide ... they have the same policy.

I never really touched IDE. That might explain why I only ran into this
issue with SCSI.

 Try to understand the reason: we have a long tail of people using
 obsolete systems who we try not to break.  Any change to an unmaintained
 driver which can't be tested risks that ... and I'm the one who would
 have to try to sort out the problem when it's noticed, hence the
 caution.  If we allow trivial churn, by the time the breakage is noticed
 (usually months to years later), the driver will have picked up a ton of
 changes and finding the problem one becomes really hard.  So
 unmaintained drivers get a default deep freeze maintenance policy.

Thanks for taking the time to explain this to me. I appreciate that.
This is the first time, I think, that I've seen you explain that policy.
(I might have missed earlier explanations to other people.) Now I might
not entirely agree with you, but it does help to know where you're
coming from.

Thanks,


Paul Bolle

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

[PATCH] acornscsi: remove linked command support

[Paul Bolle]

The acornscsi driver was added in v2.1.88. It has always #undef-ed
CONFIG_SCSI_ACORNSCSI_LINK near the top of acornscsi.c. And, just to be
sure, it has also always triggered a preprocessor error if
CONFIG_SCSI_ACORNSCSI_LINK was still defined. But, as far as I can see,
it has never even been possible to set SCSI_ACORNSCSI_LINK through
kconfig, or its predecessors, in the first place.

Let's remove the code involved.

Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
Untested.

Also interesting: SCSI_ACORNSCSI_TAGGED_QUEUE can be set through
kconfig, but its macro will be #undef-ed at the top of acornscsi.c. I
suppose that #undef could be dropped.

And finally: CONFIG_ACORNSCSI_CONSTANTS has to be set manually. But if
we'd just drop the CONFIG_ prefix acornscsi.c would become a pet peeve
free zone.

 drivers/scsi/arm/acornscsi.c | 53 
 1 file changed, 53 deletions(-)

diff --git a/drivers/scsi/arm/acornscsi.c b/drivers/scsi/arm/acornscsi.c
index 059ff477a398..2e797a367608 100644
--- a/drivers/scsi/arm/acornscsi.c
+++ b/drivers/scsi/arm/acornscsi.c
@@ -62,13 +62,6 @@
  */
 #undef CONFIG_SCSI_ACORNSCSI_TAGGED_QUEUE
 /*
- * SCSI-II Linked command support.
- *
- * The higher level code doesn't support linked commands yet, and so the option
- * is undef'd here.
- */
-#undef CONFIG_SCSI_ACORNSCSI_LINK
-/*
  * SCSI-II Synchronous transfer support.
  *
  * Tried and tested...
@@ -160,10 +153,6 @@
 #error Yippee!  ABORT TAG is now defined!  Remove this error!
 #endif
 
-#ifdef CONFIG_SCSI_ACORNSCSI_LINK
-#error SCSI2 LINKed commands not supported (yet)!
-#endif
-
 #ifdef USE_DMAC
 /*
  * DMAC setup parameters
@@ -1668,42 +1657,6 @@ void acornscsi_message(AS_Host *host)
}
break;
 
-#ifdef CONFIG_SCSI_ACORNSCSI_LINK
-case LINKED_CMD_COMPLETE:
-case LINKED_FLG_CMD_COMPLETE:
-   /*
-* We don't support linked commands yet
-*/
-   if (0) {
-#if (DEBUG  DEBUG_LINK)
-   printk(scsi%d.%c: lun %d tag %d linked command complete\n,
-   host-host-host_no, acornscsi_target(host), 
host-SCpnt-tag);
-#endif
-   /*
-* A linked command should only terminate with one of these messages
-* if there are more linked commands available.
-*/
-   if (!host-SCpnt-next_link) {
-   printk(KERN_WARNING scsi%d.%c: lun %d tag %d linked command 
complete, but no next_link\n,
-   instance-host_no, acornscsi_target(host), 
host-SCpnt-tag);
-   acornscsi_sbic_issuecmd(host, CMND_ASSERTATN);
-   msgqueue_addmsg(host-scsi.msgs, 1, ABORT);
-   } else {
-   struct scsi_cmnd *SCpnt = host-SCpnt;
-
-   acornscsi_dma_cleanup(host);
-
-   host-SCpnt = host-SCpnt-next_link;
-   host-SCpnt-tag = SCpnt-tag;
-   SCpnt-result = DID_OK | host-scsi.SCp.Message  8 | 
host-Scsi.SCp.Status;
-   SCpnt-done(SCpnt);
-
-   /* initialise host-SCpnt-SCp */
-   }
-   break;
-   }
-#endif
-
 default: /* reject message */
printk(KERN_ERR scsi%d.%c: unrecognised message %02X, rejecting\n,
host-host-host_no, acornscsi_target(host),
@@ -2825,9 +2778,6 @@ char *acornscsi_info(struct Scsi_Host *host)
 #ifdef CONFIG_SCSI_ACORNSCSI_TAGGED_QUEUE
  TAG
 #endif
-#ifdef CONFIG_SCSI_ACORNSCSI_LINK
- LINK
-#endif
 #if (DEBUG  DEBUG_NO_WRITE)
  NOWRITE ( __stringify(NO_WRITE) )
 #endif
@@ -2851,9 +2801,6 @@ static int acornscsi_show_info(struct seq_file *m, struct 
Scsi_Host *instance)
 #ifdef CONFIG_SCSI_ACORNSCSI_TAGGED_QUEUE
  TAG
 #endif
-#ifdef CONFIG_SCSI_ACORNSCSI_LINK
- LINK
-#endif
 #if (DEBUG  DEBUG_NO_WRITE)
  NOWRITE ( __stringify(NO_WRITE) )
 #endif
-- 
1.9.0

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

[PATCH] [SCSI] dc395x: remove check for CONFIG_SCSI_DC395x_TRMS1040_TRADMAP

[Paul Bolle]

This driver has always contained a check for
CONFIG_SCSI_DC395x_TRMS1040_TRADMAP. But a Kconfig symbol
SCSI_DC395x_TRMS1040_TRADMAP was never added to the tree. Besides, the
code that this check hides can't compile, since geom is never defined.
Remove it.

Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
Compile tested.

 drivers/scsi/dc395x.c | 22 --
 1 file changed, 22 deletions(-)

diff --git a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c
index 83d9bf6fa6ca..cd0b2bf4369e 100644
--- a/drivers/scsi/dc395x.c
+++ b/drivers/scsi/dc395x.c
@@ -1164,29 +1164,7 @@ static DEF_SCSI_QCMD(dc395x_queue_command)
 static int dc395x_bios_param(struct scsi_device *sdev,
struct block_device *bdev, sector_t capacity, int *info)
 {
-#ifdef CONFIG_SCSI_DC395x_TRMS1040_TRADMAP
-   int heads, sectors, cylinders;
-   struct AdapterCtlBlk *acb;
-   int size = capacity;
-
-   dprintkdbg(DBG_0, dc395x_bios_param..\n);
-   acb = (struct AdapterCtlBlk *)sdev-host-hostdata;
-   heads = 64;
-   sectors = 32;
-   cylinders = size / (heads * sectors);
-
-   if ((acb-gmode2  NAC_GREATER_1G)  (cylinders  1024)) {
-   heads = 255;
-   sectors = 63;
-   cylinders = size / (heads * sectors);
-   }
-   geom[0] = heads;
-   geom[1] = sectors;
-   geom[2] = cylinders;
-   return 0;
-#else
return scsicam_bios_param(bdev, capacity, info);
-#endif
 }
 

-- 
1.9.0

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

Re: [PATCH] [SCSI] dc395x: remove check for CONFIG_SCSI_DC395x_TRMS1040_TRADMAP

[Paul Bolle]

 This is an automatically generated Delivery Status Notification.
 
 Delivery to the following recipients failed.
 
dc3...@twibble.org

Anyone else seeing this too?


Paul Bolle

--
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] acornscsi: remove linked command support

[Paul Bolle]

On Sat, 2014-05-24 at 16:13 +0400, James Bottomley wrote:
 Wait, no, that's not a good idea.  We leave obsolete drivers to bitrot.
 Particularly we try not to touch them unless we have to because there
 might be a few people still using them and the more we tamper, the
 greater the risk that something gets broken.

Which is also a way to notice whether people still use those obsolete
drivers.

 On that principle, since
 there's no real reason to remove the code,

(Unless one carries the hope that any check, treewide, for a CONFIG_*
macro can be linked to a proper Kconfig symbol.)

 it should stay ... until the
 whole driver bitrots to the extent that we can no-longer compile it.

I've run into this depreciation policy before. With aic7xxx_old (which I
eventually convinced Fedora to disable, a few relases before it was
removed from the tree). With aic94xx (which I also convinced Fedora to
disable). I also tried multiple times to shut up advansys' compile
time[1]. It seems SCSI might risk not to notice their bitrot, because
eventually everybody stops compiling these obsolete drivers, leaving
SCSI without feedback on their state.

Anyhow, SCSI seems to be the only subsystem that uses this subcategory
of not-really-maintained drivers. Other subsystems appear to allow
anything to be fixed, even trivialities, which are what I tend to fix,
and only stop allowing fixes if the drivers involved are going to be
removed anyway. But maybe I just never ran into that category in other
subsystems.

 However, I'll do this if the Maintainer (rmk) acks ... because if it
 breaks he gets to fix it.


Paul Bolle

[1] advansys prints a pointless compile time warning, on purpose.
Clearly no one cares about its wide board support, but for some reason
that warning needs to stay in. (Fedora declined to disable that driver.)

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

[PATCH] qla2xxx: remove stub qlt_check_srr_debug()

[Paul Bolle]

qlt_check_srr_debug() was added in v3.5. It is a stub function unless
CONFIG_QLA_TGT_DEBUG_SRR is defined. But CONFIG_QLA_TGT_DEBUG_SRR will
never be defined, because the Kconfig symbol QLA_TGT_DEBUG_SRR was never
added to the tree.

Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
Compile tested.

Or was it intended for this to set it by hand? In that case: please drop
the CONFIG_ prefix, and, perhaps add a comment about its purpose.

 drivers/scsi/qla2xxx/qla_target.c | 90 ---
 1 file changed, 90 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_target.c 
b/drivers/scsi/qla2xxx/qla_target.c
index 0cb73074c199..9729411faf45 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -1747,95 +1747,6 @@ static inline int qlt_need_explicit_conf(struct 
qla_hw_data *ha,
cmd-conf_compl_supported;
 }
 
-#ifdef CONFIG_QLA_TGT_DEBUG_SRR
-/*
- *  Original taken from the XFS code
- */
-static unsigned long qlt_srr_random(void)
-{
-   static int Inited;
-   static unsigned long RandomValue;
-   static DEFINE_SPINLOCK(lock);
-   /* cycles pseudo-randomly through all values between 1 and 2^31 - 2 */
-   register long rv;
-   register long lo;
-   register long hi;
-   unsigned long flags;
-
-   spin_lock_irqsave(lock, flags);
-   if (!Inited) {
-   RandomValue = jiffies;
-   Inited = 1;
-   }
-   rv = RandomValue;
-   hi = rv / 127773;
-   lo = rv % 127773;
-   rv = 16807 * lo - 2836 * hi;
-   if (rv = 0)
-   rv += 2147483647;
-   RandomValue = rv;
-   spin_unlock_irqrestore(lock, flags);
-   return rv;
-}
-
-static void qlt_check_srr_debug(struct qla_tgt_cmd *cmd, int *xmit_type)
-{
-#if 0 /* This is not a real status packets lost, so it won't lead to SRR */
-   if ((*xmit_type  QLA_TGT_XMIT_STATUS)  (qlt_srr_random() % 200)
-   == 50) {
-   *xmit_type = ~QLA_TGT_XMIT_STATUS;
-   ql_dbg(ql_dbg_tgt_mgt, cmd-vha, 0xf015,
-   Dropping cmd %p (tag %d) status, cmd, cmd-tag);
-   }
-#endif
-   /*
-* It's currently not possible to simulate SRRs for FCP_WRITE without
-* a physical link layer failure, so don't even try here..
-*/
-   if (cmd-dma_data_direction != DMA_FROM_DEVICE)
-   return;
-
-   if (qlt_has_data(cmd)  (cmd-sg_cnt  1) 
-   ((qlt_srr_random() % 100) == 20)) {
-   int i, leave = 0;
-   unsigned int tot_len = 0;
-
-   while (leave == 0)
-   leave = qlt_srr_random() % cmd-sg_cnt;
-
-   for (i = 0; i  leave; i++)
-   tot_len += cmd-sg[i].length;
-
-   ql_dbg(ql_dbg_tgt_mgt, cmd-vha, 0xf016,
-   Cutting cmd %p (tag %d) buffer
-tail to len %d, sg_cnt %d (cmd-bufflen %d,
-cmd-sg_cnt %d), cmd, cmd-tag, tot_len, leave,
-   cmd-bufflen, cmd-sg_cnt);
-
-   cmd-bufflen = tot_len;
-   cmd-sg_cnt = leave;
-   }
-
-   if (qlt_has_data(cmd)  ((qlt_srr_random() % 100) == 70)) {
-   unsigned int offset = qlt_srr_random() % cmd-bufflen;
-
-   ql_dbg(ql_dbg_tgt_mgt, cmd-vha, 0xf017,
-   Cutting cmd %p (tag %d) buffer head 
-   to offset %d (cmd-bufflen %d), cmd, cmd-tag, offset,
-   cmd-bufflen);
-   if (offset == 0)
-   *xmit_type = ~QLA_TGT_XMIT_DATA;
-   else if (qlt_set_data_offset(cmd, offset)) {
-   ql_dbg(ql_dbg_tgt_mgt, cmd-vha, 0xf018,
-   qlt_set_data_offset() failed (tag %d), cmd-tag);
-   }
-   }
-}
-#else
-static inline void qlt_check_srr_debug(struct qla_tgt_cmd *cmd, int *xmit_type)
-{}
-#endif
-
 static void qlt_24xx_init_ctio_to_isp(struct ctio7_to_24xx *ctio,
struct qla_tgt_prm *prm)
 {
@@ -1918,7 +1829,6 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int 
xmit_type,
int res;
 
memset(prm, 0, sizeof(prm));
-   qlt_check_srr_debug(cmd, xmit_type);
 
ql_dbg(ql_dbg_tgt, cmd-vha, 0xe018,
is_send_status=%d, cmd-bufflen=%d, cmd-sg_cnt=%d, 
-- 
1.9.0

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

[PATCH] [SCSI] mptfusion: remove check for CONFIG_FUSION_MAX_FC_SGE

[Paul Bolle]

A check for CONFIG_FUSION_MAX_FC_SGE was added in v2.6.31. But the
related Kconfig symbol was never added to the tree. Remove this check,
as it always evaluates to false.

Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
Compile tested only.

 drivers/message/fusion/mptbase.h | 10 --
 1 file changed, 10 deletions(-)

diff --git a/drivers/message/fusion/mptbase.h b/drivers/message/fusion/mptbase.h
index 76c05bc24cb7..43695ec7ac83 100644
--- a/drivers/message/fusion/mptbase.h
+++ b/drivers/message/fusion/mptbase.h
@@ -177,17 +177,7 @@
 #define MPT_SCSI_SG_DEPTH  40
 #endif
 
-#ifdef CONFIG_FUSION_MAX_FC_SGE
-#if CONFIG_FUSION_MAX_FC_SGE   16
-#define MPT_SCSI_FC_SG_DEPTH   16
-#elif CONFIG_FUSION_MAX_FC_SGE   256
-#define MPT_SCSI_FC_SG_DEPTH   256
-#else
-#define MPT_SCSI_FC_SG_DEPTH   CONFIG_FUSION_MAX_FC_SGE
-#endif
-#else
 #define MPT_SCSI_FC_SG_DEPTH   40
-#endif
 
 /* debug print string length used for events and iocstatus */
 # define EVENT_DESCR_STR_SZ 100
-- 
1.9.0

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

Re: [PATCH] [SCSI] mptfusion: remove check for CONFIG_FUSION_MAX_FC_SGE

[Paul Bolle]

On Thu, 2014-05-22 at 19:58 +, Address Change LSI wrote:
 The person you are trying to email is no longer receiving email @lsi.com. 
 Please try sending to them at
 Avago Technologies using firstname.lastn...@avagotech.com.
 
 Please Note: Your original message has been forwarded to this recipient’s new 
 Avago email account.

It seems lsi.com addresses in MAINTAINERS should be be updated.


Paul Bolle

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

[PATCH v2] target_core_alua: silence GCC warning

[Paul Bolle]

Building target_core_alua.o triggers a GCC warning:
drivers/target/target_core_alua.c: In function ‘target_alua_state_check’:
drivers/target/target_core_alua.c:773:18: warning: ‘alua_ascq’ may be used 
uninitialized in this function [-Wmaybe-uninitialized]
   cmd-scsi_ascq = alua_ascq;
  ^

This is a false positive. A little trial and error shows it is
apparently caused by core_alua_state_lba_dependent(). It must be hard
for GCC to track the branches of a switch statement, inside a
list_for_each_entry loop, inside a while loop.

But if we add a small (inline) helper function we can reorganize the
code a bit. That also allows to drop alua_ascq which, obviously, gets
rid of this warning.

Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
v2: Make core_alua_state_nonoptimized() return void, as Geert
suggested. 

Also keep core_alua_state_lba_dependent() inline. Setting that function
noinline was just a leftover from the trial and error fase, and isn't
needed to make the warning go away. Ie, I was sloppy in v1!

Still compile tested only.

 drivers/target/target_core_alua.c | 95 ++-
 1 file changed, 44 insertions(+), 51 deletions(-)

diff --git a/drivers/target/target_core_alua.c 
b/drivers/target/target_core_alua.c
index c3d9df6..fcbe612 100644
--- a/drivers/target/target_core_alua.c
+++ b/drivers/target/target_core_alua.c
@@ -455,11 +455,26 @@ out:
return rc;
 }
 
-static inline int core_alua_state_nonoptimized(
+static inline void set_ascq(struct se_cmd *cmd, u8 alua_ascq)
+{
+   /*
+* Set SCSI additional sense code (ASC) to 'LUN Not Accessible';
+* The ALUA additional sense code qualifier (ASCQ) is determined
+* by the ALUA primary or secondary access state..
+*/
+   pr_debug([%s]: ALUA TG Port not available, 
+   SenseKey: NOT_READY, ASC/ASCQ: 
+   0x04/0x%02x\n,
+   cmd-se_tfo-get_fabric_name(), alua_ascq);
+
+   cmd-scsi_asc = 0x04;
+   cmd-scsi_ascq = alua_ascq;
+}
+
+static inline void core_alua_state_nonoptimized(
struct se_cmd *cmd,
unsigned char *cdb,
-   int nonop_delay_msecs,
-   u8 *alua_ascq)
+   int nonop_delay_msecs)
 {
/*
 * Set SCF_ALUA_NON_OPTIMIZED here, this value will be checked
@@ -468,13 +483,11 @@ static inline int core_alua_state_nonoptimized(
 */
cmd-se_cmd_flags |= SCF_ALUA_NON_OPTIMIZED;
cmd-alua_nonop_delay = nonop_delay_msecs;
-   return 0;
 }
 
 static inline int core_alua_state_lba_dependent(
struct se_cmd *cmd,
-   struct t10_alua_tg_pt_gp *tg_pt_gp,
-   u8 *alua_ascq)
+   struct t10_alua_tg_pt_gp *tg_pt_gp)
 {
struct se_device *dev = cmd-se_dev;
u64 segment_size, segment_mult, sectors, lba;
@@ -520,7 +533,7 @@ static inline int core_alua_state_lba_dependent(
}
if (!cur_map) {
spin_unlock(dev-t10_alua.lba_map_lock);
-   *alua_ascq = ASCQ_04H_ALUA_TG_PT_UNAVAILABLE;
+   set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_UNAVAILABLE);
return 1;
}
list_for_each_entry(map_mem, cur_map-lba_map_mem_list,
@@ -531,11 +544,11 @@ static inline int core_alua_state_lba_dependent(
switch(map_mem-lba_map_mem_alua_state) {
case ALUA_ACCESS_STATE_STANDBY:
spin_unlock(dev-t10_alua.lba_map_lock);
-   *alua_ascq = ASCQ_04H_ALUA_TG_PT_STANDBY;
+   set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_STANDBY);
return 1;
case ALUA_ACCESS_STATE_UNAVAILABLE:
spin_unlock(dev-t10_alua.lba_map_lock);
-   *alua_ascq = ASCQ_04H_ALUA_TG_PT_UNAVAILABLE;
+   set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_UNAVAILABLE);
return 1;
default:
break;
@@ -548,8 +561,7 @@ static inline int core_alua_state_lba_dependent(
 
 static inline int core_alua_state_standby(
struct se_cmd *cmd,
-   unsigned char *cdb,
-   u8 *alua_ascq)
+   unsigned char *cdb)
 {
/*
 * Allowed CDBs for ALUA_ACCESS_STATE_STANDBY as defined by
@@ -570,7 +582,7 @@ static inline int core_alua_state_standby(
case MI_REPORT_TARGET_PGS:
return 0;
default:
-   *alua_ascq = ASCQ_04H_ALUA_TG_PT_STANDBY;
+   set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_STANDBY);
return 1;
}
case MAINTENANCE_OUT:
@@ -578,7 +590,7 @@ static inline int core_alua_state_standby(
case MO_SET_TARGET_PGS:
return 0;
default

[PATCH 2/2] MAINTAINERS: mark Advansys Orphan

[Paul Bolle]

Matthew Wilcox hasn't touched the Advansys driver since v2.6.29. I have
not been able to get Matthew to respond to a trivial patch I'd like to
see go in. Mark this driver Orphan and add this driver to Matthew's
CREDITS entry.

Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
 CREDITS | 2 +-
 MAINTAINERS | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/CREDITS b/CREDITS
index e371c55..4adbf38 100644
--- a/CREDITS
+++ b/CREDITS
@@ -3805,7 +3805,7 @@ N: Matthew Wilcox
 E: matt...@wil.cx
 W: ftp://ftp.uk.linux.org/pub/linux/people/willy/
 D: Linux/PARISC hacker.  Filesystem hacker.  Random other hacking.  Custom
-D: PPC port hacking.
+D: PPC port hacking.  Advansys SCSI driver.
 
 N: G\unter Windau
 E: gun...@mbfys.kun.nl
diff --git a/MAINTAINERS b/MAINTAINERS
index fb08dce..c45d72e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -426,9 +426,8 @@ S:  Supported
 F: drivers/input/misc/adxl34x.c
 
 ADVANSYS SCSI DRIVER
-M: Matthew Wilcox matt...@wil.cx
 L: linux-scsi@vger.kernel.org
-S: Maintained
+S: Orphan
 F: Documentation/scsi/advansys.txt
 F: drivers/scsi/advansys.c
 
-- 
1.8.5.3

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

[PATCH 1/2] advansys: change buildtime warning into runtime error

[Paul Bolle]

Building advansys.o triggers this warning:
drivers/scsi/advansys.c:71:2: warning: #warning this driver is still not 
properly converted to the DMA API [-Wcpp]

This warning can be traced back to a patch called advansys: add warning
and convert #includes which was included in v2.6.10. That patch also
marked the driver as BROKEN.

Commit 4661e3eace2c7b8433476b5bf0ee437ab3c7dfd4 ([SCSI] advansys
driver: limp along on x86) enabled this driver for x86-32. And commit
9d511a4b29de6764931343d03e493f2e04df0271 ([SCSI] advansys: Changes to
work on parisc) enabled this driver for all architectures. But the
commit explanation stated:
I haven't removed the #warning yet because virt_to_bus/bus_to_virt are
only eliminated for narrow boards.  Wide boards need more work.

Six years have passed and, apparently, those wide boards still need
more work. So let's change the buildtime warning into a runtime error,
only printed for those wide boards. Perhaps that might push the people
using those wide boards to convert this driver. And for all others
there's now one less buildtime warning to ignore.

Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
Compile tested only. I don't have any Advansys SCSI boards (neither
narrow nor wide).

 drivers/scsi/advansys.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
index d814588..5999c60 100644
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -68,7 +68,6 @@
  *  7. advansys_info is not safe against multiple simultaneous callers
  *  8. Add module_param to override ISA/VLB ioport array
  */
-#warning this driver is still not properly converted to the DMA API
 
 /* Enable driver /proc statistics. */
 #define ADVANSYS_STATS
@@ -12260,6 +12259,7 @@ static int advansys_pci_probe(struct pci_dev *pdev,
pdev-device == PCI_DEVICE_ID_38C0800_REV1 ||
pdev-device == PCI_DEVICE_ID_38C1600_REV1) {
board-flags |= ASC_IS_WIDE_BOARD;
+   pr_err(This driver is still not properly converted to the DMA 
API);
}
 
err = advansys_board_found(shost, ioport, ASC_IS_PCI);
-- 
1.8.5.3

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

[PATCH 0/2] advansys: remove warning and mark Orphan

[Paul Bolle]

0) I've tried to get a trivial patch to the Advansys SCSI driver merged.
It silences a build time warning. See:
- November 2012: https://lkml.org/lkml/2012/11/5/164 ;
- January 2013: https://lkml.org/lkml/2013/1/29/144 ; and
- January 2014: https://lkml.org/lkml/2014/1/8/569 .

That patch seems to be going nowhere. So a kernel build still generates
a warning when this driver is included. After seven years that is
getting pointless.

1) It seems that advansys is unmaintained. The last time its maintainer
touched it - by acking a patch - was in 2008, with commit 25729a7fb88e
([SCSI] advansys, arcmsr, ipr, nsp32, qla1280, stex: use
pci_ioremap_bar()). 

2) Fedora kernel people suggested to send this trivial patch to Andrew
Morton directly (see
https://lists.fedoraproject.org/pipermail/kernel/2014-January/004832.html ). 
They also suggested I'd mark this driver Orphan.

This small series does both.

Paul Bolle (2):
  advansys: change buildtime warning into runtime error
  MAINTAINERS: mark Advansys Orphan

 CREDITS | 2 +-
 MAINTAINERS | 3 +--
 drivers/scsi/advansys.c | 2 +-
 3 files changed, 3 insertions(+), 4 deletions(-)

-- 
1.8.5.3

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

Re: [PATCH] target_core_alua: silence GCC warning

[Paul Bolle]

On Wed, 2014-02-19 at 10:59 +0100, Geert Uytterhoeven wrote:
 On Wed, Feb 19, 2014 at 10:52 AM, Paul Bolle pebo...@tiscali.nl wrote:
  -   ret = core_alua_state_nonoptimized(cmd, cdb,
  -   nonop_delay_msecs, alua_ascq);
  +   core_alua_state_nonoptimized(cmd, cdb, nonop_delay_msecs);
 
 I suggest making core_alua_state_nonoptimized()  return void, too.
 Currently it always returns zero.

Good catch. Not sure how I missed that, since I deliberately ignore its
return value here.

I'll wait with sending a v2 until after Nicholas or the other target
developers have given their feedback.


Paul Bolle

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

[PATCH] target_core_alua: silence GCC warning

[Paul Bolle]

On Tue, 2014-02-18 at 10:02 +0100, Geert Uytterhoeven wrote:
 *** WARNINGS ***
 
 188 regressions:
   [...]
   + /scratch/kisskb/src/drivers/target/target_core_alua.c: warning: 
 'alua_ascq' may be used uninitialized in this function [-Wuninitialized]:  = 
 773:18

This one popped up on my (Fedora 20 based) builds on 32 and 64 bits x86.
Geert seems to have seen this on v3.12 already. I first saw it in
v3.14-rc1. Not clear why that is. Anyhow, this is what I came up with to
make this warning go away. (Compile tested only. I don't use this driver
myself.)
8
From: Paul Bolle pebo...@tiscali.nl

Building target_core_alua.o triggers a GCC warning:
drivers/target/target_core_alua.c: In function ‘target_alua_state_check’:
drivers/target/target_core_alua.c:773:18: warning: ‘alua_ascq’ may be used 
uninitialized in this function [-Wmaybe-uninitialized]
   cmd-scsi_ascq = alua_ascq;
  ^

This is a false positive. A little trial and error shows it is
apparently caused by core_alua_state_lba_dependent(). It must be hard
for GCC to track the branches of switch statement, inside a
list_for_each_entry loop, inside a while loop.

But if we add a small (inline) helper function we can reorganize the
code a bit. That also allows to drop alua_ascq which, obviously, gets
rid of this warning.

Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
 drivers/target/target_core_alua.c | 94 ++-
 1 file changed, 44 insertions(+), 50 deletions(-)

diff --git a/drivers/target/target_core_alua.c 
b/drivers/target/target_core_alua.c
index c3d9df6..4111f43 100644
--- a/drivers/target/target_core_alua.c
+++ b/drivers/target/target_core_alua.c
@@ -455,11 +455,26 @@ out:
return rc;
 }
 
+static inline void set_ascq(struct se_cmd *cmd, u8 alua_ascq)
+{
+   /*
+* Set SCSI additional sense code (ASC) to 'LUN Not Accessible';
+* The ALUA additional sense code qualifier (ASCQ) is determined
+* by the ALUA primary or secondary access state..
+*/
+   pr_debug([%s]: ALUA TG Port not available, 
+   SenseKey: NOT_READY, ASC/ASCQ: 
+   0x04/0x%02x\n,
+   cmd-se_tfo-get_fabric_name(), alua_ascq);
+
+   cmd-scsi_asc = 0x04;
+   cmd-scsi_ascq = alua_ascq;
+}
+
 static inline int core_alua_state_nonoptimized(
struct se_cmd *cmd,
unsigned char *cdb,
-   int nonop_delay_msecs,
-   u8 *alua_ascq)
+   int nonop_delay_msecs)
 {
/*
 * Set SCF_ALUA_NON_OPTIMIZED here, this value will be checked
@@ -471,10 +486,9 @@ static inline int core_alua_state_nonoptimized(
return 0;
 }
 
-static inline int core_alua_state_lba_dependent(
+static noinline int core_alua_state_lba_dependent(
struct se_cmd *cmd,
-   struct t10_alua_tg_pt_gp *tg_pt_gp,
-   u8 *alua_ascq)
+   struct t10_alua_tg_pt_gp *tg_pt_gp)
 {
struct se_device *dev = cmd-se_dev;
u64 segment_size, segment_mult, sectors, lba;
@@ -520,7 +534,7 @@ static inline int core_alua_state_lba_dependent(
}
if (!cur_map) {
spin_unlock(dev-t10_alua.lba_map_lock);
-   *alua_ascq = ASCQ_04H_ALUA_TG_PT_UNAVAILABLE;
+   set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_UNAVAILABLE);
return 1;
}
list_for_each_entry(map_mem, cur_map-lba_map_mem_list,
@@ -531,11 +545,11 @@ static inline int core_alua_state_lba_dependent(
switch(map_mem-lba_map_mem_alua_state) {
case ALUA_ACCESS_STATE_STANDBY:
spin_unlock(dev-t10_alua.lba_map_lock);
-   *alua_ascq = ASCQ_04H_ALUA_TG_PT_STANDBY;
+   set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_STANDBY);
return 1;
case ALUA_ACCESS_STATE_UNAVAILABLE:
spin_unlock(dev-t10_alua.lba_map_lock);
-   *alua_ascq = ASCQ_04H_ALUA_TG_PT_UNAVAILABLE;
+   set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_UNAVAILABLE);
return 1;
default:
break;
@@ -548,8 +562,7 @@ static inline int core_alua_state_lba_dependent(
 
 static inline int core_alua_state_standby(
struct se_cmd *cmd,
-   unsigned char *cdb,
-   u8 *alua_ascq)
+   unsigned char *cdb)
 {
/*
 * Allowed CDBs for ALUA_ACCESS_STATE_STANDBY as defined by
@@ -570,7 +583,7 @@ static inline int core_alua_state_standby(
case MI_REPORT_TARGET_PGS:
return 0;
default:
-   *alua_ascq = ASCQ_04H_ALUA_TG_PT_STANDBY;
+   set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_STANDBY);
return 1

Re: [PATCH] [SCSI] advansys: change buildtime warning into runtime error

[Paul Bolle]

On Tue, 2013-01-29 at 11:20 +0100, Paul Bolle wrote:
 On Mon, 2012-11-05 at 11:58 +0100, Paul Bolle wrote:
  Building advansys.o triggers this warning:
  drivers/scsi/advansys.c:71:2: warning: #warning this driver is still 
  not properly converted to the DMA API [-Wcpp]
  
  This warning can be traced back to a patch called advansys: add warning
  and convert #includes which was included in v2.6.10. That patch also
  marked the driver as BROKEN.
  
  Commit 4661e3eace2c7b8433476b5bf0ee437ab3c7dfd4 ([SCSI] advansys
  driver: limp along on x86) enabled this driver for x86-32. And commit
  9d511a4b29de6764931343d03e493f2e04df0271 ([SCSI] advansys: Changes to
  work on parisc) enabled this driver for all architectures. But the
  commit explanation stated:
  I haven't removed the #warning yet because virt_to_bus/bus_to_virt are
  only eliminated for narrow boards.  Wide boards need more work.
  
  Five years have passed and, apparently, those wide boards still need
  more work. So let's change the buildtime warning into a runtime error,
  only printed for those wide boards. Perhaps that might push the people
  using those wide boards to convert this driver. And for all others
  there's now one less buildtime warning to ignore.
  
  Signed-off-by: Paul Bolle pebo...@tiscali.nl
  ---
  Compile tested only. I don't have any AdvanSys SCSI boards (neither
  narrow nor wide).
 
 The date of this message suggests I submitted this patch for a warning
 seen while building v3.7-rc4. An identical warning can be seen while
 building v3.8-rc5. What's the status of my patch? Did anyone find some
 time to have a look at it?

I've been carrying this patch locally for over a year. Is there any
chance of someone trying to remove this buildtime warning?

I'm inclined to submit a path to Fedora - my local builds use
their .config as a base - to just disable this driver. It seems that
would increase my chances of finally shutting down this warning.


Paul Bolle

--
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] advansys: change buildtime warning into runtime error

[Paul Bolle]

On Wed, 2014-01-08 at 21:15 +0100, Ondrej Zary wrote:
 The driver works fine with narrow boards. Please don't break working drivers 
 because of some stupid warning.

My patch changed a buildtime warning for both narrow and wide boards in
a runtime error on wide boards only. That wouldn't break the narrow
boards. (I happen to own neither a narrow nor a wide board.)

In seven years that buildtime warning has not resulted in a fix for wide
boards, has it? So at this point that warning is rather pointless.

Is anybody even using these wide boards?


Paul Bolle

--
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] bnx2i: add a prefix to a printk

[Paul Bolle]

(Eddie was added as maintainer a few months ago, so Eddie receives this
message too.)

On Wed, 2012-06-27 at 21:26 +0200, Paul Bolle wrote:
 During each suspend and resume cycle a mysterious message is added to
 the logs:
 CPU 1 offline: Remove Rx thread
 
 Some grepping of the tree revealed this message is printed by bnx2i.
 Add a prefix to this message to make it clear that it is printed by
 bnx2i. That should also make it obvious it is the mirror of the
 bnx2i: CPU 1 online: Create Rx thread
 
 message.
 
 Signed-off-by: Paul Bolle pebo...@tiscali.nl

This issue is still present. Could someone please have a look at this
patch?

 0) Untested!

It's tested now (on top of v3.12.1).

 1) I only noticed this because I've been looking at the log messages
 generated by suspend and resume (for some iwlegacy and i915 issues) and
 because Fedora 16 enables bnx2i by default.

Fedora 18 still enables bnx2i by default.

 2) This triggers a checkpatch warning:
 WARNING: Prefer pr_info(... to printk(KERN_INFO, ...
 
 So I added Joe to the Cc's to suggest a way to make all printks in this
 file end up with a bnx2i: prefix (or whatever prefix is preferred).
 
  drivers/scsi/bnx2i/bnx2i_init.c |3 ++-
  1 files changed, 2 insertions(+), 1 deletions(-)
 
 diff --git a/drivers/scsi/bnx2i/bnx2i_init.c b/drivers/scsi/bnx2i/bnx2i_init.c
 index 8b68167..b75864b 100644
 --- a/drivers/scsi/bnx2i/bnx2i_init.c
 +++ b/drivers/scsi/bnx2i/bnx2i_init.c
 @@ -456,7 +456,8 @@ static int bnx2i_cpu_callback(struct notifier_block *nfb,
   break;
   case CPU_DEAD:
   case CPU_DEAD_FROZEN:
 - printk(KERN_INFO CPU %x offline: Remove Rx thread\n, cpu);
 + printk(KERN_INFO bnx2i: CPU %x offline: Remove Rx thread\n,
 + cpu);
   bnx2i_percpu_thread_destroy(cpu);
   break;
   default:


Paul Bolle

--
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: fix the build warning

[Paul Bolle]

On Thu, 2013-08-22 at 23:49 +0900, Akinobu Mita wrote:
 2013/8/22 James Bottomley jbottom...@parallels.com:
  On Thu, 2013-08-22 at 21:42 +0900, Akinobu Mita wrote:
  Unfortunately, this warning isn't fixed in linux-next, either.
  Paul Bolle also sent a patch that fixes the same warning in a little
  bit different way.
 
  Well, it is and it isn't.  Whether you see the warning seems to depend
  on how gcc was built.  My take is that an impossible default case just
  to keep some versions of gcc quiet is a bit pointless.
 
 As Joe said in the other reply, scsi_debug_guard could be a negative
 value (scsi_debug_guard  1 is only prohibited).  So this warning
 does not seem a false positive.

I too think that GCC is correct here. Perhaps the people not seeing this
warning don't have CONFIG_SCSI_DEBUG set.

A week ago Antonia also submitted a patch to silence this warning
( https://lkml.org/lkml/2013/9/13/649 ). That's at least the third time
someone tried to silence it since it got introduced in the v3.11 cycle.

Akinobu, could you please say how you'd like this warning to be
silenced? Or is an actual fix queued somewhere?


Paul Bolle

--
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: fix the build warning

[Paul Bolle]

On Fri, 2013-09-20 at 23:32 +0900, Akinobu Mita wrote:
 Yesterday, I sent a patch set which includes two fixes for this issue.
 I wish this to be merged and I'll do my best.

I hadn't yet stumbled onto these patches. Thanks!


Paul Bolle

--
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: scsi: delete decade+ obsolete aic7xxx_old driver

[Paul Bolle]

On Mon, 2013-09-16 at 21:51 -0400, Paul Gortmaker wrote:
 Currently we have people wasting time building it during routine testing,
 and then wasting more time re-researching the known reported warnings,
 only to find that nobody really is willing to integrate the fixes[3] for
 it.
 
[...]

 [3] https://lkml.org/lkml/2012/10/29/215

Well, this didn't end up as an entire waste of my time. After that
message I sent a patch to Fedora's kernel list, and a reminder a few
months later[1]. That prompted Josh Boyer to remove this old driver from
the Fedora build[2].

And now that driver is disabled in all kernels that Fedora currently
ships. I'm not familiar with any complaints about this decision.


Paul Bolle

[1] https://lists.fedoraproject.org/pipermail/kernel/2013-February/004102.html
[2] 
http://pkgs.fedoraproject.org/cgit/kernel.git/commit/?id=2192022f4bf13b30e389e77170da7ae08fd28ecf

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

[PATCH v3] [SCSI] scsi_debug: silence GCC warning

[Paul Bolle]

Building scsi_debug.o triggers a GCC warning:
drivers/scsi/scsi_debug.c: In function ‘dif_verify’:
drivers/scsi/scsi_debug.c:1755:3: warning: ‘csum’ may be used uninitialized 
in this function [-Wmaybe-uninitialized]

This warning was apparently introduced (in v3.11-rc1) by commit
beb40ea42b ([SCSI] scsi_debug: reduce duplication between
prot_verify_read and prot_verify_write). It is a false positive. But if
we transform the switch statement in dif_compute_csum() to a
straightforward if/else statement we supply GCC with enough information
to determine that csum will not be used uninitialized.

Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
0) v2: much simpler, and doesn't change any behavior;
   v3: mention commit bebe40ea42b, as Akinobu suggested.

1) Still only compile tested.

 drivers/scsi/scsi_debug.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index cb4fefa..7565ec5 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1735,14 +1735,11 @@ static u16 dif_compute_csum(const void *buf, int len)
 {
u16 csum;
 
-   switch (scsi_debug_guard) {
-   case 1:
+   if (scsi_debug_guard == 1)
csum = ip_compute_csum(buf, len);
-   break;
-   case 0:
+   else
csum = cpu_to_be16(crc_t10dif(buf, len));
-   break;
-   }
+
return csum;
 }
 
-- 
1.8.1.4

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

Re: [PATCH v2] [SCSI] scsi_debug: silence GCC warning

[Paul Bolle]

On Wed, 2013-07-17 at 00:21 +0900, Akinobu Mita wrote:
 This one looks good to me.

Thanks.

 It would be much better if this commit
 log had a reference to the commit that introduced this warning as
 you described after '---' in v1 patch.

Do you mean that I should submit a v3, with a commit explanation that
also has one or two lines that mention commit beb40ea42b ([SCSI]
scsi_debug: reduce duplication between prot_verify_read and
prot_verify_write)?


Paul Bolle

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

[PATCH] [SCSI] scsi_debug: silence GCC warning

[Paul Bolle]

Building scsi_debug.o triggers a GCC warning:
drivers/scsi/scsi_debug.c: In function ‘dif_verify’:
drivers/scsi/scsi_debug.c:1755:3: warning: ‘csum’ may be used uninitialized 
in this function [-Wmaybe-uninitialized]

This is a false positive, but if we make scsi_debug_guard a bool, we
supply GCC with enough information to determine that csum will not be
used uninitialized. It also allows for a minor cleanup.

Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
0) Compile tested only.

1) This warning is apparently introduced in v3.11-rc1 by commit beb40ea42b
([SCSI] scsi_debug: reduce duplication between prot_verify_read and
prot_verify_write)

 drivers/scsi/scsi_debug.c | 15 +--
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index cb4fefa..98e6436 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -169,7 +169,7 @@ static int scsi_debug_dix = DEF_DIX;
 static int scsi_debug_dsense = DEF_D_SENSE;
 static int scsi_debug_every_nth = DEF_EVERY_NTH;
 static int scsi_debug_fake_rw = DEF_FAKE_RW;
-static int scsi_debug_guard = DEF_GUARD;
+static bool scsi_debug_guard = DEF_GUARD;
 static int scsi_debug_lowest_aligned = DEF_LOWEST_ALIGNED;
 static int scsi_debug_max_luns = DEF_MAX_LUNS;
 static int scsi_debug_max_queue = SCSI_DEBUG_CANQUEUE;
@@ -1736,10 +1736,10 @@ static u16 dif_compute_csum(const void *buf, int len)
u16 csum;
 
switch (scsi_debug_guard) {
-   case 1:
+   case true:
csum = ip_compute_csum(buf, len);
break;
-   case 0:
+   case false:
csum = cpu_to_be16(crc_t10dif(buf, len));
break;
}
@@ -2736,7 +2736,7 @@ module_param_named(dix, scsi_debug_dix, int, S_IRUGO);
 module_param_named(dsense, scsi_debug_dsense, int, S_IRUGO | S_IWUSR);
 module_param_named(every_nth, scsi_debug_every_nth, int, S_IRUGO | S_IWUSR);
 module_param_named(fake_rw, scsi_debug_fake_rw, int, S_IRUGO | S_IWUSR);
-module_param_named(guard, scsi_debug_guard, int, S_IRUGO);
+module_param_named(guard, scsi_debug_guard, bool, S_IRUGO);
 module_param_named(lbpu, scsi_debug_lbpu, int, S_IRUGO);
 module_param_named(lbpws, scsi_debug_lbpws, int, S_IRUGO);
 module_param_named(lbpws10, scsi_debug_lbpws10, int, S_IRUGO);
@@ -3312,11 +3312,6 @@ static int __init scsi_debug_init(void)
return -EINVAL;
}
 
-   if (scsi_debug_guard  1) {
-   printk(KERN_ERR scsi_debug_init: guard must be 0 or 1\n);
-   return -EINVAL;
-   }
-
if (scsi_debug_ato  1) {
printk(KERN_ERR scsi_debug_init: ato must be 0 or 1\n);
return -EINVAL;
@@ -4028,7 +4023,7 @@ static int sdebug_driver_probe(struct device * dev)
   (host_prot  SHOST_DIX_TYPE2_PROTECTION) ?  DIX2 : ,
   (host_prot  SHOST_DIX_TYPE3_PROTECTION) ?  DIX3 : );
 
-   if (scsi_debug_guard == 1)
+   if (scsi_debug_guard == true)
scsi_host_set_guard(hpnt, SHOST_DIX_GUARD_IP);
else
scsi_host_set_guard(hpnt, SHOST_DIX_GUARD_CRC);
-- 
1.8.1.4

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

[PATCH v2] [SCSI] scsi_debug: silence GCC warning

[Paul Bolle]

Building scsi_debug.o triggers a GCC warning:
drivers/scsi/scsi_debug.c: In function ‘dif_verify’:
drivers/scsi/scsi_debug.c:1755:3: warning: ‘csum’ may be used uninitialized 
in this function [-Wmaybe-uninitialized]

This is a false positive. But if we transform the switch statement in
dif_compute_csum() to a straightforward if/else statement we supply GCC
with enough information to determine that csum will not be used
uninitialized.

Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
0) v2 because I started to worry whether v1 would change an interface
(ie, the way the guard module parameter behaves). This patch is much
simpler, and doesn't change any behavior.

1) Still only compile tested.

 drivers/scsi/scsi_debug.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index cb4fefa..7565ec5 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1735,14 +1735,11 @@ static u16 dif_compute_csum(const void *buf, int len)
 {
u16 csum;
 
-   switch (scsi_debug_guard) {
-   case 1:
+   if (scsi_debug_guard == 1)
csum = ip_compute_csum(buf, len);
-   break;
-   case 0:
+   else
csum = cpu_to_be16(crc_t10dif(buf, len));
-   break;
-   }
+
return csum;
 }
 
-- 
1.8.1.4

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

Re: [PATCH] [SCSI] aacraid: silence two GCC warnings

[Paul Bolle]

On Wed, 2013-02-20 at 10:40 +, James Bottomley wrote:
 On Sat, 2013-02-09 at 21:09 +0100, Paul Bolle wrote:
  --- a/drivers/scsi/aacraid/src.c
  +++ b/drivers/scsi/aacraid/src.c
  @@ -407,7 +407,7 @@ static int aac_src_deliver_message(struct fib *fib)
  fib-hw_fib_va-header.StructType = FIB_MAGIC2;
  fib-hw_fib_va-header.SenderFibAddress = (u32)address;
  fib-hw_fib_va-header.u.TimeStamp = 0;
  -   BUG_ON((u32)(address  32) != 0L);
  +   BUG_ON(((u64)address  32) != 0UL);
 
 Actually, this isn't the right way to do this.  The correct way would be
 address  16  16, but no-one is expected to remember this, so we have
 it macroised in kernel.h as upper_32_bits().
 
 So this should become
 
 BUG_ON(upper_32_bits(address) != 0L);

Thanks. I hadn't yet stumbled onto that macro.

 and a similar change for the below.

I hope to send an updated patch shortly.


Paul Bolle

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

[PATCH] [SCSI] aacraid: silence two GCC warnings

[Paul Bolle]

Compiling src.o for a 32 bit system triggers two GCC warnings:
drivers/scsi/aacraid/src.c: In function ‘aac_src_deliver_message’:
drivers/scsi/aacraid/src.c:410:3: warning: right shift count = width of 
type [enabled by default]
drivers/scsi/aacraid/src.c:434:2: warning: right shift count = width of 
type [enabled by default]

Silence these warnings by casting the 'address' variable (of type
dma_addr_t) to u64 on those two lines.

Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
0) Compile tested only.

1) Changing '0L' to 'OUL' might be a bit of cargo cult programming. I
doubt it's necessary. Members of that cult might also change
'0x' to '0xUL', but I didn't.

 drivers/scsi/aacraid/src.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/aacraid/src.c b/drivers/scsi/aacraid/src.c
index 3b021ec..dfa8a37 100644
--- a/drivers/scsi/aacraid/src.c
+++ b/drivers/scsi/aacraid/src.c
@@ -407,7 +407,7 @@ static int aac_src_deliver_message(struct fib *fib)
fib-hw_fib_va-header.StructType = FIB_MAGIC2;
fib-hw_fib_va-header.SenderFibAddress = (u32)address;
fib-hw_fib_va-header.u.TimeStamp = 0;
-   BUG_ON((u32)(address  32) != 0L);
+   BUG_ON(((u64)address  32) != 0UL);
address |= fibsize;
} else {
/* Calculate the amount to the fibsize bits */
@@ -431,7 +431,7 @@ static int aac_src_deliver_message(struct fib *fib)
address |= fibsize;
}
 
-   src_writel(dev, MUnit.IQ_H, (address  32)  0x);
+   src_writel(dev, MUnit.IQ_H, ((u64)address  32)  0x);
src_writel(dev, MUnit.IQ_L, address  0x);
 
return 0;
-- 
1.8.1.2

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

Re: [PATCH v3] qla2xxx: silence two GCC warnings

[Paul Bolle]

On Wed, 2013-01-30 at 08:07 +, Saurav Kashyap wrote:
 I am submitting some correction patches today and this patch will be part
 of the scsi-misc submission after that set.

Thanks.


Paul Bolle

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

Re: [PATCH v3] qla2xxx: silence two GCC warnings

[Paul Bolle]

On Mon, 2012-10-08 at 11:15 -0500, Saurav Kashyap wrote:
 Acked-by: Saurav Kashyap saurav.kash...@qlogic.com
 
 Thanks,
 ~Saurav
 
 Compiling qla_gs.o (part of the qla2xxx module) triggers two GCC
 warnings:
 drivers/scsi/qla2xxx/qla_gs.c: In function Œqla2x00_fdmi_rhba¹:
 drivers/scsi/qla2xxx/qla_gs.c:1339:7: warning: array subscript is
 above array bounds [-Warray-bounds]
 drivers/scsi/qla2xxx/qla_gs.c: In function Œqla2x00_fdmi_register¹:
 drivers/scsi/qla2xxx/qla_gs.c:1663:15: warning: array subscript is
 above array bounds [-Warray-bounds]

This patch was originally posted to silence two GCC warnings while
building v3.6-rc7. Basically identical warnings can still be seen while
building v3.8-rc5. What's the status of this patch?


Paul Bolle

--
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] aic7xxx_old: silence GCC warnings

[Paul Bolle]

On Mon, 2012-10-29 at 13:45 +0100, Paul Bolle wrote:
 On Mon, 2012-10-29 at 12:17 +, James Bottomley wrote:
  mvsas has a maintainer: poke them harder
 
 According to MAINTAINERS that's you. Is Xiangliang Yu perhaps the actual
 maintainer?

Building the mvsas driver triggers identical warnings in v3.8-rc5:
drivers/scsi/mvsas/mv_sas.c: In function ‘mvs_update_phyinfo’:
drivers/scsi/mvsas/mv_sas.c:1156:34: warning: comparison between ‘enum 
sas_device_type’ and ‘enum sas_dev_type’ [-Wenum-compare]
drivers/scsi/mvsas/mv_sas.c:1159:39: warning: comparison between ‘enum 
sas_device_type’ and ‘enum sas_dev_type’ [-Wenum-compare]

And according to MAINTAINERS you're still the maintainer. So I'd guess
I'm poking the maintainer now.


Paul Bolle

--
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] advansys: change buildtime warning into runtime error

[Paul Bolle]

On Mon, 2012-11-05 at 11:58 +0100, Paul Bolle wrote:
 Building advansys.o triggers this warning:
 drivers/scsi/advansys.c:71:2: warning: #warning this driver is still not 
 properly converted to the DMA API [-Wcpp]
 
 This warning can be traced back to a patch called advansys: add warning
 and convert #includes which was included in v2.6.10. That patch also
 marked the driver as BROKEN.
 
 Commit 4661e3eace2c7b8433476b5bf0ee437ab3c7dfd4 ([SCSI] advansys
 driver: limp along on x86) enabled this driver for x86-32. And commit
 9d511a4b29de6764931343d03e493f2e04df0271 ([SCSI] advansys: Changes to
 work on parisc) enabled this driver for all architectures. But the
 commit explanation stated:
 I haven't removed the #warning yet because virt_to_bus/bus_to_virt are
 only eliminated for narrow boards.  Wide boards need more work.
 
 Five years have passed and, apparently, those wide boards still need
 more work. So let's change the buildtime warning into a runtime error,
 only printed for those wide boards. Perhaps that might push the people
 using those wide boards to convert this driver. And for all others
 there's now one less buildtime warning to ignore.
 
 Signed-off-by: Paul Bolle pebo...@tiscali.nl
 ---
 Compile tested only. I don't have any AdvanSys SCSI boards (neither
 narrow nor wide).

The date of this message suggests I submitted this patch for a warning
seen while building v3.7-rc4. An identical warning can be seen while
building v3.8-rc5. What's the status of my patch? Did anyone find some
time to have a look at it?


Paul Bolle

--
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: linux-3.7.4: BUG: unable to handle kernel NULL pointer dereference at target_fabric_port_link

[Paul Bolle]

On Thu, 2013-01-24 at 01:07 +0900, Kouichi ONO wrote:
 At target_fabric_port_link(), struct se_device *dev is used before set?

It seems the (stable specific) patch in
http://article.gmane.org/gmane.linux.kernel.stable/40880 should fix
that.


Paul Bolle

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

[PATCH] [SCSI] advansys: change buildtime warning into runtime error

[Paul Bolle]

Building advansys.o triggers this warning:
drivers/scsi/advansys.c:71:2: warning: #warning this driver is still not 
properly converted to the DMA API [-Wcpp]

This warning can be traced back to a patch called advansys: add warning
and convert #includes which was included in v2.6.10. That patch also
marked the driver as BROKEN.

Commit 4661e3eace2c7b8433476b5bf0ee437ab3c7dfd4 ([SCSI] advansys
driver: limp along on x86) enabled this driver for x86-32. And commit
9d511a4b29de6764931343d03e493f2e04df0271 ([SCSI] advansys: Changes to
work on parisc) enabled this driver for all architectures. But the
commit explanation stated:
I haven't removed the #warning yet because virt_to_bus/bus_to_virt are
only eliminated for narrow boards.  Wide boards need more work.

Five years have passed and, apparently, those wide boards still need
more work. So let's change the buildtime warning into a runtime error,
only printed for those wide boards. Perhaps that might push the people
using those wide boards to convert this driver. And for all others
there's now one less buildtime warning to ignore.

Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
Compile tested only. I don't have any AdvanSys SCSI boards (neither
narrow nor wide).

 drivers/scsi/advansys.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
index 374c4ed..23d347d 100644
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -68,7 +68,6 @@
  *  7. advansys_info is not safe against multiple simultaneous callers
  *  8. Add module_param to override ISA/VLB ioport array
  */
-#warning this driver is still not properly converted to the DMA API
 
 /* Enable driver /proc statistics. */
 #define ADVANSYS_STATS
@@ -12772,6 +12771,7 @@ advansys_pci_probe(struct pci_dev *pdev, const struct 
pci_device_id *ent)
pdev-device == PCI_DEVICE_ID_38C0800_REV1 ||
pdev-device == PCI_DEVICE_ID_38C1600_REV1) {
board-flags |= ASC_IS_WIDE_BOARD;
+   pr_err(This driver is still not properly converted to the DMA 
API);
}
 
err = advansys_board_found(shost, ioport, ASC_IS_PCI);
-- 
1.7.11.7

--
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] aic7xxx_old: silence GCC warnings

[Paul Bolle]

On Fri, 2012-09-21 at 11:28 +0200, Paul Bolle wrote:
 Building the aic7xxx_old driver triggers these GCC warnings:
 drivers/scsi/aic7xxx_old.c:7901:5: warning: case value '257' not in 
 enumerated type 'ahc_chip' [-Wswitch]
 drivers/scsi/aic7xxx_old.c:7898:5: warning: case value '513' not in 
 enumerated type 'ahc_chip' [-Wswitch]
 drivers/scsi/aic7xxx_old.c:8517:5: warning: case value '257' not in 
 enumerated type 'ahc_chip' [-Wswitch]
 drivers/scsi/aic7xxx_old.c:8510:5: warning: case value '513' not in 
 enumerated type 'ahc_chip' [-Wswitch]
 
 Fix these warnings by adopting the idiom used elsewhere in this driver.
 Since AHC_EISA and AHC_VL are only ever set for AHC_AIC7770 this fix
 should not lead to any functional change.
 
 Signed-off-by: Paul Bolle pebo...@tiscali.nl
 ---
 0) I noticed these warnings while building v3.6-rc6 on current Fedora
 17, using Fedora's default config.

Identical warnings can be seen while building v3.7-rc3. What's the
status of my patch? Did anyone found some time to have a look at it?


Paul Bolle

--
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] aic94xx: fix handling of default CTRL-A settings

[Paul Bolle]

On Tue, 2012-09-25 at 14:07 +0200, Paul Bolle wrote:
 Compiling aic94xx_sds.o (part of the aic94xx driver) triggers this GCC
 warning:
 drivers/scsi/aic94xx/aic94xx_sds.c: In function 'asd_read_flash':
 drivers/scsi/aic94xx/aic94xx_sds.c:597:21: warning: 'offs' may be used 
 uninitialized in this function [-Wmaybe-uninitialized]
 drivers/scsi/aic94xx/aic94xx_sds.c:985:6: note: 'offs' was declared here
 
 This warning can be traced back to asd_find_flash_de(). If it fails to
 find a FLASH_DE_CTRL_A_USER entry it will return without setting 'offs'.
 And that will leave 'offs' uninitialized when asd_read_flash_seg() is
 called a little later.
 
 But that call of asd_read_flash_seg() isn't needed in that case.
 Everything this code cares about can already be found in the default
 CTRL-A user settings section that was created in the error path. So
 let's just jump over that call (and all other unneeded code) after
 creating that section.
 
 Signed-off-by: Paul Bolle pebo...@tiscali.nl
 ---
 0) I noticed this warning while building v3.6-rc7 on current Fedora
 17, using Fedora's default config.

And, again, identical warnings can be seen while building v3.7-rc3.
What's the status of my patch? Did anyone found some time to have a look
at it?


Paul Bolle

--
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] aic7xxx_old: silence GCC warnings

[Paul Bolle]

On Mon, 2012-10-29 at 12:17 +, James Bottomley wrote:
 the aic7xxx_old driver is in deep maintenance; I don't think anyone can
 actually test changes to it anymore, so we just keep it around unchanged
 for the odd really old card that can't be driven by the current driver.
 For this reason, we tend only to do tested bug fixes to it because
 no-one will notice if any error is introduced.
 
 mvsas has a maintainer: poke them harder

According to MAINTAINERS that's you. Is Xiangliang Yu perhaps the actual
maintainer?

 and aic94xx is currently
 maintainerless, so it will depend on someone finding the time to test.

Thanks,


Paul Bolle

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

[PATCH v3] qla2xxx: silence two GCC warnings

[Paul Bolle]

Compiling qla_gs.o (part of the qla2xxx module) triggers two GCC
warnings:
drivers/scsi/qla2xxx/qla_gs.c: In function ‘qla2x00_fdmi_rhba’:
drivers/scsi/qla2xxx/qla_gs.c:1339:7: warning: array subscript is above 
array bounds [-Warray-bounds]
drivers/scsi/qla2xxx/qla_gs.c: In function ‘qla2x00_fdmi_register’:
drivers/scsi/qla2xxx/qla_gs.c:1663:15: warning: array subscript is above 
array bounds [-Warray-bounds]

It seems that the sequence of a strcpy followed by a strlen confuses GCC
when it is keeping track of array bounds here. (It is not clear to me
which array triggers this warning and by how much GCC thinks the
subscript is above its bounds. Neither is it clear to me why comparable
code in these two functions doesn't trigger this warning.)

An easy way to silence these warnings is to use preprocessor macros and
strncpy, as that apparently gives GCC enough information to keep track
of array bounds.

Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
0) Updated for Saurav's request to use strncpy().

1) Still only compile tested.

 drivers/scsi/qla2xxx/qla_def.h | 1 +
 drivers/scsi/qla2xxx/qla_gs.c  | 8 
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index 39007f5..8895038 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -37,6 +37,7 @@
 #include qla_nx.h
 #define QLA2XXX_DRIVER_NAMEqla2xxx
 #define QLA2XXX_APIDEV ql2xapidev
+#define QLA2XXX_MANUFACTURER   QLogic Corporation
 
 /*
  * We have MAILBOX_REGISTER_COUNT sized arrays in a few places,
diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c
index 05260d2..824cbcf 100644
--- a/drivers/scsi/qla2xxx/qla_gs.c
+++ b/drivers/scsi/qla2xxx/qla_gs.c
@@ -1325,8 +1325,8 @@ qla2x00_fdmi_rhba(scsi_qla_host_t *vha)
/* Manufacturer. */
eiter = (struct ct_fdmi_hba_attr *) (entries + size);
eiter-type = __constant_cpu_to_be16(FDMI_HBA_MANUFACTURER);
-   strcpy(eiter-a.manufacturer, QLogic Corporation);
-   alen = strlen(eiter-a.manufacturer);
+   alen = strlen(QLA2XXX_MANUFACTURER);
+   strncpy(eiter-a.manufacturer, QLA2XXX_MANUFACTURER, alen + 1);
alen += (alen  3) ? (4 - (alen  3)) : 4;
eiter-len = cpu_to_be16(4 + alen);
size += 4 + alen;
@@ -1646,8 +1646,8 @@ qla2x00_fdmi_rpa(scsi_qla_host_t *vha)
/* OS device name. */
eiter = (struct ct_fdmi_port_attr *) (entries + size);
eiter-type = __constant_cpu_to_be16(FDMI_PORT_OS_DEVICE_NAME);
-   strcpy(eiter-a.os_dev_name, QLA2XXX_DRIVER_NAME);
-   alen = strlen(eiter-a.os_dev_name);
+   alen = strlen(QLA2XXX_DRIVER_NAME);
+   strncpy(eiter-a.os_dev_name, QLA2XXX_DRIVER_NAME, alen + 1);
alen += (alen  3) ? (4 - (alen  3)) : 4;
eiter-len = cpu_to_be16(4 + alen);
size += 4 + alen;
-- 
1.7.11.4

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

[PATCH v2] qla2xxx: silence two GCC warnings

[Paul Bolle]

Compiling qla_gs.o (part of the qla2xxx module) triggers two GCC
warnings:
drivers/scsi/qla2xxx/qla_gs.c: In function ‘qla2x00_fdmi_rhba’:
drivers/scsi/qla2xxx/qla_gs.c:1339:7: warning: array subscript is above 
array bounds [-Warray-bounds]
drivers/scsi/qla2xxx/qla_gs.c: In function ‘qla2x00_fdmi_register’:
drivers/scsi/qla2xxx/qla_gs.c:1663:15: warning: array subscript is above 
array bounds [-Warray-bounds]

It seems that the sequence of a strcpy followed by a strlen confuses GCC
when it is keeping track of array bounds here. (It is not clear to me
which array triggers this warning and by how much GCC thinks the
subscript is above its bounds. Neither is it clear to me why comparable
code in these two functions doesn't trigger this warning.)

An easy way to silence these warnings is to use preprocessor macros
here, as that apparently gives GCC enough information to keep track of
array bounds.

Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
0) Rolf suggested to not use magic constants, to make sure things keep
working when these strings change in the future. A trivial solution is
to use preprocessor macros. I needed to add one for the manufacturer
string.

1) Still only compile tested.

 drivers/scsi/qla2xxx/qla_def.h | 1 +
 drivers/scsi/qla2xxx/qla_gs.c  | 6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index 39007f5..8895038 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -37,6 +37,7 @@
 #include qla_nx.h
 #define QLA2XXX_DRIVER_NAMEqla2xxx
 #define QLA2XXX_APIDEV ql2xapidev
+#define QLA2XXX_MANUFACTURER   QLogic Corporation
 
 /*
  * We have MAILBOX_REGISTER_COUNT sized arrays in a few places,
diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c
index 05260d2..1714035 100644
--- a/drivers/scsi/qla2xxx/qla_gs.c
+++ b/drivers/scsi/qla2xxx/qla_gs.c
@@ -1325,8 +1325,8 @@ qla2x00_fdmi_rhba(scsi_qla_host_t *vha)
/* Manufacturer. */
eiter = (struct ct_fdmi_hba_attr *) (entries + size);
eiter-type = __constant_cpu_to_be16(FDMI_HBA_MANUFACTURER);
-   strcpy(eiter-a.manufacturer, QLogic Corporation);
-   alen = strlen(eiter-a.manufacturer);
+   strcpy(eiter-a.manufacturer, QLA2XXX_MANUFACTURER);
+   alen = strlen(QLA2XXX_MANUFACTURER);
alen += (alen  3) ? (4 - (alen  3)) : 4;
eiter-len = cpu_to_be16(4 + alen);
size += 4 + alen;
@@ -1647,7 +1647,7 @@ qla2x00_fdmi_rpa(scsi_qla_host_t *vha)
eiter = (struct ct_fdmi_port_attr *) (entries + size);
eiter-type = __constant_cpu_to_be16(FDMI_PORT_OS_DEVICE_NAME);
strcpy(eiter-a.os_dev_name, QLA2XXX_DRIVER_NAME);
-   alen = strlen(eiter-a.os_dev_name);
+   alen = strlen(QLA2XXX_DRIVER_NAME);
alen += (alen  3) ? (4 - (alen  3)) : 4;
eiter-len = cpu_to_be16(4 + alen);
size += 4 + alen;
-- 
1.7.11.4

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

[PATCH] qla2xxx: silence two GCC warnings

[Paul Bolle]

Compiling qla_gs.o (part of the qla2xxx module) triggers two GCC
warnings:
drivers/scsi/qla2xxx/qla_gs.c: In function ‘qla2x00_fdmi_rhba’:
drivers/scsi/qla2xxx/qla_gs.c:1339:7: warning: array subscript is above 
array bounds [-Warray-bounds]
drivers/scsi/qla2xxx/qla_gs.c: In function ‘qla2x00_fdmi_register’:
drivers/scsi/qla2xxx/qla_gs.c:1663:15: warning: array subscript is above 
array bounds [-Warray-bounds]

It seems that the sequence of a strcpy followed by a strlen confuses GCC
when it is keeping track of array bounds here. (It is not clear to me
which array triggers this warning and by how much GCC thinks the
subscript is above its bounds. Neither is it clear to me why comparable
code in these two functions doesn't trigger this warning.)

The easiest way to silence these warnings is to hardcode the length of
these two strings in the code here. The length used here is the length
of the string, including its NUL terminator, rounded up to the next
multiple of four.

Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
0) I noticed this warning while building v3.6-rc7 on current Fedora
17, using Fedora's default config.

1) Compile tested only. 

 drivers/scsi/qla2xxx/qla_gs.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c
index 05260d2..a3ef5d0 100644
--- a/drivers/scsi/qla2xxx/qla_gs.c
+++ b/drivers/scsi/qla2xxx/qla_gs.c
@@ -1326,10 +1326,8 @@ qla2x00_fdmi_rhba(scsi_qla_host_t *vha)
eiter = (struct ct_fdmi_hba_attr *) (entries + size);
eiter-type = __constant_cpu_to_be16(FDMI_HBA_MANUFACTURER);
strcpy(eiter-a.manufacturer, QLogic Corporation);
-   alen = strlen(eiter-a.manufacturer);
-   alen += (alen  3) ? (4 - (alen  3)) : 4;
-   eiter-len = cpu_to_be16(4 + alen);
-   size += 4 + alen;
+   eiter-len = cpu_to_be16(4 + 20);
+   size += 4 + 20;
 
ql_dbg(ql_dbg_disc, vha, 0x2026,
Manufacturer = %s.\n, eiter-a.manufacturer);
@@ -1647,10 +1645,8 @@ qla2x00_fdmi_rpa(scsi_qla_host_t *vha)
eiter = (struct ct_fdmi_port_attr *) (entries + size);
eiter-type = __constant_cpu_to_be16(FDMI_PORT_OS_DEVICE_NAME);
strcpy(eiter-a.os_dev_name, QLA2XXX_DRIVER_NAME);
-   alen = strlen(eiter-a.os_dev_name);
-   alen += (alen  3) ? (4 - (alen  3)) : 4;
-   eiter-len = cpu_to_be16(4 + alen);
-   size += 4 + alen;
+   eiter-len = cpu_to_be16(4 + 8);
+   size += 4 + 8;
 
ql_dbg(ql_dbg_disc, vha, 0x204b,
OS_Device_Name=%s.\n, eiter-a.os_dev_name);
-- 
1.7.11.4

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

[PATCH] aic94xx: fix handling of default CTRL-A settings

[Paul Bolle]

Compiling aic94xx_sds.o (part of the aic94xx driver) triggers this GCC
warning:
drivers/scsi/aic94xx/aic94xx_sds.c: In function 'asd_read_flash':
drivers/scsi/aic94xx/aic94xx_sds.c:597:21: warning: 'offs' may be used 
uninitialized in this function [-Wmaybe-uninitialized]
drivers/scsi/aic94xx/aic94xx_sds.c:985:6: note: 'offs' was declared here

This warning can be traced back to asd_find_flash_de(). If it fails to
find a FLASH_DE_CTRL_A_USER entry it will return without setting 'offs'.
And that will leave 'offs' uninitialized when asd_read_flash_seg() is
called a little later.

But that call of asd_read_flash_seg() isn't needed in that case.
Everything this code cares about can already be found in the default
CTRL-A user settings section that was created in the error path. So
let's just jump over that call (and all other unneeded code) after
creating that section.

Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
0) I noticed this warning while building v3.6-rc7 on current Fedora
17, using Fedora's default config.

1) Compile tested only. It might be best to run test this too, if only
to test whether the non-error path is unaffected.

2) This piece of code has not been touched ever since it was added in
v2.6.19, with commit 2908d778ab3e244900c310974e1fc1c69066e450 ([SCSI]
aic94xx: new driver). So, given the current code, chances are that a
CTRL-A user settings section is always present and the code to create
a default section might as well be dropped. So perhaps a more drastic
patch is preferable.

 drivers/scsi/aic94xx/aic94xx_sds.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/aic94xx/aic94xx_sds.c 
b/drivers/scsi/aic94xx/aic94xx_sds.c
index edb43fd..ecb4a1c 100644
--- a/drivers/scsi/aic94xx/aic94xx_sds.c
+++ b/drivers/scsi/aic94xx/aic94xx_sds.c
@@ -983,7 +983,7 @@ static int asd_process_ctrl_a_user(struct asd_ha_struct 
*asd_ha,
 {
int err, i;
u32 offs, size;
-   struct asd_ll_el *el;
+   struct asd_ll_el *el = NULL;
struct asd_ctrla_phy_settings *ps;
struct asd_ctrla_phy_settings dflt_ps;
 
@@ -1004,6 +1004,7 @@ static int asd_process_ctrl_a_user(struct asd_ha_struct 
*asd_ha,
 
size = sizeof(struct asd_ctrla_phy_settings);
ps = dflt_ps;
+   goto ctrla_phy_settings;
}
 
if (size == 0)
@@ -1029,6 +1030,7 @@ static int asd_process_ctrl_a_user(struct asd_ha_struct 
*asd_ha,
goto out2;
}
 
+ctrla_phy_settings:
err = asd_process_ctrla_phy_settings(asd_ha, ps);
if (err) {
ASD_DPRINTK(couldn't process ctrla phy settings\n);
-- 
1.7.11.4

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

[PATCH] aic7xxx_old: silence GCC warnings

[Paul Bolle]

Building the aic7xxx_old driver triggers these GCC warnings:
drivers/scsi/aic7xxx_old.c:7901:5: warning: case value '257' not in 
enumerated type 'ahc_chip' [-Wswitch]
drivers/scsi/aic7xxx_old.c:7898:5: warning: case value '513' not in 
enumerated type 'ahc_chip' [-Wswitch]
drivers/scsi/aic7xxx_old.c:8517:5: warning: case value '257' not in 
enumerated type 'ahc_chip' [-Wswitch]
drivers/scsi/aic7xxx_old.c:8510:5: warning: case value '513' not in 
enumerated type 'ahc_chip' [-Wswitch]

Fix these warnings by adopting the idiom used elsewhere in this driver.
Since AHC_EISA and AHC_VL are only ever set for AHC_AIC7770 this fix
should not lead to any functional change.

Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
0) I noticed these warnings while building v3.6-rc6 on current Fedora
17, using Fedora's default config.

1) Compile tested only.

2) This patch is not checkpatch clean. But this file actually triggers
thousands of checkpatch errors and warnings:
total: 4779 errors, 7528 warnings, 11149 lines checked

So I didn't bother to fix the few errors and warnings this patch
triggers by sticking to this file (lack of) coding style.

 drivers/scsi/aic7xxx_old.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/aic7xxx_old.c b/drivers/scsi/aic7xxx_old.c
index 5b212f0..1a381c8 100644
--- a/drivers/scsi/aic7xxx_old.c
+++ b/drivers/scsi/aic7xxx_old.c
@@ -7893,12 +7893,12 @@ aic7xxx_register(struct scsi_host_template *template, 
struct aic7xxx_host *p,
 
   printk(KERN_INFO (scsi%d) %s found at , p-host_no,
 board_names[p-board_name_index]);
-  switch(p-chip)
+  switch(p-chip  ~AHC_CHIPID_MASK)
   {
-case (AHC_AIC7770|AHC_EISA):
+case AHC_EISA:
   printk(EISA slot %d\n, p-pci_device_fn);
   break;
-case (AHC_AIC7770|AHC_VL):
+case AHC_VL:
   printk(VLB slot %d\n, p-pci_device_fn);
   break;
 default:
@@ -8505,16 +8505,16 @@ aic7xxx_load_seeprom(struct aic7xxx_host *p, unsigned 
char *sxfrctl1)
   {
 printk(KERN_INFO aic7xxx: Loading serial EEPROM...);
   }
-  switch (p-chip)
+  switch (p-chip  ~AHC_CHIPID_MASK)
   {
-case (AHC_AIC7770|AHC_EISA):  /* None of these adapters have seeproms. */
+case AHC_EISA:  /* None of these adapters have seeproms. */
   if (aic_inb(p, SCSICONF)  TERM_ENB)
 p-flags |= AHC_TERM_ENB_A;
   if ( (p-features  AHC_TWIN)  (aic_inb(p, SCSICONF + 1)  TERM_ENB) )
 p-flags |= AHC_TERM_ENB_B;
   break;
 
-case (AHC_AIC7770|AHC_VL):
+case AHC_VL:
   have_seeprom = read_284x_seeprom(p, (struct seeprom_config *) scarray);
   break;
 
-- 
1.7.11.4

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

[PATCH] mvsas: use correct named constants

[Paul Bolle]

Building the mvsas driver triggers these GCC warnings:
drivers/scsi/mvsas/mv_sas.c:1156:34: warning: comparison between 'enum 
sas_device_type' and 'enum sas_dev_type' [-Wenum-compare]
drivers/scsi/mvsas/mv_sas.c:1159:39: warning: comparison between 'enum 
sas_device_type' and 'enum sas_dev_type' [-Wenum-compare]

Silence these warnings by using the named constants from enum
sas_device_type with the same value as the currently used named
constants.

Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
0) I noticed these warnings while building v3.6-rc6 on current Fedora
17, using Fedora's default config.

1) Compile tested only.

 drivers/scsi/mvsas/mv_sas.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
index 4539d59..f663d9a 100644
--- a/drivers/scsi/mvsas/mv_sas.c
+++ b/drivers/scsi/mvsas/mv_sas.c
@@ -1153,10 +1153,10 @@ void mvs_update_phyinfo(struct mvs_info *mvi, int i, 
int get_st)
phy-identify.device_type =
phy-att_dev_info  PORT_DEV_TYPE_MASK;
 
-   if (phy-identify.device_type == SAS_END_DEV)
+   if (phy-identify.device_type == SAS_END_DEVICE)
phy-identify.target_port_protocols =
SAS_PROTOCOL_SSP;
-   else if (phy-identify.device_type != NO_DEVICE)
+   else if (phy-identify.device_type != SAS_PHY_UNUSED)
phy-identify.target_port_protocols =
SAS_PROTOCOL_SMP;
if (oob_done)
-- 
1.7.11.4

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