drivers/net/wwan/t7xx/t7xx_modem_ops.c | 47 ++++++++++++++++--- drivers/net/wwan/t7xx/t7xx_modem_ops.h | 9 +++- drivers/net/wwan/t7xx/t7xx_pci.c | 53 ++++++++++++++++++---- drivers/net/wwan/t7xx/t7xx_pci.h | 3 ++ drivers/net/wwan/t7xx/t7xx_port_proxy.c | 1 - drivers/net/wwan/t7xx/t7xx_port_trace.c | 1 + drivers/net/wwan/t7xx/t7xx_state_monitor.c | 34 +++++--------- 7 files changed, 105 insertions(+), 43 deletions(-)
WWAN device is programmed to boot in normal mode or fastboot mode,
when triggering a device reset through ACPI call or fastboot switch
command.Maintain state machine synchronization and reprobe logic
after a device reset.
Suggestion from Bjorn:
Link: https://lore.kernel.org/all/20230127133034.GA1364550@bhelgaas/
Signed-off-by: Jinjian Song <jinjian.song@fibocom.com>
---
V2:
* initialize the variable 'ret' in t7xx_reset_device() function
---
drivers/net/wwan/t7xx/t7xx_modem_ops.c | 47 ++++++++++++++++---
drivers/net/wwan/t7xx/t7xx_modem_ops.h | 9 +++-
drivers/net/wwan/t7xx/t7xx_pci.c | 53 ++++++++++++++++++----
drivers/net/wwan/t7xx/t7xx_pci.h | 3 ++
drivers/net/wwan/t7xx/t7xx_port_proxy.c | 1 -
drivers/net/wwan/t7xx/t7xx_port_trace.c | 1 +
drivers/net/wwan/t7xx/t7xx_state_monitor.c | 34 +++++---------
7 files changed, 105 insertions(+), 43 deletions(-)
diff --git a/drivers/net/wwan/t7xx/t7xx_modem_ops.c b/drivers/net/wwan/t7xx/t7xx_modem_ops.c
index 8d864d4ed77f..79f17100f70b 100644
--- a/drivers/net/wwan/t7xx/t7xx_modem_ops.c
+++ b/drivers/net/wwan/t7xx/t7xx_modem_ops.c
@@ -53,6 +53,7 @@
#define RGU_RESET_DELAY_MS 10
#define PORT_RESET_DELAY_MS 2000
+#define FASTBOOT_RESET_DELAY_MS 2000
#define EX_HS_TIMEOUT_MS 5000
#define EX_HS_POLL_DELAY_MS 10
@@ -167,19 +168,52 @@ static int t7xx_acpi_reset(struct t7xx_pci_dev *t7xx_dev, char *fn_name)
}
kfree(buffer.pointer);
+#else
+ struct device *dev = &t7xx_dev->pdev->dev;
+ int ret;
+ ret = pci_reset_function(t7xx_dev->pdev);
+ if (ret) {
+ dev_err(dev, "Failed to reset device, error:%d\n", ret);
+ return ret;
+ }
#endif
return 0;
}
-int t7xx_acpi_fldr_func(struct t7xx_pci_dev *t7xx_dev)
+static void t7xx_host_event_notify(struct t7xx_pci_dev *t7xx_dev, unsigned int event_id)
{
- return t7xx_acpi_reset(t7xx_dev, "_RST");
+ u32 value;
+
+ value = ioread32(IREG_BASE(t7xx_dev) + T7XX_PCIE_MISC_DEV_STATUS);
+ value &= ~HOST_EVENT_MASK;
+ value |= FIELD_PREP(HOST_EVENT_MASK, event_id);
+ iowrite32(value, IREG_BASE(t7xx_dev) + T7XX_PCIE_MISC_DEV_STATUS);
}
-int t7xx_acpi_pldr_func(struct t7xx_pci_dev *t7xx_dev)
+int t7xx_reset_device(struct t7xx_pci_dev *t7xx_dev, enum reset_type type)
{
- return t7xx_acpi_reset(t7xx_dev, "MRST._RST");
+ int ret = 0;
+
+ pci_save_state(t7xx_dev->pdev);
+ t7xx_pci_reprobe_early(t7xx_dev);
+ t7xx_mode_update(t7xx_dev, T7XX_RESET);
+
+ if (type == FLDR) {
+ ret = t7xx_acpi_reset(t7xx_dev, "_RST");
+ } else if (type == PLDR) {
+ ret = t7xx_acpi_reset(t7xx_dev, "MRST._RST");
+ } else if (type == FASTBOOT) {
+ t7xx_host_event_notify(t7xx_dev, FASTBOOT_DL_NOTIFY);
+ t7xx_mhccif_h2d_swint_trigger(t7xx_dev, H2D_CH_DEVICE_RESET);
+ msleep(FASTBOOT_RESET_DELAY_MS);
+ }
+
+ pci_restore_state(t7xx_dev->pdev);
+ if (ret)
+ return ret;
+
+ return t7xx_pci_reprobe(t7xx_dev, true);
}
static void t7xx_reset_device_via_pmic(struct t7xx_pci_dev *t7xx_dev)
@@ -188,16 +222,15 @@ static void t7xx_reset_device_via_pmic(struct t7xx_pci_dev *t7xx_dev)
val = ioread32(IREG_BASE(t7xx_dev) + T7XX_PCIE_MISC_DEV_STATUS);
if (val & MISC_RESET_TYPE_PLDR)
- t7xx_acpi_reset(t7xx_dev, "MRST._RST");
+ t7xx_reset_device(t7xx_dev, PLDR);
else if (val & MISC_RESET_TYPE_FLDR)
- t7xx_acpi_fldr_func(t7xx_dev);
+ t7xx_reset_device(t7xx_dev, FLDR);
}
static irqreturn_t t7xx_rgu_isr_thread(int irq, void *data)
{
struct t7xx_pci_dev *t7xx_dev = data;
- t7xx_mode_update(t7xx_dev, T7XX_RESET);
msleep(RGU_RESET_DELAY_MS);
t7xx_reset_device_via_pmic(t7xx_dev);
return IRQ_HANDLED;
diff --git a/drivers/net/wwan/t7xx/t7xx_modem_ops.h b/drivers/net/wwan/t7xx/t7xx_modem_ops.h
index b39e945a92e0..39ed0000fbba 100644
--- a/drivers/net/wwan/t7xx/t7xx_modem_ops.h
+++ b/drivers/net/wwan/t7xx/t7xx_modem_ops.h
@@ -78,14 +78,19 @@ struct t7xx_modem {
spinlock_t exp_lock; /* Protects exception events */
};
+enum reset_type {
+ FLDR,
+ PLDR,
+ FASTBOOT,
+};
+
void t7xx_md_exception_handshake(struct t7xx_modem *md);
void t7xx_md_event_notify(struct t7xx_modem *md, enum md_event_id evt_id);
int t7xx_md_reset(struct t7xx_pci_dev *t7xx_dev);
int t7xx_md_init(struct t7xx_pci_dev *t7xx_dev);
void t7xx_md_exit(struct t7xx_pci_dev *t7xx_dev);
void t7xx_clear_rgu_irq(struct t7xx_pci_dev *t7xx_dev);
-int t7xx_acpi_fldr_func(struct t7xx_pci_dev *t7xx_dev);
-int t7xx_acpi_pldr_func(struct t7xx_pci_dev *t7xx_dev);
+int t7xx_reset_device(struct t7xx_pci_dev *t7xx_dev, enum reset_type type);
int t7xx_pci_mhccif_isr(struct t7xx_pci_dev *t7xx_dev);
#endif /* __T7XX_MODEM_OPS_H__ */
diff --git a/drivers/net/wwan/t7xx/t7xx_pci.c b/drivers/net/wwan/t7xx/t7xx_pci.c
index 10a8c1080b10..2398f41046ce 100644
--- a/drivers/net/wwan/t7xx/t7xx_pci.c
+++ b/drivers/net/wwan/t7xx/t7xx_pci.c
@@ -67,6 +67,7 @@ static ssize_t t7xx_mode_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
+ enum t7xx_mode mode;
struct t7xx_pci_dev *t7xx_dev;
struct pci_dev *pdev;
int index = 0;
@@ -76,12 +77,22 @@ static ssize_t t7xx_mode_store(struct device *dev,
if (!t7xx_dev)
return -ENODEV;
+ mode = READ_ONCE(t7xx_dev->mode);
+
index = sysfs_match_string(t7xx_mode_names, buf);
+ if (index == mode)
+ return -EBUSY;
+
if (index == T7XX_FASTBOOT_SWITCHING) {
+ if (mode == T7XX_FASTBOOT_DOWNLOAD)
+ return count;
+
WRITE_ONCE(t7xx_dev->mode, T7XX_FASTBOOT_SWITCHING);
+ pm_runtime_resume(dev);
+ t7xx_reset_device(t7xx_dev, FASTBOOT);
} else if (index == T7XX_RESET) {
- WRITE_ONCE(t7xx_dev->mode, T7XX_RESET);
- t7xx_acpi_pldr_func(t7xx_dev);
+ pm_runtime_resume(dev);
+ t7xx_reset_device(t7xx_dev, PLDR);
}
return count;
@@ -446,7 +457,7 @@ static int t7xx_pcie_reinit(struct t7xx_pci_dev *t7xx_dev, bool is_d3)
if (is_d3) {
t7xx_mhccif_init(t7xx_dev);
- return t7xx_pci_pm_reinit(t7xx_dev);
+ t7xx_pci_pm_reinit(t7xx_dev);
}
return 0;
@@ -481,6 +492,33 @@ static int t7xx_send_fsm_command(struct t7xx_pci_dev *t7xx_dev, u32 event)
return ret;
}
+int t7xx_pci_reprobe_early(struct t7xx_pci_dev *t7xx_dev)
+{
+ enum t7xx_mode mode = READ_ONCE(t7xx_dev->mode);
+ int ret;
+
+ if (mode == T7XX_FASTBOOT_DOWNLOAD)
+ pm_runtime_put_noidle(&t7xx_dev->pdev->dev);
+
+ ret = t7xx_send_fsm_command(t7xx_dev, FSM_CMD_STOP);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+int t7xx_pci_reprobe(struct t7xx_pci_dev *t7xx_dev, bool boot)
+{
+ int ret;
+
+ ret = t7xx_pcie_reinit(t7xx_dev, boot);
+ if (ret)
+ return ret;
+
+ t7xx_clear_rgu_irq(t7xx_dev);
+ return t7xx_send_fsm_command(t7xx_dev, FSM_CMD_START);
+}
+
static int __t7xx_pci_pm_resume(struct pci_dev *pdev, bool state_check)
{
struct t7xx_pci_dev *t7xx_dev;
@@ -507,16 +545,11 @@ static int __t7xx_pci_pm_resume(struct pci_dev *pdev, bool state_check)
if (prev_state == PM_RESUME_REG_STATE_L3 ||
(prev_state == PM_RESUME_REG_STATE_INIT &&
atr_reg_val == ATR_SRC_ADDR_INVALID)) {
- ret = t7xx_send_fsm_command(t7xx_dev, FSM_CMD_STOP);
- if (ret)
- return ret;
-
- ret = t7xx_pcie_reinit(t7xx_dev, true);
+ ret = t7xx_pci_reprobe_early(t7xx_dev);
if (ret)
return ret;
- t7xx_clear_rgu_irq(t7xx_dev);
- return t7xx_send_fsm_command(t7xx_dev, FSM_CMD_START);
+ return t7xx_pci_reprobe(t7xx_dev, true);
}
if (prev_state == PM_RESUME_REG_STATE_EXP ||
diff --git a/drivers/net/wwan/t7xx/t7xx_pci.h b/drivers/net/wwan/t7xx/t7xx_pci.h
index 49a11586d8d8..cd8ea17c2644 100644
--- a/drivers/net/wwan/t7xx/t7xx_pci.h
+++ b/drivers/net/wwan/t7xx/t7xx_pci.h
@@ -133,4 +133,7 @@ int t7xx_pci_pm_entity_unregister(struct t7xx_pci_dev *t7xx_dev, struct md_pm_en
void t7xx_pci_pm_init_late(struct t7xx_pci_dev *t7xx_dev);
void t7xx_pci_pm_exp_detected(struct t7xx_pci_dev *t7xx_dev);
void t7xx_mode_update(struct t7xx_pci_dev *t7xx_dev, enum t7xx_mode mode);
+int t7xx_pci_reprobe(struct t7xx_pci_dev *t7xx_dev, bool boot);
+int t7xx_pci_reprobe_early(struct t7xx_pci_dev *t7xx_dev);
+
#endif /* __T7XX_PCI_H__ */
diff --git a/drivers/net/wwan/t7xx/t7xx_port_proxy.c b/drivers/net/wwan/t7xx/t7xx_port_proxy.c
index 7d6388bf1d7c..35743e7de0c3 100644
--- a/drivers/net/wwan/t7xx/t7xx_port_proxy.c
+++ b/drivers/net/wwan/t7xx/t7xx_port_proxy.c
@@ -553,7 +553,6 @@ static int t7xx_proxy_alloc(struct t7xx_modem *md)
md->port_prox = port_prox;
port_prox->dev = dev;
- t7xx_port_proxy_set_cfg(md, PORT_CFG_ID_EARLY);
return 0;
}
diff --git a/drivers/net/wwan/t7xx/t7xx_port_trace.c b/drivers/net/wwan/t7xx/t7xx_port_trace.c
index 6a3f36385865..4ed8b4e29bf1 100644
--- a/drivers/net/wwan/t7xx/t7xx_port_trace.c
+++ b/drivers/net/wwan/t7xx/t7xx_port_trace.c
@@ -59,6 +59,7 @@ static void t7xx_trace_port_uninit(struct t7xx_port *port)
relay_close(relaych);
debugfs_remove_recursive(debugfs_dir);
+ port->log.relaych = NULL;
}
static int t7xx_trace_port_recv_skb(struct t7xx_port *port, struct sk_buff *skb)
diff --git a/drivers/net/wwan/t7xx/t7xx_state_monitor.c b/drivers/net/wwan/t7xx/t7xx_state_monitor.c
index 9889ca4621cf..3931c7a13f5a 100644
--- a/drivers/net/wwan/t7xx/t7xx_state_monitor.c
+++ b/drivers/net/wwan/t7xx/t7xx_state_monitor.c
@@ -213,16 +213,6 @@ static void fsm_routine_exception(struct t7xx_fsm_ctl *ctl, struct t7xx_fsm_comm
fsm_finish_command(ctl, cmd, 0);
}
-static void t7xx_host_event_notify(struct t7xx_modem *md, unsigned int event_id)
-{
- u32 value;
-
- value = ioread32(IREG_BASE(md->t7xx_dev) + T7XX_PCIE_MISC_DEV_STATUS);
- value &= ~HOST_EVENT_MASK;
- value |= FIELD_PREP(HOST_EVENT_MASK, event_id);
- iowrite32(value, IREG_BASE(md->t7xx_dev) + T7XX_PCIE_MISC_DEV_STATUS);
-}
-
static void t7xx_lk_stage_event_handling(struct t7xx_fsm_ctl *ctl, unsigned int status)
{
struct t7xx_modem *md = ctl->md;
@@ -264,8 +254,14 @@ static void t7xx_lk_stage_event_handling(struct t7xx_fsm_ctl *ctl, unsigned int
static int fsm_stopped_handler(struct t7xx_fsm_ctl *ctl)
{
+ enum t7xx_mode mode;
+
ctl->curr_state = FSM_STATE_STOPPED;
+ mode = READ_ONCE(ctl->md->t7xx_dev->mode);
+ if (mode == T7XX_FASTBOOT_DOWNLOAD || mode == T7XX_FASTBOOT_DUMP)
+ return 0;
+
t7xx_fsm_broadcast_state(ctl, MD_STATE_STOPPED);
return t7xx_md_reset(ctl->md->t7xx_dev);
}
@@ -284,8 +280,6 @@ static void fsm_routine_stopping(struct t7xx_fsm_ctl *ctl, struct t7xx_fsm_comma
{
struct cldma_ctrl *md_ctrl = ctl->md->md_ctrl[CLDMA_ID_MD];
struct t7xx_pci_dev *t7xx_dev = ctl->md->t7xx_dev;
- enum t7xx_mode mode = READ_ONCE(t7xx_dev->mode);
- int err;
if (ctl->curr_state == FSM_STATE_STOPPED || ctl->curr_state == FSM_STATE_STOPPING) {
fsm_finish_command(ctl, cmd, -EINVAL);
@@ -296,21 +290,10 @@ static void fsm_routine_stopping(struct t7xx_fsm_ctl *ctl, struct t7xx_fsm_comma
t7xx_fsm_broadcast_state(ctl, MD_STATE_WAITING_TO_STOP);
t7xx_cldma_stop(md_ctrl);
- if (mode == T7XX_FASTBOOT_SWITCHING)
- t7xx_host_event_notify(ctl->md, FASTBOOT_DL_NOTIFY);
-
t7xx_mhccif_h2d_swint_trigger(t7xx_dev, H2D_CH_DRM_DISABLE_AP);
/* Wait for the DRM disable to take effect */
msleep(FSM_DRM_DISABLE_DELAY_MS);
- if (mode == T7XX_FASTBOOT_SWITCHING) {
- t7xx_mhccif_h2d_swint_trigger(t7xx_dev, H2D_CH_DEVICE_RESET);
- } else {
- err = t7xx_acpi_fldr_func(t7xx_dev);
- if (err)
- t7xx_mhccif_h2d_swint_trigger(t7xx_dev, H2D_CH_DEVICE_RESET);
- }
-
fsm_finish_command(ctl, cmd, fsm_stopped_handler(ctl));
}
@@ -414,7 +397,9 @@ static void fsm_routine_start(struct t7xx_fsm_ctl *ctl, struct t7xx_fsm_command
case T7XX_DEV_STAGE_LK:
dev_dbg(dev, "LK_STAGE Entered\n");
+ t7xx_port_proxy_set_cfg(md, PORT_CFG_ID_EARLY);
t7xx_lk_stage_event_handling(ctl, status);
+
break;
case T7XX_DEV_STAGE_LINUX:
@@ -436,6 +421,9 @@ static void fsm_routine_start(struct t7xx_fsm_ctl *ctl, struct t7xx_fsm_command
}
finish_command:
+ if (ret)
+ t7xx_mode_update(md->t7xx_dev, T7XX_UNKNOWN);
+
fsm_finish_command(ctl, cmd, ret);
}
--
2.34.1From: Paolo Abeni <pabeni@redhat.com>
>On 8/9/24 05:37, Jinjian Song wrote:
>> WWAN device is programmed to boot in normal mode or fastboot mode,
>> when triggering a device reset through ACPI call or fastboot switch
>> command.Maintain state machine synchronization and reprobe logic
> ^^^ ^^^
>Minor nits: missing space and missing article above.
Please let me modify it.
>> after a device reset.
>>
>> Suggestion from Bjorn:
>> Link: https://lore.kernel.org/all/20230127133034.GA1364550@bhelgaas/
>>
>> Signed-off-by: Jinjian Song <jinjian.song@fibocom.com>
>> ---
>> V2:
>> * initialize the variable 'ret' in t7xx_reset_device() function
>> ---
>> drivers/net/wwan/t7xx/t7xx_modem_ops.c | 47 ++++++++++++++++---
>> drivers/net/wwan/t7xx/t7xx_modem_ops.h | 9 +++-
>> drivers/net/wwan/t7xx/t7xx_pci.c | 53 ++++++++++++++++++----
>> drivers/net/wwan/t7xx/t7xx_pci.h | 3 ++
>> drivers/net/wwan/t7xx/t7xx_port_proxy.c | 1 -
>> drivers/net/wwan/t7xx/t7xx_port_trace.c | 1 +
>> drivers/net/wwan/t7xx/t7xx_state_monitor.c | 34 +++++---------
>> 7 files changed, 105 insertions(+), 43 deletions(-)
>>
>> diff --git a/drivers/net/wwan/t7xx/t7xx_modem_ops.c b/drivers/net/wwan/t7xx/t7xx_modem_ops.c
>> index 8d864d4ed77f..79f17100f70b 100644
>> --- a/drivers/net/wwan/t7xx/t7xx_modem_ops.c
>> +++ b/drivers/net/wwan/t7xx/t7xx_modem_ops.c
>> @@ -53,6 +53,7 @@
>>
>> #define RGU_RESET_DELAY_MS 10
>> #define PORT_RESET_DELAY_MS 2000
>> +#define FASTBOOT_RESET_DELAY_MS 2000
>> #define EX_HS_TIMEOUT_MS 5000
>> #define EX_HS_POLL_DELAY_MS 10
>>
>> @@ -167,19 +168,52 @@ static int t7xx_acpi_reset(struct t7xx_pci_dev *t7xx_dev, char *fn_name)
>> }
>>
>> kfree(buffer.pointer);
>> +#else
>> + struct device *dev = &t7xx_dev->pdev->dev;
>> + int ret;
>>
>> + ret = pci_reset_function(t7xx_dev->pdev);
>> + if (ret) {
>> + dev_err(dev, "Failed to reset device, error:%d\n", ret);
>> + return ret;
>> + }
>> #endif
>> return 0;
>> }
>>
>> -int t7xx_acpi_fldr_func(struct t7xx_pci_dev *t7xx_dev)
>> +static void t7xx_host_event_notify(struct t7xx_pci_dev *t7xx_dev, unsigned int event_id)
>> {
>> - return t7xx_acpi_reset(t7xx_dev, "_RST");
>> + u32 value;
>> +
>> + value = ioread32(IREG_BASE(t7xx_dev) + T7XX_PCIE_MISC_DEV_STATUS);
>> + value &= ~HOST_EVENT_MASK;
>> + value |= FIELD_PREP(HOST_EVENT_MASK, event_id);
>> + iowrite32(value, IREG_BASE(t7xx_dev) + T7XX_PCIE_MISC_DEV_STATUS);
>> }
>>
>> -int t7xx_acpi_pldr_func(struct t7xx_pci_dev *t7xx_dev)
>> +int t7xx_reset_device(struct t7xx_pci_dev *t7xx_dev, enum reset_type type)
>> {
>> - return t7xx_acpi_reset(t7xx_dev, "MRST._RST");
>> + int ret = 0;
>> +
>> + pci_save_state(t7xx_dev->pdev);
>> + t7xx_pci_reprobe_early(t7xx_dev);
>> + t7xx_mode_update(t7xx_dev, T7XX_RESET);
>> +
>> + if (type == FLDR) {
>> + ret = t7xx_acpi_reset(t7xx_dev, "_RST");
>> + } else if (type == PLDR) {
>> + ret = t7xx_acpi_reset(t7xx_dev, "MRST._RST");
>> + } else if (type == FASTBOOT) {
>> + t7xx_host_event_notify(t7xx_dev, FASTBOOT_DL_NOTIFY);
>> + t7xx_mhccif_h2d_swint_trigger(t7xx_dev, H2D_CH_DEVICE_RESET);
>> + msleep(FASTBOOT_RESET_DELAY_MS);
>
>Did you test this with CONFIG_DEBUG_ATOMIC_SLEEP?
>
>AFAICS the above adds a 2s (FASTBOOT_RESET_DELAY_MS) unconditional sleep
>for 'fastboot' inside am IRQ thread function via:
>
>t7xx_rgu_isr_thread() -> t7xx_reset_device_via_pmic() -> t7xx_reset_device()
>
>Well, the existing code already looks broken with a 10 ms sleep there,
>still...
I have not test this with CONFIG_DEBUG_ATOMIC_SLEEP, The FATBOOT reset don't use
during IRQ thread, it only used by sysfs attribute interface, so I think we don't
have to worry about this.
Device IRQ -> t7xx_rgu_isr_thread()->t7xx_reset_device_via_pmic()-> only support FLDR and PLDR reset as before.
sysfs attribute 't7xx_mode' -> t7xx_reset_device(FASTBOOT), this way will use FASTBOOT reset.
>> diff --git a/drivers/net/wwan/t7xx/t7xx_pci.c b/drivers/net/wwan/t7xx/t7xx_pci.c
>> index 10a8c1080b10..2398f41046ce 100644
>> --- a/drivers/net/wwan/t7xx/t7xx_pci.c
>> +++ b/drivers/net/wwan/t7xx/t7xx_pci.c
>> @@ -67,6 +67,7 @@ static ssize_t t7xx_mode_store(struct device *dev,
>> struct device_attribute *attr,
>> const char *buf, size_t count)
>> {
>> + enum t7xx_mode mode;
>> struct t7xx_pci_dev *t7xx_dev;
>> struct pci_dev *pdev;
>> int index = 0;
>
>Minor nit: please respect the reverse xmas tree above.
>
Please let me modify it.
Thanks,
Best Regards,
Jinjian
On 8/9/24 05:37, Jinjian Song wrote:
> WWAN device is programmed to boot in normal mode or fastboot mode,
> when triggering a device reset through ACPI call or fastboot switch
> command.Maintain state machine synchronization and reprobe logic
^^^ ^^^
Minor nits: missing space and missing article above.
> after a device reset.
>
> Suggestion from Bjorn:
> Link: https://lore.kernel.org/all/20230127133034.GA1364550@bhelgaas/
>
> Signed-off-by: Jinjian Song <jinjian.song@fibocom.com>
> ---
> V2:
> * initialize the variable 'ret' in t7xx_reset_device() function
> ---
> drivers/net/wwan/t7xx/t7xx_modem_ops.c | 47 ++++++++++++++++---
> drivers/net/wwan/t7xx/t7xx_modem_ops.h | 9 +++-
> drivers/net/wwan/t7xx/t7xx_pci.c | 53 ++++++++++++++++++----
> drivers/net/wwan/t7xx/t7xx_pci.h | 3 ++
> drivers/net/wwan/t7xx/t7xx_port_proxy.c | 1 -
> drivers/net/wwan/t7xx/t7xx_port_trace.c | 1 +
> drivers/net/wwan/t7xx/t7xx_state_monitor.c | 34 +++++---------
> 7 files changed, 105 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/net/wwan/t7xx/t7xx_modem_ops.c b/drivers/net/wwan/t7xx/t7xx_modem_ops.c
> index 8d864d4ed77f..79f17100f70b 100644
> --- a/drivers/net/wwan/t7xx/t7xx_modem_ops.c
> +++ b/drivers/net/wwan/t7xx/t7xx_modem_ops.c
> @@ -53,6 +53,7 @@
>
> #define RGU_RESET_DELAY_MS 10
> #define PORT_RESET_DELAY_MS 2000
> +#define FASTBOOT_RESET_DELAY_MS 2000
> #define EX_HS_TIMEOUT_MS 5000
> #define EX_HS_POLL_DELAY_MS 10
>
> @@ -167,19 +168,52 @@ static int t7xx_acpi_reset(struct t7xx_pci_dev *t7xx_dev, char *fn_name)
> }
>
> kfree(buffer.pointer);
> +#else
> + struct device *dev = &t7xx_dev->pdev->dev;
> + int ret;
>
> + ret = pci_reset_function(t7xx_dev->pdev);
> + if (ret) {
> + dev_err(dev, "Failed to reset device, error:%d\n", ret);
> + return ret;
> + }
> #endif
> return 0;
> }
>
> -int t7xx_acpi_fldr_func(struct t7xx_pci_dev *t7xx_dev)
> +static void t7xx_host_event_notify(struct t7xx_pci_dev *t7xx_dev, unsigned int event_id)
> {
> - return t7xx_acpi_reset(t7xx_dev, "_RST");
> + u32 value;
> +
> + value = ioread32(IREG_BASE(t7xx_dev) + T7XX_PCIE_MISC_DEV_STATUS);
> + value &= ~HOST_EVENT_MASK;
> + value |= FIELD_PREP(HOST_EVENT_MASK, event_id);
> + iowrite32(value, IREG_BASE(t7xx_dev) + T7XX_PCIE_MISC_DEV_STATUS);
> }
>
> -int t7xx_acpi_pldr_func(struct t7xx_pci_dev *t7xx_dev)
> +int t7xx_reset_device(struct t7xx_pci_dev *t7xx_dev, enum reset_type type)
> {
> - return t7xx_acpi_reset(t7xx_dev, "MRST._RST");
> + int ret = 0;
> +
> + pci_save_state(t7xx_dev->pdev);
> + t7xx_pci_reprobe_early(t7xx_dev);
> + t7xx_mode_update(t7xx_dev, T7XX_RESET);
> +
> + if (type == FLDR) {
> + ret = t7xx_acpi_reset(t7xx_dev, "_RST");
> + } else if (type == PLDR) {
> + ret = t7xx_acpi_reset(t7xx_dev, "MRST._RST");
> + } else if (type == FASTBOOT) {
> + t7xx_host_event_notify(t7xx_dev, FASTBOOT_DL_NOTIFY);
> + t7xx_mhccif_h2d_swint_trigger(t7xx_dev, H2D_CH_DEVICE_RESET);
> + msleep(FASTBOOT_RESET_DELAY_MS);
Did you test this with CONFIG_DEBUG_ATOMIC_SLEEP?
AFAICS the above adds a 2s (FASTBOOT_RESET_DELAY_MS) unconditional sleep
for 'fastboot' inside am IRQ thread function via:
t7xx_rgu_isr_thread() -> t7xx_reset_device_via_pmic() -> t7xx_reset_device()
Well, the existing code already looks broken with a 10 ms sleep there,
still...
> diff --git a/drivers/net/wwan/t7xx/t7xx_pci.c b/drivers/net/wwan/t7xx/t7xx_pci.c
> index 10a8c1080b10..2398f41046ce 100644
> --- a/drivers/net/wwan/t7xx/t7xx_pci.c
> +++ b/drivers/net/wwan/t7xx/t7xx_pci.c
> @@ -67,6 +67,7 @@ static ssize_t t7xx_mode_store(struct device *dev,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
> + enum t7xx_mode mode;
> struct t7xx_pci_dev *t7xx_dev;
> struct pci_dev *pdev;
> int index = 0;
Minor nit: please respect the reverse xmas tree above.
Cheers,
Paolo
On Fri, Aug 09, 2024 at 11:37:01AM +0800, Jinjian Song wrote: > WWAN device is programmed to boot in normal mode or fastboot mode, > when triggering a device reset through ACPI call or fastboot switch > command.Maintain state machine synchronization and reprobe logic > after a device reset. > > Suggestion from Bjorn: > Link: https://lore.kernel.org/all/20230127133034.GA1364550@bhelgaas/ > > Signed-off-by: Jinjian Song <jinjian.song@fibocom.com> I'm glad you managed to do this without all the magic workqueue and remove and rescan of the device. Thanks for working on that! Bjorn
© 2016 - 2024 Red Hat, Inc.