linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 2/3] OMAP4: Ethernet: KS8851 Board Support
@ 2010-05-05  7:10 Arce, Abraham
       [not found] ` <27F9C60D11D683428E133F85D2BB4A53043DEEB8C1-lTKHBJngVwKIQmiDNMet8wC/G2K4zDHf@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Arce, Abraham @ 2010-05-05  7:10 UTC (permalink / raw)
  To: linux-omap, spi-devel-general


Enable Micrel KS8851 SPI network chip for OMAP4430

Signed-off-by: Abraham Arce <x0066660@ti.com>
---
 arch/arm/mach-omap2/board-4430sdp.c |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c
index b88f28c..801c2a7 100644
--- a/arch/arm/mach-omap2/board-4430sdp.c
+++ b/arch/arm/mach-omap2/board-4430sdp.c
@@ -18,6 +18,7 @@
 #include <linux/io.h>
 #include <linux/gpio.h>
 #include <linux/usb/otg.h>
+#include <linux/spi/spi.h>
 
 #include <mach/hardware.h>
 #include <asm/mach-types.h>
@@ -32,6 +33,30 @@
 #include <asm/hardware/gic.h>
 #include <asm/hardware/cache-l2x0.h>
 
+#define ETHERNET_KS8851_IRQ		34
+#define ETHERNET_KS8851_POWER_ENABLE	48
+#define ETHERNET_KS8851_QUART		138
+
+static struct spi_board_info sdp4430_spi_board_info[] __initdata = {
+	{
+		.modalias               = "ks8851",
+		.bus_num                = 1,
+		.chip_select            = 0,
+		.max_speed_hz           = 24000000,
+		.irq                    = ETHERNET_KS8851_IRQ,
+	},
+};
+
+static void omap_ethernet_init(void)
+{
+	gpio_request(ETHERNET_KS8851_POWER_ENABLE, "ethernet");
+	gpio_direction_output(ETHERNET_KS8851_POWER_ENABLE, 1);
+	gpio_request(ETHERNET_KS8851_QUART, "quart");
+	gpio_direction_output(ETHERNET_KS8851_QUART, 1);
+	gpio_request(ETHERNET_KS8851_IRQ, "ks8851");
+	gpio_direction_input(ETHERNET_KS8851_IRQ);
+}
+
 static struct platform_device sdp4430_lcd_device = {
 	.name		= "sdp4430_lcd",
 	.id		= -1,
@@ -120,6 +145,11 @@ static void __init omap_4430sdp_init(void)
 	/* FIXME: allow multi-omap to boot until musb is updated for omap4 */
 	if (!cpu_is_omap44xx())
 		usb_musb_init(&musb_board_data);
+
+	omap_ethernet_init();
+	sdp4430_spi_board_info[0].irq = gpio_to_irq(ETHERNET_KS8851_IRQ);
+	spi_register_board_info(sdp4430_spi_board_info,
+			ARRAY_SIZE(sdp4430_spi_board_info));
 }
 
 static void __init omap_4430sdp_map_io(void)
-- 
1.5.4.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 2/3] OMAP4: Ethernet: KS8851 Board Support
       [not found] ` <27F9C60D11D683428E133F85D2BB4A53043DEEB8C1-lTKHBJngVwKIQmiDNMet8wC/G2K4zDHf@public.gmane.org>
@ 2010-05-05  7:34   ` G, Manjunath Kondaiah
       [not found]     ` <E0D41E29EB0DAC4E9F3FF173962E9E94026F8C7096-/tLxBxkBPtCIQmiDNMet8wC/G2K4zDHf@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: G, Manjunath Kondaiah @ 2010-05-05  7:34 UTC (permalink / raw)
  To: Arce, Abraham, linux-omap-u79uwXL29TY76Z2rM5mHXA,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f



