| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 25 Jan 2011 15:30:03 +0300
From: Sergei Shtylyov <sshtylyov@...sta.com>
To: "Anoop P.A" <anoop.pa@...il.com>
CC: ralf@...ux-mips.org, linux-mips@...ux-mips.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 5/6] Platform support for On-chip MSP ethernet devices.
Hello.
On 25-01-2011 13:28, Anoop P.A wrote:
> From: Anoop P A<anoop.pa@...il.com>
> 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@...il.com>
[...]
> 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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists