Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 22 Jun 2023 15:36:30 +0300
From:   Andy Shevchenko <andriy.shevchenko@intel.com>
To:     Martin =?utf-8?B?WmHFpW92acSN?= <m.zatovic1@gmail.com>
Cc:     linux-kernel@vger.kernel.org, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
        gregkh@linuxfoundation.org, beanhuo@micron.com,
        nipun.gupta@amd.com, linus.walleij@linaro.org, mwen@igalia.com,
        bvanassche@acm.org, arnd@arndb.de, ogabbay@kernel.org,
        linux@zary.sk, jacek.lawrynowicz@linux.intel.com,
        geert+renesas@glider.be, benjamin.tissoires@redhat.com,
        masahiroy@kernel.org, yangyicong@hisilicon.com,
        devicetree@vger.kernel.org
Subject: Re: [PATCHv4 0/4] Wiegand bus driver and GPIO bitbanged Wiegand
Message-ID: <ZJRATnQ6R+EYnUok@smile.fi.intel.com>
References: <20230510162243.95820-1-m.zatovic1@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20230510162243.95820-1-m.zatovic1@gmail.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
Precedence: bulk

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