linux-kernel - Re: [PATCH v3 08/15] phy: qcom-qmp-pcie: add register init helper

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]

Message-ID: <Y1vT94blD9PJHKDp@hovoldconsulting.com>
Date:   Fri, 28 Oct 2022 15:07:03 +0200
From:   Johan Hovold <johan@...nel.org>
To:     Vinod Koul <vkoul@...nel.org>
Cc:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Johan Hovold <johan+linaro@...nel.org>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        linux-arm-msm@...r.kernel.org, linux-phy@...ts.infradead.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 08/15] phy: qcom-qmp-pcie: add register init helper

On Fri, Oct 28, 2022 at 06:24:38PM +0530, Vinod Koul wrote:
> On 21-10-22, 13:32, Johan Hovold wrote:

> > [...] This is not about keeping the
> > diff small, this is about readability of the new helper function as I
> > already told you.
> > 
> > And this is a *local* identifier, not some state member that needs a
> > super descriptive name. And the rest of the driver used "tbl"
> > consistently until your EP/RC mode patches for that matter.
> 
> I would disagree here... You can change tbls/tables but then it does not
> help _this_ patch
> 
> Right thing would be to change tbls to tables first and then add init
> helper... For a reviewer seeing an undocumented change and unnecessary
> diff is not right..

I still think that it belongs in the patch adding the new helper
because it is essentially only in that new function that the improved
readability due to the shorter identifier matters (the earlier helpers
where per table type).
 
> Pls split if you would still like the rename

But if you prefer I'll split it out in a preparatory patch.

Johan