> -----Original Message-----
> From: linux-omap-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org 
> [mailto:linux-omap-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Arce, Abraham
> Sent: Wednesday, May 05, 2010 12:40 PM
> To: linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; 
> spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> Subject: [PATCH v1 2/3] OMAP4: Ethernet: KS8851 Board Support
> 
> 
> Enable Micrel KS8851 SPI network chip for OMAP4430
> 
> Signed-off-by: Abraham Arce <x0066660-l0cyMroinI0@public.gmane.org>
> ---
>  arch/arm/mach-omap2/board-4430sdp.c |   30 
> ++++++++++++++++++++++++++++++
>  1 files changed, 30 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/board-4430sdp.c 
> b/arch/arm/mach-omap2/board-4430sdp.c
> index b88f28c..801c2a7 100644
> --- a/arch/arm/mach-omap2/board-4430sdp.c
> +++ b/arch/arm/mach-omap2/board-4430sdp.c
> @@ -18,6 +18,7 @@
>  #include <linux/io.h>
>  #include <linux/gpio.h>
>  #include <linux/usb/otg.h>
> +#include <linux/spi/spi.h>
>  
>  #include <mach/hardware.h>
>  #include <asm/mach-types.h>
> @@ -32,6 +33,30 @@
>  #include <asm/hardware/gic.h>
>  #include <asm/hardware/cache-l2x0.h>
>  
> +#define ETHERNET_KS8851_IRQ		34
> +#define ETHERNET_KS8851_POWER_ENABLE	48
> +#define ETHERNET_KS8851_QUART		138
> +
> +static struct spi_board_info sdp4430_spi_board_info[] __initdata = {
> +	{
> +		.modalias               = "ks8851",
> +		.bus_num                = 1,
> +		.chip_select            = 0,
> +		.max_speed_hz           = 24000000,
> +		.irq                    = ETHERNET_KS8851_IRQ,
> +	},
> +};
> +
> +static void omap_ethernet_init(void)
> +{
> +	gpio_request(ETHERNET_KS8851_POWER_ENABLE, "ethernet");
> +	gpio_direction_output(ETHERNET_KS8851_POWER_ENABLE, 1);
> +	gpio_request(ETHERNET_KS8851_QUART, "quart");
> +	gpio_direction_output(ETHERNET_KS8851_QUART, 1);
> +	gpio_request(ETHERNET_KS8851_IRQ, "ks8851");

Any reason for not checking return value of gpio_request()?

-Manjunath
------------------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 2/3] OMAP4: Ethernet: KS8851 Board Support
       [not found]     ` <E0D41E29EB0DAC4E9F3FF173962E9E94026F8C7096-/tLxBxkBPtCIQmiDNMet8wC/G2K4zDHf@public.gmane.org>
@ 2010-05-05  8:11       ` Arce, Abraham
  2010-05-05 17:19         ` Tony Lindgren
  0 siblings, 1 reply; 10+ messages in thread
From: Arce, Abraham @ 2010-05-05  8:11 UTC (permalink / raw)
  To: G, Manjunath Kondaiah, linux-omap-u79uwXL29TY76Z2rM5mHXA,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

Manjunath,

> > +
> > +static void omap_ethernet_init(void)
> > +{
> > +	gpio_request(ETHERNET_KS8851_POWER_ENABLE, "ethernet");
> > +	gpio_direction_output(ETHERNET_KS8851_POWER_ENABLE, 1);
> > +	gpio_request(ETHERNET_KS8851_QUART, "quart");
> > +	gpio_direction_output(ETHERNET_KS8851_QUART, 1);
> > +	gpio_request(ETHERNET_KS8851_IRQ, "ks8851");
> 
> Any reason for not checking return value of gpio_request()?
> 

Thought initially about them as dedicated lines for ethernet avoiding
both reasons to check for a gpio's request:

 1. invalid gpio
 2. already claimed gpio

Best Regards
Abraham


------------------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 2/3] OMAP4: Ethernet: KS8851 Board Support
  2010-05-05  8:11       ` Arce, Abraham
@ 2010-05-05 17:19         ` Tony Lindgren
  2010-05-06  0:47           ` Arce, Abraham
  0 siblings, 1 reply; 10+ messages in thread
From: Tony Lindgren @ 2010-05-05 17:19 UTC (permalink / raw)
  To: Arce, Abraham; +Cc: G, Manjunath Kondaiah, linux-omap, spi-devel-general

* Arce, Abraham <x0066660@ti.com> [100505 01:06]:
> Manjunath,
> 
> > > +
> > > +static void omap_ethernet_init(void)
> > > +{
> > > +	gpio_request(ETHERNET_KS8851_POWER_ENABLE, "ethernet");
> > > +	gpio_direction_output(ETHERNET_KS8851_POWER_ENABLE, 1);
> > > +	gpio_request(ETHERNET_KS8851_QUART, "quart");
> > > +	gpio_direction_output(ETHERNET_KS8851_QUART, 1);
> > > +	gpio_request(ETHERNET_KS8851_IRQ, "ks8851");
> > 
> > Any reason for not checking return value of gpio_request()?
> > 
> 
> Thought initially about them as dedicated lines for ethernet avoiding
> both reasons to check for a gpio's request:
> 
>  1. invalid gpio
>  2. already claimed gpio

You still need to check for the result.

Tony

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH v1 2/3] OMAP4: Ethernet: KS8851 Board Support
  2010-05-05 17:19         ` Tony Lindgren
@ 2010-05-06  0:47           ` Arce, Abraham
  2010-05-06  3:14             ` G, Manjunath Kondaiah
  0 siblings, 1 reply; 10+ messages in thread
From: Arce, Abraham @ 2010-05-06  0:47 UTC (permalink / raw)
  To: Tony Lindgren; +Cc: G, Manjunath Kondaiah, linux-omap, spi-devel-general

> > > > +
> > > > +static void omap_ethernet_init(void)
> > > > +{
> > > > +	gpio_request(ETHERNET_KS8851_POWER_ENABLE, "ethernet");
> > > > +	gpio_direction_output(ETHERNET_KS8851_POWER_ENABLE, 1);
> > > > +	gpio_request(ETHERNET_KS8851_QUART, "quart");
> > > > +	gpio_direction_output(ETHERNET_KS8851_QUART, 1);
> > > > +	gpio_request(ETHERNET_KS8851_IRQ, "ks8851");
> > >
> > > Any reason for not checking return value of gpio_request()?
> > >
> >
> > Thought initially about them as dedicated lines for ethernet avoiding
> > both reasons to check for a gpio's request:
> >
> >  1. invalid gpio
> >  2. already claimed gpio
> 
> You still need to check for the result.
> 

Is the below approach ok? Using goto would make it better?

        int status;

        status = gpio_request(ETHERNET_KS8851_POWER_ENABLE, "ethernet");
        if (status < 0)
                return status;

        gpio_request(ETHERNET_KS8851_QUART, "quart");
        if (status < 0) {
                gpio_free(ETHERNET_KS8851_POWER_ENABLE);
                return status;
        }

        gpio_request(ETHERNET_KS8851_IRQ, "ks8851");
        if (status < 0) {
                gpio_free(ETHERNET_KS8851_POWER_ENABLE);
                gpio_free(ETHERNET_KS8851_QUART);
                return status;
        }

        gpio_direction_output(ETHERNET_KS8851_POWER_ENABLE, 1);
        gpio_direction_output(ETHERNET_KS8851_QUART, 1);
        gpio_direction_input(ETHERNET_KS8851_IRQ);

Best Regards
Abraham

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH v1 2/3] OMAP4: Ethernet: KS8851 Board Support
  2010-05-06  0:47           ` Arce, Abraham
@ 2010-05-06  3:14             ` G, Manjunath Kondaiah
  2010-05-06  3:41               ` G, Manjunath Kondaiah
  0 siblings, 1 reply; 10+ messages in thread
From: G, Manjunath Kondaiah @ 2010-05-06  3:14 UTC (permalink / raw)
  To: Arce, Abraham, Tony Lindgren; +Cc: linux-omap, spi-devel-general



> -----Original Message-----
> From: Arce, Abraham 
> Sent: Thursday, May 06, 2010 6:18 AM
> To: Tony Lindgren
> Cc: G, Manjunath Kondaiah; linux-omap@vger.kernel.org; 
> spi-devel-general@lists.sourceforge.net
> Subject: RE: [PATCH v1 2/3] OMAP4: Ethernet: KS8851 Board Support
> 
> > > > > +
> > > > > +static void omap_ethernet_init(void)
> > > > > +{
> > > > > +	gpio_request(ETHERNET_KS8851_POWER_ENABLE, "ethernet");
> > > > > +	gpio_direction_output(ETHERNET_KS8851_POWER_ENABLE, 1);
> > > > > +	gpio_request(ETHERNET_KS8851_QUART, "quart");
> > > > > +	gpio_direction_output(ETHERNET_KS8851_QUART, 1);
> > > > > +	gpio_request(ETHERNET_KS8851_IRQ, "ks8851");
> > > >
> > > > Any reason for not checking return value of gpio_request()?
> > > >
> > >
> > > Thought initially about them as dedicated lines for 
> ethernet avoiding
> > > both reasons to check for a gpio's request:
> > >
> > >  1. invalid gpio
> > >  2. already claimed gpio
> > 
> > You still need to check for the result.
> > 
> 
> Is the below approach ok? Using goto would make it better?
> 
>         int status;
> 
>         status = gpio_request(ETHERNET_KS8851_POWER_ENABLE, 
> "ethernet");
>         if (status < 0)

You need to have warning message about this failure.

>                 return status;
> 
>         gpio_request(ETHERNET_KS8851_QUART, "quart");
>         if (status < 0) {

-Ditto-

>                 gpio_free(ETHERNET_KS8851_POWER_ENABLE);
>                 return status;

Return to where? This function seems to be void, change signature
of this API.

>         }
> 
>         gpio_request(ETHERNET_KS8851_IRQ, "ks8851");
>         if (status < 0) {

Bug! Checking previous status.

>                 gpio_free(ETHERNET_KS8851_POWER_ENABLE);
>                 gpio_free(ETHERNET_KS8851_QUART);
>                 return status;
>         }
> 
>         gpio_direction_output(ETHERNET_KS8851_POWER_ENABLE, 1);
>         gpio_direction_output(ETHERNET_KS8851_QUART, 1);
>         gpio_direction_input(ETHERNET_KS8851_IRQ);

Goto will be better approach  compared to above logic.

if(gpio_request(x)
	goto err1;

if(gpio_request(y)
	goto err2;

if(gpio_request(z)
	goto err3;
...	
	return 0;

err3:
	free(x);
err2:
	free(y);
err1:
	free(z);

	return status;

-Manjunath 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH v1 2/3] OMAP4: Ethernet: KS8851 Board Support
  2010-05-06  3:14             ` G, Manjunath Kondaiah
@ 2010-05-06  3:41               ` G, Manjunath Kondaiah
       [not found]                 ` <E0D41E29EB0DAC4E9F3FF173962E9E94026F8C74DE-/tLxBxkBPtCIQmiDNMet8wC/G2K4zDHf@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: G, Manjunath Kondaiah @ 2010-05-06  3:41 UTC (permalink / raw)
  To: Arce, Abraham, Tony Lindgren; +Cc: linux-omap, spi-devel-general

 

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org 
> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of G, 
> Manjunath Kondaiah
> Sent: Thursday, May 06, 2010 8:45 AM
> To: Arce, Abraham; Tony Lindgren
> Cc: linux-omap@vger.kernel.org; 
> spi-devel-general@lists.sourceforge.net
> Subject: RE: [PATCH v1 2/3] OMAP4: Ethernet: KS8851 Board Support
> 
> 
> 
> > -----Original Message-----
> > From: Arce, Abraham 
> > Sent: Thursday, May 06, 2010 6:18 AM
> > To: Tony Lindgren
> > Cc: G, Manjunath Kondaiah; linux-omap@vger.kernel.org; 
> > spi-devel-general@lists.sourceforge.net
> > Subject: RE: [PATCH v1 2/3] OMAP4: Ethernet: KS8851 Board Support
> > 
> > > > > > +
> > > > > > +static void omap_ethernet_init(void)
> > > > > > +{
> > > > > > +	gpio_request(ETHERNET_KS8851_POWER_ENABLE, "ethernet");
> > > > > > +	gpio_direction_output(ETHERNET_KS8851_POWER_ENABLE, 1);
> > > > > > +	gpio_request(ETHERNET_KS8851_QUART, "quart");
> > > > > > +	gpio_direction_output(ETHERNET_KS8851_QUART, 1);
> > > > > > +	gpio_request(ETHERNET_KS8851_IRQ, "ks8851");
> > > > >
> > > > > Any reason for not checking return value of gpio_request()?
> > > > >
> > > >
> > > > Thought initially about them as dedicated lines for 
> > ethernet avoiding
> > > > both reasons to check for a gpio's request:
> > > >
> > > >  1. invalid gpio
> > > >  2. already claimed gpio
> > > 
> > > You still need to check for the result.
> > > 
> > 
> > Is the below approach ok? Using goto would make it better?
> > 
> >         int status;
> > 
> >         status = gpio_request(ETHERNET_KS8851_POWER_ENABLE, 
> > "ethernet");
> >         if (status < 0)
> 
> You need to have warning message about this failure.
> 
> >                 return status;
> > 
> >         gpio_request(ETHERNET_KS8851_QUART, "quart");
> >         if (status < 0) {
> 
> -Ditto-
> 
> >                 gpio_free(ETHERNET_KS8851_POWER_ENABLE);
> >                 return status;
> 
> Return to where? This function seems to be void, change signature
> of this API.
> 
> >         }
> > 
> >         gpio_request(ETHERNET_KS8851_IRQ, "ks8851");
> >         if (status < 0) {
> 
> Bug! Checking previous status.
> 
> >                 gpio_free(ETHERNET_KS8851_POWER_ENABLE);
> >                 gpio_free(ETHERNET_KS8851_QUART);
> >                 return status;
> >         }
> > 
> >         gpio_direction_output(ETHERNET_KS8851_POWER_ENABLE, 1);
> >         gpio_direction_output(ETHERNET_KS8851_QUART, 1);
> >         gpio_direction_input(ETHERNET_KS8851_IRQ);
> 
> Goto will be better approach  compared to above logic.
> 
> if(gpio_request(x)
> 	goto err1;
> 
> if(gpio_request(y)
> 	goto err2;
> 
> if(gpio_request(z)
> 	goto err3;
> ...	
> 	return 0;
> 
> err3:
> 	free(x);
> err2:
> 	free(y);
> err1:
> 	free(z);
> 
> 	return status;

Minor changes:
if (gpio_request(x))
	return status;
if (gpio_request(y))
	goto err1;
if (gpio_request(z))
	goto err2;
...
	return 0;
	
err2:
	free(y);

err1:
	free(x);
	return status;

-Manjunath


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 2/3] OMAP4: Ethernet: KS8851 Board Support
       [not found]                 ` <E0D41E29EB0DAC4E9F3FF173962E9E94026F8C74DE-/tLxBxkBPtCIQmiDNMet8wC/G2K4zDHf@public.gmane.org>
@ 2010-05-06  4:18                   ` Arce, Abraham
  2010-05-06  9:03                     ` G, Manjunath Kondaiah
  0 siblings, 1 reply; 10+ messages in thread
From: Arce, Abraham @ 2010-05-06  4:18 UTC (permalink / raw)
  To: G, Manjunath Kondaiah, Tony Lindgren
  Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-omap-u79uwXL29TY76Z2rM5mHXA

Manjunath,

> > > > > > > +
> > > > > > > +static void omap_ethernet_init(void)
> > > > > > > +{
> > > > > > > +	gpio_request(ETHERNET_KS8851_POWER_ENABLE, "ethernet");
> > > > > > > +	gpio_direction_output(ETHERNET_KS8851_POWER_ENABLE, 1);
> > > > > > > +	gpio_request(ETHERNET_KS8851_QUART, "quart");
> > > > > > > +	gpio_direction_output(ETHERNET_KS8851_QUART, 1);
> > > > > > > +	gpio_request(ETHERNET_KS8851_IRQ, "ks8851");
> > > > > >
> > > > > > Any reason for not checking return value of gpio_request()?
> > > > > >

[snip]

> Minor changes:
> if (gpio_request(x))
> 	return status;
> if (gpio_request(y))
> 	goto err1;
> if (gpio_request(z))
> 	goto err2;
> ...
> 	return 0;
> 
> err2:
> 	free(y);
> 
> err1:
> 	free(x);
> 	return status;

How about this patch?

Note. I am thinking in spi_register_board_info registration:

1. to be done if ethernet fails, other spi dev can be added in future
2. not to be done if ethernet fails, no other dev in spi bus 1 for now

What is the best approach? Is it to keep #2?

---

#define ETH_KS8851_IRQ			34
#define ETH_KS8851_POWER_ON		48
#define ETH_KS8851_QUART		138

static struct spi_board_info sdp4430_spi_board_info[] __initdata = {
	{
		.modalias               = "ks8851",
		.bus_num                = 1,
		.chip_select            = 0,
		.max_speed_hz           = 24000000,
		.irq                    = ETH_KS8851_IRQ,
	},
};

static int omap_ethernet_init(void)
{
	int status;

	status = gpio_request(ETH_KS8851_POWER_ON, "eth_power");
	if (status < 0) {
		pr_warning("Cannot request GPIO %d\n", ETH_KS8851_POWER_ON);
		return status;
	}

	status = gpio_request(ETH_KS8851_QUART, "quart");
	if (status < 0) {
		pr_warning("Cannot request GPIO %d\n", ETH_KS8851_QUART);
		goto err1;
	}

	status = gpio_request(ETH_KS8851_IRQ, "eth_irq");
	if (status < 0) {
		pr_warning("Cannot request GPIO %d\n", ETH_KS8851_IRQ);
		goto err2;
	}

	gpio_direction_output(ETH_KS8851_POWER_ON, 1);
	gpio_direction_output(ETH_KS8851_QUART, 1);
	gpio_direction_input(ETH_KS8851_IRQ);

	return status;

err2:
	gpio_free(ETH_KS8851_QUART);
err1:
	gpio_free(ETH_KS8851_POWER_ON);
	return status;

}


[...]


 	/* FIXME: allow multi-omap to boot until musb is updated for omap4 */
 	if (!cpu_is_omap44xx())
 		usb_musb_init(&musb_board_data);

	status = omap_ethernet_init();
	if (status)
		pr_warning("Ethernet initialization failed: %d\n", status);
	else
		sdp4430_spi_board_info[0].irq = gpio_to_irq(ETH_KS8851_IRQ);

	spi_register_board_info(sdp4430_spi_board_info,
			ARRAY_SIZE(sdp4430_spi_board_info));


Best Regards
Abraham

------------------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH v1 2/3] OMAP4: Ethernet: KS8851 Board Support
  2010-05-06  4:18                   ` Arce, Abraham
@ 2010-05-06  9:03                     ` G, Manjunath Kondaiah
  2010-05-06  9:43                       ` Shilimkar, Santosh
  0 siblings, 1 reply; 10+ messages in thread
From: G, Manjunath Kondaiah @ 2010-05-06  9:03 UTC (permalink / raw)
  To: Arce, Abraham, Tony Lindgren; +Cc: linux-omap, spi-devel-general



> -----Original Message-----
> From: Arce, Abraham 
> Sent: Thursday, May 06, 2010 9:49 AM
> To: G, Manjunath Kondaiah; Tony Lindgren
> Cc: linux-omap@vger.kernel.org; 
> spi-devel-general@lists.sourceforge.net
> Subject: RE: [PATCH v1 2/3] OMAP4: Ethernet: KS8851 Board Support
> 
> Manjunath,
> 
> > > > > > > > +
> > > > > > > > +static void omap_ethernet_init(void)
> > > > > > > > +{
> > > > > > > > +	
> gpio_request(ETHERNET_KS8851_POWER_ENABLE, "ethernet");
> > > > > > > > +	
> gpio_direction_output(ETHERNET_KS8851_POWER_ENABLE, 1);
> > > > > > > > +	gpio_request(ETHERNET_KS8851_QUART, "quart");
> > > > > > > > +	gpio_direction_output(ETHERNET_KS8851_QUART, 1);
> > > > > > > > +	gpio_request(ETHERNET_KS8851_IRQ, "ks8851");
> > > > > > >
> > > > > > > Any reason for not checking return value of 
> gpio_request()?
> > > > > > >
> 
> [snip]
> 
> > Minor changes:
> > if (gpio_request(x))
> > 	return status;
> > if (gpio_request(y))
> > 	goto err1;
> > if (gpio_request(z))
> > 	goto err2;
> > ...
> > 	return 0;
> > 
> > err2:
> > 	free(y);
> > 
> > err1:
> > 	free(x);
> > 	return status;
> 
> How about this patch?
> 
> Note. I am thinking in spi_register_board_info registration:
> 
> 1. to be done if ethernet fails, other spi dev can be added in future

No use. Since, for a given board SPI chip select lines are physically 
connected to external slave devices, you can't register another SPI 
slave device for same chip select.

> 2. not to be done if ethernet fails, no other dev in spi bus 1 for now
> 
> What is the best approach? Is it to keep #2?

Seems to be ok for chip select #0. If we have another device on bus 1 with
different chip select number, then registration should succeed. However, this
may not be the case with omap4 sdp board since it might have only one spi
device on bus1.

-Manjunath

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH v1 2/3] OMAP4: Ethernet: KS8851 Board Support
  2010-05-06  9:03                     ` G, Manjunath Kondaiah
@ 2010-05-06  9:43                       ` Shilimkar, Santosh
  0 siblings, 0 replies; 10+ messages in thread
From: Shilimkar, Santosh @ 2010-05-06  9:43 UTC (permalink / raw)
  To: G, Manjunath Kondaiah, Arce, Abraham, Tony Lindgren
  Cc: linux-omap, spi-devel-general

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of G,
> Manjunath Kondaiah
> Sent: Thursday, May 06, 2010 2:34 PM
> To: Arce, Abraham; Tony Lindgren
> Cc: linux-omap@vger.kernel.org; spi-devel-general@lists.sourceforge.net
> Subject: RE: [PATCH v1 2/3] OMAP4: Ethernet: KS8851 Board Support
> 
> 
> 
> > -----Original Message-----
> > From: Arce, Abraham
> > Sent: Thursday, May 06, 2010 9:49 AM
> > To: G, Manjunath Kondaiah; Tony Lindgren
> > Cc: linux-omap@vger.kernel.org;
> > spi-devel-general@lists.sourceforge.net
> > Subject: RE: [PATCH v1 2/3] OMAP4: Ethernet: KS8851 Board Support
> >
> > Manjunath,
> >
> > > > > > > > > +
> > > > > > > > > +static void omap_ethernet_init(void)
> > > > > > > > > +{
> > > > > > > > > +
> > gpio_request(ETHERNET_KS8851_POWER_ENABLE, "ethernet");
> > > > > > > > > +
> > gpio_direction_output(ETHERNET_KS8851_POWER_ENABLE, 1);
> > > > > > > > > +	gpio_request(ETHERNET_KS8851_QUART, "quart");
> > > > > > > > > +	gpio_direction_output(ETHERNET_KS8851_QUART, 1);
> > > > > > > > > +	gpio_request(ETHERNET_KS8851_IRQ, "ks8851");
> > > > > > > >
> > > > > > > > Any reason for not checking return value of
> > gpio_request()?
> > > > > > > >
> >
> > [snip]
> >
> > > Minor changes:
> > > if (gpio_request(x))
> > > 	return status;
> > > if (gpio_request(y))
> > > 	goto err1;
> > > if (gpio_request(z))
> > > 	goto err2;
> > > ...
> > > 	return 0;
> > >
> > > err2:
> > > 	free(y);
> > >
> > > err1:
> > > 	free(x);
> > > 	return status;
> >
> > How about this patch?
> >
> > Note. I am thinking in spi_register_board_info registration:
> >
> > 1. to be done if ethernet fails, other spi dev can be added in future
> 
> No use. Since, for a given board SPI chip select lines are physically
> connected to external slave devices, you can't register another SPI
> slave device for same chip select.
Be careful here Manju. There are 4 channels per SPI controller
with 4 chip selects. You can add more devices on same controller

> 
> > 2. not to be done if ethernet fails, no other dev in spi bus 1 for now
> >
> > What is the best approach? Is it to keep #2?
> 
> Seems to be ok for chip select #0. If we have another device on bus 1 with
> different chip select number, then registration should succeed. However, this
> may not be the case with omap4 sdp board since it might have only one spi
> device on bus1.
There is a quad-uart on SPI but not being used though.
> 
> -Manjunath--
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2010-05-06  9:43 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-05  7:10 [PATCH v1 2/3] OMAP4: Ethernet: KS8851 Board Support Arce, Abraham
     [not found] ` <27F9C60D11D683428E133F85D2BB4A53043DEEB8C1-lTKHBJngVwKIQmiDNMet8wC/G2K4zDHf@public.gmane.org>
2010-05-05  7:34   ` G, Manjunath Kondaiah
     [not found]     ` <E0D41E29EB0DAC4E9F3FF173962E9E94026F8C7096-/tLxBxkBPtCIQmiDNMet8wC/G2K4zDHf@public.gmane.org>
2010-05-05  8:11       ` Arce, Abraham
2010-05-05 17:19         ` Tony Lindgren
2010-05-06  0:47           ` Arce, Abraham
2010-05-06  3:14             ` G, Manjunath Kondaiah
2010-05-06  3:41               ` G, Manjunath Kondaiah
     [not found]                 ` <E0D41E29EB0DAC4E9F3FF173962E9E94026F8C74DE-/tLxBxkBPtCIQmiDNMet8wC/G2K4zDHf@public.gmane.org>
2010-05-06  4:18                   ` Arce, Abraham
2010-05-06  9:03                     ` G, Manjunath Kondaiah
2010-05-06  9:43                       ` Shilimkar, Santosh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).