On Thu, Jan 25, 2024 at 12:11 AM Nuno Sá <noname.nuno@xxxxxxxxx> wrote: > > > Hi Saravana, > > Thanks for your feedback, > > On Wed, 2024-01-24 at 19:21 -0800, Saravana Kannan wrote: > > On Tue, Jan 23, 2024 at 7:14 AM Nuno Sa via B4 Relay > > <devnull+nuno.sa.analog.com@xxxxxxxxxx> wrote: > > > > > > From: Nuno Sa <nuno.sa@xxxxxxxxxx> > > > > > > If a device_link is previously created (eg: via > > > fw_devlink_create_devlink()) before the supplier + consumer are both > > > present and bound to their respective drivers, there's no way to set > > > DL_FLAG_AUTOREMOVE_CONSUMER anymore while one can still set > > > DL_FLAG_AUTOREMOVE_SUPPLIER. Hence, rework the flags checks to allow > > > for DL_FLAG_AUTOREMOVE_CONSUMER in the same way > > > DL_FLAG_AUTOREMOVE_SUPPLIER is done. > > > > Curious, why do you want to set DL_FLAG_AUTOREMOVE_CONSUMER? > > Especially if fw_devlink already created the link? You are effectively > > trying to delete the link fw_devlink created if any of your devices > > unbind. > > > > Well, this is still useful in the modules case as the link will be relaxed after > all devices are initialized and that will already clear AUTOPROBE_CONSUMER > AFAIU. But, more importantly, if I'm not missing anything, in [1], fw_devlinks > will be dropped after the consumer + supplier are bound which means I definitely > want to create a link between my consumer and supplier. > > FWIW, I was misunderstanding things since I thought DL_FLAG_AUTOREMOVE_CONSUMER > was needed to make sure the consumer is unbound before the supplier. But for > that I think I can even pass 0 in the flags as I only need the link to be > MANAGED. Still, I think having DL_FLAG_AUTOREMOVE_CONSUMER makes sense. As you noticed, your understanding of DL_FLAG_AUTOREMOVE_CONSUMER is not correct. There's almost never a good reason to drop a device link. Even when a device/driver are unbound, we still want future probe attempts to make use of the dependency info and block a device from probing if the supplier hasn't probed. If you don't want the links created by fw_devlink to be relaxed, I think you should instead set the kernel command line param so that the kernel doesn't time out and give up on enforcing dependencies. deferred_probe_timeout=-1 Then you don't have to worry about creating device links. > Also note that there are more places in the kernel with > DL_FLAG_AUTOREMOVE_CONSUMER and that flag is likely being ignored in case the > link already exists. > > I'm also clearing DL_FLAG_AUTOPROBE_CONSUMER as from the first check in > device_link_add(() check I realize that we can't/shouldn't have it together with > one of AUTOREMOVE_CONSUMER | AUTOREMOVE_SUPPLIER, right? At this point, > AUTOPROBE_CONSUMER is also likely not that useful anymore as both supplier and > consumer are up and I guess that's the typical case for subsystems/drivers to > call device_link_add(). > > And since I have your attention, it would be nice if you could look in another > sensible patch [2] that I've resended 3 times already. You're not in CC but I > see you've done quite some work in dev_links so... Completely unrelated I know > :) Regarding [2], I'll try. -Saravana > > [1]: https://elixir.bootlin.com/linux/latest/source/drivers/base/core.c#L1292 > [2]: https://lore.kernel.org/all/20231213-fix-device-links-overlays-v1-1-f091b213777c@xxxxxxxxxx/#t > > - Nuno Sá > > >
