Re: [PATCH v4 6/8] iio: add the IIO backend framework

[Olivier MOYSAN <olivier.moysan@foss.st.com>]

Hi Nuno,

On 1/9/24 13:15, Nuno Sá wrote:
> On Tue, 2023-12-26 at 15:59 +0000, Jonathan Cameron wrote:
>>
>>>>> +
>>>>> +       ret = devm_add_action_or_reset(dev, iio_backend_release, back);
>>>>> +       if (ret)
>>>>> +               return ret;
>>>>> +
>>>>> +       link = device_link_add(dev, back->dev,
>>>>> DL_FLAG_AUTOREMOVE_CONSUMER);
>>>>> +       if (!link)
>>>>> +               pr_warn("%s: Could not link to supplier(%s)\n",
>>>>> dev_name(dev),
>>>>> +                       dev_name(back->dev));
>>>>
>>>> Why is that not an error and we try to carry on?
>>>
>>> I guess having the links are not really mandatory for the whole thing to
>>> work (more
>>> like a nice to have). That's also how this is handled in another subsystems
>>> so I
>>> figured it would be fine.
>>>
>>> But since you are speaking about this... After you pointing me to Bartosz's
>>> talk and
>>> sawing it (as stuff like this is mentioned), I started to question this. The
>>> goal
>>> with the above comment is that if you remove the backend, all the consumers
>>> are
>>> automatically removed (unbound). Just not sure if that's what we always want
>>> (and we
>>> are already handling the situation where a backend goes away with -ENODEV)
>>> as some
>>> frontends could still be useful without the backend (I guess that could be
>>> plausible). I think for now we don't really have such usecases so the links
>>> can make
>>> sense (and we can figure something like optionally creating these links if
>>> we ever
>>> need too) but having your inputs on this will definitely be valuable.
>>
>> I'm not keen on both trying to make everything tear down cleanly AND making
>> sure
>> it all works even if we don't. That just adds two code paths to test when
>> either
>> should be sufficient on its own.  I don't really mind which.  Bartosz's stuff
> 
> Agreed...
> 
>> is nice, but it may not be the right solution here.
> 
> There's pros and cons on both options...
> 
> For the device links the cons I see is that it depends on patch 3 for it to work
> (or some other approach if the one in that patch is not good) - not really a
> real con though :). The biggest concern is (possible) future uses where we end
> up with cases where removing a backend is not really a "deal breaker". I could
> think of frontends that have multiple backends (one per data path) and removing
> one backend would not tear the whole thing down (we would just have one non
> functional data paths/port where the others are still ok).
> 
> Olivier, for STM usecases, do we always need the backend? I mean, does it make
> sense to always remove/unbind the frontend in case the backend is unbound?
> 

In STM usecases we may have severals backends linked to a frontend. But 
I cannot find a usecase where it may be necessary to remove a backend 
while keeping the others alive. So from my side it is acceptable to tear 
everythings down if a backend if removed.

Olivier

> Maybe some of your usecases already "forces" us with a decision.
> 
> The biggest pro I see is code simplicity. If we can assume the frontend can
> never exist in case one of the backends is gone, we can:
> 
>   * get rid of the sync mutex;
>   * get rid of the kref and bind the backend object lifetime to the backend
> device (using devm_kzalloc() instead of kzalloc + refcount.
> 
> Basically, we would not need to care about syncing the backend existence with
> accessing it...
> To sum up, the device_links approach tends to be similar (not identical) to the
> previous approach using the component API.
> 
> The biggest pro I see in Bartosz's stuff is flexibility. So it should just work
> in whatever future usecases we might have. I fear that going the device_links
> road we might end up needing this stuff anyways.
> 
> Obviously, the biggest con is code complexity (not that bad though) as we always
> need to properly sync any backend callback.
> 
> - Nuno Sá
>>