Re: [PATCH] mpt2sas: Fall back to 64 bit coherent mask if 64 bit DMA / 32 bit coherent mask not supported
On Wednesday 13 May 2015 08:23:41 Brian King wrote: > On 05/13/2015 03:10 AM, Arnd Bergmann wrote: > > On Tuesday 12 May 2015 18:24:43 Brian King wrote: > >> > >> Commit 5fb1bf8aaa832e1e9ca3198de7bbecb8eff7db9c broke 64 bit DMA for > >> mpt2sas on Power. > >> That commit changed the sequence for setting up the DMA and coherent DMA > >> masks so > >> that during initialization the driver requests a 64 bit DMA mask and a 32 > >> bit consistent > >> DMA mask, then later requests a 64 bit consistent DMA mask. The Power > >> architecture does > >> not currently support this, which results in always falling back to a 32 > >> bit DMA window, > >> which has a negative impact on performance. Tweak this algorithm slightly > >> so that > >> if requesting a 32 bit consistent mask fails after we've successfully set > >> a 64 bit > >> DMA mask, just try to get a 64 bit consistent mask. This should preserve > >> existing > >> behavior on platforms that support mixed mask setting and restore previous > >> functionality > >> to those that do not. > >> > >> Signed-off-by: Brian King > > > > I believe the way the API is designed, it should guarantee that after > > dma_set_mask() > > succeeds for a device, dma_set_coherent_mask() with the same mask will also > > succeed. > > > > Could you just call dma_set_mask_and_coherent() here to avoid that complex > > logic? > > I don't think that would work. The mpt2sas driver wants to set the dma mask > to 64bits > but set the coherent mask to 32 bits, then my change is to set the coherent > mask to > 64bits if setting it to 32bit fails. I'm not seeing how using > dma_set_mask_and_coherent > would simplify anything here. My question was more about why the driver would ask for a 32-bit coherent mask to start with. Is this a limitation in the mpt2sas device that can only have descriptors at low addresses, or is it trying to work around some bug in a particular host system? Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [RESEND] SCSI: initio: remove duplicate module device table
The initio driver has for many years had two copies of the
same module device table. One of them is also used for registering
the other driver, the other one is entirely useless after the
large scale cleanup that Alan Cox did back in 2007.
The compiler warns about this whenever the driver is built-in:
drivers/scsi/initio.c:131:29: warning: 'i91u_pci_devices' defined but not used
[-Wunused-variable]
This removes the extraneous table and the warning.
Signed-off-by: Arnd Bergmann
Fixes: 72d39fea901 ("[SCSI] initio: Convert into a real Linux driver and update
to modern style")
---
Submitted in January, but got no reply.
diff --git a/drivers/scsi/initio.c b/drivers/scsi/initio.c
index e5dae7b54d9a..51063177f18e 100644
--- a/drivers/scsi/initio.c
+++ b/drivers/scsi/initio.c
@@ -110,11 +110,6 @@
#define i91u_MAXQUEUE 2
#define i91u_REVID "Initio INI-9X00U/UW SCSI device driver; Revision: 1.04a"
-#define I950_DEVICE_ID 0x9500 /* Initio's inic-950 product ID */
-#define I940_DEVICE_ID 0x9400 /* Initio's inic-940 product ID */
-#define I935_DEVICE_ID 0x9401 /* Initio's inic-935 product ID */
-#define I920_DEVICE_ID 0x0002 /* Initio's other product ID */
-
#ifdef DEBUG_i91u
static unsigned int i91u_debug = DEBUG_DEFAULT;
#endif
@@ -127,17 +122,6 @@ static int setup_debug = 0;
static void i91uSCBPost(u8 * pHcb, u8 * pScb);
-/* PCI Devices supported by this driver */
-static struct pci_device_id i91u_pci_devices[] = {
- { PCI_VENDOR_ID_INIT, I950_DEVICE_ID, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
- { PCI_VENDOR_ID_INIT, I940_DEVICE_ID, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
- { PCI_VENDOR_ID_INIT, I935_DEVICE_ID, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
- { PCI_VENDOR_ID_INIT, I920_DEVICE_ID, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
- { PCI_VENDOR_ID_DOMEX, I920_DEVICE_ID, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
- { }
-};
-MODULE_DEVICE_TABLE(pci, i91u_pci_devices);
-
#define DEBUG_INTERRUPT 0
#define DEBUG_QUEUE 0
#define DEBUG_STATE 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: debug: fix type mismatch warning for sg_pcopy_from_buffer
On Wednesday 20 May 2015 12:53:29 Andrew Morton wrote: > On Tue, 19 May 2015 23:22:39 +0200 Arnd Bergmann wrote: > > > > I can't decide if this is actually a good idea, or if we should rather drop > > the sg_pcopy_from_buffer() patch. Maybe someone else sees a better solution. > > Could make do_device_access() call sg_copy_buffer() directly. > > But yes, dropping the sg_pcopy_from/to_buffer changes is reasonable. > sg_copy_buffer() is bidirectional and that won't be changing, so > putting constified wrapeprs around it is kinda fake. Ok. The part I only saw now is that do_device_access() is the only user of sg_pcopy_from_buffer(), so if that passes a non-const argument, there is dropping the patch will be teh best solution. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: debug: fix type mismatch warning for sg_pcopy_from_buffer
On Thursday 21 May 2015 12:09:58 Dave Gordon wrote:
> From b304c5a99ea260eac1cf98ced5f3c79c793ad4fd Mon Sep 17 00:00:00 2001
> From: Dave Gordon
> Date: Thu, 21 May 2015 12:06:27 +0100
> Subject: [PATCH] scsi: resolve sg buffer const-ness issue
>
> do_device_access() takes a separate parameter to indicate the direction
> of data transfer, which it used to use to select the appropriate function
> out of sg_pcopy_{to,from}_buffer(). However these two functions now have
> different const-ness in their signatures, leading to compiler warnings.
>
> So this patch makes it bypass these wrappers and call the underlying
> function sg_copy_buffer() directly; this has the same calling style as
> do_device_access() i.e. a separate direction-of-transfer parameter and
> no pointers-to-const, so skipping the wrappers not only eliminates the
> warning, it also make the code simpler
>
> Signed-off-by: Dave Gordon
> ---
Acked-by: Arnd Bergmann
--
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: NCR5380: no longer mark irq probing as __init
The g_NCR5380 has been converted to more regular probing, which
means its probe function can now be invoked after the __init section
is discarded, as pointed out by this kbuild warning:
WARNING: drivers/scsi/built-in.o(.text+0x3a105): Section mismatch in reference
from the function generic_NCR5380_isa_match() to the function
.init.text:probe_intr()
WARNING: drivers/scsi/built-in.o(.text+0x3a145): Section mismatch in reference
from the function generic_NCR5380_isa_match() to the variable
.init.data:probe_irq
To make sure this works correctly in all cases, let's remove
the __init and __initdata annotations.
Fixes: a8cfbcaec0c1 ("scsi: g_NCR5380: Stop using scsi_module.c")
Signed-off-by: Arnd Bergmann
---
drivers/scsi/NCR5380.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c
index 7053de5bd468..61f34aca2fa0 100644
--- a/drivers/scsi/NCR5380.c
+++ b/drivers/scsi/NCR5380.c
@@ -353,7 +353,7 @@ static void NCR5380_print_phase(struct Scsi_Host *instance)
#endif
-static int probe_irq __initdata;
+static int probe_irq;
/**
* probe_intr - helper for IRQ autoprobe
@@ -365,7 +365,7 @@ static int probe_irq __initdata;
* used by the IRQ probe code.
*/
-static irqreturn_t __init probe_intr(int irq, void *dev_id)
+static irqreturn_t probe_intr(int irq, void *dev_id)
{
probe_irq = irq;
return IRQ_HANDLED;
@@ -380,7 +380,7 @@ static irqreturn_t __init probe_intr(int irq, void *dev_id)
* and then looking to see what interrupt actually turned up.
*/
-static int __init __maybe_unused NCR5380_probe_irq(struct Scsi_Host *instance,
+static int __maybe_unused NCR5380_probe_irq(struct Scsi_Host *instance,
int possible)
{
struct NCR5380_hostdata *hostdata = shost_priv(instance);
--
2.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] lpfc: use %zd format string for size_t
A recent bugfix introduced a harmless warning in the lpfc driver:
drivers/scsi/lpfc/lpfc_init.c: In function 'lpfc_write_firmware':
drivers/scsi/lpfc/lpfc_logmsg.h:56:45: error: format '%ld' expects argument of
type 'long int', but argument 9 has type 'size_t {aka const unsigned int}'
[-Werror=format=]
'size_t' is always the same width as 'long' in the kernel, but the compiler
doesn't know that. The %z modifier is what the standard expects to be
used here, and this shuts up the warning.
Fixes: 679053c651fb ("scsi: lpfc: Fix fw download on SLI-4 FC adapters")
Signed-off-by: Arnd Bergmann
---
drivers/scsi/lpfc/lpfc_init.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index 7be9b8a7bb19..4776fd85514f 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -10332,7 +10332,7 @@ lpfc_write_firmware(const struct firmware *fw, void
*context)
ftype != LPFC_FILE_TYPE_GROUP || fsize != fw->size) {
lpfc_printf_log(phba, KERN_ERR, LOG_INIT,
"3022 Invalid FW image found. "
- "Magic:%x Type:%x ID:%x Size %d %ld\n",
+ "Magic:%x Type:%x ID:%x Size %d %zd\n",
magic_number, ftype, fid, fsize, fw->size);
rc = -EINVAL;
goto release_out;
--
2.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: g_NCR5380: add HAS_IOPORT_MAP dependency
The driver was changed to call ioport_map, which breaks platforms that
cannot provide this function:
drivers/scsi/g_NCR5380.o: In function `generic_NCR5380_init_one.constprop.0':
g_NCR5380.c:(.text.generic_NCR5380_init_one.constprop.0+0x388): undefined
reference to `ioport_map'
This adds a Kconfig dependency.
Fixes: 04c40f82ccc5 ("scsi: g_NCR5380: Merge g_NCR5380 and g_NCR5380_mmio
drivers")
Signed-off-by: Arnd Bergmann
---
drivers/scsi/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 98451fe031a4..3b416c9efe5e 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -744,7 +744,7 @@ config SCSI_ISCI
config SCSI_GENERIC_NCR5380
tristate "Generic NCR5380/53c400 SCSI ISA card support"
- depends on ISA && SCSI
+ depends on ISA && SCSI && HAS_IOPORT_MAP
select SCSI_SPI_ATTRS
---help---
This is a driver for old ISA card SCSI controllers based on a
--
2.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 1/2] scsi: smartpqi: Replace semaphore sync_request_sem with mutex
On Thursday, October 20, 2016 2:24:01 PM CEST Binoy Jayan wrote: > Semaphores are going away in the future, so replace the semaphore > sync_request_sem with the a mutex lock. timeout_msecs is not used > for the lock sync_request_sem, so remove the timed locking too. > > Signed-off-by: Binoy Jayan The patch looks correct to me, but I think if you remove the support for handling timeouts, you should update the prototype of pqi_submit_raid_request_synchronous to no longer pass the timeout argument in the first place. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] scsi: smartpqi: Replace semaphore lun_reset_sem with mutex
On Thursday, October 20, 2016 2:24:02 PM CEST Binoy Jayan wrote: > Semaphores are going away in the future, so replace the semaphore > lun_reset_sem with the a mutex lock. > > Signed-off-by: Binoy Jayan > Reviewed-by: Arnd Bergmann -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] scsi: smartpqi: Replace semaphore sync_request_sem with mutex
On Thursday, October 20, 2016 2:24:01 PM CEST Binoy Jayan wrote: > - sema_init(&ctrl_info->sync_request_sem, > - PQI_RESERVED_IO_SLOTS_SYNCHRONOUS_REQUESTS); > + mutex_init(&ctrl_info->sync_request_mutex); > Looking at this again, I see that PQI_RESERVED_IO_SLOTS_SYNCHRONOUS_REQUESTS is '3', so this is in fact a counting semaphore rather than a mutex, and the conversion is changing the behavior. The patch can't go in unless you either show that it should be a normal mutex rather than a counting semaphore, or you find a way to keep the behavior the same. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] sd: fix uninitialized variable access in error handling
If sd_zbc_report_zones fails, the check for 'zone_blocks == 0'
later in the function accesses uninitialized data:
drivers/scsi/sd_zbc.c: In function ‘sd_zbc_read_zones’:
drivers/scsi/sd_zbc.c:520:7: error: ‘zone_blocks’ may be used uninitialized in
this function [-Werror=maybe-uninitialized]
This sets it to zero, which has the desired effect of leaving
the sd_zbc_read_zones successfully with sdkp->zone_blocks = 0.
Fixes: 89d947561077 ("sd: Implement support for ZBC devices")
Signed-off-by: Arnd Bergmann
---
drivers/scsi/sd_zbc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 16d3fa62d8ac..d5b3bd915d9e 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -455,8 +455,10 @@ static int sd_zbc_check_zone_size(struct scsi_disk *sdkp)
/* Do a report zone to get the same field */
ret = sd_zbc_report_zones(sdkp, buf, SD_ZBC_BUF_SIZE, 0);
- if (ret)
+ if (ret) {
+ zone_blocks = 0;
goto out;
+ }
same = buf[4] & 0x0f;
if (same > 0) {
--
2.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 1/2] scsi: smartpqi: Replace semaphore sync_request_sem with mutex
On Monday, October 24, 2016 3:34:27 PM CEST Binoy Jayan wrote: > Hi Arnd > > On 20 October 2016 at 14:36, Arnd Bergmann wrote: > > On Thursday, October 20, 2016 2:24:01 PM CEST Binoy Jayan wrote: > >> Semaphores are going away in the future, so replace the semaphore > >> sync_request_sem with the a mutex lock. timeout_msecs is not used > >> for the lock sync_request_sem, so remove the timed locking too. > >> > >> Signed-off-by: Binoy Jayan > > > > The patch looks correct to me, but I think if you remove the support > > for handling timeouts, you should update the prototype of > > pqi_submit_raid_request_synchronous to no longer pass the timeout > > argument in the first place. > > But we still need "timeout_msecs" in a call to > pqi_submit_raid_request_synchronous_with_io_request() > > drivers/scsi/smartpqi/smartpqi_init.c +3484 Why? If it's always zero, we can remove that too. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] advansys: fix build warning for PCI=n
The advansys probe function tries to handle both ISA and PCI cases, each hidden in an #ifdef when unused. This leads to a warning indicating that when PCI is disabled we could be using uninitialized data: drivers/scsi/advansys.c: In function ‘advansys_board_found’: drivers/scsi/advansys.c:11036:5: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized] drivers/scsi/advansys.c:10928:28: note: ‘ret’ was declared here drivers/scsi/advansys.c:11309:8: error: ‘share_irq’ may be used uninitialized in this function [-Werror=maybe-uninitialized] drivers/scsi/advansys.c:10928:6: note: ‘share_irq’ was declared here This cannot happen in practice because the hardware in question only exists for PCI, but changing the code to just error out here is better for consistency and avoids the warning. Signed-off-by: Arnd Bergmann --- drivers/scsi/advansys.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c index febbd83e2ecd..81dd0927246b 100644 --- a/drivers/scsi/advansys.c +++ b/drivers/scsi/advansys.c @@ -11030,6 +11030,9 @@ static int advansys_board_found(struct Scsi_Host *shost, unsigned int iop, ASC_DBG(2, "AdvInitGetConfig()\n"); ret = AdvInitGetConfig(pdev, shost) ? -ENODEV : 0; +#else + share_irq = 0; + ret = -ENODEV; #endif /* CONFIG_PCI */ } -- 2.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] mvsas: fix command_active typo
gcc-7 notices that the condition in mvs_94xx_command_active looks
suspicious:
drivers/scsi/mvsas/mv_94xx.c: In function 'mvs_94xx_command_active':
drivers/scsi/mvsas/mv_94xx.c:671:15: error: '<<' in boolean context, did you
mean '<' ? [-Werror=int-in-bool-context]
This was introduced when the mv_printk() statement got added,
and leads to the condition being ignored. This is probably harmless.
Changing '&&' to '&' makes the code look reasonable, as we check the
command bit before setting and printing it.
Fixes: a4632aae8b66 ("[SCSI] mvsas: Add new macros and functions")
Signed-off-by: Arnd Bergmann
---
drivers/scsi/mvsas/mv_94xx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/mvsas/mv_94xx.c b/drivers/scsi/mvsas/mv_94xx.c
index 4c57d9abce7b..7de5d8d75480 100644
--- a/drivers/scsi/mvsas/mv_94xx.c
+++ b/drivers/scsi/mvsas/mv_94xx.c
@@ -668,7 +668,7 @@ static void mvs_94xx_command_active(struct mvs_info *mvi,
u32 slot_idx)
{
u32 tmp;
tmp = mvs_cr32(mvi, MVS_COMMAND_ACTIVE+(slot_idx >> 3));
- if (tmp && 1 << (slot_idx % 32)) {
+ if (tmp & 1 << (slot_idx % 32)) {
mv_printk("command active %08X, slot [%x].\n", tmp, slot_idx);
mvs_cw32(mvi, MVS_COMMAND_ACTIVE + (slot_idx >> 3),
1 << (slot_idx % 32));
--
2.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] bfa: turn bfa_mem_{kva,dma}_setup into inline functions
These two macros cause lots of warnings with gcc-7:
drivers/scsi/bfa/bfa_svc.c: In function 'bfa_fcxp_meminfo':
drivers/scsi/bfa/bfa_svc.c:521:103: error: '*' in boolean context, suggest '&&'
instead [-Werror=int-in-bool-context]
Using inline functions makes them much more readable and avoids
the warnings.
Signed-off-by: Arnd Bergmann
---
drivers/scsi/bfa/bfa_ioc.h | 30 +-
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/drivers/scsi/bfa/bfa_ioc.h b/drivers/scsi/bfa/bfa_ioc.h
index 523fb02109b6..c27ed80f12ab 100644
--- a/drivers/scsi/bfa/bfa_ioc.h
+++ b/drivers/scsi/bfa/bfa_ioc.h
@@ -111,20 +111,24 @@ struct bfa_meminfo_s {
struct bfa_mem_kva_s kva_info;
};
-/* BFA memory segment setup macros */
-#define bfa_mem_dma_setup(_meminfo, _dm_ptr, _seg_sz) do { \
- ((bfa_mem_dma_t *)(_dm_ptr))->mem_len = (_seg_sz); \
- if (_seg_sz)\
- list_add_tail(&((bfa_mem_dma_t *)_dm_ptr)->qe, \
- &(_meminfo)->dma_info.qe);\
-} while (0)
+/* BFA memory segment setup helpers */
+static inline void bfa_mem_dma_setup(struct bfa_meminfo_s *meminfo,
+struct bfa_mem_dma_s *dm_ptr,
+size_t seg_sz)
+{
+ dm_ptr->mem_len = seg_sz;
+ if (seg_sz)
+ list_add_tail(&dm_ptr->qe, &meminfo->dma_info.qe);
+}
-#define bfa_mem_kva_setup(_meminfo, _kva_ptr, _seg_sz) do {\
- ((bfa_mem_kva_t *)(_kva_ptr))->mem_len = (_seg_sz); \
- if (_seg_sz)\
- list_add_tail(&((bfa_mem_kva_t *)_kva_ptr)->qe, \
- &(_meminfo)->kva_info.qe);\
-} while (0)
+static inline void bfa_mem_kva_setup(struct bfa_meminfo_s *meminfo,
+struct bfa_mem_kva_s *kva_ptr,
+size_t seg_sz)
+{
+ kva_ptr->mem_len = seg_sz;
+ if (seg_sz)
+ list_add_tail(&kva_ptr->qe, &meminfo->kva_info.qe);
+}
/* BFA dma memory segments iterator */
#define bfa_mem_dma_sptr(_mod, _i) (&(_mod)->dma_seg[(_i)])
--
2.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] libfc: fix seconds_since_last_reset miscalculation
On Tuesday, November 15, 2016 4:05:31 PM CET Johannes Thumshirn wrote:
> On Tue, Nov 15, 2016 at 02:50:17PM +, Bart Van Assche wrote:
> > On Tue, 2016-11-15 at 10:18 +0100, Johannes Thumshirn wrote:
> > > On Tue, Nov 08, 2016 at 03:04:43PM +, Bart Van Assche wrote:
> > > > I think the above code will miscalculate seconds_since_last_reset
> > > > if
> > > > 'jiffies' wraps around after an lport has been created and before
> > > > seconds_since_last_reset is computed. Shouldn't
> > > > seconds_since_last_reset
> > > > be computed as follows?
> > > >
> > > > fc_stats->seconds_since_last_reset = (jiffies - boot_time) /
> > > > HZ;
> > >
> > > But what happens when jiffies - boot_time becomes negative? Then we
> > > reintroduce the bug again and have 'fcoeadm -s' show weird values.
> >
> > Hello Johannes,
> >
> > If your concern is about 'jiffies' wrapping around on 32-bit systems
> > then you should use get_jiffies_64(). get_jiffies_64() - boot_time
> > can't become negative. It namely takes several million years before a
> > 64-bit HZ counter wraps around.
>
> You're right. I'll respin using get_jiffies_64() and resent once it is tested.
Sorry for the bug I introduced and for not noticing this thread earlier.
Looking at this again now, I think it's clear that the bug was simply
mixing up the left and right side of the subtraction, the simple fix
would be
diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c
index 2d3133f62463..fe643f2195f0 100644
--- a/drivers/scsi/libfc/fc_lport.c
+++ b/drivers/scsi/libfc/fc_lport.c
@@ -311,7 +311,7 @@ struct fc_host_statistics *fc_get_host_stats(struct
Scsi_Host *shost)
fc_stats = &lport->host_stats;
memset(fc_stats, 0, sizeof(struct fc_host_statistics));
- fc_stats->seconds_since_last_reset = (lport->boot_time - jiffies) / HZ;
+ fc_stats->seconds_since_last_reset = (jiffies - lport->boot_time) / HZ;
for_each_possible_cpu(cpu) {
struct fc_stats *stats;
This works correctly across jiffies overflows, as long as there is at least one
reset for every jiffies overflow (49 days or more). If we can have longer times
between resets, then we could either use get_jiffies_64() or
ktime_get_seconds().
The latter would only need a 32-bit variable (overflow is after 136 years).
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] libfc: fix seconds_since_last_reset miscalculation
On Thursday, November 17, 2016 1:39:27 PM CET Johannes Thumshirn wrote:
> Commit 540eb1eef 'scsi: libfc: fix seconds_since_last_reset calculation'
> removed the use of 'struct timespec' from fc_get_host_stats(). This broke the
> output of 'fcoeadm -s' after kernel 4.8-rc1.
>
> Fixes: 540eb1eef ('scsi: libfc: fix seconds_since_last_reset calculation')
> Signed-off-by: Johannes Thumshirn
>
Acked-by: Arnd Bergmann
I actually made the same patch today and was going to send it out
after getting my other patches to build cleanly, but you were faster.
Are we worried about 32-bit systems overflowing here when the elapsed
time since the last reset is over 49 days? If we are, we probably
want another patch on top, though this one is sufficient to fix
the regression, and it has no problems on 64-bit machines.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] isci: avoid array subscript warning
I'm getting a new warning with gcc-7:
isci/remote_node_context.c: In function 'sci_remote_node_context_destruct':
isci/remote_node_context.c:69:16: error: array subscript is above array bounds
[-Werror=array-bounds]
This is odd, since we clearly cover all values for enum
scis_sds_remote_node_context_states here. Anyway, checking
for an array overflow can't harm and it makes the warning
go away.
Signed-off-by: Arnd Bergmann
---
drivers/scsi/isci/remote_node_context.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/scsi/isci/remote_node_context.c
b/drivers/scsi/isci/remote_node_context.c
index 30bd80052e03..e3f2a5359d71 100644
--- a/drivers/scsi/isci/remote_node_context.c
+++ b/drivers/scsi/isci/remote_node_context.c
@@ -66,6 +66,9 @@ const char *rnc_state_name(enum
scis_sds_remote_node_context_states state)
{
static const char * const strings[] = RNC_STATES;
+ if (state >= ARRAY_SIZE(strings))
+ return "UNKNOWN";
+
return strings[state];
}
#undef C
--
2.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: hpsa: fix uninitialized variable access
A bugfix has left the 'sd' variable uninitialized:
drivers/scsi/hpsa.c: In function 'hpsa_slave_alloc':
drivers/scsi/hpsa.c:2033:5: error: 'sd' may be used uninitialized in this
function [-Werror=maybe-uninitialized]
This reverts back to calling lookup_hpsa_scsi_dev() for the
HPSA_PHYSICAL_DEVICE_BUS case, but also keeps doing that when
hpsa_find_device_by_sas_rphy() returns NULL, as is currently
done.
The patch that caused this is marked for stable backports,
so this one has to be backported on top as well.
Fixes: 4eb307f7b18d ("scsi: hpsa: use bus '3' for legacy HBA devices")
Signed-off-by: Arnd Bergmann
---
I did not try hard to figure out what the correct behavior
should be, so please treat this as a bugreport that might contain
the right fix.
---
drivers/scsi/hpsa.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index ea64c01f3d42..d17ee63045c3 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -2029,7 +2029,10 @@ static int hpsa_slave_alloc(struct scsi_device *sdev)
sd->target = sdev_id(sdev);
sd->lun = sdev->lun;
}
+ } else {
+ sd = NULL;
}
+
if (!sd)
sd = lookup_hpsa_scsi_dev(h, sdev_channel(sdev),
sdev_id(sdev), sdev->lun);
--
2.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: hpsa: fix uninitialized variable access
On Tuesday, November 22, 2016 3:47:09 PM CET Hannes Reinecke wrote:
> index 05f7782..ee6f852 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -2031,7 +2031,7 @@ static struct hpsa_scsi_dev_t
> *lookup_hpsa_scsi_dev(struct ctlr_info *h,
>
> static int hpsa_slave_alloc(struct scsi_device *sdev)
> {
> - struct hpsa_scsi_dev_t *sd;
> + struct hpsa_scsi_dev_t *sd = NULL;
> unsigned long flags;
> struct ctlr_info *h;
>
>
I try not to add initializations like this in general, since they
prevent us from finding the bug, but here that seems fine too
as we immediately test it for NULL anyway.
Can you follow up with a patch to do that?
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hpsa: fix uninitialized variable
On Tuesday, November 22, 2016 4:05:38 PM CET Hannes Reinecke wrote:
> With the recent patch to hpsa this warning is issued:
>
> drivers/scsi/hpsa.c: In function 'hpsa_slave_alloc':
> drivers/scsi/hpsa.c:2033:5: error: 'sd' may be used uninitialized in this
> function [-Werror=maybe-uninitialized]
>
> The patch that caused this is marked for stable backports,
> so this one has to be backported on top as well.
>
> Fixes: 4eb307f7b18d ("scsi: hpsa: use bus '3' for legacy HBA devices")
>
> Signed-off-by: Hannes Reinecke
>
Reported-by: Arnd Bergmann
Acked-by: Arnd Bergmann
Thanks!
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] scsi: qedi: select UIO
The newly added qedi driver links against the UIO framework, but can
be built without that:
drivers/scsi/qedi/qedi_main.o: In function `qedi_free_uio':
qedi_main.c:(.text.qedi_free_uio+0x78): undefined reference to
`uio_unregister_device'
drivers/scsi/qedi/qedi_main.o: In function `qedi_ll2_recv_thread':
qedi_main.c:(.text.qedi_ll2_recv_thread+0x18c): undefined reference to
`uio_event_notify'
drivers/scsi/qedi/qedi_main.o: In function `__qedi_probe.constprop.1':
qedi_main.c:(.text.__qedi_probe.constprop.1+0x1368): undefined reference to
`__uio_register_device'
This adds a compile-time dependency.
Fixes: ace7f46ba5fd ("scsi: qedi: Add QLogic FastLinQ offload iSCSI driver
framework.")
Signed-off-by: Arnd Bergmann
---
drivers/scsi/qedi/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/scsi/qedi/Kconfig b/drivers/scsi/qedi/Kconfig
index 23ca8a274586..913610f3d274 100644
--- a/drivers/scsi/qedi/Kconfig
+++ b/drivers/scsi/qedi/Kconfig
@@ -2,6 +2,7 @@ config QEDI
tristate "QLogic QEDI 25/40/100Gb iSCSI Initiator Driver Support"
depends on PCI && SCSI
depends on QED
+ depends on UIO
select SCSI_ISCSI_ATTRS
select QED_LL2
select QED_ISCSI
--
2.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: aacraid: avoid open-coded upper_32_bits
Shifting a dma_addr_t right by 32 bits causes a compile-time warning when
that type is only 32 bit wide:
drivers/scsi/aacraid/src.c: In function 'aac_src_start_adapter':
drivers/scsi/aacraid/src.c:414:29: error: right shift count >= width of type
[-Werror=shift-count-overflow]
This changes the driver to use the predefined macros consistently, including
one correct but open-coded upper_32_bits() instance.
Fixes: d1ef4da8487f ("scsi: aacraid: added support for init_struct_8")
Fixes: 423400e64d37 ("scsi: aacraid: Include HBA direct interface")
Signed-off-by: Arnd Bergmann
---
drivers/scsi/aacraid/src.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/aacraid/src.c b/drivers/scsi/aacraid/src.c
index 46976a3b6952..8e4e2ddbafd7 100644
--- a/drivers/scsi/aacraid/src.c
+++ b/drivers/scsi/aacraid/src.c
@@ -410,8 +410,8 @@ static void aac_src_start_adapter(struct aac_dev *dev)
if (dev->comm_interface == AAC_COMM_MESSAGE_TYPE3) {
init->r8.host_elapsed_seconds = cpu_to_le32(get_seconds());
src_sync_cmd(dev, INIT_STRUCT_BASE_ADDRESS,
- (u32)(ulong)dev->init_pa,
- (u32)((ulong)dev->init_pa>>32),
+ lower_32_bits(dev->init_pa),
+ upper_32_bits(dev->init_pa),
sizeof(struct _r8) +
(AAC_MAX_HRRQ - 1) * sizeof(struct _rrq),
0, 0, 0, NULL, NULL, NULL, NULL, NULL);
@@ -563,7 +563,7 @@ static int aac_src_deliver_message(struct fib *fib)
fib->hw_fib_va->header.SenderFibAddress =
cpu_to_le32((u32)address);
fib->hw_fib_va->header.u.TimeStamp = 0;
- WARN_ON(((u32)(((address) >> 16) >> 16)) != 0L);
+ WARN_ON(upper_32_bits(address) != 0L);
} else {
/* Calculate the amount to the fibsize bits */
fibsize = (sizeof(struct aac_fib_xporthdr) +
--
2.9.0
[PATCH] scsi: megaraid_sas: handle dma_addr_t right on 32-bit
When building with a dma_addr_t that is different from pointer size, we get
this warning:
drivers/scsi/megaraid/megaraid_sas_fusion.c: In function
'megasas_make_prp_nvme':
drivers/scsi/megaraid/megaraid_sas_fusion.c:1654:17: error: cast to pointer
from integer of different size [-Werror=int-to-pointer-cast]
It's better to not pretend that the dma address is a pointer and instead
use a dma_addr_t consistently.
Fixes: 33203bc4d61b ("scsi: megaraid_sas: NVME fast path io support")
Signed-off-by: Arnd Bergmann
---
drivers/scsi/megaraid/megaraid_sas_fusion.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c
b/drivers/scsi/megaraid/megaraid_sas_fusion.c
index 750090119f81..29650ba669da 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
@@ -1619,7 +1619,8 @@ megasas_make_prp_nvme(struct megasas_instance *instance,
struct scsi_cmnd *scmd,
{
int sge_len, offset, num_prp_in_chain = 0;
struct MPI25_IEEE_SGE_CHAIN64 *main_chain_element, *ptr_first_sgl;
- u64 *ptr_sgl, *ptr_sgl_phys;
+ u64 *ptr_sgl;
+ dma_addr_t ptr_sgl_phys;
u64 sge_addr;
u32 page_mask, page_mask_result;
struct scatterlist *sg_scmd;
@@ -1651,14 +1652,14 @@ megasas_make_prp_nvme(struct megasas_instance
*instance, struct scsi_cmnd *scmd,
*/
page_mask = mr_nvme_pg_size - 1;
ptr_sgl = (u64 *)cmd->sg_frame;
- ptr_sgl_phys = (u64 *)cmd->sg_frame_phys_addr;
+ ptr_sgl_phys = cmd->sg_frame_phys_addr;
memset(ptr_sgl, 0, instance->max_chain_frame_sz);
/* Build chain frame element which holds all prps except first*/
main_chain_element = (struct MPI25_IEEE_SGE_CHAIN64 *)
((u8 *)sgl_ptr + sizeof(struct MPI25_IEEE_SGE_CHAIN64));
- main_chain_element->Address = cpu_to_le64((uintptr_t)ptr_sgl_phys);
+ main_chain_element->Address = cpu_to_le64(ptr_sgl_phys);
main_chain_element->NextChainOffset = 0;
main_chain_element->Flags = IEEE_SGE_FLAGS_CHAIN_ELEMENT |
IEEE_SGE_FLAGS_SYSTEM_ADDR |
@@ -1696,16 +1697,15 @@ megasas_make_prp_nvme(struct megasas_instance
*instance, struct scsi_cmnd *scmd,
scmd_printk(KERN_NOTICE,
scmd, "page boundary ptr_sgl: 0x%p\n",
ptr_sgl);
- ptr_sgl_phys++;
- *ptr_sgl =
- cpu_to_le64((uintptr_t)ptr_sgl_phys);
+ ptr_sgl_phys += 8;
+ *ptr_sgl = cpu_to_le64(ptr_sgl_phys);
ptr_sgl++;
num_prp_in_chain++;
}
*ptr_sgl = cpu_to_le64(sge_addr);
ptr_sgl++;
- ptr_sgl_phys++;
+ ptr_sgl_phys += 8;
num_prp_in_chain++;
sge_addr += mr_nvme_pg_size;
--
2.9.0
[PATCH] smartpqi: fix time handling
When we have turned off RTC support, the smartpqi driver fails to build:
ERROR: "rtc_time64_to_tm" [drivers/scsi/smartpqi/smartpqi.ko] undefined!
This is easily avoided by using the generic 'struct tm' based helper rather
than the RTC specific one. While fixing this, I noticed that even though
the driver uses time64_t for storing seconds, it gets them from the
old 32-bit struct timeval. To address this, we can simplify the code
by calling ktime_get_real_seconds() directly.
Fixes: 6c223761eb54 ("smartpqi: initial commit of Microsemi smartpqi driver")
Signed-off-by: Arnd Bergmann
---
drivers/scsi/smartpqi/smartpqi_init.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/smartpqi/smartpqi_init.c
b/drivers/scsi/smartpqi/smartpqi_init.c
index 11c0dfb3dfa3..657ad15682a3 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -534,8 +534,7 @@ static int pqi_write_current_time_to_host_wellness(
size_t buffer_length;
time64_t local_time;
unsigned int year;
- struct timeval time;
- struct rtc_time tm;
+ struct tm tm;
buffer_length = sizeof(*buffer);
@@ -552,9 +551,8 @@ static int pqi_write_current_time_to_host_wellness(
put_unaligned_le16(sizeof(buffer->time),
&buffer->time_length);
- do_gettimeofday(&time);
- local_time = time.tv_sec - (sys_tz.tz_minuteswest * 60);
- rtc_time64_to_tm(local_time, &tm);
+ local_time = ktime_get_real_seconds();
+ time64_to_tm(local_time, -sys_tz.tz_minuteswest * 60, &tm);
year = tm.tm_year + 1900;
buffer->time[0] = bin2bcd(tm.tm_hour);
--
2.9.0
[PATCH] scsi: lpfc: use proper format string for dma_addr_t
dma_addr_t may be either u32 or u64, depending on the kernel configuration,
and we get a warning for the 32-bit case:
drivers/scsi/lpfc/lpfc_nvme.c: In function 'lpfc_nvme_ls_req':
drivers/scsi/lpfc/lpfc_logmsg.h:52:52: error: format '%llu' expects argument of
type 'long long unsigned int', but argument 11 has type 'dma_addr_t {aka
unsigned int}' [-Werror=format=]
drivers/scsi/lpfc/lpfc_logmsg.h:52:52: error: format '%llu' expects argument of
type 'long long unsigned int', but argument 12 has type 'dma_addr_t {aka
unsigned int}' [-Werror=format=]
drivers/scsi/lpfc/lpfc_nvme.c: In function 'lpfc_nvme_ls_abort':
drivers/scsi/lpfc/lpfc_logmsg.h:52:52: error: format '%llu' expects argument of
type 'long long unsigned int', but argument 11 has type 'dma_addr_t {aka
unsigned int}' [-Werror=format=]
drivers/scsi/lpfc/lpfc_logmsg.h:52:52: error: format '%llu' expects argument of
type 'long long unsigned int', but argument 12 has type 'dma_addr_t {aka
unsigned int}' [-Werror=format=]
printk has a special "%pad" format string that passes the dma address by
reference to solve this problem.
Fixes: 01649561a8b4 ("scsi: lpfc: NVME Initiator: bind to nvme_fc api")
Signed-off-by: Arnd Bergmann
---
drivers/scsi/lpfc/lpfc_nvme.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_nvme.c b/drivers/scsi/lpfc/lpfc_nvme.c
index 625b6589a34d..609a908ea9db 100644
--- a/drivers/scsi/lpfc/lpfc_nvme.c
+++ b/drivers/scsi/lpfc/lpfc_nvme.c
@@ -457,11 +457,11 @@ lpfc_nvme_ls_req(struct nvme_fc_local_port *pnvme_lport,
/* Expand print to include key fields. */
lpfc_printf_vlog(vport, KERN_INFO, LOG_NVME_DISC,
"6051 ENTER. lport %p, rport %p lsreq%p rqstlen:%d "
-"rsplen:%d %llux %llux\n",
+"rsplen:%d %pad %pad\n",
pnvme_lport, pnvme_rport,
pnvme_lsreq, pnvme_lsreq->rqstlen,
-pnvme_lsreq->rsplen, pnvme_lsreq->rqstdma,
-pnvme_lsreq->rspdma);
+pnvme_lsreq->rsplen, &pnvme_lsreq->rqstdma,
+&pnvme_lsreq->rspdma);
vport->phba->fc4NvmeLsRequests++;
@@ -527,11 +527,11 @@ lpfc_nvme_ls_abort(struct nvme_fc_local_port *pnvme_lport,
/* Expand print to include key fields. */
lpfc_printf_vlog(vport, KERN_INFO, LOG_NVME_ABTS,
"6040 ENTER. lport %p, rport %p lsreq %p rqstlen:%d "
-"rsplen:%d %llux %llux\n",
+"rsplen:%d %pad %pad\n",
pnvme_lport, pnvme_rport,
pnvme_lsreq, pnvme_lsreq->rqstlen,
-pnvme_lsreq->rsplen, pnvme_lsreq->rqstdma,
-pnvme_lsreq->rspdma);
+pnvme_lsreq->rsplen, &pnvme_lsreq->rqstdma,
+&pnvme_lsreq->rspdma);
/*
* Lock the ELS ring txcmplq and build a local list of all ELS IOs
--
2.9.0
[PATCH] scsi: lpfc: use div_u64 for 64-bit division
The new debugfs output causes a link error on 32-bit architectures:
ERROR: "__aeabi_uldivmod" [drivers/scsi/lpfc/lpfc.ko] undefined!
This code is not performance critical, so we can simply use div_u64().
Fixes: bd2cdd5e400f ("scsi: lpfc: NVME Initiator: Add debugfs support")
Fixes: 2b65e18202fd ("scsi: lpfc: NVME Target: Add debugfs support")
Signed-off-by: Arnd Bergmann
---
drivers/scsi/lpfc/lpfc_debugfs.c | 64
1 file changed, 32 insertions(+), 32 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_debugfs.c b/drivers/scsi/lpfc/lpfc_debugfs.c
index 599fde4ea8b1..47c67bf0514e 100644
--- a/drivers/scsi/lpfc/lpfc_debugfs.c
+++ b/drivers/scsi/lpfc/lpfc_debugfs.c
@@ -873,8 +873,8 @@ lpfc_debugfs_nvmektime_data(struct lpfc_vport *vport, char
*buf, int size)
len += snprintf(
buf + len, PAGE_SIZE - len,
"avg:%08lld min:%08lld max %08lld\n",
- phba->ktime_seg1_total /
- phba->ktime_data_samples,
+ div_u64(phba->ktime_seg1_total,
+ phba->ktime_data_samples),
phba->ktime_seg1_min,
phba->ktime_seg1_max);
len += snprintf(
@@ -884,8 +884,8 @@ lpfc_debugfs_nvmektime_data(struct lpfc_vport *vport, char
*buf, int size)
len += snprintf(
buf + len, PAGE_SIZE - len,
"avg:%08lld min:%08lld max %08lld\n",
- phba->ktime_seg2_total /
- phba->ktime_data_samples,
+ div_u64(phba->ktime_seg2_total,
+ phba->ktime_data_samples),
phba->ktime_seg2_min,
phba->ktime_seg2_max);
len += snprintf(
@@ -895,8 +895,8 @@ lpfc_debugfs_nvmektime_data(struct lpfc_vport *vport, char
*buf, int size)
len += snprintf(
buf + len, PAGE_SIZE - len,
"avg:%08lld min:%08lld max %08lld\n",
- phba->ktime_seg3_total /
- phba->ktime_data_samples,
+ div_u64(phba->ktime_seg3_total,
+ phba->ktime_data_samples),
phba->ktime_seg3_min,
phba->ktime_seg3_max);
len += snprintf(
@@ -906,17 +906,17 @@ lpfc_debugfs_nvmektime_data(struct lpfc_vport *vport,
char *buf, int size)
len += snprintf(
buf + len, PAGE_SIZE - len,
"avg:%08lld min:%08lld max %08lld\n",
- phba->ktime_seg4_total /
- phba->ktime_data_samples,
+ div_u64(phba->ktime_seg4_total,
+ phba->ktime_data_samples),
phba->ktime_seg4_min,
phba->ktime_seg4_max);
len += snprintf(
buf + len, PAGE_SIZE - len,
"Total IO avg time: %08lld\n",
- ((phba->ktime_seg1_total +
+ div_u64(phba->ktime_seg1_total +
phba->ktime_seg2_total +
phba->ktime_seg3_total +
- phba->ktime_seg4_total) /
+ phba->ktime_seg4_total,
phba->ktime_data_samples));
return len;
}
@@ -935,8 +935,8 @@ lpfc_debugfs_nvmektime_data(struct lpfc_vport *vport, char
*buf, int size)
"cmd pass to NVME Layer\n");
len += snprintf(buf + len, PAGE_SIZE-len,
"avg:%08lld min:%08lld max %08lld\n",
- phba->ktime_seg1_total /
- phba->ktime_data_samples,
+ div_u64(phba->ktime_seg1_total,
+ phba->ktime_data_samples),
phba->ktime_seg1_min,
phba->ktime_seg1_max);
len += snprintf(buf + len, PAGE_SIZE-len,
@@ -944,8 +944,8 @@ lpfc_debugfs_nvmektime_data(struct lpfc_vport *vport, char
*buf, int size)
"-to- Driver rcv cmd OP (action)\n");
len += snprintf(buf + len, PAGE_SIZE-len,
"avg:%08lld min:%08lld max %08lld\n",
- phba->ktime_seg2_total /
- phba->ktime_data_samples,
+ div_u64(phba->ktime_seg2_total,
+ phba->ktime_data_samples),
phba->ktime_seg2_min,
phba->ktime_seg2_max);
[PATCH] scsi: qedi: fix build error without DEBUG_FS
Without CONFIG_DEBUG_FS, we run into a link error:
drivers/scsi/qedi/qedi_iscsi.o: In function `qedi_ep_poll':
qedi_iscsi.c:(.text.qedi_ep_poll+0x134): undefined reference to `do_not_recover'
drivers/scsi/qedi/qedi_iscsi.o: In function `qedi_ep_disconnect':
qedi_iscsi.c:(.text.qedi_ep_disconnect+0x36c): undefined reference to
`do_not_recover'
drivers/scsi/qedi/qedi_iscsi.o: In function `qedi_ep_connect':
qedi_iscsi.c:(.text.qedi_ep_connect+0x350): undefined reference to
`do_not_recover'
drivers/scsi/qedi/qedi_fw.o: In function `qedi_tmf_work':
qedi_fw.c:(.text.qedi_tmf_work+0x3b4): undefined reference to `do_not_recover'
This defines the symbol as a constant in this case, as there is no way to
set it to anything other than zero without DEBUG_FS. In addition, I'm renaming
it to qedi_do_not_recover in order to put it into a driver specific namespace,
as "do_not_recover" is a really bad name for a kernel-wide global identifier
when it is used only in one driver.
Fixes: ace7f46ba5fd ("scsi: qedi: Add QLogic FastLinQ offload iSCSI driver
framework.")
Signed-off-by: Arnd Bergmann
---
drivers/scsi/qedi/qedi_debugfs.c | 22 +++---
drivers/scsi/qedi/qedi_fw.c | 4 ++--
drivers/scsi/qedi/qedi_gbl.h | 8 +++-
drivers/scsi/qedi/qedi_iscsi.c | 8
4 files changed, 24 insertions(+), 18 deletions(-)
diff --git a/drivers/scsi/qedi/qedi_debugfs.c b/drivers/scsi/qedi/qedi_debugfs.c
index 955936274241..6baf35eefc1c 100644
--- a/drivers/scsi/qedi/qedi_debugfs.c
+++ b/drivers/scsi/qedi/qedi_debugfs.c
@@ -14,7 +14,7 @@
#include
#include
-int do_not_recover;
+int qedi_do_not_recover;
static struct dentry *qedi_dbg_root;
void
@@ -74,22 +74,22 @@ qedi_dbg_exit(void)
static ssize_t
qedi_dbg_do_not_recover_enable(struct qedi_dbg_ctx *qedi_dbg)
{
- if (!do_not_recover)
- do_not_recover = 1;
+ if (!qedi_do_not_recover)
+ qedi_do_not_recover = 1;
- QEDI_INFO(qedi_dbg, QEDI_LOG_DEBUGFS, "do_not_recover=%d\n",
- do_not_recover);
+ QEDI_INFO(qedi_dbg, QEDI_LOG_DEBUGFS, "qedi_do_not_recover=%d\n",
+ qedi_do_not_recover);
return 0;
}
static ssize_t
qedi_dbg_do_not_recover_disable(struct qedi_dbg_ctx *qedi_dbg)
{
- if (do_not_recover)
- do_not_recover = 0;
+ if (qedi_do_not_recover)
+ qedi_do_not_recover = 0;
- QEDI_INFO(qedi_dbg, QEDI_LOG_DEBUGFS, "do_not_recover=%d\n",
- do_not_recover);
+ QEDI_INFO(qedi_dbg, QEDI_LOG_DEBUGFS, "qedi_do_not_recover=%d\n",
+ qedi_do_not_recover);
return 0;
}
@@ -101,7 +101,7 @@ static struct qedi_list_of_funcs
qedi_dbg_do_not_recover_ops[] = {
struct qedi_debugfs_ops qedi_debugfs_ops[] = {
{ "gbl_ctx", NULL },
- { "do_not_recover", qedi_dbg_do_not_recover_ops},
+ { "qedi_do_not_recover", qedi_dbg_do_not_recover_ops},
{ "io_trace", NULL },
{ NULL, NULL }
};
@@ -141,7 +141,7 @@ qedi_dbg_do_not_recover_cmd_read(struct file *filp, char
__user *buffer,
if (*ppos)
return 0;
- cnt = sprintf(buffer, "do_not_recover=%d\n", do_not_recover);
+ cnt = sprintf(buffer, "qedi_do_not_recover=%d\n", qedi_do_not_recover);
cnt = min_t(int, count, cnt - *ppos);
*ppos += cnt;
return cnt;
diff --git a/drivers/scsi/qedi/qedi_fw.c b/drivers/scsi/qedi/qedi_fw.c
index c9f0ef4e11b3..2bce3efc66a4 100644
--- a/drivers/scsi/qedi/qedi_fw.c
+++ b/drivers/scsi/qedi/qedi_fw.c
@@ -1461,9 +1461,9 @@ static void qedi_tmf_work(struct work_struct *work)
get_itt(tmf_hdr->rtt), get_itt(ctask->itt), cmd->task_id,
qedi_conn->iscsi_conn_id);
- if (do_not_recover) {
+ if (qedi_do_not_recover) {
QEDI_ERR(&qedi->dbg_ctx, "DONT SEND CLEANUP/ABORT %d\n",
-do_not_recover);
+qedi_do_not_recover);
goto abort_ret;
}
diff --git a/drivers/scsi/qedi/qedi_gbl.h b/drivers/scsi/qedi/qedi_gbl.h
index 8e488de88ece..63d793f46064 100644
--- a/drivers/scsi/qedi/qedi_gbl.h
+++ b/drivers/scsi/qedi/qedi_gbl.h
@@ -12,8 +12,14 @@
#include "qedi_iscsi.h"
+#ifdef CONFIG_DEBUG_FS
+extern int qedi_do_not_recover;
+#else
+#define qedi_do_not_recover (0)
+#endif
+
extern uint qedi_io_tracing;
-extern int do_not_recover;
+
extern struct scsi_host_template qedi_host_template;
extern struct iscsi_transport qedi_iscsi_transport;
extern const struct qed_iscsi_ops *qedi_ops;
diff --git a/drivers/scsi/qedi/qedi_iscsi.c b/drivers/scsi/qedi/qedi_iscsi.c
index b9f79d36142d..4cc474364c50 100644
--- a/drivers/scsi/qedi/qedi_iscsi.c
+++ b/drivers/scsi/qedi/qedi_iscsi.c
@@ -833,7 +833,7 @
[PATCH] [v2] scsi: qedi: fix build error without DEBUG_FS
Without CONFIG_DEBUG_FS, we run into a link error:
drivers/scsi/qedi/qedi_iscsi.o: In function `qedi_ep_poll':
qedi_iscsi.c:(.text.qedi_ep_poll+0x134): undefined reference to `do_not_recover'
drivers/scsi/qedi/qedi_iscsi.o: In function `qedi_ep_disconnect':
qedi_iscsi.c:(.text.qedi_ep_disconnect+0x36c): undefined reference to
`do_not_recover'
drivers/scsi/qedi/qedi_iscsi.o: In function `qedi_ep_connect':
qedi_iscsi.c:(.text.qedi_ep_connect+0x350): undefined reference to
`do_not_recover'
drivers/scsi/qedi/qedi_fw.o: In function `qedi_tmf_work':
qedi_fw.c:(.text.qedi_tmf_work+0x3b4): undefined reference to `do_not_recover'
This defines the symbol as a constant in this case, as there is no way to
set it to anything other than zero without DEBUG_FS. In addition, I'm renaming
it to qedi_do_not_recover in order to put it into a driver specific namespace,
as "do_not_recover" is a really bad name for a kernel-wide global identifier
when it is used only in one driver.
Fixes: ace7f46ba5fd ("scsi: qedi: Add QLogic FastLinQ offload iSCSI driver
framework.")
Reviewed-by: Johannes Thumshirn
Signed-off-by: Arnd Bergmann
---
v2: don't rename references to do_not_recover in string literals
---
drivers/scsi/qedi/qedi_debugfs.c | 16
drivers/scsi/qedi/qedi_fw.c | 4 ++--
drivers/scsi/qedi/qedi_gbl.h | 8 +++-
drivers/scsi/qedi/qedi_iscsi.c | 8
4 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/drivers/scsi/qedi/qedi_debugfs.c b/drivers/scsi/qedi/qedi_debugfs.c
index 955936274241..59417199bf36 100644
--- a/drivers/scsi/qedi/qedi_debugfs.c
+++ b/drivers/scsi/qedi/qedi_debugfs.c
@@ -14,7 +14,7 @@
#include
#include
-int do_not_recover;
+int qedi_do_not_recover;
static struct dentry *qedi_dbg_root;
void
@@ -74,22 +74,22 @@ qedi_dbg_exit(void)
static ssize_t
qedi_dbg_do_not_recover_enable(struct qedi_dbg_ctx *qedi_dbg)
{
- if (!do_not_recover)
- do_not_recover = 1;
+ if (!qedi_do_not_recover)
+ qedi_do_not_recover = 1;
QEDI_INFO(qedi_dbg, QEDI_LOG_DEBUGFS, "do_not_recover=%d\n",
- do_not_recover);
+ qedi_do_not_recover);
return 0;
}
static ssize_t
qedi_dbg_do_not_recover_disable(struct qedi_dbg_ctx *qedi_dbg)
{
- if (do_not_recover)
- do_not_recover = 0;
+ if (qedi_do_not_recover)
+ qedi_do_not_recover = 0;
QEDI_INFO(qedi_dbg, QEDI_LOG_DEBUGFS, "do_not_recover=%d\n",
- do_not_recover);
+ qedi_do_not_recover);
return 0;
}
@@ -141,7 +141,7 @@ qedi_dbg_do_not_recover_cmd_read(struct file *filp, char
__user *buffer,
if (*ppos)
return 0;
- cnt = sprintf(buffer, "do_not_recover=%d\n", do_not_recover);
+ cnt = sprintf(buffer, "do_not_recover=%d\n", qedi_do_not_recover);
cnt = min_t(int, count, cnt - *ppos);
*ppos += cnt;
return cnt;
diff --git a/drivers/scsi/qedi/qedi_fw.c b/drivers/scsi/qedi/qedi_fw.c
index c9f0ef4e11b3..2bce3efc66a4 100644
--- a/drivers/scsi/qedi/qedi_fw.c
+++ b/drivers/scsi/qedi/qedi_fw.c
@@ -1461,9 +1461,9 @@ static void qedi_tmf_work(struct work_struct *work)
get_itt(tmf_hdr->rtt), get_itt(ctask->itt), cmd->task_id,
qedi_conn->iscsi_conn_id);
- if (do_not_recover) {
+ if (qedi_do_not_recover) {
QEDI_ERR(&qedi->dbg_ctx, "DONT SEND CLEANUP/ABORT %d\n",
-do_not_recover);
+qedi_do_not_recover);
goto abort_ret;
}
diff --git a/drivers/scsi/qedi/qedi_gbl.h b/drivers/scsi/qedi/qedi_gbl.h
index 8e488de88ece..63d793f46064 100644
--- a/drivers/scsi/qedi/qedi_gbl.h
+++ b/drivers/scsi/qedi/qedi_gbl.h
@@ -12,8 +12,14 @@
#include "qedi_iscsi.h"
+#ifdef CONFIG_DEBUG_FS
+extern int qedi_do_not_recover;
+#else
+#define qedi_do_not_recover (0)
+#endif
+
extern uint qedi_io_tracing;
-extern int do_not_recover;
+
extern struct scsi_host_template qedi_host_template;
extern struct iscsi_transport qedi_iscsi_transport;
extern const struct qed_iscsi_ops *qedi_ops;
diff --git a/drivers/scsi/qedi/qedi_iscsi.c b/drivers/scsi/qedi/qedi_iscsi.c
index b9f79d36142d..4cc474364c50 100644
--- a/drivers/scsi/qedi/qedi_iscsi.c
+++ b/drivers/scsi/qedi/qedi_iscsi.c
@@ -833,7 +833,7 @@ qedi_ep_connect(struct Scsi_Host *shost, struct sockaddr
*dst_addr,
return ERR_PTR(ret);
}
- if (do_not_recover) {
+ if (qedi_do_not_recover) {
ret = -ENOMEM;
return ERR_PTR(ret);
}
@@ -957,7 +957,7 @@ static int qedi_ep_poll(struct iscsi_endpoint *ep, int
timeout_ms)
struct qedi_endpoint *qedi_ep;
int ret = 0;
- if (do_not_recover)
+ if (qedi_
Re: [PATCH] scsi: qedi: fix build error without DEBUG_FS
On Thu, Mar 2, 2017 at 1:10 PM, Arnd Bergmann wrote:
>
> - QEDI_INFO(qedi_dbg, QEDI_LOG_DEBUGFS, "do_not_recover=%d\n",
> - do_not_recover);
> + QEDI_INFO(qedi_dbg, QEDI_LOG_DEBUGFS, "qedi_do_not_recover=%d\n",
> + qedi_do_not_recover);
> - QEDI_INFO(qedi_dbg, QEDI_LOG_DEBUGFS, "do_not_recover=%d\n",
> - do_not_recover);
> + QEDI_INFO(qedi_dbg, QEDI_LOG_DEBUGFS, "qedi_do_not_recover=%d\n",
> + qedi_do_not_recover);
> return 0;
> }
> - { "do_not_recover", qedi_dbg_do_not_recover_ops},
> + { "qedi_do_not_recover", qedi_dbg_do_not_recover_ops},
> { "io_trace", NULL },
> { NULL, NULL }
>
> - cnt = sprintf(buffer, "do_not_recover=%d\n", do_not_recover);
> + cnt = sprintf(buffer, "qedi_do_not_recover=%d\n",
> qedi_do_not_recover);
> cnt = min_t(int, count, cnt - *ppos);
I just noticed that my search&replace went a little too far, I should not have
touched the instances within strings. Will resend.
Arnd
Re: [PATCH v4 2/4] arm64: Remove the clock and PHY reference from the APM X-Gene SoC AHCI SATA Host controller dts node.
On Tuesday 29 July 2014 12:24:50 Suman Tripathi wrote: > This patch removes all clocks and PHY references from the APM X-Gene > SoC AHCI SATA host controller and PHY DTS nodes. The clock and PHY > are no longer needed as they are handled by the firmware. By removing > only the reference is not enough as any un-used clock entry will get > disabled by the clock framework. > > Signed-off-by: Loc Ho > Signed-off-by: Suman Tripathi > This will break booting on older firmware versions I guess, and it does not confirm to the binding that lists the phy and clock properties as "required". It doesn't sound like a good idea to apply it. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 2/2] arm64: Remove the clock and PHY reference from the APM X-Gene SoC AHCI SATA Host controller dts node.
On Friday 08 August 2014, Suman Tripathi wrote: > This patch removes all clocks and PHY references from the APM X-Gene > SoC AHCI SATA host controller and PHY DTS nodes. The clock and PHY > are no longer needed as they are handled by the firmware. By removing > only the reference is not enough as any un-used clock entry will get > disabled by the clock framework. This patch also updates the APM X-Gene > SOC AHCI SATA Host controller clock and PHY bindings as optional > properties. > > Signed-off-by: Loc Ho > Signed-off-by: Suman Tripathi > --- > .../devicetree/bindings/ata/apm-xgene.txt | 10 +-- > arch/arm64/boot/dts/apm-storm.dtsi | 93 > -- > 2 files changed, 5 insertions(+), 98 deletions(-) > What is the upgrade path here? It sounds like this will still break new kernels running on old firmware if you also upgrade the dtb file. You write in the introductory email that this will work with old firmware as well, but I don't understand how. How about modifying the dtb at boot time by the firmware according to the requirements of the firmware itself? Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: ips.c: use 64-bit time types
On Wednesday 08 October 2014 23:14:08 Ebru Akagunduz wrote: > This patch changes 32-bit time types to 64-bit in > ips.c > > time_t can only represent signed 32-bit dates but > the driver should represent dates that are after > January 2038. > > Use time64_t type instead of time_t. > > Signed-off-by: Ebru Akagunduz > Hi Ebru, I think you missed the location in which ffdc_time is initially assigned, and wher it gets compared to the current time: struct timeval tv; do_gettimeofday(&tv); ha->last_ffdc = tv.tv_sec; This needs to be changed to timespec64 to actually avoid the overflow problem. I think this one should also use monotonic time, i.e. ktime_get_ts64 rather than getnstimeofday64. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: ips.c: use 64-bit time types
On Wednesday 08 October 2014 13:44:55 James Bottomley wrote:
> > diff --git a/drivers/scsi/ips.h b/drivers/scsi/ips.h
> > index 45b9566..ff2a0b3 100644
> > --- a/drivers/scsi/ips.h
> > +++ b/drivers/scsi/ips.h
> > @@ -1054,7 +1054,7 @@ typedef struct ips_ha {
> > uint8_tactive;
> > intioctl_reset;/* IOCTL Requested Reset Flag */
> > uint16_t reset_count;/* number of resets */
> > - time_t last_ffdc; /* last time we sent ffdc info*/
> > + time64_t last_ffdc; /* last time we sent ffdc
> > info*/
> > uint8_tslot_num; /* PCI Slot Number*/
> > intioctl_len; /* size of ioctl buffer */
> > dma_addr_t ioctl_busaddr; /* dma address of ioctl buffer*/
>
> This is completely pointless, isn't it? All the ips driver cares about
> is that we send a FFDC time update every eight hours or so, so we can
> happily truncate the number of seconds to 32 bits for that calculation
> just keep the variable at 32 bits and do a time_after thing for the
> comparison.
Good point. The same has come up in a few other places, so I wonder if we
should introduce a proper way to do it that doesn't involve time_t.
While the current code works, we will have to audit 2000 other locations
in which time_t/timespec/timeval are used in the kernel, so we are going
to need some form of annotation to make sure we don't get everyone to
look at the driver again just to come to the same conclusion after working
on a patch first.
> However, what the code *should* be doing is using jiffies and
> time_before/after since the interval is so tiny rather than a
> do_gettimeofday() call in the fast path.
Yes, this would probably be best for this particular driver, it also
means we end up with a monotonic clock source rather than a wall-clock.
Ebru, when I explained the various data types we have for timekeeping,
I failed to mention jiffies. That is one that is very fast to access
and has a resolution between 1 and 10 milliseconds but will overflow
within a few months, so it can only be used in places where overflow
is known to be handled safely, as time_before() does.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: ips.c: use 64-bit time types
On Thursday 09 October 2014 06:40:26 James Bottomley wrote:
> On Wed, 2014-10-08 at 22:58 +0200, Arnd Bergmann wrote:
> > On Wednesday 08 October 2014 13:44:55 James Bottomley wrote:
> > > > diff --git a/drivers/scsi/ips.h b/drivers/scsi/ips.h
> > > > index 45b9566..ff2a0b3 100644
> > > > --- a/drivers/scsi/ips.h
> > > > +++ b/drivers/scsi/ips.h
> > > > @@ -1054,7 +1054,7 @@ typedef struct ips_ha {
> > > > uint8_tactive;
> > > > intioctl_reset;/* IOCTL Requested Reset
> > > > Flag */
> > > > uint16_t reset_count;/* number of resets
> > > > */
> > > > - time_t last_ffdc; /* last time we sent ffdc
> > > > info*/
> > > > + time64_t last_ffdc; /* last time we sent ffdc
> > > > info*/
> > > > uint8_tslot_num; /* PCI Slot Number
> > > > */
> > > > intioctl_len; /* size of ioctl buffer
> > > > */
> > > > dma_addr_t ioctl_busaddr; /* dma address of ioctl
> > > > buffer*/
> > >
> > > This is completely pointless, isn't it? All the ips driver cares about
> > > is that we send a FFDC time update every eight hours or so, so we can
> > > happily truncate the number of seconds to 32 bits for that calculation
> > > just keep the variable at 32 bits and do a time_after thing for the
> > > comparison.
> >
> > Good point. The same has come up in a few other places, so I wonder if we
> > should introduce a proper way to do it that doesn't involve time_t.
>
> We have, it's jiffies ... that's why I'm slightly non-plussed that this
> driver is using gettimeofday for something like this ... it was clearly
> a review failure when we put it in.
Actually there is more to it, as I just found upon reading the code
again (I had noticed it before when I first looked at the driver but
then forgotten about it):
ips_fix_ffdc_time() needs the correct current wall-clock time, no overflow
allowed, to stick the year/month/day/hour/minute/second value into
the ffdc command.
My comment to Ebru about ktime_get_ts64 for monotonic time was unfortunately
completely wrong, since that would break whatever timekeeping it is
in the hardware that wants the correct year/month/day/hour/minute/second
values.
> or are you thinking we need a time_t_time_before doing for time_t what
> we do for jiffies?
The part I'm interested in is getting rid of any mention of time_t,
timespec and timeval in the kernel by replacing each use with something
that is known to be y2038-safe. Using jiffies correctly would solve
a number of them, but is not sufficient for this driver because of the
ffdc command.
We could use jiffies to test whether we need to send ffdc but then
we still need to read the correct time.
> > While the current code works, we will have to audit 2000 other locations
> > in which time_t/timespec/timeval are used in the kernel, so we are going
> > to need some form of annotation to make sure we don't get everyone to
> > look at the driver again just to come to the same conclusion after working
> > on a patch first.
> >
> > > However, what the code *should* be doing is using jiffies and
> > > time_before/after since the interval is so tiny rather than a
> > > do_gettimeofday() call in the fast path.
> >
> > Yes, this would probably be best for this particular driver, it also
> > means we end up with a monotonic clock source rather than a wall-clock.
>
> Right, and it's a 32 bit read instead of a system call every time the
> thing dispatches a command ... to be honest the overhead of 64 bit
> arithmetic is peanuts to making a syscall in the fast path.
It's not a system call, all we need is a simple function call that reads
tk->xtime_sec. We can use get_seconds() today, but it returns an
'unsigned long', so that won't be enough on 32-bit architectures.
It's still slightly more expensive to do the function call and use a 64-bit
number on a 32-bit CPU, but it's not on the scale of doing a system call
here. You can probably judge best if it's worth the increase in complexity
to use jiffies for determining whether to send the update and then
use get_seconds64 (or similar) to read the wall-clock time, or whether
always using get_seconds64 would be good enough.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: ips.c: use 64-bit time types
On Thursday 09 October 2014 08:13:14 James Bottomley wrote:
> On Thu, 2014-10-09 at 16:29 +0200, Arnd Bergmann wrote:
> > On Thursday 09 October 2014 06:40:26 James Bottomley wrote:
> > > On Wed, 2014-10-08 at 22:58 +0200, Arnd Bergmann wrote:
> > > > On Wednesday 08 October 2014 13:44:55 James Bottomley wrote:
> > > > > > diff --git a/drivers/scsi/ips.h b/drivers/scsi/ips.h
> > > > > > index 45b9566..ff2a0b3 100644
> > > > > > --- a/drivers/scsi/ips.h
> > > > > > +++ b/drivers/scsi/ips.h
> > > > > > @@ -1054,7 +1054,7 @@ typedef struct ips_ha {
> > > > > > uint8_tactive;
> > > > > > intioctl_reset;/* IOCTL Requested Reset
> > > > > > Flag */
> > > > > > uint16_t reset_count;/* number of resets
> > > > > > */
> > > > > > - time_t last_ffdc; /* last time we sent
> > > > > > ffdc info*/
> > > > > > + time64_t last_ffdc; /* last time we sent
> > > > > > ffdc info*/
> > > > > > uint8_tslot_num; /* PCI Slot Number
> > > > > > */
> > > > > > intioctl_len; /* size of ioctl buffer
> > > > > > */
> > > > > > dma_addr_t ioctl_busaddr; /* dma address of ioctl
> > > > > > buffer*/
> > > > >
> > > > > This is completely pointless, isn't it? All the ips driver cares
> > > > > about
> > > > > is that we send a FFDC time update every eight hours or so, so we can
> > > > > happily truncate the number of seconds to 32 bits for that calculation
> > > > > just keep the variable at 32 bits and do a time_after thing for the
> > > > > comparison.
> > > >
> > > > Good point. The same has come up in a few other places, so I wonder if
> > > > we
> > > > should introduce a proper way to do it that doesn't involve time_t.
> > >
> > > We have, it's jiffies ... that's why I'm slightly non-plussed that this
> > > driver is using gettimeofday for something like this ... it was clearly
> > > a review failure when we put it in.
> >
> > Actually there is more to it, as I just found upon reading the code
> > again (I had noticed it before when I first looked at the driver but
> > then forgotten about it):
> >
> > ips_fix_ffdc_time() needs the correct current wall-clock time, no overflow
> > allowed, to stick the year/month/day/hour/minute/second value into
> > the ffdc command.
>
> true, but we could call do_gettimeofday() in the routine when we know
> we're sending it. And it only does this once every 8 hours. My
> complaint is the do_gettimeofday() sitting in the fast path to see if
> the eight hours since the last time we sent the ffdc timestamp have
> elapsed.
Ok, fair enough.
> Actually, isn't there a version of the syscall that does return what
> this firmware is looking for (the year, month, day, hour, seconds
> values)?
Maybe rtc_ktime_to_tm()?
We would need a time64_t version of that anyway.
> > It's still slightly more expensive to do the function call and use a 64-bit
> > number on a 32-bit CPU, but it's not on the scale of doing a system call
> > here. You can probably judge best if it's worth the increase in complexity
> > to use jiffies for determining whether to send the update and then
> > use get_seconds64 (or similar) to read the wall-clock time, or whether
> > always using get_seconds64 would be good enough.
>
> heh, well we need to correct ips_fix_ffdc_time() somehow. I think
> converting the trigger mechanism to jiffies makes sense because the
> interval is so small and we already have the jiffies code overflow safe.
Ok. Ebru, can you have a look at doing this? I guess we have two separate
issues now, you can do one of them first:
a) replacing the use of do_gettimeofday() from ips_next() with jiffies
comparison
b) fixing ips_fix_ffdc_time() to use 64-bit time, possibly using
rtc_ktime_to_tm(ktime_get_real()) in the process to simplify the
code.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] target/cxgbit: add INET dependency
The newly added cxgbit driver directly calls functions of the
IPv4 network stack, which fails if that is disabled:
ERROR: "ip_route_output_flow" [drivers/target/iscsi/cxgbit/cxgbit.ko] undefined!
ERROR: "__ip_dev_find" [drivers/target/iscsi/cxgbit/cxgbit.ko] undefined!
This adds a Kconfig dependency to ensure we can't enable this
driver without the complete network stack it needs.
Signed-off-by: Arnd Bergmann
Fixes: b7ca3321e114 ("cxgbit: add Kconfig and Makefile")
---
drivers/target/iscsi/cxgbit/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/target/iscsi/cxgbit/Kconfig
b/drivers/target/iscsi/cxgbit/Kconfig
index cf335b4dbb4e..ce6f5cca315d 100644
--- a/drivers/target/iscsi/cxgbit/Kconfig
+++ b/drivers/target/iscsi/cxgbit/Kconfig
@@ -1,6 +1,7 @@
config ISCSI_TARGET_CXGB4
tristate "Chelsio iSCSI target offload driver"
depends on ISCSI_TARGET && CHELSIO_T4
+ depends on INET
select CHELSIO_T4_UWIRE
---help---
To compile this driver as module, choose M here: the module
--
2.7.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] target/cxgbit: add INET dependency
On Monday 29 February 2016 21:28:28 Nicholas A. Bellinger wrote: > > diff --git a/drivers/target/iscsi/cxgbit/Kconfig > > b/drivers/target/iscsi/cxgbit/Kconfig > > index cf335b4dbb4e..ce6f5cca315d 100644 > > --- a/drivers/target/iscsi/cxgbit/Kconfig > > +++ b/drivers/target/iscsi/cxgbit/Kconfig > > @@ -1,6 +1,7 @@ > > config ISCSI_TARGET_CXGB4 > > tristate "Chelsio iSCSI target offload driver" > > depends on ISCSI_TARGET && CHELSIO_T4 > > + depends on INET > > select CHELSIO_T4_UWIRE > > ---help--- > > To compile this driver as module, choose M here: the module > > Odd, ISCSI_TARGET already depends on NET, but I guess that's not enough > for cxgbit.. ;) > The problem is that you can have CONFIG_NET=y but CONFIG_INET=n It would probably be reasonable to make the ISCSI_TARGET option depend on CONFIG_INET directly, which would solve the problem as well. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/14] scsi: mvumi: use __maybe_unused to hide pm functions
The mvumi scsi hides the references to its suspend/resume functions
in an #ifdef but does not hide the implementation the same way:
drivers/scsi/mvumi.c:2632:12: error: 'mvumi_suspend' defined but not used
[-Werror=unused-function]
drivers/scsi/mvumi.c:2651:12: error: 'mvumi_resume' defined but not used
[-Werror=unused-function]
This adds __maybe_unused annotations so the compiler knows
it can silently drop them instead of warning, while avoiding
the addition of another #ifdef.
Signed-off-by: Arnd Bergmann
---
drivers/scsi/mvumi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/mvumi.c b/drivers/scsi/mvumi.c
index 02360de6b7e0..39285070f3b5 100644
--- a/drivers/scsi/mvumi.c
+++ b/drivers/scsi/mvumi.c
@@ -2629,7 +2629,7 @@ static void mvumi_shutdown(struct pci_dev *pdev)
mvumi_flush_cache(mhba);
}
-static int mvumi_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused mvumi_suspend(struct pci_dev *pdev, pm_message_t
state)
{
struct mvumi_hba *mhba = NULL;
@@ -2648,7 +2648,7 @@ static int mvumi_suspend(struct pci_dev *pdev,
pm_message_t state)
return 0;
}
-static int mvumi_resume(struct pci_dev *pdev)
+static int __maybe_unused mvumi_resume(struct pci_dev *pdev)
{
int ret;
struct mvumi_hba *mhba = NULL;
--
2.7.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 00/14] drivers: use __maybe_unused to hide pm functions
I found many variations of the bug in these device drivers (and some USB drivers I already send patches for in a separate series). In each case, the power management operations structure conditionally references suspend/resume functions, but the functions are hidden in an incorrect #ifdef or not hidden at all. We could try to correct the #ifdefs, but it seems easier to just mark those functions as __maybe_unused, which has the same effect but provides better compile-time test coverage and (subjectively) looks a bit nicer. I have a patch series that avoids all warnings in ARM randconfig builds, and I have verified that all these patches fix a warning that is still present in today's linux-next, and that they do not introduce new warnings in any configuration I found. Note that all these drivers are ARM specific, so I assume that all portable drivers got fixed already when someone rand into the problem on x86. There are no dependencies between the patches, so I'd appreciate subsystem maintainers to put them directly into their git trees. Arnd Arnd Bergmann (14): pinctrl: at91: use __maybe_unused to hide pm functions irqchip: st: use __maybe_unused to hide st_irq_syscfg_resume power: ipaq-micro-battery: use __maybe_unused to hide pm functions power: pm2301-charger: use __maybe_unused to hide pm functions mfd: ipaq-micro: use __maybe_unused to hide pm functions dma: sirf: use __maybe_unused to hide pm functions hw_random: exynos: use __maybe_unused to hide pm functions scsi: mvumi: use __maybe_unused to hide pm functions amd-xgbe: use __maybe_unused to hide pm functions wireless: cw1200: use __maybe_unused to hide pm functions_ input: spear-keyboard: use __maybe_unused to hide pm functions keyboard: snvs-pwrkey: use __maybe_unused to hide pm functions [media] omap3isp: use IS_ENABLED() to hide pm functions ASoC: rockchip: use __maybe_unused to hide st_irq_syscfg_resume drivers/char/hw_random/exynos-rng.c | 10 -- drivers/dma/sirf-dma.c | 10 -- drivers/input/keyboard/snvs_pwrkey.c| 4 ++-- drivers/input/keyboard/spear-keyboard.c | 6 ++ drivers/irqchip/irq-st.c| 2 +- drivers/media/platform/omap3isp/isp.c | 13 + drivers/mfd/ipaq-micro.c| 2 +- drivers/net/ethernet/amd/xgbe/xgbe-main.c | 6 ++ drivers/net/wireless/st/cw1200/cw1200_spi.c | 9 ++--- drivers/net/wireless/st/cw1200/pm.h | 9 +++-- drivers/pinctrl/pinctrl-at91-pio4.c | 4 ++-- drivers/power/ipaq_micro_battery.c | 4 ++-- drivers/power/pm2301_charger.c | 22 ++ drivers/scsi/mvumi.c| 4 ++-- sound/soc/rockchip/rockchip_spdif.c | 4 ++-- 15 files changed, 40 insertions(+), 69 deletions(-) -- 2.7.0 Cc: herb...@gondor.apana.org.au Cc: k.kozlow...@samsung.com Cc: dan.j.willi...@intel.com Cc: vinod.k...@intel.com Cc: bao...@kernel.org Cc: dmitry.torok...@gmail.com Cc: t...@linutronix.de Cc: ja...@lakedaemon.net Cc: marc.zyng...@arm.com Cc: laurent.pinch...@ideasonboard.com Cc: mche...@osg.samsung.com Cc: lee.jo...@linaro.org Cc: kv...@codeaurora.org Cc: ludovic.desroc...@atmel.com Cc: linus.wall...@linaro.org Cc: s...@kernel.org Cc: dbarysh...@gmail.com Cc: jbottom...@odin.com Cc: martin.peter...@oracle.com Cc: broo...@kernel.org Cc: linux-cry...@vger.kernel.org Cc: linux-arm-ker...@lists.infradead.org Cc: linux-samsung-...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: dmaeng...@vger.kernel.org Cc: linux-in...@vger.kernel.org Cc: linux-me...@vger.kernel.org Cc: net...@vger.kernel.org Cc: linux-wirel...@vger.kernel.org Cc: linux-g...@vger.kernel.org Cc: linux...@vger.kernel.org Cc: linux-scsi@vger.kernel.org Cc: alsa-de...@alsa-project.org Cc: linux-rockc...@lists.infradead.org -- 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 v9 3/3] add support for DWC UFS Host Controller
On Wednesday 02 March 2016 16:46:47 Joao Pinto wrote: > On 2/19/2016 3:03 PM, Arnd Bergmann wrote: > > On Thursday 18 February 2016 17:20:27 Joao Pinto wrote: > >> diff --git a/Documentation/devicetree/bindings/ufs/ufs-dwc.txt > >> b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt > > Please for the last time (!) add a proper version number of the > > specific IP block to the compatible strings so you can identify > > what hardware you are talking to. > > > > You earlier confused the version number of the UFS standard with > > the version number of the controller, and that has been clarified > > now, but you really still need to use a version for the hardware > > as well. > > Ok, we can have a "snps,ufshcd-dwc-1.1" and a "snps,ufshcd-dwc-2.0". Agree? I have no idea what versions of the dwc hardware block exist, but I find it highly suspicious that the numbers happen to be the same as the UFS protocol numbers, so I'd say that is probably not the version of the IP block. There are a few things you could try to find out the actual version: * If you are able to contact the team that worked on the test chip, please ask them what hardware revision you have. * if you have some form of documentation of the hardware, look on the first few pages of the manual that describes the registers to see if the document has a revision history. * If you have access to the hardware design files, look at the file names. On https://www.synopsys.com/dw/ipdir.php?ds=ufs, I see a version "1.30a" listed, which follows the typical numbering scheme that your employer uses, a single digit followed by a dot and a two-digit number and then a letter. There is also a test chip with version 1.10a listed on the same page, and that follows the same numbering system. See if you can find a version number that fits into that scheme in the documents you have available. > >> +config SCSI_UFS_DWC_TC > >> + bool "Support for the Synopsys Test Chip" > >> + depends on SCSI_UFS_DWC_HOOKS && (SCSI_UFSHCD_PCI || SCSI_UFS_DWC_PLAT) > >> + ---help--- > >> +Synopsys Test Chip is a Phy for prototyping purposes. > >> +This selects the support for the Synopsys Test Chip. > >> + > >> +Select this if you have a Synopsys Test Chip. > >> +If unsure, say N. > >> + > >> +config SCSI_UFS_DWC_20BIT_RMMI > >> + bool "20-bit RMMI support" > >> + depends on SCSI_UFS_DWC_TC > >> + ---help--- > >> +This specifies that the Synopsys Test Chip supports 20-bit RMMI. > >> + > >> +Select this if you are using a 20-bit RMMI Synopsys Test Chip. > >> +If unsure, say N. > >> + > >> +config SCSI_UFS_DWC_40BIT_RMMI > >> + bool "40-bit RMMI support" > >> + depends on SCSI_UFS_DWC_TC > >> + ---help--- > >> +Synopsys Test Chip is a Phy for prototyping purposes. > >> + > >> +Select this if you are using a 40-bit RMMI Synopsys Test Chip. > >> +If unsure, say N. > > > > I think it would be better to remove the SCSI_UFS_DWC_20BIT_RMMI > > and SCSI_UFS_DWC_40BIT_RMMI configuration options now, and always > > support both. There is not really much need for the options > > as this is just a test chip, and nobody is going to care much > > about saving a few bytes of object code. > > We need to know if we are dealing with a 20-bit test chip or a 40-bit test > chip > because the initialization is different. That can be made in the device tree > as > you say bellow, but we can also use a setup in which the host is a PC (so no > device tree) connected by pci bus to an fpga containing the UFS controller. > Having this, I think that the only way is to choose the 20/40bit stuff in the > menuconfig. NAK. Mutually exclusive compile-time configuration options are always wrong. If the PCI hosts both have the same PCI device ID and there is no other identification register that lets you find out which one you have, maybe you can use a module parameter that defaults to invalid and that has to be set explicitly when loading the PCI driver? Are both test chips the same way? I see that the driver supports two distinct PCI device IDs, so please check of they both come in variations for the two PHYs, or if at least one of them always uses the same PHY so you don't need the module parameter for that. > >> diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile > >> index 8303bcc..bab8c05 100644 > >> --- a/drivers/scsi/ufs/Makefile > >> +++ b/drivers/scsi/ufs/Makefile > >> @@ -1,4 +1,7 @@ > >> # UFSHCD mak
Re: [PATCH v9 3/3] add support for DWC UFS Host Controller
On Thursday 03 March 2016 11:39:05 Joao Pinto wrote: > Hi Arnd, > > On 3/2/2016 7:55 PM, Arnd Bergmann wrote: > > On Wednesday 02 March 2016 16:46:47 Joao Pinto wrote: > >> On 2/19/2016 3:03 PM, Arnd Bergmann wrote: > >>> On Thursday 18 February 2016 17:20:27 Joao Pinto wrote: > > Facts: > > - Test Chip type are currently not detectable in runtime through the > controller > - In the future the Test Chip version will be available in the controller > - Test Chip initialization is different for each type > - The IP Core version is 1.40a > - Test Chip version is 6.00 > - Teh UFS version is 2.0 Ok. > Suggested driver architecture: > > Platform setup: > tc-dwc-g210-pltfrm --> tc-dwc-g210 --> ufshcd-dwc-pltfrm --> ufshcd-dwc --> > ufs > > The test chip platform driver could be called through 2 compatibility strings. > indicating the chip's version and bit type: > "snps, g210-tc-6.00-20bit" > "snps, g210-tc-6.00-40bit" Yes, this sounds good. We can probably skip one of the middle layers, but basically that is what I was looking for. > The device tree node would have additional info compatibility strings as the > DWC > IP core version and UFS version: > "snps, dwc-ufshcd-1.40a" > "jedec, ufs-2.0" > > PCI based setup: > tc-dwc-g210-pci --> tc-dwc-g210 --> ufshcd-dwc-pci --> ufshcd-dwc --> ufs The tc-dwc-g210 portion probably shouldn't depend on both ufshcd-dwc-pltfrm and ufshcd-dwc-pci here, so how about leaving those two out? Then it becomes tc-dwc-g210-pci ---> tc-dwc-g210 --> ufshcd-dwc --> ufs tc-dwc-g210-pltfrm --/ > The test chip type would be configured by a parameter to be passed in the > kernel > boot args: tc_type = 20 (20-bit) or tc_type = 40 (40-bit) Right. With module_param() helper, this will be either a boot command line option, or a module load option, depending on whether the driver is built-on or not. modprobe tc-dwc-g210-pci tc_type=20 command line: tc-dwc-g210-pci.tc_type=20 > Having this in mind the KConfig would be: > > "config SCSI_UFS_DWC_HOOKS > bool I think we can now remove the config option for the hooks as well. > config SCSI_UFS_DWC_PLAT > tristate "DesignWare UFS controller platform glue driver" > depends on SCSI_UFSHCD_PLATFORM > select SCSI_UFS_DWC_HOOKS > help > This selects the DesignWare UFS host controller platform glue driver. > > Select this if you have a DesignWare UFS controller on Platform bus. > If unsure, say N. > > config SCSI_UFS_DWC_PCI > tristate "DesignWare UFS controller pci glue driver" > depends on SCSI_UFSHCD_PCI > select SCSI_UFS_DWC_HOOKS > help > This selects the DesignWare UFS host controller pci glue driver. > > Select this if you have a DesignWare UFS controller on pci bus. > If unsure, say N. > > config SCSI_UFS_DWC_TC > bool "Support for the Synopsys Test Chip" > depends on SCSI_UFS_DWC_HOOKS && (SCSI_UFSHCD_PCI || SCSI_UFS_DWC_PLAT) > ---help--- > Synopsys Test Chip is a Phy for prototyping purposes. > This selects the support for the Synopsys Test Chip. > > Select this if you have a Synopsys Test Chip. > If unsure, say N." > > Agree with the approach? This would work, but I think it's better to define the options in terms of the top-level drivers, i.e. SCSI_UFS_DWC_TC_PCI and SCSI_UFS_DWC_TC_PLATFORM, and then make the other options hidden and implicitly turned out by them. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 3/3] add support for DWC UFS Host Controller
On Thursday 03 March 2016 13:52:39 Joao Pinto wrote: > > config SCSI_UFS_DWC > bool > > config SCSI_UFS_DWC_TC_PLATFORM > tristate "DesignWare platform support using a G210 Test Chip" > depends on SCSI_UFSHCD_PLATFORM > select SCSI_UFS_DWC > ---help--- > Synopsys Test Chip is a PHY for prototyping purposes. > > If unsure, say N." > > config SCSI_UFS_DWC_TC_PCI > tristate "DesignWare pci support using a G210 Test Chip" > depends on SCSI_UFSHCD_PCI > select SCSI_UFS_DWC > ---help--- > Synopsys Test Chip is a PHY for prototyping purposes. > > If unsure, say N." > > I would keep SCSI_UFS_DWC to avoid building DWC specific source everytime. > > Agree? > Yes, looks good to me. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 2/6] added UFS 2.0 capabilities
On Friday 04 March 2016 17:22:15 Joao Pinto wrote: > Adding UFS 2.0 support to the UFS core driver. > > Signed-off-by: Joao Pinto > Acked-by: Arnd Bergmann -- 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 v10 3/6] added support for DesignWare Controller
On Friday 04 March 2016 17:22:16 Joao Pinto wrote: > This patch has the goal to add support for DesignWare UFS Controller > specific operations. > > Signed-off-by: Joao Pinto > Acked-by: Arnd Bergmann -- 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 v10 4/6] added support for Synopsys G210 Test Chip
On Friday 04 March 2016 17:22:17 Joao Pinto wrote: > This patch adds support for Synopsys G210 Test Chip. > > Signed-off-by: Joao Pinto > Acked-by: Arnd Bergmann -- 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 v10 5/6] add TC G210 platform driver
On Friday 04 March 2016 17:22:18 Joao Pinto wrote:
> This patch adds a glue platform driver for the Synopsys G210 Test Chip.
>
> Signed-off-by: Joao Pinto
Looks basically ok, but I think it can be simplified a little:
> +/**
> + * struct ufs_hba_dwc_vops - UFS DWC specific variant operations
> + */
> +static struct ufs_hba_variant_ops tc_dwc_g210_pltfm_hba_vops = {
> + .name = "tc-dwc-g210-pltfm",
> + .link_startup_notify= ufshcd_dwc_link_startup_notify,
> +};
> +
> +/**
> + * tc_dwc_g210_pltfm_probe()
> + * @pdev: pointer to platform device structure
> + *
> + */
> +static int tc_dwc_g210_pltfm_probe(struct platform_device *pdev)
> +{
> + int err;
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> +
> + /* Check Test Chip type and set the specific setup routine */
> + if (of_device_is_compatible(np, "snps, g210-tc-6.00-20bit")) {
> + tc_dwc_g210_pltfm_hba_vops.custom_phy_initialization =
> + tc_dwc_g210_config_20_bit;
> + } else if (of_device_is_compatible(np, "snps, g210-tc-6.00-40bit")) {
> + tc_dwc_g210_pltfm_hba_vops.custom_phy_initialization =
> + tc_dwc_g210_config_40_bit;
> + }
Instead of manually checking the compatible string, define two copies
of the ufs_hba_variant_ops, and put a pointer to them into the
.data field of tc_dwc_g210_pltfm_match.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 6/6] add TC G210 pci driver
On Friday 04 March 2016 17:22:19 Joao Pinto wrote:
> This patch adds a glue pci driver for the Synopsys G210 Test Chip.
>
> Signed-off-by: Joao Pinto
Mostly ok, just a few suggestions:
> +
> +/* Test Chip type expected values */
> +#define TC_G210_20BIT 20
> +#define TC_G210_40BIT 40
> +#define TC_G210_DEFAULTBIT 40
> +
> +static int tc_type = TC_G210_DEFAULTBIT;
> +module_param(tc_type, int, 0);
> +MODULE_PARM_DESC(tc_type, "Test Chip Type (20 = 20-bit, 40 = 40-bit)");
>
What is the effect of setting the wrong one here? I was thinking
it would be best to have the default be 'invalid' and then return
an error from the probe() function when you neither value is
set.
> +
> + /* Check Test Chip type and set the specific setup routine */
> + if (tc_type == TC_G210_20BIT) {
> + tc_dwc_g210_pci_hba_vops.custom_phy_initialization =
> + tc_dwc_g210_config_20_bit;
> + } else if (tc_type == TC_G210_40BIT) {
> + tc_dwc_g210_pci_hba_vops.custom_phy_initialization =
> + tc_dwc_g210_config_40_bit;
> + }
As for the platform driver, I would define two separate structures here,
and then mark the operations as 'const'.
> +static const struct pci_device_id tc_dwc_g210_pci_tbl[] = {
> + { PCI_VENDOR_ID_SYNOPSYS, 0xB101, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
> + { PCI_VENDOR_ID_SYNOPSYS, 0xB102, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
> + { } /* terminate list */
> +};
Is there any difference between these two IDs?
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 1/6] fixed typo in ufshcd-pltfrm
On Friday 04 March 2016 17:22:14 Joao Pinto wrote: > Fixed typo in ufshcd-pltfrm. > > Signed-off-by: Joao Pinto Acked-by: Arnd Bergmann -- 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] cxgbit: fix dma_addr_t printk format
The newly added driver prints a dma_addr_t using the %llx format string,
but that is wrong on most 32-bit architectures:
drivers/target/iscsi/cxgbit/cxgbit_ddp.c: In function 'cxgbit_dump_sgl':
drivers/target/iscsi/cxgbit/cxgbit_ddp.c:180:10: error: format '%llx' expects
argument of type 'long long unsigned int', but argument 8 has type 'dma_addr_t
{aka unsigned int}' [-Werror=format=]
pr_info("\t%d/%u, 0x%p: len %u, off %u, pg 0x%p, dma 0x%llx, %u\n",
Unfortunately, we can't use the %pad format string here because
we are not printing an lvalue, so we have to add a cast to u64, which
matches the format string on all architectures.
Signed-off-by: Arnd Bergmann
Fixes: c49aa56e556d ("cxgbit: add cxgbit_ddp.c")
---
drivers/target/iscsi/cxgbit/cxgbit_ddp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/target/iscsi/cxgbit/cxgbit_ddp.c
b/drivers/target/iscsi/cxgbit/cxgbit_ddp.c
index 07e2bc86d0df..d667bc88e21d 100644
--- a/drivers/target/iscsi/cxgbit/cxgbit_ddp.c
+++ b/drivers/target/iscsi/cxgbit/cxgbit_ddp.c
@@ -179,7 +179,7 @@ cxgbit_dump_sgl(const char *cap, struct scatterlist *sgl,
int nents)
for_each_sg(sgl, sg, nents, i)
pr_info("\t%d/%u, 0x%p: len %u, off %u, pg 0x%p, dma 0x%llx,
%u\n",
i, nents, sg, sg->length, sg->offset, sg_page(sg),
- sg_dma_address(sg), sg_dma_len(sg));
+ (u64)sg_dma_address(sg), sg_dma_len(sg));
}
static int cxgbit_ddp_sgl_check(struct scatterlist *sgl, int nents)
--
2.7.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] cxgbit: fix dma_addr_t printk format
On Friday 04 March 2016 16:25:07 Joe Perches wrote:
> >
> > diff --git a/drivers/target/iscsi/cxgbit/cxgbit_ddp.c
> > b/drivers/target/iscsi/cxgbit/cxgbit_ddp.c
> > index 07e2bc86d0df..d667bc88e21d 100644
> > --- a/drivers/target/iscsi/cxgbit/cxgbit_ddp.c
> > +++ b/drivers/target/iscsi/cxgbit/cxgbit_ddp.c
> > @@ -179,7 +179,7 @@ cxgbit_dump_sgl(const char *cap, struct scatterlist
> > *sgl, int nents)
> > for_each_sg(sgl, sg, nents, i)
> > pr_info("\t%d/%u, 0x%p: len %u, off %u, pg 0x%p, dma 0x%llx,
> > %u\n",
> > i, nents, sg, sg->length, sg->offset, sg_page(sg),
> > - sg_dma_address(sg), sg_dma_len(sg));
> > + (u64)sg_dma_address(sg), sg_dma_len(sg));
> > }
> >
> > static int cxgbit_ddp_sgl_check(struct scatterlist *sgl, int nents)
>
> You could create a temporary:
>
> for_each_sg(sgl, sg, nents, i) {
> dma_addr_t addr = sg_dma_address(sg);
>
> pr_info("\t%d/%u, 0x%p: len %u, off %u, pg 0x%p, dma %pad,
> %u\n",
> i, nents, sg, sg->length, sg->offset, sg_page(sg),
> &addr, sg_dma_len(sg));
> }
>
Sure, but the cast seemed nicer in this case, the result is the same.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7] scsi: ufs: add ioctl interface for query request
On Wednesday 09 March 2016, yga...@codeaurora.org wrote: > Any userspace application can be a tool. > We already implemented and used a user space application, that sent > queries to the UFS devices in order to get information and descriptors. > Not only ioctl interface is a useful way to interact with the device, > we used it, and found it very helpful in varies cases. > hence, this patch. > This patch has been already addressed all comments of Arnd Bergman from 5 > months ago, and now, re-uploaded again. Do you have a pointer to that review? It's been a long while, so I have completely forgotten what issues I raised and how it got resolved. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7] scsi: ufs: add ioctl interface for query request
On Thursday 10 March 2016, Arnd Bergmann wrote: > On Wednesday 09 March 2016, yga...@codeaurora.org wrote: > > Any userspace application can be a tool. > > We already implemented and used a user space application, that sent > > queries to the UFS devices in order to get information and descriptors. > > Not only ioctl interface is a useful way to interact with the device, > > we used it, and found it very helpful in varies cases. > > hence, this patch. > > This patch has been already addressed all comments of Arnd Bergman from 5 > > months ago, and now, re-uploaded again. > > Do you have a pointer to that review? It's been a long while, so I > have completely forgotten what issues I raised and how it got resolved. I got your link in private message and read up on it again now. To clarify: I commented on the formal API definition, and you indeed addressed all my concerns, so this is now an ioctl command that follows our usual calling conventions. However, this is orthogonal to the question of whether it is a good idea to have this interface implemented as an ioctl as asked by Greg, and who is actually using it. I'm lacking the detailed subsystem knowledge to answer this, but I note that other block drivers have similar passthrough interfaces. Looking through what other drivers do, I've found a couple of patterns now. n particular, most use the SG_IO ioctl to pass down commands from user space into a device specific command queue. Have you looked at that interface in the past to see if it would fit your use case? There is also a 'bsg' API that some drivers implement, which I think would be another alternative. Could any of the SCSI experts comment on what they expect a driver to use out of those three alternatives (if any): * private ioctl * bsg * sg_io Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
gcc-6.0 warnings for scsi
gcc-6 found a couple of bugs in scsi drivers that are not yet fixed in linux-next. In all three cases, the code is indented in a rather misleading way, but actually correct. I think it would be good to have this fixed in 4.6, but no backports are needed even though the problems have all been around for a while. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] lpfc: fix misleading indentation
gcc-6 complains about the indentation of the lpfc_destroy_vport_work_array()
call in lpfc_online(), which clearly doesn't look right:
drivers/scsi/lpfc/lpfc_init.c: In function 'lpfc_online':
drivers/scsi/lpfc/lpfc_init.c:2880:3: warning: statement is indented as if it
were guarded by... [-Wmisleading-indentation]
lpfc_destroy_vport_work_array(phba, vports);
^
drivers/scsi/lpfc/lpfc_init.c:2863:2: note: ...this 'if' clause, but it is not
if (vports != NULL)
^~
Looking at the patch that introduced this code, it's clear that the
behavior is correct and the indentation is wrong.
This fixes the indentation and adds curly braces around the previous
if() block for clarity, as that is most likely what caused the code
to be misindented in the first place.
Signed-off-by: Arnd Bergmann
Fixes: 549e55cd2a1b ("[SCSI] lpfc 8.2.2 : Fix locking around HBA's port_list")
---
drivers/scsi/lpfc/lpfc_init.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index a544366a367e..f57d02c3b6cf 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -2860,7 +2860,7 @@ lpfc_online(struct lpfc_hba *phba)
}
vports = lpfc_create_vport_work_array(phba);
- if (vports != NULL)
+ if (vports != NULL) {
for (i = 0; i <= phba->max_vports && vports[i] != NULL; i++) {
struct Scsi_Host *shost;
shost = lpfc_shost_from_vport(vports[i]);
@@ -2877,7 +2877,8 @@ lpfc_online(struct lpfc_hba *phba)
}
spin_unlock_irq(shost->host_lock);
}
- lpfc_destroy_vport_work_array(phba, vports);
+ }
+ lpfc_destroy_vport_work_array(phba, vports);
lpfc_unblock_mgmt_io(phba);
return 0;
--
2.7.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 3/3] megaraid_sas: add missing curly braces in ioctl handler
gcc-6 found a dubious indentation in the megasas_mgmt_fw_ioctl
function:
drivers/scsi/megaraid/megaraid_sas_base.c: In function 'megasas_mgmt_fw_ioctl':
drivers/scsi/megaraid/megaraid_sas_base.c:6658:4: warning: statement is
indented as if it were guarded by... [-Wmisleading-indentation]
kbuff_arr[i] = NULL;
^
drivers/scsi/megaraid/megaraid_sas_base.c:6653:3: note: ...this 'if' clause,
but it is not
if (kbuff_arr[i])
^~
The code is actually correct, as there is no downside in clearing a NULL
pointer again.
This clarifies the code and avoids the warning by adding extra curly
braces.
Signed-off-by: Arnd Bergmann
Fixes: 90dc9d98f01b ("megaraid_sas : MFI MPT linked list corruption fix")
---
drivers/scsi/megaraid/megaraid_sas_base.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
b/drivers/scsi/megaraid/megaraid_sas_base.c
index 5c08568ccfbf..2627200d4f82 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -6650,12 +6650,13 @@ out:
}
for (i = 0; i < ioc->sge_count; i++) {
- if (kbuff_arr[i])
+ if (kbuff_arr[i]) {
dma_free_coherent(&instance->pdev->dev,
le32_to_cpu(kern_sge32[i].length),
kbuff_arr[i],
le32_to_cpu(kern_sge32[i].phys_addr));
kbuff_arr[i] = NULL;
+ }
}
megasas_return_cmd(instance, cmd);
--
2.7.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 1/3] aacraid: add missing curly braces
gcc-6 warns about obviously wrong indentation for newly added
code in aac_slave_configure():
drivers/scsi/aacraid/linit.c: In function 'aac_slave_configure':
drivers/scsi/aacraid/linit.c:458:3: warning: statement is indented as if it
were guarded by... [-Wmisleading-indentation]
sdev->tagged_supported = 1;
^~~~
drivers/scsi/aacraid/linit.c:455:4: note: ...this 'else' clause, but it is not
gcc is correct, and evidently this was meant to be within the curly
braces that should have been there to start with. This patch adds
them, which avoids the warning and makes it clear what was intended
here.
Nothing changes in behavior because in the 'if' block, the
sdev->tagged_supported flag is known to be set already.
Signed-off-by: Arnd Bergmann
Fixes: 6bf3b630d0a7 ("aacraid: SCSI blk tag support")
---
drivers/scsi/aacraid/linit.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 21a67ed047e8..ff6caab8cc8b 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -452,10 +452,11 @@ static int aac_slave_configure(struct scsi_device *sdev)
else if (depth < 2)
depth = 2;
scsi_change_queue_depth(sdev, depth);
- } else
+ } else {
scsi_change_queue_depth(sdev, 1);
sdev->tagged_supported = 1;
+ }
return 0;
}
--
2.7.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 2/3] lpfc: fix misleading indentation
On Monday 14 March 2016 16:19:58 Hannes Reinecke wrote:
> > vports = lpfc_create_vport_work_array(phba);
> > - if (vports != NULL)
> > + if (vports != NULL) {
> > for (i = 0; i <= phba->max_vports && vports[i] != NULL; i++) {
> > struct Scsi_Host *shost;
> > shost = lpfc_shost_from_vport(vports[i]);
> > @@ -2877,7 +2877,8 @@ lpfc_online(struct lpfc_hba *phba)
> > }
> > spin_unlock_irq(shost->host_lock);
> > }
> > - lpfc_destroy_vport_work_array(phba, vports);
> > + }
> > + lpfc_destroy_vport_work_array(phba, vports);
> >
> > lpfc_unblock_mgmt_io(phba);
> > return 0;
> >
> Nope.
>
> vports is only valid from within the indentation block, so it should
> be moved into it.
>
>
Well, every other user of the function also looks like
vports = lpfc_create_vport_work_array(phba);
if (vports != NULL) {
do_something(vports);
}
lpfc_destroy_vport_work_array(phba, vports);
and lpfc_destroy_vport_work_array() does nothing if its argument is NULL.
I still think my patch is the correct fix for the warning.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] qla2xxx: avoid maybe_uninitialized warning
The qlt_check_reserve_free_req() function produces an incorrect warning
when CONFIG_PROFILE_ANNOTATED_BRANCHES is set:
drivers/scsi/qla2xxx/qla_target.c: In function 'qlt_check_reserve_free_req':
drivers/scsi/qla2xxx/qla_target.c:1887:3: error: 'cnt_in' may be used
uninitialized in this function [-Werror=maybe-uninitialized]
ql_dbg(ql_dbg_io, vha, 0x305a,
^~
"qla_target(%d): There is no room in the request ring:
vha->req->ring_index=%d, vha->req->cnt=%d, req_cnt=%d Req-out=%d Req-in=%d
Req-Length=%d\n",
~~~
vha->vp_idx, vha->req->ring_index,
~~
vha->req->cnt, req_cnt, cnt, cnt_in, vha->req->length);
~~
drivers/scsi/qla2xxx/qla_target.c:1887:3: error: 'cnt' may be used
uninitialized in this function [-Werror=maybe-uninitialized]
The problem is that gcc fails to track the state of the condition across
an annotated branch.
This slightly rearranges the code to move the second if() block
into the first one, to avoid the warning while retaining the
behavior of the code.
Signed-off-by: Arnd Bergmann
---
drivers/scsi/qla2xxx/qla_target.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_target.c
b/drivers/scsi/qla2xxx/qla_target.c
index 985231900aca..8a44d1541eb4 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -1881,15 +1881,17 @@ static int qlt_check_reserve_free_req(struct
scsi_qla_host *vha,
else
vha->req->cnt = vha->req->length -
(vha->req->ring_index - cnt);
- }
- if (unlikely(vha->req->cnt < (req_cnt + 2))) {
- ql_dbg(ql_dbg_io, vha, 0x305a,
- "qla_target(%d): There is no room in the request ring:
vha->req->ring_index=%d, vha->req->cnt=%d, req_cnt=%d Req-out=%d Req-in=%d
Req-Length=%d\n",
- vha->vp_idx, vha->req->ring_index,
- vha->req->cnt, req_cnt, cnt, cnt_in, vha->req->length);
- return -EAGAIN;
+ if (unlikely(vha->req->cnt < (req_cnt + 2))) {
+ ql_dbg(ql_dbg_io, vha, 0x305a,
+ "qla_target(%d): There is no room in the request
ring: vha->req->ring_index=%d, vha->req->cnt=%d, req_cnt=%d Req-out=%d
Req-in=%d Req-Length=%d\n",
+ vha->vp_idx, vha->req->ring_index,
+ vha->req->cnt, req_cnt, cnt, cnt_in,
+ vha->req->length);
+ return -EAGAIN;
+ }
}
+
vha->req->cnt -= req_cnt;
return 0;
--
2.7.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: fc: use get/put_unaligned64 for wwn access
A bug in the gcc-6.0 prerelease version caused at least one
driver (lpfc) to have excessive stack usage when dealing with
wwn data, on the ARM architecture.
lpfc_scsi.c: In function 'lpfc_find_next_oas_lun':
lpfc_scsi.c:117:1: warning: the frame size of 1152 bytes is larger than 1024
bytes [-Wframe-larger-than=]
I have reported this as a gcc regression in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70232
However, using a better implementation of wwn_to_u64() not only
helps with the particular gcc problem but also leads to better
object code for any version or architecture.
The kernel already provides get_unaligned_be64() and
put_unaligned_be64() helper functions that provide an
optimized implementation with the desired semantics.
The lpfc_find_next_oas_lun() function in the example that
grew from 1146 bytes to 5144 bytes when moving from gcc-5.3
to gcc-6.0 is now 804 bytes, as the optimized
get_unaligned_be64() load can be done in three instructions.
The stack usage is now down to 28 bytes from 128 bytes with
gcc-5.3 before.
Signed-off-by: Arnd Bergmann
---
include/scsi/scsi_transport_fc.h | 15 +++
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/include/scsi/scsi_transport_fc.h b/include/scsi/scsi_transport_fc.h
index 784bc2c0929f..bf66ea6bed2b 100644
--- a/include/scsi/scsi_transport_fc.h
+++ b/include/scsi/scsi_transport_fc.h
@@ -28,6 +28,7 @@
#define SCSI_TRANSPORT_FC_H
#include
+#include
#include
#include
@@ -797,22 +798,12 @@ fc_remote_port_chkready(struct fc_rport *rport)
static inline u64 wwn_to_u64(u8 *wwn)
{
- return (u64)wwn[0] << 56 | (u64)wwn[1] << 48 |
- (u64)wwn[2] << 40 | (u64)wwn[3] << 32 |
- (u64)wwn[4] << 24 | (u64)wwn[5] << 16 |
- (u64)wwn[6] << 8 | (u64)wwn[7];
+ return get_unaligned_be64(wwn);
}
static inline void u64_to_wwn(u64 inm, u8 *wwn)
{
- wwn[0] = (inm >> 56) & 0xff;
- wwn[1] = (inm >> 48) & 0xff;
- wwn[2] = (inm >> 40) & 0xff;
- wwn[3] = (inm >> 32) & 0xff;
- wwn[4] = (inm >> 24) & 0xff;
- wwn[5] = (inm >> 16) & 0xff;
- wwn[6] = (inm >> 8) & 0xff;
- wwn[7] = inm & 0xff;
+ put_unaligned_be64(inm, wwn);
}
/**
--
2.7.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] qla2xxx: avoid maybe_uninitialized warning
On Tuesday 15 March 2016 14:49:14 James Bottomley wrote: > On Tue, 2016-03-15 at 22:40 +0100, Arnd Bergmann wrote: > > > > This slightly rearranges the code to move the second if() block > > into the first one, to avoid the warning while retaining the > > behavior of the code. > > I thought our usual policy was to ask someone to fix the compiler when > it emitted a spurious warning. No, the rule is that we shouldn't blindly add initializations to the variables when the compiler should have figured it out. In this case, I wouldn't expect the compiler to ever see through the unlikely() macro, and I'm not adding a potentially counterproductive initialization, so I see no reason not to apply the patch. Making it easier for the compiler to figure out what is going on should also lead to slightly better object code. If you think my patch makes it less readable, an alternative would be to remove the 'unlikely', which also gets rid of the warning. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: UFS V11 patch-set
On Friday 18 March 2016 12:52:13 Joao Pinto wrote: > Hi! > > Could you please check the following patch-set in order to evaluate if it is > ready for v4.6? > I think the code is ok now, but the timing apparently didn't work for 4.6. I'd suggest you resend as soon as 4.6-rc1 is out so it can get merged into 4.7. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] scsi: ufs: select CONFIG_NLS
A recent change to ufshcd introduced a call to utf16s_to_utf8s,
a function that is provided by the NLS module, so we get a link
error when that is not present:
drivers/scsi/built-in.o: In function `ufshcd_read_string_desc':
:(.text+0x124d0): undefined reference to `utf16s_to_utf8s'
This adds a Kconfig 'select' statement to avoid the build error.
Signed-off-by: Arnd Bergmann
Fixes: b573d484e4ff ("scsi: ufs: add support to read device and string
descriptors")
---
drivers/scsi/ufs/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
index 5f4530744e0a..097894a1fab5 100644
--- a/drivers/scsi/ufs/Kconfig
+++ b/drivers/scsi/ufs/Kconfig
@@ -37,6 +37,7 @@ config SCSI_UFSHCD
depends on SCSI && SCSI_DMA
select PM_DEVFREQ
select DEVFREQ_GOV_SIMPLE_ONDEMAND
+ select NLS
---help---
This selects the support for UFS devices in Linux, say Y and make
sure that you know the name of your UFS host adapter (the card
--
2.7.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: ufs: select CONFIG_NLS
On Tuesday 22 March 2016 07:43:19 James Bottomley wrote: > > diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig > > index 5f4530744e0a..097894a1fab5 100644 > > --- a/drivers/scsi/ufs/Kconfig > > +++ b/drivers/scsi/ufs/Kconfig > > @@ -37,6 +37,7 @@ config SCSI_UFSHCD > > depends on SCSI && SCSI_DMA > > select PM_DEVFREQ > > select DEVFREQ_GOV_SIMPLE_ONDEMAND > > + select NLS > > This looks like a bad solution: CONFIG_NLS is nothing more than a menu > selector for the NLS subsystem. It's a bit of both: CONFIG_NLS by itself controls the compilation of the fs/nls/nls_base.c file, which has the definition of the utf16s_to_utf8s function called by this driver. The same file also contains the NLS subsystem with the register_nls/unregister_nls/unload_nls/load_nls APIs that are used by the other files. > The problem is that selecting it will > allow a kernel to be build with NLS and without NLS_DEFAULT which is > going to cause all sorts of interesting problems on boot. I think you > really mean depends on NLS here. I agree that 'depends on NLS' makes more sense here, however I was just following what all other users of the API are doing. Mixing 'select' and 'depends on' can easily lead to circular dependencies, and I was trying to avoid that. A cleaner solution is probably to split out the utf16 access functions from the NLS subsystem, and have all these files that currently 'select NLS' pick that other interface (or both, if necessary) instead: drivers/acpi/device_sysfs.c drivers/hid/hid-cp2112.c drivers/hv/hv_fcopy.c drivers/hv/hv_kvp.c drivers/net/fjes/fjes_main.c drivers/net/hyperv/rndis_filter.c drivers/pci/pci-label.c drivers/scsi/ufs/ufshcd.c drivers/usb/core/message.c drivers/usb/gadget/configfs.c drivers/usb/gadget/u_os_desc.h drivers/usb/gadget/usbstring.c fs/cifs/cifs_unicode.c fs/fat/dir.c fs/fat/namei_vfat.c fs/isofs/joliet.c Not sure if that's worth the work though. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: This patch triggers a bad gcc bug (was Re: [PATCH] force inlining of some byteswap operations)
On Friday 15 April 2016 07:45:19 Ingo Molnar wrote:
>
> * Denys Vlasenko wrote:
>
> > > In fact, the following patch seems to fix it:
> > >
> > > diff --git a/include/scsi/scsi_transport_fc.h
> > > b/include/scsi/scsi_transport_fc.h
> > > index bf66ea6..56b9e81 100644
> > > --- a/include/scsi/scsi_transport_fc.h
> > > +++ b/include/scsi/scsi_transport_fc.h
> > > @@ -796,7 +796,7 @@ fc_remote_port_chkready(struct fc_rport *rport)
> > > return result;
> > > }
> > >
> > > -static inline u64 wwn_to_u64(u8 *wwn)
> > > +static __always_inline u64 wwn_to_u64(u8 *wwn)
> > > {
> > > return get_unaligned_be64(wwn);
> > > }
> >
> > It is not a guarantee.
>
> Of course it's a workaround - but is there any deterministic way to turn off
> this
> GCC bug (by activating some GCC command line switch), or do we have to live
> with
> objtool warning about this GCC?
>
> Which, by the way, is pretty cool!
I have done a patch for the asm-generic/unaligned handling recently that
reworks the implementation to avoid an ARM specific bug (gcc uses certain
CPU instructions that require aligned data when we tell it that unaligned
data is not).
It changes the code enough that the gcc bug might not be triggered any more,
aside from generating far superior code in some cases.
I thought I had submitted that patch before, but I can't find a version
with a proper changelog any more now, so I probably haven't. However, I did
all the research to show that it only makes things better on ARM and x86
except in cases where the gcc inliner happens to pick a different set of
functions to be inline (these have a 50:50 chance of better vs worse, the
result on average seems to be the same).
Arnd
commit 752b719f6675be02a3dd29fe5d92b2f380b5743d
Author: Arnd Bergmann
Date: Fri Mar 4 16:15:20 2016 +0100
asm-generic: always use struct based unaligned access
Signed-off-by: Arnd Bergmann
diff --git a/include/asm-generic/unaligned.h b/include/asm-generic/unaligned.h
index 1ac097279db1..e8f5523eeb0a 100644
--- a/include/asm-generic/unaligned.h
+++ b/include/asm-generic/unaligned.h
@@ -3,29 +3,19 @@
/*
* This is the most generic implementation of unaligned accesses
- * and should work almost anywhere.
+ * and should work almost anywhere, we trust that the compiler
+ * knows how to handle unaligned accesses.
*/
#include
-/* Set by the arch if it can handle unaligned accesses in hardware. */
-#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
-# include
-#endif
+#include
+#include
+#include
#if defined(__LITTLE_ENDIAN)
-# ifndef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
-# include
-# include
-# endif
-# include
# define get_unaligned __get_unaligned_le
# define put_unaligned __put_unaligned_le
#elif defined(__BIG_ENDIAN)
-# ifndef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
-# include
-# include
-# endif
-# include
# define get_unaligned __get_unaligned_be
# define put_unaligned __put_unaligned_be
#else
diff --git a/include/linux/unaligned/be_struct.h
b/include/linux/unaligned/be_struct.h
index 132415836c50..9ab8c53bb3fe 100644
--- a/include/linux/unaligned/be_struct.h
+++ b/include/linux/unaligned/be_struct.h
@@ -2,35 +2,36 @@
#define _LINUX_UNALIGNED_BE_STRUCT_H
#include
+#include
static inline u16 get_unaligned_be16(const void *p)
{
- return __get_unaligned_cpu16((const u8 *)p);
+ return be16_to_cpu((__le16 __force)__get_unaligned_cpu16((const u8
*)p));
}
static inline u32 get_unaligned_be32(const void *p)
{
- return __get_unaligned_cpu32((const u8 *)p);
+ return be32_to_cpu((__le32 __force)__get_unaligned_cpu32((const u8
*)p));
}
static inline u64 get_unaligned_be64(const void *p)
{
- return __get_unaligned_cpu64((const u8 *)p);
+ return be64_to_cpu((__le64 __force)__get_unaligned_cpu64((const u8
*)p));
}
static inline void put_unaligned_be16(u16 val, void *p)
{
- __put_unaligned_cpu16(val, p);
+ __put_unaligned_cpu16((u16 __force)cpu_to_be16(val), p);
}
static inline void put_unaligned_be32(u32 val, void *p)
{
- __put_unaligned_cpu32(val, p);
+ __put_unaligned_cpu32((u32 __force)cpu_to_be32(val), p);
}
static inline void put_unaligned_be64(u64 val, void *p)
{
- __put_unaligned_cpu64(val, p);
+ __put_unaligned_cpu64((u64 __force)cpu_to_be64(val), p);
}
#endif /* _LINUX_UNALIGNED_BE_STRUCT_H */
diff --git a/include/linux/unaligned/le_struct.h
b/include/linux/unaligned/le_struct.h
index 088c4572faa8..64171ad0b100 100644
--- a/include/linux/unaligned/le_struct.h
+++ b/include/linux/unaligned/le_struct.h
@@ -2,35 +2,36 @@
#define _LINUX_UNALIGNED_LE_STRUCT_H
#include
+#include
static inline u16 get_unaligned_le16(const void *p)
{
- return __get_unaligned_cpu16
[PATCH] aha1542: probe correct address for isapnp
gcc warns about an out of bounds access after a recent cleanup:
drivers/scsi/aha1542.c: In function 'aha1542_pnp_probe':
drivers/scsi/aha1542.c:703:27: error: array subscript is above array bounds
[-Werror=array-bounds]
unsigned int base_io = io[indx];
~~^~
drivers/scsi/aha1542.c:728:2: error: array subscript is above array bounds
[-Werror=array-bounds]
aha1542_set_bus_times(sh, bus_on[indx], bus_off[indx], dma_speed[indx]);
Indeed the aha1542_pnp_probe() function that was added cannot possibly
have worked, as it first tries to find an available index to use for
the probed device, but then passes one after the last index to the
aha1542_hw_init() function.
This is an attempt to rework the probe function into what it should
have been. It is however not tested any more than the original patch
other than making sure we get no warnings.
Signed-off-by: Arnd Bergmann
Fixes: 643a7c43f11e ("aha1542: Stop using scsi_module.c")
---
drivers/scsi/aha1542.c | 28 +---
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/aha1542.c b/drivers/scsi/aha1542.c
index 7db448ec8beb..132c311ef605 100644
--- a/drivers/scsi/aha1542.c
+++ b/drivers/scsi/aha1542.c
@@ -996,25 +996,31 @@ static int aha1542_pnp_probe(struct pnp_dev *pdev, const
struct pnp_device_id *i
{
int indx;
struct Scsi_Host *sh;
+ int ret;
- for (indx = 0; indx < ARRAY_SIZE(io); indx++) {
- if (io[indx])
- continue;
+ for (indx = 0; indx < ARRAY_SIZE(io); indx++)
+ if (!io[indx])
+ break;
- if (pnp_activate_dev(pdev) < 0)
- continue;
+ if (indx == ARRAY_SIZE(io))
+ return -ENXIO;
- io[indx] = pnp_port_start(pdev, 0);
+ ret = pnp_activate_dev(pdev);
+ if (ret < 0)
+ return ret;
- /* The card can be queried for its DMA, we have
- the DMA set up that is enough */
+ io[indx] = pnp_port_start(pdev, 0);
- dev_info(&pdev->dev, "ISAPnP found an AHA1535 at I/O 0x%03X",
io[indx]);
- }
+ /* The card can be queried for its DMA, we have
+ the DMA set up that is enough */
+
+ dev_info(&pdev->dev, "ISAPnP found an AHA1535 at I/O 0x%03X", io[indx]);
sh = aha1542_hw_init(&driver_template, &pdev->dev, indx);
- if (!sh)
+ if (!sh) {
+ pnp_disable_dev(pdev);
return -ENODEV;
+ }
pnp_set_drvdata(pdev, sh);
return 0;
--
2.7.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: This patch triggers a bad gcc bug (was Re: [PATCH] force inlining of some byteswap operations)
On Monday 18 April 2016 09:12:41 Josh Poimboeuf wrote:
> On Mon, Apr 18, 2016 at 04:07:51PM +0200, Arnd Bergmann wrote:
> > On Monday 18 April 2016 08:39:32 Josh Poimboeuf wrote:
> > >
> > > I agree. So how should we work around the bug in this case? There have
> > > been several suggestions:
> > >
> > > - change wwn_to_u64() to __always_inline
> > >
> > > - change qla2x00_get_host_fabric_name() to skip the unnecessary call to
> > > wwn_to_u64()
> > >
> > > - revert one of the two commits:
> > > bc27fb68aaad ("include/uapi/linux/byteorder, swab: force inlining of
> > > some byteswap operations")
> > > ef3fb2422ffe ("scsi: fc: use get/put_unaligned64 for wwn access")
> >
> > What about the patch to change get_unaligned_be64() that I posted?
> >
> > I think we want to merge that anyway, I just don't know if that helps
> > with this particular problem as well.
>
> I replied to your other email about that -- it doesn't seem to help this
> issue.
>
Ok, I see. I had problems with my mail server last week, your reply
must have been a victim of that as I never saw it (found it on the
web archive now).
I'd vote for the wwn_to_u64 change then as it should prevent the
same thing from happining in other drivers. I would prefer not to
see ef3fb2422ffe reverted, as that works around another gcc-6 bug
on ARM.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: This patch triggers a bad gcc bug (was Re: [PATCH] force inlining of some byteswap operations)
On Monday 18 April 2016 08:39:32 Josh Poimboeuf wrote:
>
> I agree. So how should we work around the bug in this case? There have
> been several suggestions:
>
> - change wwn_to_u64() to __always_inline
>
> - change qla2x00_get_host_fabric_name() to skip the unnecessary call to
> wwn_to_u64()
>
> - revert one of the two commits:
> bc27fb68aaad ("include/uapi/linux/byteorder, swab: force inlining of some
> byteswap operations")
> ef3fb2422ffe ("scsi: fc: use get/put_unaligned64 for wwn access")
What about the patch to change get_unaligned_be64() that I posted?
I think we want to merge that anyway, I just don't know if that helps
with this particular problem as well.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: fc: force inlining of wwn conversion functions
On Monday 25 April 2016 20:37:31 James Bottomley wrote:
> On Mon, 2016-04-25 at 22:40 -0400, Martin K. Petersen wrote:
> > > > > > > "Josh" == Josh Poimboeuf writes:
> >
> > Josh> Can you merge this patch for 4.6?
> >
> > I am really not a big fan of working around compiler bugs in a device
> > driver.
>
> Me neither
>
> > Are we sure there are no other get_unaligned_be64() calls in the
> > kernel that suffer the same fate?
>
> Agree, plus, as I've said before, we have 3-4 weeks before we go final,
> so we still have some time before a decision has to be made. It looks
> like the gcc people already have a patch for the compiler, so the
> distributions could just push that out through channels.
I don't think we can realistically blacklist gcc-4.9.{0,1,2,3},
gcc-5.{0,1,2,3}.* and gcc-6.0 and require everyone to upgrade to compilers
that have not been released yet in order to build a linux-4.6 kernel.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: fc: force inlining of wwn conversion functions
On Tuesday 26 April 2016 01:35:16 Christoph Hellwig wrote:
> On Tue, Apr 26, 2016 at 09:22:46AM +0200, Arnd Bergmann wrote:
> > > Agree, plus, as I've said before, we have 3-4 weeks before we go final,
> > > so we still have some time before a decision has to be made. It looks
> > > like the gcc people already have a patch for the compiler, so the
> > > distributions could just push that out through channels.
> >
> > I don't think we can realistically blacklist gcc-4.9.{0,1,2,3},
> > gcc-5.{0,1,2,3}.* and gcc-6.0 and require everyone to upgrade to compilers
> > that have not been released yet in order to build a linux-4.6 kernel.
>
> Agreed. What about just removing the wrappers? They seem fairly
> pointless to start with.
I think at this point it's mainly a question of whether we want such a
big (however trivial) patch in v4.6. We can certainly do that for 4.7,
but as a fixup for the existing problem, either the __always_inline
hack or using a macro should be sufficient:
diff --git a/include/scsi/scsi_transport_fc.h b/include/scsi/scsi_transport_fc.h
index bf66ea6bed2b..51a98b182a67 100644
--- a/include/scsi/scsi_transport_fc.h
+++ b/include/scsi/scsi_transport_fc.h
@@ -796,15 +796,8 @@ fc_remote_port_chkready(struct fc_rport *rport)
return result;
}
-static inline u64 wwn_to_u64(u8 *wwn)
-{
- return get_unaligned_be64(wwn);
-}
-
-static inline void u64_to_wwn(u64 inm, u8 *wwn)
-{
- put_unaligned_be64(inm, wwn);
-}
+#define wwn_to_u64(wwn) get_unaligned_be64(wwn)
+#define u64_to_wwn(inm, wwn) put_unaligned_be64(inm, wwn)
/**
* fc_vport_set_state() - called to set a vport's state. Saves the old state,
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: fc: force inlining of wwn conversion functions
On Tuesday 26 April 2016 09:06:54 Martin K. Petersen wrote:
> >>>>> "Arnd" == Arnd Bergmann writes:
>
> Arnd> I don't think we can realistically blacklist gcc-4.9.{0,1,2,3},
> Arnd> gcc-5.{0,1,2,3}.* and gcc-6.0 and require everyone to upgrade to
> Arnd> compilers that have not been released yet in order to build a
> Arnd> linux-4.6 kernel.
>
> I agree that compiler blacklisting is problematic and I'd like to avoid
> it. The question is how far we go in the kernel to accommodate various
> levels of brokenness.
>
> In any case. Sticking compiler workarounds in device driver code is akin
> to putting demolition orders on display on Alpha Centauri. At the very
> minimum the patch should put a fat comment in the code stating that
> these wrapper functions or #defines should not be changed in the future
> because that'll break builds using gcc XYZ. But that does not solve the
> problem for anybody else that might be doing something similar.
> Converting between u64 and $RANDOM_TYPE in an inline wrapper does not
> seem like a rare and unusual programming pattern.
It's not the driver really, it's the core scsi/fc layer, which makes
it a little dangerous that a random driver.
I agree that putting a comment in would also help. What I understand
from the bug report is that to trigger this bug you need these elements:
1. an inline function marked __always_inline
2. another inline function that is automatically inlined (not __always_inline)
3. CONFIG_OPTIMIZE_INLINING=y to guarantee 2
4. __builtin_compatible_p inside that inline function
The last point is what Denys introduced in the kernel with
bc27fb68aaad ("include/uapi/linux/byteorder, swab: force inlining of some
byteswap operations"). So maybe it's better after all to revert that
patch, to have a higher confidence in the same bug not appearing
elsewhere. It's also really a workaround for another quirk of the
compiler, but that one only results in duplicated functions in object
code rather than functions that end in the middle.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: fc: force inlining of wwn conversion functions
On Wednesday 27 April 2016 13:05:03 Martin Jambor wrote:
> On Tue, Apr 26, 2016 at 05:58:20PM +0200, Arnd Bergmann wrote:
> > On Tuesday 26 April 2016 09:06:54 Martin K. Petersen wrote:
> > > >>>>> "Arnd" == Arnd Bergmann writes:
> > >
> > > Arnd> I don't think we can realistically blacklist gcc-4.9.{0,1,2,3},
> > > Arnd> gcc-5.{0,1,2,3}.* and gcc-6.0 and require everyone to upgrade to
> > > Arnd> compilers that have not been released yet in order to build a
> > > Arnd> linux-4.6 kernel.
> > >
> > > I agree that compiler blacklisting is problematic and I'd like to avoid
> > > it. The question is how far we go in the kernel to accommodate various
> > > levels of brokenness.
> > >
> > > In any case. Sticking compiler workarounds in device driver code is akin
> > > to putting demolition orders on display on Alpha Centauri. At the very
> > > minimum the patch should put a fat comment in the code stating that
> > > these wrapper functions or #defines should not be changed in the future
> > > because that'll break builds using gcc XYZ. But that does not solve the
> > > problem for anybody else that might be doing something similar.
> > > Converting between u64 and $RANDOM_TYPE in an inline wrapper does not
> > > seem like a rare and unusual programming pattern.
> >
> > It's not the driver really, it's the core scsi/fc layer, which makes
> > it a little dangerous that a random driver.
> >
> > I agree that putting a comment in would also help. What I understand
> > from the bug report is that to trigger this bug you need these elements:
> >
> > 1. an inline function marked __always_inline
> > 2. another inline function that is automatically inlined (not
> > __always_inline)
> > 3. CONFIG_OPTIMIZE_INLINING=y to guarantee 2
> > 4. __builtin_compatible_p inside that inline function
>
> The __always_inline requirement is not true. In fact, if you look at
> the example testcase filed in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646#c7 you'll see it
> uses __builtin_compatible_p in an __always inline function that is
> called from one that is not tagged with that attribute.
>
> And generally speaking, always inline is never a requirement, any call
> or chain of calls that the inliner can decide to inline can lead to
> the bug (if it complies with the condition below).
Ok, thanks for the clarification, I thought you always had to have both
kinds of inline functions.
> What is a requirement, though, is that __builtin_compatible_p is
> called on something passed in an argument by reference or in an
> aggregate (i.e. struct or array) argument.
>
> So,
>
> int foo1 (unsigned long *ref)
> {
> if (__builtin_constant (*ref))
> ...
> else
> /* wrongly unreachable code */
> }
>
> }
>
> cannot, and is fine. But please note that wrapping a foo[12]-like
> function into a dereferencing wrapper might not help if foo[12] would
> be early-inlined into such wrapper (GCC has two inliners, a very
> simple early-inliner that only handles simple cases and a full-blown
> IPA inliner that contains the bug). I believe this can be ensured by
> making the wrapper always_inline and never calling it indirectly (via
> a pointer). Honza (CCed), you know inlining heuristics better, please
> correct me if my last statement is somehow inaccurate (or indeed if
> you have a better idea how kernel developers can make sure they do not
> hit the bug).
I guess that means that any user of this code in the kernel:
static inline __attribute_const__ __u64 __fswab64(__u64 val)
{
#ifdef __HAVE_BUILTIN_BSWAP64__
return __builtin_bswap64(val);
#elif defined (__arch_swab64)
return __arch_swab64(val);
#elif defined(__SWAB_64_THRU_32__)
__u32 h = val >> 32;
__u32 l = val & ((1ULL << 32) - 1);
return (((__u64)__fswab32(l)) << 32) | ((__u64)(__fswab32(h)));
#else
return ___constant_swab64(val);
#endif
}
#define __swab64(x) \
(__builtin_constant_p((__u64)(x)) ? \
___constant_swab64(x) : \
__fswab64(x))
static __always_inline __u64 __swab64p(const __u64 *p)
{
#ifdef __arch_swab64p
return __arch_swab64p(p);
#else
return __swab64(*p);
#endif
}
has a chance of running into the same problem, and we may want to solve
it at the root. For architectures that define __HAVE_BUILTIN_BSWAP64__
(i.e. ARM, MIPS, POWERPC, S390, and x86 with gcc-4.4 or higher, 4.8
for __HAVE_BUILTIN_BSWAP16__), we can probably just change the logic
to avoid __b
[PATCH, RFT] byteswap: try to avoid __builtin_constant_p gcc bug
This is another attempt to avoid a regression in wwn_to_u64()
after that started using get_unaligned_be64(), which in turn
ran into a bug on gcc-4.9 through 6.1.
As part of the problem is how __builtin_constant_p gets evaluated
on an argument passed by reference into an inline function, this
avoids the use of __builtin_constant_p() for all architectures
that set CONFIG_ARCH_USE_BUILTIN_BSWAP. Most architectures do not
set ARCH_SUPPORTS_OPTIMIZED_INLINING, which means they probably
do not suffer from the problem in the qla2xxx driver, but they
might still run into it elsewhere.
I have not been able to reproduce the original problem, so I don't
know if this patch solves it, but at least it leads to simpler
code doing the same thing, so at least there should be no downsides.
Please test.
Signed-off-by: Arnd Bergmann
diff --git a/include/uapi/linux/swab.h b/include/uapi/linux/swab.h
index 3f10e5317b46..de56fd54428d 100644
--- a/include/uapi/linux/swab.h
+++ b/include/uapi/linux/swab.h
@@ -45,9 +45,7 @@
static inline __attribute_const__ __u16 __fswab16(__u16 val)
{
-#ifdef __HAVE_BUILTIN_BSWAP16__
- return __builtin_bswap16(val);
-#elif defined (__arch_swab16)
+#if defined (__arch_swab16)
return __arch_swab16(val);
#else
return ___constant_swab16(val);
@@ -56,9 +54,7 @@ static inline __attribute_const__ __u16 __fswab16(__u16 val)
static inline __attribute_const__ __u32 __fswab32(__u32 val)
{
-#ifdef __HAVE_BUILTIN_BSWAP32__
- return __builtin_bswap32(val);
-#elif defined(__arch_swab32)
+#if defined(__arch_swab32)
return __arch_swab32(val);
#else
return ___constant_swab32(val);
@@ -67,9 +63,7 @@ static inline __attribute_const__ __u32 __fswab32(__u32 val)
static inline __attribute_const__ __u64 __fswab64(__u64 val)
{
-#ifdef __HAVE_BUILTIN_BSWAP64__
- return __builtin_bswap64(val);
-#elif defined (__arch_swab64)
+#if defined (__arch_swab64)
return __arch_swab64(val);
#elif defined(__SWAB_64_THRU_32__)
__u32 h = val >> 32;
@@ -102,28 +96,40 @@ static inline __attribute_const__ __u32 __fswahb32(__u32
val)
* __swab16 - return a byteswapped 16-bit value
* @x: value to byteswap
*/
+#ifdef __HAVE_BUILTIN_BSWAP16__
+#define __swab16(x) __builtin_bswap16((__u16)(x))
+#else
#define __swab16(x)\
(__builtin_constant_p((__u16)(x)) ? \
___constant_swab16(x) : \
__fswab16(x))
+#endif
/**
* __swab32 - return a byteswapped 32-bit value
* @x: value to byteswap
*/
+#ifdef __HAVE_BUILTIN_BSWAP32__
+#define __swab32(x) __builtin_bswap32((__u32)(x))
+#else
#define __swab32(x)\
(__builtin_constant_p((__u32)(x)) ? \
___constant_swab32(x) : \
__fswab32(x))
+#endif
/**
* __swab64 - return a byteswapped 64-bit value
* @x: value to byteswap
*/
+#ifdef __HAVE_BUILTIN_BSWAP64__
+#define __swab64(x) __builtin_bswap64((__u64)(x))
+#else
#define __swab64(x)\
(__builtin_constant_p((__u64)(x)) ? \
___constant_swab64(x) : \
__fswab64(x))
+#endif
/**
* __swahw32 - return a word-swapped 32-bit value
--
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: fc: force inlining of wwn conversion functions
On Thursday 28 April 2016 10:58:43 Chris Metcalf wrote: > (Resending as text/plain) > > On 4/27/2016 5:34 PM, Arnd Bergmann wrote: > > This won't help on TILE, which is the one architecture that sets > > ARCH_SUPPORTS_OPTIMIZED_INLINING but does not set ARCH_USE_BUILTIN_BSWAP. > > Chris Metcalf should be able to figure out whether we can just > > set ARCH_USE_BUILTIN_BSWAP for tile as well. > > We certainly could enable ARCH_USE_BUILTIN_BSWAP. The only problem is > that we never added explicit support for bswap16() in gcc, which is > efficiently done on tilegx via the "revbytes" instruction and a 48-bit > right-shift. So gcc instead does a generic thing with four > instructions in three bundles, so really not as good as our asm/swab.h. > > I'm not sure how to weigh the implications of converting to > builtin_bswap16 (and possibly upstreaming a better implementation to > gcc), vs. disabling ARCH_SUPPORTS_OPTIMIZED_INLINING (which no one > else but x86 uses anyway), vs. just ignoring the compiler bug and > hoping it's not an issue in practice How about figuring out whether you hit the gcc bug on tile as a first step? Another idea would be to adapt this section in include/linux/compiler-gcc.h: #if !defined(CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING) ||\ !defined(CONFIG_OPTIMIZE_INLINING) || (__GNUC__ < 4) #define inline inline __attribute__((always_inline)) notrace #define __inline__ __inline__ __attribute__((always_inline)) notrace #define __inline__inline__attribute__((always_inline)) notrace #else /* A lot of inline functions can cause havoc with function tracing */ #define inline inline notrace #define __inline__ __inline__ notrace #define __inline__inlinenotrace #endif to work around the issue. We already check for gcc before 4.0, and we could also check for the affected releases (4.9, 5.x, 6.1) in the same place, possibly conditional on ARCH_USE_BUILTIN_BSWAP with a comment pointing to the gcc bug tracker. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] byteswap: try to avoid __builtin_constant_p gcc bug
This is another attempt to avoid a regression in wwn_to_u64() after
that started using get_unaligned_be64(), which in turn ran into a
bug on gcc-4.9 through 6.1.
The regression got introduced due to the combination of two
separate workarounds (e3bde9568d99 and ef3fb2422ffe) that each
try to sidestep distinct problems with gcc behavior (code growth
and increased stack usage). Unfortunately after both have been
applied, a more series gcc bug has been uncovered, leading to
incorrect object code that discards part of a function and
causes undefined behavior.
As part of this problem is how __builtin_constant_p gets evaluated
on an argument passed by reference into an inline function, this
avoids the use of __builtin_constant_p() for all architectures
that set CONFIG_ARCH_USE_BUILTIN_BSWAP. Most architectures do not
set ARCH_SUPPORTS_OPTIMIZED_INLINING, which means they probably
do not suffer from the problem in the qla2xxx driver, but they
might still run into it elsewhere.
Both of the original workarounds were only merged in the 4.6 kernel,
and the bug that is fixed by this patch should only appear if
both are there, so we probably don't need to backport the fix.
On the other hand, it works by simplifying the code path and
should not have any negative effects.
Link: https://lkml.org/lkml/headers/2016/4/12/1103
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66122
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70232
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646
Fixes: e3bde9568d99 ("include/linux/unaligned: force inlining of byteswap
operations")
Fixes: ef3fb2422ffe ("scsi: fc: use get/put_unaligned64 for wwn access")
Tested-by: Josh Poimboeuf # on gcc-5.3
Tested-by: Quinn Tran
Reviewed-by: Josh Poimboeuf
Signed-off-by: Arnd Bergmann
This contains the extra cast to fix up 64-bit builds, and has
an expanded changelog, compared to the original version.
diff --git a/include/uapi/linux/swab.h b/include/uapi/linux/swab.h
index 3f10e5317b46..de56fd54428d 100644
--- a/include/uapi/linux/swab.h
+++ b/include/uapi/linux/swab.h
@@ -45,9 +45,7 @@
static inline __attribute_const__ __u16 __fswab16(__u16 val)
{
-#ifdef __HAVE_BUILTIN_BSWAP16__
- return __builtin_bswap16(val);
-#elif defined (__arch_swab16)
+#if defined (__arch_swab16)
return __arch_swab16(val);
#else
return ___constant_swab16(val);
@@ -56,9 +54,7 @@ static inline __attribute_const__ __u16 __fswab16(__u16 val)
static inline __attribute_const__ __u32 __fswab32(__u32 val)
{
-#ifdef __HAVE_BUILTIN_BSWAP32__
- return __builtin_bswap32(val);
-#elif defined(__arch_swab32)
+#if defined(__arch_swab32)
return __arch_swab32(val);
#else
return ___constant_swab32(val);
@@ -67,9 +63,7 @@ static inline __attribute_const__ __u32 __fswab32(__u32 val)
static inline __attribute_const__ __u64 __fswab64(__u64 val)
{
-#ifdef __HAVE_BUILTIN_BSWAP64__
- return __builtin_bswap64(val);
-#elif defined (__arch_swab64)
+#if defined (__arch_swab64)
return __arch_swab64(val);
#elif defined(__SWAB_64_THRU_32__)
__u32 h = val >> 32;
@@ -102,28 +96,40 @@ static inline __attribute_const__ __u32 __fswahb32(__u32
val)
* __swab16 - return a byteswapped 16-bit value
* @x: value to byteswap
*/
+#ifdef __HAVE_BUILTIN_BSWAP16__
+#define __swab16(x) __builtin_bswap16((__u16)(x))
+#else
#define __swab16(x)\
(__builtin_constant_p((__u16)(x)) ? \
___constant_swab16(x) : \
__fswab16(x))
+#endif
/**
* __swab32 - return a byteswapped 32-bit value
* @x: value to byteswap
*/
+#ifdef __HAVE_BUILTIN_BSWAP32__
+#define __swab32(x) __builtin_bswap32((__u32)(x))
+#else
#define __swab32(x)\
(__builtin_constant_p((__u32)(x)) ? \
___constant_swab32(x) : \
__fswab32(x))
+#endif
/**
* __swab64 - return a byteswapped 64-bit value
* @x: value to byteswap
*/
+#ifdef __HAVE_BUILTIN_BSWAP64__
+#define __swab64(x) (__u64)__builtin_bswap64((__u64)(x))
+#else
#define __swab64(x)\
(__builtin_constant_p((__u64)(x)) ? \
___constant_swab64(x) : \
__fswab64(x))
+#endif
/**
* __swahw32 - return a word-swapped 32-bit value
--
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] byteswap: try to avoid __builtin_constant_p gcc bug
On Monday 02 May 2016 16:02:18 Andrew Morton wrote: > On Mon, 02 May 2016 23:48:19 +0200 Arnd Bergmann wrote: > > > This is another attempt to avoid a regression in wwn_to_u64() after > > that started using get_unaligned_be64(), which in turn ran into a > > bug on gcc-4.9 through 6.1. > > I'm still getting a couple screenfuls of things like > > net/tipc/name_distr.c: In function 'tipc_named_process_backlog': > net/tipc/name_distr.c:330: warning: format '%u' expects type 'unsigned int', > but argument 3 has type 'unsigned int' > net/tipc/name_distr.c:330: warning: format '%u' expects type 'unsigned int', > but argument 4 has type 'unsigned int' > net/tipc/name_distr.c:330: warning: format '%u' expects type 'unsigned int', > but argument 5 has type 'unsigned int' > net/tipc/name_distr.c:330: warning: format '%u' expects type 'unsigned int', > but argument 7 has type 'unsigned int' I've built a few thousand kernels (arm32 with gcc-6.1) with the patch applied, but didn't see this one. What target architecture and compiler version produced this? Does it go away if you add a (__u32) cast? I don't even know what the warning is trying to tell me. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] byteswap: try to avoid __builtin_constant_p gcc bug
On Monday 02 May 2016 16:32:25 Andrew Morton wrote: > On Tue, 03 May 2016 01:10:16 +0200 Arnd Bergmann wrote: > > > On Monday 02 May 2016 16:02:18 Andrew Morton wrote: > > > On Mon, 02 May 2016 23:48:19 +0200 Arnd Bergmann wrote: > > > > > > > This is another attempt to avoid a regression in wwn_to_u64() after > > > > that started using get_unaligned_be64(), which in turn ran into a > > > > bug on gcc-4.9 through 6.1. > > > > > > I'm still getting a couple screenfuls of things like > > > > > > net/tipc/name_distr.c: In function 'tipc_named_process_backlog': > > > net/tipc/name_distr.c:330: warning: format '%u' expects type 'unsigned > > > int', but argument 3 has type 'unsigned int' > > > net/tipc/name_distr.c:330: warning: format '%u' expects type 'unsigned > > > int', but argument 4 has type 'unsigned int' > > > net/tipc/name_distr.c:330: warning: format '%u' expects type 'unsigned > > > int', but argument 5 has type 'unsigned int' > > > net/tipc/name_distr.c:330: warning: format '%u' expects type 'unsigned > > > int', but argument 7 has type 'unsigned int' > > > > I've built a few thousand kernels (arm32 with gcc-6.1) with the patch > > applied, > > but didn't see this one. What target architecture and compiler version > > produced > > this? Does it go away if you add a (__u32) cast? I don't even know what the > > warning is trying to tell me. > > heh, I didn't actually read it. > > Hopefully we can write this off as a gcc-4.4.4 glitch. 4.8.4 is OK. Ah, old compiler. I've tried gcc-4.3 now on ARM, and I don't get this warning (just a lot "may be used uninitialized"), but unlike gcc-4.4, my version doesn't actually get into the code path I have changed because __builtin_bswap32 was only introduced with 4.4. I don't have gcc-4.4 and 4.5 here, but the warning does show up with 4.6, 4.7 and 4.8: drivers/soc/sunxi/sunxi_sram.c: In function ‘sunxi_sram_show’: drivers/soc/sunxi/sunxi_sram.c:103:7: warning: format ‘%x’ expects argument of type ‘unsigned int’, but argument 3 has type ‘unsigned int’ [-Wformat=] 4.8 is probably still common enough that we should try to address this. This change addresses the problem for me with ARM gcc-4.8, but adding two more type casts. This also makes the 16/32/64 bit swaps all look the same. I would expect this to also have the same effect on 4.4. Please fold into the previous patch. Signed-off-by: Arnd Bergmann diff --git a/include/uapi/linux/swab.h b/include/uapi/linux/swab.h index d737804af181..8f3a8f606fd9 100644 --- a/include/uapi/linux/swab.h +++ b/include/uapi/linux/swab.h @@ -97,7 +97,7 @@ static inline __attribute_const__ __u32 __fswahb32(__u32 val) * @x: value to byteswap */ #ifdef __HAVE_BUILTIN_BSWAP16__ -#define __swab16(x) __builtin_bswap16((__u16)(x)) +#define __swab16(x) (__u16)__builtin_bswap16((__u16)(x)) #else #define __swab16(x)\ (__builtin_constant_p((__u16)(x)) ? \ @@ -110,7 +110,7 @@ static inline __attribute_const__ __u32 __fswahb32(__u32 val) * @x: value to byteswap */ #ifdef __HAVE_BUILTIN_BSWAP32__ -#define __swab32(x) __builtin_bswap32((__u32)(x)) +#define __swab32(x) (__u32)__builtin_bswap32((__u32)(x)) #else #define __swab32(x)\ (__builtin_constant_p((__u32)(x)) ? \ -- 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 00/15] Linux-3.10 ARM randconfig fixes
Hi subsystem maintainers, These are a few patches left over from doing randconfig tests a couple of weeks ago. Please apply them directly into your trees unless you see problems. All patches can theoretically be seen as bug fixes for 3.10, but they are not critical, so applying them for 3.11 is fine as well. Arnd Arnd Bergmann (15): irqdomain: export irq_domain_add_simple mtd: omap2: allow bulding as a module drm/nouveau: use mdelay instead of large udelay constants [SCSI] nsp32: use mdelay instead of large udelay constants hwrng: bcm2835: fix MODULE_LICENSE tag cpuidle: calxeda: select ARM_CPU_SUSPEND cpufreq: spear needs cpufreq table thermal: cpu_cooling: fix stub function drm: always provide debugfs function prototypes drm/tilcd: select BACKLIGHT_LCD_SUPPORT iwlegacy: il_pm_ops is only provided for PM_SLEEP [media] davinci: vpfe_capture needs i2c [media] omap3isp: include linux/mm_types.h clk: tegra: provide tegra_periph_reset_assert alternative OF: remove #ifdef from linux/of_platform.h drivers/char/hw_random/bcm2835-rng.c | 2 +- drivers/cpufreq/Kconfig.arm| 1 + drivers/cpuidle/Kconfig| 1 + drivers/gpu/drm/nouveau/core/engine/disp/dacnv50.c | 3 ++- drivers/gpu/drm/tilcdc/Kconfig | 1 + drivers/media/platform/davinci/Kconfig | 3 +++ drivers/media/platform/omap3isp/ispqueue.h | 1 + drivers/mtd/nand/Kconfig | 2 +- drivers/net/wireless/iwlegacy/common.h | 6 +++--- drivers/scsi/nsp32.c | 2 +- include/drm/drmP.h | 3 +-- include/linux/clk/tegra.h | 5 + include/linux/cpu_cooling.h| 4 ++-- include/linux/of_platform.h| 14 +++--- kernel/irq/irqdomain.c | 1 + 15 files changed, 27 insertions(+), 22 deletions(-) Cc: "James E.J. Bottomley" Cc: Artem Bityutskiy Cc: Dave Airlie Cc: David Woodhouse Cc: Herbert Xu Cc: John W. Linville Cc: Laurent Pinchart Cc: Mauro Carvalho Chehab Cc: Rafael J. Wysocki Cc: Rob Clark Cc: Rob Herring Cc: Russell King Cc: Stephen Warren Cc: Thomas Gleixner Cc: Viresh Kumar Cc: cpuf...@vger.kernel.org Cc: dri-de...@lists.freedesktop.org Cc: linux-me...@vger.kernel.org Cc: linux-...@lists.infradead.org Cc: linux...@vger.kernel.org Cc: linux-rpi-ker...@lists.infradead.org Cc: linux-scsi@vger.kernel.org -- 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
[PATCH 04/15] [SCSI] nsp32: use mdelay instead of large udelay constants
ARM cannot handle udelay for more than 2 miliseconds, so we
should use mdelay instead for those.
Signed-off-by: Arnd Bergmann
Acked-by: GOTO Masanori
Cc: YOKOTA Hiroshi
Cc: "James E.J. Bottomley"
Cc: linux-scsi@vger.kernel.org
---
drivers/scsi/nsp32.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/nsp32.c b/drivers/scsi/nsp32.c
index 1e3879d..0665f9c 100644
--- a/drivers/scsi/nsp32.c
+++ b/drivers/scsi/nsp32.c
@@ -2899,7 +2899,7 @@ static void nsp32_do_bus_reset(nsp32_hw_data *data)
* reset SCSI bus
*/
nsp32_write1(base, SCSI_BUS_CONTROL, BUSCTL_RST);
- udelay(RESET_HOLD_TIME);
+ mdelay(RESET_HOLD_TIME / 1000);
nsp32_write1(base, SCSI_BUS_CONTROL, 0);
for(i = 0; i < 5; i++) {
intrdat = nsp32_read2(base, IRQ_STATUS); /* dummy read */
--
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] scsi, be2iscsi, LLVMLinux: Add missing MODULE_DEVICE_TABLE()
On Wednesday 28 January 2015 17:37:02 Behan Webster wrote: > Missing MODULE_DEVICE_TABLE for pci ids from be2iscsi driver found by clang. > > Signed-off-by: Behan Webster > Reviewed-by: Mark Charlebois > Suggested-by: Arnd Bergmann > Cc: Arnd Bergmann > --- Like one previous patch, the change is good, the description is wrong. This patch removes a duplicate MODULE_DEVICE_TABLE entry. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mpt3sas: Remove usage of 'struct timeval'
On Wednesday 04 February 2015 10:15:12 Tina Ruchandani wrote: > 'struct timeval' will have its tv_sec value overflow on 32-bit systems > in year 2038 and beyond. This patch replaces the use of struct timeval > for computing mpi_request.TimeStamp, and instead uses ktime_t which provides > 64-bit seconds value. The timestamp computed remains unaffected (milliseconds > since Unix epoch). > > Signed-off-by: Tina Ruchandani > Same change as for mpt2sas, so Acked-by: Arnd Bergmann -- 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] mpt2sas: Remove usage of 'struct timeval'
On Wednesday 04 February 2015 11:28:35 Tina Ruchandani wrote: > 'struct timeval' will have its tv_sec value overflow on 32-bit systems > in year 2038 and beyond. This patch replaces the use of struct timeval > for computing mpi_request.TimeStamp, and instead uses ktime_t which provides > 64-bit seconds value. The timestamp computed remains unaffected (milliseconds > since Unix epoch). > > Signed-off-by: Tina Ruchandani > Looks good to me, Acked-by: Arnd Bergmann -- 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: Remove usage of struct timeval
On Wednesday 04 February 2015 08:39:54 Tina Ruchandani wrote: > struct timeval will have its tv_sec field overflow on 32-bit systems > in year 2038 and beyond. This patch removes the usage of struct timeval > and instead uses 64-bit ktime_t to get the current milliseconds > to populate pmcraid_timestamp_data. > > Signed-off-by: Tina Ruchandani The change you did looks correct, but I see two problems: - The subject line should mention the name of the driver you change, as you are not doing this for the entire scsi subsystem at once. > @@ -5569,11 +5570,9 @@ static void pmcraid_set_timestamp(struct pmcraid_cmd > *cmd) > __be32 time_stamp_len = cpu_to_be32(PMCRAID_TIMESTAMP_LEN); > struct pmcraid_ioadl_desc *ioadl = ioarcb->add_data.u.ioadl; > > - struct timeval tv; > __le64 timestamp; > > - do_gettimeofday(&tv); > - timestamp = tv.tv_sec * 1000; > + timestamp = ktime_to_ms(ktime_get_real()); > > pinstance->timestamp_data->timestamp[0] = (__u8)(timestamp); > pinstance->timestamp_data->timestamp[1] = (__u8)((timestamp) >> 8); It looks like a preexisting bug here, but it makes sense to fix it at the same time and describe that change in the patch changelog: timestamp is declared as an __le64 type but gets set to a cpu-endian value from tv.tv_sec or ktime_to_ms, which is wrong. The code indeed gets the endianess right by copying the bytes individually, so I think you should just use a u64 type here. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: Remove use of struct timeval
On Wednesday 04 February 2015 08:34:48 Tina Ruchandani wrote: > Function stex_gettime uses 'struct timeval' whose tv_sec value > will overflow on 32-bit systems in year 2038 and beyond. This patch > replaces the use of struct timeval and do_gettimeofday with > ktime_get_real_seconds, which returns a 64-bit seconds value. > > Signed-off-by: Tina Ruchandani > Acked-by: Arnd Bergmann The patch looks good, but please send it again with a proper subject that mentions the name of the driver. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [SCSI] bfa: Remove use of struct timeval
On Wednesday 04 February 2015 08:42:03 Tina Ruchandani wrote: > struct timeval will have its tv_sec field overflow on 32-bit systems > in year 2038 and beyond. This patch removes the usage of struct timeval > and instead uses ktime_get_real_seconds() which returns 64-bit wall-clock > seconds. > > Signed-off-by: Tina Ruchandani > The patch is correct, but you can do it better by removing the bfa_get_log_time() function completely and changing the four callers to call ktime_get_real_seconds() directly. The subject line could be improved too: you don't remember all uses of timeval in this patch, but only one of them. When you do the first change, a good subject line would be: [SCSI] bfa: replace bfa_get_log_time with ktime_get_real_seconds Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] scsi: pmcraid: Remove usage of struct timeval
On Wednesday 25 February 2015 07:49:21 Tina Ruchandani wrote: > struct timeval will have its tv_sec field overflow on 32-bit systems > in year 2038 and beyond. This patch removes the usage of struct timeval > and instead uses 64-bit ktime_t to get the current milliseconds > to populate pmcraid_timestamp_data. > > Signed-off-by: Tina Ruchandani > Reviewed-by: Arnd Bergmann I notice that this patch also increases the accuracy of the timestamp from seconds to milliseconds, where the original code ignores the sub-second portion of the timeval. Doing this will make the the function a little slower, but I have verified that it is only called for probe, reset and config-change, all of which are not timing critical. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] qla2xxx: Remove use of 'struct timeval'
On Wednesday 25 February 2015 07:45:12 Tina Ruchandani wrote: > struct register_host_info stores a 64-bit UTC system time timestamp. > This patch removes the use of 'struct timeval' to obtain that timestamp > as its tv_sec value will overflow on 32-bit systems in year 2038 beyond. > The patch uses ktime_get_real_seconds() which returns a 64-bit seconds value. > > Signed-off-by: Tina Ruchandani Reviewed-by: Tina Ruchandani -- 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] bfa: Replace bfa_get_log_time()
On Wednesday 25 February 2015 07:44:39 Tina Ruchandani wrote: > struct timeval will have its tv_sec field overflow on 32-bit systems > in year 2038 and beyond. This patch removes bfa_get_log_time which > uses struct timeval, and instead replaces calls to it with > calls to ktime_get_real_seconds() which returns 64-bit wall-clock > seconds. > > Signed-off-by: Tina Ruchandani > Suggested-by: Arnd Bergmann > -- > Changes in v2: > - Removed call to bfa_get_log_time entirely, instead of retaining > it and making it call ktime_get_real_seconds internally. > - Made subject line more clear. Nice! Reviewed-by: Arnd Bergmann -- 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: stex: Remove use of struct timeval
On Wednesday 25 February 2015 07:41:36 Tina Ruchandani wrote:
> @@ -364,10 +365,7 @@ MODULE_VERSION(ST_DRIVER_VERSION);
>
> static void stex_gettime(__le64 *time)
> {
> - struct timeval tv;
> -
> - do_gettimeofday(&tv);
> - *time = cpu_to_le64(tv.tv_sec);
> + *time = cpu_to_le64(ktime_get_real_seconds());
> }
>
> static struct status_msg *stex_get_status(struct st_hba *hba)
>
The patch looks correct to me, but upon second reading, I think it
would be nicer to eliminate the function entirely and open-code
the cpu_to_le64(ktime_get_real_seconds()) in the two callers.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [SCSI] mvumi: 64bit value for seconds_since1970
On Wednesday 25 February 2015 07:43:54 Tina Ruchandani wrote: > struct mvumi_hs_page2 stores a "seconds_since1970" field which is of > type u64. It is however, written to, using 'struct timeval' which has > a 32-bit seconds field and whose value will overflow in year 2038. > This patch uses ktime_get_real_seconds() instead since it provides a > 64-bit seconds value, which is 2038 safe. > > Signed-off-by: Tina Ruchandani Reviewed-by: Arnd Bergmann I note that in this case, the bug exists and is fixed by your patch on both 32-bit and 64-bit kernels. I am unsure whether it is correct that this driver along with 3w-9xxx.c and 3w-sas.c, but unlike all others, uses local time instead of UTC, but your patch does not change that. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] ata: ahci_xgene: Add AHCI Support for second generation of APM X-Gene SoC
On Monday 13 April 2015 15:02:40 Suman Tripathi wrote:
> This patch enables full AHCI feature support for APM X-Gene SoC SATA host host
> controller. The following errata's are removed:
>
> 1. 2a0bdff6b95 ("ahci-xgene: fix the dma state machine lockup for the
> IDENTIFY DEVICE PIO mode command")
> 2. 09c32aaa368 ("ahci_xgene: Fix the dma state machine lockup for the
> ATA_CMD_SMART PIO mode command")
> 3. 1540035da71 ("ahci_xgene: Implement the xgene_ahci_poll_reg_val to
> support PMP")
> 4. a3a84bc7c88 ("ahci_xgene: Implement the workaround to support PMP
> enumeration and discovery")
> 5. 1102407bb71 ("ahci_xgene: Fix the DMA state machine lockup for the
> ATA_CMD_PACKET PIO mode command")
> 6. 72f79f9e35b ("ahci_xgene: Removing NCQ support from the APM X-Gene SoC
> AHCI SATA Host Controller driver")
>
> In addition, enable PMP support for APM X-Gene SoC and enable FBS support
> for second generation APM X-Gene SoC.
>
Is this now close enough to AHCI that you can just use the normal AHCI
driver, or are there still too many remaining errata?
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 00/21] more arm build fixes
Hi subsystem maintainers, Here is another set of patches that resulted from build testing on linux-next. Please apply directly into your trees if you agree, or let me know if I made a mistake. I can take whatever remains through the arm-soc tree if you prefer that or I don't hear back. Arnd Arnd Bergmann (21): ARM: topology: export cpu_topology ARM: default machine descriptor for multiplatform ARM: shmobile: don't call irqchip_init unconditionally ARM: orion5x: include linux/cpu.h atm: he: use mdelay instead of large udelay constants ALSA: ali5451: use mdelay instead of large udelay constants oss/dmabuf: use dma_map_single drm/nouveau: use mdelay instead of large udelay constants drm: export drm_vm_open_locked [SCSI] nsp32: use mdelay instead of large udelay constants irqdomain: export irq_domain_add_simple irqchip: s3c24xx: add missing __init annotations iommu: tegra: print dma_addr_t using %lld cpufreq: pxa2xx: initialize variables thermal: cooling: avoid uninitialied used gcc warning OF: remove #ifdef from linux/of_platform.h X.509: do not emit any informational output USB: ehci-msm: USB_MSM_OTG needs USB_PHY USB: lpc32xx: ISP1301 needs USB_PHY USB: OMAP: ISP1301 needs USB_PHY USB: OHCI: avoid conflicting platform drivers arch/arm/Kconfig | 1 - arch/arm/configs/lpc32xx_defconfig | 1 + arch/arm/configs/msm_defconfig | 1 + arch/arm/configs/omap1_defconfig | 1 + arch/arm/kernel/devtree.c | 7 ++ arch/arm/kernel/setup.c| 11 +- arch/arm/kernel/topology.c | 1 + arch/arm/mach-orion5x/common.c | 1 + arch/arm/mach-shmobile/intc-r8a7740.c | 13 +- drivers/atm/he.c | 2 +- drivers/cpufreq/pxa2xx-cpufreq.c | 5 +- drivers/gpu/drm/drm_vm.c | 1 + drivers/gpu/drm/nouveau/core/engine/disp/dacnv50.c | 3 +- drivers/iommu/tegra-gart.c | 3 +- drivers/iommu/tegra-smmu.c | 2 +- drivers/irqchip/irq-s3c24xx.c | 4 +- drivers/scsi/nsp32.c | 2 +- drivers/thermal/cpu_cooling.c | 17 +-- drivers/usb/gadget/Kconfig | 2 + drivers/usb/host/Kconfig | 1 + drivers/usb/host/ohci-hcd.c| 136 ++--- drivers/usb/phy/Makefile | 2 +- include/linux/of_platform.h| 13 +- kernel/irq/irqdomain.c | 1 + lib/build_OID_registry | 2 - sound/oss/dmabuf.c | 3 +- sound/pci/ali5451/ali5451.c| 8 +- 27 files changed, 187 insertions(+), 57 deletions(-) -- 1.8.1.2 Cc: "James E.J. Bottomley" Cc: Benjamin Herrenschmidt Cc: David Airlie Cc: Felipe Balbi Cc: Grant Likely Cc: Greg Kroah-Hartman Cc: Inki Dae Cc: Jason Cooper Cc: Joerg Roedel Cc: Nicolas Pitre Cc: Rafael J. Wysocki Cc: Rob Herring Cc: Russell King Cc: Simon Horman Cc: Thomas Gleixner Cc: Viresh Kumar Cc: Will Deacon Cc: alsa-de...@alsa-project.org Cc: dri-de...@lists.freedesktop.org Cc: linux-atm-gene...@lists.sourceforge.net Cc: linux-scsi@vger.kernel.org Cc: linux-...@vger.kernel.org Cc: net...@vger.kernel.org -- 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 10/21] [SCSI] nsp32: use mdelay instead of large udelay constants
ARM cannot handle udelay for more than 2 miliseconds, so we
should use mdelay instead for those.
Signed-off-by: Arnd Bergmann
Cc: GOTO Masanori
Cc: YOKOTA Hiroshi
Cc: "James E.J. Bottomley"
Cc: linux-scsi@vger.kernel.org
---
drivers/scsi/nsp32.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/nsp32.c b/drivers/scsi/nsp32.c
index 1e3879d..0665f9c 100644
--- a/drivers/scsi/nsp32.c
+++ b/drivers/scsi/nsp32.c
@@ -2899,7 +2899,7 @@ static void nsp32_do_bus_reset(nsp32_hw_data *data)
* reset SCSI bus
*/
nsp32_write1(base, SCSI_BUS_CONTROL, BUSCTL_RST);
- udelay(RESET_HOLD_TIME);
+ mdelay(RESET_HOLD_TIME / 1000);
nsp32_write1(base, SCSI_BUS_CONTROL, 0);
for(i = 0; i < 5; i++) {
intrdat = nsp32_read2(base, IRQ_STATUS); /* dummy read */
--
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 10/21] [SCSI] nsp32: use mdelay instead of large udelay constants
On Friday 26 April 2013, Masanori Goto wrote: > 2013/4/25 Arnd Bergmann > > > > ARM cannot handle udelay for more than 2 miliseconds, so we > > should use mdelay instead for those. > > > > Singed-off-by: GOTO Masanori Thanks. I assume you mean "Acked-by", not "Singed-off" as in "burnt" or "Signed-off" as a notification that you have applied it to your own git tree. I'll keep the patch with an your "Acked-by" line in my tree unless James wants to apply to the scsi tree. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mpt3sas: add PCI dependency for CONFIG_SCSI_MPT2SAS
CONFIG_SCSI_MPT2SAS was added as a backwards-compatibility helper that
selects the replacement SCSI_MPT3SAS symbol, but lacks the dependencies:
warning: (SCSI_MPT2SAS) selects SCSI_MPT3SAS which has unmet direct
dependencies (SCSI_LOWLEVEL && PCI && SCSI)
0x7E5F9A79 Fri Dec 4 12:36:08 CET 2015 failed
drivers/scsi/mpt3sas/mpt3sas_base.c: In function 'mpt3sas_remove_dead_ioc_func':
drivers/scsi/mpt3sas/mpt3sas_base.c:140:2: error: implicit declaration of
function 'pci_stop_and_remove_bus_device_locked'
[-Werror=implicit-function-declaration]
drivers/scsi/mpt3sas/mpt3sas_base.c: In function '_base_disable_msix':
drivers/scsi/mpt3sas/mpt3sas_base.c:1921:2: error: implicit declaration of
function 'pci_disable_msix' [-Werror=implicit-function-declaration]
This adds the same dependencies that SCSI_MPT3SAS has.
Signed-off-by: Arnd Bergmann
Fixes: b840c3627b6f ("mpt3sas: Add dummy Kconfig option for backwards
compatibility")
---
This appeared on today's linux-next with ARM randconfig builds
diff --git a/drivers/scsi/mpt3sas/Kconfig b/drivers/scsi/mpt3sas/Kconfig
index 25dc38f25ec6..33dc427cfe9a 100644
--- a/drivers/scsi/mpt3sas/Kconfig
+++ b/drivers/scsi/mpt3sas/Kconfig
@@ -74,6 +74,7 @@ config SCSI_MPT3SAS_MAX_SGE
config SCSI_MPT2SAS
tristate "Legacy MPT2SAS config option"
+ depends on PCI && SCSI
default n
select SCSI_MPT3SAS
---help---
--
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] mpt3sas: add PCI dependency for CONFIG_SCSI_MPT2SAS
On Friday 04 December 2015 08:28:51 James Bottomley wrote: > On Fri, 2015-12-04 at 15:27 +0100, Arnd Bergmann wrote: > > CONFIG_SCSI_MPT2SAS was added as a backwards-compatibility helper that > > selects the replacement SCSI_MPT3SAS symbol, but lacks the dependencies: > > > > warning: (SCSI_MPT2SAS) selects SCSI_MPT3SAS which has unmet direct > > dependencies (SCSI_LOWLEVEL && PCI && SCSI) > > 0x7E5F9A79 Fri Dec 4 12:36:08 CET 2015 failed > > drivers/scsi/mpt3sas/mpt3sas_base.c: In function > > 'mpt3sas_remove_dead_ioc_func': > > drivers/scsi/mpt3sas/mpt3sas_base.c:140:2: error: implicit declaration of > > function 'pci_stop_and_remove_bus_device_locked' > > [-Werror=implicit-function-declaration] > > drivers/scsi/mpt3sas/mpt3sas_base.c: In function '_base_disable_msix': > > drivers/scsi/mpt3sas/mpt3sas_base.c:1921:2: error: implicit declaration of > > function 'pci_disable_msix' [-Werror=implicit-function-declaration] > > > > This adds the same dependencies that SCSI_MPT3SAS has. > > OK, you're about the fifth person to complain about this and this patch > was posted a few days ago and is now here: > > http://git.kernel.org/cgit/linux/kernel/git/jejb/scsi.git/commit/?h=fixes&id=3ddda3e4c82dea58933bde8d0f6ef34470c360cb > > It's even been in for-next for nearly 24h I only found the bug on today on the latest linux-next, which should have contained it if you pushed out the branch in time, but it's not in there. 2015-12-03 16:33:05 6e3ac04845fb "Merge branch 'fixes' into for-next" 2015-12-03 21:44:54 de84dfc24842 "Merge remote-tracking branch 'scsi/for-next'" I'm sure it will be there in the next next. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH, RESEND 2] qla2xxx: Remove use of 'struct timeval'
struct register_host_info stores a 64-bit UTC system time timestamp. This patch removes the use of 'struct timeval' to obtain that timestamp as its tv_sec value will overflow on 32-bit systems in year 2038 beyond. The patch uses ktime_get_real_seconds() which returns a 64-bit seconds value. Signed-off-by: Tina Ruchandani Reviewed-by: Johannes Thumshirn Signed-off-by: Arnd Bergmann diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c index b5029e543b91..15dff7099955 100644 --- a/drivers/scsi/qla2xxx/qla_mr.c +++ b/drivers/scsi/qla2xxx/qla_mr.c @@ -6,6 +6,7 @@ */ #include "qla_def.h" #include +#include #include #include #include @@ -1812,7 +1813,6 @@ qlafx00_fx_disc(scsi_qla_host_t *vha, fc_port_t *fcport, uint16_t fx_type) struct host_system_info *phost_info; struct register_host_info *preg_hsi; struct new_utsname *p_sysid = NULL; - struct timeval tv; sp = qla2x00_get_sp(vha, fcport, GFP_KERNEL); if (!sp) @@ -1886,8 +1886,7 @@ qlafx00_fx_disc(scsi_qla_host_t *vha, fc_port_t *fcport, uint16_t fx_type) p_sysid->domainname, DOMNAME_LENGTH); strncpy(phost_info->hostdriver, QLA2XXX_VERSION, VERSION_LENGTH); - do_gettimeofday(&tv); - preg_hsi->utc = (uint64_t)tv.tv_sec; + preg_hsi->utc = (uint64_t)ktime_get_real_seconds(); ql_dbg(ql_dbg_init, vha, 0x0149, "ISP%04X: Host registration with firmware\n", ha->pdev->device); -- 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: [Y2038] [PATCH, RESEND 2] qla2xxx: Remove use of 'struct timeval'
On Friday 22 January 2016 15:28:32 Arnd Bergmann wrote: > struct register_host_info stores a 64-bit UTC system time timestamp. > This patch removes the use of 'struct timeval' to obtain that timestamp > as its tv_sec value will overflow on 32-bit systems in year 2038 beyond. > The patch uses ktime_get_real_seconds() which returns a 64-bit seconds value. > > Signed-off-by: Tina Ruchandani > Reviewed-by: Johannes Thumshirn > Signed-off-by: Arnd Bergmann > I was missing From: Tina Ruchandani If anyone is going to pick this up, let me know whether you will fix it up yourself or if I should resend. -- 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, RESEND 3] qla2xxx: Remove use of 'struct timeval'
From: Tina Ruchandani struct register_host_info stores a 64-bit UTC system time timestamp. This patch removes the use of 'struct timeval' to obtain that timestamp as its tv_sec value will overflow on 32-bit systems in year 2038 beyond. The patch uses ktime_get_real_seconds() which returns a 64-bit seconds value. Signed-off-by: Tina Ruchandani Reviewed-by: Johannes Thumshirn Acked-by: Himanshu Madhani Signed-off-by: Arnd Bergmann --- Resent with correct author field and added Ack diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c index b5029e543b91..15dff7099955 100644 --- a/drivers/scsi/qla2xxx/qla_mr.c +++ b/drivers/scsi/qla2xxx/qla_mr.c @@ -6,6 +6,7 @@ */ #include "qla_def.h" #include +#include #include #include #include @@ -1812,7 +1813,6 @@ qlafx00_fx_disc(scsi_qla_host_t *vha, fc_port_t *fcport, uint16_t fx_type) struct host_system_info *phost_info; struct register_host_info *preg_hsi; struct new_utsname *p_sysid = NULL; - struct timeval tv; sp = qla2x00_get_sp(vha, fcport, GFP_KERNEL); if (!sp) @@ -1886,8 +1886,7 @@ qlafx00_fx_disc(scsi_qla_host_t *vha, fc_port_t *fcport, uint16_t fx_type) p_sysid->domainname, DOMNAME_LENGTH); strncpy(phost_info->hostdriver, QLA2XXX_VERSION, VERSION_LENGTH); - do_gettimeofday(&tv); - preg_hsi->utc = (uint64_t)tv.tv_sec; + preg_hsi->utc = (uint64_t)ktime_get_real_seconds(); ql_dbg(ql_dbg_init, vha, 0x0149, "ISP%04X: Host registration with firmware\n", ha->pdev->device); -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/7] mptfusion: hide unused seq_mpt_print_ioc_summary function
The seq_mpt_print_ioc_summary function is used for the /proc/mpt/iocN/summary
implementation and never gets called when CONFIG_PROC_FS is disabled:
drivers/message/fusion/mptbase.c:6851:13: warning: 'seq_mpt_print_ioc_summary'
defined but not used [-Wunused-function]
static void seq_mpt_print_ioc_summary(MPT_ADAPTER *ioc, struct seq_file *m,
int showlan)
This adds an #ifdef to hide the function definition in that case
and avoid the warning.
Signed-off-by: Arnd Bergmann
---
drivers/message/fusion/mptbase.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 5dcc0313c38a..207370d68c17 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -6848,6 +6848,7 @@ mpt_print_ioc_summary(MPT_ADAPTER *ioc, char *buffer, int
*size, int len, int sh
*size = y;
}
+#ifdef CONFIG_PROC_FS
static void seq_mpt_print_ioc_summary(MPT_ADAPTER *ioc, struct seq_file *m,
int showlan)
{
char expVer[32];
@@ -6879,6 +6880,7 @@ static void seq_mpt_print_ioc_summary(MPT_ADAPTER *ioc,
struct seq_file *m, int
seq_putc(m, '\n');
}
+#endif
/**
* mpt_set_taskmgmt_in_progress_flag - set flags associated with task
management
--
2.7.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
