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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 22 Jun 2023 15:36:30 +0300
From:   Andy Shevchenko <andriy.shevchenko@...el.com>
To:     Martin Zaťovič <m.zatovic1@...il.com>
Cc:     linux-kernel@...r.kernel.org, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
        gregkh@...uxfoundation.org, beanhuo@...ron.com,
        nipun.gupta@....com, linus.walleij@...aro.org, mwen@...lia.com,
        bvanassche@....org, arnd@...db.de, ogabbay@...nel.org,
        linux@...y.sk, jacek.lawrynowicz@...ux.intel.com,
        geert+renesas@...der.be, benjamin.tissoires@...hat.com,
        masahiroy@...nel.org, yangyicong@...ilicon.com,
        devicetree@...r.kernel.org
Subject: Re: [PATCHv4 0/4] Wiegand bus driver and GPIO bitbanged Wiegand

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