2024-04-23 10:34:07

by Qiang Yu

[permalink] [raw]
Subject: [PATCH v4 0/3] Add sysfs entry to EDL mode

Add EDL sysfs entry for mhi controller that provides edl_trigger callback.
Add mhi_pci_generic_edl_trigger for qualcomm sdx55,sdx65 and sdx75 as
edl_trigger callback.

v3->v4:
1. Modify some comments, commit message and sysfs entry description.

2. Add error cleanups if get channel doorbell offset fail.

3. s/force_edl/trigger_edl

4. s/mhi_get_channel_doorbell/mhi_get_channel_doorbell_offset

5. Use mhi_get_channel_doorbell_offset in mhi_init_mmio()

v2->v3:
1. Update Documentation/ABI/stable/sysfs-bus-mhi with description of
force_edl sysfs entry.

2. Add comments about edl_trigger callback in mhi_controller struct.

3. Follow reverse christmas tree in mhi_pci_generic_edl_trigger.

4. Add a new API in MHI to allow controller to get CHDB address and avoid
duplicating the definition of CHDBOFF.

v1->v2:
1. Move all process needed by qualcomm sdx55,sdx65,sdx75 to enter EDL into
mhi_pci_generic_edl_trigger() as the callback to edl_trigger.

2. MHI stack creates EDL sysfs entry to invoke edl_trigger callback so
that devices need different mechanism to enter EDL can provide its own
edl_trigger callabck .

Qiang Yu (3):
bus: mhi: host: Add sysfs entry to force device to enter EDL
bus: mhi: host: Add a new API for getting channel doorbell address
bus: mhi: host: pci_generic: Add edl callback to enter EDL

Documentation/ABI/stable/sysfs-bus-mhi | 13 ++++++++++
drivers/bus/mhi/host/init.c | 39 ++++++++++++++++++++++++++---
drivers/bus/mhi/host/main.c | 16 ++++++++++++
drivers/bus/mhi/host/pci_generic.c | 45 ++++++++++++++++++++++++++++++++++
include/linux/mhi.h | 8 ++++++
5 files changed, 117 insertions(+), 4 deletions(-)

--
2.7.4



2024-04-23 10:34:08

by Qiang Yu

[permalink] [raw]
Subject: [PATCH v4 3/3] bus: mhi: host: pci_generic: Add edl callback to enter EDL

Some of the MHI modems like SDX65 based ones are capable of entering the EDL
mode as per the standard triggering mechanism defined in the MHI spec v1.2. So
let's add a common mhi_pci_generic_edl_trigger() function that triggers the EDL
mode in the device when user writes to the /sys/bus/mhi/devices/.../trigger_edl
file.

Signed-off-by: Qiang Yu <[email protected]>
---
drivers/bus/mhi/host/pci_generic.c | 45 ++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)

diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
index 51639bf..c65eaa8 100644
--- a/drivers/bus/mhi/host/pci_generic.c
+++ b/drivers/bus/mhi/host/pci_generic.c
@@ -27,12 +27,16 @@
#define PCI_VENDOR_ID_THALES 0x1269
#define PCI_VENDOR_ID_QUECTEL 0x1eac

