* Re: [PATCH net-next-2.6 8/17 v2] can: EG20T PCH: Change Copyright and module description
From: Tomoya MORINAGA @ 2010-11-24 12:32 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
margie.foster-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Wolfgang Grandegger,
joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CECEE1E.9030201@pengutronix.de>
On Wednesday, November 24, 2010 7:51 PM, Marc Kleine-Budde wrote:
> Did your mailer spawn some extra spaces here?
Sorry for in convenience.
Once saving patch-mail temporary, it seems sometimes mail format is broken.
I can see the phonemenon.
I sent v3 patch series without temporary savings.
---
Thanks,
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.
^ permalink raw reply
* Re: [PATCH net-next-2.6 v3] can: Topcliff: PCH_CAN driver: Add Flow control,
From: Marc Kleine-Budde @ 2010-11-24 12:34 UTC (permalink / raw)
To: Tomoya MORINAGA
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
margie.foster-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w,
joel.clark-ral2JQCrhuEAvxtiuMwx3w, qi.wang-ral2JQCrhuEAvxtiuMwx3w,
David S. Miller, Christian Pellegrin, Wolfgang Grandegger
In-Reply-To: <000b01cb8b6b$d7542a90$66f8800a-a06+6cuVnkTSQfdrb5gaxUEOCMrvLtNR@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 1795 bytes --]
On 11/24/2010 01:09 AM, Tomoya MORINAGA wrote:
> On Monday, November 22, 2010 5:27 PM, Marc Kleine-Budde wrote:
>>>>>> Still we have the busy waiting in the TX path. Maybe you can move the
>>>>>> waiting before accessing the if[1] and remove the busy waiting here.
>>>>> I can't understand your saying.
>>>>> For transmitting data, calling pch_can_rw_msg_obj is mandatory.
>>>> Yes, but the busy wait is not needed. It should be enough to do the
>>>> busy-waiting _before_ accessing the if[1].
>>>
>>> Do you mean we should create other pch_can_rw_msg_obj which doesn't have busy wait ?
>> ACK, and this non busy waiting is use in the TX path. But you add a busy
>> wait only function before accessing the if[1] in the TX path.
>
> The "busy waiting" of pch_can_rw_msg_obj is for next processing accesses to Message object.
> If deleting this busy waiting, next processing can access to Message object, regardless previous transfer doesn't
> complete yet.
> Thus, I think, the "busy waiting" is necessary.
Yes, it's necessary, but not where it is done currently.
Let me outline how I think the TX path should look like:
pch_xmit() {
take_care_about_flow_control();
prepare_can_frame_to_be_copied_to_tx_if();
/* most likely we don't have to wait here */
wait_until_tx_if_is_ready();
copy_can_frame_to_tx_if();
/* trigger tx in hardware */
send_tx_if_but_dont_do_busywait();
/* tx_if is busy now, but before we access it, we'll check tx_if is ready */
}
cheers, Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
[-- Attachment #2: Type: text/plain, Size: 188 bytes --]
_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core
^ permalink raw reply
* [PATCH 1/4] stmmac: tidy-up stmmac_priv structure
From: Peppe CAVALLARO @ 2010-11-24 12:37 UTC (permalink / raw)
To: netdev@vger.kernel.org; +Cc: Peppe CAVALLARO
This patch tidies-up the stmmac_priv structure
that had many fileds alredy defined in the
plat_stmmacenet_data structure.
Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
drivers/net/stmmac/stmmac.h | 15 ++------
drivers/net/stmmac/stmmac_ethtool.c | 4 +-
drivers/net/stmmac/stmmac_main.c | 69 ++++++++++++++++++-----------------
drivers/net/stmmac/stmmac_mdio.c | 8 ++--
4 files changed, 44 insertions(+), 52 deletions(-)
diff --git a/drivers/net/stmmac/stmmac.h b/drivers/net/stmmac/stmmac.h
index 79bdc2e..3157567 100644
--- a/drivers/net/stmmac/stmmac.h
+++ b/drivers/net/stmmac/stmmac.h
@@ -37,7 +37,6 @@ struct stmmac_priv {
unsigned int cur_tx;
unsigned int dirty_tx;
unsigned int dma_tx_size;
- int tx_coe;
int tx_coalesce;
struct dma_desc *dma_rx ;
@@ -48,7 +47,6 @@ struct stmmac_priv {
struct sk_buff_head rx_recycle;
struct net_device *dev;
- int is_gmac;
dma_addr_t dma_rx_phy;
unsigned int dma_rx_size;
unsigned int dma_buf_sz;
@@ -60,14 +58,11 @@ struct stmmac_priv {
struct napi_struct napi;
phy_interface_t phy_interface;
- int pbl;
- int bus_id;
int phy_addr;
int phy_mask;
int (*phy_reset) (void *priv);
- void (*fix_mac_speed) (void *priv, unsigned int speed);
- void (*bus_setup)(void __iomem *ioaddr);
- void *bsp_priv;
+ int rx_coe;
+ int no_csum_insertion;
int phy_irq;
struct phy_device *phydev;
@@ -77,7 +72,6 @@ struct stmmac_priv {
unsigned int flow_ctrl;
unsigned int pause;
struct mii_bus *mii;
- int mii_clk_csr;
u32 msg_enable;
spinlock_t lock;
@@ -90,10 +84,7 @@ struct stmmac_priv {
#ifdef STMMAC_VLAN_TAG_USED
struct vlan_group *vlgrp;
#endif
- int enh_desc;
- int rx_coe;
- int bugged_jumbo;
- int no_csum_insertion;
+ struct plat_stmmacenet_data *plat;
};
#ifdef CONFIG_STM_DRIVERS
diff --git a/drivers/net/stmmac/stmmac_ethtool.c b/drivers/net/stmmac/stmmac_ethtool.c
index 6d65482..f2695fd 100644
--- a/drivers/net/stmmac/stmmac_ethtool.c
+++ b/drivers/net/stmmac/stmmac_ethtool.c
@@ -94,7 +94,7 @@ static void stmmac_ethtool_getdrvinfo(struct net_device *dev,
{
struct stmmac_priv *priv = netdev_priv(dev);
- if (!priv->is_gmac)
+ if (!priv->plat->has_gmac)
strcpy(info->driver, MAC100_ETHTOOL_NAME);
else
strcpy(info->driver, GMAC_ETHTOOL_NAME);
@@ -176,7 +176,7 @@ static void stmmac_ethtool_gregs(struct net_device *dev,
memset(reg_space, 0x0, REG_SPACE_SIZE);
- if (!priv->is_gmac) {
+ if (!priv->plat->has_gmac) {
/* MAC registers */
for (i = 0; i < 12; i++)
reg_space[i] = readl(priv->ioaddr + (i * 4));
diff --git a/drivers/net/stmmac/stmmac_main.c b/drivers/net/stmmac/stmmac_main.c
index 06bc603..29ba286 100644
--- a/drivers/net/stmmac/stmmac_main.c
+++ b/drivers/net/stmmac/stmmac_main.c
@@ -186,6 +186,18 @@ static inline u32 stmmac_tx_avail(struct stmmac_priv *priv)
return priv->dirty_tx + priv->dma_tx_size - priv->cur_tx - 1;
}
+/* On some ST platforms, some HW system configuraton registers have to be
+ * set according to the link speed negotiated.
+ */
+static inline void stmmac_hw_fix_mac_speed(struct stmmac_priv *priv)
+{
+ struct phy_device *phydev = priv->phydev;
+
+ if (likely(priv->plat->fix_mac_speed))
+ priv->plat->fix_mac_speed(priv->plat->bsp_priv,
+ phydev->speed);
+}
+
/**
* stmmac_adjust_link
* @dev: net device structure
@@ -228,15 +240,13 @@ static void stmmac_adjust_link(struct net_device *dev)
new_state = 1;
switch (phydev->speed) {
case 1000:
- if (likely(priv->is_gmac))
+ if (likely(priv->plat->has_gmac))
ctrl &= ~priv->hw->link.port;
- if (likely(priv->fix_mac_speed))
- priv->fix_mac_speed(priv->bsp_priv,
- phydev->speed);
+ stmmac_hw_fix_mac_speed(priv);
break;
case 100:
case 10:
- if (priv->is_gmac) {
+ if (priv->plat->has_gmac) {
ctrl |= priv->hw->link.port;
if (phydev->speed == SPEED_100) {
ctrl |= priv->hw->link.speed;
@@ -246,9 +256,7 @@ static void stmmac_adjust_link(struct net_device *dev)
} else {
ctrl &= ~priv->hw->link.port;
}
- if (likely(priv->fix_mac_speed))
- priv->fix_mac_speed(priv->bsp_priv,
- phydev->speed);
+ stmmac_hw_fix_mac_speed(priv);
break;
default:
if (netif_msg_link(priv))
@@ -305,7 +313,7 @@ static int stmmac_init_phy(struct net_device *dev)
return 0;
}
- snprintf(bus_id, MII_BUS_ID_SIZE, "%x", priv->bus_id);
+ snprintf(bus_id, MII_BUS_ID_SIZE, "%x", priv->plat->bus_id);
snprintf(phy_id, MII_BUS_ID_SIZE + 3, PHY_ID_FMT, bus_id,
priv->phy_addr);
pr_debug("stmmac_init_phy: trying to attach to %s\n", phy_id);
@@ -552,7 +560,7 @@ static void free_dma_desc_resources(struct stmmac_priv *priv)
*/
static void stmmac_dma_operation_mode(struct stmmac_priv *priv)
{
- if (likely((priv->tx_coe) && (!priv->no_csum_insertion))) {
+ if (likely((priv->plat->tx_coe) && (!priv->no_csum_insertion))) {
/* In case of GMAC, SF mode has to be enabled
* to perform the TX COE. This depends on:
* 1) TX COE if actually supported
@@ -814,7 +822,7 @@ static int stmmac_open(struct net_device *dev)
init_dma_desc_rings(dev);
/* DMA initialization and SW reset */
- if (unlikely(priv->hw->dma->init(priv->ioaddr, priv->pbl,
+ if (unlikely(priv->hw->dma->init(priv->ioaddr, priv->plat->pbl,
priv->dma_tx_phy,
priv->dma_rx_phy) < 0)) {
@@ -825,15 +833,15 @@ static int stmmac_open(struct net_device *dev)
/* Copy the MAC addr into the HW */
priv->hw->mac->set_umac_addr(priv->ioaddr, dev->dev_addr, 0);
/* If required, perform hw setup of the bus. */
- if (priv->bus_setup)
- priv->bus_setup(priv->ioaddr);
+ if (priv->plat->bus_setup)
+ priv->plat->bus_setup(priv->ioaddr);
/* Initialize the MAC Core */
priv->hw->mac->core_init(priv->ioaddr);
priv->rx_coe = priv->hw->mac->rx_coe(priv->ioaddr);
if (priv->rx_coe)
pr_info("stmmac: Rx Checksum Offload Engine supported\n");
- if (priv->tx_coe)
+ if (priv->plat->tx_coe)
pr_info("\tTX Checksum insertion supported\n");
priv->shutdown = 0;
@@ -1042,7 +1050,8 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
return stmmac_sw_tso(priv, skb);
if (likely((skb->ip_summed == CHECKSUM_PARTIAL))) {
- if (unlikely((!priv->tx_coe) || (priv->no_csum_insertion)))
+ if (unlikely((!priv->plat->tx_coe) ||
+ (priv->no_csum_insertion)))
skb_checksum_help(skb);
else
csum_insertion = 1;
@@ -1146,7 +1155,7 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv)
DMA_FROM_DEVICE);
(p + entry)->des2 = priv->rx_skbuff_dma[entry];
- if (unlikely(priv->is_gmac)) {
+ if (unlikely(priv->plat->has_gmac)) {
if (bfsize >= BUF_SIZE_8KiB)
(p + entry)->des3 =
(p + entry)->des2 + BUF_SIZE_8KiB;
@@ -1356,7 +1365,7 @@ static int stmmac_change_mtu(struct net_device *dev, int new_mtu)
return -EBUSY;
}
- if (priv->is_gmac)
+ if (priv->plat->has_gmac)
max_mtu = JUMBO_LEN;
else
max_mtu = ETH_DATA_LEN;
@@ -1370,7 +1379,7 @@ static int stmmac_change_mtu(struct net_device *dev, int new_mtu)
* needs to have the Tx COE disabled for oversized frames
* (due to limited buffer sizes). In this case we disable
* the TX csum insertionin the TDES and not use SF. */
- if ((priv->bugged_jumbo) && (priv->dev->mtu > ETH_DATA_LEN))
+ if ((priv->plat->bugged_jumbo) && (priv->dev->mtu > ETH_DATA_LEN))
priv->no_csum_insertion = 1;
else
priv->no_csum_insertion = 0;
@@ -1390,7 +1399,7 @@ static irqreturn_t stmmac_interrupt(int irq, void *dev_id)
return IRQ_NONE;
}
- if (priv->is_gmac)
+ if (priv->plat->has_gmac)
/* To handle GMAC own interrupts */
priv->hw->mac->host_irq_status((void __iomem *) dev->base_addr);
@@ -1536,7 +1545,7 @@ static int stmmac_mac_device_setup(struct net_device *dev)
struct mac_device_info *device;
- if (priv->is_gmac)
+ if (priv->plat->has_gmac)
device = dwmac1000_setup(priv->ioaddr);
else
device = dwmac100_setup(priv->ioaddr);
@@ -1544,7 +1553,7 @@ static int stmmac_mac_device_setup(struct net_device *dev)
if (!device)
return -ENOMEM;
- if (priv->enh_desc) {
+ if (priv->plat->enh_desc) {
device->desc = &enh_desc_ops;
pr_info("\tEnhanced descriptor structure\n");
} else
@@ -1598,7 +1607,7 @@ static int stmmac_associate_phy(struct device *dev, void *data)
plat_dat->bus_id);
/* Check that this phy is for the MAC being initialised */
- if (priv->bus_id != plat_dat->bus_id)
+ if (priv->plat->bus_id != plat_dat->bus_id)
return 0;
/* OK, this PHY is connected to the MAC.
@@ -1683,13 +1692,9 @@ static int stmmac_dvr_probe(struct platform_device *pdev)
priv->device = &(pdev->dev);
priv->dev = ndev;
plat_dat = pdev->dev.platform_data;
- priv->bus_id = plat_dat->bus_id;
- priv->pbl = plat_dat->pbl; /* TLI */
- priv->mii_clk_csr = plat_dat->clk_csr;
- priv->tx_coe = plat_dat->tx_coe;
- priv->bugged_jumbo = plat_dat->bugged_jumbo;
- priv->is_gmac = plat_dat->has_gmac; /* GMAC is on board */
- priv->enh_desc = plat_dat->enh_desc;
+
+ priv->plat = plat_dat;
+
priv->ioaddr = addr;
/* PMT module is not integrated in all the MAC devices. */
@@ -1727,16 +1732,12 @@ static int stmmac_dvr_probe(struct platform_device *pdev)
goto out;
}
- priv->fix_mac_speed = plat_dat->fix_mac_speed;
- priv->bus_setup = plat_dat->bus_setup;
- priv->bsp_priv = plat_dat->bsp_priv;
-
pr_info("\t%s - (dev. name: %s - id: %d, IRQ #%d\n"
"\tIO base addr: 0x%p)\n", ndev->name, pdev->name,
pdev->id, ndev->irq, addr);
/* MDIO bus Registration */
- pr_debug("\tMDIO bus (id: %d)...", priv->bus_id);
+ pr_debug("\tMDIO bus (id: %d)...", priv->plat->bus_id);
ret = stmmac_mdio_register(ndev);
if (ret < 0)
goto out;
diff --git a/drivers/net/stmmac/stmmac_mdio.c b/drivers/net/stmmac/stmmac_mdio.c
index d744161..234b406 100644
--- a/drivers/net/stmmac/stmmac_mdio.c
+++ b/drivers/net/stmmac/stmmac_mdio.c
@@ -53,7 +53,7 @@ static int stmmac_mdio_read(struct mii_bus *bus, int phyaddr, int phyreg)
int data;
u16 regValue = (((phyaddr << 11) & (0x0000F800)) |
((phyreg << 6) & (0x000007C0)));
- regValue |= MII_BUSY | ((priv->mii_clk_csr & 7) << 2);
+ regValue |= MII_BUSY | ((priv->plat->clk_csr & 7) << 2);
do {} while (((readl(priv->ioaddr + mii_address)) & MII_BUSY) == 1);
writel(regValue, priv->ioaddr + mii_address);
@@ -85,7 +85,7 @@ static int stmmac_mdio_write(struct mii_bus *bus, int phyaddr, int phyreg,
(((phyaddr << 11) & (0x0000F800)) | ((phyreg << 6) & (0x000007C0)))
| MII_WRITE;
- value |= MII_BUSY | ((priv->mii_clk_csr & 7) << 2);
+ value |= MII_BUSY | ((priv->plat->clk_csr & 7) << 2);
/* Wait until any existing MII operation is complete */
@@ -114,7 +114,7 @@ static int stmmac_mdio_reset(struct mii_bus *bus)
if (priv->phy_reset) {
pr_debug("stmmac_mdio_reset: calling phy_reset\n");
- priv->phy_reset(priv->bsp_priv);
+ priv->phy_reset(priv->plat->bsp_priv);
}
/* This is a workaround for problems with the STE101P PHY.
@@ -157,7 +157,7 @@ int stmmac_mdio_register(struct net_device *ndev)
new_bus->read = &stmmac_mdio_read;
new_bus->write = &stmmac_mdio_write;
new_bus->reset = &stmmac_mdio_reset;
- snprintf(new_bus->id, MII_BUS_ID_SIZE, "%x", priv->bus_id);
+ snprintf(new_bus->id, MII_BUS_ID_SIZE, "%x", priv->plat->bus_id);
new_bus->priv = ndev;
new_bus->irq = irqlist;
new_bus->phy_mask = priv->phy_mask;
--
1.5.5.6
^ permalink raw reply related
* [PATCH 2/4] stmmac: add init/exit callback in plat_stmmacenet_data struct
From: Peppe CAVALLARO @ 2010-11-24 12:38 UTC (permalink / raw)
To: netdev@vger.kernel.org; +Cc: Peppe CAVALLARO
In-Reply-To: <1290561646-9429-1-git-send-email-peppe.cavallaro@st.com>
This patch adds in the plat_stmmacenet_data
the init and exit callbacks that can be used
for invoking specific platform functions.
For example, on ST targets, these call the
PAD manager functions to set PIO lines and
syscfg registers.
The patch removes the stmmac_claim_resource
only used on STM Kernels as well.
Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
drivers/net/stmmac/stmmac.h | 22 ----------------------
drivers/net/stmmac/stmmac_main.c | 18 +++++++++++++-----
include/linux/stmmac.h | 6 +++---
3 files changed, 16 insertions(+), 30 deletions(-)
diff --git a/drivers/net/stmmac/stmmac.h b/drivers/net/stmmac/stmmac.h
index 3157567..8ae7650 100644
--- a/drivers/net/stmmac/stmmac.h
+++ b/drivers/net/stmmac/stmmac.h
@@ -87,28 +87,6 @@ struct stmmac_priv {
struct plat_stmmacenet_data *plat;
};
-#ifdef CONFIG_STM_DRIVERS
-#include <linux/stm/pad.h>
-static inline int stmmac_claim_resource(struct platform_device *pdev)
-{
- int ret = 0;
- struct plat_stmmacenet_data *plat_dat = pdev->dev.platform_data;
-
- /* Pad routing setup */
- if (IS_ERR(devm_stm_pad_claim(&pdev->dev, plat_dat->pad_config,
- dev_name(&pdev->dev)))) {
- printk(KERN_ERR "%s: Failed to request pads!\n", __func__);
- ret = -ENODEV;
- }
- return ret;
-}
-#else
-static inline int stmmac_claim_resource(struct platform_device *pdev)
-{
- return 0;
-}
-#endif
-
extern int stmmac_mdio_unregister(struct net_device *ndev);
extern int stmmac_mdio_register(struct net_device *ndev);
extern void stmmac_set_ethtool_ops(struct net_device *netdev);
diff --git a/drivers/net/stmmac/stmmac_main.c b/drivers/net/stmmac/stmmac_main.c
index 29ba286..b806cd3 100644
--- a/drivers/net/stmmac/stmmac_main.c
+++ b/drivers/net/stmmac/stmmac_main.c
@@ -1643,7 +1643,7 @@ static int stmmac_dvr_probe(struct platform_device *pdev)
struct resource *res;
void __iomem *addr = NULL;
struct net_device *ndev = NULL;
- struct stmmac_priv *priv;
+ struct stmmac_priv *priv = NULL;
struct plat_stmmacenet_data *plat_dat;
pr_info("STMMAC driver:\n\tplatform registration... ");
@@ -1708,10 +1708,12 @@ static int stmmac_dvr_probe(struct platform_device *pdev)
/* Set the I/O base addr */
ndev->base_addr = (unsigned long)addr;
- /* Verify embedded resource for the platform */
- ret = stmmac_claim_resource(pdev);
- if (ret < 0)
- goto out;
+ /* Custom initialisation */
+ if (priv->plat->init) {
+ ret = priv->plat->init(pdev);
+ if (unlikely(ret))
+ goto out;
+ }
/* MAC HW revice detection */
ret = stmmac_mac_device_setup(ndev);
@@ -1745,6 +1747,9 @@ static int stmmac_dvr_probe(struct platform_device *pdev)
out:
if (ret < 0) {
+ if (priv->plat->exit)
+ priv->plat->exit(pdev);
+
platform_set_drvdata(pdev, NULL);
release_mem_region(res->start, resource_size(res));
if (addr != NULL)
@@ -1778,6 +1783,9 @@ static int stmmac_dvr_remove(struct platform_device *pdev)
stmmac_mdio_unregister(ndev);
+ if (priv->plat->exit)
+ priv->plat->exit(pdev);
+
platform_set_drvdata(pdev, NULL);
unregister_netdev(ndev);
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index d66c617..e103529 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -40,9 +40,9 @@ struct plat_stmmacenet_data {
int pmt;
void (*fix_mac_speed)(void *priv, unsigned int speed);
void (*bus_setup)(void __iomem *ioaddr);
-#ifdef CONFIG_STM_DRIVERS
- struct stm_pad_config *pad_config;
-#endif
+ int (*init)(struct platform_device *pdev);
+ void (*exit)(struct platform_device *pdev);
+ void *custom_cfg;
void *bsp_priv;
};
--
1.5.5.6
^ permalink raw reply related
* [PATCH 3/4] stmmac: convert to dev_pm_ops.
From: Peppe CAVALLARO @ 2010-11-24 12:38 UTC (permalink / raw)
To: netdev@vger.kernel.org; +Cc: Peppe CAVALLARO
In-Reply-To: <1290561646-9429-2-git-send-email-peppe.cavallaro@st.com>
This patch updates the PM support using the dev_pm_ops
and reviews the hibernation support.
Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
drivers/net/stmmac/stmmac.h | 1 -
drivers/net/stmmac/stmmac_main.c | 124 ++++++++++++++++++++++----------------
2 files changed, 71 insertions(+), 54 deletions(-)
diff --git a/drivers/net/stmmac/stmmac.h b/drivers/net/stmmac/stmmac.h
index 8ae7650..eb258e2 100644
--- a/drivers/net/stmmac/stmmac.h
+++ b/drivers/net/stmmac/stmmac.h
@@ -77,7 +77,6 @@ struct stmmac_priv {
spinlock_t lock;
int wolopts;
int wolenabled;
- int shutdown;
#ifdef CONFIG_STMMAC_TIMER
struct stmmac_timer *tm;
#endif
diff --git a/drivers/net/stmmac/stmmac_main.c b/drivers/net/stmmac/stmmac_main.c
index b806cd3..f1dbc18 100644
--- a/drivers/net/stmmac/stmmac_main.c
+++ b/drivers/net/stmmac/stmmac_main.c
@@ -844,8 +844,6 @@ static int stmmac_open(struct net_device *dev)
if (priv->plat->tx_coe)
pr_info("\tTX Checksum insertion supported\n");
- priv->shutdown = 0;
-
/* Initialise the MMC (if present) to disable all interrupts. */
writel(0xffffffff, priv->ioaddr + MMC_HIGH_INTR_MASK);
writel(0xffffffff, priv->ioaddr + MMC_LOW_INTR_MASK);
@@ -1799,61 +1797,53 @@ static int stmmac_dvr_remove(struct platform_device *pdev)
}
#ifdef CONFIG_PM
-static int stmmac_suspend(struct platform_device *pdev, pm_message_t state)
+static int stmmac_suspend(struct device *dev)
{
- struct net_device *dev = platform_get_drvdata(pdev);
- struct stmmac_priv *priv = netdev_priv(dev);
+ struct net_device *ndev = dev_get_drvdata(dev);
+ struct stmmac_priv *priv = netdev_priv(ndev);
int dis_ic = 0;
- if (!dev || !netif_running(dev))
+ if (!ndev || !netif_running(ndev))
return 0;
spin_lock(&priv->lock);
- if (state.event == PM_EVENT_SUSPEND) {
- netif_device_detach(dev);
- netif_stop_queue(dev);
- if (priv->phydev)
- phy_stop(priv->phydev);
+ netif_device_detach(ndev);
+ netif_stop_queue(ndev);
+ if (priv->phydev)
+ phy_stop(priv->phydev);
#ifdef CONFIG_STMMAC_TIMER
- priv->tm->timer_stop();
- if (likely(priv->tm->enable))
- dis_ic = 1;
+ priv->tm->timer_stop();
+ if (likely(priv->tm->enable))
+ dis_ic = 1;
#endif
- napi_disable(&priv->napi);
-
- /* Stop TX/RX DMA */
- priv->hw->dma->stop_tx(priv->ioaddr);
- priv->hw->dma->stop_rx(priv->ioaddr);
- /* Clear the Rx/Tx descriptors */
- priv->hw->desc->init_rx_desc(priv->dma_rx, priv->dma_rx_size,
- dis_ic);
- priv->hw->desc->init_tx_desc(priv->dma_tx, priv->dma_tx_size);
-
- /* Enable Power down mode by programming the PMT regs */
- if (device_can_wakeup(priv->device))
- priv->hw->mac->pmt(priv->ioaddr, priv->wolopts);
- else
- stmmac_disable_mac(priv->ioaddr);
- } else {
- priv->shutdown = 1;
- /* Although this can appear slightly redundant it actually
- * makes fast the standby operation and guarantees the driver
- * working if hibernation is on media. */
- stmmac_release(dev);
- }
+ napi_disable(&priv->napi);
+
+ /* Stop TX/RX DMA */
+ priv->hw->dma->stop_tx(priv->ioaddr);
+ priv->hw->dma->stop_rx(priv->ioaddr);
+ /* Clear the Rx/Tx descriptors */
+ priv->hw->desc->init_rx_desc(priv->dma_rx, priv->dma_rx_size,
+ dis_ic);
+ priv->hw->desc->init_tx_desc(priv->dma_tx, priv->dma_tx_size);
+
+ /* Enable Power down mode by programming the PMT regs */
+ if (device_may_wakeup(priv->device))
+ priv->hw->mac->pmt(priv->ioaddr, priv->wolopts);
+ else
+ stmmac_disable_mac(priv->ioaddr);
spin_unlock(&priv->lock);
return 0;
}
-static int stmmac_resume(struct platform_device *pdev)
+static int stmmac_resume(struct device *dev)
{
- struct net_device *dev = platform_get_drvdata(pdev);
- struct stmmac_priv *priv = netdev_priv(dev);
+ struct net_device *ndev = dev_get_drvdata(dev);
+ struct stmmac_priv *priv = netdev_priv(ndev);
- if (!netif_running(dev))
+ if (!netif_running(ndev))
return 0;
if (priv->shutdown) {
@@ -1870,10 +1860,10 @@ static int stmmac_resume(struct platform_device *pdev)
* is received. Anyway, it's better to manually clear
* this bit because it can generate problems while resuming
* from another devices (e.g. serial console). */
- if (device_can_wakeup(priv->device))
+ if (device_may_wakeup(priv->device))
priv->hw->mac->pmt(priv->ioaddr, 0);
- netif_device_attach(dev);
+ netif_device_attach(ndev);
/* Enable the MAC and DMA */
stmmac_enable_mac(priv->ioaddr);
@@ -1881,31 +1871,59 @@ static int stmmac_resume(struct platform_device *pdev)
priv->hw->dma->start_rx(priv->ioaddr);
#ifdef CONFIG_STMMAC_TIMER
- priv->tm->timer_start(tmrate);
+ if (likely(priv->tm->enable))
+ priv->tm->timer_start(tmrate);
#endif
napi_enable(&priv->napi);
if (priv->phydev)
phy_start(priv->phydev);
- netif_start_queue(dev);
+ netif_start_queue(ndev);
spin_unlock(&priv->lock);
return 0;
}
-#endif
-static struct platform_driver stmmac_driver = {
- .driver = {
- .name = STMMAC_RESOURCE_NAME,
- },
- .probe = stmmac_dvr_probe,
- .remove = stmmac_dvr_remove,
-#ifdef CONFIG_PM
+static int stmmac_freeze(struct device *dev)
+{
+ struct net_device *ndev = dev_get_drvdata(dev);
+
+ if (!ndev || !netif_running(ndev))
+ return 0;
+
+ return stmmac_release(ndev);
+}
+
+static int stmmac_restore(struct device *dev)
+{
+ struct net_device *ndev = dev_get_drvdata(dev);
+
+ if (!ndev || !netif_running(ndev))
+ return 0;
+
+ return stmmac_open(ndev);
+}
+
+static const struct dev_pm_ops stmmac_pm_ops = {
.suspend = stmmac_suspend,
.resume = stmmac_resume,
-#endif
+ .freeze = stmmac_freeze,
+ .thaw = stmmac_restore,
+ .restore = stmmac_restore,
+};
+#else
+static const struct dev_pm_ops stmmac_pm_ops;
+#endif /* CONFIG_PM */
+static struct platform_driver stmmac_driver = {
+ .probe = stmmac_dvr_probe,
+ .remove = stmmac_dvr_remove,
+ .driver = {
+ .name = STMMAC_RESOURCE_NAME,
+ .owner = THIS_MODULE,
+ .pm = &stmmac_pm_ops,
+ },
};
/**
--
1.5.5.6
^ permalink raw reply related
* [PATCH 4/4] stmmac: update the driver version
From: Peppe CAVALLARO @ 2010-11-24 12:38 UTC (permalink / raw)
To: netdev@vger.kernel.org; +Cc: Peppe CAVALLARO
In-Reply-To: <1290561646-9429-3-git-send-email-peppe.cavallaro@st.com>
Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
drivers/net/stmmac/stmmac.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/stmmac/stmmac.h b/drivers/net/stmmac/stmmac.h
index eb258e2..5f06c47 100644
--- a/drivers/net/stmmac/stmmac.h
+++ b/drivers/net/stmmac/stmmac.h
@@ -20,7 +20,7 @@
Author: Giuseppe Cavallaro <peppe.cavallaro@st.com>
*******************************************************************************/
-#define DRV_MODULE_VERSION "Apr_2010"
+#define DRV_MODULE_VERSION "Nov_2010"
#include <linux/platform_device.h>
#include <linux/stmmac.h>
--
1.5.5.6
^ permalink raw reply related
* Re: [PATCH net-2.6] net, ppp: Report correct error code if unit allocation failed
From: Jarek Poplawski @ 2010-11-24 12:49 UTC (permalink / raw)
To: Cyrill Gorcunov
Cc: LNML, Paul Mackerras, David Miller, linux-ppp, linux-kernel
In-Reply-To: <20101123214344.GB1839@lenovo>
On 2010-11-23 22:43, Cyrill Gorcunov wrote:
> Allocating unit from ird might return several error codes
> not only -EAGAIN, so it should not be changed and returned
> precisely. Same time unit release procedure should be invoked
> only if device is unregistering.
IMHO this unit release fix should be in a separate patch.
...
> @@ -2668,10 +2668,10 @@ static void ppp_shutdown_interface(struc
> ppp->closing = 1;
> ppp_unlock(ppp);
> unregister_netdev(ppp->dev);
> + unit_put(&pn->units_idr, ppp->file.index);
> } else
> ppp_unlock(ppp);
>
> - unit_put(&pn->units_idr, ppp->file.index);
> ppp->file.dead = 1;
> ppp->owner = NULL;
> wake_up_interruptible(&ppp->file.rwait);
Btw, it seems these last 3 lines could be moved similarly.
Jarek P.
^ permalink raw reply
* Re: [PATCH net-next-2.6 12/17 v3] can: EG20T PCH: Fix bit timing calculation issue
From: Marc Kleine-Budde @ 2010-11-24 12:58 UTC (permalink / raw)
To: Tomoya MORINAGA
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
margie.foster-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Wolfgang Grandegger,
joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CED037F.6060306-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 1912 bytes --]
On 11/24/2010 01:22 PM, Tomoya MORINAGA wrote:
> Fix bit timing calculation issue
> Modify like use calculated value directly passed by CAN core module.
>
> Signed-off-by: Tomoya MORINAGA <tomoya-linux-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
Acked-by: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
> drivers/net/can/pch_can.c | 6 ++----
> 1 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
> index 7342030..466f011 100644
> --- a/drivers/net/can/pch_can.c
> +++ b/drivers/net/can/pch_can.c
> @@ -800,17 +800,15 @@ static int pch_set_bittiming(struct net_device *ndev)
> const struct can_bittiming *bt = &priv->can.bittiming;
> u32 canbit;
> u32 bepe;
> - u32 brp;
>
> /* Setting the CCE bit for accessing the Can Timing register. */
> pch_can_bit_set(&priv->regs->cont, PCH_CTRL_CCE);
>
> - brp = (bt->tq) / (1000000000/PCH_CAN_CLK) - 1;
> - canbit = brp & PCH_MSK_BITT_BRP;
> + canbit = (bt->brp - 1) & PCH_MSK_BITT_BRP;
Masking here shouldn't be necessary but won't hurt, better play safe.
> canbit |= (bt->sjw - 1) << PCH_BIT_SJW_SHIFT;
> canbit |= (bt->phase_seg1 + bt->prop_seg - 1) << PCH_BIT_TSEG1_SHIFT;
> canbit |= (bt->phase_seg2 - 1) << PCH_BIT_TSEG2_SHIFT;
> - bepe = (brp & PCH_MSK_BRPE_BRPE) >> PCH_BIT_BRPE_BRPE_SHIFT;
> + bepe = ((bt->brp - 1) & PCH_MSK_BRPE_BRPE) >> PCH_BIT_BRPE_BRPE_SHIFT;
dito
> iowrite32(canbit, &priv->regs->bitt);
> iowrite32(bepe, &priv->regs->brpe);
> pch_can_bit_clear(&priv->regs->cont, PCH_CTRL_CCE);
cheers, Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
[-- Attachment #2: Type: text/plain, Size: 188 bytes --]
_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core
^ permalink raw reply
* Re: [PATCH net-next-2.6 5/17 v3] can: EG20T PCH: Delete unnecessary spin_lock
From: Marc Kleine-Budde @ 2010-11-24 13:01 UTC (permalink / raw)
To: Tomoya MORINAGA
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
margie.foster-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Wolfgang Grandegger,
joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CED0200.1030103-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 707 bytes --]
On 11/24/2010 01:16 PM, Tomoya MORINAGA wrote:
> Delete unnecessary spin_lock for accessing Message Object.
> Since all message objects are divided into tx/rx area completely,
> spin_lock processing is unnecessary.
>
> Signed-off-by: Tomoya MORINAGA <tomoya-linux-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
Looks good, however there're some wrapped lines in the patch. Add my
Acked-by in the fixed patch.
cheers, Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
[-- Attachment #2: Type: text/plain, Size: 188 bytes --]
_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core
^ permalink raw reply
* Re: [PATCH net-next-2.6 3/17 v3] can: EG20T PCH: Enumerate some macros
From: Marc Kleine-Budde @ 2010-11-24 13:07 UTC (permalink / raw)
To: Tomoya MORINAGA
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
margie.foster-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Wolfgang Grandegger,
joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CED00A9.50006-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 5184 bytes --]
On 11/24/2010 01:10 PM, Tomoya MORINAGA wrote:
> For easy to readable, LEC and interface register number macros are
> replaced to enums.
Your patch description is a bit misleading. You're not only replacing
defines by enums, you also add rx and tx error counters. Please change
the patch description or split into two patches. (see comments inline)
Wolfgang, can you say something to the generation of the error frames.
cheers, Marc
> Signed-off-by: Tomoya MORINAGA <tomoya-linux-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
> ---
> drivers/net/can/pch_can.c | 90
> +++++++++++++++++++++++++--------------------
> 1 files changed, 50 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
> index b028ae4..5c21f5c 100644
> --- a/drivers/net/can/pch_can.c
> +++ b/drivers/net/can/pch_can.c
> @@ -69,21 +69,12 @@
> #define PCH_REC 0x00007f00
> #define PCH_TEC 0x000000ff
>
> +
> #define PCH_TX_OK BIT(3)
> #define PCH_RX_OK BIT(4)
> #define PCH_EPASSIV BIT(5)
> #define PCH_EWARN BIT(6)
> #define PCH_BUS_OFF BIT(7)
> -#define PCH_LEC0 BIT(0)
> -#define PCH_LEC1 BIT(1)
> -#define PCH_LEC2 BIT(2)
> -#define PCH_LEC_ALL (PCH_LEC0 | PCH_LEC1 | PCH_LEC2)
> -#define PCH_STUF_ERR PCH_LEC0
> -#define PCH_FORM_ERR PCH_LEC1
> -#define PCH_ACK_ERR (PCH_LEC0 | PCH_LEC1)
> -#define PCH_BIT1_ERR PCH_LEC2
> -#define PCH_BIT0_ERR (PCH_LEC0 | PCH_LEC2)
> -#define PCH_CRC_ERR (PCH_LEC1 | PCH_LEC2)
>
> /* bit position of certain controller bits. */
> #define PCH_BIT_BRP 0
> @@ -96,9 +87,6 @@
> #define PCH_MSK_CTRL_IE_SIE_EIE 0x07
> #define PCH_COUNTER_LIMIT 10
>
> -#define PCH_RX_IFREG 0
> -#define PCH_TX_IFREG 1
> -
> #define PCH_CAN_CLK 50000000 /* 50MHz */
>
> /* Define the number of message object.
> @@ -113,6 +101,21 @@
>
> #define PCH_FIFO_THRESH 16
>
> +enum pch_ifreg {
> + PCH_RX_IFREG,
> + PCH_TX_IFREG,
> +};
please fold the "enum pch_ifreg" related changes into patch 1.
> +
> +enum pch_can_err {
> + PCH_STUF_ERR = 1,
> + PCH_FORM_ERR,
> + PCH_ACK_ERR,
> + PCH_BIT1_ERR,
> + PCH_BIT0_ERR,
> + PCH_CRC_ERR,
> + PCH_LEC_ALL,
> +};
> +
> enum pch_can_mode {
> PCH_CAN_ENABLE,
> PCH_CAN_DISABLE,
> @@ -302,7 +305,7 @@ static void pch_can_check_if_busy(u32 __iomem
> *creq_addr, u32 num)
> }
>
> static void pch_can_set_rxtx(struct pch_can_priv *priv, u32 buff_num,
> - int set, u32 dir)
> + int set, enum pch_ifreg dir)
this should go into patch 1.
> {
> unsigned long flags;
> u32 ie;
> @@ -616,7 +619,7 @@ static void pch_can_error(struct net_device *ndev,
> u32 status)
> struct sk_buff *skb;
> struct pch_can_priv *priv = netdev_priv(ndev);
> struct can_frame *cf;
> - u32 errc;
> + u32 errc, lec;
> struct net_device_stats *stats = &(priv->ndev->stats);
> enum can_state state = priv->can.state;
>
> @@ -661,35 +664,42 @@ static void pch_can_error(struct net_device *ndev,
> u32 status)
> "%s -> CAN controller is ERROR PASSIVE .\n", __func__);
> }
>
> - if (status & PCH_LEC_ALL) {
> + lec = status & PCH_LEC_ALL;
> + switch (lec) {
> + case PCH_STUF_ERR:
> + cf->data[2] |= CAN_ERR_PROT_STUFF;
> priv->can.can_stats.bus_error++;
> stats->rx_errors++;
> - switch (status & PCH_LEC_ALL) {
> - case PCH_STUF_ERR:
> - cf->data[2] |= CAN_ERR_PROT_STUFF;
> - break;
> - case PCH_FORM_ERR:
> - cf->data[2] |= CAN_ERR_PROT_FORM;
> - break;
> - case PCH_ACK_ERR:
> - cf->data[2] |= CAN_ERR_PROT_LOC_ACK |
> - CAN_ERR_PROT_LOC_ACK_DEL;
> - break;
> - case PCH_BIT1_ERR:
> - case PCH_BIT0_ERR:
> - cf->data[2] |= CAN_ERR_PROT_BIT;
> - break;
> - case PCH_CRC_ERR:
> - cf->data[2] |= CAN_ERR_PROT_LOC_CRC_SEQ |
> - CAN_ERR_PROT_LOC_CRC_DEL;
> - break;
> - default:
> - iowrite32(status | PCH_LEC_ALL, &priv->regs->stat);
> - break;
> - }
> -
> + break;
> + case PCH_FORM_ERR:
> + cf->data[2] |= CAN_ERR_PROT_FORM;
> + priv->can.can_stats.bus_error++;
> + stats->rx_errors++;
> + break;
> + case PCH_ACK_ERR:
> + cf->can_id |= CAN_ERR_ACK;
> + priv->can.can_stats.bus_error++;
> + stats->rx_errors++;
> + break;
> + case PCH_BIT1_ERR:
> + case PCH_BIT0_ERR:
> + cf->data[2] |= CAN_ERR_PROT_BIT;
> + priv->can.can_stats.bus_error++;
> + stats->rx_errors++;
> + break;
> + case PCH_CRC_ERR:
> + cf->data[2] |= CAN_ERR_PROT_LOC_CRC_SEQ |
> + CAN_ERR_PROT_LOC_CRC_DEL;
> + priv->can.can_stats.bus_error++;
> + stats->rx_errors++;
> + break;
> + case PCH_LEC_ALL: /* Written by CPU. No error status */
> + break;
> }
>
> + cf->data[6] = errc & PCH_TEC;
> + cf->data[7] = (errc & PCH_REC) >> 8;
> +
You patch description doesn't say anything about this
> priv->can.state = state;
> netif_rx(skb);
>
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
[-- Attachment #2: Type: text/plain, Size: 188 bytes --]
_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core
^ permalink raw reply
* Re: [PATCH net-next-2.6 6/17 v3] can: EG20T PCH: Fix endianness issue
From: Marc Kleine-Budde @ 2010-11-24 13:31 UTC (permalink / raw)
To: Tomoya MORINAGA
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
margie.foster-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Wolfgang Grandegger,
joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CED023E.1070009-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 16311 bytes --]
On 11/24/2010 01:17 PM, Tomoya MORINAGA wrote:
> Fix endianness issue.
> there is endianness issue both Tx and Rx.
> Currently, data is set like below.
> Register:
> MSB--LSB
> x x D0 D1
> x x D2 D3
> x x D4 D5
> x x D6 D7
>
> But Data to be sent must be set like below.
> Register:
> MSB--LSB
> x x D1 D0
> x x D3 D2
> x x D5 D4
> x x D7 D6 (x means reserved area.)
The endianess handling looks good now! (the & 0xff can be removed,
though, see linline)
>
>
> Modify netif_rx() to netif_receive_skb().
Please give a reason, I suppose because the function is used from NAPI.
(Frankly speaking this has nothing to do with endianess issues).
> For easy to read, some sub-functions are created.
>
>
> Modify complex "goto" to do~while.
What about the next_flag in the rx_normal function? Can you get rid of
it, too?
>
> Signed-off-by: Tomoya MORINAGA <tomoya-linux-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
> ---
> From 468cf576aadc0ce26496eaf13fbac4bb2670a9de Mon Sep 17 00:00:00 2001
> From: Tomoya MORINAGA <tomoya-linux-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
> Date: Thu, 18 Nov 2010 10:23:32 +0900
> Subject: [PATCH] CAN : Improve RxTx processing
>
> Signed-off-by: Tomoya MORINAGA <tomoya-linux-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
> ---
> drivers/net/can/pch_can.c | 306
> +++++++++++++++++++++++----------------------
> 1 files changed, 156 insertions(+), 150 deletions(-)
>
> diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
> index 843bbce..fa47707 100644
> --- a/drivers/net/can/pch_can.c
> +++ b/drivers/net/can/pch_can.c
> @@ -133,10 +133,7 @@ struct pch_can_if_regs {
> u32 id1;
> u32 id2;
> u32 mcont;
> - u32 dataa1;
> - u32 dataa2;
> - u32 datab1;
> - u32 datab2;
> + u32 data[4];
> u32 rsv[13];
> };
>
> @@ -421,10 +418,10 @@ static void pch_can_clear_buffers(struct
> pch_can_priv *priv)
> iowrite32(0x0, &priv->regs->ifregs[0].id1);
> iowrite32(0x0, &priv->regs->ifregs[0].id2);
> iowrite32(0x0, &priv->regs->ifregs[0].mcont);
> - iowrite32(0x0, &priv->regs->ifregs[0].dataa1);
> - iowrite32(0x0, &priv->regs->ifregs[0].dataa2);
> - iowrite32(0x0, &priv->regs->ifregs[0].datab1);
> - iowrite32(0x0, &priv->regs->ifregs[0].datab2);
> + iowrite32(0x0, &priv->regs->ifregs[0].data[0]);
> + iowrite32(0x0, &priv->regs->ifregs[0].data[1]);
> + iowrite32(0x0, &priv->regs->ifregs[0].data[2]);
> + iowrite32(0x0, &priv->regs->ifregs[0].data[3]);
> iowrite32(PCH_CMASK_RDWR | PCH_CMASK_MASK |
> PCH_CMASK_ARB | PCH_CMASK_CTRL,
> &priv->regs->ifregs[0].cmask);
> @@ -438,10 +435,10 @@ static void pch_can_clear_buffers(struct
> pch_can_priv *priv)
> iowrite32(0x0, &priv->regs->ifregs[1].id1);
> iowrite32(0x0, &priv->regs->ifregs[1].id2);
> iowrite32(0x0, &priv->regs->ifregs[1].mcont);
> - iowrite32(0x0, &priv->regs->ifregs[1].dataa1);
> - iowrite32(0x0, &priv->regs->ifregs[1].dataa2);
> - iowrite32(0x0, &priv->regs->ifregs[1].datab1);
> - iowrite32(0x0, &priv->regs->ifregs[1].datab2);
> + iowrite32(0x0, &priv->regs->ifregs[1].data[0]);
> + iowrite32(0x0, &priv->regs->ifregs[1].data[1]);
> + iowrite32(0x0, &priv->regs->ifregs[1].data[2]);
> + iowrite32(0x0, &priv->regs->ifregs[1].data[3]);
> iowrite32(PCH_CMASK_RDWR | PCH_CMASK_MASK |
> PCH_CMASK_ARB | PCH_CMASK_CTRL,
> &priv->regs->ifregs[1].cmask);
> @@ -683,7 +680,7 @@ static void pch_can_error(struct net_device *ndev,
> u32 status)
> cf->data[7] = (errc & PCH_REC) >> 8;
>
> priv->can.state = state;
> - netif_rx(skb);
> + netif_receive_skb(skb);
This is an unrelated fix. Please split into separate patch.
>
> stats->rx_packets++;
> stats->rx_bytes += cf->can_dlc;
> @@ -701,190 +698,202 @@ static irqreturn_t pch_can_interrupt(int irq,
> void *dev_id)
> return IRQ_HANDLED;
> }
>
> -static int pch_can_rx_normal(struct net_device *ndev, u32 int_stat)
> +static void pch_fifo_thresh(struct pch_can_priv *priv, int obj_id)
> +{
> + if (obj_id < PCH_FIFO_THRESH) {
> + iowrite32(PCH_CMASK_RDWR | PCH_CMASK_CTRL |
> + PCH_CMASK_ARB, &priv->regs->ifregs[0].cmask);
> +
> + /* Clearing the Dir bit. */
> + pch_can_bit_clear(&priv->regs->ifregs[0].id2, PCH_ID2_DIR);
> +
> + /* Clearing NewDat & IntPnd */
> + pch_can_bit_clear(&priv->regs->ifregs[0].mcont,
> + PCH_IF_MCONT_INTPND);
> + pch_can_check_if_busy(&priv->regs->ifregs[0].creq, obj_id);
> + } else if (obj_id > PCH_FIFO_THRESH) {
> + pch_can_int_clr(priv, obj_id);
> + } else if (obj_id == PCH_FIFO_THRESH) {
> + int cnt;
> + for (cnt = 0; cnt < PCH_FIFO_THRESH; cnt++)
> + pch_can_int_clr(priv, cnt+1);
^^^^^
nitpick: there should be spaces around the "+".
> + }
> +}
> +
> +static int pch_can_rx_msg_lost(struct net_device *ndev, int obj_id)
> +{
> + struct pch_can_priv *priv = netdev_priv(ndev);
> + struct net_device_stats *stats = &(priv->ndev->stats);
> + struct sk_buff *skb;
> + struct can_frame *cf;
> +
> + netdev_dbg(priv->ndev, "Msg Obj is overwritten.\n");
> + pch_can_bit_clear(&priv->regs->ifregs[0].mcont,
> + PCH_IF_MCONT_MSGLOST);
> + iowrite32(PCH_CMASK_RDWR | PCH_CMASK_CTRL,
> + &priv->regs->ifregs[0].cmask);
> + pch_can_check_if_busy(&priv->regs->ifregs[0].creq, obj_id);
> +
> + skb = alloc_can_err_skb(ndev, &cf);
> + if (!skb)
> + return -ENOMEM;
> +
> + cf->can_id |= CAN_ERR_CRTL;
> + cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
> + stats->rx_over_errors++;
> + stats->rx_errors++;
> +
> + netif_receive_skb(skb);
> +
> + return 0;
> +}
> +
> +static int pch_can_rx_normal(struct net_device *ndev, u32 obj_num, int
> quota)
> {
> u32 reg;
> canid_t id;
> - u32 ide;
> - u32 rtr;
> - int i, j, k;
> int rcv_pkts = 0;
> + int rtn;
> + int next_flag = 0;
> struct sk_buff *skb;
> struct can_frame *cf;
> struct pch_can_priv *priv = netdev_priv(ndev);
> struct net_device_stats *stats = &(priv->ndev->stats);
> + int i;
> + u32 id2;
> + u16 data_reg;
>
> - /* Reading the messsage object from the Message RAM */
> - iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[0].cmask);
> - pch_can_check_if_busy(&priv->regs->ifregs[0].creq, int_stat);
> + do {
> + /* Reading the messsage object from the Message RAM */
> + iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[0].cmask);
> + pch_can_check_if_busy(&priv->regs->ifregs[0].creq, obj_num);
>
> - /* Reading the MCONT register. */
> - reg = ioread32(&priv->regs->ifregs[0].mcont);
> - reg &= 0xffff;
> + /* Reading the MCONT register. */
> + reg = ioread32(&priv->regs->ifregs[0].mcont);
> +
> + if (reg & PCH_IF_MCONT_EOB)
> + break;
>
> - for (k = int_stat; !(reg & PCH_IF_MCONT_EOB); k++) {
> /* If MsgLost bit set. */
> if (reg & PCH_IF_MCONT_MSGLOST) {
> - dev_err(&priv->ndev->dev, "Msg Obj is overwritten.\n");
> - pch_can_bit_clear(&priv->regs->ifregs[0].mcont,
> - PCH_IF_MCONT_MSGLOST);
> - iowrite32(PCH_CMASK_RDWR | PCH_CMASK_CTRL,
> - &priv->regs->ifregs[0].cmask);
> - pch_can_check_if_busy(&priv->regs->ifregs[0].creq, k);
> -
> - skb = alloc_can_err_skb(ndev, &cf);
> + rtn = pch_can_rx_msg_lost(ndev, obj_num);
> + if (!rtn)
> + return rtn;
The function return "0" on success or -ENOMEM. The logic seems inverted.
IMHO just keep going if you had an OOM situation.
> + rcv_pkts++;
> + quota--;
> + next_flag = 1;
> + } else if (!(reg & PCH_IF_MCONT_NEWDAT))
> + next_flag = 1;
Please get rid of the next_flag, rearrange your code and use "continue".
> + if (!next_flag) {
> + skb = alloc_can_skb(priv->ndev, &cf);
> if (!skb)
> return -ENOMEM;
>
> - priv->can.can_stats.error_passive++;
> - priv->can.state = CAN_STATE_ERROR_PASSIVE;
> - cf->can_id |= CAN_ERR_CRTL;
> - cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
> - cf->data[2] |= CAN_ERR_PROT_OVERLOAD;
> - stats->rx_packets++;
> - stats->rx_bytes += cf->can_dlc;
> + /* Get Received data */
> + id2 = ioread32(&priv->regs->ifregs[0].id2);
> + if (id2 & PCH_ID2_XTD) {
> + id = (ioread32(&priv->regs->ifregs[0].id1) &
> + 0xffff);
> + id |= (((id2) & 0x1fff) << 16);
> + cf->can_id = id | CAN_EFF_FLAG;
> + } else {
> + id = ((id2 & (CAN_SFF_MASK << 2)) >> 2);
> + cf->can_id = id;
> + }
> +
> + if (id2 & PCH_ID2_DIR)
> + cf->can_id |= CAN_RTR_FLAG;
> +
> + cf->can_dlc = get_can_dlc((ioread32(&priv->regs->
> + ifregs[0].mcont)) & 0xF);
> +
> + for (i = 0; i < cf->can_dlc; i += 2) {
> + data_reg = ioread16(&priv->regs->ifregs[0].
> + data[i / 2]);
> + cf->data[i] = data_reg & 0xff;
> + cf->data[i + 1] = (data_reg >> 8) & 0xff;
the & 0xff are not needed, cf->data[] is 8bit only.
> + }
>
> netif_receive_skb(skb);
> rcv_pkts++;
> - goto RX_NEXT;
> - }
> - if (!(reg & PCH_IF_MCONT_NEWDAT))
> - goto RX_NEXT;
> -
> - skb = alloc_can_skb(priv->ndev, &cf);
> - if (!skb)
> - return -ENOMEM;
> -
> - /* Get Received data */
> - ide = ((ioread32(&priv->regs->ifregs[0].id2)) & PCH_ID2_XTD) >>
> - 14;
> - if (ide) {
> - id = (ioread32(&priv->regs->ifregs[0].id1) & 0xffff);
> - id |= (((ioread32(&priv->regs->ifregs[0].id2)) &
> - 0x1fff) << 16);
> - cf->can_id = (id & CAN_EFF_MASK) | CAN_EFF_FLAG;
> - } else {
> - id = (((ioread32(&priv->regs->ifregs[0].id2)) &
> - (CAN_SFF_MASK << 2)) >> 2);
> - cf->can_id = (id & CAN_SFF_MASK);
> - }
> + stats->rx_packets++;
> + quota--;
> + stats->rx_bytes += cf->can_dlc;
>
> - rtr = (ioread32(&priv->regs->ifregs[0].id2) & PCH_ID2_DIR);
> - if (rtr) {
> - cf->can_dlc = 0;
> - cf->can_id |= CAN_RTR_FLAG;
> - } else {
> - cf->can_dlc =
> - ((ioread32(&priv->regs->ifregs[0].mcont)) & 0x0f);
> + pch_fifo_thresh(priv, obj_num);
> }
> + obj_num++;
> + next_flag = 0;
> + } while (quota > 0);
>
> - for (i = 0, j = 0; i < cf->can_dlc; j++) {
> - reg = ioread32(&priv->regs->ifregs[0].dataa1 + j*4);
> - cf->data[i++] = cpu_to_le32(reg & 0xff);
> - if (i == cf->can_dlc)
> - break;
> - cf->data[i++] = cpu_to_le32((reg >> 8) & 0xff);
> - }
> + return rcv_pkts;
> +}
>
> - netif_receive_skb(skb);
> - rcv_pkts++;
> - stats->rx_packets++;
> - stats->rx_bytes += cf->can_dlc;
> -
> - if (k < PCH_FIFO_THRESH) {
> - iowrite32(PCH_CMASK_RDWR | PCH_CMASK_CTRL |
> - PCH_CMASK_ARB, &priv->regs->ifregs[0].cmask);
> -
> - /* Clearing the Dir bit. */
> - pch_can_bit_clear(&priv->regs->ifregs[0].id2,
> - PCH_ID2_DIR);
> -
> - /* Clearing NewDat & IntPnd */
> - pch_can_bit_clear(&priv->regs->ifregs[0].mcont,
> - PCH_IF_MCONT_INTPND);
> - pch_can_check_if_busy(&priv->regs->ifregs[0].creq, k);
> - } else if (k > PCH_FIFO_THRESH) {
> - pch_can_int_clr(priv, k);
> - } else if (k == PCH_FIFO_THRESH) {
> - int cnt;
> - for (cnt = 0; cnt < PCH_FIFO_THRESH; cnt++)
> - pch_can_int_clr(priv, cnt+1);
> - }
> -RX_NEXT:
> - /* Reading the messsage object from the Message RAM */
> - iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[0].cmask);
> - pch_can_check_if_busy(&priv->regs->ifregs[0].creq, k);
> - reg = ioread32(&priv->regs->ifregs[0].mcont);
> - }
> +static void pch_can_tx_complete(struct net_device *ndev, u32 int_stat)
> +{
> + struct pch_can_priv *priv = netdev_priv(ndev);
> + struct net_device_stats *stats = &(priv->ndev->stats);
> + u32 dlc;
>
> - return rcv_pkts;
> + can_get_echo_skb(ndev, int_stat - PCH_RX_OBJ_END - 1);
> + iowrite32(PCH_CMASK_RX_TX_GET | PCH_CMASK_CLRINTPND,
> + &priv->regs->ifregs[1].cmask);
> + dlc = get_can_dlc(ioread32(&priv->regs->ifregs[1].mcont) &
> + PCH_IF_MCONT_DLC);
> + pch_can_check_if_busy(&priv->regs->ifregs[1].creq, int_stat);
This is the TX-Complete interrupt handler, right? What does the
pch_can_check_if_busy trigger here?
> + stats->tx_bytes += dlc;
> + stats->tx_packets++;
> + if (int_stat == PCH_TX_OBJ_END)
> + netif_wake_queue(ndev);
> }
> +
> static int pch_can_rx_poll(struct napi_struct *napi, int quota)
> {
> struct net_device *ndev = napi->dev;
> struct pch_can_priv *priv = netdev_priv(ndev);
> - struct net_device_stats *stats = &(priv->ndev->stats);
> - u32 dlc;
> u32 int_stat;
> int rcv_pkts = 0;
> u32 reg_stat;
>
> int_stat = pch_can_int_pending(priv);
> if (!int_stat)
> - return 0;
> + goto end;
>
> -INT_STAT:
> - if (int_stat == PCH_STATUS_INT) {
> + if ((int_stat == PCH_STATUS_INT) && (quota > 0)) {
> reg_stat = ioread32(&priv->regs->stat);
> if (reg_stat & (PCH_BUS_OFF | PCH_LEC_ALL)) {
> - if ((reg_stat & PCH_LEC_ALL) != PCH_LEC_ALL)
> + if (reg_stat & PCH_BUS_OFF ||
> + (reg_stat & PCH_LEC_ALL) != PCH_LEC_ALL) {
> pch_can_error(ndev, reg_stat);
> + quota--;
> + }
> }
>
> - if (reg_stat & PCH_TX_OK) {
> - iowrite32(PCH_CMASK_RX_TX_GET,
> - &priv->regs->ifregs[1].cmask);
> - pch_can_check_if_busy(&priv->regs->ifregs[1].creq,
> - ioread32(&priv->regs->intr));
> + if (reg_stat & PCH_TX_OK)
> pch_can_bit_clear(&priv->regs->stat, PCH_TX_OK);
> - }
>
> if (reg_stat & PCH_RX_OK)
> pch_can_bit_clear(&priv->regs->stat, PCH_RX_OK);
>
> int_stat = pch_can_int_pending(priv);
> - if (int_stat == PCH_STATUS_INT)
> - goto INT_STAT;
> }
>
> -MSG_OBJ:
> + if (quota == 0)
> + goto end;
> +
> if ((int_stat >= PCH_RX_OBJ_START) && (int_stat <= PCH_RX_OBJ_END)) {
> - rcv_pkts = pch_can_rx_normal(ndev, int_stat);
> - if (rcv_pkts < 0)
> - return 0;
> + rcv_pkts += pch_can_rx_normal(ndev, int_stat, quota);
> + quota -= rcv_pkts;
> + if (quota < 0)
> + goto end;
> } else if ((int_stat >= PCH_TX_OBJ_START) &&
> (int_stat <= PCH_TX_OBJ_END)) {
> /* Handle transmission interrupt */
> - can_get_echo_skb(ndev, int_stat - PCH_RX_OBJ_END - 1);
> - iowrite32(PCH_CMASK_RX_TX_GET | PCH_CMASK_CLRINTPND,
> - &priv->regs->ifregs[1].cmask);
> - dlc = ioread32(&priv->regs->ifregs[1].mcont) &
> - PCH_IF_MCONT_DLC;
> - pch_can_check_if_busy(&priv->regs->ifregs[1].creq, int_stat);
> - if (dlc > 8)
> - dlc = 8;
> - stats->tx_bytes += dlc;
> - stats->tx_packets++;
> - if (int_stat == PCH_TX_OBJ_END)
> - netif_wake_queue(ndev);
> + pch_can_tx_complete(ndev, int_stat);
> }
>
> - int_stat = pch_can_int_pending(priv);
> - if (int_stat == PCH_STATUS_INT)
> - goto INT_STAT;
> - else if (int_stat >= 1 && int_stat <= 32)
> - goto MSG_OBJ;
> -
> +end:
> napi_complete(napi);
> pch_can_set_int_enables(priv, PCH_CAN_ALL);
>
> @@ -1017,10 +1026,10 @@ static int pch_close(struct net_device *ndev)
>
> static netdev_tx_t pch_xmit(struct sk_buff *skb, struct net_device *ndev)
> {
> - int i, j;
> struct pch_can_priv *priv = netdev_priv(ndev);
> struct can_frame *cf = (struct can_frame *)skb->data;
> int tx_obj_no = 0;
> + int i;
>
> if (can_dropped_invalid_skb(ndev, skb))
> return NETDEV_TX_OK;
> @@ -1061,13 +1070,10 @@ static netdev_tx_t pch_xmit(struct sk_buff *skb,
> struct net_device *ndev)
> if (cf->can_id & CAN_RTR_FLAG)
> pch_can_bit_clear(&priv->regs->ifregs[1].id2, PCH_ID2_DIR);
>
> - for (i = 0, j = 0; i < cf->can_dlc; j++) {
> - iowrite32(le32_to_cpu(cf->data[i++]),
> - (&priv->regs->ifregs[1].dataa1) + j*4);
> - if (i == cf->can_dlc)
> - break;
> - iowrite32(le32_to_cpu(cf->data[i++] << 8),
> - (&priv->regs->ifregs[1].dataa1) + j*4);
> + /* Copy data to register */
> + for (i = 0; i < cf->can_dlc; i += 2) {
> + iowrite16(cf->data[i] | (cf->data[i + 1] << 8),
> + &priv->regs->ifregs[1].data[i / 2]);
> }
>
> can_put_echo_skb(skb, ndev, tx_obj_no - PCH_RX_OBJ_END - 1);
cheers, Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
[-- Attachment #2: Type: text/plain, Size: 188 bytes --]
_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core
^ permalink raw reply
* Re: [PATCH net-next-2.6 8/17 v3] can: EG20T PCH: Change Copyright and module description
From: Marc Kleine-Budde @ 2010-11-24 13:35 UTC (permalink / raw)
To: Tomoya MORINAGA
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
margie.foster-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Wolfgang Grandegger,
joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CED02C5.4080504-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 526 bytes --]
On 11/24/2010 01:19 PM, Tomoya MORINAGA wrote:
> Currently, Copyright and module description are not formal.
As David pointed out in an older version of this series, this patch does
more than the subject indicates. Please fix.
cheers, Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
[-- Attachment #2: Type: text/plain, Size: 188 bytes --]
_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core
^ permalink raw reply
* Re: [PATCH net-next-2.6 16/17 v3] can: EG20T PCH: Fix incorrect return processing
From: Marc Kleine-Budde @ 2010-11-24 13:39 UTC (permalink / raw)
To: Tomoya MORINAGA
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
margie.foster-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Wolfgang Grandegger,
joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CED0412.60305-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 2831 bytes --]
On 11/24/2010 01:24 PM, Tomoya MORINAGA wrote:
> Fix incorrect return processing
see comments inline
> Signed-off-by: Tomoya MORINAGA <tomoya-linux-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
> ---
> drivers/net/can/pch_can.c | 20 ++++++++++++--------
> 1 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
> index c612a99..48f4a2e 100644
> --- a/drivers/net/can/pch_can.c
> +++ b/drivers/net/can/pch_can.c
> @@ -589,10 +589,12 @@ static irqreturn_t pch_can_interrupt(int irq, void
> *dev_id)
> struct net_device *ndev = (struct net_device *)dev_id;
> struct pch_can_priv *priv = netdev_priv(ndev);
>
> - pch_can_set_int_enables(priv, PCH_CAN_NONE);
> - napi_schedule(&priv->napi);
> -
> - return IRQ_HANDLED;
> + if ((pch_can_int_pending(priv) > 0) && (dev_id != NULL)) {
> + pch_can_set_int_enables(priv, PCH_CAN_NONE);
> + napi_schedule(&priv->napi);
> + return IRQ_HANDLED;
> + }
> + return IRQ_NONE;
My comment from the first review still applied here:
dev_id is always != NULL, because you registered your IRQ handler with
it. (BTW: dev_id has already been dereferenced in netdev_priv(), so if
this code is executed, dev_if is != NULL)
Just write:
if (!pch_can_int_pending(priv))
return IRQ_NONE;
> }
>
> static void pch_fifo_thresh(struct pch_can_priv *priv, int obj_id)
> @@ -674,7 +676,7 @@ static int pch_can_rx_normal(struct net_device
> *ndev, u32 obj_num, int quota)
> if (reg & PCH_IF_MCONT_MSGLOST) {
> rtn = pch_can_rx_msg_lost(ndev, obj_num);
> if (!rtn)
> - return rtn;
> + return rcv_pkts;
> rcv_pkts++;
> quota--;
> obj_num++;
> @@ -777,10 +779,12 @@ static int pch_can_poll(struct napi_struct *napi,
> int quota)
> goto end;
>
> if ((int_stat >= PCH_RX_OBJ_START) && (int_stat <= PCH_RX_OBJ_END)) {
> - rcv_pkts += pch_can_rx_normal(ndev, int_stat, quota);
> - quota -= rcv_pkts;
> - if (quota < 0)
> + rcv_pkts = pch_can_rx_normal(ndev, int_stat, quota);
> + if (rcv_pkts < 0) {
> + rcv_pkts = 0;
> goto end;
> + }
> + quota -= rcv_pkts;
you introduced the problem in patch "[PATCH net-next-2.6 6/17 v3] can:
EG20T PCH: Fix endianness issue", please don't do this in the first place.
( This is an example why you should split your patches and give them
proper subject.)
> } else if ((int_stat >= PCH_TX_OBJ_START) &&
> (int_stat <= PCH_TX_OBJ_END)) {
> /* Handle transmission interrupt */
cheers, Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
[-- Attachment #2: Type: text/plain, Size: 188 bytes --]
_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core
^ permalink raw reply
* [PATCH net-next] bnx2x: Disable local BHes to prevent a dead-lock situation
From: Vladislav Zolotarov @ 2010-11-24 13:41 UTC (permalink / raw)
To: Dave Miller; +Cc: Eilon Greenstein, netdev list
From: Eric Dumazet <eric.dumazet@gmail.com>
Disable local BHes to prevent a dead-lock situation between sch_direct_xmit()
(Soft_IRQ context) and bnx2x_tx_int (called by bnx2x_run_loopback() - syscall
context), as both are taking a netif_tx_lock().
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Vladislav Zolotarov <vladz@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/bnx2x/bnx2x_ethtool.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/drivers/net/bnx2x/bnx2x_ethtool.c b/drivers/net/bnx2x/bnx2x_ethtool.c
index d02ffbd..0301278 100644
--- a/drivers/net/bnx2x/bnx2x_ethtool.c
+++ b/drivers/net/bnx2x/bnx2x_ethtool.c
@@ -1499,8 +1499,15 @@ static int bnx2x_run_loopback(struct bnx2x *bp, int loopback_mode, u8 link_up)
* updates that have been performed while interrupts were
* disabled.
*/
- if (bp->common.int_block == INT_BLOCK_IGU)
+ if (bp->common.int_block == INT_BLOCK_IGU) {
+ /* Disable local BHes to prevent a dead-lock situation between
+ * sch_direct_xmit() and bnx2x_run_loopback() (calling
+ * bnx2x_tx_int()), as both are taking netif_tx_lock().
+ */
+ local_bh_disable();
bnx2x_tx_int(fp_tx);
+ local_bh_enable();
+ }
rx_idx = le16_to_cpu(*fp_rx->rx_cons_sb);
if (rx_idx != rx_start_idx + num_pkts)
--
1.7.0.4
^ permalink raw reply related
* [PATCH net-next] bnx2x: Disable local BHes to prevent a dead-lock situation
From: Vladislav Zolotarov @ 2010-11-24 13:45 UTC (permalink / raw)
To: Dave Miller; +Cc: Eilon Greenstein, netdev list, Eric Dumazet
From: Eric Dumazet <eric.dumazet@gmail.com>
According to Eric's suggestion:
Disable local BHes to prevent a dead-lock situation between sch_direct_xmit()
(Soft_IRQ context) and bnx2x_tx_int (called by bnx2x_run_loopback() - syscall
context), as both are taking a netif_tx_lock().
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Vladislav Zolotarov <vladz@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
CCing Eric... :)
drivers/net/bnx2x/bnx2x_ethtool.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/drivers/net/bnx2x/bnx2x_ethtool.c b/drivers/net/bnx2x/bnx2x_ethtool.c
index d02ffbd..0301278 100644
--- a/drivers/net/bnx2x/bnx2x_ethtool.c
+++ b/drivers/net/bnx2x/bnx2x_ethtool.c
@@ -1499,8 +1499,15 @@ static int bnx2x_run_loopback(struct bnx2x *bp, int loopback_mode, u8 link_up)
* updates that have been performed while interrupts were
* disabled.
*/
- if (bp->common.int_block == INT_BLOCK_IGU)
+ if (bp->common.int_block == INT_BLOCK_IGU) {
+ /* Disable local BHes to prevent a dead-lock situation between
+ * sch_direct_xmit() and bnx2x_run_loopback() (calling
+ * bnx2x_tx_int()), as both are taking netif_tx_lock().
+ */
+ local_bh_disable();
bnx2x_tx_int(fp_tx);
+ local_bh_enable();
+ }
rx_idx = le16_to_cpu(*fp_rx->rx_cons_sb);
if (rx_idx != rx_start_idx + num_pkts)
--
1.7.0.4
^ permalink raw reply related
* [PATCH] infiniband: remove dev_base_lock use
From: Eric Dumazet @ 2010-11-24 13:47 UTC (permalink / raw)
To: David Miller
Cc: netdev, Roland Dreier, Sean Hefty, Hal Rosenstock,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1288350226.2560.11.camel@edumazet-laptop>
Le vendredi 29 octobre 2010 à 13:03 +0200, Eric Dumazet a écrit :
> dev_base_lock is the legacy way to lock the device list, and is planned
> to disappear. (writers hold RTNL, readers hold RCU lock)
>
> Convert rdma_translate_ip() to RCU locking.
>
> Signed-off-by: Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> drivers/infiniband/core/addr.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
> index a5ea1bc..309b477 100644
> --- a/drivers/infiniband/core/addr.c
> +++ b/drivers/infiniband/core/addr.c
> @@ -130,8 +130,8 @@ int rdma_translate_ip(struct sockaddr *addr, struct rdma_dev_addr *dev_addr)
>
> #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
> case AF_INET6:
> - read_lock(&dev_base_lock);
> - for_each_netdev(&init_net, dev) {
> + rcu_read_lock();
> + for_each_netdev_rcu(&init_net, dev) {
> if (ipv6_chk_addr(&init_net,
> &((struct sockaddr_in6 *) addr)->sin6_addr,
> dev, 1)) {
> @@ -139,7 +139,7 @@ int rdma_translate_ip(struct sockaddr *addr, struct rdma_dev_addr *dev_addr)
> break;
> }
> }
> - read_unlock(&dev_base_lock);
> + rcu_read_unlock();
> break;
> #endif
> }
>
Here is a new version of patch.
David, it seems no infiniband guy commented this patch, could you take
it in your tree ?
Thanks
[PATCH] infiniband: remove dev_base_lock use
dev_base_lock is the legacy way to lock the device list, and is planned
to disappear. (writers hold RTNL, readers hold RCU lock)
Convert rdma_translate_ip() and update_ipv6_gids() to RCU locking.
Signed-off-by: Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/infiniband/core/addr.c | 6 +++---
drivers/infiniband/hw/mlx4/main.c | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index c15fd2e..8aba0ba 100644
--- a/drivers/infiniband/core/addr.c
+++ b/drivers/infiniband/core/addr.c
@@ -130,8 +130,8 @@ int rdma_translate_ip(struct sockaddr *addr, struct rdma_dev_addr *dev_addr)
#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
case AF_INET6:
- read_lock(&dev_base_lock);
- for_each_netdev(&init_net, dev) {
+ rcu_read_lock();
+ for_each_netdev_rcu(&init_net, dev) {
if (ipv6_chk_addr(&init_net,
&((struct sockaddr_in6 *) addr)->sin6_addr,
dev, 1)) {
@@ -139,7 +139,7 @@ int rdma_translate_ip(struct sockaddr *addr, struct rdma_dev_addr *dev_addr)
break;
}
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
break;
#endif
}
diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index bf3e20c..4e55a28 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -848,8 +848,8 @@ static int update_ipv6_gids(struct mlx4_ib_dev *dev, int port, int clear)
goto out;
}
- read_lock(&dev_base_lock);
- for_each_netdev(&init_net, tmp) {
+ rcu_read_lock();
+ for_each_netdev_rcu(&init_net, tmp) {
if (ndev && (tmp == ndev || rdma_vlan_dev_real_dev(tmp) == ndev)) {
gid.global.subnet_prefix = cpu_to_be64(0xfe80000000000000LL);
vid = rdma_vlan_dev_vlan_id(tmp);
@@ -884,7 +884,7 @@ static int update_ipv6_gids(struct mlx4_ib_dev *dev, int port, int clear)
}
}
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
for (i = 0; i < 128; ++i)
if (!hits[i]) {
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* RE: [PATCH 1/2] usbnet: changes for upcoming cdc_ncm driver
From: Alexey ORISHKO @ 2010-11-24 13:53 UTC (permalink / raw)
To: Viral Mehta, gregkh-l3A5Bk7waGM@public.gmane.org,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org,
yauheni.kaliuta-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org,
balbi-l0cyMroinI0@public.gmane.org, Sjur BRENDELAND,
Alexey ORISHKO
In-Reply-To: <D69C90565D53114396BF743585AF5A09122E61E8A3-fVvhrSDAkVjuuPCCJ6VnObSn4PsL5ZDKvpI+GvaZM4lBDgjK7y7TUQ@public.gmane.org>
> -----Original Message-----
> From: Viral Mehta [mailto:Viral.Mehta-oonXi/34qI7c+919tysfdA@public.gmane.org]
> Sent: 23 ноября 2010 г. 18:40
> To: Alexey Orishko; gregkh-l3A5Bk7waGM@public.gmane.org; linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org;
> yauheni.kaliuta-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org; felipe.balbi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org; Sjur BRENDELAND;
> Alexey ORISHKO
> Subject: RE: [PATCH 1/2] usbnet: changes for upcoming cdc_ncm driver
>
> Hi,
>
> >Signed-off-by: Alexey Orishko <alexey.orishko-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org>
> >---
> >Changes:
> >drivers/net/usb/usbnet.c:
> >- the procedure of counting packets in usbnet was updated due to the
> >accumulating of IP packets in the CDC NCM driver
> >- no short packets sent for cdc_ncm driver
> >include/linux/usb/usbnet.h:
> >- a new flag to indicate driver's capability to accumulate IP packets
> in one
> >skb before sending them (Tx direction)
>
> Feels like good to have these comments in comment log itself (i..e,
> above Signed-off-by line)
> Same for another patch as well.
No problem, I will repost it with comments above signed-off line
>
> Viral
>
> This Email may contain confidential or privileged information for the
> intended recipient (s) If you are not the intended recipient, please do
> not use or disseminate the information, notify the sender and delete it
> from your system.
>
> ______________________________________________________________________
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net-next-2.6 10/17 v3] can: EG20T PCH: Fix coding rule violation
From: Marc Kleine-Budde @ 2010-11-24 13:54 UTC (permalink / raw)
To: Tomoya MORINAGA
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
margie.foster-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Wolfgang Grandegger,
joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CED031E.7030403-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 6441 bytes --]
On 11/24/2010 01:20 PM, Tomoya MORINAGA wrote:
> Fix coding rule violation.
>
> Signed-off-by: Tomoya MORINAGA <tomoya-linux-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
Fix my comment (see online) , check if the lines stay (mostly) <= 80
chars and add my Acked-by.
cheers, Marc
> ---
> drivers/net/can/pch_can.c | 48
> ++++++++++++++++++++------------------------
> 1 files changed, 22 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
> index e71817d..318eb1f 100644
> --- a/drivers/net/can/pch_can.c
> +++ b/drivers/net/can/pch_can.c
> @@ -89,9 +89,11 @@
>
> #define PCH_CAN_CLK 50000000 /* 50MHz */
>
> -/* Define the number of message object.
> +/*
> + * Define the number of message object.
> * PCH CAN communications are done via Message RAM.
> - * The Message RAM consists of 32 message objects. */
> + * The Message RAM consists of 32 message objects.
> + */
> #define PCH_RX_OBJ_NUM 26
> #define PCH_TX_OBJ_NUM 6
> #define PCH_RX_OBJ_START 1
> @@ -126,7 +128,7 @@ enum pch_can_mode {
> PCH_CAN_ALL,
> PCH_CAN_NONE,
> PCH_CAN_STOP,
> - PCH_CAN_RUN
> + PCH_CAN_RUN,
> };
>
> struct pch_can_if_regs {
> @@ -290,21 +292,20 @@ static void pch_can_set_rxtx(struct pch_can_priv
> *priv, u32 buff_num,
> else
> ie = PCH_IF_MCONT_RXIE;
>
> - /* Reading the receive buffer data from RAM to Interface1 registers */
> + /* Reading the receive buffer data from RAM to Interface1/2 registers */
> iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[dir].cmask);
> pch_can_rw_msg_obj(&priv->regs->ifregs[dir].creq, buff_num);
>
> - /* Setting the IF1MASK1 register to access MsgVal and RxIE bits */
> + /* Setting the IF1/2MASK1 register to access MsgVal and RxIE bits */
> iowrite32(PCH_CMASK_RDWR | PCH_CMASK_ARB | PCH_CMASK_CTRL,
> &priv->regs->ifregs[dir].cmask);
>
> if (set) {
> - /* Setting the MsgVal and RxIE bits */
> + /* Setting the MsgVal and RxIE/TxIE bits */
> pch_can_bit_set(&priv->regs->ifregs[dir].mcont, ie);
> pch_can_bit_set(&priv->regs->ifregs[dir].id2, PCH_ID_MSGVAL);
> -
> } else {
> - /* Resetting the MsgVal and RxIE bits */
> + /* Clearing the MsgVal and RxIE/TxIE bits */
> pch_can_bit_clear(&priv->regs->ifregs[dir].mcont, ie);
> pch_can_bit_clear(&priv->regs->ifregs[dir].id2, PCH_ID_MSGVAL);
> }
> @@ -312,7 +313,6 @@ static void pch_can_set_rxtx(struct pch_can_priv
> *priv, u32 buff_num,
> pch_can_rw_msg_obj(&priv->regs->ifregs[dir].creq, buff_num);
> }
>
> -
> static void pch_can_set_rx_all(struct pch_can_priv *priv, int set)
> {
> int i;
> @@ -328,7 +328,7 @@ static void pch_can_set_tx_all(struct pch_can_priv
> *priv, int set)
>
> /* Traversing to obtain the object configured as transmit object. */
> for (i = PCH_TX_OBJ_START; i <= PCH_TX_OBJ_END; i++)
> - pch_can_set_rxtx(priv, i, set, 1);
> + pch_can_set_rxtx(priv, i, set, PCH_TX_IFREG);
this should be folded into the patch 1 (along with the introduction of
the enums)
> }
>
> static u32 pch_can_int_pending(struct pch_can_priv *priv)
> @@ -363,8 +363,7 @@ static void pch_can_config_rx_tx_buffers(struct
> pch_can_priv *priv)
> int i;
>
> for (i = PCH_RX_OBJ_START; i <= PCH_RX_OBJ_END; i++) {
> - iowrite32(PCH_CMASK_RX_TX_GET,
> - &priv->regs->ifregs[0].cmask);
> + iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[0].cmask);
> pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, i);
>
> iowrite32(0x0, &priv->regs->ifregs[0].id1);
> @@ -386,16 +385,14 @@ static void pch_can_config_rx_tx_buffers(struct
> pch_can_priv *priv)
> 0x1fff | PCH_MASK2_MDIR_MXTD);
>
> /* Setting CMASK for writing */
> - iowrite32(PCH_CMASK_RDWR | PCH_CMASK_MASK |
> - PCH_CMASK_ARB | PCH_CMASK_CTRL,
> - &priv->regs->ifregs[0].cmask);
> + iowrite32(PCH_CMASK_RDWR | PCH_CMASK_MASK | PCH_CMASK_ARB |
> + PCH_CMASK_CTRL, &priv->regs->ifregs[0].cmask);
>
> pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, i);
> }
>
> for (i = PCH_TX_OBJ_START; i <= PCH_TX_OBJ_END; i++) {
> - iowrite32(PCH_CMASK_RX_TX_GET,
> - &priv->regs->ifregs[1].cmask);
> + iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[1].cmask);
> pch_can_rw_msg_obj(&priv->regs->ifregs[1].creq, i);
>
> /* Resetting DIR bit for reception */
> @@ -410,9 +407,8 @@ static void pch_can_config_rx_tx_buffers(struct
> pch_can_priv *priv)
> pch_can_bit_clear(&priv->regs->ifregs[1].mask2, 0x1fff);
>
> /* Setting CMASK for writing */
> - iowrite32(PCH_CMASK_RDWR | PCH_CMASK_MASK |
> - PCH_CMASK_ARB | PCH_CMASK_CTRL,
> - &priv->regs->ifregs[1].cmask);
> + iowrite32(PCH_CMASK_RDWR | PCH_CMASK_MASK | PCH_CMASK_ARB |
> + PCH_CMASK_CTRL, &priv->regs->ifregs[1].cmask);
>
> pch_can_rw_msg_obj(&priv->regs->ifregs[1].creq, i);
> }
> @@ -471,8 +467,9 @@ static void pch_can_int_clr(struct pch_can_priv
> *priv, u32 mask)
>
> pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, mask);
> } else if ((mask >= PCH_TX_OBJ_START) && (mask <= PCH_TX_OBJ_END)) {
> - /* Setting CMASK for clearing interrupts for
> - frame transmission. */
> + /*
> + * Setting CMASK for clearing interrupts for frame transmission.
> + */
> iowrite32(PCH_CMASK_RDWR | PCH_CMASK_CTRL | PCH_CMASK_ARB,
> &priv->regs->ifregs[1].cmask);
>
> @@ -600,7 +597,6 @@ static irqreturn_t pch_can_interrupt(int irq, void
> *dev_id)
> struct pch_can_priv *priv = netdev_priv(ndev);
>
> pch_can_set_int_enables(priv, PCH_CAN_NONE);
> -
> napi_schedule(&priv->napi);
>
> return IRQ_HANDLED;
> @@ -1048,11 +1044,11 @@ static u32 pch_can_get_rxtx_ir(struct
> pch_can_priv *priv, u32 buff_num, u32 dir)
> pch_can_rw_msg_obj(&priv->regs->ifregs[dir].creq, buff_num);
>
> if (((ioread32(&priv->regs->ifregs[dir].id2)) & PCH_ID_MSGVAL) &&
> - ((ioread32(&priv->regs->ifregs[dir].mcont)) & ie)) {
> + ((ioread32(&priv->regs->ifregs[dir].mcont)) & ie))
> enable = 1;
> - } else {
> + else
> enable = 0;
> - }
> +
> return enable;
> }
>
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
[-- Attachment #2: Type: text/plain, Size: 188 bytes --]
_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core
^ permalink raw reply
* [PATCH 7/8] ethoc: rework mdio read/write
From: Jonas Bonn @ 2010-11-24 13:40 UTC (permalink / raw)
To: netdev; +Cc: Jonas Bonn
In-Reply-To: <1290606058-26703-1-git-send-email-jonas@southpole.se>
MDIO read and write were checking whether a timeout had expired to determine
whether to recheck the result of the MDIO operation. Under heavy CPU usage,
however, it was possible for the timeout to expire before the routine got
around to be able to check a second time even, thus erroneousy returning an
-EBUSY.
This patch changes the the MDIO IO routines to try up to five times to complete
the operation before giving up, thus lessening the dependency on CPU load.
This resolves a problem whereby a ping flood would keep the CPU so busy that
the above problem would manifest itself; the MDIO command to check link status
would fail and the interface would erroneously be shut down.
Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
drivers/net/ethoc.c | 14 ++++++--------
1 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index 70ca2f3..4f7c9ad 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -616,13 +616,13 @@ static int ethoc_poll(struct napi_struct *napi, int budget)
static int ethoc_mdio_read(struct mii_bus *bus, int phy, int reg)
{
- unsigned long timeout = jiffies + ETHOC_MII_TIMEOUT;
struct ethoc *priv = bus->priv;
+ int i;
ethoc_write(priv, MIIADDRESS, MIIADDRESS_ADDR(phy, reg));
ethoc_write(priv, MIICOMMAND, MIICOMMAND_READ);
- while (time_before(jiffies, timeout)) {
+ for (i=0; i < 5; i++) {
u32 status = ethoc_read(priv, MIISTATUS);
if (!(status & MIISTATUS_BUSY)) {
u32 data = ethoc_read(priv, MIIRX_DATA);
@@ -630,8 +630,7 @@ static int ethoc_mdio_read(struct mii_bus *bus, int phy, int reg)
ethoc_write(priv, MIICOMMAND, 0);
return data;
}
-
- schedule();
+ usleep_range(100,200);
}
return -EBUSY;
@@ -639,22 +638,21 @@ static int ethoc_mdio_read(struct mii_bus *bus, int phy, int reg)
static int ethoc_mdio_write(struct mii_bus *bus, int phy, int reg, u16 val)
{
- unsigned long timeout = jiffies + ETHOC_MII_TIMEOUT;
struct ethoc *priv = bus->priv;
+ int i;
ethoc_write(priv, MIIADDRESS, MIIADDRESS_ADDR(phy, reg));
ethoc_write(priv, MIITX_DATA, val);
ethoc_write(priv, MIICOMMAND, MIICOMMAND_WRITE);
- while (time_before(jiffies, timeout)) {
+ for (i=0; i < 5; i++) {
u32 stat = ethoc_read(priv, MIISTATUS);
if (!(stat & MIISTATUS_BUSY)) {
/* reset MII command register */
ethoc_write(priv, MIICOMMAND, 0);
return 0;
}
-
- schedule();
+ usleep_range(100,200);
}
return -EBUSY;
--
1.7.1
^ permalink raw reply related
* ethoc driver updates
From: Jonas Bonn @ 2010-11-24 13:40 UTC (permalink / raw)
To: netdev
Here's a set of patches that fix up some aspects of the ethoc network
driver:
i) device tree configuration
ii) reworked interrupt handling
iii) cleanups
This has been tested on an OpenRISC 1200 board; the OpenRISC Linux port
is using device trees now, hence the device tree bits.
/Jonas
^ permalink raw reply
* [PATCH 4/8] ethoc: prevent overflow of rx counter
From: Jonas Bonn @ 2010-11-24 13:40 UTC (permalink / raw)
To: netdev; +Cc: Jonas Bonn
In-Reply-To: <1290606058-26703-1-git-send-email-jonas@southpole.se>
Rewind cur_rx to prevent it from overflowing.
Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
drivers/net/ethoc.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index 53c03f2..7d1b5d8 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -408,6 +408,9 @@ static int ethoc_rx(struct net_device *dev, int limit)
struct ethoc *priv = netdev_priv(dev);
int count;
+ /* Prevent overflow of priv->cur_rx by rewinding it */
+ priv->cur_rx = priv->cur_rx % priv->num_rx;
+
for (count = 0; count < limit; ++count) {
unsigned int entry;
struct ethoc_bd bd;
--
1.7.1
^ permalink raw reply related
* [PATCH 6/8] ethoc: rework interrupt handling
From: Jonas Bonn @ 2010-11-24 13:40 UTC (permalink / raw)
To: netdev; +Cc: Jonas Bonn
In-Reply-To: <1290606058-26703-1-git-send-email-jonas@southpole.se>
The old interrupt handling was incorrect in that it did not account for the
fact that the interrupt source bits get set irregardless of whether or not
their corresponding mask is set. This patch fixes that by masking off the
source bits for masked interrupts.
Furthermore, the handling of transmission events is moved to the NAPI polling
handler alongside the reception handler, thus preventing a whole bunch of
interrupts during heavy traffic.
Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
drivers/net/ethoc.c | 79 +++++++++++++++++++++++++++++++++------------------
1 files changed, 51 insertions(+), 28 deletions(-)
diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index 5afa934..70ca2f3 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -499,29 +499,43 @@ static int ethoc_update_tx_stats(struct ethoc *dev, struct ethoc_bd *bd)
return 0;
}
-static void ethoc_tx(struct net_device *dev)
+static int ethoc_tx(struct net_device *dev, int limit)
{
struct ethoc *priv = netdev_priv(dev);
+ int count;
+ struct ethoc_bd bd;
- spin_lock(&priv->lock);
+ for (count = 0; count < limit; ++count) {
+ unsigned int entry;
- while (priv->dty_tx != priv->cur_tx) {
- unsigned int entry = priv->dty_tx % priv->num_tx;
- struct ethoc_bd bd;
+ entry = priv->dty_tx % priv->num_tx;
ethoc_read_bd(priv, entry, &bd);
- if (bd.stat & TX_BD_READY)
- break;
- entry = (++priv->dty_tx) % priv->num_tx;
+ if (bd.stat & TX_BD_READY || (priv->dty_tx == priv->cur_tx)) {
+ ethoc_ack_irq(priv, INT_MASK_TX);
+ /* If interrupt came in between reading in the BD
+ * and clearing the interrupt source, then we risk
+ * missing the event as the TX interrupt won't trigger
+ * right away when we reenable it; hence, check
+ * BD_EMPTY here again to make sure there isn't such an
+ * event pending...
+ */
+ ethoc_read_bd(priv, entry, &bd);
+ if (bd.stat & TX_BD_READY ||
+ (priv->dty_tx == priv->cur_tx))
+ break;
+ }
+
(void)ethoc_update_tx_stats(priv, &bd);
+ priv->dty_tx++;
}
- if ((priv->cur_tx - priv->dty_tx) <= (priv->num_tx / 2))
+ if ((priv->cur_tx - priv->dty_tx) <= (priv->num_tx / 2)) {
netif_wake_queue(dev);
+ }
- ethoc_ack_irq(priv, INT_MASK_TX);
- spin_unlock(&priv->lock);
+ return count;
}
static irqreturn_t ethoc_interrupt(int irq, void *dev_id)
@@ -529,32 +543,38 @@ static irqreturn_t ethoc_interrupt(int irq, void *dev_id)
struct net_device *dev = dev_id;
struct ethoc *priv = netdev_priv(dev);
u32 pending;
-
- ethoc_disable_irq(priv, INT_MASK_ALL);
+ u32 mask;
+
+ /* Figure out what triggered the interrupt...
+ * The tricky bit here is that the interrupt source bits get
+ * set in INT_SOURCE for an event irregardless of whether that
+ * event is masked or not. Thus, in order to figure out what
+ * triggered the interrupt, we need to remove the sources
+ * for all events that are currently masked. This behaviour
+ * is not particularly well documented but reasonable...
+ */
+ mask = ethoc_read(priv, INT_MASK);
pending = ethoc_read(priv, INT_SOURCE);
+ pending &= mask;
+
if (unlikely(pending == 0)) {
- ethoc_enable_irq(priv, INT_MASK_ALL);
return IRQ_NONE;
}
ethoc_ack_irq(priv, pending);
+ /* We always handle the dropped packet interrupt */
if (pending & INT_MASK_BUSY) {
dev_err(&dev->dev, "packet dropped\n");
dev->stats.rx_dropped++;
}
- if (pending & INT_MASK_RX) {
- if (napi_schedule_prep(&priv->napi))
- __napi_schedule(&priv->napi);
- } else {
- ethoc_enable_irq(priv, INT_MASK_RX);
+ /* Handle receive/transmit event by switching to polling */
+ if (pending & (INT_MASK_TX | INT_MASK_RX)) {
+ ethoc_disable_irq(priv, INT_MASK_TX | INT_MASK_RX);
+ napi_schedule(&priv->napi);
}
- if (pending & INT_MASK_TX)
- ethoc_tx(dev);
-
- ethoc_enable_irq(priv, INT_MASK_ALL & ~INT_MASK_RX);
return IRQ_HANDLED;
}
@@ -580,15 +600,18 @@ static int ethoc_get_mac_address(struct net_device *dev, void *addr)
static int ethoc_poll(struct napi_struct *napi, int budget)
{
struct ethoc *priv = container_of(napi, struct ethoc, napi);
- int work_done = 0;
+ int rx_work_done = 0;
+ int tx_work_done = 0;
+
+ rx_work_done = ethoc_rx(priv->netdev, budget);
+ tx_work_done = ethoc_tx(priv->netdev, budget);
- work_done = ethoc_rx(priv->netdev, budget);
- if (work_done < budget) {
+ if (rx_work_done < budget && tx_work_done < budget) {
napi_complete(napi);
- ethoc_enable_irq(priv, INT_MASK_RX);
+ ethoc_enable_irq(priv, INT_MASK_TX | INT_MASK_RX);
}
- return work_done;
+ return rx_work_done;
}
static int ethoc_mdio_read(struct mii_bus *bus, int phy, int reg)
--
1.7.1
^ permalink raw reply related
* [PATCH 5/8] ethoc: Double check pending RX packet
From: Jonas Bonn @ 2010-11-24 13:40 UTC (permalink / raw)
To: netdev; +Cc: Jonas Bonn
In-Reply-To: <1290606058-26703-1-git-send-email-jonas@southpole.se>
An interrupt may occur between checking bd.stat and clearing the
interrupt source register which would result in the packet going totally
unnoticed as the interrupt will be missed. Double check bd.stat after
clearing the interrupt source register to guard against such an
occurrence.
Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
drivers/net/ethoc.c | 16 ++++++++++++++--
1 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index 7d1b5d8..5afa934 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -417,8 +417,20 @@ static int ethoc_rx(struct net_device *dev, int limit)
entry = priv->num_tx + (priv->cur_rx % priv->num_rx);
ethoc_read_bd(priv, entry, &bd);
- if (bd.stat & RX_BD_EMPTY)
- break;
+ if (bd.stat & RX_BD_EMPTY) {
+ ethoc_ack_irq(priv, INT_MASK_RX);
+ /* If packet (interrupt) came in between checking
+ * BD_EMTPY and clearing the interrupt source, then we
+ * risk missing the packet as the RX interrupt won't
+ * trigger right away when we reenable it; hence, check
+ * BD_EMTPY here again to make sure there isn't such a
+ * packet waiting for us...
+ */
+ ethoc_read_bd(priv, entry, &bd);
+ if (bd.stat & RX_BD_EMPTY)
+ break;
+
+ }
if (ethoc_update_rx_stats(priv, &bd) == 0) {
int size = bd.stat >> 16;
--
1.7.1
^ permalink raw reply related
* [PATCH 8/8] ethoc: fix function return type
From: Jonas Bonn @ 2010-11-24 13:40 UTC (permalink / raw)
To: netdev; +Cc: Jonas Bonn
In-Reply-To: <1290606058-26703-1-git-send-email-jonas@southpole.se>
update_ethoc_tx_stats doesn't need to return anything so make its return
type void in order to avoid an unnecessary cast when the function is called.
Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
drivers/net/ethoc.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index 4f7c9ad..b12c588 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -466,7 +466,7 @@ static int ethoc_rx(struct net_device *dev, int limit)
return count;
}
-static int ethoc_update_tx_stats(struct ethoc *dev, struct ethoc_bd *bd)
+static void ethoc_update_tx_stats(struct ethoc *dev, struct ethoc_bd *bd)
{
struct net_device *netdev = dev->netdev;
@@ -496,7 +496,6 @@ static int ethoc_update_tx_stats(struct ethoc *dev, struct ethoc_bd *bd)
netdev->stats.collisions += (bd->stat >> 4) & 0xf;
netdev->stats.tx_bytes += bd->stat >> 16;
netdev->stats.tx_packets++;
- return 0;
}
static int ethoc_tx(struct net_device *dev, int limit)
@@ -527,7 +526,7 @@ static int ethoc_tx(struct net_device *dev, int limit)
break;
}
- (void)ethoc_update_tx_stats(priv, &bd);
+ ethoc_update_tx_stats(priv, &bd);
priv->dty_tx++;
}
--
1.7.1
^ permalink raw reply related
* [PATCH 2/8] ethoc: remove unused spinlock
From: Jonas Bonn @ 2010-11-24 13:40 UTC (permalink / raw)
To: netdev; +Cc: Jonas Bonn
In-Reply-To: <1290606058-26703-1-git-send-email-jonas@southpole.se>
Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
drivers/net/ethoc.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index 6b4b9e1..fc8c044 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -185,7 +185,6 @@ MODULE_PARM_DESC(buffer_size, "DMA buffer allocation size");
* @netdev: pointer to network device structure
* @napi: NAPI structure
* @msg_enable: device state flags
- * @rx_lock: receive lock
* @lock: device lock
* @phy: attached PHY
* @mdio: MDIO bus for PHY access
@@ -210,7 +209,6 @@ struct ethoc {
struct napi_struct napi;
u32 msg_enable;
- spinlock_t rx_lock;
spinlock_t lock;
struct phy_device *phy;
@@ -1061,7 +1059,6 @@ static int __devinit ethoc_probe(struct platform_device *pdev)
/* setup NAPI */
netif_napi_add(netdev, &priv->napi, ethoc_poll, 64);
- spin_lock_init(&priv->rx_lock);
spin_lock_init(&priv->lock);
ret = register_netdev(netdev);
--
1.7.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox