at86rf230: convert to gpio descriptors

Message ID	20230126135215.3387820-1-arnd@kernel.org (mailing list archive)
State	Superseded
Delegated to:	Netdev Maintainers
Headers	show Return-Path: <netdev-owner@vger.kernel.org> From: Arnd Bergmann <arnd@kernel.org> To: Alexander Aring <alex.aring@gmail.com>, Stefan Schmidt <stefan@datenfreihafen.org> Cc: linux-gpio@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Miquel Raynal <miquel.raynal@bootlin.com>, Andy Shevchenko <andriy.shevchenko@linux.intel.com>, =?utf-8?q?Uwe_Kleine-K?= =?utf-8?q?=C3=B6nig?= <u.kleine-koenig@pengutronix.de>, linux-wpan@vger.kernel.org, netdev@vger.kernel.org Subject: [PATCH] at86rf230: convert to gpio descriptors Date: Thu, 26 Jan 2023 14:51:23 +0100 Message-Id: <20230126135215.3387820-1-arnd@kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	at86rf230: convert to gpio descriptors \| expand at86rf230: convert to gpio descriptors

Context	Check	Description
netdev/tree_selection	success	Guessed tree name to be net-next
netdev/fixes_present	success	Fixes tag not required for -next series
netdev/subject_prefix	warning	Target tree name not specified in the subject
netdev/cover_letter	success	Single patches do not need cover letters
netdev/patch_count	success	Link
netdev/header_inline	success	No static functions without inline keyword in header files
netdev/build_32bit	success	Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers	warning	2 maintainers not CCed: linux-spi@vger.kernel.org broonie@kernel.org
netdev/build_clang	success	Errors and warnings before: 0 this patch: 0
netdev/module_param	success	Was 0 now: 0
netdev/verify_signedoff	success	Signed-off-by tag matches author and committer
netdev/check_selftest	success	No net selftest shell script
netdev/verify_fixes	success	No Fixes tag
netdev/build_allmodconfig_warn	success	Errors and warnings before: 0 this patch: 0
netdev/checkpatch	warning	WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
netdev/kdoc	success	Errors and warnings before: 0 this patch: 0
netdev/source_inline	success	Was 0 now: 0

Arnd Bergmann Jan. 26, 2023, 1:51 p.m. UTC

From: Arnd Bergmann <arnd@arndb.de>

There are no remaining in-tree users of the platform_data,
so this driver can be converted to using the simpler gpiod
interfaces.

Any out-of-tree users that rely on the platform data can
provide the data using the device_property and gpio_lookup
interfaces instead.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ieee802154/at86rf230.c | 82 +++++++++---------------------
 include/linux/spi/at86rf230.h      | 20 --------
 2 files changed, 25 insertions(+), 77 deletions(-)
 delete mode 100644 include/linux/spi/at86rf230.h

Miquel Raynal Jan. 26, 2023, 2:12 p.m. UTC | #1

Hi Arnd,

arnd@kernel.org wrote on Thu, 26 Jan 2023 14:51:23 +0100:

> From: Arnd Bergmann <arnd@arndb.de>
> 
> There are no remaining in-tree users of the platform_data,
> so this driver can be converted to using the simpler gpiod
> interfaces.
> 
> Any out-of-tree users that rely on the platform data can
> provide the data using the device_property and gpio_lookup
> interfaces instead.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/net/ieee802154/at86rf230.c | 82 +++++++++---------------------
>  include/linux/spi/at86rf230.h      | 20 --------
>  2 files changed, 25 insertions(+), 77 deletions(-)
>  delete mode 100644 include/linux/spi/at86rf230.h
> 

[...]