+#define MHI_EDL_DB 91
+#define MHI_EDL_COOKIE 0xEDEDEDED
+
/**
* struct mhi_pci_dev_info - MHI PCI device specific information
* @config: MHI controller configuration
* @name: name of the PCI module
* @fw: firmware path (if any)
* @edl: emergency download mode firmware path (if any)
+ * @edl_trigger: capable of triggering EDL mode in the device (if supported)
* @bar_num: PCI base address register to use for MHI MMIO register space
* @dma_data_width: DMA transfer word size (32 or 64 bits)
* @mru_default: default MRU size for MBIM network packets
@@ -44,6 +48,7 @@ struct mhi_pci_dev_info {
const char *name;
const char *fw;
const char *edl;
+ bool edl_trigger;
unsigned int bar_num;
unsigned int dma_data_width;
unsigned int mru_default;
@@ -292,6 +297,7 @@ static const struct mhi_pci_dev_info mhi_qcom_sdx75_info = {
.name = "qcom-sdx75m",
.fw = "qcom/sdx75m/xbl.elf",
.edl = "qcom/sdx75m/edl.mbn",
+ .edl_trigger = true,
.config = &modem_qcom_v2_mhiv_config,
.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
.dma_data_width = 32,
@@ -302,6 +308,7 @@ static const struct mhi_pci_dev_info mhi_qcom_sdx65_info = {
.name = "qcom-sdx65m",
.fw = "qcom/sdx65m/xbl.elf",
.edl = "qcom/sdx65m/edl.mbn",
+ .edl_trigger = true,
.config = &modem_qcom_v1_mhiv_config,
.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
.dma_data_width = 32,
@@ -312,6 +319,7 @@ static const struct mhi_pci_dev_info mhi_qcom_sdx55_info = {
.name = "qcom-sdx55m",
.fw = "qcom/sdx55m/sbl1.mbn",
.edl = "qcom/sdx55m/edl.mbn",
+ .edl_trigger = true,
.config = &modem_qcom_v1_mhiv_config,
.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
.dma_data_width = 32,
@@ -928,6 +936,40 @@ static void health_check(struct timer_list *t)
mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD);
}

+static int mhi_pci_generic_edl_trigger(struct mhi_controller *mhi_cntrl)
+{
+ void __iomem *base = mhi_cntrl->regs;
+ void __iomem *edl_db;
+ int ret = 0;
+ u32 val;
+
+ ret = mhi_device_get_sync(mhi_cntrl->mhi_dev);
+ if (ret) {
+ dev_err(mhi_cntrl->cntrl_dev, "Failed to wakeup the device\n");
+ return ret;
+ }
+
+ pm_wakeup_event(&mhi_cntrl->mhi_dev->dev, 0);
+ mhi_cntrl->runtime_get(mhi_cntrl);
+
+ ret = mhi_get_channel_doorbell_offset(mhi_cntrl, &val);
+ if (ret)
+ goto err_get_chdb;
+
+ edl_db = base + val + (8 * MHI_EDL_DB);
+
+ mhi_cntrl->write_reg(mhi_cntrl, edl_db + 4, upper_32_bits(MHI_EDL_COOKIE));
+ mhi_cntrl->write_reg(mhi_cntrl, edl_db, lower_32_bits(MHI_EDL_COOKIE));
+
+ mhi_soc_reset(mhi_cntrl);
+
+err_get_chdb:
+ mhi_cntrl->runtime_put(mhi_cntrl);
+ mhi_device_put(mhi_cntrl->mhi_dev);
+
+ return ret;
+}
+
static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
const struct mhi_pci_dev_info *info = (struct mhi_pci_dev_info *) id->driver_data;
@@ -962,6 +1004,9 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
mhi_cntrl->runtime_put = mhi_pci_runtime_put;
mhi_cntrl->mru = info->mru_default;

+ if (info->edl_trigger)
+ mhi_cntrl->edl_trigger = mhi_pci_generic_edl_trigger;
+
if (info->sideband_wake) {
mhi_cntrl->wake_get = mhi_pci_wake_get_nop;
mhi_cntrl->wake_put = mhi_pci_wake_put_nop;
--
2.7.4


2024-04-23 10:34:24

by Qiang Yu

[permalink] [raw]
Subject: [PATCH v4 2/3] bus: mhi: host: Add a new API for getting channel doorbell address

Some controllers may want to know the address of a certain doorbell. Hence
add a new API where we read CHDBOFF register to get the base address of
doorbell, so that the controller can calculate the address of the doorbell
it wants by adding additional offset.

Signed-off-by: Qiang Yu <[email protected]>
---
drivers/bus/mhi/host/init.c | 6 ++----
drivers/bus/mhi/host/main.c | 16 ++++++++++++++++
include/linux/mhi.h | 6 ++++++
3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
index 7104c18..6e0fa79 100644
--- a/drivers/bus/mhi/host/init.c
+++ b/drivers/bus/mhi/host/init.c
@@ -541,11 +541,9 @@ int mhi_init_mmio(struct mhi_controller *mhi_cntrl)
dev_dbg(dev, "Initializing MHI registers\n");

/* Read channel db offset */
- ret = mhi_read_reg(mhi_cntrl, base, CHDBOFF, &val);
- if (ret) {
- dev_err(dev, "Unable to read CHDBOFF register\n");
+ ret = mhi_get_channel_doorbell_offset(mhi_cntrl, &val);
+ if (ret)
return -EIO;
- }

