Re: [PATCHv4 0/4] Wiegand bus driver and GPIO bitbanged Wiegand

From: Andy Shevchenko
Date: Thu Jun 22 2023 - 08:36:45 EST


On Wed, May 10, 2023 at 06:22:39PM +0200, Martin Zaťovič wrote:
> Hello,
>
> thank you for the feedback regarding the previous version of this patch series.
> I have tried to follow all of the advice I got and fix all the pointed issues.
> One of the main issues was the usage of of API for device registration. This
> has now been fixed to use fwnode API, however I was not yet able to get rid of
> the of_device include, since it is required for of_driver_match_device. Please
> let me know if this is correct.

Since it is a bus, I think we need that.

> CHANGELOG:
>
> wiegand.c:
> - changed ID allocation API from IDR to IDA, since the references associated to
> the IDs are not needed
> - removed the board_lock mutex, because it was only guarding the allocacion
> and freeing of IDs, which is already supported by IDA API
> - restructured the file, so that most functions are close to their caller, or
> defined them at the top for better code readability

> - in the function devm_wiegand_register_controller, the devres management of
> the pointer to wiegand_controller structure has been replaced with
> devm_add_action_or_reset function. It was intended to do the same with
> devm_wiegand_alloc_controller, however, the kernel kept panicing, despite the
> call order of the unregister and release functions being proper(same as with
> devres managed pointer). Please let me know if this is an absolute must, if so
> I will look into it further.

What panic? Can you elaborate?

> - moved the miscdevice from wiegand-gpio driver to be a part of the bus
> driver. Now every controller is associated a /dev file. The file operation
> functions were simply moved and renamed and the miscdevice structure was moved
> to be a part of wiegand_controller structure
> - since now every controller has a miscdevice assosciated, the data_buffer was
> also moved to be a part of the controller structure, and it was made a bitmap
> - used fwnode API for device registration instead of of API
> - removed warnings when driver fails to get wiegand properties, instead
> implemented mechanism for setting a default value similar I2C
> - removed the driver matching code in register driver, as
> of_driver_match_device does that already
> - made wiegand_device and opaque pointer
> - changed the terminology to primary and secondary

--
With Best Regards,
Andy Shevchenko