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:   Asmaa Mnebhi <asmaa@nvidia.com>
To:     "andy.shevchenko@gmail.com" <andy.shevchenko@gmail.com>,
        "linus.walleij@linaro.org" <linus.walleij@linaro.org>,
        "bgolaszewski@baylibre.com" <bgolaszewski@baylibre.com>,
        "brgl@bgdev.pl" <brgl@bgdev.pl>,
        "linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
CC:     David Thompson <davthompson@nvidia.com>
Subject: RE: [PATCH v2 1/1] gpio: mmio: handle "ngpios" properly in
 bgpio_init()
Thread-Topic: [PATCH v2 1/1] gpio: mmio: handle "ngpios" properly in
 bgpio_init()
Thread-Index: AQHZtAoywWb5HJNqyUC9IpZWu2Qdu6+/5bsA
Date:   Tue, 18 Jul 2023 18:38:20 +0000
Message-ID: <CH2PR12MB389544F08A0A20AAB06123D3D738A@CH2PR12MB3895.namprd12.prod.outlook.com>
References: <20230711151248.4750-1-asmaa@nvidia.com>
In-Reply-To: <20230711151248.4750-1-asmaa@nvidia.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?TF+bQ45QCgKWB94+rrThQsq9e5ac4iL/iLAyaNfIVi3VY60cjn0zBYhyAK/8?=
 =?us-ascii?Q?VQBhYaIUNFwQKwaEkibptonbKn5gOuvMlaA+FT4VxqbfifVVI/DsxzEdpfDC?=
 =?us-ascii?Q?1wlP0mBmHit/3SZGyRBsHXiJBIoyfoQYXE5t+Q4XnR1d68dgXn8gghSCjOPZ?=
 =?us-ascii?Q?dwMjsQNX+Ivrkwh/mkBBE7+iFe+bfz9aQKLsMylfWyas2xIfPMencQi8e69e?=
 =?us-ascii?Q?2c0Axekvy7WgGJIxN0Q29vYD4LT4+XkHQFo41BYkLFY7B7ElDEhQjw/Zj31C?=
 =?us-ascii?Q?Dihd5/5Tr+O7DWLwoMfkwBBQmbDDpQpoIrZ60Ob6iJZA1k6GklYfce4aopPS?=
 =?us-ascii?Q?+ozAei821+UrYEVKy4MwL/9sioqNrhSNIL58hwTuM4VoHiy0EzSHtC4rkulA?=
 =?us-ascii?Q?n2SONneNpezogC4I36xmb6MI+SsZBdc8nihVBGXtiuBLVQcz1bMTSFh/zDHR?=
 =?us-ascii?Q?1FZGR9kEKFIG5dkflAUPUrR4e4X3gcatjyAYgKsMc2cFfst/AgCky4klUKec?=
 =?us-ascii?Q?5i3C6fTNP4Da+PtdyoK0kAPehbD29xnNqCQ+7kxTZMfZ6zcXYMRFtf6X1KlN?=
 =?us-ascii?Q?HGI4l6FAyX/mL9MG0iUffVz/jT0SNYedKz4o+m3kOK3w27rAxFRr5aGM40vj?=
 =?us-ascii?Q?r3ACHRhd2JsuDcgqCEVuT8aHqgYKM1D1ckl/WsoemT9htBnWI6fjJYOI6jHd?=
 =?us-ascii?Q?Y8+fue5rXc0AqJuOH6WBSchQPPb6JMFCPrUOOeRujm14poWTWEKpHjAaF9q8?=
 =?us-ascii?Q?/ZbMGFixqSjbG2eleXbK4AMOB3vZ0ir8Ew5QEKh+Rqfe/11i7/ZCP2nv1lNh?=
 =?us-ascii?Q?CPziSBFfCnIoq6w1vKCvuHsu8JKyfbzefrJuv6irWnGENb6+yttiOvp4niiQ?=
 =?us-ascii?Q?0OrSJRiTcxYIEBE8tK9gPsh+bLQg1eUHXWPOhqaD+XEkHwa8CBoSG1YiJegs?=
 =?us-ascii?Q?sV+rGD8FrOQnE6ZEssUwQJ20E4GcZPqqOAvHYI8mK9FMQ4Z0I3EtFke6L/Ob?=
 =?us-ascii?Q?YbMx2Xso8xu3h/DhzUEdDkQoq7/uFJzq+3TPg4XKNLT7YCunE2M0jj5ZXx4d?=
 =?us-ascii?Q?ICQhWY8t5LUhof9BCXy2liavgn8JE2MfEaF6Uu0xJuk1fTbs/bHxbAfWmZiz?=
 =?us-ascii?Q?jEkBU23OTSLRwUxtdKvOEMb468ouuglAfJXfZkry9Uu624TlVzSchht0Y6+o?=
 =?us-ascii?Q?lSI9VXg98zH2kpehAh+Q6jjjqg4InDjYznmQm78AZgcBy0K0o4cRnAtOro2Q?=
 =?us-ascii?Q?vG0MNweCiHy6NhPpjA5r7a1cgin/hfDIBmin0mBEW0C+O89JFY1OFPu57ijz?=
 =?us-ascii?Q?xlMAMCTjTQX9CmaTkVyaZKqaGxY9+sXxNpLDBToD1UyHKM/O8xw8XORC6TrK?=
 =?us-ascii?Q?6tDwPv1ie5Cf7pGRFVHaP6uIVQo+7pePwS6EgP8YcfSKxW/cvs9M9wFnL2Bq?=
 =?us-ascii?Q?soG/Pm7EQR42g5yvUkyyzBCEiZ5On8nqNayXfMIKytKBeghi8DCab6qDI2xi?=
 =?us-ascii?Q?5xlivjzp9/RwIno4dXWeiGZSzBb6XMVW2/Q3ce4SLCYD4Ent3xJYUuorei/5?=
 =?us-ascii?Q?5FNaR4PD1pDVWtOvbGY=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB3895.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 09a90a5c-3ae6-484d-3cac-08db87be294e
