On Mon, 2023-11-20 at 12:40 +0000, Jonathan Cameron wrote: > On Mon, 06 Nov 2023 10:26:38 +1030 > Andrew Jeffery <and...@codeconstruct.com.au> wrote: > > > On Fri, 2023-11-03 at 15:05 +0000, Jonathan Cameron wrote: > > > On Fri, 3 Nov 2023 16:45:20 +1030 > > > Andrew Jeffery <and...@codeconstruct.com.au> wrote: > > > > > > > I ran out of spoons before I could come up with a better client tracking > > > > scheme back in the original refactoring series: > > > > > > > > https://lore.kernel.org/all/20210608104757.582199-1-and...@aj.id.au/ > > > > > > > > Jonathan prodded Konstantin about the issue in a review of Konstantin's > > > > MCTP patches[1], prompting an attempt to clean it up. > > > > > > > > [1]: https://lore.kernel.org/all/20230929120835.00001...@huawei.com/ > > > > > > > > Prevent client modules from having to track their own instances by > > > > requiring they return a pointer to a client object from their > > > > add_device() implementation. We can then track this in the core, and > > > > provide it as the argument to the remove_device() implementation to save > > > > the client module from further work. The usual container_of() pattern > > > > gets the client module access to its private data. > > > > > > > > Signed-off-by: Andrew Jeffery <and...@codeconstruct.com.au> > > > > > > Hi Andrew, > > > > > > A few comments inline. > > > More generally, whilst this is definitely an improvement I'd have been > > > tempted > > > to make more use of the linux device model for this with the clients added > > > as devices with a parent of the kcs_bmc_device. That would then allow for > > > simple dependency tracking, binding of individual drivers and all that. > > > > > > What you have here feels fine though and is a much less invasive change. > > > Sorry for slow reply, been traveling.
No worries, I've just got back from travel myself :) > > > Yeah, I had this debate with myself before posting the patches. My > > reasoning for the current approach is that the clients don't typically > > represent a device, rather a protocol implementation that is > > communicated over a KCS device (maybe more like pairing a line > > discipline with a UART). It was unclear to me whether associating a > > `struct device` with a protocol implementation was stretching the > > abstraction a bit, or whether I haven't considered some other > > perspective hard enough - maybe we treat the client as the remote > > device, similar to e.g. a `struct i2c_client`? > > That was my thinking. The protocol is used to talk to someone - the endpoint > (similar to i2c_client) so represent that. If that device is handling multiple > protocols (no idea if that is possible) - that is fine as well, it just > becomes > like having multiple i2c_clients in a single package (fairly common for > sensors), > or the many other cases where we use a struct device to represent just part > of a larger device that operates largely independently of other parts (or with > well defined boundaries). Alright, let me think about that a bit more. Thanks for the feedback, Andrew _______________________________________________ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer
