linux-kernel - Re: [patch v3 0/3] drivers/platform: replace module x86/mlxcpld-hotplug with mellanox/mlxreg-hotplug

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] [day] [month] [year] [list]

Message-ID: <20171016204211.GA14419@fury>
Date:   Mon, 16 Oct 2017 13:42:11 -0700
From:   Darren Hart <dvhart@...radead.org>
To:     Vadim Pasternak <vadimp@...lanox.com>
Cc:     gregkh@...uxfoundation.org, andy.shevchenko@...il.com,
        linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org,
        linux-leds@...r.kernel.org, jacek.anaszewski@...il.com,
        lee.jones@...aro.org, robh+dt@...nel.org, jiri@...nulli.us,
        pavel@....cz
Subject: Re: [patch v3 0/3] drivers/platform: replace module
 x86/mlxcpld-hotplug with mellanox/mlxreg-hotplug

On Mon, Sep 11, 2017 at 03:12:36PM +0000, Vadim Pasternak wrote:
> The patchset modifies x86/mlxcpld-hotplug.c for making it architecture
> independent, renames to mlxreg-hotplug and moves it from
> drivers/platform/x86 folder to new drivers/platform/mellanox folder.
> This modification also introduces regmap interface for mlxreg-hotplug
> driver to allow hotplug event functionality over programmable devices
> logic, when these devices can be attached to different interfaces types,
> like I2C, LPC, SPI.
> Module include/linux/platform_data/mlxcpld-hotplug.h is removed and new
> module include/linux/platform_data/mlxreg.h is to be used instead.
> Driver drivers/platform/x86/mlx-platform.c is updated according to new
> interface.
> Relevant Makefile and Kconfig are updated.
> 

Hi Vadim,

This didn't really land under anyone's maintainer domain. Apologies for
the long response time here.

I am happy to include the new platform/mellanox tree within the
platform-driver-x86 purvue, as we have done with platform/chrome. If
anyone objects, or would prefer to take this through their tree - please
let me know.

With respect to the series itself, the patches are not done in a way
that lend themselves to incremental review. The cover letter above says
the mlxcpld-hotplug driver is:

1) made architecture independent 2) renamed to mlxreg-hotplug 3) Moved
to platform/mellanox/

But the series does the following:

1) Create a new platform/mellanox/mlxreg-hotplug driver 2) Modify
mlx-platform to use the new driver 3) Delete mlxcpld-hotplug driver

This makes it very difficult to review the incremental changes to the
hotplug driver. Please refactor the patches to take the follow approach:

1) Use git-mv to move x86/mlxcpld-hotplug to mellanox/mlxreg-hotplug,
add the necessary Kconfig and Makefile entries, and update mlx-platform
to use the correct header. No functional changes. This gives us a
starting point for the next steps.

2) Make mlxreg-hotplug architecture independent 3) Add the new regmap
interface

Give some thought to what the incremental functional changes should be
(2,3 above are just an example). This will become more apparent to the
reviewer once the changes come in after the mv.

You'll want to add Andy and Myself and the platform-driver-x86 mailing
list to the MAINTAINERS file for platform/mellanox as we will still be
responsible for reviewing the patches and submitting the pull requests
to Linus.

Thanks,

-- 
Darren Hart
VMware Open Source Technology Center