On 01/05/2024 17:12, Greg KH wrote:
On Wed, May 01, 2024 at 08:58:19AM +0300, Shahar Avidar wrote:
Make use of a higher level API.
What does this mean?
By "higher level" I meant a wrapper function that includes the
"class_register" call.
Reduce global memory allocation from struct class to pointer size.
No, you increased memory allocation here, why do you think you reduced
it?
Reducing *global* memory allocation.
I understand the tradeoff would be allocating in run time the class
struct anyway, but than, it could also be freed.
Since the Pi433 is a RasPi expansion board and can be attached\removed
in an asynchronous matter by the user, and only one can be attached at a
time, I thought it is best not to statically allocate memory which won't
be freed even if the hat is removed.
By using the class_create & class_destroy I thought of reducing memory
allocated by the RasPi if the pi433 is removed.
But following your response I now actually see that the class struct
will have the same lifespan anyway if allocated statically or
dynamically if its alive between the init\exit calls.
Also, this looks like a revert of commit f267da65bb6b ("staging: pi433:
make pi433_class constant"), accepted a few months ago, why not just
call it out as an explicit revert if that's what you want to do?
I actually saw this commit, but for some reason did not connect the dots
when I wrote this patch. My bad.
class_create is going away "soon", why add this back when people are
working so hard to remove its usage? What tutorial did you read that
made you want to make this change?
It's true, I got it the wrong way I guess. I thought class_create is the
preferred API (but now that you mentioned commit f267da65bb6b, I see
it's not). I did notice it in many other drivers though, and took them
as an example (e.g. gnss).
thanks,
greg k-h
I actually initially thought that the pi433 class should be removed
since it doesn't bring any new attributes with it, and that
spi_slave_class is more appropriate, but then I saw no other driver
using it. Any thoughts about that?
--
Regards,
Shahar
