All of lore.kernel.org
 help / color / mirror / Atom feed
From: nsekhar@ti.com (Sekhar Nori)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: davinci: da8xx_register_spi() should not register SPI board info
Date: Wed, 28 Nov 2012 14:19:02 +0530	[thread overview]
Message-ID: <50B5CFFE.7010701@ti.com> (raw)
In-Reply-To: <1347323353-19764-1-git-send-email-vivien.didelot@savoirfairelinux.com>

Hi Vivien,

On 9/11/2012 5:59 AM, Vivien Didelot wrote:
> Without this patch, da8xx_register_spi() registers the SPI board info,
> the SPI controller, and sets its number of chipselect to the size of the
> static spi_board_info array. This is bad because a SPI board info may
> declare devices for different SPI buses, and because other code can also
> call spi_register_board_info() (e.g. a daughter board might provide
> additional SPI devices).
> 
> This patch removes the spi_register_board_info() call from
> da8xx_register_spi(), renames this last one to da8xx_register_spi_bus()
> to be more explicit, takes the number of chipselect as a function
> parameter, and updates the impacted board-da8{3,5}0-evm.c, and
> board-mityomapl138.c files accordingly. It also sets the SPI platform
> data static, as it doesn't need to be exported.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Sorry about the late reply. It seems I missed seeing this completely
until you reminded Kevin about it.

The patch looks good to me except some minor issues. I made those
changes myself locally since I am the one guilty of getting back on this
so late. See below:

> ---
>  arch/arm/mach-davinci/board-da830-evm.c    |  9 +++++++--
>  arch/arm/mach-davinci/board-da850-evm.c    |  9 +++++++--
>  arch/arm/mach-davinci/board-mityomapl138.c |  9 +++++++--
>  arch/arm/mach-davinci/devices-da8xx.c      | 14 +++-----------
>  arch/arm/mach-davinci/include/mach/da8xx.h |  3 +--
>  5 files changed, 25 insertions(+), 19 deletions(-)

The patch doesn't apply to the latest kernel anymore. I resolved that.

> 
> diff --git a/arch/arm/mach-davinci/board-da830-evm.c b/arch/arm/mach-davinci/board-da830-evm.c
> index 11c3db9..adb8d87 100644
> --- a/arch/arm/mach-davinci/board-da830-evm.c
> +++ b/arch/arm/mach-davinci/board-da830-evm.c
> @@ -652,8 +652,13 @@ static __init void da830_evm_init(void)
>  	if (ret)
>  		pr_warning("da830_evm_init: rtc setup failed: %d\n", ret);
>  
> -	ret = da8xx_register_spi(0, da830evm_spi_info,
> -				 ARRAY_SIZE(da830evm_spi_info));
> +	ret = spi_register_board_info(da830evm_spi_info,
> +				      ARRAY_SIZE(da830evm_spi_info));
> +	if (ret)
> +		pr_warning("da830_evm_init: spi info registration failed: %d\n",
> +			   ret);

Checkpatch asks us to use pr_warn() instead. Fixed that. Also, used
__func__ to print function name. Rob Tivy has patches fixing these all
through this file.

Thanks,
Sekhar

WARNING: multiple messages have this Message-ID (diff)
From: Sekhar Nori <nsekhar@ti.com>
To: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Cc: <linux-arm-kernel@lists.infradead.org>,
	Russell King <linux@arm.linux.org.uk>,
	Kevin Hilman <khilman@ti.com>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ARM: davinci: da8xx_register_spi() should not register SPI board info
Date: Wed, 28 Nov 2012 14:19:02 +0530	[thread overview]
Message-ID: <50B5CFFE.7010701@ti.com> (raw)
In-Reply-To: <1347323353-19764-1-git-send-email-vivien.didelot@savoirfairelinux.com>

Hi Vivien,

On 9/11/2012 5:59 AM, Vivien Didelot wrote:
> Without this patch, da8xx_register_spi() registers the SPI board info,
> the SPI controller, and sets its number of chipselect to the size of the
> static spi_board_info array. This is bad because a SPI board info may
> declare devices for different SPI buses, and because other code can also
> call spi_register_board_info() (e.g. a daughter board might provide
> additional SPI devices).
> 
> This patch removes the spi_register_board_info() call from
> da8xx_register_spi(), renames this last one to da8xx_register_spi_bus()
> to be more explicit, takes the number of chipselect as a function
> parameter, and updates the impacted board-da8{3,5}0-evm.c, and
> board-mityomapl138.c files accordingly. It also sets the SPI platform
> data static, as it doesn't need to be exported.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Sorry about the late reply. It seems I missed seeing this completely
until you reminded Kevin about it.

The patch looks good to me except some minor issues. I made those
changes myself locally since I am the one guilty of getting back on this
so late. See below:

> ---
>  arch/arm/mach-davinci/board-da830-evm.c    |  9 +++++++--
>  arch/arm/mach-davinci/board-da850-evm.c    |  9 +++++++--
>  arch/arm/mach-davinci/board-mityomapl138.c |  9 +++++++--
>  arch/arm/mach-davinci/devices-da8xx.c      | 14 +++-----------
>  arch/arm/mach-davinci/include/mach/da8xx.h |  3 +--
>  5 files changed, 25 insertions(+), 19 deletions(-)

The patch doesn't apply to the latest kernel anymore. I resolved that.

> 
> diff --git a/arch/arm/mach-davinci/board-da830-evm.c b/arch/arm/mach-davinci/board-da830-evm.c
> index 11c3db9..adb8d87 100644
> --- a/arch/arm/mach-davinci/board-da830-evm.c
> +++ b/arch/arm/mach-davinci/board-da830-evm.c
> @@ -652,8 +652,13 @@ static __init void da830_evm_init(void)
>  	if (ret)
>  		pr_warning("da830_evm_init: rtc setup failed: %d\n", ret);
>  
> -	ret = da8xx_register_spi(0, da830evm_spi_info,
> -				 ARRAY_SIZE(da830evm_spi_info));
> +	ret = spi_register_board_info(da830evm_spi_info,
> +				      ARRAY_SIZE(da830evm_spi_info));
> +	if (ret)
> +		pr_warning("da830_evm_init: spi info registration failed: %d\n",
> +			   ret);

Checkpatch asks us to use pr_warn() instead. Fixed that. Also, used
__func__ to print function name. Rob Tivy has patches fixing these all
through this file.

Thanks,
Sekhar

  reply	other threads:[~2012-11-28  8:49 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-11  0:29 [PATCH] ARM: davinci: da8xx_register_spi() should not register SPI board info Vivien Didelot
2012-09-11  0:29 ` Vivien Didelot
2012-11-28  8:49 ` Sekhar Nori [this message]
2012-11-28  8:49   ` Sekhar Nori
2012-11-28 17:10   ` Vivien Didelot
2012-11-28 17:10     ` Vivien Didelot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50B5CFFE.7010701@ti.com \
    --to=nsekhar@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.