Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Sender: Guenter Roeck <groeck7@gmail.com>
Message-ID: <4fb6abe0-a68e-f391-5111-dab139891837@roeck-us.net>
Date:   Tue, 29 Nov 2022 10:26:28 -0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.2
Subject: Re: [PATCH v2 1/6] hwmon: (gxp-fan-ctrl) Add GXP fan controller
Content-Language: en-US
To:     "Hawkins, Nick" <nick.hawkins@hpe.com>
Cc:     "jdelvare@suse.com" <jdelvare@suse.com>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "krzysztof.kozlowski+dt@linaro.org" 
        <krzysztof.kozlowski+dt@linaro.org>,
        "Verdun, Jean-Marie" <verdun@hpe.com>,
        "corbet@lwn.net" <corbet@lwn.net>,
        "linux@armlinux.org.uk" <linux@armlinux.org.uk>,
        "linux-hwmon@vger.kernel.org" <linux-hwmon@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>
References: <20221128230219.39537-1-nick.hawkins@hpe.com>
 <20221128230219.39537-2-nick.hawkins@hpe.com>
 <20221129040232.GA1901150@roeck-us.net>
 <B3B2AA1C-0AD8-406D-B766-4CB86C39A834@hpe.com>
From:   Guenter Roeck <linux@roeck-us.net>
In-Reply-To: <B3B2AA1C-0AD8-406D-B766-4CB86C39A834@hpe.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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