Re: [PATCH v1 5/6] Platform support for On-chip MSP ethernet devices.
From: Sergei Shtylyov
Date: Tue Jan 25 2011 - 07:31:18 EST
Hello.
On 25-01-2011 13:28, Anoop P.A wrote:
From: Anoop P A<anoop.pa@xxxxxxxxx>
Previous version of patch was line wrapped and had some style issues. correcting it.
Such remarks should folow the --- tear line.
Some of MSP family SoC's come with legacy 100Mbps mspeth while some comes with
newer Gigabit TSMAC.Following patch adds platform support for both types of MAC's.
If TSMAC is not selected assume platform having legacy mspeth.This patch will add
gpio_macros as well which is required for resetting phy
Signed-off-by: Anoop P A<anoop.pa@xxxxxxxxx>
[...]
diff --git a/arch/mips/include/asm/pmc-sierra/msp71xx/msp_gpio_macros.h b/arch/mips/include/asm/pmc-sierra/msp71xx/msp_gpio_macros.h
new file mode 100644
index 0000000..d83d78f
--- /dev/null
+++ b/arch/mips/include/asm/pmc-sierra/msp71xx/msp_gpio_macros.h
@@ -0,0 +1,343 @@
+/*
+ *
+ * Macros for external SMP-safe access to the PMC MSP71xx reference
+ * board GPIO pins
+ *
+ * Copyright 2010 PMC-Sierra, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
+ * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
+ * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#ifndef __MSP_GPIO_MACROS_H__
+#define __MSP_GPIO_MACROS_H__
+
+#include <msp_regops.h>
+#include <msp_regs.h>
+
+#ifdef CONFIG_PMC_MSP7120_GW
+#define MSP_NUM_GPIOS (20)
Useless parens.
+#else
+#define MSP_NUM_GPIOS (28)
Here too.
[...]
+/* -- Bit masks -- */
+
+/* This gives you the 'register relative offlet gpio' number */
Offlet?
+#define OFFSET_GPIO_NUMBER(gpio) (gpio - MSP_GPIO_OFFSET[gpio])
+
+/* These take the 'register relative offset gpio' number */
+#define BASIC_DATA_REG_MASK(ogpio) \
+ (1 << ogpio)
+#define BASIC_MODE_REG_VALUE(mode, ogpio) \
+ (mode << BASIC_MODE_REG_SHIFT(ogpio))
+#define BASIC_MODE_REG_MASK(ogpio) \
+ BASIC_MODE_REG_VALUE(0xf, ogpio)
+#define BASIC_MODE_REG_SHIFT(ogpio) \
+ (ogpio * 4)
Why break the lines if there's enough space for the definition on the
first line?
+/* Sets the specified pin to the specified value */
+static inline void msp_gpio_pin_set(msp_gpio_data_t data, unsigned int gpio)
+{
+ if (gpio>= MSP_NUM_GPIOS)
+ return;
+
+ if (gpio < 16) {
+ if (data == MSP_GPIO_TOGGLE)
+ toggle_reg32(MSP_GPIO_DATA_REGISTER[gpio],
+ BASIC_DATA_MASK(gpio));
+ else if (data == MSP_GPIO_HI)
+ set_reg32(MSP_GPIO_DATA_REGISTER[gpio],
+ BASIC_DATA_MASK(gpio));
+ else
+ clear_reg32(MSP_GPIO_DATA_REGISTER[gpio],
+ BASIC_DATA_MASK(gpio));
+ } else {
+ if (data == MSP_GPIO_TOGGLE) {
+ /* Special ugly case:
+ * We have to read the CLR bit.
+ * If set, we write the CLR bit.
+ * If not, we write the SET bit.
+ */
+ u32 tmpdata;
Empty line wouldn't hurt here.
+ custom_read_reg32(MSP_GPIO_DATA_REGISTER[gpio],
+ tmpdata);
+ if (tmpdata & EXTENDED_CLR(gpio))
+ tmpdata = EXTENDED_CLR(gpio);
+ else
+ tmpdata = EXTENDED_SET(gpio);
+ custom_write_reg32(MSP_GPIO_DATA_REGISTER[gpio],
+ tmpdata);
+ } else {
+ u32 newdata;
Here too.
+ if (data == MSP_GPIO_HI)
+ newdata = EXTENDED_SET(gpio);
+ else
+ newdata = EXTENDED_CLR(gpio);
+ set_value_reg32(MSP_GPIO_DATA_REGISTER[gpio],
+ EXTENDED_FULL_MASK, newdata);
+ }
+ }
+}
[...]
diff --git a/arch/mips/pmc-sierra/msp71xx/msp_eth.c b/arch/mips/pmc-sierra/msp71xx/msp_eth.c
new file mode 100644
index 0000000..fa510ca
--- /dev/null
+++ b/arch/mips/pmc-sierra/msp71xx/msp_eth.c
@@ -0,0 +1,188 @@
+int __init msp_eth_setup(void)
+{
+ int i, ret = 0;
+
+ /* Configure the GPIO and take the ethernet PHY out of reset */
+ msp_gpio_pin_mode(MSP_GPIO_OUTPUT, MSP_ETHERNET_GPIO0);
+ msp_gpio_pin_hi(MSP_ETHERNET_GPIO0);
+
+#ifdef CONFIG_MSP_HAS_TSMAC
+ /* 3 phys on boards with TSMAC */
+ msp_gpio_pin_mode(MSP_GPIO_OUTPUT, MSP_ETHERNET_GPIO1);
+ msp_gpio_pin_hi(MSP_ETHERNET_GPIO1);
+
+ msp_gpio_pin_mode(MSP_GPIO_OUTPUT, MSP_ETHERNET_GPIO2);
+ msp_gpio_pin_hi(MSP_ETHERNET_GPIO2);
+#endif
+ for (i = 0; i< ARRAY_SIZE(msp_eth_devs); i++) {
+ ret = platform_device_register(&msp_eth_devs[i]);
+ printk(KERN_INFO"device: %d, return value = %d\n", i, ret);
^
Space wouldn't hurt here.
+ if (ret) {
+ while (--i >= 0)
+ platform_device_unregister(&msp_eth_devs[i]);
Why unregister all devices if one registration fails?
+ break;
+ }
+ }
+
+ if (ret)
+ printk(KERN_WARNING "Could not initialize \
+ MSPETH device structures.\n");
String literals are not broken up like this, instead use:
printk(KERN_WARNING "Could not initialize "
"MSPETH device structures.\n");
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/