Re: [PATCH 0/3] ASoC/soundwire: remove ghost peripherals from the mach table

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

>> Well, you've got two problems here.
>>
>> a) there's a difference between generations. Starting with ACE2.0, the
>> SoundWire bus is started much earlier than in previous generations,
>> where the SoundWire bus was started after firmware download.
>>
>> In other words, the timing to sync and report as attached differs by
>> orders of magnitude. I am worried that this code might break stuff on
>> older platforms.
> 
> The commits will not check the existence of the peripherals for the
> ACE2.0- platforms. So it should be fine.

ok

>> b) there's nothing in the MIPI spec that says how much time a device
>> should take to report as ATTACHED. A well-intended implementation
>> following Recommendation {2121} would e.g. try to find the sync word
>> with a default frame shape of 48x2, which isn't used at all by Intel.
> 
> Can't we rely on reading device 0 to know whether all peripherals are
> enumerated? Is it possible that a peripheral can be attached after some
> time when all other peripherals on the same bus are attached?

yes that's entirely possible. A peripheral may be extra cautious to
avoid synchronizing on a wrong pattern, and take extra time to report as
Device 0.

> Will it trigger another interrupt in that case? My assumption is that
> a bus will only get one attached interrupt when it starts. And we will
> keep reading the device 0 and programing the device number nutil there
> is no device 0.

yes that's the problem. Worst case you would get N interrupts for each
of the N peripherals.

>> If you want to keep the code above, you absolutely need to record the
>> time at which the bus started and wait TBD ms before signaling
>> enumeration_complete(). You would also need to deal with a case where
>> another device shows up with a delay, would you throw another
>> complete_all() then?
>>
> 
> Can we add a peripheral property to indicate a waiting time and wait the
> largest waiting time of the peripherals?

that would be ideal, but it's not easy to get this sort of information.
This could depend on the clock base and frame shape as well which is
system dependent.

It's probably easier to introduce a default at the bus level and a
kernel parameter to bump that value up in case of problems.

You should also flag the case with a dev_err in case of a second
interrupt, that'd be a clear evidence of the delay not being large enough.

Keeping track of the time since the bus reset completed would help as well.

It's all small potatoes but it'd help harden your solution.