Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <e098e8a9-533b-319e-ea0c-24af28714471@linux.intel.com>
Date:   Wed, 21 Jun 2023 13:28:42 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.11.0
Subject: Re: [PATCH 2/4] soundwire: introduce SDW_DEV_NUM_ALLOC_IDA_WAKE_ONLY
Content-Language: en-US
To:     Vinod Koul <vkoul@kernel.org>
Cc:     Bard Liao <yung-chuan.liao@linux.intel.com>,
        alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
        bard.liao@intel.com
References: <20230531033736.792464-1-yung-chuan.liao@linux.intel.com>
 <20230531033736.792464-3-yung-chuan.liao@linux.intel.com>
 <ZIF94vZHzeGXfyin@matsya>
 <6c75e986-29a4-d97c-3862-d20397f8b8b4@linux.intel.com>
 <ZJLYQCwvvIwEj47H@matsya>
From:   Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
In-Reply-To: <ZJLYQCwvvIwEj47H@matsya>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk


>>> This seems to be a consequence of Intel hardware decisions, so I guess
>>> best suited place for this is Intel controller, do we really want to
>>> have this in core logic?
>>
>> It's a valid objection.
>>
>> The reason why I added the alternate strategies in the core logic is
>> that the IDA and hybrid approach are just software-based with no
>> specific hardware dependencies. If QCOM or AMD wanted to use the
>> strategies contributed and tested by Intel, it'd be a two-line change on
>> their side.
>>
>> That said, it's likely that at some point *someone* will want to
>> constrain the device number allocation further, be it with ACPI/DT
>> properties or reading hardware registers. The device number is a
>> de-facto priority given the way we scan the PING frames, so some systems
>> may want to give a higher priority to a specific peripherals.
>>
>> This would push us to add a master ops callback to control the device
>> number allocation. It's a bit invasive but that would give the ultimate
>> flexibility. Reuse between vendors could be possible if 'generic'
>> callbacks were part of a library to pick from.
>>
>> I don't really have any objections if this vendor-specific callback was
>> preferred, it may be a bit early to add this but long-term it's probably
>> what makes more sense.
>>
>> I'll go with the flow on suggested recommendations.
> 
> Thanks, if it all one of the other two controller start using this, it
> would make sense to move it to core then, for now would be better to
> have this in specific driver

The code is much cleaner indeed that way.

I still have to work on a race condition if the codec driver probe
happens *after* the enumeration. In that case, the properties needed to
decide which allocation to use are not initialized yet.

We may need to either force the codec to re-enumerate with a ForceReset,
or to switch the device number. In theory the latter is straightforward
but there can be additional races if there are interrupts thrown just
before the device number change happens.