Re: [PATCH v5 2/4] input: apple_z2: Add a driver for Apple Z2 touchscreens

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Wed, Jan 22, 2025 at 06:40:50PM +0100, Sasha Finkelstein wrote:
> On Wed, 22 Jan 2025 at 07:18, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > > +     z2->input_dev->phys = "apple_z2";
> >
> > Phys is supposed to be unique, however my understanding there could be 2
> > devices in the system?
> 
> All existing devices have at most one z2 device, and while i do not
> have visibility
> into future apple product decisions, judging by the current stack, it seems
> unlikely for them to make one that needs two of them.
> 
> > -static int apple_z2_build_cal_blob(struct apple_z2 *z2, u32 address, size_t cal_size, char *data)
> > +/* Build calibration blob, caller is responsible for freeing the blob data. */
> 
> A comment on a previous version of this patch requested to not have functions
> that require the caller to free the return value
> https://lore.kernel.org/all/ZAlM2DzMmwzWIZEF@nixie71/

You have to pick your poison. Either the caller has to inspect the
property to figure out the size of the allocation, handle errors, and
provide diagnostic, and then have apple_z2_build_cal_blob() re-parse the
property and fill the provided buffer, or you can hide it all in
apple_z2_build_cal_blob() and task the caller with freeing the blob when
they're done with it. Similar to request_firmware() and put_firmware().

I think the latter works better in this particular case.

Thanks.

-- 
Dmitry