X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Jul 2023 18:38:20.9357
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: YIDq9Uj0J2zFwKjgYZEVnOiqNQkF30EsiwMGXT8KVmzoNdTTDG67hqXIeifWulcUJlSWKU/Rt+yR9UsadyHc8A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR12MB8658
X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        RCVD_IN_DNSWL_BLOCKED,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=no 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

 > bgpio_init() uses "sz" argument to populate ngpio, which is not accurate=
.
> Instead, read the "ngpios" property from the DT and if it doesn't exist, =
use the
> "sz" argument. With this change, drivers no longer need to overwrite the =
ngpio
> variable after calling bgpio_init().
>=20
> If the "ngpios" property is specified, bgpio_bits is calculated as the ro=
und up
> value of ngpio. At the moment, the only requirement specified is that the=
 round
> up value must be a multiple of 8 but it should also be a power of 2 becau=
se we
> provide accessors based on the bank size in bgpio_setup_accessors().
>=20
> Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> ---
> The following 2 patches were approved in March 2023 but didn't make it in=
to
> the tree:
> [PATCH v4] gpio: mmio: handle "ngpios" properly in bgpio_init() [PATCH v1=
]
> gpio: mmio: fix calculation of bgpio_bits
>=20
> They needed a rebase and were combined into a single patch since
> "gpio: mmio: fix calculation of bgpio_bits" fixes a bug in
> "gpio: mmio: handle "ngpios" properly in bgpio_init()"
>=20
> v1->v2:
> - Added the tags
> - Updated the changelog
>=20
>  drivers/gpio/gpio-mmio.c |  9 +++++-
>  drivers/gpio/gpiolib.c   | 68 ++++++++++++++++++++++------------------
>  drivers/gpio/gpiolib.h   |  1 +
>  3 files changed, 46 insertions(+), 32 deletions(-)
>=20
> diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c index
> d9dff3dc92ae..74fdf0d87b2c 100644
> --- a/drivers/gpio/gpio-mmio.c
> +++ b/drivers/gpio/gpio-mmio.c
> @@ -60,6 +60,8 @@ o        `                     ~~~~\___/~~~~    ` contr=
oller in FPGA
> is ,.`
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>=20
> +#include "gpiolib.h"
> +
>  static void bgpio_write8(void __iomem *reg, unsigned long data)  {
>  	writeb(data, reg);
> @@ -614,10 +616,15 @@ int bgpio_init(struct gpio_chip *gc, struct device
> *dev,
>  	gc->parent =3D dev;
>  	gc->label =3D dev_name(dev);
>  	gc->base =3D -1;
> -	gc->ngpio =3D gc->bgpio_bits;
>  	gc->request =3D bgpio_request;
>  	gc->be_bits =3D !!(flags & BGPIOF_BIG_ENDIAN);
>=20
> +	ret =3D gpiochip_get_ngpios(gc, dev);
> +	if (ret)
> +		gc->ngpio =3D gc->bgpio_bits;
> +	else
> +		gc->bgpio_bits =3D roundup_pow_of_two(round_up(gc->ngpio,
> 8));
> +
>  	ret =3D bgpio_setup_io(gc, dat, set, clr, flags);
>  	if (ret)
>  		return ret;
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index
> 251c875b5c34..7dac8bb9905a 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -700,6 +700,40 @@ void *gpiochip_get_data(struct gpio_chip *gc)  }
> EXPORT_SYMBOL_GPL(gpiochip_get_data);
>=20
> +int gpiochip_get_ngpios(struct gpio_chip *gc, struct device *dev) {
> +	u32 ngpios =3D gc->ngpio;
> +	int ret;
> +
> +	if (ngpios =3D=3D 0) {
> +		ret =3D device_property_read_u32(dev, "ngpios", &ngpios);
> +		if (ret =3D=3D -ENODATA)
> +			/*
> +			 * -ENODATA means that there is no property found
> and
> +			 * we want to issue the error message to the user.
> +			 * Besides that, we want to return different error code
> +			 * to state that supplied value is not valid.
> +			 */
> +			ngpios =3D 0;
> +		else if (ret)
> +			return ret;
> +
> +		gc->ngpio =3D ngpios;
> +	}
> +
> +	if (gc->ngpio =3D=3D 0) {
> +		chip_err(gc, "tried to insert a GPIO chip with zero lines\n");
> +		return -EINVAL;
> +	}
> +
> +	if (gc->ngpio > FASTPATH_NGPIO)
> +		chip_warn(gc, "line cnt %u is greater than fast path cnt %u\n",
> +			gc->ngpio, FASTPATH_NGPIO);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(gpiochip_get_ngpios);
> +
>  int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
>  			       struct lock_class_key *lock_key,
>  			       struct lock_class_key *request_key) @@ -707,7
> +741,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data=
,
>  	struct gpio_device *gdev;
>  	unsigned long flags;
>  	unsigned int i;
> -	u32 ngpios =3D 0;
>  	int base =3D 0;
>  	int ret =3D 0;
>=20
> @@ -753,36 +786,9 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc,
> void *data,
>  	else
>  		gdev->owner =3D THIS_MODULE;
>=20
> -	/*
> -	 * Try the device properties if the driver didn't supply the number
> -	 * of GPIO lines.
> -	 */
> -	ngpios =3D gc->ngpio;
> -	if (ngpios =3D=3D 0) {
> -		ret =3D device_property_read_u32(&gdev->dev, "ngpios",
> &ngpios);
> -		if (ret =3D=3D -ENODATA)
> -			/*
> -			 * -ENODATA means that there is no property found
> and
> -			 * we want to issue the error message to the user.
> -			 * Besides that, we want to return different error code
> -			 * to state that supplied value is not valid.
> -			 */
> -			ngpios =3D 0;
> -		else if (ret)
> -			goto err_free_dev_name;
> -
> -		gc->ngpio =3D ngpios;
> -	}
> -
> -	if (gc->ngpio =3D=3D 0) {
> -		chip_err(gc, "tried to insert a GPIO chip with zero lines\n");
> -		ret =3D -EINVAL;
> +	ret =3D gpiochip_get_ngpios(gc, &gdev->dev);
> +	if (ret)
>  		goto err_free_dev_name;
> -	}
> -
> -	if (gc->ngpio > FASTPATH_NGPIO)
> -		chip_warn(gc, "line cnt %u is greater than fast path cnt %u\n",
> -			  gc->ngpio, FASTPATH_NGPIO);
>=20
>  	gdev->descs =3D kcalloc(gc->ngpio, sizeof(*gdev->descs), GFP_KERNEL);
>  	if (!gdev->descs) {
> @@ -947,7 +953,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc,
> void *data,
>  	/* failures here can mean systems won't boot... */
>  	if (ret !=3D -EPROBE_DEFER) {
>  		pr_err("%s: GPIOs %d..%d (%s) failed to register, %d\n",
> __func__,
> -		       base, base + (int)ngpios - 1,
> +		       base, base + (int)gc->ngpio - 1,
>  		       gc->label ? : "generic", ret);
>  	}
>  	return ret;
> diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h index
> cca81375f127..8de748a16d13 100644
> --- a/drivers/gpio/gpiolib.h
> +++ b/drivers/gpio/gpiolib.h
> @@ -217,6 +217,7 @@ int gpiod_configure_flags(struct gpio_desc *desc, con=
st
> char *con_id,  int gpio_set_debounce_timeout(struct gpio_desc *desc,
> unsigned int debounce);  int gpiod_hog(struct gpio_desc *desc, const char
> *name,
>  		unsigned long lflags, enum gpiod_flags dflags);
> +int gpiochip_get_ngpios(struct gpio_chip *gc, struct device *dev);
>=20
>  /*
>   * Return the GPIO number of the passed descriptor relative to its chip
> --
> 2.30.1

Hi Bart,

This is the final approved patch by both Linus and Andy. Please discard all=
 others.