> @@ -1682,7 +1650,7 @@ MODULE_DEVICE_TABLE(spi, at86rf230_device_id);
>  static struct spi_driver at86rf230_driver = {
>  	.id_table = at86rf230_device_id,
>  	.driver = {
> -		.of_match_table = of_match_ptr(at86rf230_of_match),
> +		.of_match_table = at86rf230_of_match,linux-gnueabihf embed a C library which relies on kernel headers (for example, to provide an open API which translates to an open syscall), for exam

Looks like an unrelated change? Or is it a consequence of "not having
any in-tree users of platform_data" that plays a role here?

Anyhow, the changes in the driver look good, so:

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

Andy Shevchenko Jan. 26, 2023, 2:13 p.m. UTC | #2

On Thu, Jan 26, 2023 at 02:51:23PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> There are no remaining in-tree users of the platform_data,
> so this driver can be converted to using the simpler gpiod
> interfaces.
> 
> Any out-of-tree users that rely on the platform data can
> provide the data using the device_property and gpio_lookup
> interfaces instead.

Precisely!

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Some not-picks below.

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/net/ieee802154/at86rf230.c | 82 +++++++++---------------------
>  include/linux/spi/at86rf230.h      | 20 --------
>  2 files changed, 25 insertions(+), 77 deletions(-)
>  delete mode 100644 include/linux/spi/at86rf230.h
> 
> diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
> index 15f283b26721..66193756c891 100644
> --- a/drivers/net/ieee802154/at86rf230.c
> +++ b/drivers/net/ieee802154/at86rf230.c
> @@ -15,14 +15,13 @@
>  #include <linux/jiffies.h>
>  #include <linux/interrupt.h>
>  #include <linux/irq.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/delay.h>
>  #include <linux/spi/spi.h>
> -#include <linux/spi/at86rf230.h>
>  #include <linux/regmap.h>
>  #include <linux/skbuff.h>
> -#include <linux/of_gpio.h>
>  #include <linux/ieee802154.h>
> +#include <linux/property.h>

This can be sorted a bit better, i.e. squeezing property.h before regmap.h,
gpio/consumer.h after delay.h.

>  #include <net/mac802154.h>
>  #include <net/cfg802154.h>
> @@ -82,7 +81,7 @@ struct at86rf230_local {
>  	struct ieee802154_hw *hw;
>  	struct at86rf2xx_chip_data *data;
>  	struct regmap *regmap;
> -	int slp_tr;
> +	struct gpio_desc *slp_tr;
>  	bool sleep;
>  
>  	struct completion state_complete;
> @@ -107,8 +106,8 @@ at86rf230_async_state_change(struct at86rf230_local *lp,
>  static inline void
>  at86rf230_sleep(struct at86rf230_local *lp)
>  {
> -	if (gpio_is_valid(lp->slp_tr)) {
> -		gpio_set_value(lp->slp_tr, 1);
> +	if (lp->slp_tr) {
> +		gpiod_set_value(lp->slp_tr, 1);
>  		usleep_range(lp->data->t_off_to_sleep,
>  			     lp->data->t_off_to_sleep + 10);
>  		lp->sleep = true;
> @@ -118,8 +117,8 @@ at86rf230_sleep(struct at86rf230_local *lp)
>  static inline void
>  at86rf230_awake(struct at86rf230_local *lp)
>  {
> -	if (gpio_is_valid(lp->slp_tr)) {
> -		gpio_set_value(lp->slp_tr, 0);
> +	if (lp->slp_tr) {
> +		gpiod_set_value(lp->slp_tr, 0);
>  		usleep_range(lp->data->t_sleep_to_off,
>  			     lp->data->t_sleep_to_off + 100);
>  		lp->sleep = false;
> @@ -204,9 +203,9 @@ at86rf230_write_subreg(struct at86rf230_local *lp,
>  static inline void
>  at86rf230_slp_tr_rising_edge(struct at86rf230_local *lp)
>  {
> -	gpio_set_value(lp->slp_tr, 1);
> +	gpiod_set_value(lp->slp_tr, 1);
>  	udelay(1);
> -	gpio_set_value(lp->slp_tr, 0);
> +	gpiod_set_value(lp->slp_tr, 0);
>  }
>  
>  static bool
> @@ -819,7 +818,7 @@ at86rf230_write_frame_complete(void *context)
>  
>  	ctx->trx.len = 2;
>  
> -	if (gpio_is_valid(lp->slp_tr))
> +	if (lp->slp_tr)
>  		at86rf230_slp_tr_rising_edge(lp);
>  	else
>  		at86rf230_async_write_reg(lp, RG_TRX_STATE, STATE_BUSY_TX, ctx,
> @@ -1415,32 +1414,6 @@ static int at86rf230_hw_init(struct at86rf230_local *lp, u8 xtal_trim)
>  	return at86rf230_write_subreg(lp, SR_SLOTTED_OPERATION, 0);
>  }
>  
> -static int
> -at86rf230_get_pdata(struct spi_device *spi, int *rstn, int *slp_tr,
> -		    u8 *xtal_trim)
> -{
> -	struct at86rf230_platform_data *pdata = spi->dev.platform_data;
> -	int ret;
> -
> -	if (!IS_ENABLED(CONFIG_OF) || !spi->dev.of_node) {
> -		if (!pdata)
> -			return -ENOENT;
> -
> -		*rstn = pdata->rstn;
> -		*slp_tr = pdata->slp_tr;
> -		*xtal_trim = pdata->xtal_trim;
> -		return 0;
> -	}
> -
> -	*rstn = of_get_named_gpio(spi->dev.of_node, "reset-gpio", 0);
> -	*slp_tr = of_get_named_gpio(spi->dev.of_node, "sleep-gpio", 0);
> -	ret = of_property_read_u8(spi->dev.of_node, "xtal-trim", xtal_trim);
> -	if (ret < 0 && ret != -EINVAL)
> -		return ret;
> -
> -	return 0;
> -}
> -
>  static int
>  at86rf230_detect_device(struct at86rf230_local *lp)
>  {
> @@ -1547,7 +1520,8 @@ static int at86rf230_probe(struct spi_device *spi)
>  	struct ieee802154_hw *hw;
>  	struct at86rf230_local *lp;
>  	unsigned int status;
> -	int rc, irq_type, rstn, slp_tr;
> +	int rc, irq_type;
> +	struct gpio_desc *rstn, *slp_tr;
>  	u8 xtal_trim = 0;
>  
>  	if (!spi->irq) {
> @@ -1555,32 +1529,26 @@ static int at86rf230_probe(struct spi_device *spi)
>  		return -EINVAL;
>  	}
>  
> -	rc = at86rf230_get_pdata(spi, &rstn, &slp_tr, &xtal_trim);
> -	if (rc < 0) {
> -		dev_err(&spi->dev, "failed to parse platform_data: %d\n", rc);
> +	rc = device_property_read_u8(&spi->dev, "xtal-trim", &xtal_trim);
> +	if (rc < 0 && rc != -EINVAL) {
> +		dev_err(&spi->dev, "failed to parse xtal-trim: %d\n", rc);
>  		return rc;
>  	}
>  
> -	if (gpio_is_valid(rstn)) {
> -		rc = devm_gpio_request_one(&spi->dev, rstn,
> -					   GPIOF_OUT_INIT_HIGH, "rstn");
> -		if (rc)
> -			return rc;
> -	}
> +	rstn = devm_gpiod_get_optional(&spi->dev, "rstn", GPIOD_OUT_HIGH);
> +	if (IS_ERR(rstn))
> +		return PTR_ERR(rstn);
>  
> -	if (gpio_is_valid(slp_tr)) {
> -		rc = devm_gpio_request_one(&spi->dev, slp_tr,
> -					   GPIOF_OUT_INIT_LOW, "slp_tr");
> -		if (rc)
> -			return rc;
> -	}
> +	slp_tr = devm_gpiod_get_optional(&spi->dev, "slp_tr", GPIOD_OUT_LOW);
> +	if (IS_ERR(slp_tr))
> +		return PTR_ERR(slp_tr);
>  
>  	/* Reset */
> -	if (gpio_is_valid(rstn)) {
> +	if (rstn) {
>  		udelay(1);
> -		gpio_set_value_cansleep(rstn, 0);
> +		gpiod_set_value_cansleep(rstn, 0);
>  		udelay(1);
> -		gpio_set_value_cansleep(rstn, 1);
> +		gpiod_set_value_cansleep(rstn, 1);
>  		usleep_range(120, 240);
>  	}
>  
> @@ -1682,7 +1650,7 @@ MODULE_DEVICE_TABLE(spi, at86rf230_device_id);
>  static struct spi_driver at86rf230_driver = {
>  	.id_table = at86rf230_device_id,
>  	.driver = {
> -		.of_match_table = of_match_ptr(at86rf230_of_match),
> +		.of_match_table = at86rf230_of_match,
>  		.name	= "at86rf230",
>  	},
>  	.probe      = at86rf230_probe,
> diff --git a/include/linux/spi/at86rf230.h b/include/linux/spi/at86rf230.h
> deleted file mode 100644
> index d278576ab692..000000000000
> --- a/include/linux/spi/at86rf230.h
> +++ /dev/null
> @@ -1,20 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -/*
> - * AT86RF230/RF231 driver
> - *
> - * Copyright (C) 2009-2012 Siemens AG
> - *
> - * Written by:
> - * Dmitry Eremin-Solenikov <dmitry.baryshkov@siemens.com>
> - */
> -#ifndef AT86RF230_H
> -#define AT86RF230_H
> -
> -struct at86rf230_platform_data {
> -	int rstn;
> -	int slp_tr;
> -	int dig2;
> -	u8 xtal_trim;
> -};
> -
> -#endif
> -- 
> 2.39.0
>

Andy Shevchenko Jan. 26, 2023, 2:24 p.m. UTC | #3

On Thu, Jan 26, 2023 at 03:12:43PM +0100, Miquel Raynal wrote:
> arnd@kernel.org wrote on Thu, 26 Jan 2023 14:51:23 +0100:

...

> > There are no remaining in-tree users of the platform_data,
> > so this driver can be converted to using the simpler gpiod
> > interfaces.
> > 
> > Any out-of-tree users that rely on the platform data can
> > provide the data using the device_property and gpio_lookup
> > interfaces instead.

[...]

> > @@ -1682,7 +1650,7 @@ MODULE_DEVICE_TABLE(spi, at86rf230_device_id);
> >  static struct spi_driver at86rf230_driver = {
> >  	.id_table = at86rf230_device_id,
> >  	.driver = {
> > -		.of_match_table = of_match_ptr(at86rf230_of_match),
> > +		.of_match_table = at86rf230_of_match,linux-gnueabihf embed a C library which relies on kernel headers (for example, to provide an open API which translates to an open syscall), for exam
> 
> Looks like an unrelated change? Or is it a consequence of "not having
> any in-tree users of platform_data" that plays a role here?

This enables this driver to work on ACPI-based platforms without needed the
legacy platform data. Dunno if it will be ever the case, but still...

> Anyhow, the changes in the driver look good, so:
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thank you!

Arnd Bergmann Jan. 26, 2023, 2:39 p.m. UTC | #4

On Thu, Jan 26, 2023, at 15:12, Miquel Raynal wrote:

>> @@ -1682,7 +1650,7 @@ MODULE_DEVICE_TABLE(spi, at86rf230_device_id);
>>  static struct spi_driver at86rf230_driver = {
>>  	.id_table = at86rf230_device_id,
>>  	.driver = {
>> -		.of_match_table = of_match_ptr(at86rf230_of_match),
>> +		.of_match_table = at86rf230_of_match,
>
> Looks like an unrelated change? Or is it a consequence of "not having
> any in-tree users of platform_data" that plays a role here?

I probably did it because I thought I had removed the matching #ifdef
for at86rf230_of_match in the process of making the driver DT-only.

Without this trivial change, building the driver as built-in with
CONFIG_OF=n can result in a warning like

drivers/net/ieee802154/at86rf230.c:1632:28: error: unused variable 'at86rf230_of_match' [-Werror,-Wunused-variable]

It looks like this was already removed in a8b66db804f0 ("at86rf230:
remove #ifdef CONFIG_OF"), which was not technically correct, but
nobody noticed, including me.

I could split this out as a separate patch, but it's probably
not worth it.

> Anyhow, the changes in the driver look good, so:
>
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,

    Arnd

at86rf230: convert to gpio descriptors

Checks

Commit Message

Comments

Patch