[PATCH 1/2] dm: core: add support for fallback drivers

[Caleb Connolly]

Hi all,

Sorry for the slow follow-up.

On 11/04/2024 04:37, Sean Anderson wrote:
> On 4/10/24 15:44, Tom Rini wrote:
>> On Wed, Apr 10, 2024 at 07:27:17PM +0200, Heinrich Schuchardt wrote:
>>>
>>>
>>> Am 10. April 2024 19:06:57 MESZ schrieb Caleb Connolly 
>>> <caleb.connolly at linaro.org>:
>>>> Introduce support for a uclass to provide a fallback/stub driver which
>>>> can be used when no device is found for a given node. This might be
>>>> useful for handling non-essential clock controllers like the RPMh on
>>>> Qualcomm platforms, or during early bringup to get UART output before a
>>>> real clock driver has been created.
>>>>
>>>> Signed-off-by: Caleb Connolly <caleb.connolly at linaro.org>
>>>> ---
>>>> drivers/core/Kconfig  | 10 ++++++++++
>>>> drivers/core/uclass.c | 24 +++++++++++++++++++++++-
>>>> include/dm/uclass.h   |  3 +++
>>>> 3 files changed, 36 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
>>>> index 1081d61fcf01..09075b9b7a15 100644
>>>> --- a/drivers/core/Kconfig
>>>> +++ b/drivers/core/Kconfig
>>>> @@ -466,5 +466,15 @@ config BOUNCE_BUFFER
>>>>
>>>>       A second possible use of bounce buffers is their ability to
>>>>       provide aligned buffers for DMA operations.
>>>>
>>>> +menuconfig FALLBACK_DRIVERS
>>>
>>> Wouldn't it be preferable to mark individual drivers as fallback 
>>> drivers in their declaration?
>>>
>>> This would allow alternative fallback drivers and would not require 
>>> any definitions at uclass level.
>>>
>>> Just by building a fallback driver you would enable the fallback 
>>> behavior for its uclass.
>>
>> I think some of this is addressed in the cover letter. My concern /
>> questions come down to I think just a matter of naming. Both "fallback"
>> and "stub" are used at times. As a concept, we have some areas where we
>> need a no-op driver because whereas the DT describes a relationship in
>> the hardware, here in U-Boot we can just accept things as configured. To
>> me "fallback" implies more functionality of the driver when it's really
>> just a generic no-op driver to fill in the dependencies within the tree.
>> Can we rename things a bit along those lines?

Yes, will do. I originally used "stub" but then decided "fallback" was 
better, but I think you're right that stub is the right way to go here. 
As I agree that "fallback" implies some kind of real implementation.
>>
> 
> I would rather just have a stub driver with compatibles for all clocks 
> we want
> it to match. I think having a generic fallback driver could cause issues 
> in the
> future if we need to switch over to using a real driver.

I don't think either approach is significantly better from a developer 
perspective. With my patches if there a driver with a matching 
compatible available then you will NEVER bind the stub, even if probe 
fails on the real driver. With your proposal we'd have to remember to 
remove the compatible from the stub driver or risk the race condition 
when binding drivers.

There is another subtle but important difference if we were to use a 
compatible list in the stub. It turns out that CONFIG_OF_LIVE affects 
the bind behaviour - U-Boot won't try to bind the children of a node 
with no driver when using the livetree. This is arguably more "correct", 
as usually child devices depend on their parent, so we save some cycles 
by not binding unsupported devices.

But, the RPM clock controller which I originally implemented this 
feature for is such a case, this is the DT (simplified for brevity):

rpm: remoteproc {
     compatible = "qcom,sm6115-rpm-proc", "qcom,rpm-proc";

     glink-edge {
         compatible = "qcom,glink-rpm";

         rpm_requests: rpm-requests {
             compatible = "qcom,rpm-sm6115";

             rpmcc: clock-controller {
                 compatible = "qcom,rpmcc-sm6115", "qcom,rpmcc";
             };
     };
};

With livetree (which we use on qcom) we would need to stub not just the 
rpmcc but also the rpm-requests, the glink bus, and the remoteproc (yeah 
these platforms are a bit silly, I know XD).

However with the fallback solution as implemented in these patches, we 
totally sidestep this issue by directly binding the node on-demand, 
something we can only safely do for a stub driver as we don't need the 
parent devices.

So I can either:

1) Make OF_LIVE behave like flattree and descend into all nodes (I don't 
know if this would cause any issues, but I think the livetree behaviour 
is more "correct" tbh).
2) Add stubs for all 3 of the parent nodes (and manually maintain a 
table of compatibles for clock drivers we want to stub).
3) Stick with this approach of binding the stub as a last resort.

Obviously I've already put the time into 3, and that would still be my 
preferred approach, but I'm open to other ideas.

> 
> --Sean

-- 
// Caleb (they/them)