On Fri, Jan 26, 2024 at 6:24 AM Nuno Sá <noname.nuno@xxxxxxxxx> wrote: > > On Fri, 2024-01-26 at 09:13 +0100, Nuno Sá wrote: > > On Thu, 2024-01-25 at 16:50 -0800, Saravana Kannan wrote: > > > 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. > > > > > > > Yeah that makes sense and is making me thinking that I should change my call > > (in > > patch 7 to use the MANAGED flag instead of AUTOREMOVE_CONSUMER). Sure, > > AUTOREMOVE_CONSUMER won't matter most cases but if someone disables > > fw_devlinks > > then it matters. I don't fully understand the patch series, but what exactly are you gaining by adding device links explicitly. I'd rather that every driver didn't explicitly create a device link. That's just a lot of useless code in most cases and we shouldn't have useless code lying around. If someone does fw_devlink=off and you don't create a device link explicitly, what's the worst that's going to happen? Useless deferred probes? fw_devlink is really only there as a debug tool at this point -- I don't think you need to worry about people setting it to off/permissive. > > > > Yeah, just realized MANAGED is not a valid flag one can pass to > device_link_add() :) If you don't pass the STATELESS flag, a link is assumed to be MANAGED. So, you can still create managed device links. -Saravana
