[PATCH 1/5] thermal: tegra-bpmp: Handle errors in BPMP response

[PATCH 1/5] thermal: tegra-bpmp: Handle errors in BPMP response

[Mikko Perttunen]

The return value from tegra_bpmp_transfer indicates the success or
failure of the IPC transaction with BPMP. If the transaction
succeeded, we also need to check the actual command's result code.
Add code to do this.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/thermal/tegra/tegra-bpmp-thermal.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/tegra/tegra-bpmp-thermal.c b/drivers/thermal/tegra/tegra-bpmp-thermal.c
index 94f1da1dcd69..5affc3d196be 100644
--- a/drivers/thermal/tegra/tegra-bpmp-thermal.c
+++ b/drivers/thermal/tegra/tegra-bpmp-thermal.c
@@ -52,6 +52,8 @@ static int tegra_bpmp_thermal_get_temp(void *data, int *out_temp)
 	err = tegra_bpmp_transfer(zone->tegra->bpmp, &msg);
 	if (err)
 		return err;
+	if (msg.rx.ret)
+		return -EINVAL;
 
 	*out_temp = reply.get_temp.temp;
 
@@ -63,6 +65,7 @@ static int tegra_bpmp_thermal_set_trips(void *data, int low, int high)
 	struct tegra_bpmp_thermal_zone *zone = data;
 	struct mrq_thermal_host_to_bpmp_request req;
 	struct tegra_bpmp_message msg;
+	int err;
 
 	memset(&req, 0, sizeof(req));
 	req.type = CMD_THERMAL_SET_TRIP;
@@ -76,7 +79,13 @@ static int tegra_bpmp_thermal_set_trips(void *data, int low, int high)
 	msg.tx.data = &req;
 	msg.tx.size = sizeof(req);
 
-	return tegra_bpmp_transfer(zone->tegra->bpmp, &msg);
+	err = tegra_bpmp_transfer(zone->tegra->bpmp, &msg);
+	if (err)
+		return err;
+	if (msg.rx.ret)
+		return -EINVAL;
+
+	return 0;
 }
 
 static void tz_device_update_work_fn(struct work_struct *work)
