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;
From:   Vladimir Oltean <vladimir.oltean@nxp.com>
To:     Colin Foster <colin.foster@in-advantage.com>
CC:     "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
        Lee Jones <lee.jones@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Andrew Lunn <andrew@lunn.ch>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Lars Povlsen <lars.povlsen@microchip.com>,
        Steen Hegelund <Steen.Hegelund@microchip.com>,
        "UNGLinuxDriver@microchip.com" <UNGLinuxDriver@microchip.com>,
        Linus Walleij <linus.walleij@linaro.org>,
        Wolfram Sang <wsa@kernel.org>,
        Terry Bowman <terry.bowman@amd.com>,
        Andy Shevchenko <andy.shevchenko@gmail.com>,
        "katie.morris@in-advantage.com" <katie.morris@in-advantage.com>
Subject: Re: [PATCH v13 net-next 1/9] mfd: ocelot: add helper to get regmap
 from a resource
Thread-Topic: [PATCH v13 net-next 1/9] mfd: ocelot: add helper to get regmap
 from a resource
Thread-Index: AQHYkLCMdCdG3kd5wEePWYl2oYvNba12ab4A
Date:   Sat, 9 Jul 2022 18:56:15 +0000
Message-ID: <20220709185614.4rrzo4hvmkena3ob@skbuf>
References: <20220705204743.3224692-1-colin.foster@in-advantage.com>
 <20220705204743.3224692-2-colin.foster@in-advantage.com>
In-Reply-To: <20220705204743.3224692-2-colin.foster@in-advantage.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?k8YySaJHCMZXstI1K2PfJtjrHjbkKa779CERcUKxqb9StqTGuoXGtWYsqZBD?=
 =?us-ascii?Q?mJ/0uqqQwlw1VpASbarlQxGfjt8/aClGwT5HolRO8lCcvQn2VJsOx4tdkOWR?=
 =?us-ascii?Q?MA2Sy7yPYRQ33Q3lyh3hnqjMI/eoZTDCGjFBnByZcBmMd1y+B3T/+U6b9c3G?=
 =?us-ascii?Q?zhhEXoNqnQwGMh9oPCXEE66MA0VNiMFChBUroyzCJY1vDcl16q40VeEWuVgC?=
 =?us-ascii?Q?zOcqQwRwDYp7GJOGzBE3kk9DWn7q4PkxqaTa7vSyF0ld8FksD8eJ03doRxE9?=
 =?us-ascii?Q?07XUGXNk2jaKlSuW+f5kHU+GD6R4yWsrbEMsJ92QnyEf214UN8OcLNAOAp8Q?=
 =?us-ascii?Q?4yrOta19aPFWOAfsz9NjaRG6eKyIvZKOpPa2LR8jtR+1kxgclMktZNZfMMug?=
 =?us-ascii?Q?FRDVFrSwF6Dfv8NDxVGYLbmGMVkjvTrBqQWDAwbw717DleTRPd7v5oDl5xwN?=
 =?us-ascii?Q?RdaqV2TGwRQxbu5iChY8naA7/AgdBS8HnbhTIyzLBLY6jzZHugVlJ7nFPfBA?=
 =?us-ascii?Q?cf4jIJKvZqpkmOTIJ6rGCrPLWlbF52BwF/I2ohRHU9IZIJzagv5AqbAnre45?=
 =?us-ascii?Q?Zkly/T64hct0zlt0FsWKFHK76dOXE1JWnqSIeE2FfMDB0h4alY61FpumkkKZ?=
 =?us-ascii?Q?hiULx6/vlDv3+1E5efnBoS6puIqePff+QxQihDLgT6YNp8T0TPAJKfHqXo7+?=
 =?us-ascii?Q?2qNgza9CQIizmP/UxleAdeDeTOulPgsFWUR3cvgbQwvHq4TvHxp9UA500WKx?=
 =?us-ascii?Q?SUcVASBQj1zI91L7gc0lKevzQuQY5utpcoTJG+37/mHb+gOZukabg6D29CvK?=
 =?us-ascii?Q?NuqIYoUdlZbKh1mKvrxGya7T7JDL2OxgTMJiCEm3OfW8LC9ToJIM67Dwz5L4?=
 =?us-ascii?Q?M2Hlr5bljtcA4ftSUU9exsDehIUp1Hv4Ibf8aynpmN5E9DldnemXaRKOufRB?=
 =?us-ascii?Q?elP+wmLNyexJGG08O367VulZ2boTQ7P2WhSLNoX7epQ6hypxB25L5aFm/TDN?=
 =?us-ascii?Q?n5U3QwWaS3CzWJEzz9JAy3L1HQmOzhxEywogP6uM2sBvlLt7trLdBrc/wGaE?=
 =?us-ascii?Q?52IGT2Ft3sQfPzSYYh34EMKnZ3oJKFxj5x4Oq80DN87zVlSkO51+QJHjtSm8?=
 =?us-ascii?Q?bcan9t2LsZEVB9rl84xjgk9cSYcTgmz5CZ8ZQF/Ase2yIQxkaYLHAJDy4x+1?=
 =?us-ascii?Q?5g3ufssdzp6iMO09R5vE8KtOTLHwegROGOGN9sUiV+jsjzqS++9bGfh+w8t0?=
 =?us-ascii?Q?66LCfjw+fzuGYw/xeztO8bt/3HVEX+3yqv5tHjhzW9VCPCnqUsuxKpCmzFSK?=
 =?us-ascii?Q?omPvwSUPXvuel00w9ki5g8gai98RiwyxKO6swKFJmtQsmCQWqGZKcnlTSfD+?=
 =?us-ascii?Q?glvN9ESEjEdl6m5i9Kc+CTXGmtUG4efvRGsiDDJyN5t86xn9FJvYuwmxqTsR?=
 =?us-ascii?Q?INz9VoGKMkt3F+nJqT4GRJCVMqtjMK6kgV88LCmnZUSjz3jt9hjb3AhLc+JD?=
 =?us-ascii?Q?jVYQSgMS07UtTRSct8ARNaDnlV6fJOwChbNhcx1waO0LrngN0nuEonhL7piM?=
 =?us-ascii?Q?ZS7o5QGeqAglsTJ8tjYG9Zw+PDIzMCZK7U0RHusStwZD6aEwveoxzIfmjiaE?=
 =?us-ascii?Q?oQ=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <64FFC8B0678A144B8940B4D0D14A45B2@eurprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: VI1PR04MB5136.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 76a99870-ad99-483f-5e35-08da61dcb31d
