linux-kernel - Re: [PATCH 9/9] soc/qcom: Add REVID driver

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: <CAMS8qEV7jhbHqpXE2UOaXBVM5WbCThaGrcD3wiH9kf6h_K-qeA@mail.gmail.com>
Date:   Sun, 26 Jul 2020 13:40:46 +0200
From:   Konrad Dybcio <konradybcio@...il.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Martin Botka <martin.botka1@...il.com>,
        Rob Clark <robdclark@...il.com>, Sean Paul <sean@...rly.run>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Rob Herring <robh+dt@...nel.org>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Kishon Vijay Abraham I <kishon@...com>,
        Vinod Koul <vkoul@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Felipe Balbi <balbi@...nel.org>,
        Jordan Crouse <jcrouse@...eaurora.org>,
        zhengbin <zhengbin13@...wei.com>,
        Jeffrey Hugo <jeffrey.l.hugo@...il.com>,
        AngeloGioacchino Del Regno <kholk11@...il.com>,
        Ben Dooks <ben.dooks@...ethink.co.uk>,
        Krzysztof Wilczynski <kw@...ux.com>,
        Harigovindan P <harigovi@...eaurora.org>,
        Brian Masney <masneyb@...tation.org>,
        Sam Ravnborg <sam@...nborg.org>,
        Xiaozhe Shi <xiaozhes@...eaurora.org>,
        Manu Gautam <mgautam@...eaurora.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        "open list:DRM PANEL DRIVERS" <dri-devel@...ts.freedesktop.org>,
        freedreno <freedreno@...ts.freedesktop.org>,
        DTML <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-usb@...r.kernel.org, linux-clk@...r.kernel.org
Subject: Re: [PATCH 9/9] soc/qcom: Add REVID driver

Hi Greg, thanks for your review!

>Why do we need this noise in the kernel log?

I guess it could be left there as a debug print? Knowing your hardware
revision seems like a good, but yeah, not a necessary thing.

>You can drop the GPL boilerplate text and add a proper SPDX line at the
>top.

Seems I only did that in my other local tree.. whoops!

>Drivers should always use dev_err() and friends, as you have access to a
>struct device * always.  Please fix up the driver here to use that api
>instead, no pr_* should be needed at all.

Will do.

>Horrible global symbol name.  Who calls this?

Welcome to development on qcom platforms :D

>This is the last patch in
>the series, so if there is no user for this, please don't export it.

Other downstream drivers make use of it.. need to get this up first, sorry :V

>Why do you need a .h file in the include directory if only a single .c
>file needs it?  Just put that info in the .c file itself.

Again, other downstream drivers which some people and I intend to
bring to upstream standards use that to access the PMIC model/hw revision.

>But again, who uses this module?  If it's only good for a single line in
>the kernel log, that feels like a huge waste to me.

downstream-kernel-dir$ rg -l qpnp-revid.h | wc -l
25

So yeah, quite a bunch of other qcom-specific drivers.

I'll try to fix these and send a v2.

Regards
Konrad