On Tue, 27 Aug 2019 11:57:07 +0000 Parav Pandit <[email protected]> wrote:
> > -----Original Message----- > > From: Cornelia Huck <[email protected]> > > Sent: Tuesday, August 27, 2019 5:11 PM > > To: Parav Pandit <[email protected]> > > Cc: [email protected]; Jiri Pirko <[email protected]>; > > [email protected]; [email protected]; [email protected]; linux- > > [email protected]; [email protected] > > Subject: Re: [PATCH 1/4] mdev: Introduce sha1 based mdev alias > > > > On Tue, 27 Aug 2019 11:33:54 +0000 > > Parav Pandit <[email protected]> wrote: > > > > > > -----Original Message----- > > > > From: Cornelia Huck <[email protected]> > > > > Sent: Tuesday, August 27, 2019 4:54 PM > > > > To: Parav Pandit <[email protected]> > > > > Cc: [email protected]; Jiri Pirko <[email protected]>; > > > > [email protected]; [email protected]; [email protected]; > > > > linux- [email protected]; [email protected] > > > > Subject: Re: [PATCH 1/4] mdev: Introduce sha1 based mdev alias > > > > > > > > On Tue, 27 Aug 2019 11:12:23 +0000 > > > > Parav Pandit <[email protected]> wrote: > > > > > > > > > > -----Original Message----- > > > > > > From: Cornelia Huck <[email protected]> > > > > > > Sent: Tuesday, August 27, 2019 3:54 PM > > > > > > To: Parav Pandit <[email protected]> > > > > > > Cc: [email protected]; Jiri Pirko <[email protected]>; > > > > > > [email protected]; [email protected]; [email protected]; > > > > > > linux- [email protected]; [email protected] > > > > > > Subject: Re: [PATCH 1/4] mdev: Introduce sha1 based mdev alias > > > > > > > > > > > > > > What about: > > > > > > > > > > > > * @get_alias_length: optional callback to specify length of the > > > > > > alias to > > > > create > > > > > > * Returns unsigned integer: length of the alias > > > > > > to be created, > > > > > > * 0 to not create an > > > > > > alias > > > > > > > > > > > Ack. > > > > > > > > > > > I also think it might be beneficial to add a device parameter > > > > > > here now (rather than later); that seems to be something that makes > > > > > > > > sense. > > > > > > > > > > > Without showing the use, it shouldn't be added. > > > > > > > > It just feels like an omission: Why should the vendor driver only be > > > > able to return one value here, without knowing which device it is for? > > > > If a driver supports different devices, it may have different > > > > requirements for them. > > > > > > > Sure. Lets first have this requirement to add it. > > > I am against adding this length field itself without an actual vendor use > > > case, > > which is adding some complexity in code today. > > > But it was ok to have length field instead of bool. > > > > > > Lets not further add "no-requirement futuristic knobs" which hasn't shown > > > its > > need yet. > > > When a vendor driver needs it, there is nothing prevents such addition. > > > > Frankly, I do not see how it adds complexity; the other callbacks have > > device > > arguments already, > Other ioctls such as create, remove, mmap, likely need to access the parent. > Hence it make sense to have parent pointer in there. > > I am not against complexity, I am just saying, at present there is no > use-case. Let have use case and we add it. > > > and the vendor driver is free to ignore it if it does not have > > a use for it. I'd rather add the argument before a possible future user > > tries > > weird hacks to allow multiple values, but I'll leave the decision to the > > maintainers. > Why would a possible future user tries a weird hack? > If user needs to access parent device, that driver maintainer should ask for > it. I've seen the situation often enough that folks tried to do hacks instead of enhancing the interface. Again, let's get a maintainer opinion.
