Re: [bug report] ASoC: spacemit: add i2s support for K1 SoC

[Alex Elder <elder@ieee.org>]

On 10/24/25 6:43 AM, Dan Carpenter wrote:
> On Fri, Oct 24, 2025 at 07:37:51PM +0800, Troy Mitchell wrote:
>> On Fri, Oct 24, 2025 at 02:25:40PM +0300, Dan Carpenter wrote:
>>>      424         i2s->reset = devm_reset_control_get_exclusive(&pdev->dev, NULL);
>>>      425         if (IS_ERR(i2s->reset))
>>>      426                 return dev_err_probe(i2s->dev, PTR_ERR(i2s->reset),
>>>      427                                      "failed to get reset control");
>>>      428
>>>      429         dev_set_drvdata(i2s->dev, i2s);
>>>      430
>>>      431         spacemit_i2s_init_dai(i2s, &dai, res->start + SSDATR);
>>>                                             ^^^^
>>> dai is not initialized on failure.  Generally in the kernel we always
>>> check for allocation failures.  Even when they can't happen in real life.
>>> I was hoping they were going to make a rule that allocations under 4k
>>> wouldn't have to be checked but it hasn't happened yet.
>> Should I send a fix patch now, or should I wait for the maintainer to
>> revert it first, and then fix this error and send?
>>
>> Thanks for your report!
>>
> 
> You should send a follow on patch which just fixes the one issue.  If the
> maintainer wants to squash them together that's very easy to do in git.

Yes I agree, send a new patch, no point in reverting.  But I think
a fix commit is better than squashing (for what that's worth).


A very simple fix is assigning *dp to NULL in spacemit_i2s_init_dai().

         if (dp)
                 *dp = dai ? : NULL;

Also:  Why aren't you checking for spacemit_i2s_init_dai() returning
an error?  (Another--separate--suggested fix.)

					-Alex

> 
> regards,
> dan carpenter
> 
>