linux-kernel - Re: [PATCH 09/12] PCI/switchtec: Add gen4 support in struct flash_info

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 for Android: free password hash cracker in your pocket

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

Date:   Wed, 8 Jan 2020 15:23:47 -0600
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Logan Gunthorpe <logang@...tatee.com>
Cc:     linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
        Kelvin.Cao@...rochip.com, Eric Pilmore <epilmore@...aio.com>,
        Doug Meyer <dmeyer@...aio.com>
Subject: Re: [PATCH 09/12] PCI/switchtec: Add gen4 support in struct
 flash_info_regs

On Mon, Jan 06, 2020 at 12:03:34PM -0700, Logan Gunthorpe wrote:
> From: Kelvin Cao <kelvin.cao@...rochip.com>
> 
> Add a union with gen3 and gen4 flash_info structs.

This does a lot more than add a union :)

I think this looks reasonable, but I would like it even better if this
and related patches could be split up a little bit differently:

  - Rename SWITCHTEC_CFG0_RUNNING to SWITCHTEC_GEN3_CFG0_RUNNING, etc
    (purely mechanical change, so trivial and obvious).

  - Add switchtec_gen and the tests where it's needed, but with only
    SWITCHTEC_GEN3 cases for now.

  - Refactor ioctl_flash_part_info() (still only supports GEN3).
    Maybe adds struct flash_info_regs and union, but only with gen3.

  - Add GEN4 support (patch basically contains only GEN4-related
    things and doesn't touch GEN3 things at all).  Maybe it would
    still make sense to split the GEN4 support into multiple patches
    (as in this series), or maybe they could be squashed into a single
    GEN4 patch?

  - It seems like at least the aliasing quirk and the driver device ID
    update could/should be squashed since they contain the same
    constants.

Bjorn