X-MS-Exchange-CrossTenant-originalarrivaltime: 09 Jul 2022 18:56:15.1935
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: lUs/3Op5eT70ZiweT9FQe/OufHyT2G4eHKr0dl77ABb/sNWLLTprfD7IaePemh5tRjoEIStOKtspH6NgqQ3AvQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PA4PR04MB9462
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Jul 05, 2022 at 01:47:35PM -0700, Colin Foster wrote:
> Several ocelot-related modules are designed for MMIO / regmaps. As such,
> they often use a combination of devm_platform_get_and_ioremap_resource an=
d
> devm_regmap_init_mmio.
>=20
> Operating in an MFD might be different, in that it could be memory mapped=
,
> or it could be SPI, I2C... In these cases a fallback to use IORESOURCE_RE=
G
> instead of IORESOURCE_MEM becomes necessary.
>=20
> When this happens, there's redundant logic that needs to be implemented i=
n
> every driver. In order to avoid this redundancy, utilize a single functio=
n
> that, if the MFD scenario is enabled, will perform this fallback logic.
>=20
> Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
> ---

To me this looks good, I'll just add a few minor comments which I think
you don't necessarily need to address by resending, they're just things
I noticed.

Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>

>  MAINTAINERS                |  5 ++++
>  include/linux/mfd/ocelot.h | 55 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 60 insertions(+)
>  create mode 100644 include/linux/mfd/ocelot.h
>=20
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 28108e4fdb8f..f781caceeb38 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14467,6 +14467,11 @@ F:	net/dsa/tag_ocelot.c
>  F:	net/dsa/tag_ocelot_8021q.c
>  F:	tools/testing/selftests/drivers/net/ocelot/*
> =20
> +OCELOT EXTERNAL SWITCH CONTROL
> +M:	Colin Foster <colin.foster@in-advantage.com>
> +S:	Supported
> +F:	include/linux/mfd/ocelot.h
> +
>  OCXL (Open Coherent Accelerator Processor Interface OpenCAPI) DRIVER
>  M:	Frederic Barrat <fbarrat@linux.ibm.com>
>  M:	Andrew Donnellan <ajd@linux.ibm.com>
> diff --git a/include/linux/mfd/ocelot.h b/include/linux/mfd/ocelot.h
> new file mode 100644
> index 000000000000..353b7c2ee445
> --- /dev/null
> +++ b/include/linux/mfd/ocelot.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0 OR MIT */
> +/* Copyright 2022 Innovative Advantage Inc. */

A header file should have ifdefs which should prevent double inclusion,
like

#ifndef _MFD_OCELOT_H
#define _MFD_OCELOT_H

...

#endif

> +
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/types.h>
> +
> +struct resource;

IMO if include/linux/platform_device.h doesn't provide "struct resource"
that's a problem for that header to solve, not for its users.

> +
> +static inline struct regmap *
> +ocelot_regmap_from_resource_optional(struct platform_device *pdev,
> +				     unsigned int index,
> +				     const struct regmap_config *config)
> +{
> +	struct device *dev =3D &pdev->dev;
> +	struct resource *res;
> +	u32 __iomem *regs;

"regs" could be void *.

> +
> +	/*
> +	 * Don't use get_and_ioremap_resource here, since that will invoke
> +	 * prints of "invalid resource" which simply add confusion
> +	 */
> +	res =3D platform_get_resource(pdev, IORESOURCE_MEM, index);
> +	if (res) {
> +		regs =3D devm_ioremap_resource(dev, res);
> +		if (IS_ERR(regs))
> +			return ERR_CAST(regs);
> +		return devm_regmap_init_mmio(dev, regs, config);
> +	}
> +
> +	/*
> +	 * Fall back to using REG and getting the resource from the parent
> +	 * device, which is possible in an MFD configuration
> +	 */
> +	if (dev->parent) {
> +		res =3D platform_get_resource(pdev, IORESOURCE_REG, index);
> +		if (!res)
> +			return NULL;
> +
> +		return dev_get_regmap(dev->parent, res->name);
> +	}
> +
> +	return NULL;
> +}
> +
> +static inline struct regmap *
> +ocelot_regmap_from_resource(struct platform_device *pdev, unsigned int i=
ndex,
> +			    const struct regmap_config *config)
> +{
> +	struct regmap *map;
> +
> +	map =3D ocelot_regmap_from_resource_optional(pdev, index, config);
> +	return map ?: ERR_PTR(-ENOENT);
> +}
> --=20
> 2.25.1
>=