Re: [PATCH v5] hid-mcp2200: added driver for GPIOs of MCP2200

From: Johannes Roith
Date: Thu Aug 31 2023 - 14:54:00 EST


Hi Rahul,

thanks for the feedback, I will add it to the driver.

> My personal recommendation is to just have a single DMA buffer allocated
> for the mcp2200 instance rather than having to call the allocator and
> release the memory per command.

I added an 16-byte Array hid_report to the mcp2000 struct. When I need the
report, I do the following:

struct mcp_set_clear_outputs *cmd;

mutex_lock(&mcp->lock);
cmd = (struct mcp_set_clear_outputs *) mcp->hid_report

Do you think this is a valid implementation or do I really have to add a
pointer to the mcp2200 struct instead of the 16 byte array and allocate
another 16 byte for it in the probe function?

> The reason you run into this is likely because of the action added to
> devm conflicting with hid_device_remove....
>
> I recommend not depending on devm for teardown rather than making a stub
> remove function to work around the issue.

I am not sure, if I have understand this correctly, but basically I already
have a stub remove function (which is empty). First the remove function gets
called, then the unregister function and everything is cleaned up correctly.
Did I get this right or do you have any other recommendation for me?

So, do I need any adaptions, or can we go with the empty remove function?

Best regards,
Johannes