[PATCH v4 0/3] spi: geni-qcom: Undo runtime PM changes at driver exit time

[PATCH v4 0/3] spi: geni-qcom: Undo runtime PM changes at driver exit time

[Jinjie Ruan]

Fix two bugs for geni-qcom and use dev managed function
to simplify code.

Compile-tested only.

Changes in v4:
- Correct the "data" of devm_add_action_or_reset().
- Add reviewed-by.
- Update the commit message.

Changes in v3:
- Adjust the runtime PM patch to be the first.
- Use devm_pm_runtime_enable() to undo runtime PM changes.
- Land the rest of the cleanups afterwards.
- Update the commit message.

Changes in v2:
- Split out the device managed cleanup patch.
- PATCH -next -> PATCH
- Also fix the incorrect free_irq() sequence.


Jinjie Ruan (3):
  spi: geni-qcom: Undo runtime PM changes at driver exit time
  spi: geni-qcom: Fix incorrect free_irq() sequence
  spi: geni-qcom: Use devm functions to simplify code

 drivers/spi/spi-geni-qcom.c | 50 ++++++++++++++-----------------------
 1 file changed, 19 insertions(+), 31 deletions(-)

-- 
2.34.1

[PATCH v4 1/3] spi: geni-qcom: Undo runtime PM changes at driver exit time

[Jinjie Ruan]

It's important to undo pm_runtime_use_autosuspend() with
pm_runtime_dont_use_autosuspend() at driver exit time unless driver
initially enabled pm_runtime with devm_pm_runtime_enable()
(which handles it for you).

Hence, switch to devm_pm_runtime_enable() to fix it, so the
pm_runtime_disable() in probe error path and remove function
can be removed.

Fixes: cfdab2cd85ec ("spi: spi-geni-qcom: Set an autosuspend delay of 250 ms")
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
v4:
- Add reviewed-by.
v3:
- Fix it with devm_pm_runtime_enable() as Dmitry suggested.
- Adjust to be the first patch.
- Add suggested-by.
v2:
- Fix it directly instead of use devm_pm_runtime_enable().
---
 drivers/spi/spi-geni-qcom.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index 37ef8c40b276..fef522fece1b 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -1110,25 +1110,27 @@ static int spi_geni_probe(struct platform_device *pdev)
 	spin_lock_init(&mas->lock);
 	pm_runtime_use_autosuspend(&pdev->dev);
 	pm_runtime_set_autosuspend_delay(&pdev->dev, 250);
-	pm_runtime_enable(dev);
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return ret;
 
 	if (device_property_read_bool(&pdev->dev, "spi-slave"))
 		spi->target = true;
 
 	ret = geni_icc_get(&mas->se, NULL);
 	if (ret)
-		goto spi_geni_probe_runtime_disable;
+		return ret;
 	/* Set the bus quota to a reasonable value for register access */
 	mas->se.icc_paths[GENI_TO_CORE].avg_bw = Bps_to_icc(CORE_2X_50_MHZ);
 	mas->se.icc_paths[CPU_TO_GENI].avg_bw = GENI_DEFAULT_BW;
 
 	ret = geni_icc_set_bw(&mas->se);
 	if (ret)
-		goto spi_geni_probe_runtime_disable;
+		return ret;
 
 	ret = spi_geni_init(mas);
 	if (ret)
