Re: [PATCH V14 1/2] scsi: ufs: set the device reference clock setting
>+static struct ufs_ref_clk ufs_ref_clk_freqs[] = {
>+ {1920, REF_CLK_FREQ_19_2_MHZ},
>+ {2600, REF_CLK_FREQ_26_MHZ},
>+ {3840, REF_CLK_FREQ_38_4_MHZ},
>+ {5200, REF_CLK_FREQ_52_MHZ},
>+ {0, REF_CLK_FREQ_INVAL},
>+};
>+
>+static inline enum ufs_ref_clk_freq
>+ufs_get_bref_clk_from_hz(u32 freq)
>+{
>+ int i = 0;
>+
>+ while (ufs_ref_clk_freqs[i].freq_hz != freq) {
>+ if (!ufs_ref_clk_freqs[i].freq_hz)
>+ return REF_CLK_FREQ_INVAL;
Is the if clause really needed?
you will return REF_CLK_FREQ_INVAL anyway
>+ i++;
You might overrun here if freq is not what you've expected
>+ }
>+
>+ return ufs_ref_clk_freqs[i].val;
>+}
Re: [PATCH 0/3] scsi: ufs-qcom: Remove all direct calls to qcom-ufs phy
Hi all, On Tue, Sep 4, 2018 at 3:47 PM Vivek Gautam wrote: > > Cleaning up the ufs-qcom host further to remove all direct calls > into qcom-ufs driver. > Only phy-qcom-ufs-qmp-20nm phy handles these direct calls from ufs host > and this phy is not used in any supported qcom platform in current kernel. > So, while we free up the host from all the ufs_qcom_phy_*() API calls > we should declare 20nm phy as broken. > For this we fork out couple of configs from PHY_QCOM_UFS - > PHY_QCOM_UFS_14NM and PHY_QCOM_UFS_20NM out of which we declare > PHY_QCOM_UFS_20NM as 'broken'. > > This series helps in a clean use of ufs phy support for sdm845 > and further SoCs that will also use phy-qcom-qmp phy driver. Gentle ping, any inputs on this series? Thanks Vivek > > Vivek Gautam (3): > phy: qcom-ufs: Remove stale methods that handle ref clk > scsi/ufs: qcom: Remove ufs_qcom_phy_*() calls from host > phy: qcom-ufs: Declare 20nm qcom ufs qmp phy as Broken > > drivers/phy/qualcomm/Kconfig | 17 > drivers/phy/qualcomm/Makefile | 4 +-- > drivers/phy/qualcomm/phy-qcom-ufs-i.h | 2 +- > drivers/phy/qualcomm/phy-qcom-ufs.c | 50 > --- > drivers/scsi/ufs/ufs-qcom.c | 28 +--- > drivers/scsi/ufs/ufs-qcom.h | 5 > include/linux/phy/phy-qcom-ufs.h | 38 -- > 7 files changed, 21 insertions(+), 123 deletions(-) > delete mode 100644 include/linux/phy/phy-qcom-ufs.h > > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation > -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH V14 1/2] scsi: ufs: set the device reference clock setting
On 9/24/2018 1:28 PM, Avri Altman wrote:
+static struct ufs_ref_clk ufs_ref_clk_freqs[] = {
+ {1920, REF_CLK_FREQ_19_2_MHZ},
+ {2600, REF_CLK_FREQ_26_MHZ},
+ {3840, REF_CLK_FREQ_38_4_MHZ},
+ {5200, REF_CLK_FREQ_52_MHZ},
+ {0, REF_CLK_FREQ_INVAL},
+};
+
+static inline enum ufs_ref_clk_freq
+ufs_get_bref_clk_from_hz(u32 freq)
+{
+ int i = 0;
+
+ while (ufs_ref_clk_freqs[i].freq_hz != freq) {
+ if (!ufs_ref_clk_freqs[i].freq_hz)
+ return REF_CLK_FREQ_INVAL;
Is the if clause really needed?
you will return REF_CLK_FREQ_INVAL anyway
Yes. the if condition makes sure to return REF_CLK_FREQ_INVAL if freq is
not what we expect.
+ i++;
You might overrun here if freq is not what you've expected
Above if condition "if (!ufs_ref_clk_freqs[i].freq_hz)" prevents such
overrun
as we will reach end of ufs_ref_clk_freqs[] (i.e upto 0,
REF_CLK_FREQ_INVAL) when there is no match and thus we will return
REF_CLK_FREQ_INVAL.
+ }
+
+ return ufs_ref_clk_freqs[i].val;
+}
Re: [PATCH V14 2/2] scsi: ufs: Add configfs support for UFS provisioning
> obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
>obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
>ufshcd-core-objs := ufshcd.o ufs-sysfs.o
>+obj-$(CONFIG_SCSI_UFS_PROVISION) += ufs-configfs.o
Isn't ufs-configfs should be part of ufshcd-core? like ufs-sysfs ?
>+static ssize_t ufs_config_desc_show(struct config_item *item, char *buf,
>+ u8 index)
>+{
The read part already exist in ufs-sysfs.
>+ssize_t ufshcd_desc_configfs_store(struct config_item *item, const char *buf,
>+ size_t count, u8 index)
>+{
>+
>+ /*
>+* First read the current configuration descriptor
>+* and then update with user provided parameters
>+*/
if originally only lun0 was configured, and you want to configure a new set of
luns -
luns 8 to 15 (config index 0x1) - won't the read fail in that case?
Thanks,
Avri
[PATCH 4.18 229/235] blk-mq: avoid to synchronize rcu inside blk_cleanup_queue()
4.18-stable review patch. If anyone has any objections, please let me know.
--
From: Ming Lei
[ Upstream commit 1311326cf4755c7ffefd20f576144ecf46d9906b ]
SCSI probing may synchronously create and destroy a lot of request_queues
for non-existent devices. Any synchronize_rcu() in queue creation or
destroy path may introduce long latency during booting, see detailed
description in comment of blk_register_queue().
This patch removes one synchronize_rcu() inside blk_cleanup_queue()
for this case, commit c2856ae2f315d75(blk-mq: quiesce queue before freeing
queue)
needs synchronize_rcu() for implementing blk_mq_quiesce_queue(), but
when queue isn't initialized, it isn't necessary to do that since
only pass-through requests are involved, no original issue in
scsi_execute() at all.
Without this patch and previous one, it may take more 20+ seconds for
virtio-scsi to complete disk probe. With the two patches, the time becomes
less than 100ms.
Fixes: c2856ae2f315d75 ("blk-mq: quiesce queue before freeing queue")
Reported-by: Andrew Jones
Cc: Omar Sandoval
Cc: Bart Van Assche
Cc: linux-scsi@vger.kernel.org
Cc: "Martin K. Petersen"
Cc: Christoph Hellwig
Tested-by: Andrew Jones
Signed-off-by: Ming Lei
Signed-off-by: Jens Axboe
Signed-off-by: Sasha Levin
Signed-off-by: Greg Kroah-Hartman
---
block/blk-core.c |8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -791,9 +791,13 @@ void blk_cleanup_queue(struct request_qu
* make sure all in-progress dispatch are completed because
* blk_freeze_queue() can only complete all requests, and
* dispatch may still be in-progress since we dispatch requests
-* from more than one contexts
+* from more than one contexts.
+*
+* No need to quiesce queue if it isn't initialized yet since
+* blk_freeze_queue() should be enough for cases of passthrough
+* request.
*/
- if (q->mq_ops)
+ if (q->mq_ops && blk_queue_init_done(q))
blk_mq_quiesce_queue(q);
/* for synchronous bio-based driver finish in-flight integrity i/o */
[PATCH 4.14 155/173] blk-mq: avoid to synchronize rcu inside blk_cleanup_queue()
4.14-stable review patch. If anyone has any objections, please let me know.
--
From: Ming Lei
[ Upstream commit 1311326cf4755c7ffefd20f576144ecf46d9906b ]
SCSI probing may synchronously create and destroy a lot of request_queues
for non-existent devices. Any synchronize_rcu() in queue creation or
destroy path may introduce long latency during booting, see detailed
description in comment of blk_register_queue().
This patch removes one synchronize_rcu() inside blk_cleanup_queue()
for this case, commit c2856ae2f315d75(blk-mq: quiesce queue before freeing
queue)
needs synchronize_rcu() for implementing blk_mq_quiesce_queue(), but
when queue isn't initialized, it isn't necessary to do that since
only pass-through requests are involved, no original issue in
scsi_execute() at all.
Without this patch and previous one, it may take more 20+ seconds for
virtio-scsi to complete disk probe. With the two patches, the time becomes
less than 100ms.
Fixes: c2856ae2f315d75 ("blk-mq: quiesce queue before freeing queue")
Reported-by: Andrew Jones
Cc: Omar Sandoval
Cc: Bart Van Assche
Cc: linux-scsi@vger.kernel.org
Cc: "Martin K. Petersen"
Cc: Christoph Hellwig
Tested-by: Andrew Jones
Signed-off-by: Ming Lei
Signed-off-by: Jens Axboe
Signed-off-by: Sasha Levin
Signed-off-by: Greg Kroah-Hartman
---
block/blk-core.c |8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -669,9 +669,13 @@ void blk_cleanup_queue(struct request_qu
* make sure all in-progress dispatch are completed because
* blk_freeze_queue() can only complete all requests, and
* dispatch may still be in-progress since we dispatch requests
-* from more than one contexts
+* from more than one contexts.
+*
+* No need to quiesce queue if it isn't initialized yet since
+* blk_freeze_queue() should be enough for cases of passthrough
+* request.
*/
- if (q->mq_ops)
+ if (q->mq_ops && blk_queue_init_done(q))
blk_mq_quiesce_queue(q);
/* for synchronous bio-based driver finish in-flight integrity i/o */
[PATCH 6/7] scsi: hisi_sas: Use block layer tag instead for IPTT
From: Xiang Chen
Currently we use the IPTT defined in LLDD to identify IOs. Actually for
IOs which are from the block layer, they have tags to identify them. So
for those IOs, use tag of the block layer directly, and for IOs which is
not from the block layer (such as internal IOs from libsas/LLDD), reserve
96 IPTTs for them.
Signed-off-by: Xiang Chen
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas.h | 3 +-
drivers/scsi/hisi_sas/hisi_sas_main.c | 89 ++
drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 1 -
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 9 ++--
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 8 +--
5 files changed, 70 insertions(+), 40 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index 6c7d2e2..0ddb53c 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -34,6 +34,7 @@
#define HISI_SAS_MAX_DEVICES HISI_SAS_MAX_ITCT_ENTRIES
#define HISI_SAS_RESET_BIT 0
#define HISI_SAS_REJECT_CMD_BIT1
+#define HISI_SAS_RESERVED_IPTT_CNT 96
#define HISI_SAS_STATUS_BUF_SZ (sizeof(struct hisi_sas_status_buffer))
#define HISI_SAS_COMMAND_TABLE_SZ (sizeof(union hisi_sas_command_table))
@@ -217,7 +218,7 @@ struct hisi_sas_hw {
int (*hw_init)(struct hisi_hba *hisi_hba);
void (*setup_itct)(struct hisi_hba *hisi_hba,
struct hisi_sas_device *device);
- int (*slot_index_alloc)(struct hisi_hba *hisi_hba, int *slot_idx,
+ int (*slot_index_alloc)(struct hisi_hba *hisi_hba,
struct domain_device *device);
struct hisi_sas_device *(*alloc_dev)(struct domain_device *device);
void (*sl_notify)(struct hisi_hba *hisi_hba, int phy_no);
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 416f2c0..8fcd1ab 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -183,7 +183,14 @@ static void hisi_sas_slot_index_clear(struct hisi_hba
*hisi_hba, int slot_idx)
static void hisi_sas_slot_index_free(struct hisi_hba *hisi_hba, int slot_idx)
{
- hisi_sas_slot_index_clear(hisi_hba, slot_idx);
+ unsigned long flags;
+
+ if (hisi_hba->hw->slot_index_alloc || (slot_idx >=
+ hisi_hba->hw->max_command_entries - HISI_SAS_RESERVED_IPTT_CNT)) {
+ spin_lock_irqsave(&hisi_hba->lock, flags);
+ hisi_sas_slot_index_clear(hisi_hba, slot_idx);
+ spin_unlock_irqrestore(&hisi_hba->lock, flags);
+ }
}
static void hisi_sas_slot_index_set(struct hisi_hba *hisi_hba, int slot_idx)
@@ -193,24 +200,34 @@ static void hisi_sas_slot_index_set(struct hisi_hba
*hisi_hba, int slot_idx)
set_bit(slot_idx, bitmap);
}
-static int hisi_sas_slot_index_alloc(struct hisi_hba *hisi_hba, int *slot_idx)
+static int hisi_sas_slot_index_alloc(struct hisi_hba *hisi_hba,
+struct scsi_cmnd *scsi_cmnd)
{
- unsigned int index;
+ int index;
void *bitmap = hisi_hba->slot_index_tags;
+ unsigned long flags;
+ if (scsi_cmnd)
+ return scsi_cmnd->request->tag;
+
+ spin_lock_irqsave(&hisi_hba->lock, flags);
index = find_next_zero_bit(bitmap, hisi_hba->slot_index_count,
- hisi_hba->last_slot_index + 1);
+ hisi_hba->last_slot_index + 1);
if (index >= hisi_hba->slot_index_count) {
- index = find_next_zero_bit(bitmap, hisi_hba->slot_index_count,
- 0);
- if (index >= hisi_hba->slot_index_count)
+ index = find_next_zero_bit(bitmap,
+ hisi_hba->slot_index_count,
+ hisi_hba->hw->max_command_entries -
+ HISI_SAS_RESERVED_IPTT_CNT);
+ if (index >= hisi_hba->slot_index_count) {
+ spin_unlock_irqrestore(&hisi_hba->lock, flags);
return -SAS_QUEUE_FULL;
+ }
}
hisi_sas_slot_index_set(hisi_hba, index);
- *slot_idx = index;
hisi_hba->last_slot_index = index;
+ spin_unlock_irqrestore(&hisi_hba->lock, flags);
- return 0;
+ return index;
}
static void hisi_sas_slot_index_init(struct hisi_hba *hisi_hba)
@@ -249,9 +266,7 @@ void hisi_sas_slot_task_free(struct hisi_hba *hisi_hba,
struct sas_task *task,
memset(slot, 0, offsetof(struct hisi_sas_slot, buf));
- spin_lock_irqsave(&hisi_hba->lock, flags);
hisi_sas_slot_index_free(hisi_hba, slot->idx);
- spin_unlock_irqrestore(&hisi_hba->lock, flags);
}
EXPORT_SYMBOL_GPL(hisi_sas_slot_task_free);
@@ -384,16 +399,27 @@ static int hisi_sas_task_prep(struct sas_task *task,
goto err_out_dma_unmap;
}
- spin_lock_irqsave(&hisi_hba->lock
[PATCH 1/7] scsi: hisi_sas: Feed back linkrate(max/min) when re-attached
From: Luo Jiaxing At directly attached situation, if the user modifies the sysfs interface of maximum_linkrate and minimum_linkrate to renegotiate the linkrate between SAS controller and target, the value of both files mentioned above should have change to user setting after renegotiate is over, but it remain unchanged. To fix this bug, maximum_linkrate and minimum_linkrate will be directly fed back to relevant sas_phy structure. Signed-off-by: Luo Jiaxing Signed-off-by: John Garry --- drivers/scsi/hisi_sas/hisi_sas_main.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c index a4e2e6a..ba6fb535 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_main.c +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c @@ -904,6 +904,9 @@ static void hisi_sas_phy_set_linkrate(struct hisi_hba *hisi_hba, int phy_no, _r.maximum_linkrate = max; _r.minimum_linkrate = min; + sas_phy->phy->maximum_linkrate = max; + sas_phy->phy->minimum_linkrate = min; + hisi_hba->hw->phy_disable(hisi_hba, phy_no); msleep(100); hisi_hba->hw->phy_set_linkrate(hisi_hba, phy_no, &_r); -- 1.9.1
[PATCH 7/7] scsi: hisi_sas: Update v3 hw AIP_LIMIT and CFG_AGING_TIME register values
From: Xiang Chen
Update registers as follows:
- Default value of AIP timer is 1ms, and it is easy for some expanders to
cause IO error. Change the value to max value 65ms to avoid IO error for
those expanders.
- A CQ completion will be reported by HW when 4 CQs have occurred or the
aging timer expires, whichever happens first. Sor serial IO scenario, it
will still wait 8us for every IO before it is reported. So in the
situation, the performance is poor. So to improve it, change the limit
time to the least value.
For other scenario, it does little affect to the performance.
Signed-off-by: Xiang Chen
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index f30c4e4..bd4ce38 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -127,6 +127,7 @@
#define PHY_CTRL_RESET_OFF 0
#define PHY_CTRL_RESET_MSK (0x1 << PHY_CTRL_RESET_OFF)
#define SL_CFG (PORT_BASE + 0x84)
+#define AIP_LIMIT (PORT_BASE + 0x90)
#define SL_CONTROL (PORT_BASE + 0x94)
#define SL_CONTROL_NOTIFY_EN_OFF 0
#define SL_CONTROL_NOTIFY_EN_MSK (0x1 << SL_CONTROL_NOTIFY_EN_OFF)
@@ -431,6 +432,7 @@ static void init_reg_v3_hw(struct hisi_hba *hisi_hba)
(u32)((1ULL << hisi_hba->queue_count) - 1));
hisi_sas_write32(hisi_hba, CFG_MAX_TAG, 0xfff0400);
hisi_sas_write32(hisi_hba, HGC_SAS_TXFAIL_RETRY_CTRL, 0x108);
+ hisi_sas_write32(hisi_hba, CFG_AGING_TIME, 0x1);
hisi_sas_write32(hisi_hba, INT_COAL_EN, 0x1);
hisi_sas_write32(hisi_hba, OQ_INT_COAL_TIME, 0x1);
hisi_sas_write32(hisi_hba, OQ_INT_COAL_CNT, 0x1);
@@ -495,6 +497,7 @@ static void init_reg_v3_hw(struct hisi_hba *hisi_hba)
/* used for 12G negotiate */
hisi_sas_phy_write32(hisi_hba, i, COARSETUNE_TIME, 0x1e);
+ hisi_sas_phy_write32(hisi_hba, i, AIP_LIMIT, 0x2);
}
for (i = 0; i < hisi_hba->queue_count; i++) {
--
1.9.1
[PATCH 5/7] scsi: hisi_sas: unmask interrupts ent72 and ent74
From: Xiang Chen The interrupts of ent72 and ent74 are not processed by PCIe AER handling, so we need to unmask the interrupts and process them first in the driver. Signed-off-by: Xiang Chen Signed-off-by: John Garry --- drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c index 3995ff6..34c8f30 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c @@ -441,7 +441,7 @@ static void init_reg_v3_hw(struct hisi_hba *hisi_hba) hisi_sas_write32(hisi_hba, ENT_INT_SRC_MSK1, 0xfefefefe); hisi_sas_write32(hisi_hba, ENT_INT_SRC_MSK2, 0xfefefefe); if (pdev->revision >= 0x21) - hisi_sas_write32(hisi_hba, ENT_INT_SRC_MSK3, 0x7fff); + hisi_sas_write32(hisi_hba, ENT_INT_SRC_MSK3, 0x7aff); else hisi_sas_write32(hisi_hba, ENT_INT_SRC_MSK3, 0xfffe20ff); hisi_sas_write32(hisi_hba, CHNL_PHYUPDOWN_INT_MSK, 0x0); -- 1.9.1
[PATCH 4/7] scsi: hisi_sas: Free slot later in slot_complete_vx_hw()
From: Xiang Chen
If an SSP/SMP IO times out, it may be actually in reality be
simultaneously processing completion of the slot in slot_complete_vx_hw().
Then if the slot is freed in slot_complete_vx_hw() (this IPTT is freed and
it may be re-used by other slot), and we may abort the wrong slot in
hisi_sas_abort_task().
So to solve the issue, free the slot after the check of
SAS_TASK_STATE_ABORTED in slot_complete_vx_hw().
Signed-off-by: Xiang Chen
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 2 +-
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index 9c5c5a6..67134b4 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -2483,7 +2483,6 @@ static void slot_err_v2_hw(struct hisi_hba *hisi_hba,
}
out:
- hisi_sas_slot_task_free(hisi_hba, task, slot);
sts = ts->stat;
spin_lock_irqsave(&task->task_state_lock, flags);
if (task->task_state_flags & SAS_TASK_STATE_ABORTED) {
@@ -2493,6 +2492,7 @@ static void slot_err_v2_hw(struct hisi_hba *hisi_hba,
}
task->task_state_flags |= SAS_TASK_STATE_DONE;
spin_unlock_irqrestore(&task->task_state_lock, flags);
+ hisi_sas_slot_task_free(hisi_hba, task, slot);
if (!is_internal && (task->task_proto != SAS_PROTOCOL_SMP)) {
spin_lock_irqsave(&device->done_lock, flags);
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 08b503e2..3995ff6 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -1751,7 +1751,6 @@ static irqreturn_t fatal_axi_int_v3_hw(int irq_no, void
*p)
}
out:
- hisi_sas_slot_task_free(hisi_hba, task, slot);
sts = ts->stat;
spin_lock_irqsave(&task->task_state_lock, flags);
if (task->task_state_flags & SAS_TASK_STATE_ABORTED) {
@@ -1761,6 +1760,7 @@ static irqreturn_t fatal_axi_int_v3_hw(int irq_no, void
*p)
}
task->task_state_flags |= SAS_TASK_STATE_DONE;
spin_unlock_irqrestore(&task->task_state_lock, flags);
+ hisi_sas_slot_task_free(hisi_hba, task, slot);
if (!is_internal && (task->task_proto != SAS_PROTOCOL_SMP)) {
spin_lock_irqsave(&device->done_lock, flags);
--
1.9.1
[PATCH 3/7] scsi: hisi_sas: Fix the race between IO completion and timeout for SMP/internal IO
From: Xiang Chen
If SMP/internal IO times out, we will possibly free the task immediately.
However if the IO actually completes at the same time, the IO completion
may refer to task which have been freed.
So to solve the issue, flush the tasklet to finish IO completion before
free'ing slot/task.
Signed-off-by: Xiang Chen
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas_main.c | 55 +--
1 file changed, 46 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 3b95a7a..416f2c0 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -956,8 +956,7 @@ static int hisi_sas_control_phy(struct asd_sas_phy
*sas_phy, enum phy_func func,
static void hisi_sas_task_done(struct sas_task *task)
{
- if (!del_timer(&task->slow_task->timer))
- return;
+ del_timer(&task->slow_task->timer);
complete(&task->slow_task->completion);
}
@@ -966,13 +965,17 @@ static void hisi_sas_tmf_timedout(struct timer_list *t)
struct sas_task_slow *slow = from_timer(slow, t, timer);
struct sas_task *task = slow->task;
unsigned long flags;
+ bool is_completed = true;
spin_lock_irqsave(&task->task_state_lock, flags);
- if (!(task->task_state_flags & SAS_TASK_STATE_DONE))
+ if (!(task->task_state_flags & SAS_TASK_STATE_DONE)) {
task->task_state_flags |= SAS_TASK_STATE_ABORTED;
+ is_completed = false;
+ }
spin_unlock_irqrestore(&task->task_state_lock, flags);
- complete(&task->slow_task->completion);
+ if (!is_completed)
+ complete(&task->slow_task->completion);
}
#define TASK_TIMEOUT 20
@@ -1023,10 +1026,18 @@ static int hisi_sas_exec_internal_tmf_task(struct
domain_device *device,
if ((task->task_state_flags & SAS_TASK_STATE_ABORTED)) {
if (!(task->task_state_flags & SAS_TASK_STATE_DONE)) {
struct hisi_sas_slot *slot = task->lldd_task;
+ struct hisi_sas_cq *cq =
+ &hisi_hba->cq[slot->dlvry_queue];
dev_err(dev, "abort tmf: TMF task timeout and
not done\n");
- if (slot)
+ if (slot) {
+ /*
+* flush tasklet to avoid free'ing task
+* before using task in IO completion
+*/
+ tasklet_kill(&cq->tasklet);
slot->task = NULL;
+ }
goto ex_err;
} else
@@ -1402,6 +1413,17 @@ static int hisi_sas_abort_task(struct sas_task *task)
spin_lock_irqsave(&task->task_state_lock, flags);
if (task->task_state_flags & SAS_TASK_STATE_DONE) {
+ struct hisi_sas_slot *slot = task->lldd_task;
+ struct hisi_sas_cq *cq;
+
+ if (slot) {
+ /*
+* flush tasklet to avoid free'ing task
+* before using task in IO completion
+*/
+ cq = &hisi_hba->cq[slot->dlvry_queue];
+ tasklet_kill(&cq->tasklet);
+ }
spin_unlock_irqrestore(&task->task_state_lock, flags);
rc = TMF_RESP_FUNC_COMPLETE;
goto out;
@@ -1457,12 +1479,19 @@ static int hisi_sas_abort_task(struct sas_task *task)
/* SMP */
struct hisi_sas_slot *slot = task->lldd_task;
u32 tag = slot->idx;
+ struct hisi_sas_cq *cq = &hisi_hba->cq[slot->dlvry_queue];
rc = hisi_sas_internal_task_abort(hisi_hba, device,
HISI_SAS_INT_ABT_CMD, tag);
if (((rc < 0) || (rc == TMF_RESP_FUNC_FAILED)) &&
- task->lldd_task)
- hisi_sas_do_release_task(hisi_hba, task, slot);
+ task->lldd_task) {
+ /*
+* flush tasklet to avoid free'ing task
+* before using task in IO completion
+*/
+ tasklet_kill(&cq->tasklet);
+ slot->task = NULL;
+ }
}
out:
@@ -1828,9 +1857,17 @@ static int hisi_sas_query_task(struct sas_task *task)
if ((task->task_state_flags & SAS_TASK_STATE_ABORTED)) {
if (!(task->task_state_flags & SAS_TASK_STATE_DONE)) {
struct hisi_sas_slot *slot = task->lldd_task;
-
- if
[PATCH 2/7] scsi: hisi_sas: Move evaluation of hisi_hba in hisi_sas_task_prep()
From: Luo Jiaxing
In evaluating hisi_hba, the sas_port may be NULL, so for safety relocate
the the check to value possible NULL deference.
Signed-off-by: Luo Jiaxing
Signed-off-by: John Garry
---
drivers/scsi/hisi_sas/hisi_sas_main.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index ba6fb535..3b95a7a 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -287,13 +287,13 @@ static int hisi_sas_task_prep(struct sas_task *task,
int *pass)
{
struct domain_device *device = task->dev;
- struct hisi_hba *hisi_hba = dev_to_hisi_hba(device);
+ struct hisi_hba *hisi_hba;
struct hisi_sas_device *sas_dev = device->lldd_dev;
struct hisi_sas_port *port;
struct hisi_sas_slot *slot;
struct hisi_sas_cmd_hdr *cmd_hdr_base;
struct asd_sas_port *sas_port = device->port;
- struct device *dev = hisi_hba->dev;
+ struct device *dev;
int dlvry_queue_slot, dlvry_queue, rc, slot_idx;
int n_elem = 0, n_elem_req = 0, n_elem_resp = 0;
struct hisi_sas_dq *dq;
@@ -314,6 +314,9 @@ static int hisi_sas_task_prep(struct sas_task *task,
return -ECOMM;
}
+ hisi_hba = dev_to_hisi_hba(device);
+ dev = hisi_hba->dev;
+
if (DEV_IS_GONE(sas_dev)) {
if (sas_dev)
dev_info(dev, "task prep: device %d not ready\n",
--
1.9.1
[PATCH 0/7] hisi_sas: Misc bugfixes and an optimisation patch
This patchset introduces mostly more minor/obscure bugfixes for the driver. Also included is an optimisation to use the block layer tag for the IPTT indexing. This quite a nice optimisation as it means we don't have to evaluate this in the driver - it was a bit of a bottle-neck. However it does block us in future from enabling SCSI MQ in the driver. This is because the IPTT index must be a unique value per HBA. However, if we switched to SCSI MQ, the block layer tag becomes unique per queue, and not per HBA. Having said this, testing has shown that performance is better by using this block layer tag instead of enabling SCSI MQ in the driver. Luo Jiaxing (2): scsi: hisi_sas: Feed back linkrate(max/min) when re-attached scsi: hisi_sas: Move evaluation of hisi_hba in hisi_sas_task_prep() Xiang Chen (5): scsi: hisi_sas: Fix the race between IO completion and timeout for SMP/internal IO scsi: hisi_sas: Free slot later in slot_complete_vx_hw() scsi: hisi_sas: unmask interrupts ent72 and ent74 scsi: hisi_sas: Use block layer tag instead for IPTT scsi: hisi_sas: Update v3 hw AIP_LIMIT and CFG_AGING_TIME register values drivers/scsi/hisi_sas/hisi_sas.h | 3 +- drivers/scsi/hisi_sas/hisi_sas_main.c | 154 - drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 1 - drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 11 +-- drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 15 ++-- 5 files changed, 130 insertions(+), 54 deletions(-) -- 1.9.1
Re: [PATCH 1/3] phy: qcom-ufs: Remove stale methods that handle ref clk
On Tue 04 Sep 03:17 PDT 2018, Vivek Gautam wrote:
> Remove ufs_qcom_phy_enable/(disable)_dev_ref_clk() that
> are not being used by any code.
>
> Signed-off-by: Vivek Gautam
Thanks for the ping Vivek, I didn't spot these when you posted them.
Reviewed-by: Bjorn Andersson
Regards,
Bjorn
> ---
> drivers/phy/qualcomm/phy-qcom-ufs.c | 50
> -
> include/linux/phy/phy-qcom-ufs.h| 14 ---
> 2 files changed, 64 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-ufs.c
> b/drivers/phy/qualcomm/phy-qcom-ufs.c
> index c5493ea51282..f2979ccad00a 100644
> --- a/drivers/phy/qualcomm/phy-qcom-ufs.c
> +++ b/drivers/phy/qualcomm/phy-qcom-ufs.c
> @@ -431,56 +431,6 @@ static void ufs_qcom_phy_disable_ref_clk(struct
> ufs_qcom_phy *phy)
> }
> }
>
> -#define UFS_REF_CLK_EN (1 << 5)
> -
> -static void ufs_qcom_phy_dev_ref_clk_ctrl(struct phy *generic_phy, bool
> enable)
> -{
> - struct ufs_qcom_phy *phy = get_ufs_qcom_phy(generic_phy);
> -
> - if (phy->dev_ref_clk_ctrl_mmio &&
> - (enable ^ phy->is_dev_ref_clk_enabled)) {
> - u32 temp = readl_relaxed(phy->dev_ref_clk_ctrl_mmio);
> -
> - if (enable)
> - temp |= UFS_REF_CLK_EN;
> - else
> - temp &= ~UFS_REF_CLK_EN;
> -
> - /*
> - * If we are here to disable this clock immediately after
> - * entering into hibern8, we need to make sure that device
> - * ref_clk is active atleast 1us after the hibern8 enter.
> - */
> - if (!enable)
> - udelay(1);
> -
> - writel_relaxed(temp, phy->dev_ref_clk_ctrl_mmio);
> - /* ensure that ref_clk is enabled/disabled before we return */
> - wmb();
> - /*
> - * If we call hibern8 exit after this, we need to make sure that
> - * device ref_clk is stable for atleast 1us before the hibern8
> - * exit command.
> - */
> - if (enable)
> - udelay(1);
> -
> - phy->is_dev_ref_clk_enabled = enable;
> - }
> -}
> -
> -void ufs_qcom_phy_enable_dev_ref_clk(struct phy *generic_phy)
> -{
> - ufs_qcom_phy_dev_ref_clk_ctrl(generic_phy, true);
> -}
> -EXPORT_SYMBOL_GPL(ufs_qcom_phy_enable_dev_ref_clk);
> -
> -void ufs_qcom_phy_disable_dev_ref_clk(struct phy *generic_phy)
> -{
> - ufs_qcom_phy_dev_ref_clk_ctrl(generic_phy, false);
> -}
> -EXPORT_SYMBOL_GPL(ufs_qcom_phy_disable_dev_ref_clk);
> -
> /* Turn ON M-PHY RMMI interface clocks */
> static int ufs_qcom_phy_enable_iface_clk(struct ufs_qcom_phy *phy)
> {
> diff --git a/include/linux/phy/phy-qcom-ufs.h
> b/include/linux/phy/phy-qcom-ufs.h
> index 0a2c18a9771d..9dd85071bcce 100644
> --- a/include/linux/phy/phy-qcom-ufs.h
> +++ b/include/linux/phy/phy-qcom-ufs.h
> @@ -17,20 +17,6 @@
>
> #include "phy.h"
>
> -/**
> - * ufs_qcom_phy_enable_dev_ref_clk() - Enable the device
> - * ref clock.
> - * @phy: reference to a generic phy.
> - */
> -void ufs_qcom_phy_enable_dev_ref_clk(struct phy *phy);
> -
> -/**
> - * ufs_qcom_phy_disable_dev_ref_clk() - Disable the device
> - * ref clock.
> - * @phy: reference to a generic phy.
> - */
> -void ufs_qcom_phy_disable_dev_ref_clk(struct phy *phy);
> -
> int ufs_qcom_phy_set_tx_lane_enable(struct phy *phy, u32 tx_lanes);
> void ufs_qcom_phy_save_controller_version(struct phy *phy,
> u8 major, u16 minor, u16 step);
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>
Re: [PATCH 2/3] scsi/ufs: qcom: Remove ufs_qcom_phy_*() calls from host
On Tue 04 Sep 03:17 PDT 2018, Vivek Gautam wrote:
> The host makes direct calls into phy using ufs_qcom_phy_*()
> APIs. These APIs are only defined for 20nm qcom-ufs-qmp phy
> which is not being used by any architecture as yet. Future
> architectures too are not going to use 20nm ufs phy.
> So remove these ufs_qcom_phy_*() calls from host to let further
> change declare the 20nm phy as broken.
> Also remove couple of stale enum defines for ufs phy.
>
> Signed-off-by: Vivek Gautam
Reviewed-by: Bjorn Andersson
Regards,
Bjorn
> ---
> drivers/phy/qualcomm/phy-qcom-ufs-i.h | 2 +-
> drivers/scsi/ufs/ufs-qcom.c | 28 +---
> drivers/scsi/ufs/ufs-qcom.h | 5 -
> include/linux/phy/phy-qcom-ufs.h | 24
> 4 files changed, 2 insertions(+), 57 deletions(-)
> delete mode 100644 include/linux/phy/phy-qcom-ufs.h
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-ufs-i.h
> b/drivers/phy/qualcomm/phy-qcom-ufs-i.h
> index 822c83b8efcd..681644e43248 100644
> --- a/drivers/phy/qualcomm/phy-qcom-ufs-i.h
> +++ b/drivers/phy/qualcomm/phy-qcom-ufs-i.h
> @@ -17,9 +17,9 @@
>
> #include
> #include
> +#include
> #include
> #include
> -#include
> #include
> #include
> #include
> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
> index 75ee5906b966..3dc4501c6945 100644
> --- a/drivers/scsi/ufs/ufs-qcom.c
> +++ b/drivers/scsi/ufs/ufs-qcom.c
> @@ -16,7 +16,6 @@
> #include
> #include
> #include
> -#include
>
> #include "ufshcd.h"
> #include "ufshcd-pltfrm.h"
> @@ -189,22 +188,9 @@ static int ufs_qcom_init_lane_clks(struct ufs_qcom_host
> *host)
>
> static int ufs_qcom_link_startup_post_change(struct ufs_hba *hba)
> {
> - struct ufs_qcom_host *host = ufshcd_get_variant(hba);
> - struct phy *phy = host->generic_phy;
> u32 tx_lanes;
> - int err = 0;
> -
> - err = ufs_qcom_get_connected_tx_lanes(hba, &tx_lanes);
> - if (err)
> - goto out;
>
> - err = ufs_qcom_phy_set_tx_lane_enable(phy, tx_lanes);
> - if (err)
> - dev_err(hba->dev, "%s: ufs_qcom_phy_set_tx_lane_enable
> failed\n",
> - __func__);
> -
> -out:
> - return err;
> + return ufs_qcom_get_connected_tx_lanes(hba, &tx_lanes);
> }
>
> static int ufs_qcom_check_hibern8(struct ufs_hba *hba)
> @@ -932,10 +918,8 @@ static int ufs_qcom_pwr_change_notify(struct ufs_hba
> *hba,
> {
> u32 val;
> struct ufs_qcom_host *host = ufshcd_get_variant(hba);
> - struct phy *phy = host->generic_phy;
> struct ufs_qcom_dev_params ufs_qcom_cap;
> int ret = 0;
> - int res = 0;
>
> if (!dev_req_params) {
> pr_err("%s: incoming dev_req_params is NULL\n", __func__);
> @@ -1002,12 +986,6 @@ static int ufs_qcom_pwr_change_notify(struct ufs_hba
> *hba,
> }
>
> val = ~(MAX_U32 << dev_req_params->lane_tx);
> - res = ufs_qcom_phy_set_tx_lane_enable(phy, val);
> - if (res) {
> - dev_err(hba->dev, "%s:
> ufs_qcom_phy_set_tx_lane_enable() failed res = %d\n",
> - __func__, res);
> - ret = res;
> - }
>
> /* cache the power mode parameters to use internally */
> memcpy(&host->dev_req_params,
> @@ -1264,10 +1242,6 @@ static int ufs_qcom_init(struct ufs_hba *hba)
> }
> }
>
> - /* update phy revision information before calling phy_init() */
> - ufs_qcom_phy_save_controller_version(host->generic_phy,
> - host->hw_ver.major, host->hw_ver.minor, host->hw_ver.step);
> -
> err = ufs_qcom_init_lane_clks(host);
> if (err)
> goto out_variant_clear;
> diff --git a/drivers/scsi/ufs/ufs-qcom.h b/drivers/scsi/ufs/ufs-qcom.h
> index 295f4bef6a0e..c114826316eb 100644
> --- a/drivers/scsi/ufs/ufs-qcom.h
> +++ b/drivers/scsi/ufs/ufs-qcom.h
> @@ -129,11 +129,6 @@ enum {
> MASK_CLK_NS_REG = 0xFFFC00,
> };
>
> -enum ufs_qcom_phy_init_type {
> - UFS_PHY_INIT_FULL,
> - UFS_PHY_INIT_CFG_RESTORE,
> -};
> -
> /* QCOM UFS debug print bit mask */
> #define UFS_QCOM_DBG_PRINT_REGS_EN BIT(0)
> #define UFS_QCOM_DBG_PRINT_ICE_REGS_EN BIT(1)
> diff --git a/include/linux/phy/phy-qcom-ufs.h
> b/include/linux/phy/phy-qcom-ufs.h
> deleted file mode 100644
> index 9dd85071bcce..
> --- a/include/linux/phy/phy-qcom-ufs.h
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -/*
> - * Copyright (c) 2013-2015, Linux Foundation. All rights reserved.
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 and
> - * only version 2 as published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
>
Re: [PATCH 3/3] phy: qcom-ufs: Declare 20nm qcom ufs qmp phy as Broken
On Tue 04 Sep 03:17 PDT 2018, Vivek Gautam wrote: > Fork out separate configs for 14nm and 20nm qcom ufs qmp phys > to declare the 20nm phy as broken. > > Signed-off-by: Vivek Gautam Reviewed-by: Bjorn Andersson Regards, Bjorn > --- > drivers/phy/qualcomm/Kconfig | 17 + > drivers/phy/qualcomm/Makefile | 4 ++-- > 2 files changed, 19 insertions(+), 2 deletions(-) > > diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig > index 632a0e73ee10..32f7d34eb784 100644 > --- a/drivers/phy/qualcomm/Kconfig > +++ b/drivers/phy/qualcomm/Kconfig > @@ -50,6 +50,23 @@ config PHY_QCOM_UFS > help > Support for UFS PHY on QCOM chipsets. > > +if PHY_QCOM_UFS > + > +config PHY_QCOM_UFS_14NM > + tristate > + default PHY_QCOM_UFS > + help > + Support for 14nm UFS QMP phy present on QCOM chipsets. > + > +config PHY_QCOM_UFS_20NM > + tristate > + default PHY_QCOM_UFS > + depends on BROKEN > + help > + Support for 20nm UFS QMP phy present on QCOM chipsets. > + > +endif > + > config PHY_QCOM_USB_HS > tristate "Qualcomm USB HS PHY module" > depends on USB_ULPI_BUS > diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile > index deb831f453ae..c56efd3af205 100644 > --- a/drivers/phy/qualcomm/Makefile > +++ b/drivers/phy/qualcomm/Makefile > @@ -5,7 +5,7 @@ obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA) += > phy-qcom-ipq806x-sata.o > obj-$(CONFIG_PHY_QCOM_QMP) += phy-qcom-qmp.o > obj-$(CONFIG_PHY_QCOM_QUSB2) += phy-qcom-qusb2.o > obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs.o > -obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-14nm.o > -obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-20nm.o > +obj-$(CONFIG_PHY_QCOM_UFS_14NM) += phy-qcom-ufs-qmp-14nm.o > +obj-$(CONFIG_PHY_QCOM_UFS_20NM) += phy-qcom-ufs-qmp-20nm.o > obj-$(CONFIG_PHY_QCOM_USB_HS)+= phy-qcom-usb-hs.o > obj-$(CONFIG_PHY_QCOM_USB_HSIC) += phy-qcom-usb-hsic.o > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation >
Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg
On Tue, Sep 18, 2018 at 7:59 PM Jason Gunthorpe wrote: > > On Tue, Sep 18, 2018 at 10:51:08AM -0700, Darren Hart wrote: > > On Fri, Sep 14, 2018 at 09:57:48PM +0100, Al Viro wrote: > > > On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote: > > > > > > > Acked-by: Darren Hart (VMware) > > > > > > > > As for a longer term solution, would it be possible to init fops in such > > > > a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg > > > > so we don't have to duplicate this boilerplate for every ioctl fops > > > > structure? > > > > > > Bad idea, that... Because several years down the road somebody will > > > add > > > an ioctl that takes an unsigned int for argument. Without so much as > > > looking > > > at your magical mystery macro being used to initialize file_operations. > > > > Fair, being explicit in the declaration as it is currently may be > > preferable then. > > It would be much cleaner and safer if you could arrange things to add > something like this to struct file_operations: > > long (*ptr_ioctl) (struct file *, unsigned int, void __user *); > > Where the core code automatically converts the unsigned long to the > void __user * as appropriate. > > Then it just works right always and the compiler will help address > Al's concern down the road. I think if we wanted to do this with a new file operation, the best way would be to do the copy_from_user()/copy_to_user() in the caller as well. We already do this inside of some subsystems, notably drivers/media/, and it simplifies the implementation of the ioctl handler function significantly. We obviously cannot do this in general, both because of traditional drivers that have 16-bit command codes (drivers/tty and others) and also because of drivers that by accident defined the commands incorrectly and use the wrong type or the wrong direction in the definition. Arnd
Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg
On Mon, Sep 24, 2018 at 10:18:52PM +0200, Arnd Bergmann wrote: > On Tue, Sep 18, 2018 at 7:59 PM Jason Gunthorpe wrote: > > > > On Tue, Sep 18, 2018 at 10:51:08AM -0700, Darren Hart wrote: > > > On Fri, Sep 14, 2018 at 09:57:48PM +0100, Al Viro wrote: > > > > On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote: > > > > > > > > > Acked-by: Darren Hart (VMware) > > > > > > > > > > As for a longer term solution, would it be possible to init fops in > > > > > such > > > > > a way that the compat_ioctl call defaults to > > > > > generic_compat_ioctl_ptrarg > > > > > so we don't have to duplicate this boilerplate for every ioctl fops > > > > > structure? > > > > > > > > Bad idea, that... Because several years down the road somebody > > > > will add > > > > an ioctl that takes an unsigned int for argument. Without so much as > > > > looking > > > > at your magical mystery macro being used to initialize file_operations. > > > > > > Fair, being explicit in the declaration as it is currently may be > > > preferable then. > > > > It would be much cleaner and safer if you could arrange things to add > > something like this to struct file_operations: > > > > long (*ptr_ioctl) (struct file *, unsigned int, void __user *); > > > > Where the core code automatically converts the unsigned long to the > > void __user * as appropriate. > > > > Then it just works right always and the compiler will help address > > Al's concern down the road. > > I think if we wanted to do this with a new file operation, the best > way would be to do the copy_from_user()/copy_to_user() in the caller > as well. > > We already do this inside of some subsystems, notably drivers/media/, > and it simplifies the implementation of the ioctl handler function > significantly. We obviously cannot do this in general, both because of > traditional drivers that have 16-bit command codes (drivers/tty and others) > and also because of drivers that by accident defined the commands > incorrectly and use the wrong type or the wrong direction in the > definition. That could work well, but the first idea could be done globally and mechanically, while this would require very careful per-driver investigation. Particularly if the core code has worse performance.. ie due to kmalloc calls or something. I think it would make more sense to start by having the core do the case to __user and then add another entry point to have the core do the copy_from_user, and so on. Jason
Re: [PATCH V14 2/2] scsi: ufs: Add configfs support for UFS provisioning
On Sun, Sep 23, 2018 at 11:29 PM Sayali Lokhande wrote: > > This patch adds configfs support to provision UFS device at > runtime. This feature can be primarily useful in factory or > assembly line as some devices may be required to be configured > multiple times during initial system development phase. > Configuration Descriptors can be written multiple times until > bConfigDescrLock attribute is zero. > > Configuration descriptor buffer consists of Device and Unit > descriptor configurable parameters which are parsed from vendor > specific provisioning file and then passed via configfs node at > runtime to provision ufs device. > CONFIG_CONFIGFS_FS and CONFIG_SCSI_UFS_PROVISION needs to be enabled > for using this feature. > > Usage: > 1) To read current configuration descriptor with index X >(where index X can be 0/1/2/3) : >cat /config//ufs_config_desc_X > > 2) To write configuration descriptor with index X : >echo > /config//ufs_config_desc_X > > Signed-off-by: Sayali Lokhande > --- > Documentation/ABI/testing/configfs-driver-ufs | 12 ++ > drivers/scsi/ufs/Kconfig | 10 ++ > drivers/scsi/ufs/Makefile | 1 + > drivers/scsi/ufs/ufs-configfs.c | 237 > ++ > drivers/scsi/ufs/ufshcd.c | 3 +- > drivers/scsi/ufs/ufshcd.h | 18 ++ > 6 files changed, 280 insertions(+), 1 deletion(-) > create mode 100644 Documentation/ABI/testing/configfs-driver-ufs > create mode 100644 drivers/scsi/ufs/ufs-configfs.c > Reviewed-by: Evan Green
Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg
On Mon, Sep 24, 2018 at 10:35 PM Jason Gunthorpe wrote: > On Mon, Sep 24, 2018 at 10:18:52PM +0200, Arnd Bergmann wrote: > > On Tue, Sep 18, 2018 at 7:59 PM Jason Gunthorpe wrote: > > > On Tue, Sep 18, 2018 at 10:51:08AM -0700, Darren Hart wrote: > > > > On Fri, Sep 14, 2018 at 09:57:48PM +0100, Al Viro wrote: > > > > > On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote: > > We already do this inside of some subsystems, notably drivers/media/, > > and it simplifies the implementation of the ioctl handler function > > significantly. We obviously cannot do this in general, both because of > > traditional drivers that have 16-bit command codes (drivers/tty and others) > > and also because of drivers that by accident defined the commands > > incorrectly and use the wrong type or the wrong direction in the > > definition. > > That could work well, but the first idea could be done globally and > mechanically, while this would require very careful per-driver > investigation. > > Particularly if the core code has worse performance.. ie due to > kmalloc calls or something. > > I think it would make more sense to start by having the core do the > case to __user and then add another entry point to have the core do > the copy_from_user, and so on. Having six separate callback pointers to implement a single system call seems a bit excessive though. Arnd
[PATCH v3 0/5] scsi: libsas: some code cleanups and bug fixes
I split some code cleanups and bug fixes patches from my earlier series: https://lkml.org/lkml/2018/5/28/2154 These patches are separate to the subject of the earlier series and are just small fixes. Hope it is much easier to review and test. v2: fix some typos and add reviewed-by tags. v3: change the wording suggested by Christoph Hellwig. Jason Yan (5): scsi: libsas: delete dead code in scsi_transport_sas.c scsi: libsas: make the lldd_port_deformed method optional scsi: libsas: always unregister the old device if going to discover new scsi: libsas: check the ata device status by ata_dev_enabled() scsi: libsas: fix a race condition when smp task timeout drivers/scsi/hisi_sas/hisi_sas_main.c | 9 ++--- drivers/scsi/libsas/sas_ata.c | 2 +- drivers/scsi/libsas/sas_discover.c| 2 +- drivers/scsi/libsas/sas_expander.c| 22 +- drivers/scsi/scsi_transport_sas.c | 2 -- 5 files changed, 13 insertions(+), 24 deletions(-) -- 2.14.4
[PATCH v3 5/5] scsi: libsas: fix a race condition when smp task timeout
When the lldd is processing the complete sas task in interrupt and set
the task stat as SAS_TASK_STATE_DONE, the smp timeout timer is able to
be triggered at the same time. And smp_task_timedout() will complete the
task wheter the SAS_TASK_STATE_DONE is set or not. Then the sas task may
freed before lldd end the interrupt process. Thus a use-after-free will
happen.
Fix this by calling the complete() only when SAS_TASK_STATE_DONE is not
set. And remove the check of the return value of the del_timer(). Once
the LLDD sets DONE, it must call task->done(), which will call
smp_task_done()->complete() and the task will be completed and freed
correctly.
Reported-by: chenxiang
Signed-off-by: Jason Yan
CC: John Garry
CC: Johannes Thumshirn
CC: Ewan Milne
CC: Christoph Hellwig
CC: Tomas Henzl
CC: Dan Williams
CC: Hannes Reinecke
Reviewed-by: Hannes Reinecke
Reviewed-by: John Garry
Reviewed-by: Johannes Thumshirn
---
drivers/scsi/libsas/sas_expander.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/libsas/sas_expander.c
b/drivers/scsi/libsas/sas_expander.c
index 5940d398..0d1f72752ca2 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -48,17 +48,16 @@ static void smp_task_timedout(struct timer_list *t)
unsigned long flags;
spin_lock_irqsave(&task->task_state_lock, flags);
- if (!(task->task_state_flags & SAS_TASK_STATE_DONE))
+ if (!(task->task_state_flags & SAS_TASK_STATE_DONE)) {
task->task_state_flags |= SAS_TASK_STATE_ABORTED;
+ complete(&task->slow_task->completion);
+ }
spin_unlock_irqrestore(&task->task_state_lock, flags);
-
- complete(&task->slow_task->completion);
}
static void smp_task_done(struct sas_task *task)
{
- if (!del_timer(&task->slow_task->timer))
- return;
+ del_timer(&task->slow_task->timer);
complete(&task->slow_task->completion);
}
--
2.14.4
[PATCH v3 2/5] scsi: libsas: make the lldd_port_deformed method optional
Now LLDDs have to implement lldd_port_deformed method otherwise NULL
dereference will happen. Make it optional and remove the dummy
implementation in hisi_sas.
Signed-off-by: Jason Yan
CC: John Garry
CC: Johannes Thumshirn
CC: Ewan Milne
CC: Christoph Hellwig
CC: Tomas Henzl
CC: Dan Williams
CC: Hannes Reinecke
Acked-by: John Garry
Reviewed-by: Hannes Reinecke
Reviewed-by: Johannes Thumshirn
Reviewed-by: Christoph Hellwig
---
drivers/scsi/hisi_sas/hisi_sas_main.c | 9 ++---
drivers/scsi/libsas/sas_discover.c| 2 +-
2 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index a4e2e6aa9a6b..1975c9266978 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -1861,10 +1861,6 @@ static void hisi_sas_port_formed(struct asd_sas_phy
*sas_phy)
hisi_sas_port_notify_formed(sas_phy);
}
-static void hisi_sas_port_deformed(struct asd_sas_phy *sas_phy)
-{
-}
-
static int hisi_sas_write_gpio(struct sas_ha_struct *sha, u8 reg_type,
u8 reg_index, u8 reg_count, u8 *write_data)
{
@@ -1954,10 +1950,9 @@ static struct sas_domain_function_template
hisi_sas_transport_ops = {
.lldd_I_T_nexus_reset = hisi_sas_I_T_nexus_reset,
.lldd_lu_reset = hisi_sas_lu_reset,
.lldd_query_task= hisi_sas_query_task,
- .lldd_clear_nexus_ha = hisi_sas_clear_nexus_ha,
+ .lldd_clear_nexus_ha= hisi_sas_clear_nexus_ha,
.lldd_port_formed = hisi_sas_port_formed,
- .lldd_port_deformed = hisi_sas_port_deformed,
- .lldd_write_gpio = hisi_sas_write_gpio,
+ .lldd_write_gpio= hisi_sas_write_gpio,
};
void hisi_sas_init_mem(struct hisi_hba *hisi_hba)
diff --git a/drivers/scsi/libsas/sas_discover.c
b/drivers/scsi/libsas/sas_discover.c
index 0148ae62a52a..dde433aa59c2 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -260,7 +260,7 @@ static void sas_suspend_devices(struct work_struct *work)
* phy_list is not being mutated
*/
list_for_each_entry(phy, &port->phy_list, port_phy_el) {
- if (si->dft->lldd_port_formed)
+ if (si->dft->lldd_port_deformed)
si->dft->lldd_port_deformed(phy);
phy->suspended = 1;
port->suspended = 1;
--
2.14.4
[PATCH v3 3/5] scsi: libsas: always unregister the old device if going to discover new
If we went into sas_rediscover_dev() the attached_sas_addr was already
insured not to be zero. So it's unnecessary to check if the
attached_sas_addr is zero.
And although if the sas address is not changed, we always have to
unregister the old device when we are going to register a new one. We
cannot just leave the device there and bring up the new.
Signed-off-by: Jason Yan
CC: chenxiang
CC: John Garry
CC: Johannes Thumshirn
CC: Ewan Milne
CC: Christoph Hellwig
CC: Tomas Henzl
CC: Dan Williams
CC: Hannes Reinecke
Reviewed-by: Johannes Thumshirn
Reviewed-by: Hannes Reinecke
---
drivers/scsi/libsas/sas_expander.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/libsas/sas_expander.c
b/drivers/scsi/libsas/sas_expander.c
index fadc99cb60df..5940d398 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -2054,14 +2054,11 @@ static int sas_rediscover_dev(struct domain_device
*dev, int phy_id, bool last)
return res;
}
- /* delete the old link */
- if (SAS_ADDR(phy->attached_sas_addr) &&
- SAS_ADDR(sas_addr) != SAS_ADDR(phy->attached_sas_addr)) {
- SAS_DPRINTK("ex %016llx phy 0x%x replace %016llx\n",
- SAS_ADDR(dev->sas_addr), phy_id,
- SAS_ADDR(phy->attached_sas_addr));
- sas_unregister_devs_sas_addr(dev, phy_id, last);
- }
+ /* we always have to delete the old device when we went here */
+ SAS_DPRINTK("ex %016llx phy 0x%x replace %016llx\n",
+ SAS_ADDR(dev->sas_addr), phy_id,
+ SAS_ADDR(phy->attached_sas_addr));
+ sas_unregister_devs_sas_addr(dev, phy_id, last);
return sas_discover_new(dev, phy_id);
}
--
2.14.4
[PATCH v3 4/5] scsi: libsas: check the ata device status by ata_dev_enabled()
When ata device IDENTIFY failed, the ata device status is ATA_DEV_UNKNOWN. The libata reported like: [113518.620433] ata5.00: qc timeout (cmd 0xec) [113518.653646] ata5.00: failed to IDENTIFY (I/O error, err_mask=0x4) But libsas verifies the device status by ata_dev_disabled(), which skipped ATA_DEV_UNKNOWN. This will make libsas think the ata device probing succeed the device cannot be actually brought up. And even the new bcast of this device will be considered as flutter and will not probe this device again. Change ata_dev_disabled() to !ata_dev_enabled() so that libsas can deal with this if the ata device probe failed. New bcasts can let us try to probe the device again and bring it up if it is fine to IDENTIFY. Tested-by: Zhou Yupeng Signed-off-by: Jason Yan CC: John Garry CC: Johannes Thumshirn CC: Ewan Milne CC: Christoph Hellwig CC: Tomas Henzl CC: Dan Williams CC: Hannes Reinecke Reviewed-by: John Garry Reviewed-by: Hannes Reinecke Reviewed-by: Johannes Thumshirn Reviewed-by: Christoph Hellwig --- drivers/scsi/libsas/sas_ata.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c index 64a958a99f6a..4f6cdf53e913 100644 --- a/drivers/scsi/libsas/sas_ata.c +++ b/drivers/scsi/libsas/sas_ata.c @@ -654,7 +654,7 @@ void sas_probe_sata(struct asd_sas_port *port) /* if libata could not bring the link up, don't surface * the device */ - if (ata_dev_disabled(sas_to_ata_dev(dev))) + if (!ata_dev_enabled(sas_to_ata_dev(dev))) sas_fail_probe(dev, __func__, -ENODEV); } -- 2.14.4
[PATCH v3 1/5] scsi: libsas: delete dead code in scsi_transport_sas.c
This code is dead and no clue implies that it will be back again. Signed-off-by: Jason Yan CC: John Garry CC: Johannes Thumshirn CC: Ewan Milne CC: Christoph Hellwig CC: Tomas Henzl CC: Dan Williams CC: Hannes Reinecke Reviewed-by: Johannes Thumshirn Reviewed-by: John Garry Reviewed-by: Hannes Reinecke Reviewed-by: Christoph Hellwig --- drivers/scsi/scsi_transport_sas.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c index 0cd16e80b019..0a165b2b3e81 100644 --- a/drivers/scsi/scsi_transport_sas.c +++ b/drivers/scsi/scsi_transport_sas.c @@ -612,7 +612,6 @@ sas_phy_protocol_attr(identify.target_port_protocols, sas_phy_simple_attr(identify.sas_address, sas_address, "0x%016llx\n", unsigned long long); sas_phy_simple_attr(identify.phy_identifier, phy_identifier, "%d\n", u8); -//sas_phy_simple_attr(port_identifier, port_identifier, "%d\n", int); sas_phy_linkspeed_attr(negotiated_linkrate); sas_phy_linkspeed_attr(minimum_linkrate_hw); sas_phy_linkspeed_rw_attr(minimum_linkrate); @@ -1802,7 +1801,6 @@ sas_attach_transport(struct sas_function_template *ft) SETUP_PHY_ATTRIBUTE(device_type); SETUP_PHY_ATTRIBUTE(sas_address); SETUP_PHY_ATTRIBUTE(phy_identifier); - //SETUP_PHY_ATTRIBUTE(port_identifier); SETUP_PHY_ATTRIBUTE(negotiated_linkrate); SETUP_PHY_ATTRIBUTE(minimum_linkrate_hw); SETUP_PHY_ATTRIBUTE_RW(minimum_linkrate); -- 2.14.4
Re: [PATCH 1/3] phy: qcom-ufs: Remove stale methods that handle ref clk
On 9/24/2018 10:53 PM, Bjorn Andersson wrote:
On Tue 04 Sep 03:17 PDT 2018, Vivek Gautam wrote:
Remove ufs_qcom_phy_enable/(disable)_dev_ref_clk() that
are not being used by any code.
Signed-off-by: Vivek Gautam
Thanks for the ping Vivek, I didn't spot these when you posted them.
Reviewed-by: Bjorn Andersson
Thanks for reviewing the series, Bjorn.
Best regards
Vivek
Regards,
Bjorn
---
drivers/phy/qualcomm/phy-qcom-ufs.c | 50 -
include/linux/phy/phy-qcom-ufs.h| 14 ---
2 files changed, 64 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-ufs.c
b/drivers/phy/qualcomm/phy-qcom-ufs.c
index c5493ea51282..f2979ccad00a 100644
--- a/drivers/phy/qualcomm/phy-qcom-ufs.c
+++ b/drivers/phy/qualcomm/phy-qcom-ufs.c
@@ -431,56 +431,6 @@ static void ufs_qcom_phy_disable_ref_clk(struct
ufs_qcom_phy *phy)
}
}
-#define UFS_REF_CLK_EN (1 << 5)
-
-static void ufs_qcom_phy_dev_ref_clk_ctrl(struct phy *generic_phy, bool enable)
-{
- struct ufs_qcom_phy *phy = get_ufs_qcom_phy(generic_phy);
-
- if (phy->dev_ref_clk_ctrl_mmio &&
- (enable ^ phy->is_dev_ref_clk_enabled)) {
- u32 temp = readl_relaxed(phy->dev_ref_clk_ctrl_mmio);
-
- if (enable)
- temp |= UFS_REF_CLK_EN;
- else
- temp &= ~UFS_REF_CLK_EN;
-
- /*
-* If we are here to disable this clock immediately after
-* entering into hibern8, we need to make sure that device
-* ref_clk is active atleast 1us after the hibern8 enter.
-*/
- if (!enable)
- udelay(1);
-
- writel_relaxed(temp, phy->dev_ref_clk_ctrl_mmio);
- /* ensure that ref_clk is enabled/disabled before we return */
- wmb();
- /*
-* If we call hibern8 exit after this, we need to make sure that
-* device ref_clk is stable for atleast 1us before the hibern8
-* exit command.
-*/
- if (enable)
- udelay(1);
-
- phy->is_dev_ref_clk_enabled = enable;
- }
-}
-
-void ufs_qcom_phy_enable_dev_ref_clk(struct phy *generic_phy)
-{
- ufs_qcom_phy_dev_ref_clk_ctrl(generic_phy, true);
-}
-EXPORT_SYMBOL_GPL(ufs_qcom_phy_enable_dev_ref_clk);
-
-void ufs_qcom_phy_disable_dev_ref_clk(struct phy *generic_phy)
-{
- ufs_qcom_phy_dev_ref_clk_ctrl(generic_phy, false);
-}
-EXPORT_SYMBOL_GPL(ufs_qcom_phy_disable_dev_ref_clk);
-
/* Turn ON M-PHY RMMI interface clocks */
static int ufs_qcom_phy_enable_iface_clk(struct ufs_qcom_phy *phy)
{
diff --git a/include/linux/phy/phy-qcom-ufs.h b/include/linux/phy/phy-qcom-ufs.h
index 0a2c18a9771d..9dd85071bcce 100644
--- a/include/linux/phy/phy-qcom-ufs.h
+++ b/include/linux/phy/phy-qcom-ufs.h
@@ -17,20 +17,6 @@
#include "phy.h"
-/**
- * ufs_qcom_phy_enable_dev_ref_clk() - Enable the device
- * ref clock.
- * @phy: reference to a generic phy.
- */
-void ufs_qcom_phy_enable_dev_ref_clk(struct phy *phy);
-
-/**
- * ufs_qcom_phy_disable_dev_ref_clk() - Disable the device
- * ref clock.
- * @phy: reference to a generic phy.
- */
-void ufs_qcom_phy_disable_dev_ref_clk(struct phy *phy);
-
int ufs_qcom_phy_set_tx_lane_enable(struct phy *phy, u32 tx_lanes);
void ufs_qcom_phy_save_controller_version(struct phy *phy,
u8 major, u16 minor, u16 step);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 0/3] scsi: ufs-qcom: Remove all direct calls to qcom-ufs phy
Hi Vivek, On Tuesday 04 September 2018 03:47 PM, Vivek Gautam wrote: > Cleaning up the ufs-qcom host further to remove all direct calls > into qcom-ufs driver. > Only phy-qcom-ufs-qmp-20nm phy handles these direct calls from ufs host > and this phy is not used in any supported qcom platform in current kernel. > So, while we free up the host from all the ufs_qcom_phy_*() API calls > we should declare 20nm phy as broken. > For this we fork out couple of configs from PHY_QCOM_UFS - > PHY_QCOM_UFS_14NM and PHY_QCOM_UFS_20NM out of which we declare > PHY_QCOM_UFS_20NM as 'broken'. > > This series helps in a clean use of ufs phy support for sdm845 > and further SoCs that will also use phy-qcom-qmp phy driver. I think this entire series should go via linux-phy tree. I need ACK from UFS MAINTAINER for the second patch. Thanks Kishon
Re: [PATCH V14 0/2] Add UFS provisioning support in driver
>Configuration descriptor buffer consists of Device and Unit >descriptor configurable parameters which are parsed from vendor >specific provisioning file and then passed via configfs node at >runtime to provision ufs device. Can you describe your test setup? Will try to re-test it if not too complicated to reproduce it on a hikey-960. Thanks, Avri
[Bug 201221] New: USB drive shows up with write protection enabled
https://bugzilla.kernel.org/show_bug.cgi?id=201221 Bug ID: 201221 Summary: USB drive shows up with write protection enabled Product: IO/Storage Version: 2.5 Kernel Version: 4.14.44-4.19 Hardware: All OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: SCSI Assignee: linux-scsi@vger.kernel.org Reporter: ta...@tasossah.com Regression: No Created attachment 278743 --> https://bugzilla.kernel.org/attachment.cgi?id=278743&action=edit Kernel log I have a "Kingston DT Ultimate G3" USB flash drive that was functioning normally with past kernels. In newer ones, however, it shows up as having write protection enabled, making it unable to be written to. I have verified that it is not a hardware issue, by testing it on various computers with different operating systems, and the issue only happened on stable kernel 4.14 and newer. Other kernels are possibly affected too, but I haven't checked it. Bisecting the kernel showed that commit 20bd1d026aacc5399464f8328f305985c493cde3 [1], is the cause of this issue. It appears that the normal behaviour of this flash drive is to first show up as being write protected, and presumably after it is done initialising, it disables write protection. I believe that the commit mentioned above doesn't account for drives that may disable write protection later on, resulting in this issue. Running "blockdev --setrw /dev/sdX && blockdev --rereadpt /dev/sdX" allows the drive to continue functioning normally and be written to until it gets unplugged. Reverting the changes done by that commit also resolves this issue. Attached is the kernel (4.18.7) log during device init, and an attempt to mount the partition on it immediately afterwards. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=20bd1d026aacc5399464f8328f305985c493cde3 -- You are receiving this mail because: You are the assignee for the bug.
