Re: [PATCH v4 00/17] ASoC: Intel: haswell and broadwell boards update

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

On 6/21/22 12:47, Cezary Rojewski wrote:
> On 2022-06-21 6:36 PM, Pierre-Louis Bossart wrote:
>> On 6/20/22 05:13, Cezary Rojewski wrote:
>>> A number of patches improving overall quality and readability of
>>> haswell.c and broadwell.c source files found in sound/soc/intel/boards.
>>> Both files are first renamed and only then actual changes are being
>>> incrementally added. The respective names are: hsw_rt5640 and bdw_rt286
>>> to match the pattern found in more recent boards.
>>>
>>> Most patches bring no functional change - the more impactful patches at
>>> are placed the end:
>>>
>>> Refactor of suspend/resume flow for the bdw_rt286 board by dropping
>>> dev->remove() in favour of card->remove() and adjust jack handling to
>>> reduce code size slightly by implementing card_set_jack().
>>>
>>> The last patch is removing of FE DAI ops. Given the existence of
>>> platform FE DAI capabilities (either static declaration or through
>>> topology file), this code is redundant.
>>
>> Possibly a mistake in our tests, but this error seems to be introduced:
>>
>> [  107.397637] kernel: rt286 i2c-INT343A:00: ASoC: DAPM unknown pin LDO1
>>
>> I'll have to re-run the tests, sharing this information as is.
> 
> 
> Hello,
> 
> Thanks for the report! However, this has been reported earlier during
> the v2 review [1]. This is also why a fix have been provided [2] earlier
> today. Notice that shape of link->exit() found here is shared by other
> Intel boards e.g.: SOF ones. In general, the initial discussion
> regarding card->remove() revealed some 'probe vs remove' problems within
> the framework.
> 
> 
> [1]:
> https://lore.kernel.org/alsa-devel/69e4263a-e036-cb21-2360-55b06600911e@intel.com/
> 
> [2]:
> https://lore.kernel.org/alsa-devel/1cff4ac0-6d45-95e1-ed9f-6abaded3f8b7@intel.com/T/#t

It's rather difficult to follow these changes and error reports buried
in email report sent on a Sunday of a three-day week-end for me.
I also had additional errors not reported,

[   36.125113] kernel: rt286 i2c-INT343A:00: ASoC: unknown pin HV
[   36.125128] kernel: rt286 i2c-INT343A:00: ASoC: unknown pin VREF
[   36.125130] kernel: rt286 i2c-INT343A:00: ASoC: unknown pin LDO1
[   36.125921] kernel: rt286 i2c-INT343A:00: ASoC: DAPM unknown pin LDO1

it's unclear to me why a dailink change in a machine driver would cause
such codec-side issues.

If the changes in this 17-patch series need to be tied to a framework
fix, you have to make the dependencies explicit and better yet provide a
self-contained patch series that does not introduce a temporary
regression, or introduce the framework change first and clearly describe
the dependency in a longer Broadwell-specific patchset. This is an 8-yr
old device, it shouldn't be that hard.