-		goto spi_geni_probe_runtime_disable;
+		return ret;
 
 	/*
 	 * check the mode supported and set_cs for fifo mode only
@@ -1157,8 +1159,6 @@ static int spi_geni_probe(struct platform_device *pdev)
 	free_irq(mas->irq, spi);
 spi_geni_release_dma:
 	spi_geni_release_dma_chan(mas);
-spi_geni_probe_runtime_disable:
-	pm_runtime_disable(dev);
 	return ret;
 }
 
@@ -1173,7 +1173,6 @@ static void spi_geni_remove(struct platform_device *pdev)
 	spi_geni_release_dma_chan(mas);
 
 	free_irq(mas->irq, spi);
-	pm_runtime_disable(&pdev->dev);
 }
 
 static int __maybe_unused spi_geni_runtime_suspend(struct device *dev)
-- 
2.34.1

[PATCH v4 2/3] spi: geni-qcom: Fix incorrect free_irq() sequence

[Jinjie Ruan]

In spi_geni_remove(), the free_irq() sequence is different from that
on the probe error path. And the IRQ will still remain and it's interrupt
handler may use the dma channel after release dma channel and before free
irq, which is not secure, fix it.

Fixes: b59c122484ec ("spi: spi-geni-qcom: Add support for GPI dma")
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
v4:
- Add reviewed-by.
v3:
- Rebased on the devm_pm_runtime_enable() patch.
- Update the commit message.
---
 drivers/spi/spi-geni-qcom.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index fef522fece1b..6f4057330444 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -1170,9 +1170,9 @@ static void spi_geni_remove(struct platform_device *pdev)
 	/* Unregister _before_ disabling pm_runtime() so we stop transfers */
 	spi_unregister_controller(spi);
 
-	spi_geni_release_dma_chan(mas);
-
 	free_irq(mas->irq, spi);
+
+	spi_geni_release_dma_chan(mas);
 }
 
 static int __maybe_unused spi_geni_runtime_suspend(struct device *dev)
-- 
2.34.1

[PATCH v4 3/3] spi: geni-qcom: Use devm functions to simplify code

[Jinjie Ruan]

Use devm_pm_runtime_enable(), devm_request_irq() and
devm_spi_register_controller() to simplify code.

And also register a callback spi_geni_release_dma_chan() with
devm_add_action_or_reset(), to release dma channel in both error
and device detach path, which can make sure the release sequence is
consistent with the original one.

1. Unregister spi controller.
2. Free the IRQ.
3. Free DMA chans
4. Disable runtime PM.

So the remove function can also be removed.

Suggested-by: Doug Anderson <dianders@chromium.org>
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
v4:
- Correct the "data" of devm_add_action_or_reset().
v3:
- Land the rest of the cleanups afterwards.
---
 drivers/spi/spi-geni-qcom.c | 37 +++++++++++++------------------------
 1 file changed, 13 insertions(+), 24 deletions(-)

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index 6f4057330444..5cb002d7d4a6 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -632,8 +632,10 @@ static int spi_geni_grab_gpi_chan(struct spi_geni_master *mas)
 	return ret;
 }
 
