linux-kernel - Re: [PATCH v2 1/6] hwmon: (gxp-fan-ctrl) Add GXP fan controller

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]

Date:   Tue, 29 Nov 2022 10:26:28 -0800
From:   Guenter Roeck <linux@...ck-us.net>
To:     "Hawkins, Nick" <nick.hawkins@....com>
Cc:     "jdelvare@...e.com" <jdelvare@...e.com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "krzysztof.kozlowski+dt@...aro.org" 
        <krzysztof.kozlowski+dt@...aro.org>,
        "Verdun, Jean-Marie" <verdun@....com>,
        "corbet@....net" <corbet@....net>,
        "linux@...linux.org.uk" <linux@...linux.org.uk>,
        "linux-hwmon@...r.kernel.org" <linux-hwmon@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 1/6] hwmon: (gxp-fan-ctrl) Add GXP fan controller

On 11/29/22 09:13, Hawkins, Nick wrote:
>>> +		writeb(val, drvdata->base + channel);
> 
>> The mixed use of direct writes and regmap is odd and confusing.
>> Why use regmap for plreg_map and for fn2_map but not for base ?
>> Can this be unified ? If not, why ?
> 
> I will still require three separate areas to access these registers as they
> are spaced far apart (> 0x20000000). I will create a comment to
> explain this. As for being unified methods I believe they can be
> all __iomem's or regmaps.
> 

I understand that there are three regions. What I don't understand is
that two of them are accessed through regmap and one directly. Either
access all regions using regmap, or all regions using iomem accessors
directly. If regmap does not support access to the base region,
don't use regmap at all, explain why regmap doesn't support this kind
of access, and make sure to copy the regmap maintainer.

Thanks,
Guenter