On Sun, 2025-01-12 at 16:11 +0000, Jonathan Cameron wrote: > On Fri, 10 Jan 2025 09:42:53 +0200 > Robert Budai <robert.budai@xxxxxxxxxx> wrote: > > > The ADIS16550 is a complete inertial system that includes a triaxis > > gyroscope and a triaxis accelerometer. Each inertial sensor in > > the ADIS16550 combines industry leading MEMS only technology > > with signal conditioning that optimizes dynamic performance. The > > factory calibration characterizes each sensor for sensitivity, bias, > > and alignment. As a result, each sensor has its own dynamic com- > > pensation formulas that provide accurate sensor measurements > > > > Co-developed-by: Ramona Gradinariu <ramona.gradinariu@xxxxxxxxxx> > > Signed-off-by: Ramona Gradinariu <ramona.gradinariu@xxxxxxxxxx> > > Co-developed-by: Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx> > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx> > > Signed-off-by: Nuno Sá <nuno.sa@xxxxxxxxxx> > > Signed-off-by: Robert Budai <robert.budai@xxxxxxxxxx> > > --- > > > > 4: > > - reorganized channels to match the order in the datasheet > > - removed extra checks and goto statements > > - for buffer memory allocation used only kfree, since adis library already > > takes care of freeing the buffer > > That last bit makes for a mess wrt to who owns the buffer and lifetime > management. Suggestions inline. > > ... > > + > > + /* > > + * Allocate the xfer and buffer for the burst read operation. The > > buffer > > + * is used to store the burst data and the xfer is used to send the > > burst > > + * command and receive the data. On device remove the adis libraary > > is going > > library. > > > + * to free the xfer and buffer. > > That is a horrible lifetime control. They should either be allocated in the > library > and freed in the library or allocated and freed in the driver. Mixing that > doesn't > make sense. Agreed. TBH, I did not remembered at all to suggest ignoring the library and let it call kfree() on NULL. I also don't like that much of how things are done in the lib but I don't think it would be that straight to change things for the non 'burst mode' cases. For the devices implementing 'burst mode' doing the free + allocation on every update_scan_modes seems unnecessary. Anyways, maybe something worth looking at in the future. - Nuno Sá
