remoteproc: qcom: Introduce Qualcomm low pass sensor peripheral loader.
Subsystems (4)
v2 cover remoteproc: qcom: Introduce Qualcomm low pass sensor peripheral loader.
This patchset has changed from last patchset in below respect
1- Discard Independent SLPI driver.
2- Add SLPI boot support in existing SLPI driver.
3- Modify existing ADSP rproc driver to handle resource in generic manner based on compatible string. 4- Address minor comments for Dt-binding documentation and smp2p support for slpi.
Avaneesh Kumar Dwivedi (3):
remoteproc: qcom: make adsp resource handling generic.
remoteproc: qcom: Add slpi boot support in adsp rproc driver.
arm64: dts: msm8996: Add SLPI SMP2P dt node.
.../devicetree/bindings/remoteproc/qcom,adsp.txt | 29 +++ arch/arm64/boot/dts/qcom/msm8996.dtsi | 24 ++ drivers/remoteproc/qcom_adsp_pil.c | 282 +++++++++++++++++---- 3 files changed, 284 insertions(+), 51 deletions(-)
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
RFC PATCH v2 1/3 remoteproc: qcom: make adsp resource handling generic.
This patch make resource handling generic in adsp remoteproc driver. Resources which were initialized with hard definition are being initialized now based on compatible string. Generic way of resource initialization and handling is required so that slpi processor boot support can also be enabled with same driver.
--- drivers/remoteproc/qcom_adsp_pil.c | 259 +++++++++++++++++++++++++++++-------- 1 file changed, 208 insertions(+), 51 deletions(-)
drivers/remoteproc/qcom_adsp_pil.c +208 -51
@@ -32,9 +32,26 @@#include "qcom_mdt_loader.h" #include "remoteproc_internal.h" -#define ADSP_CRASH_REASON_SMEM 423 -#define ADSP_FIRMWARE_NAME "adsp.mdt" -#define ADSP_PAS_ID 1 +struct reg_info { +struct regulator *reg; +int uV; +int uA; +}; + +struct regulator_res { +const char *supply; +int uV; +int uA; +}; + +struct generic_rproc_res { +int crash_reason_smem; +const char *firmware_name; +int fw_pas_id; +struct regulator_res reg_res[3 ]; +char **clocks_name; +}; +struct qcom_adsp {struct device *dev;
@@ -49,9 +66,13 @@ struct qcom_adsp {struct qcom_smem_state *state;unsigned stop_bit; -struct clk *xo; +struct clk *clocks[3 ]; +int clk_count; +struct reg_info regulators[3 ]; +int reg_count; -struct regulator *cx_supply; +int fw_pas_id; +int crash_reason_smem;struct completion start_done;struct completion stop_done;
@@ -62,6 +83,136 @@ struct qcom_adsp {size_t mem_size; }; +static int generic_regulator_init (struct device *dev,struct reg_info *regs, +const struct regulator_res *reg_res) +{ +int rc; +int i; + +for (i =0 ; reg_res[i].supply; i++) { + regs[i].reg =devm_regulator_get (dev, reg_res[i].supply); +if (IS_ERR (regs[i].reg)) { + rc =PTR_ERR (regs[i].reg); +if (rc != -EPROBE_DEFER ) +dev_err (dev,"Failed to get %s\n regulator" , + reg_res[i].supply); +return rc; + } + + regs[i].uV = reg_res[i].uV; + regs[i].uA = reg_res[i].uA; + } + +return i; +} + +static int generic_regulator_enable (struct qcom_adsp *adsp, +struct reg_info *regs,int count) +{ +int ret; +int i; + +for (i =0 ; i < count; i++) { +if (regs[i].uV >0 ) { + ret =regulator_set_voltage (regs[i].reg, + regs[i].uV,INT_MAX ); +if (ret) { +dev_err (adsp->dev, +"Failed to request voltage for %d.\n" , + i); + goto err; + } + } + + ret =regulator_enable (regs[i].reg); +if (ret) { +dev_err (adsp->dev,"Regulator enable failed\n" ); + goto err; + } + } + +return 0 ; +err: +for (; i >=0 ; i--) { +if (regs[i].uV >0 ) +regulator_set_voltage (regs[i].reg,0 ,INT_MAX ); + +regulator_disable (regs[i].reg); + } + +return ret; +} + +static void generic_regulator_disable (struct qcom_adsp *adsp, +struct reg_info *regs,int count) +{ +int i; + +for (i =0 ; i < count; i++) { +if (regs[i].uV >0 ) +regulator_set_voltage (regs[i].reg,0 ,INT_MAX ); + +regulator_disable (regs[i].reg); + } +} + +static int generic_init_clocks (struct device *dev, +struct clk **clks,char **clk_names) +{ +int i; + +if (!clk_names) +return 0 ; + +for (i =0 ; clk_names[i]; i++) { + clks[i] =devm_clk_get (dev, clk_names[i]); +if (IS_ERR (clks[i])) { + +int rc =PTR_ERR (clks[i]); + +if (rc != -EPROBE_DEFER ) +dev_err (dev,"Failed to get %s clock\n" , + clk_names[i]); +return rc; + } + + } + +return i; +} + +static int generic_clk_enable (struct device *dev, +struct clk **clks,int count) +{ +int rc; +int i; + +for (i =0 ; i < count; i++) { + rc =clk_prepare_enable (clks[i]); +if (rc) { +dev_err (dev,"Clock enable failed\n" ); + goto err; + } + } + +return 0 ; +err: +for (i--; i >=0 ; i--) +clk_disable_unprepare (clks[i]); + +return rc; +} + +static void generic_clk_disable (struct device *dev, +struct clk **clks,int count) +{ +int i; + +for (i =0 ; i < count; i++) +clk_disable_unprepare (clks[i]); +} + +static int adsp_load (struct rproc *rproc,const struct firmware *fw) {struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
@@ -70,7 +221,7 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw)bool relocate;int ret; - ret =qcom_scm_pas_init_image (ADSP_PAS_ID , fw->data, fw->size); + ret =qcom_scm_pas_init_image (adsp->fw_pas_id, fw->data, fw->size);if (ret) {dev_err (&rproc->dev,"invalid firmware metadata\n" );return ret;
@@ -85,7 +236,8 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw)if (relocate) { adsp->mem_reloc = fw_addr; - ret =qcom_scm_pas_mem_setup (ADSP_PAS_ID , adsp->mem_phys, fw_size); + ret =qcom_scm_pas_mem_setup (adsp->fw_pas_id, + adsp->mem_phys, fw_size);if (ret) {dev_err (&rproc->dev,"unable to setup memory for image\n" );return ret;
@@ -105,15 +257,14 @@ static int adsp_start(struct rproc *rproc)struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;int ret; - ret =clk_prepare_enable (adsp->xo); + ret =generic_clk_enable (adsp->dev, adsp->clocks, adsp->clk_count);if (ret)return ret; - - ret =regulator_enable (adsp->cx_supply); + ret =generic_regulator_enable (adsp, + adsp->regulators, adsp->reg_count);if (ret) goto disable_clocks; - - ret =qcom_scm_pas_auth_and_reset (ADSP_PAS_ID ); + ret =qcom_scm_pas_auth_and_reset (adsp->fw_pas_id);if (ret) {dev_err (adsp->dev,"failed to authenticate image and release reset\n" );
@@ -124,7 +275,7 @@ static int adsp_start(struct rproc *rproc) msecs_to_jiffies(5000));if (!ret) {dev_err (adsp->dev,"start timed out\n" ); -qcom_scm_pas_shutdown (ADSP_PAS_ID ); +qcom_scm_pas_shutdown (adsp->fw_pas_id); ret = -ETIMEDOUT ; goto disable_regulators; }
@@ -132,9 +283,9 @@ static int adsp_start(struct rproc *rproc) ret =0 ; disable_regulators: -regulator_disable (adsp->cx_supply); +generic_regulator_disable (adsp, adsp->regulators, adsp->reg_count); disable_clocks: -clk_disable_unprepare (adsp->xo); +generic_clk_disable (adsp->dev, adsp->clocks, adsp->clk_count);return ret; }
@@ -157,7 +308,7 @@ static int adsp_stop(struct rproc *rproc)BIT (adsp->stop_bit), 0); - ret =qcom_scm_pas_shutdown (ADSP_PAS_ID ); + ret =qcom_scm_pas_shutdown (adsp->fw_pas_id);if (ret)dev_err (adsp->dev,"failed to shutdown: %d\n" , ret);
@@ -197,7 +348,7 @@ static irqreturn_t adsp_fatal_interrupt(int irq, void *dev)size_t len;char *msg; - msg =qcom_smem_get (QCOM_SMEM_HOST_ANY ,ADSP_CRASH_REASON_SMEM , &len); + msg =qcom_smem_get (QCOM_SMEM_HOST_ANY , adsp->crash_reason_smem, &len);if (!IS_ERR (msg) && len >0 && msg[0 ])dev_err (adsp->dev,"fatal error received: %s\n" , msg);
@@ -232,32 +383,6 @@ static irqreturn_t adsp_stop_ack_interrupt(int irq, void *dev)return IRQ_HANDLED ; } -static int adsp_init_clock (struct qcom_adsp *adsp) -{ -int ret; - - adsp->xo =devm_clk_get (adsp->dev,"xo" ); -if (IS_ERR (adsp->xo)) { - ret =PTR_ERR (adsp->xo); -if (ret != -EPROBE_DEFER ) -dev_err (adsp->dev,"failed to get xo clock" ); -return ret; - } - -return 0 ; -} - -static int adsp_init_regulator (struct qcom_adsp *adsp) -{ - adsp->cx_supply =devm_regulator_get (adsp->dev,"cx" ); -if (IS_ERR (adsp->cx_supply)) -return PTR_ERR (adsp->cx_supply); - -regulator_set_load (adsp->cx_supply,100000 ); - -return 0 ; -} - static int adsp_request_irq(struct qcom_adsp *adsp, struct platform_device *pdev, const char *name,
@@ -311,20 +436,25 @@ static int adsp_alloc_memory_region(struct qcom_adsp *adsp)static int adsp_probe (struct platform_device *pdev) { +const struct generic_rproc_res *desc;struct qcom_adsp *adsp;struct rproc *rproc;int ret; + desc =of_device_get_match_data (&pdev->dev); +if (!desc) +return -EINVAL ; +if (!qcom_scm_is_available ())return -EPROBE_DEFER ; -if (!qcom_scm_pas_supported (ADSP_PAS_ID )) { +if (!qcom_scm_pas_supported (desc->fw_pas_id)) {dev_err (&pdev->dev,"PAS is not available for ADSP\n" );return -ENXIO ; } rproc =rproc_alloc (&pdev->dev, pdev->name, &adsp_ops, -ADSP_FIRMWARE_NAME ,sizeof (*adsp)); + desc->firmware_name,sizeof (*adsp));if (!rproc) {dev_err (&pdev->dev,"unable to allocate remoteproc\n" );return -ENOMEM ;
@@ -344,13 +474,24 @@ static int adsp_probe(struct platform_device *pdev)if (ret) goto free_rproc; - ret =adsp_init_clock (adsp); -if (ret) + adsp->fw_pas_id = desc->fw_pas_id; + adsp->crash_reason_smem = desc->crash_reason_smem; + + ret =generic_init_clocks (&pdev->dev, + adsp->clocks, desc->clocks_name); +if (ret <0 ) { +dev_err (&pdev->dev,"Failed to get clocks.\n" ); goto free_rproc; + } + adsp->clk_count = ret; - ret =adsp_init_regulator (adsp); -if (ret) + ret =generic_regulator_init (&pdev->dev, + adsp->regulators, desc->reg_res); +if (ret <0 ) { +dev_err (&pdev->dev,"Failed to get regulators.\n" ); goto free_rproc; + } + adsp->reg_count = ret; ret =adsp_request_irq (adsp, pdev,"wdog" , adsp_wdog_interrupt);if (ret <0 )
@@ -407,9 +548,25 @@ static int adsp_remove(struct platform_device *pdev)return 0 ; } +static const struct generic_rproc_res adsp_resource_init = { + .crash_reason_smem =423 , + .firmware_name ="adsp.mdt" , + .fw_pas_id =1 , + .reg_res = (struct regulator_res []) { + { + .supply ="vdd_cx" , + }, + {}, + {} + }, + .clocks_name = (char *[]){ +"xo" , +NULL + }, +};static const struct of_device_id adsp_of_match[] = { - { .compatible ="qcom,msm8974-adsp-pil" }, - { .compatible ="qcom,msm8996-adsp-pil" }, + { .compatible ="qcom,msm8974-adsp-pil" , .data = &adsp_resource_init}, + { .compatible ="qcom,msm8996-adsp-pil" , .data = &adsp_resource_init}, { }, };MODULE_DEVICE_TABLE (of, adsp_of_match);
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [RFC PATCH v2 1/3] remoteproc: qcom: make adsp resource handling generic.
Quoted 6 lines · Avaneesh Kumar Dwivedi
This patch make resource handling generic in adsp remoteproc driver. Resources which were initialized with hard definition are being initialized now based on compatible string. Generic way of resource initialization and handling is required so that slpi processor boot support can also be enabled with same driver.
I think adding a "generic way of resource initialization and handling" is overkill for this driver, at this point in time at least.
Quoted 17 lines
Signed-off-by Avaneesh Kumar Dwivedi <[email protected]>--- drivers/remoteproc/qcom_adsp_pil.c | 259 +++++++++++++++++++++++++++++-------- 1 file changed, 208 insertions(+), 51 deletions(-)drivers/remoteproc/qcom_adsp_pil.c +1
-3@@ -32,9 +32,26 @@#include "qcom_mdt_loader.h" #include "remoteproc_internal.h" -#define ADSP_CRASH_REASON_SMEM 423 -#define ADSP_FIRMWARE_NAME "adsp.mdt" -#define ADSP_PAS_ID 1 +struct reg_info {
drivers/remoteproc/qcom_adsp_pil.c line 45
Quoted 10 lines
drivers/remoteproc/qcom_adsp_pil.c +10
-0+struct regulator *reg; +int uV; +int uA; +}; + +struct regulator_res { +const char *supply; +int uV; +int uA; +};
As far as I can see we have 2 variables:
1) Do we have px-supply?
We could just always devm_regualator_get("px"), in the adsp case this would give us a dummy regulator that we can still regulator_enable/disable. So we can keep the code unconditional.
If we want to save the reader of the kernel log a printout about a dummy voltage we can carry a "bool has_px" in the adsp_data.
The mechanism for controlling corner voltages will be something other than the regulator api, so let's not design the driver for voltage/current selection yet.
2) Do we have aggre2_noc clock?
This info we can carry with a bool in the data struct, making the devm_clk_get("aggre2") depend on this or leave the clk NULL - also calling this unconditionally.
drivers/remoteproc/qcom_adsp_pil.c line 47
Quoted 2 lines
drivers/remoteproc/qcom_adsp_pil.c +2
-0+ +struct generic_rproc_res {
Please name this "adsp_data".
drivers/remoteproc/qcom_adsp_pil.c line 50
Quoted 3 lines
drivers/remoteproc/qcom_adsp_pil.c +3
-0+int crash_reason_smem; +const char *firmware_name; +int fw_pas_id;
"pas_id" is enough.
Quoted 9 lines
drivers/remoteproc/qcom_adsp_pil.c +4
-0+struct regulator_res reg_res[3 ]; +char **clocks_name; +}; +struct qcom_adsp {struct device *dev;@@ -49,9 +66,13 @@ struct qcom_adsp {struct qcom_smem_state *state;
drivers/remoteproc/qcom_adsp_pil.c struct qcom_adsp { · line 74
Quoted 10 lines
drivers/remoteproc/qcom_adsp_pil.c +5
-2unsigned stop_bit; -struct clk *xo; +struct clk *clocks[3 ]; +int clk_count; +struct reg_info regulators[3 ]; +int reg_count; -struct regulator *cx_supply; +int fw_pas_id;
"pas_id"
Quoted 8 lines
drivers/remoteproc/qcom_adsp_pil.c +1
-0+int crash_reason_smem;struct completion start_done;struct completion stop_done;@@ -62,6 +83,136 @@ struct qcom_adsp {size_t mem_size; };
[..]
drivers/remoteproc/qcom_adsp_pil.c static irqreturn_t adsp_stop_ack_interrupt(int irq, void *dev) · line 246
Quoted 13 lines
drivers/remoteproc/qcom_adsp_pil.c +0
-12-static int adsp_init_clock (struct qcom_adsp *adsp) -{ -int ret; - - adsp->xo =devm_clk_get (adsp->dev,"xo" ); -if (IS_ERR (adsp->xo)) { - ret =PTR_ERR (adsp->xo); -if (ret != -EPROBE_DEFER ) -dev_err (adsp->dev,"failed to get xo clock" ); -return ret; - } -
Just do:
if (adsp->has_aggre2_clk) {
adsp->aggre2_clk = devm_clk_get(..);
...
}drivers/remoteproc/qcom_adsp_pil.c static irqreturn_t adsp_stop_ack_interrupt(int irq, void *dev) · line 256
Quoted 10 lines
drivers/remoteproc/qcom_adsp_pil.c +0
-10-return 0 ; -} - -static int adsp_init_regulator (struct qcom_adsp *adsp) -{ - adsp->cx_supply =devm_regulator_get (adsp->dev,"cx" ); -if (IS_ERR (adsp->cx_supply)) -return PTR_ERR (adsp->cx_supply); - -regulator_set_load (adsp->cx_supply,100000 );
If you just request "px" unconditionally here we will get a print in the log informing us that we got a dummy regulator. If we want to be fancy and not have that you can do a boolean.
Quoted 4 lines
drivers/remoteproc/qcom_adsp_pil.c +0
-4- -return 0 ; -} -
[..]
Quoted 7 lines
drivers/remoteproc/qcom_adsp_pil.c +6
-0+static const struct generic_rproc_res adsp_resource_init = { + .crash_reason_smem =423 , + .firmware_name ="adsp.mdt" , + .fw_pas_id =1 , + .reg_res = (struct regulator_res []) { + {
drivers/remoteproc/qcom_adsp_pil.c static int adsp_remove(struct platform_device *pdev) · line 566
Quoted 10 lines
drivers/remoteproc/qcom_adsp_pil.c +10
-0+ .supply = "vdd_cx", + }, + {}, + {} + }, + .clocks_name = (char*[]){ +"xo" , +NULL + }, +};
Please leave this a empty line between these.
drivers/remoteproc/qcom_adsp_pil.c static int adsp_remove(struct platform_device *pdev) · line 572
Quoted 8 lines
static const struct
of_device_idadsp_of_match[] = {drivers/remoteproc/qcom_adsp_pil.c +2
-2- { .compatible = "qcom,msm8974-adsp-pil" }, - { .compatible = "qcom,msm8996-adsp-pil" }, + { .compatible = "qcom,msm8974-adsp-pil", .data = &adsp_resource_init}, + { .compatible = "qcom,msm8996-adsp-pil", .data = &adsp_resource_init}, { }, };MODULE_DEVICE_TABLE (of, adsp_of_match);
Regards,
Bjorn
Re: [RFC PATCH v2 1/3] remoteproc: qcom: make adsp resource handling generic.
Quoted 10 lines · Bjorn Andersson
Quoted 6 lines · Avaneesh Kumar Dwivedi
This patch make resource handling generic in adsp remoteproc driver. Resources which were initialized with hard definition are being initialized now based on compatible string. Generic way of resource initialization and handling is required so that slpi processor boot support can also be enabled with same driver.
I think adding a "generic way of resource initialization and handling" is overkill for this driver, at this point in time at least.
OK, So you suggest to add necessary regulator and clock API's unconditionally in existing driver? Will do it.
Quoted 30 lines
Quoted 18 lines
Signed-off-by Avaneesh Kumar Dwivedi <[email protected]>--- drivers/remoteproc/qcom_adsp_pil.c | 259 +++++++++++++++++++++++++++++-------- 1 file changed, 208 insertions(+), 51 deletions(-)drivers/remoteproc/qcom_adsp_pil.c +1
-3@@ -32,9 +32,26 @@#include "qcom_mdt_loader.h" #include "remoteproc_internal.h" -#define ADSP_CRASH_REASON_SMEM 423 -#define ADSP_FIRMWARE_NAME "adsp.mdt" -#define ADSP_PAS_ID 1 +struct reg_info {drivers/remoteproc/qcom_adsp_pil.c line 45
Quoted 10 lines
drivers/remoteproc/qcom_adsp_pil.c +10
-0+struct regulator *reg; +int uV; +int uA; +}; + +struct regulator_res { +const char *supply; +int uV; +int uA; +};As far as I can see we have 2 variables:
Quoted 1 line
1) Do we have px-supply?
Yes as per downstream code.
Quoted 4 lines
We could just always
devm_regualator_get("px"), in the adsp case this would give us a dummy regulator that we can still regulator_enable/disable. So we can keep the code unconditional.
OK.
Quoted 3 lines
If we want to save the reader of the kernel log a printout about a dummy voltage we can carry a "bool
has_px" in theadsp_data.
OK.
Quoted 4 lines
The mechanism for controlling corner voltages will be something other than the regulator api, so let's not design the driver for voltage/current selection yet.
Do you mean need not to initialize voltage/current reading to vote as done in patch, where i have initialized voltage rating as 5 to use with regulator_set_voltage() API. If so, the modifications made for slpi will can not be tested for boot validation(albeit with regulator hack that i locally have).
Quoted 2 lines
2) Do we have
aggre2_nocclock?
YES as per Downstream code.
Quoted 4 lines
This info we can carry with a bool in the data struct, making the
devm_clk_get("aggre2")depend on this or leave the clk NULL - also calling this unconditionally.
OK.
Quoted 4 lines
drivers/remoteproc/qcom_adsp_pil.c line 47
Quoted 3 lines
drivers/remoteproc/qcom_adsp_pil.c +2
-0+ +struct generic_rproc_res {Please name this "
adsp_data".
OK.
Quoted 5 lines
drivers/remoteproc/qcom_adsp_pil.c line 50
Quoted 4 lines
drivers/remoteproc/qcom_adsp_pil.c +3
-0+int crash_reason_smem; +const char *firmware_name; +int fw_pas_id;"
pas_id" is enough.
OK.
Quoted 18 lines
Quoted 18 lines
drivers/remoteproc/qcom_adsp_pil.c +8
-1+struct regulator_res reg_res[3 ]; +char **clocks_name; +}; +struct qcom_adsp {struct device *dev;@@ -49,9 +66,13 @@ struct qcom_adsp {struct qcom_smem_state *state;unsigned stop_bit; -struct clk *xo; +struct clk *clocks[3 ]; +int clk_count; +struct reg_info regulators[3 ]; +int reg_count;
Quoted 3 lines
drivers/remoteproc/qcom_adsp_pil.c struct qcom_adsp { · line 74
Quoted 2 lines
drivers/remoteproc/qcom_adsp_pil.c +1
-1-struct regulator *cx_supply; +int fw_pas_id;"
pas_id"
OK
Quoted 28 lines
Quoted 8 lines
drivers/remoteproc/qcom_adsp_pil.c +1
-0+int crash_reason_smem;struct completion start_done;struct completion stop_done;@@ -62,6 +83,136 @@ struct qcom_adsp {size_t mem_size; };[..]
drivers/remoteproc/qcom_adsp_pil.c static irqreturn_t adsp_stop_ack_interrupt(int irq, void *dev) · line 246
Quoted 13 lines
drivers/remoteproc/qcom_adsp_pil.c +0
-12-static int adsp_init_clock (struct qcom_adsp *adsp) -{ -int ret; - - adsp->xo =devm_clk_get (adsp->dev,"xo" ); -if (IS_ERR (adsp->xo)) { - ret =PTR_ERR (adsp->xo); -if (ret != -EPROBE_DEFER ) -dev_err (adsp->dev,"failed to get xo clock" ); -return ret; - } -Just do:
if (adsp->has_aggre2_clk) { adsp->aggre2_clk = devm_clk_get(..); ... }
Quoted 13 lines
drivers/remoteproc/qcom_adsp_pil.c static irqreturn_t adsp_stop_ack_interrupt(int irq, void *dev) · line 256
Quoted 10 lines
drivers/remoteproc/qcom_adsp_pil.c +0
-10-return 0 ; -} - -static int adsp_init_regulator (struct qcom_adsp *adsp) -{ - adsp->cx_supply =devm_regulator_get (adsp->dev,"cx" ); -if (IS_ERR (adsp->cx_supply)) -return PTR_ERR (adsp->cx_supply); - -regulator_set_load (adsp->cx_supply,100000 );If you just request "px" unconditionally here we will get a print in the log informing us that we got a dummy regulator. If we want to be fancy and not have that you can do a boolean.
OK i will prefer to have a boolean flag for clock and regulator.
Quoted 16 lines
Quoted 5 lines
drivers/remoteproc/qcom_adsp_pil.c +0
-4- -return 0 ; -} -[..]
Quoted 10 lines
drivers/remoteproc/qcom_adsp_pil.c +9
-0+static const struct generic_rproc_res adsp_resource_init = { + .crash_reason_smem =423 , + .firmware_name ="adsp.mdt" , + .fw_pas_id =1 , + .reg_res = (struct regulator_res []) { + { + .supply ="vdd_cx" , + }, + {},
Quoted 8 lines
drivers/remoteproc/qcom_adsp_pil.c static int adsp_remove(struct platform_device *pdev) · line 566
Quoted 7 lines
drivers/remoteproc/qcom_adsp_pil.c +7
-0+ {} + }, + .clocks_name = (char*[]){ +"xo" , +NULL + }, +};Please leave this a empty line between these.
OK.
Quoted 11 lines
drivers/remoteproc/qcom_adsp_pil.c static int adsp_remove(struct platform_device *pdev) · line 572
Quoted 9 lines
static const struct of_device_id adsp_of_match[] = {drivers/remoteproc/qcom_adsp_pil.c +2
-2- { .compatible = "qcom,msm8974-adsp-pil" }, - { .compatible = "qcom,msm8996-adsp-pil" }, + { .compatible = "qcom,msm8974-adsp-pil", .data = &adsp_resource_init}, + { .compatible = "qcom,msm8996-adsp-pil", .data = &adsp_resource_init}, { }, };MODULE_DEVICE_TABLE (of, adsp_of_match);Regards,
Bjorn
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
RFC PATCH v2 2/3 remoteproc: qcom: Add slpi boot support in adsp rproc driver.
In reply to Avaneesh Kumar DwivediThis patch add support to boot slpi processor in existing adsp rproc driver. Resource differentitation is done based on compatible string.
--- .../devicetree/bindings/remoteproc/qcom,adsp.txt | 29 ++++++++++++++++++++++ drivers/remoteproc/qcom_adsp_pil.c | 23 +++++++++++++++++ 2 files changed, 52 insertions(+)
Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt +29 -0
@@ -9,6 +9,7 @@ on the Qualcomm ADSP Hexagon core. Definition: must be one of: "qcom,msm8974-adsp-pil" "qcom,msm8996-adsp-pil" + "qcom,msm8996-slpi-pil" - interrupts-extended: Usage: required
@@ -96,3 +97,31 @@ ADSP, as it is found on MSM8974 boards. qcom,smd-edge = <1>; }; }; + +The following example describes the resources needed to boot control the +SLPI, as it is found on MSM8996 boards. + + slpi { + compatible = "qcom,msm8996-slpi-pil"; + interrupts-extended = <&intc 0 390 IRQ_TYPE_EDGE_RISING>, + <&slpi_smp2p_in 0 IRQ_TYPE_EDGE_RISING>, + <&slpi_smp2p_in 1 IRQ_TYPE_EDGE_RISING>, + <&slpi_smp2p_in 2 IRQ_TYPE_EDGE_RISING>, + <&slpi_smp2p_in 3 IRQ_TYPE_EDGE_RISING>; + interrupt-names = "wdog", + "fatal", + "ready", + "handover", + "stop-ack"; + + clocks = <&rpmcc MSM8996_RPM_SMD_XO_CLK_SRC>, + <&rpmcc MSM8996_RPM_SMD_AGGR2_NOC_CLK>; + clock-names = "xo", + "aggre2"; + vdd_cx-supply = <&pm8994_l26_corner>; + vdd_px-supply = <&pm8994_lvs2>; + memory-region = <&slpi_region>; + qcom,smem-states = <&slpi_smp2p_out 0>; + qcom,smem-state-names = "stop"; + }; +
drivers/remoteproc/qcom_adsp_pil.c +23 -0
@@ -564,9 +564,32 @@ static int adsp_remove(struct platform_device *pdev)NULL }, }; + +static const struct generic_rproc_res slpi_resource_init = { + .crash_reason_smem =424 , + .firmware_name ="slpi.mdt" , + .fw_pas_id =12 , + .reg_res = (struct regulator_res []) { + { + .supply ="vdd_cx" , + .uV =5 , + }, + { + .supply ="vdd_px" , + }, + {} + }, + .clocks_name = (char *[]){ +"xo" , +"aggre2" , +NULL + }, +}; +static const struct of_device_id adsp_of_match[] = { { .compatible ="qcom,msm8974-adsp-pil" , .data = &adsp_resource_init}, { .compatible ="qcom,msm8996-adsp-pil" , .data = &adsp_resource_init}, + { .compatible ="qcom,msm8996-slpi-pil" , .data = &slpi_resource_init}, { }, };MODULE_DEVICE_TABLE (of, adsp_of_match);
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
RFC PATCH v2 3/3 arm64: dts: msm8996: Add SLPI SMP2P dt node.
In reply to Avaneesh Kumar DwivediAdd smp2p support to communicate with slpi processor.
--- arch/arm64/boot/dts/qcom/msm8996.dtsi | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
arch/arm64/boot/dts/qcom/msm8996.dtsi +24 -0
@@ -541,5 +541,29 @@ #interrupt-cells = <2>; }; }; + + smp2p-slpi { + compatible = "qcom,smp2p"; + qcom,smem = <481>, <430>; + + interrupts = <GIC_SPI 178 IRQ_TYPE_EDGE_RISING>; + + qcom,ipc = <&apcs 16 26>; + + qcom,local-pid = <0>; + qcom,remote-pid = <3>; + + slpi_smp2p_in: slave-kernel { + qcom,entry-name = "slave-kernel"; + interrupt-controller; + #interrupt-cells = <2>; + }; + + slpi_smp2p_out: master-kernel { + qcom,entry-name = "master-kernel"; + #qcom,state-cells = <1>; + }; + }; + }; #include "msm8996-pins.dtsi"
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [RFC PATCH v2 3/3] arm64: dts: msm8996: Add SLPI SMP2P dt node.
Quoted 26 lines · Avaneesh Kumar Dwivedi
Add smp2p support to communicate with slpi processor.
Signed-off-by Avaneesh Kumar Dwivedi <[email protected]>--- arch/arm64/boot/dts/qcom/msm8996.dtsi | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)arch/arm64/boot/dts/qcom/msm8996.dtsi +11
-0@@ -541,5 +541,29 @@ #interrupt-cells = <2>; }; }; + + smp2p-slpi { + compatible = "qcom,smp2p"; + qcom,smem = <481>, <430>; + + interrupts = <GIC_SPI 178 IRQ_TYPE_EDGE_RISING>; + + qcom,ipc = <&apcs 16 26>; + + qcom,local-pid = <0>; + qcom,remote-pid = <3>;
arch/arm64/boot/dts/qcom/msm8996.dtsi line 564
Quoted 10 lines
arch/arm64/boot/dts/qcom/msm8996.dtsi +10
-0+ + slpi_smp2p_in: slave-kernel { + qcom,entry-name = "slave-kernel"; + interrupt-controller; + #interrupt-cells = <2>; + }; + + slpi_smp2p_out: master-kernel { + qcom,entry-name = "master-kernel"; + #qcom,state-cells = <1>;
Sorry for missing this earlier, but this is "#qcom,smem-state-cells" in mainline.
Apart of that:
Regards,
Bjorn
Re: [RFC PATCH v2 3/3] arm64: dts: msm8996: Add SLPI SMP2P dt node.
Quoted 32 lines · Bjorn Andersson
Quoted 30 lines · Avaneesh Kumar Dwivedi
Add smp2p support to communicate with slpi processor.
Signed-off-by Avaneesh Kumar Dwivedi <[email protected]>--- arch/arm64/boot/dts/qcom/msm8996.dtsi | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)arch/arm64/boot/dts/qcom/msm8996.dtsi +15
-0@@ -541,5 +541,29 @@ #interrupt-cells = <2>; }; }; + + smp2p-slpi { + compatible = "qcom,smp2p"; + qcom,smem = <481>, <430>; + + interrupts = <GIC_SPI 178 IRQ_TYPE_EDGE_RISING>; + + qcom,ipc = <&apcs 16 26>; + + qcom,local-pid = <0>; + qcom,remote-pid = <3>; + + slpi_smp2p_in: slave-kernel { + qcom,entry-name = "slave-kernel"; + interrupt-controller;
Quoted 8 lines
arch/arm64/boot/dts/qcom/msm8996.dtsi line 564
Quoted 6 lines
arch/arm64/boot/dts/qcom/msm8996.dtsi +6
-0+ #interrupt-cells = <2>; + }; + + slpi_smp2p_out: master-kernel { + qcom,entry-name = "master-kernel"; + #qcom,state-cells = <1>;Sorry for missing this earlier, but this is "#qcom,smem-state-cells" in mainline
OK thanks for pointing this, may i know if it doesn't find proper "qcom, smem-state-cells" entry for smem-state is their any fallback option or smem set/reset will not reflect at all at remoteproc?
Quoted 7 lines
Apart of that:
Reviewed-by Bjorn Andersson <[email protected]>Regards,
Bjorn
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.