@@ -140,6 +149,8 @@ static int tegra_bpmp_thermal_get_num_zones(struct tegra_bpmp *bpmp,
 	err = tegra_bpmp_transfer(bpmp, &msg);
 	if (err)
 		return err;
+	if (msg.rx.ret)
+		return -EINVAL;
 
 	*num_zones = reply.get_num_zones.num;
 
-- 
2.32.0

[PATCH 2/5] reset: tegra-bpmp: Handle errors in BPMP response

[Mikko Perttunen]

The return value from tegra_bpmp_transfer indicates the success or
failure of the IPC transaction with BPMP. If the transaction
succeeded, we also need to check the actual command's result code.
Add code to do this.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/reset/tegra/reset-bpmp.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/reset/tegra/reset-bpmp.c b/drivers/reset/tegra/reset-bpmp.c
index 24d3395964cc..4c5bba52b105 100644
--- a/drivers/reset/tegra/reset-bpmp.c
+++ b/drivers/reset/tegra/reset-bpmp.c
@@ -20,6 +20,7 @@ static int tegra_bpmp_reset_common(struct reset_controller_dev *rstc,
 	struct tegra_bpmp *bpmp = to_tegra_bpmp(rstc);
 	struct mrq_reset_request request;
 	struct tegra_bpmp_message msg;
+	int err;
 
 	memset(&request, 0, sizeof(request));
 	request.cmd = command;
@@ -30,7 +31,13 @@ static int tegra_bpmp_reset_common(struct reset_controller_dev *rstc,
 	msg.tx.data = &request;
 	msg.tx.size = sizeof(request);
 
-	return tegra_bpmp_transfer(bpmp, &msg);
+	err = tegra_bpmp_transfer(bpmp, &msg);
+	if (err)
+		return err;
+	if (msg.rx.ret)
+		return -EINVAL;
+
+	return 0;
 }
 
 static int tegra_bpmp_reset_module(struct reset_controller_dev *rstc,
-- 
2.32.0

Re: [PATCH 2/5] reset: tegra-bpmp: Handle errors in BPMP response

[Philipp Zabel]

On Wed, 2021-09-15 at 11:55 +0300, Mikko Perttunen wrote:
> The return value from tegra_bpmp_transfer indicates the success or
> failure of the IPC transaction with BPMP. If the transaction
> succeeded, we also need to check the actual command's result code.
> Add code to do this.
> 
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>

Thank you, applied to reset/fixes.

regards
Philipp

Re: [PATCH 2/5] reset: tegra-bpmp: Handle errors in BPMP response

[Jon Hunter]

Hi Philipp,

On 15/09/2021 09:55, Mikko Perttunen wrote:
> The return value from tegra_bpmp_transfer indicates the success or
> failure of the IPC transaction with BPMP. If the transaction
> succeeded, we also need to check the actual command's result code.
> Add code to do this.
> 
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
>   drivers/reset/tegra/reset-bpmp.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/reset/tegra/reset-bpmp.c b/drivers/reset/tegra/reset-bpmp.c
> index 24d3395964cc..4c5bba52b105 100644
> --- a/drivers/reset/tegra/reset-bpmp.c
> +++ b/drivers/reset/tegra/reset-bpmp.c
> @@ -20,6 +20,7 @@ static int tegra_bpmp_reset_common(struct reset_controller_dev *rstc,
>   	struct tegra_bpmp *bpmp = to_tegra_bpmp(rstc);
>   	struct mrq_reset_request request;
>   	struct tegra_bpmp_message msg;
> +	int err;
>   
>   	memset(&request, 0, sizeof(request));
>   	request.cmd = command;
> @@ -30,7 +31,13 @@ static int tegra_bpmp_reset_common(struct reset_controller_dev *rstc,
>   	msg.tx.data = &request;
>   	msg.tx.size = sizeof(request);
>   
> -	return tegra_bpmp_transfer(bpmp, &msg);
> +	err = tegra_bpmp_transfer(bpmp, &msg);
> +	if (err)
> +		return err;
> +	if (msg.rx.ret)
> +		return -EINVAL;
> +
> +	return 0;
>   }
>   
>   static int tegra_bpmp_reset_module(struct reset_controller_dev *rstc,
> 


I see that you have pulled this into the mainline for v5.15. 
Unfortunately, this is causing a regression for the Tegra HDA 
controller. We need to fix the Tegra HDA driver but this is too late now 
for v5.15 and so we need to revert this change for v5.15. Sorry about 
this, but I did not expect this to be pulled in so late.

Jon

-- 
nvpublic

Re: [PATCH 2/5] reset: tegra-bpmp: Handle errors in BPMP response

[Philipp Zabel]

Hi Jon,

On Thu, 2021-10-28 at 12:54 +0100, Jon Hunter wrote:
> Hi Philipp,
> 
> On 15/09/2021 09:55, Mikko Perttunen wrote:
> > The return value from tegra_bpmp_transfer indicates the success or
> > failure of the IPC transaction with BPMP. If the transaction
> > succeeded, we also need to check the actual command's result code.
> > Add code to do this.
> > 
> > Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> > ---
> >   drivers/reset/tegra/reset-bpmp.c | 9 ++++++++-
> >   1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/reset/tegra/reset-bpmp.c b/drivers/reset/tegra/reset-bpmp.c
> > index 24d3395964cc..4c5bba52b105 100644
> > --- a/drivers/reset/tegra/reset-bpmp.c
> > +++ b/drivers/reset/tegra/reset-bpmp.c
> > @@ -20,6 +20,7 @@ static int tegra_bpmp_reset_common(struct reset_controller_dev *rstc,
> >   	struct tegra_bpmp *bpmp = to_tegra_bpmp(rstc);
> >   	struct mrq_reset_request request;
> >   	struct tegra_bpmp_message msg;
> > +	int err;
> >   
> > 
> >   	memset(&request, 0, sizeof(request));
> >   	request.cmd = command;
> > @@ -30,7 +31,13 @@ static int tegra_bpmp_reset_common(struct reset_controller_dev *rstc,
> >   	msg.tx.data = &request;
> >   	msg.tx.size = sizeof(request);
> >   
> > 
> > -	return tegra_bpmp_transfer(bpmp, &msg);
> > +	err = tegra_bpmp_transfer(bpmp, &msg);
> > +	if (err)
> > +		return err;
> > +	if (msg.rx.ret)
> > +		return -EINVAL;
> > +
> > +	return 0;
> >   }
> >   
> > 
> >   static int tegra_bpmp_reset_module(struct reset_controller_dev *rstc,
> 
> I see that you have pulled this into the mainline for v5.15. 
> Unfortunately, this is causing a regression for the Tegra HDA 
> controller. We need to fix the Tegra HDA driver but this is too late now 
> for v5.15 and so we need to revert this change for v5.15. Sorry about 
> this, but I did not expect this to be pulled in so late.

I'm sorry, I picked this up as a fix and went on vacation. Now that
v5.15 has already been released, could you send a revert for stable?

regards
Philipp

Re: [PATCH 2/5] reset: tegra-bpmp: Handle errors in BPMP response

[Jon Hunter]

Hi Philipp,

On 02/11/2021 13:10, Philipp Zabel wrote:

...

> I'm sorry, I picked this up as a fix and went on vacation. Now that
> v5.15 has already been released, could you send a revert for stable?

No problem. I have sent a revert for this. If we could pull that into 
the mainline for v5.16 that would be great. Then I will get it 
backported to stable. We will undo this revert in the mainline once we 
have sorted out the problem with the HDA driver.

Thanks!
Jon

-- 
nvpublic

[PATCH 3/5] memory: tegra186-emc: Handle errors in BPMP response

[Mikko Perttunen]

The return value from tegra_bpmp_transfer indicates the success or
failure of the IPC transaction with BPMP. If the transaction
succeeded, we also need to check the actual command's result code.
Add code to do this.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/memory/tegra/tegra186-emc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/memory/tegra/tegra186-emc.c b/drivers/memory/tegra/tegra186-emc.c
index d65e7c2a580b..abc0c2eeaab7 100644
--- a/drivers/memory/tegra/tegra186-emc.c
+++ b/drivers/memory/tegra/tegra186-emc.c
@@ -197,6 +197,10 @@ static int tegra186_emc_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "failed to EMC DVFS pairs: %d\n", err);
 		goto put_bpmp;
 	}
+	if (msg.rx.ret < 0) {
+		dev_err(&pdev->dev, "EMC DVFS MRQ failed: %d (BPMP error code)\n", msg.rx.ret);
+		goto put_bpmp;
+	}
 
 	emc->debugfs.min_rate = ULONG_MAX;
 	emc->debugfs.max_rate = 0;
-- 
2.32.0

Re: (subset) [PATCH 3/5] memory: tegra186-emc: Handle errors in BPMP response

[Krzysztof Kozlowski]

On Wed, 15 Sep 2021 11:55:15 +0300, Mikko Perttunen wrote:
> The return value from tegra_bpmp_transfer indicates the success or
> failure of the IPC transaction with BPMP. If the transaction
> succeeded, we also need to check the actual command's result code.
> Add code to do this.
> 
> 

Applied, thanks!

[3/5] memory: tegra186-emc: Handle errors in BPMP response
      commit: 13324edbe9269e6fbca4d0f5146b18ef8478c958

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>

[PATCH 4/5] cpufreq: tegra186/tegra194: Handle errors in BPMP response

[Mikko Perttunen]

The return value from tegra_bpmp_transfer indicates the success or
failure of the IPC transaction with BPMP. If the transaction
succeeded, we also need to check the actual command's result code.
Add code to do this.

While at it, explicitly handle missing CPU clusters, which can
occur on floorswept chips. This worked before as well, but
possibly only by accident.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/cpufreq/tegra186-cpufreq.c | 4 ++++
 drivers/cpufreq/tegra194-cpufreq.c | 8 +++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/tegra186-cpufreq.c b/drivers/cpufreq/tegra186-cpufreq.c
index 5d1943e787b0..6c88827f4e62 100644
--- a/drivers/cpufreq/tegra186-cpufreq.c
+++ b/drivers/cpufreq/tegra186-cpufreq.c
@@ -159,6 +159,10 @@ static struct cpufreq_frequency_table *init_vhint_table(
 		table = ERR_PTR(err);
 		goto free;
 	}
+	if (msg.rx.ret) {
+		table = ERR_PTR(-EINVAL);
+		goto free;
+	}
 
 	for (i = data->vfloor; i <= data->vceil; i++) {
 		u16 ndiv = data->ndiv[i];
diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c
index a9620e4489ae..ac381db25dbe 100644
--- a/drivers/cpufreq/tegra194-cpufreq.c
+++ b/drivers/cpufreq/tegra194-cpufreq.c
@@ -242,7 +242,7 @@ static int tegra194_cpufreq_init(struct cpufreq_policy *policy)
 
 	smp_call_function_single(policy->cpu, get_cpu_cluster, &cl, true);
 
-	if (cl >= data->num_clusters)
+	if (cl >= data->num_clusters || !data->tables[cl])
 		return -EINVAL;
 
 	/* set same policy for all cpus in a cluster */
@@ -310,6 +310,12 @@ init_freq_table(struct platform_device *pdev, struct tegra_bpmp *bpmp,
 	err = tegra_bpmp_transfer(bpmp, &msg);
 	if (err)
 		return ERR_PTR(err);
+	if (msg.rx.ret == -BPMP_EINVAL) {
+		/* Cluster not available */
+		return NULL;
+	}
+	if (msg.rx.ret)
+		return ERR_PTR(-EINVAL);
 
 	/*
 	 * Make sure frequency table step is a multiple of mdiv to match
-- 
2.32.0

Re: [PATCH 4/5] cpufreq: tegra186/tegra194: Handle errors in BPMP response

[Viresh Kumar]

On 15-09-21, 11:55, Mikko Perttunen wrote:
> The return value from tegra_bpmp_transfer indicates the success or
> failure of the IPC transaction with BPMP. If the transaction
> succeeded, we also need to check the actual command's result code.
> Add code to do this.
> 
> While at it, explicitly handle missing CPU clusters, which can
> occur on floorswept chips. This worked before as well, but
> possibly only by accident.
> 
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
>  drivers/cpufreq/tegra186-cpufreq.c | 4 ++++
>  drivers/cpufreq/tegra194-cpufreq.c | 8 +++++++-
>  2 files changed, 11 insertions(+), 1 deletion(-)

Should I apply it alone ?

-- 
viresh

Re: [PATCH 4/5] cpufreq: tegra186/tegra194: Handle errors in BPMP response

[Mikko Perttunen]

On 10/4/21 9:37 AM, Viresh Kumar wrote:
> On 15-09-21, 11:55, Mikko Perttunen wrote:
>> The return value from tegra_bpmp_transfer indicates the success or
>> failure of the IPC transaction with BPMP. If the transaction
>> succeeded, we also need to check the actual command's result code.
>> Add code to do this.
>>
>> While at it, explicitly handle missing CPU clusters, which can
>> occur on floorswept chips. This worked before as well, but
>> possibly only by accident.
>>
>> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
>> ---
>>   drivers/cpufreq/tegra186-cpufreq.c | 4 ++++
>>   drivers/cpufreq/tegra194-cpufreq.c | 8 +++++++-
>>   2 files changed, 11 insertions(+), 1 deletion(-)
> 
> Should I apply it alone ?
> 

Yes please, all of these patches are independent of each other.

Mikko

Re: [PATCH 4/5] cpufreq: tegra186/tegra194: Handle errors in BPMP response

[Viresh Kumar]

On 15-09-21, 11:55, Mikko Perttunen wrote:
> The return value from tegra_bpmp_transfer indicates the success or
> failure of the IPC transaction with BPMP. If the transaction
> succeeded, we also need to check the actual command's result code.
> Add code to do this.
> 
> While at it, explicitly handle missing CPU clusters, which can
> occur on floorswept chips. This worked before as well, but
> possibly only by accident.
> 
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
>  drivers/cpufreq/tegra186-cpufreq.c | 4 ++++
>  drivers/cpufreq/tegra194-cpufreq.c | 8 +++++++-
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/tegra186-cpufreq.c b/drivers/cpufreq/tegra186-cpufreq.c
> index 5d1943e787b0..6c88827f4e62 100644
> --- a/drivers/cpufreq/tegra186-cpufreq.c
> +++ b/drivers/cpufreq/tegra186-cpufreq.c
> @@ -159,6 +159,10 @@ static struct cpufreq_frequency_table *init_vhint_table(
>  		table = ERR_PTR(err);
>  		goto free;
>  	}
> +	if (msg.rx.ret) {
> +		table = ERR_PTR(-EINVAL);
> +		goto free;
> +	}
>  
>  	for (i = data->vfloor; i <= data->vceil; i++) {
>  		u16 ndiv = data->ndiv[i];
> diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c
> index a9620e4489ae..ac381db25dbe 100644
> --- a/drivers/cpufreq/tegra194-cpufreq.c
> +++ b/drivers/cpufreq/tegra194-cpufreq.c
> @@ -242,7 +242,7 @@ static int tegra194_cpufreq_init(struct cpufreq_policy *policy)
>  
>  	smp_call_function_single(policy->cpu, get_cpu_cluster, &cl, true);
>  
> -	if (cl >= data->num_clusters)
> +	if (cl >= data->num_clusters || !data->tables[cl])
>  		return -EINVAL;
>  
>  	/* set same policy for all cpus in a cluster */
> @@ -310,6 +310,12 @@ init_freq_table(struct platform_device *pdev, struct tegra_bpmp *bpmp,
>  	err = tegra_bpmp_transfer(bpmp, &msg);
>  	if (err)
>  		return ERR_PTR(err);
> +	if (msg.rx.ret == -BPMP_EINVAL) {
> +		/* Cluster not available */
> +		return NULL;
> +	}
> +	if (msg.rx.ret)
> +		return ERR_PTR(-EINVAL);
>  
>  	/*
>  	 * Make sure frequency table step is a multiple of mdiv to match

Applied. Thanks.

-- 
viresh

Re: [PATCH 4/5] cpufreq: tegra186/tegra194: Handle errors in BPMP response

[Mikko Perttunen]

On 10/4/21 10:01 AM, Viresh Kumar wrote:
> On 15-09-21, 11:55, Mikko Perttunen wrote:
>> The return value from tegra_bpmp_transfer indicates the success or
>> failure of the IPC transaction with BPMP. If the transaction
>> succeeded, we also need to check the actual command's result code.
>> Add code to do this.
>>
>> While at it, explicitly handle missing CPU clusters, which can
>> occur on floorswept chips. This worked before as well, but
>> possibly only by accident.
>>
>> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
>> ---
>>   drivers/cpufreq/tegra186-cpufreq.c | 4 ++++
>>   drivers/cpufreq/tegra194-cpufreq.c | 8 +++++++-
>>   2 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/cpufreq/tegra186-cpufreq.c b/drivers/cpufreq/tegra186-cpufreq.c
>> index 5d1943e787b0..6c88827f4e62 100644
>> --- a/drivers/cpufreq/tegra186-cpufreq.c
>> +++ b/drivers/cpufreq/tegra186-cpufreq.c
>> @@ -159,6 +159,10 @@ static struct cpufreq_frequency_table *init_vhint_table(
>>   		table = ERR_PTR(err);
>>   		goto free;
>>   	}
>> +	if (msg.rx.ret) {
>> +		table = ERR_PTR(-EINVAL);
>> +		goto free;
>> +	}
>>   
>>   	for (i = data->vfloor; i <= data->vceil; i++) {
>>   		u16 ndiv = data->ndiv[i];
>> diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c
>> index a9620e4489ae..ac381db25dbe 100644
>> --- a/drivers/cpufreq/tegra194-cpufreq.c
>> +++ b/drivers/cpufreq/tegra194-cpufreq.c
>> @@ -242,7 +242,7 @@ static int tegra194_cpufreq_init(struct cpufreq_policy *policy)
>>   
>>   	smp_call_function_single(policy->cpu, get_cpu_cluster, &cl, true);
>>   
>> -	if (cl >= data->num_clusters)
>> +	if (cl >= data->num_clusters || !data->tables[cl])
>>   		return -EINVAL;
>>   
>>   	/* set same policy for all cpus in a cluster */
>> @@ -310,6 +310,12 @@ init_freq_table(struct platform_device *pdev, struct tegra_bpmp *bpmp,
>>   	err = tegra_bpmp_transfer(bpmp, &msg);
>>   	if (err)
>>   		return ERR_PTR(err);
>> +	if (msg.rx.ret == -BPMP_EINVAL) {
>> +		/* Cluster not available */
>> +		return NULL;
>> +	}
>> +	if (msg.rx.ret)
>> +		return ERR_PTR(-EINVAL);
>>   
>>   	/*
>>   	 * Make sure frequency table step is a multiple of mdiv to match
> 
> Applied. Thanks.
> 

Thanks!

Mikko

[PATCH 5/5] PCI: tegra194: Handle errors in BPMP response

[Mikko Perttunen]

The return value from tegra_bpmp_transfer indicates the success or
failure of the IPC transaction with BPMP. If the transaction
succeeded, we also need to check the actual command's result code.
Add code to do this.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/pci/controller/dwc/pcie-tegra194.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 904976913081..08afd2e72ec5 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -1162,6 +1162,7 @@ static int tegra_pcie_bpmp_set_ctrl_state(struct tegra_pcie_dw *pcie,
 	struct mrq_uphy_response resp;
 	struct tegra_bpmp_message msg;
 	struct mrq_uphy_request req;
+	int err;
 
 	/* Controller-5 doesn't need to have its state set by BPMP-FW */
 	if (pcie->cid == 5)
@@ -1181,7 +1182,13 @@ static int tegra_pcie_bpmp_set_ctrl_state(struct tegra_pcie_dw *pcie,
 	msg.rx.data = &resp;
 	msg.rx.size = sizeof(resp);
 
-	return tegra_bpmp_transfer(pcie->bpmp, &msg);
+	err = tegra_bpmp_transfer(pcie->bpmp, &msg);
+	if (err)
+		return err;
+	if (msg.rx.ret)
+		return -EINVAL;
+
+	return 0;
 }
 
 static int tegra_pcie_bpmp_set_pll_state(struct tegra_pcie_dw *pcie,
-- 
2.32.0

Re: [PATCH 5/5] PCI: tegra194: Handle errors in BPMP response

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 524 bytes --]

On Wed, Sep 15, 2021 at 11:55:17AM +0300, Mikko Perttunen wrote:
> The return value from tegra_bpmp_transfer indicates the success or
> failure of the IPC transaction with BPMP. If the transaction
> succeeded, we also need to check the actual command's result code.
> Add code to do this.
> 
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
>  drivers/pci/controller/dwc/pcie-tegra194.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 5/5] PCI: tegra194: Handle errors in BPMP response

[Lorenzo Pieralisi]

On Thu, Oct 07, 2021 at 08:21:11PM +0200, Thierry Reding wrote:
> On Wed, Sep 15, 2021 at 11:55:17AM +0300, Mikko Perttunen wrote:
> > The return value from tegra_bpmp_transfer indicates the success or
> > failure of the IPC transaction with BPMP. If the transaction
> > succeeded, we also need to check the actual command's result code.
> > Add code to do this.
> > 
> > Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> > ---
> >  drivers/pci/controller/dwc/pcie-tegra194.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> Acked-by: Thierry Reding <treding@nvidia.com>

Hi Thierry,

can I pull this patch into the PCI tree ? Or if you want the series
to go via another tree:

Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

Re: [PATCH 5/5] PCI: tegra194: Handle errors in BPMP response

[Lorenzo Pieralisi]

On Wed, Oct 13, 2021 at 01:59:56PM +0100, Lorenzo Pieralisi wrote:
> On Thu, Oct 07, 2021 at 08:21:11PM +0200, Thierry Reding wrote:
> > On Wed, Sep 15, 2021 at 11:55:17AM +0300, Mikko Perttunen wrote:
> > > The return value from tegra_bpmp_transfer indicates the success or
> > > failure of the IPC transaction with BPMP. If the transaction
> > > succeeded, we also need to check the actual command's result code.
> > > Add code to do this.
> > > 
> > > Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> > > ---
> > >  drivers/pci/controller/dwc/pcie-tegra194.c | 9 ++++++++-
> > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > Acked-by: Thierry Reding <treding@nvidia.com>
> 
> Hi Thierry,
> 
> can I pull this patch into the PCI tree ? Or if you want the series
> to go via another tree:
> 
> Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

Hi,

I would like to ask please how you want this series to be handled.

Thanks,
Lorenzo

Re: [PATCH 5/5] PCI: tegra194: Handle errors in BPMP response

[Lorenzo Pieralisi]

On Mon, Nov 29, 2021 at 12:19:18PM +0000, Lorenzo Pieralisi wrote:
> On Wed, Oct 13, 2021 at 01:59:56PM +0100, Lorenzo Pieralisi wrote:
> > On Thu, Oct 07, 2021 at 08:21:11PM +0200, Thierry Reding wrote:
> > > On Wed, Sep 15, 2021 at 11:55:17AM +0300, Mikko Perttunen wrote:
> > > > The return value from tegra_bpmp_transfer indicates the success or
> > > > failure of the IPC transaction with BPMP. If the transaction
> > > > succeeded, we also need to check the actual command's result code.
> > > > Add code to do this.
> > > > 
> > > > Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> > > > ---
> > > >  drivers/pci/controller/dwc/pcie-tegra194.c | 9 ++++++++-
> > > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > > 
> > > Acked-by: Thierry Reding <treding@nvidia.com>
> > 
> > Hi Thierry,
> > 
> > can I pull this patch into the PCI tree ? Or if you want the series
> > to go via another tree:
> > 
> > Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> 
> Hi,
> 
> I would like to ask please how you want this series to be handled.

Should I apply this patch stand-alone ?

I will mark all other patches in this series as Not Applicable in
the PCI queue.

Thanks,
Lorenzo

Re: [PATCH 1/5] thermal: tegra-bpmp: Handle errors in BPMP response

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 840 bytes --]

On Wed, Sep 15, 2021 at 11:55:13AM +0300, Mikko Perttunen wrote:
> The return value from tegra_bpmp_transfer indicates the success or
> failure of the IPC transaction with BPMP. If the transaction
> succeeded, we also need to check the actual command's result code.
> Add code to do this.
> 
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
>  drivers/thermal/tegra/tegra-bpmp-thermal.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)

Perhaps this should be moved into tegra_bpmp_transfer() or some new
helper to make sure we can keep this consistent across all callers.

For instance, I'm not sure -EINVAL is the right (or best) error code in
all the cases. Either way, seems fine in this case and this is certainly
an improvement, so:

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/5] thermal: tegra-bpmp: Handle errors in BPMP response

[Daniel Lezcano]

On 07/10/2021 20:20, Thierry Reding wrote:
> On Wed, Sep 15, 2021 at 11:55:13AM +0300, Mikko Perttunen wrote:
>> The return value from tegra_bpmp_transfer indicates the success or
>> failure of the IPC transaction with BPMP. If the transaction
>> succeeded, we also need to check the actual command's result code.
>> Add code to do this.
>>
>> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
>> ---
>>   drivers/thermal/tegra/tegra-bpmp-thermal.c | 13 ++++++++++++-
>>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> Perhaps this should be moved into tegra_bpmp_transfer() or some new
> helper to make sure we can keep this consistent across all callers.
> 
> For instance, I'm not sure -EINVAL is the right (or best) error code in
> all the cases. Either way, seems fine in this case and this is certainly
> an improvement, so:
> 
> Acked-by: Thierry Reding <treding@nvidia.com>

Applied, thanks

-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[thermal: thermal/next] thermal: tegra-bpmp: Handle errors in BPMP response

[thermal-bot for Mikko Perttunen]

The following commit has been merged into the thermal/next branch of thermal:

Commit-ID:     1a7c9213d5f9bfd5e1bf4f87df1abc166ce48ffc
Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git//1a7c9213d5f9bfd5e1bf4f87df1abc166ce48ffc
Author:        Mikko Perttunen <mperttunen@nvidia.com>
AuthorDate:    Wed, 15 Sep 2021 11:55:13 +03:00
Committer:     Daniel Lezcano <daniel.lezcano@linaro.org>
CommitterDate: Tue, 08 Mar 2022 21:26:09 +01:00

thermal: tegra-bpmp: Handle errors in BPMP response

The return value from tegra_bpmp_transfer indicates the success or
failure of the IPC transaction with BPMP. If the transaction
succeeded, we also need to check the actual command's result code.
Add code to do this.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
Acked-by: Thierry Reding <treding@nvidia.com>
Link: https://lore.kernel.org/r/20210915085517.1669675-1-mperttunen@nvidia.com
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/thermal/tegra/tegra-bpmp-thermal.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/tegra/tegra-bpmp-thermal.c b/drivers/thermal/tegra/tegra-bpmp-thermal.c
index 94f1da1..5affc3d 100644
--- a/drivers/thermal/tegra/tegra-bpmp-thermal.c
+++ b/drivers/thermal/tegra/tegra-bpmp-thermal.c
@@ -52,6 +52,8 @@ static int tegra_bpmp_thermal_get_temp(void *data, int *out_temp)
 	err = tegra_bpmp_transfer(zone->tegra->bpmp, &msg);
 	if (err)
 		return err;
+	if (msg.rx.ret)
+		return -EINVAL;
 
 	*out_temp = reply.get_temp.temp;
 
@@ -63,6 +65,7 @@ static int tegra_bpmp_thermal_set_trips(void *data, int low, int high)
 	struct tegra_bpmp_thermal_zone *zone = data;
 	struct mrq_thermal_host_to_bpmp_request req;
 	struct tegra_bpmp_message msg;
+	int err;
 
 	memset(&req, 0, sizeof(req));
 	req.type = CMD_THERMAL_SET_TRIP;
@@ -76,7 +79,13 @@ static int tegra_bpmp_thermal_set_trips(void *data, int low, int high)
 	msg.tx.data = &req;
 	msg.tx.size = sizeof(req);
 
-	return tegra_bpmp_transfer(zone->tegra->bpmp, &msg);
+	err = tegra_bpmp_transfer(zone->tegra->bpmp, &msg);
+	if (err)
+		return err;
+	if (msg.rx.ret)
+		return -EINVAL;
+
+	return 0;
 }
 
 static void tz_device_update_work_fn(struct work_struct *work)
@@ -140,6 +149,8 @@ static int tegra_bpmp_thermal_get_num_zones(struct tegra_bpmp *bpmp,
 	err = tegra_bpmp_transfer(bpmp, &msg);
 	if (err)
 		return err;
+	if (msg.rx.ret)
+		return -EINVAL;
 
 	*num_zones = reply.get_num_zones.num;