Date:   Mon, 30 Jan 2023 18:12:04 -0800
From:   Colin Foster <colin.foster@in-advantage.com>
To:     Daniel Golle <daniel@makrotopia.org>
Cc:     linux-kernel@vger.kernel.org, Mark Brown <broonie@kernel.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [RFC PATCH v2] regmap: apply reg_base and reg_downshift for
 single register ops
Message-ID: <Y9h49F7QxdTa/deD@euler>
References: <Y9WAPBYaLMsCbQN6@euler>
 <Y9clyVS3tQEHlUhA@makrotopia.org>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Y9clyVS3tQEHlUhA@makrotopia.org>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?dZP9VnfY/UKo+Q0tiWhtSbZjcFpx3eRbFXKh+C3NqngLAfcHGJcwwaCO0NE1?=
 =?us-ascii?Q?VkDvjjuXJvaeljin3fE9tTAQ9dR3d434idN99z1GoAb592jEdEURXndGJZbc?=
 =?us-ascii?Q?uMum80/J6G+PFdvtHQ8uInzSgjKMKFDpEulyjb3wugWXmC983n6973CzFTZ/?=
 =?us-ascii?Q?Nt2RNYrgkuwwqi+GksRVoha5xfK0yOflpwhSraqZuxNrGe6AD2DeuF5TWwpk?=
 =?us-ascii?Q?qheH5fy9+CnDYfLlJzdQt1qsBuj8QlNTbuPp2hcsu+QPT2AS/IabKk3c+2JH?=
 =?us-ascii?Q?KPajRV+Al6rANItBfxT4D7NQDUstE22SbkiGzAly2/8+dQ+ExdtLYPm/K6YB?=
 =?us-ascii?Q?fIcdoJHfC+/0L2BdeMmMdmfg9CIAIEuk2q0p41H/bUcPv+jpS9vTy0Z+9t//?=
 =?us-ascii?Q?dKyx7xGWs5nMIu4e8BDuLkw9khktcjBEiMkwI72O7lwBOqR291H+8CBIDrC7?=
 =?us-ascii?Q?7qPl6YKIKPN27xXXLkBkhDLSjHPVScOO2qFj2JohLEV7B/zbSz1/JnYxeOQ2?=
 =?us-ascii?Q?FyiRHnvEsLZD+pVOryD9dMPvNsUDSwxwLbxy3VAnnRzrzrITog4EwH/9C95m?=
 =?us-ascii?Q?R183zLLalGGAtq0dRoDofVmUx4sgy+E6iTnZaoVkD0Dbun3nVYqWO4A1tuOs?=
 =?us-ascii?Q?GZN1iHIFblct+pAP9erC1SPDfHdNiwtdNBTrj8SPfkgSrty10xEKU9oiGhVE?=
 =?us-ascii?Q?9Xo1c61Yvl5lBmTvz//uy/Xc3Nu5TLvDLSFazYR1BMKyvcF52oAvGsJCI4Ln?=
 =?us-ascii?Q?BrDtNTkK6lK9TKlEBdeIYThpf4lr0dDXKRnxZ65Mdj6pOua0VNnbJPd1sIe7?=
 =?us-ascii?Q?1di3cwo7yTTz8n6++galUTufyiq2cpKgWSmMSz+XYH5nNjKze9A2FIPliPt+?=
 =?us-ascii?Q?7pjUUN1DdSAONNuOlcge8W5r4RqzwdVYRYp/JnORukJpYYPgTZ6XbbQIvaVo?=
 =?us-ascii?Q?nc0aMHcS+Ym3D4Oxm8vHKijT46VX9mRMMJee/P2LcAvh9JyY60Z124cBfid9?=
 =?us-ascii?Q?RK6fb6vmdZ1SNFm7fWl/dPAxk7yZW5W5hSWwbyUd9H6lbXKvO/uzr4gAKvoH?=
 =?us-ascii?Q?QMaH9Bkayee7ROcRqvw4lQ2E9lNDcx176Jhha2Clvl3pxiDIR0HAMnn2rq1T?=
 =?us-ascii?Q?wLs4JXh4R9lMH9nheUYz4WBaHQA/HedXfDVYqAqmlJl7lUsLIoK/X2rfWM48?=
 =?us-ascii?Q?IPD8vaSgZ4m2Qf6kBuRtLrJHPxZlwPDRfMJ4v0Cj4XhpjLIRBHClrkrCJ2wB?=
 =?us-ascii?Q?8K3nZ+sYvuq5z6cAUP74nB0CrNwm46oJbzAktn86UZ2Xon9rERFbgepo+lV8?=
 =?us-ascii?Q?ALDbGqCJ2dx+WxW9MYZar/+09EeWwvy6AB4JnX076VWGDiGvIAMnRx72X7EA?=
 =?us-ascii?Q?lx2qiDgle85cDE3SPSEGDmGdqsKfFq88UwJ7kaplRU0KzCaCXxgb7SzC1HO5?=
 =?us-ascii?Q?OburYyVzHDQTTVrJHqjsVtdjFi0njzldbtoBzc8UmJFRa71HVoYJjrppvDmd?=
 =?us-ascii?Q?uxKhfoUbsfFiPQjxcGw9Av9YOe5Wwf2lGo8BX09ikqnyXprxoTlMXD6AtJtj?=
 =?us-ascii?Q?n9fJ93jTpub5C3I+AEJXC20NTXAlwzt76qJ1B3iBQlnSwtdYM/vUlr5PKwbp?=
 =?us-ascii?Q?0Zfg/20bFFoRWTs+3kQARU0=3D?=