if (val >= mhi_cntrl->reg_len - (8 * MHI_DEV_WAKE_DB)) {
dev_err(dev, "CHDB offset: 0x%x is out of range: 0x%zx\n",
diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
index 15d657a..4de7567 100644
--- a/drivers/bus/mhi/host/main.c
+++ b/drivers/bus/mhi/host/main.c
@@ -1691,3 +1691,19 @@ void mhi_unprepare_from_transfer(struct mhi_device *mhi_dev)
}
}
EXPORT_SYMBOL_GPL(mhi_unprepare_from_transfer);
+
+int mhi_get_channel_doorbell_offset(struct mhi_controller *mhi_cntrl, u32 *chdb_offset)
+{
+ struct device *dev = &mhi_cntrl->mhi_dev->dev;
+ void __iomem *base = mhi_cntrl->regs;
+ int ret;
+
+ ret = mhi_read_reg(mhi_cntrl, base, CHDBOFF, chdb_offset);
+ if (ret) {
+ dev_err(dev, "Unable to read CHDBOFF register\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(mhi_get_channel_doorbell_offset);
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index d968e1a..cb3b676 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -816,4 +816,10 @@ int mhi_queue_skb(struct mhi_device *mhi_dev, enum dma_data_direction dir,
*/
bool mhi_queue_is_full(struct mhi_device *mhi_dev, enum dma_data_direction dir);

+/**
+ * mhi_get_channel_doorbell_offset - Get the channel doorbell offset
+ * @mhi_cntrl: MHI controller
+ * @chdb_offset: Channel doorbell offset
+ */
+int mhi_get_channel_doorbell_offset(struct mhi_controller *mhi_cntrl, u32 *chdb_offset);
#endif /* _MHI_H_ */
--
2.7.4


2024-04-23 10:34:34

by Qiang Yu

[permalink] [raw]
Subject: [PATCH v4 1/3] bus: mhi: host: Add sysfs entry to force device to enter EDL

Add sysfs entry to allow users of MHI bus force device to enter EDL.
Considering that the way to enter EDL mode varies from device to device and
some devices even do not support EDL. Hence, add a callback edl_trigger in
mhi controller as part of the sysfs entry to be invoked and MHI core will
only create EDL sysfs entry for mhi controller that provides edl_trigger
callback. All of the process a specific device required to enter EDL mode
can be placed in this callback.

Signed-off-by: Qiang Yu <[email protected]>
---
Documentation/ABI/stable/sysfs-bus-mhi | 13 +++++++++++++
drivers/bus/mhi/host/init.c | 33 +++++++++++++++++++++++++++++++++
include/linux/mhi.h | 2 ++
3 files changed, 48 insertions(+)

diff --git a/Documentation/ABI/stable/sysfs-bus-mhi b/Documentation/ABI/stable/sysfs-bus-mhi
index 1a47f9e..b44f467 100644
--- a/Documentation/ABI/stable/sysfs-bus-mhi
+++ b/Documentation/ABI/stable/sysfs-bus-mhi
@@ -29,3 +29,16 @@ Description: Initiates a SoC reset on the MHI controller. A SoC reset is
This can be useful as a method of recovery if the device is
non-responsive, or as a means of loading new firmware as a
system administration task.
+
+What: /sys/bus/mhi/devices/.../trigger_edl
+Date: April 2024
+KernelVersion: 6.9
+Contact: [email protected]
+Description: Writing a non-zero value to this file will force devices to
+ enter EDL (Emergency Download) mode. This entry only exists for
+ devices capable of entering the EDL mode using the standard EDL
+ triggering mechanism defined in the MHI spec v1.2. Once in EDL
+ mode, the flash programmer image can be downloaded to the
+ device to enter the flash programmer execution environment.
+ This can be useful if user wants to use QDL (Qualcomm Download,
+ which is used to download firmware over EDL) to update firmware.
diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
index 44f9349..7104c18 100644
--- a/drivers/bus/mhi/host/init.c
+++ b/drivers/bus/mhi/host/init.c
@@ -127,6 +127,30 @@ static ssize_t soc_reset_store(struct device *dev,
}
static DEVICE_ATTR_WO(soc_reset);

+static ssize_t trigger_edl_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct mhi_device *mhi_dev = to_mhi_device(dev);
+ struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
+ unsigned long val;
+ int ret;
+
+ ret = kstrtoul(buf, 10, &val);
+ if (ret < 0)
+ return ret;
+
+ if (!val)
+ return -EINVAL;
+
+ ret = mhi_cntrl->edl_trigger(mhi_cntrl);
+ if (ret)
+ return ret;
+
+ return count;
+}
+static DEVICE_ATTR_WO(trigger_edl);
+
static struct attribute *mhi_dev_attrs[] = {
&dev_attr_serial_number.attr,
&dev_attr_oem_pk_hash.attr,
@@ -1018,6 +1042,12 @@ int mhi_register_controller(struct mhi_controller *mhi_cntrl,
if (ret)
goto err_release_dev;

+ if (mhi_cntrl->edl_trigger) {
+ ret = sysfs_create_file(&mhi_dev->dev.kobj, &dev_attr_trigger_edl.attr);
+ if (ret)
+ goto err_release_dev;
+ }
+
mhi_cntrl->mhi_dev = mhi_dev;

mhi_create_debugfs(mhi_cntrl);
@@ -1051,6 +1081,9 @@ void mhi_unregister_controller(struct mhi_controller *mhi_cntrl)
mhi_deinit_free_irq(mhi_cntrl);
mhi_destroy_debugfs(mhi_cntrl);

+ if (mhi_cntrl->edl_trigger)
+ sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_trigger_edl.attr);
+
destroy_workqueue(mhi_cntrl->hiprio_wq);
kfree(mhi_cntrl->mhi_cmd);
kfree(mhi_cntrl->mhi_event);
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index cde01e1..d968e1a 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -353,6 +353,7 @@ struct mhi_controller_config {
* @read_reg: Read a MHI register via the physical link (required)
* @write_reg: Write a MHI register via the physical link (required)
* @reset: Controller specific reset function (optional)
+ * @edl_trigger: CB function to trigger EDL mode (optional)
* @buffer_len: Bounce buffer length
* @index: Index of the MHI controller instance
* @bounce_buf: Use of bounce buffer
@@ -435,6 +436,7 @@ struct mhi_controller {
void (*write_reg)(struct mhi_controller *mhi_cntrl, void __iomem *addr,
u32 val);
void (*reset)(struct mhi_controller *mhi_cntrl);
+ int (*edl_trigger)(struct mhi_controller *mhi_cntrl);

size_t buffer_len;
int index;
--
2.7.4


2024-04-23 18:46:57

by Jeffrey Hugo

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] bus: mhi: host: Add sysfs entry to force device to enter EDL

On 4/23/2024 4:33 AM, Qiang Yu wrote:
> Add sysfs entry to allow users of MHI bus force device to enter EDL.
> Considering that the way to enter EDL mode varies from device to device and
> some devices even do not support EDL. Hence, add a callback edl_trigger in
> mhi controller as part of the sysfs entry to be invoked and MHI core will
> only create EDL sysfs entry for mhi controller that provides edl_trigger
> callback. All of the process a specific device required to enter EDL mode
> can be placed in this callback.
>
> Signed-off-by: Qiang Yu <[email protected]>

Reviewed-by: Jeffrey Hugo <[email protected]>

2024-04-23 18:48:22

by Jeffrey Hugo

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] bus: mhi: host: Add a new API for getting channel doorbell address

On 4/23/2024 4:33 AM, Qiang Yu wrote:
> Some controllers may want to know the address of a certain doorbell. Hence
> add a new API where we read CHDBOFF register to get the base address of
> doorbell, so that the controller can calculate the address of the doorbell
> it wants by adding additional offset.
>
> Signed-off-by: Qiang Yu <[email protected]>
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index d968e1a..cb3b676 100644
> --- a/include/linux/mhi.h
> +++ b/include/linux/mhi.h
> @@ -816,4 +816,10 @@ int mhi_queue_skb(struct mhi_device *mhi_dev, enum dma_data_direction dir,
> */
> bool mhi_queue_is_full(struct mhi_device *mhi_dev, enum dma_data_direction dir);
>
> +/**
> + * mhi_get_channel_doorbell_offset - Get the channel doorbell offset
> + * @mhi_cntrl: MHI controller
> + * @chdb_offset: Channel doorbell offset
> + */
> +int mhi_get_channel_doorbell_offset(struct mhi_controller *mhi_cntrl, u32 *chdb_offset);

Should have a blank line here before the #endif just like how the file
was before this change

> #endif /* _MHI_H_ */

With the above
Reviewed-by: Jeffrey Hugo <[email protected]>

2024-04-23 18:48:50

by Jeffrey Hugo

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] bus: mhi: host: pci_generic: Add edl callback to enter EDL

On 4/23/2024 4:33 AM, Qiang Yu wrote:
> Some of the MHI modems like SDX65 based ones are capable of entering the EDL
> mode as per the standard triggering mechanism defined in the MHI spec v1.2. So
> let's add a common mhi_pci_generic_edl_trigger() function that triggers the EDL
> mode in the device when user writes to the /sys/bus/mhi/devices/.../trigger_edl
> file.
>
> Signed-off-by: Qiang Yu <[email protected]>

Reviewed-by: Jeffrey Hugo <[email protected]>

2024-04-25 13:53:19

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] bus: mhi: host: Add sysfs entry to force device to enter EDL

On Tue, Apr 23, 2024 at 06:33:35PM +0800, Qiang Yu wrote:
> Add sysfs entry to allow users of MHI bus force device to enter EDL.
> Considering that the way to enter EDL mode varies from device to device and
> some devices even do not support EDL. Hence, add a callback edl_trigger in
> mhi controller as part of the sysfs entry to be invoked and MHI core will
> only create EDL sysfs entry for mhi controller that provides edl_trigger
> callback. All of the process a specific device required to enter EDL mode
> can be placed in this callback.
>
> Signed-off-by: Qiang Yu <[email protected]>

One nitpick below. But I'll fix it while applying.

Reviewed-by: Manivannan Sadhasivam <[email protected]>

> ---
> Documentation/ABI/stable/sysfs-bus-mhi | 13 +++++++++++++
> drivers/bus/mhi/host/init.c | 33 +++++++++++++++++++++++++++++++++
> include/linux/mhi.h | 2 ++
> 3 files changed, 48 insertions(+)
>
> diff --git a/Documentation/ABI/stable/sysfs-bus-mhi b/Documentation/ABI/stable/sysfs-bus-mhi
> index 1a47f9e..b44f467 100644
> --- a/Documentation/ABI/stable/sysfs-bus-mhi
> +++ b/Documentation/ABI/stable/sysfs-bus-mhi
> @@ -29,3 +29,16 @@ Description: Initiates a SoC reset on the MHI controller. A SoC reset is
> This can be useful as a method of recovery if the device is
> non-responsive, or as a means of loading new firmware as a
> system administration task.
> +
> +What: /sys/bus/mhi/devices/.../trigger_edl
> +Date: April 2024
> +KernelVersion: 6.9

6.9 is done, now we are working for 6.10 (feature wise).

- Mani

> +Contact: [email protected]
> +Description: Writing a non-zero value to this file will force devices to
> + enter EDL (Emergency Download) mode. This entry only exists for
> + devices capable of entering the EDL mode using the standard EDL
> + triggering mechanism defined in the MHI spec v1.2. Once in EDL
> + mode, the flash programmer image can be downloaded to the
> + device to enter the flash programmer execution environment.
> + This can be useful if user wants to use QDL (Qualcomm Download,
> + which is used to download firmware over EDL) to update firmware.
> diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
> index 44f9349..7104c18 100644
> --- a/drivers/bus/mhi/host/init.c
> +++ b/drivers/bus/mhi/host/init.c
> @@ -127,6 +127,30 @@ static ssize_t soc_reset_store(struct device *dev,
> }
> static DEVICE_ATTR_WO(soc_reset);
>
> +static ssize_t trigger_edl_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct mhi_device *mhi_dev = to_mhi_device(dev);
> + struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
> + unsigned long val;
> + int ret;
> +
> + ret = kstrtoul(buf, 10, &val);
> + if (ret < 0)
> + return ret;
> +
> + if (!val)
> + return -EINVAL;
> +
> + ret = mhi_cntrl->edl_trigger(mhi_cntrl);
> + if (ret)
> + return ret;
> +
> + return count;
> +}
> +static DEVICE_ATTR_WO(trigger_edl);
> +
> static struct attribute *mhi_dev_attrs[] = {
> &dev_attr_serial_number.attr,
> &dev_attr_oem_pk_hash.attr,
> @@ -1018,6 +1042,12 @@ int mhi_register_controller(struct mhi_controller *mhi_cntrl,
> if (ret)
> goto err_release_dev;
>
> + if (mhi_cntrl->edl_trigger) {
> + ret = sysfs_create_file(&mhi_dev->dev.kobj, &dev_attr_trigger_edl.attr);
> + if (ret)
> + goto err_release_dev;
> + }
> +
> mhi_cntrl->mhi_dev = mhi_dev;
>
> mhi_create_debugfs(mhi_cntrl);
> @@ -1051,6 +1081,9 @@ void mhi_unregister_controller(struct mhi_controller *mhi_cntrl)
> mhi_deinit_free_irq(mhi_cntrl);
> mhi_destroy_debugfs(mhi_cntrl);
>
> + if (mhi_cntrl->edl_trigger)
> + sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_trigger_edl.attr);
> +
> destroy_workqueue(mhi_cntrl->hiprio_wq);
> kfree(mhi_cntrl->mhi_cmd);
> kfree(mhi_cntrl->mhi_event);
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index cde01e1..d968e1a 100644
> --- a/include/linux/mhi.h
> +++ b/include/linux/mhi.h
> @@ -353,6 +353,7 @@ struct mhi_controller_config {
> * @read_reg: Read a MHI register via the physical link (required)
> * @write_reg: Write a MHI register via the physical link (required)
> * @reset: Controller specific reset function (optional)
> + * @edl_trigger: CB function to trigger EDL mode (optional)
> * @buffer_len: Bounce buffer length
> * @index: Index of the MHI controller instance
> * @bounce_buf: Use of bounce buffer
> @@ -435,6 +436,7 @@ struct mhi_controller {
> void (*write_reg)(struct mhi_controller *mhi_cntrl, void __iomem *addr,
> u32 val);
> void (*reset)(struct mhi_controller *mhi_cntrl);
> + int (*edl_trigger)(struct mhi_controller *mhi_cntrl);
>
> size_t buffer_len;
> int index;
> --
> 2.7.4
>
>

--
மணிவண்ணன் சதாசிவம்

2024-04-25 14:01:24

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] bus: mhi: host: pci_generic: Add edl callback to enter EDL

On Tue, Apr 23, 2024 at 06:33:37PM +0800, Qiang Yu wrote:
> Some of the MHI modems like SDX65 based ones are capable of entering the EDL
> mode as per the standard triggering mechanism defined in the MHI spec v1.2. So
> let's add a common mhi_pci_generic_edl_trigger() function that triggers the EDL
> mode in the device when user writes to the /sys/bus/mhi/devices/.../trigger_edl
> file.
>
> Signed-off-by: Qiang Yu <[email protected]>

Same comment as previous.

Reviewed-by: Manivannan Sadhasivam <[email protected]>

> ---
> drivers/bus/mhi/host/pci_generic.c | 45 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 45 insertions(+)
>
> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
> index 51639bf..c65eaa8 100644
> --- a/drivers/bus/mhi/host/pci_generic.c
> +++ b/drivers/bus/mhi/host/pci_generic.c
> @@ -27,12 +27,16 @@
> #define PCI_VENDOR_ID_THALES 0x1269
> #define PCI_VENDOR_ID_QUECTEL 0x1eac
>
> +#define MHI_EDL_DB 91
> +#define MHI_EDL_COOKIE 0xEDEDEDED
> +
> /**
> * struct mhi_pci_dev_info - MHI PCI device specific information
> * @config: MHI controller configuration
> * @name: name of the PCI module
> * @fw: firmware path (if any)
> * @edl: emergency download mode firmware path (if any)
> + * @edl_trigger: capable of triggering EDL mode in the device (if supported)
> * @bar_num: PCI base address register to use for MHI MMIO register space
> * @dma_data_width: DMA transfer word size (32 or 64 bits)
> * @mru_default: default MRU size for MBIM network packets
> @@ -44,6 +48,7 @@ struct mhi_pci_dev_info {
> const char *name;
> const char *fw;
> const char *edl;
> + bool edl_trigger;
> unsigned int bar_num;
> unsigned int dma_data_width;
> unsigned int mru_default;
> @@ -292,6 +297,7 @@ static const struct mhi_pci_dev_info mhi_qcom_sdx75_info = {
> .name = "qcom-sdx75m",
> .fw = "qcom/sdx75m/xbl.elf",
> .edl = "qcom/sdx75m/edl.mbn",
> + .edl_trigger = true,
> .config = &modem_qcom_v2_mhiv_config,
> .bar_num = MHI_PCI_DEFAULT_BAR_NUM,
> .dma_data_width = 32,
> @@ -302,6 +308,7 @@ static const struct mhi_pci_dev_info mhi_qcom_sdx65_info = {
> .name = "qcom-sdx65m",
> .fw = "qcom/sdx65m/xbl.elf",
> .edl = "qcom/sdx65m/edl.mbn",
> + .edl_trigger = true,
> .config = &modem_qcom_v1_mhiv_config,
> .bar_num = MHI_PCI_DEFAULT_BAR_NUM,
> .dma_data_width = 32,
> @@ -312,6 +319,7 @@ static const struct mhi_pci_dev_info mhi_qcom_sdx55_info = {
> .name = "qcom-sdx55m",
> .fw = "qcom/sdx55m/sbl1.mbn",
> .edl = "qcom/sdx55m/edl.mbn",
> + .edl_trigger = true,
> .config = &modem_qcom_v1_mhiv_config,
> .bar_num = MHI_PCI_DEFAULT_BAR_NUM,
> .dma_data_width = 32,
> @@ -928,6 +936,40 @@ static void health_check(struct timer_list *t)
> mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD);
> }
>
> +static int mhi_pci_generic_edl_trigger(struct mhi_controller *mhi_cntrl)
> +{
> + void __iomem *base = mhi_cntrl->regs;
> + void __iomem *edl_db;
> + int ret = 0;

No need to initialize 'ret'.

- Mani

> + u32 val;
> +
> + ret = mhi_device_get_sync(mhi_cntrl->mhi_dev);
> + if (ret) {
> + dev_err(mhi_cntrl->cntrl_dev, "Failed to wakeup the device\n");
> + return ret;
> + }
> +
> + pm_wakeup_event(&mhi_cntrl->mhi_dev->dev, 0);
> + mhi_cntrl->runtime_get(mhi_cntrl);
> +
> + ret = mhi_get_channel_doorbell_offset(mhi_cntrl, &val);
> + if (ret)
> + goto err_get_chdb;
> +
> + edl_db = base + val + (8 * MHI_EDL_DB);
> +
> + mhi_cntrl->write_reg(mhi_cntrl, edl_db + 4, upper_32_bits(MHI_EDL_COOKIE));
> + mhi_cntrl->write_reg(mhi_cntrl, edl_db, lower_32_bits(MHI_EDL_COOKIE));
> +
> + mhi_soc_reset(mhi_cntrl);
> +
> +err_get_chdb:
> + mhi_cntrl->runtime_put(mhi_cntrl);
> + mhi_device_put(mhi_cntrl->mhi_dev);
> +
> + return ret;
> +}
> +
> static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> {
> const struct mhi_pci_dev_info *info = (struct mhi_pci_dev_info *) id->driver_data;
> @@ -962,6 +1004,9 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> mhi_cntrl->runtime_put = mhi_pci_runtime_put;
> mhi_cntrl->mru = info->mru_default;
>
> + if (info->edl_trigger)
> + mhi_cntrl->edl_trigger = mhi_pci_generic_edl_trigger;
> +
> if (info->sideband_wake) {
> mhi_cntrl->wake_get = mhi_pci_wake_get_nop;
> mhi_cntrl->wake_put = mhi_pci_wake_put_nop;
> --
> 2.7.4
>
>

--
மணிவண்ணன் சதாசிவம்

2024-04-25 14:13:36

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] bus: mhi: host: Add a new API for getting channel doorbell address

On Tue, Apr 23, 2024 at 06:33:36PM +0800, Qiang Yu wrote:
> Some controllers may want to know the address of a certain doorbell. Hence
> add a new API where we read CHDBOFF register to get the base address of
> doorbell, so that the controller can calculate the address of the doorbell
> it wants by adding additional offset.
>
> Signed-off-by: Qiang Yu <[email protected]>

One nitpick below. But I'll fix it.

Reviewed-by: Manivannan Sadhasivam <[email protected]>

> ---
> drivers/bus/mhi/host/init.c | 6 ++----
> drivers/bus/mhi/host/main.c | 16 ++++++++++++++++
> include/linux/mhi.h | 6 ++++++
> 3 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
> index 7104c18..6e0fa79 100644
> --- a/drivers/bus/mhi/host/init.c
> +++ b/drivers/bus/mhi/host/init.c
> @@ -541,11 +541,9 @@ int mhi_init_mmio(struct mhi_controller *mhi_cntrl)
> dev_dbg(dev, "Initializing MHI registers\n");
>
> /* Read channel db offset */
> - ret = mhi_read_reg(mhi_cntrl, base, CHDBOFF, &val);
> - if (ret) {
> - dev_err(dev, "Unable to read CHDBOFF register\n");
> + ret = mhi_get_channel_doorbell_offset(mhi_cntrl, &val);
> + if (ret)
> return -EIO;

return ret;

- Mani

> - }
>
> if (val >= mhi_cntrl->reg_len - (8 * MHI_DEV_WAKE_DB)) {
> dev_err(dev, "CHDB offset: 0x%x is out of range: 0x%zx\n",
> diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
> index 15d657a..4de7567 100644
> --- a/drivers/bus/mhi/host/main.c
> +++ b/drivers/bus/mhi/host/main.c
> @@ -1691,3 +1691,19 @@ void mhi_unprepare_from_transfer(struct mhi_device *mhi_dev)
> }
> }
> EXPORT_SYMBOL_GPL(mhi_unprepare_from_transfer);
> +
> +int mhi_get_channel_doorbell_offset(struct mhi_controller *mhi_cntrl, u32 *chdb_offset)
> +{
> + struct device *dev = &mhi_cntrl->mhi_dev->dev;
> + void __iomem *base = mhi_cntrl->regs;
> + int ret;
> +
> + ret = mhi_read_reg(mhi_cntrl, base, CHDBOFF, chdb_offset);
> + if (ret) {
> + dev_err(dev, "Unable to read CHDBOFF register\n");
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(mhi_get_channel_doorbell_offset);
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index d968e1a..cb3b676 100644
> --- a/include/linux/mhi.h
> +++ b/include/linux/mhi.h
> @@ -816,4 +816,10 @@ int mhi_queue_skb(struct mhi_device *mhi_dev, enum dma_data_direction dir,
> */
> bool mhi_queue_is_full(struct mhi_device *mhi_dev, enum dma_data_direction dir);
>
> +/**
> + * mhi_get_channel_doorbell_offset - Get the channel doorbell offset
> + * @mhi_cntrl: MHI controller
> + * @chdb_offset: Channel doorbell offset
> + */
> +int mhi_get_channel_doorbell_offset(struct mhi_controller *mhi_cntrl, u32 *chdb_offset);
> #endif /* _MHI_H_ */
> --
> 2.7.4
>
>

--
மணிவண்ணன் சதாசிவம்