* [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).