X-OriginatorOrg: in-advantage.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 16f32703-d673-43cd-ff4b-08db03308da4
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2351.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Jan 2023 02:12:07.6521
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 48e842ca-fbd8-4633-a79d-0c955a7d3aae
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: t9BvC3mOeC3Mm/9Z4fi8P0TNDfrZLCS04OCp/6nO4fKHjswRQUwOBvDqTau2Ajy6jrrnfqT/LoWwSBF92QqPoCCcGhWRU7BZ85acQkRklos=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR10MB4633
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Daniel,

On Mon, Jan 30, 2023 at 02:04:57AM +0000, Daniel Golle wrote:
> reg_base and reg_downshift currently don't have any effect if used with
> a regmap_bus or regmap_config which only offers single register
> operations (ie. reg_read, reg_write and optionally reg_update_bits).
> 
> Fix that and take them into account also for regmap_bus with only
> reg_read and read_write operations by applying reg_base and
> reg_downshift in _regmap_bus_reg_write, _regmap_bus_reg_read.
> 
> Also apply reg_base and reg_downshift in _regmap_update_bits, but only
> in case the operation is carried out with a reg_update_bits call
> defined in either regmap_bus or regmap_config.
> 
> Fixes: 0074f3f2b1e43d ("regmap: allow a defined reg_base to be added to every address")
> Fixes: 86fc59ef818beb ("regmap: add configurable downshift for addresses")
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
> I hope that I didn't miss anything there...
> 
> @Colin Please let me know if this breaks anything with your ocelot_spi
> use-case.

I see we're working on similar things! (DSA hardware, that is)

This patch works for me. I don't konw if there's any value in
back-porting it to affected kernels, as ocelot_spi is the only user as
far as I can tell. (wishing I called it something more greppable than
'reg_base')

Tested-by: Colin Foster <colin.foster@in-advantage.com>

> 
>  drivers/base/regmap/regmap.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
> index d12d669157f24..d2a54eb0efd9b 100644
> --- a/drivers/base/regmap/regmap.c
> +++ b/drivers/base/regmap/regmap.c
> @@ -1942,6 +1942,8 @@ static int _regmap_bus_reg_write(void *context, unsigned int reg,
>  {
>  	struct regmap *map = context;
>  
> +	reg += map->reg_base;
> +	reg >>= map->format.reg_downshift;
>  	return map->bus->reg_write(map->bus_context, reg, val);
>  }
>  
> @@ -2840,6 +2842,8 @@ static int _regmap_bus_reg_read(void *context, unsigned int reg,
>  {
>  	struct regmap *map = context;
>  
> +	reg += map->reg_base;
> +	reg >>= map->format.reg_downshift;
>  	return map->bus->reg_read(map->bus_context, reg, val);
>  }
>  
> @@ -3231,6 +3235,8 @@ static int _regmap_update_bits(struct regmap *map, unsigned int reg,
>  		*change = false;
>  
>  	if (regmap_volatile(map, reg) && map->reg_update_bits) {
> +		reg += map->reg_base;
> +		reg >>= map->format.reg_downshift;
>  		ret = map->reg_update_bits(map->bus_context, reg, mask, val);
>  		if (ret == 0 && change)
>  			*change = true;
> -- 
> 2.39.1
>