-static void spi_geni_release_dma_chan(struct spi_geni_master *mas)
+static void spi_geni_release_dma_chan(void *data)
 {
+	struct spi_geni_master *mas = data;
+
 	if (mas->rx) {
 		dma_release_channel(mas->rx);
 		mas->rx = NULL;
@@ -1132,6 +1134,12 @@ static int spi_geni_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	ret = devm_add_action_or_reset(dev, spi_geni_release_dma_chan, mas);
+	if (ret) {
+		dev_err(dev, "Unable to add action.\n");
+		return ret;
+	}
+
 	/*
 	 * check the mode supported and set_cs for fifo mode only
 	 * for dma (gsi) mode, the gsi will set cs based on params passed in
@@ -1146,33 +1154,15 @@ static int spi_geni_probe(struct platform_device *pdev)
 	if (mas->cur_xfer_mode == GENI_GPI_DMA)
 		spi->flags = SPI_CONTROLLER_MUST_TX;
 
-	ret = request_irq(mas->irq, geni_spi_isr, 0, dev_name(dev), spi);
+	ret = devm_request_irq(dev, mas->irq, geni_spi_isr, 0, dev_name(dev), spi);
 	if (ret)
-		goto spi_geni_release_dma;
+		return ret;
 
-	ret = spi_register_controller(spi);
+	ret = devm_spi_register_controller(dev, spi);
 	if (ret)
-		goto spi_geni_probe_free_irq;
+		return ret;
 
 	return 0;
-spi_geni_probe_free_irq:
-	free_irq(mas->irq, spi);
-spi_geni_release_dma:
-	spi_geni_release_dma_chan(mas);
-	return ret;
-}
-
-static void spi_geni_remove(struct platform_device *pdev)
-{
-	struct spi_controller *spi = platform_get_drvdata(pdev);
-	struct spi_geni_master *mas = spi_controller_get_devdata(spi);
-
-	/* Unregister _before_ disabling pm_runtime() so we stop transfers */
-	spi_unregister_controller(spi);
-
-	free_irq(mas->irq, spi);
-
-	spi_geni_release_dma_chan(mas);
 }
 
 static int __maybe_unused spi_geni_runtime_suspend(struct device *dev)
@@ -1254,7 +1244,6 @@ MODULE_DEVICE_TABLE(of, spi_geni_dt_match);
 
 static struct platform_driver spi_geni_driver = {
 	.probe  = spi_geni_probe,
-	.remove_new = spi_geni_remove,
 	.driver = {
 		.name = "geni_spi",
 		.pm = &spi_geni_pm_ops,
-- 
2.34.1

Re: [PATCH v4 3/3] spi: geni-qcom: Use devm functions to simplify code

[Doug Anderson]

Hi,

On Mon, Sep 9, 2024 at 6:19 AM Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>
> Use devm_pm_runtime_enable(), devm_request_irq() and
> devm_spi_register_controller() to simplify code.
>
> And also register a callback spi_geni_release_dma_chan() with
> devm_add_action_or_reset(), to release dma channel in both error
> and device detach path, which can make sure the release sequence is
> consistent with the original one.
>
> 1. Unregister spi controller.
> 2. Free the IRQ.
> 3. Free DMA chans
> 4. Disable runtime PM.
>
> So the remove function can also be removed.
>
> Suggested-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> ---
> v4:
> - Correct the "data" of devm_add_action_or_reset().
> v3:
> - Land the rest of the cleanups afterwards.
> ---
>  drivers/spi/spi-geni-qcom.c | 37 +++++++++++++------------------------
>  1 file changed, 13 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> index 6f4057330444..5cb002d7d4a6 100644
> --- a/drivers/spi/spi-geni-qcom.c
> +++ b/drivers/spi/spi-geni-qcom.c
> @@ -632,8 +632,10 @@ static int spi_geni_grab_gpi_chan(struct spi_geni_master *mas)
>         return ret;
>  }
>
> -static void spi_geni_release_dma_chan(struct spi_geni_master *mas)
> +static void spi_geni_release_dma_chan(void *data)
>  {
> +       struct spi_geni_master *mas = data;
> +
>         if (mas->rx) {
>                 dma_release_channel(mas->rx);
>                 mas->rx = NULL;
> @@ -1132,6 +1134,12 @@ static int spi_geni_probe(struct platform_device *pdev)
>         if (ret)
>                 return ret;
>
> +       ret = devm_add_action_or_reset(dev, spi_geni_release_dma_chan, mas);
> +       if (ret) {
> +               dev_err(dev, "Unable to add action.\n");
> +               return ret;
> +       }

Use dev_err_probe() to simplify.

ret = devm_add_action_or_reset(dev, spi_geni_release_dma_chan, mas);
if (ret)
  return dev_err_probe(dev, ret, "Unable to add action.\n");


Personally I'd also rather that you do the devm_add_action_or_reset()
call straight in spi_geni_grab_gpi_chan(). That makes it much more
obvious what's happening. You can still use dev_err_probe() in there
since it's called (indirectly) from probe. In that case you'd probably
replace the "return 0;" in that function with just "return
dev_err_probe(...)".


> @@ -1146,33 +1154,15 @@ static int spi_geni_probe(struct platform_device *pdev)
>         if (mas->cur_xfer_mode == GENI_GPI_DMA)
>                 spi->flags = SPI_CONTROLLER_MUST_TX;
>
> -       ret = request_irq(mas->irq, geni_spi_isr, 0, dev_name(dev), spi);
> +       ret = devm_request_irq(dev, mas->irq, geni_spi_isr, 0, dev_name(dev), spi);
>         if (ret)
> -               goto spi_geni_release_dma;
> +               return ret;
>
> -       ret = spi_register_controller(spi);
> +       ret = devm_spi_register_controller(dev, spi);
>         if (ret)
> -               goto spi_geni_probe_free_irq;
> +               return ret;
>
>         return 0;

You no longer need the "if" statement or even to assign to "ret". Just:

return devm_spi_register_controller(dev, spi);


Those are just nits, though. I'd be OK with:

Reviewed-by: Douglas Anderson <dianders@chromium.org>

...since Mark has already landed the first two patches, your v5 would
just contain this one patch.

-Doug

Re: [PATCH v4 3/3] spi: geni-qcom: Use devm functions to simplify code

[Jinjie Ruan]

On 2024/9/12 6:53, Doug Anderson wrote:
> Hi,
> 
> On Mon, Sep 9, 2024 at 6:19 AM Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>>
>> Use devm_pm_runtime_enable(), devm_request_irq() and
>> devm_spi_register_controller() to simplify code.
>>
>> And also register a callback spi_geni_release_dma_chan() with
>> devm_add_action_or_reset(), to release dma channel in both error
>> and device detach path, which can make sure the release sequence is
>> consistent with the original one.
>>
>> 1. Unregister spi controller.
>> 2. Free the IRQ.
>> 3. Free DMA chans
>> 4. Disable runtime PM.
>>
>> So the remove function can also be removed.
>>
>> Suggested-by: Doug Anderson <dianders@chromium.org>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> ---
>> v4:
>> - Correct the "data" of devm_add_action_or_reset().
>> v3:
>> - Land the rest of the cleanups afterwards.
>> ---
>>  drivers/spi/spi-geni-qcom.c | 37 +++++++++++++------------------------
>>  1 file changed, 13 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
>> index 6f4057330444..5cb002d7d4a6 100644
>> --- a/drivers/spi/spi-geni-qcom.c
>> +++ b/drivers/spi/spi-geni-qcom.c
>> @@ -632,8 +632,10 @@ static int spi_geni_grab_gpi_chan(struct spi_geni_master *mas)
>>         return ret;
>>  }
>>
>> -static void spi_geni_release_dma_chan(struct spi_geni_master *mas)
>> +static void spi_geni_release_dma_chan(void *data)
>>  {
>> +       struct spi_geni_master *mas = data;
>> +
>>         if (mas->rx) {
>>                 dma_release_channel(mas->rx);
>>                 mas->rx = NULL;
>> @@ -1132,6 +1134,12 @@ static int spi_geni_probe(struct platform_device *pdev)
>>         if (ret)
>>                 return ret;
>>
>> +       ret = devm_add_action_or_reset(dev, spi_geni_release_dma_chan, mas);
>> +       if (ret) {
>> +               dev_err(dev, "Unable to add action.\n");
>> +               return ret;
>> +       }
> 
> Use dev_err_probe() to simplify.
> 
> ret = devm_add_action_or_reset(dev, spi_geni_release_dma_chan, mas);
> if (ret)
>   return dev_err_probe(dev, ret, "Unable to add action.\n");

It seems that if it only return -ENOMEM or 0, using dev_err_probe() has
not not much value for many community maintainers.

> 
> 
> Personally I'd also rather that you do the devm_add_action_or_reset()
> call straight in spi_geni_grab_gpi_chan(). That makes it much more

Yes, it will be more clear.

> obvious what's happening. You can still use dev_err_probe() in there
> since it's called (indirectly) from probe. In that case you'd probably
> replace the "return 0;" in that function with just "return
> dev_err_probe(...)".
> 
> 
>> @@ -1146,33 +1154,15 @@ static int spi_geni_probe(struct platform_device *pdev)
>>         if (mas->cur_xfer_mode == GENI_GPI_DMA)
>>                 spi->flags = SPI_CONTROLLER_MUST_TX;
>>
>> -       ret = request_irq(mas->irq, geni_spi_isr, 0, dev_name(dev), spi);
>> +       ret = devm_request_irq(dev, mas->irq, geni_spi_isr, 0, dev_name(dev), spi);
>>         if (ret)
>> -               goto spi_geni_release_dma;
>> +               return ret;
>>
>> -       ret = spi_register_controller(spi);
>> +       ret = devm_spi_register_controller(dev, spi);
>>         if (ret)
>> -               goto spi_geni_probe_free_irq;
>> +               return ret;
>>
>>         return 0;
> 
> You no longer need the "if" statement or even to assign to "ret". Just:
> 
> return devm_spi_register_controller(dev, spi);

Right!

> 
> 
> Those are just nits, though. I'd be OK with:
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> 
> ...since Mark has already landed the first two patches, your v5 would
> just contain this one patch.
> 
> -Doug

Re: [PATCH v4 3/3] spi: geni-qcom: Use devm functions to simplify code

[Doug Anderson]

Hi,

On Wed, Sep 11, 2024 at 8:53 PM Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>
> >> @@ -1132,6 +1134,12 @@ static int spi_geni_probe(struct platform_device *pdev)
> >>         if (ret)
> >>                 return ret;
> >>
> >> +       ret = devm_add_action_or_reset(dev, spi_geni_release_dma_chan, mas);
> >> +       if (ret) {
> >> +               dev_err(dev, "Unable to add action.\n");
> >> +               return ret;
> >> +       }
> >
> > Use dev_err_probe() to simplify.
> >
> > ret = devm_add_action_or_reset(dev, spi_geni_release_dma_chan, mas);
> > if (ret)
> >   return dev_err_probe(dev, ret, "Unable to add action.\n");
>
> It seems that if it only return -ENOMEM or 0, using dev_err_probe() has
> not not much value for many community maintainers.

While I won't insist, it still has some value to use dev_err_probe()
as I talked about in commit 7065f92255bb ("driver core: Clarify that
dev_err_probe() is OK even w/out -EPROBE_DEFER").

-Doug

Re: [PATCH v4 3/3] spi: geni-qcom: Use devm functions to simplify code

[Jinjie Ruan]

On 2024/9/12 21:38, Doug Anderson wrote:
> Hi,
> 
> On Wed, Sep 11, 2024 at 8:53 PM Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>>
>>>> @@ -1132,6 +1134,12 @@ static int spi_geni_probe(struct platform_device *pdev)
>>>>         if (ret)
>>>>                 return ret;
>>>>
>>>> +       ret = devm_add_action_or_reset(dev, spi_geni_release_dma_chan, mas);
>>>> +       if (ret) {
>>>> +               dev_err(dev, "Unable to add action.\n");
>>>> +               return ret;
>>>> +       }
>>>
>>> Use dev_err_probe() to simplify.
>>>
>>> ret = devm_add_action_or_reset(dev, spi_geni_release_dma_chan, mas);
>>> if (ret)
>>>   return dev_err_probe(dev, ret, "Unable to add action.\n");
>>
>> It seems that if it only return -ENOMEM or 0, using dev_err_probe() has
>> not not much value for many community maintainers.
> 
> While I won't insist, it still has some value to use dev_err_probe()
> as I talked about in commit 7065f92255bb ("driver core: Clarify that
> dev_err_probe() is OK even w/out -EPROBE_DEFER")
The main difference is that when use dev_err_probe()，there will print
anything on -ENOMEM now.



> 
> -Doug

Re: [PATCH v4 3/3] spi: geni-qcom: Use devm functions to simplify code

[Doug Anderson]

Hi,

On Thu, Sep 12, 2024 at 11:44 PM Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>
> On 2024/9/12 21:38, Doug Anderson wrote:
> > Hi,
> >
> > On Wed, Sep 11, 2024 at 8:53 PM Jinjie Ruan <ruanjinjie@huawei.com> wrote:
> >>
> >>>> @@ -1132,6 +1134,12 @@ static int spi_geni_probe(struct platform_device *pdev)
> >>>>         if (ret)
> >>>>                 return ret;
> >>>>
> >>>> +       ret = devm_add_action_or_reset(dev, spi_geni_release_dma_chan, mas);
> >>>> +       if (ret) {
> >>>> +               dev_err(dev, "Unable to add action.\n");
> >>>> +               return ret;
> >>>> +       }
> >>>
> >>> Use dev_err_probe() to simplify.
> >>>
> >>> ret = devm_add_action_or_reset(dev, spi_geni_release_dma_chan, mas);
> >>> if (ret)
> >>>   return dev_err_probe(dev, ret, "Unable to add action.\n");
> >>
> >> It seems that if it only return -ENOMEM or 0, using dev_err_probe() has
> >> not not much value for many community maintainers.
> >
> > While I won't insist, it still has some value to use dev_err_probe()
> > as I talked about in commit 7065f92255bb ("driver core: Clarify that
> > dev_err_probe() is OK even w/out -EPROBE_DEFER")
> The main difference is that when use dev_err_probe()，there will print
> anything on -ENOMEM now.

Oh, I see. You're saying that we should just get rid of the print
altogether because the only error case is -ENOMEM and the kernel
already splats there? Yeah, that sounds right to me. That doesn't
match what you did in v5, though...

-Doug

Re: [PATCH v4 3/3] spi: geni-qcom: Use devm functions to simplify code

[Jinjie Ruan]

On 2024/9/14 0:27, Doug Anderson wrote:
> Hi,
> 
> On Thu, Sep 12, 2024 at 11:44 PM Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>>
>> On 2024/9/12 21:38, Doug Anderson wrote:
>>> Hi,
>>>
>>> On Wed, Sep 11, 2024 at 8:53 PM Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>>>>
>>>>>> @@ -1132,6 +1134,12 @@ static int spi_geni_probe(struct platform_device *pdev)
>>>>>>         if (ret)
>>>>>>                 return ret;
>>>>>>
>>>>>> +       ret = devm_add_action_or_reset(dev, spi_geni_release_dma_chan, mas);
>>>>>> +       if (ret) {
>>>>>> +               dev_err(dev, "Unable to add action.\n");
>>>>>> +               return ret;
>>>>>> +       }
>>>>>
>>>>> Use dev_err_probe() to simplify.
>>>>>
>>>>> ret = devm_add_action_or_reset(dev, spi_geni_release_dma_chan, mas);
>>>>> if (ret)
>>>>>   return dev_err_probe(dev, ret, "Unable to add action.\n");
>>>>
>>>> It seems that if it only return -ENOMEM or 0, using dev_err_probe() has
>>>> not not much value for many community maintainers.
>>>
>>> While I won't insist, it still has some value to use dev_err_probe()
>>> as I talked about in commit 7065f92255bb ("driver core: Clarify that
>>> dev_err_probe() is OK even w/out -EPROBE_DEFER")
>> The main difference is that when use dev_err_probe()，there will print
>> anything on -ENOMEM now.
> 
> Oh, I see. You're saying that we should just get rid of the print
> altogether because the only error case is -ENOMEM and the kernel
> already splats there? Yeah, that sounds right to me. That doesn't
> match what you did in v5, though...

I think the following 2 soultion is both fine:

1、return ret directly.

2、dev_err() and return.

> 
> -Doug