* Re: [net-next.git 4/9] stmmac: add the support for PTP hw clock driver
From: Richard Cochran @ 2013-03-10 12:10 UTC (permalink / raw)
To: Giuseppe CAVALLARO; +Cc: netdev, bh74.an, ayagond, Rayagond Kokatanur
In-Reply-To: <1362653419-1047-5-git-send-email-peppe.cavallaro@st.com>
I have a few comments, below.
On Thu, Mar 07, 2013 at 11:50:14AM +0100, Giuseppe CAVALLARO wrote:
> From: Rayagond Kokatanur <rayagond@vayavyalabs.com>
>
> This patch implements PHC (ptp hardware clock) driver for stmmac
> driver to support 1588 PTP.
>
> Signed-off-by: Rayagond Kokatanur <rayagond@vayavyalabs.com>
> Hacked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/Makefile | 2 +-
> drivers/net/ethernet/stmicro/stmmac/common.h | 4 +
> drivers/net/ethernet/stmicro/stmmac/stmmac.h | 13 ++
> .../net/ethernet/stmicro/stmmac/stmmac_hwstamp.c | 50 +++++
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 31 +++-
> drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c | 209 ++++++++++++++++++++
> drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h | 2 +
> 7 files changed, 303 insertions(+), 8 deletions(-)
> create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> index cc97c07..40ee3df 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> @@ -3,7 +3,7 @@ stmmac-$(CONFIG_STMMAC_RING) += ring_mode.o
> stmmac-$(CONFIG_STMMAC_CHAINED) += chain_mode.o
> stmmac-$(CONFIG_STMMAC_PLATFORM) += stmmac_platform.o
> stmmac-$(CONFIG_STMMAC_PCI) += stmmac_pci.o
> -stmmac-$(CONFIG_STMMAC_USE_HWSTAMP) += stmmac_hwstamp.o
> +stmmac-$(CONFIG_STMMAC_USE_HWSTAMP) += stmmac_hwstamp.o stmmac_ptp.o
> stmmac-objs:= stmmac_main.o stmmac_ethtool.o stmmac_mdio.o \
> dwmac_lib.o dwmac1000_core.o dwmac1000_dma.o \
> dwmac100_core.o dwmac100_dma.o enh_desc.o norm_desc.o \
> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
> index df7123a..a10974c 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> @@ -387,6 +387,10 @@ struct stmmac_hwtimestamp {
> void (*config_sub_second_increment) (void __iomem *ioaddr);
> int (*init_systime) (void __iomem *ioaddr, u32 sec, u32 nsec);
> int (*config_addend)(void __iomem *ioaddr, u32 addend);
> + int (*adjust_systime)(void __iomem *ioaddr, u32 sec, u32 nsec,
> + int add_sub);
> + u32 (*get_systime_sec)(void __iomem *ioaddr);
> + u32 (*get_systime_nsec)(void __iomem *ioaddr);
No need here for two separate functions. Combine them into one get_systime().
> };
> #endif
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> index 50b5f26..2d6e2e1 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> @@ -32,6 +32,10 @@
> #include <linux/pci.h>
> #include "common.h"
>
> +#ifdef CONFIG_STMMAC_USE_HWSTAMP
> +#include <linux/ptp_clock_kernel.h>
> +#endif
> +
> struct stmmac_priv {
> /* Frequently used values are kept adjacent for cache effect */
> struct dma_desc *dma_tx ____cacheline_aligned;
> @@ -98,6 +102,9 @@ struct stmmac_priv {
> int hwts_tx_en;
> int hwts_rx_en;
> unsigned int default_addend;
> + struct ptp_clock *ptp_clock;
> + struct ptp_clock_info ptp_clock_ops;
> + spinlock_t ptp_lock;
> #endif
> int atds;
> };
> @@ -111,6 +118,12 @@ extern const struct stmmac_desc_ops enh_desc_ops;
> extern const struct stmmac_desc_ops ndesc_ops;
> #ifdef CONFIG_STMMAC_USE_HWSTAMP
> extern const struct stmmac_hwtimestamp stmmac_ptp;
> +extern int stmmac_adjust_systime(void __iomem *ioaddr, u32 sec, u32 nsec,
> + int add_sub);
> +extern u32 stmmac_get_systime_sec(void __iomem *ioaddr);
> +extern u32 stmmac_get_systime_nsec(void __iomem *ioaddr);
> +extern int stmmac_ptp_register(struct stmmac_priv *priv);
> +extern void stmmac_ptp_unregister(struct stmmac_priv *priv);
> #endif
> int stmmac_freeze(struct net_device *ndev);
> int stmmac_restore(struct net_device *ndev);
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwstamp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwstamp.c
> index be9e399..bec2278 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwstamp.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwstamp.c
> @@ -121,9 +121,59 @@ static int stmmac_config_addend(void __iomem *ioaddr, u32 addend)
> return 0;
> }
>
> +static int stmmac_adjust_systime(void __iomem *ioaddr, u32 sec, u32 nsec,
> + int add_sub)
> +{
> + u32 value;
> + int limit;
> +
> + /* wait for previous(if any) system time adjust/update to complete */
These busy loops also need fixing, like in the other patch.
> + limit = 100;
> + while (limit--) {
> + if (!(readl(ioaddr + PTP_TCR) & PTP_TCR_TSUPDT))
> + break;
> + mdelay(10);
> + }
> + if (limit < 0)
> + return -EBUSY;
> +
> + writel(sec, ioaddr + PTP_STSUR);
> + writel(((add_sub << PTP_STNSUR_ADDSUB_SHIFT) | nsec),
> + ioaddr + PTP_STNSUR);
> + /* issue command to initialize the system time value */
> + value = readl(ioaddr + PTP_TCR);
> + value |= PTP_TCR_TSUPDT;
> + writel(value, ioaddr + PTP_TCR);
> +
> + /* wait for present system time adjust/update to complete */
> + limit = 100;
> + while (limit--) {
> + if (!(readl(ioaddr + PTP_TCR) & PTP_TCR_TSUPDT))
> + break;
> + mdelay(10);
> + }
> + if (limit < 0)
> + return -EBUSY;
> +
> + return 0;
> +}
> +
> +static u32 stmmac_get_systime_sec(void __iomem *ioaddr)
> +{
> + return readl(ioaddr + PTP_STSR);
> +}
> +
> +static u32 stmmac_get_systime_nsec(void __iomem *ioaddr)
> +{
> + return readl(ioaddr + PTP_STNSR);
> +}
> +
> const struct stmmac_hwtimestamp stmmac_ptp = {
> .config_hw_tstamping = stmmac_config_hw_tstamping,
> .init_systime = stmmac_init_systime,
> .config_sub_second_increment = stmmac_config_sub_second_increment,
> .config_addend = stmmac_config_addend,
> + .adjust_systime = stmmac_adjust_systime,
> + .get_systime_sec = stmmac_get_systime_sec,
> + .get_systime_nsec = stmmac_get_systime_nsec,
> };
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 6174f34..009abf8 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -474,24 +474,37 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, struct ifreq *ifr)
> sizeof(struct hwtstamp_config)) ? -EFAULT : 0;
> }
>
> -static void stmmac_init_ptp(struct stmmac_priv *priv)
> +static int stmmac_init_ptp(struct stmmac_priv *priv)
> {
> - if (priv->dma_cap.time_stamp)
> - pr_debug("IEEE 1588-2002 Time Stamp supported\n");
> - if (priv->dma_cap.atime_stamp)
> - pr_debug("IEEE 1588-2008 Advanced Time Stamp supported\n");
> + if (!(priv->dma_cap.time_stamp || priv->dma_cap.atime_stamp))
> + return -EOPNOTSUPP;
> +
> + if (netif_msg_hw(priv)) {
> + if (priv->dma_cap.time_stamp)
> + pr_debug("IEEE 1588-2002 Time Stamp supported\n");
> + if (priv->dma_cap.atime_stamp)
> + pr_debug("IEEE 1588-2008 Advanced Time Stamp supported\n");
> + }
>
> priv->hw->ptp = &stmmac_ptp;
>
> priv->hwts_tx_en = 0;
> priv->hwts_rx_en = 0;
>
> + return stmmac_ptp_register(priv);
> +}
> +
> +static void stmmac_release_ptp(struct stmmac_priv *priv)
> +{
> + stmmac_ptp_unregister(priv);
> }
> +
> #else
> #define stmmac_hwtstamp_ioctl(dev, ifr) (-EOPNOTSUPP)
> #define stmmac_get_rx_hwtstamp(priv, p, skb)
> #define stmmac_get_tx_hwtstamp(priv, p, skb) 0
> -#define stmmac_init_ptp(priv)
> +#define stmmac_init_ptp(priv) 0
> +#define stmmac_release_ptp(priv)
> #endif /* CONFIG_STMMAC_USE_HWSTAMP */
>
> /**
> @@ -1301,7 +1314,9 @@ static int stmmac_open(struct net_device *dev)
>
> stmmac_mmc_setup(priv);
>
> - stmmac_init_ptp(priv);
> + ret = stmmac_init_ptp(priv);
> + if (ret)
> + pr_warn("%s: failed PTP initialisation\n", __func__);
>
> #ifdef CONFIG_STMMAC_DEBUG_FS
> ret = stmmac_init_fs(dev);
> @@ -1403,6 +1418,8 @@ static int stmmac_release(struct net_device *dev)
> #endif
> clk_disable_unprepare(priv->stmmac_clk);
>
> + stmmac_release_ptp(priv);
> +
> return 0;
> }
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> new file mode 100644
> index 0000000..53680bf
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> @@ -0,0 +1,209 @@
> +/*******************************************************************************
> + PTP 1588 clock using the STMMAC.
> +
> + Copyright (C) 2013 Vayavya Labs Pvt Ltd
> +
> + This program is free software; you can redistribute it and/or modify it
> + under the terms and conditions of the GNU General Public License,
> + version 2, as published by the Free Software Foundation.
> +
> + This program is distributed in the hope it will be useful, but WITHOUT
> + ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + more details.
> +
> + 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.,
> + 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
> +
> + The full GNU General Public License is included in this distribution in
> + the file called "COPYING".
> +
> + Author: Rayagond Kokatanur <rayagond@vayavyalabs.com>
> +*******************************************************************************/
> +#include "stmmac.h"
> +#include "stmmac_ptp.h"
> +
> +/**
> + * stmmac_adjust_freq
> + *
> + * @ptp: pointer to ptp_clock_info structure
> + * @ppb: desired period change in parts ber billion
> + *
> + * Description: this function will adjust the frequency of hardware clock.
> + */
> +static int stmmac_adjust_freq(struct ptp_clock_info *ptp, s32 ppb)
> +{
> + struct stmmac_priv *priv =
> + container_of(ptp, struct stmmac_priv, ptp_clock_ops);
> + u32 diff, addend;
> + int neg_adj = 0;
> + u64 adj;
> +
> + if (ppb < 0) {
> + neg_adj = 1;
> + ppb = -ppb;
> + }
> +
> + addend = priv->default_addend;
> + adj = addend;
> + adj *= ppb;
> + /* div_u64 will divided the "adj" by "1000000000ULL"
> + * and return the quotient
> + */
No need to comment what div_u64 does. We know that already.
> + diff = div_u64(adj, 1000000000ULL);
> +
> + addend = neg_adj ? (addend - diff) : (addend + diff);
> +
> + priv->hw->ptp->config_addend(priv->ioaddr, addend);
Don't you need locking here to protect against concurrent callers of
config_addend?
> +
> + return 0;
> +}
> +
> +/**
> + * stmmac_adjust_time
> + *
> + * @ptp: pointer to ptp_clock_info structure
> + * @delta: desired change in nanoseconds
> + *
> + * Description: this function will shift/adjust the hardware clock time.
> + */
> +static int stmmac_adjust_time(struct ptp_clock_info *ptp, s64 delta)
> +{
> + struct stmmac_priv *priv =
> + container_of(ptp, struct stmmac_priv, ptp_clock_ops);
> + unsigned long flags;
> + u32 sec, nsec;
> + u32 quotient, reminder;
> + int neg_adj = 0;
> +
> + spin_lock_irqsave(&priv->ptp_lock, flags);
You have locked too much code here. The arithmetic on stack variables
is already reentrant.
> +
> + if (delta < 0) {
> + neg_adj = 1;
> + delta = -delta;
> + }
> +
> + quotient = div_u64_rem(delta, 1000000000ULL, &reminder);
> + sec = quotient;
> + nsec = reminder;
Lock here instead.
> + priv->hw->ptp->adjust_systime(priv->ioaddr, sec, nsec, neg_adj);
> +
> + spin_unlock_irqrestore(&priv->lock, flags);
> +
> + return 0;
> +}
> +
> +/**
> + * stmmac_get_time
> + *
> + * @ptp: pointer to ptp_clock_info structure
> + * @ts: pointer to hold time/result
> + *
> + * Description: this function will read the current time from the
> + * hardware clock and store it in @ts.
> + */
> +static int stmmac_get_time(struct ptp_clock_info *ptp, struct timespec *ts)
> +{
> + struct stmmac_priv *priv =
> + container_of(ptp, struct stmmac_priv, ptp_clock_ops);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&priv->ptp_lock, flags);
> +
> + ts->tv_sec = priv->hw->ptp->get_systime_sec(priv->ioaddr);
> + ts->tv_nsec = priv->hw->ptp->get_systime_nsec(priv->ioaddr);
See, use just one function instead of two.
> +
> + spin_unlock_irqrestore(&priv->ptp_lock, flags);
> +
> + return 0;
> +}
> +
> +/**
> + * stmmac_set_time
> + *
> + * @ptp: pointer to ptp_clock_info structure
> + * @ts: time value to set
> + *
> + * Description: this function will set the current time on the
> + * hardware clock.
> + */
> +static int stmmac_set_time(struct ptp_clock_info *ptp,
> + const struct timespec *ts)
> +{
> + struct stmmac_priv *priv =
> + container_of(ptp, struct stmmac_priv, ptp_clock_ops);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&priv->ptp_lock, flags);
> +
> + priv->hw->ptp->init_systime(priv->ioaddr, ts->tv_sec, ts->tv_nsec);
> +
> + spin_unlock_irqrestore(&priv->ptp_lock, flags);
> +
> + return 0;
> +}
> +
> +static int stmmac_enable(struct ptp_clock_info *ptp,
> + struct ptp_clock_request *rq, int on)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +/* structure describing a PTP hardware clock */
> +static struct ptp_clock_info stmmac_ptp_clock_ops = {
> + .owner = THIS_MODULE,
> + .name = "stmmac_ptp_clock",
> + .max_adj = STMMAC_SYSCLOCK,
This should be the maximum adjustment supported by the clock, in parts
per billion, not the clock frequency.
> + .n_alarm = 0,
> + .n_ext_ts = 0,
> + .n_per_out = 0,
> + .pps = 0,
> + .adjfreq = stmmac_adjust_freq,
> + .adjtime = stmmac_adjust_time,
> + .gettime = stmmac_get_time,
> + .settime = stmmac_set_time,
> + .enable = stmmac_enable,
> +};
Thanks,
Richard
^ permalink raw reply
* Re: [net-next.git 2/9] stmmac: add IEEE 1588-2002 PTP support
From: Richard Cochran @ 2013-03-10 12:25 UTC (permalink / raw)
To: Giuseppe CAVALLARO; +Cc: netdev, bh74.an, Rayagond K
In-Reply-To: <51398CFD.9010605@st.com>
On Fri, Mar 08, 2013 at 08:02:21AM +0100, Giuseppe CAVALLARO wrote:
> On 3/8/2013 7:34 AM, Richard Cochran wrote:
> >>+
> >>+static int enh_desc_get_rx_timestamp_status(struct dma_desc *p)
> >>+{
> >>+ /* FIXME if Enhance descriptor with 8 DWORDS is enabled */
> >
> >Why not fix these FIXMEs for the next respin?
>
> This is fixed in the patch #5 where we use the extended descriptors
> for PTP2.
If possible, it is nice for the reviewers and for the logic of the
patch series to order the changes so that these FIXMEs go away.
> >>+ struct hwtstamp_config config;
> >>+ struct timespec now;
> >>+ u64 temp = 0;
> >
> >You add this new code here, but you change it all around again a few
> >patches later. Please just submit the final, combined version.
>
> we kept these separately because the patch #5 (for example) depends on
> another one that adds the extended descriptor support. Also If I add
> all the code in a single patch this will be very big. I had some
> problems to review all separately. So I suspect that if we merge all
> in a single patch this will not help (especially myself). At any rate,
> tell me if you prefer to have a single patch. I can do that.
I am not asking for bigger patches. It is good to arrange your patches
in small steps, since that makes both reviewing and bisecting easier.
However, for brand new code, I find it quite annoying to read one
patch, and then to have it all re-written in the next one. (If a new
function *only* grows during a patch series, that is easy to follow.)
So what I would like to see is a logical, understandable series of
small steps, but when new code appears, it is the real, final form.
Thanks,
Richard
^ permalink raw reply
* [PATCH] net: can: af_can.c: Fix checkpatch warnings
From: Valentin Ilie @ 2013-03-10 12:28 UTC (permalink / raw)
To: socketcan, davem; +Cc: netdev, linux-kernel, Valentin Ilie
Replace printk(KERN_ERR with pr_err
Add space before {
Signed-off-by: Valentin Ilie <valentin.ilie@gmail.com>
---
net/can/af_can.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/net/can/af_can.c b/net/can/af_can.c
index c48e522..a14c0aa 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -525,7 +525,7 @@ void can_rx_unregister(struct net_device *dev, canid_t can_id, canid_t mask,
d = find_dev_rcv_lists(dev);
if (!d) {
- printk(KERN_ERR "BUG: receive list not found for "
+ pr_err("BUG: receive list not found for "
"dev %s, id %03X, mask %03X\n",
DNAME(dev), can_id, mask);
goto out;
@@ -552,7 +552,7 @@ void can_rx_unregister(struct net_device *dev, canid_t can_id, canid_t mask,
*/
if (!r) {
- printk(KERN_ERR "BUG: receive list entry not found for "
+ pr_err("BUG: receive list entry not found for "
"dev %s, id %03X, mask %03X\n",
DNAME(dev), can_id, mask);
r = NULL;
@@ -749,7 +749,7 @@ int can_proto_register(const struct can_proto *cp)
int err = 0;
if (proto < 0 || proto >= CAN_NPROTO) {
- printk(KERN_ERR "can: protocol number %d out of range\n",
+ pr_err("can: protocol number %d out of range\n",
proto);
return -EINVAL;
}
@@ -761,7 +761,7 @@ int can_proto_register(const struct can_proto *cp)
mutex_lock(&proto_tab_lock);
if (proto_tab[proto]) {
- printk(KERN_ERR "can: protocol %d already registered\n",
+ pr_err("can: protocol %d already registered\n",
proto);
err = -EBUSY;
} else
@@ -817,8 +817,7 @@ static int can_notifier(struct notifier_block *nb, unsigned long msg,
/* create new dev_rcv_lists for this device */
d = kzalloc(sizeof(*d), GFP_KERNEL);
if (!d) {
- printk(KERN_ERR
- "can: allocation of receive list failed\n");
+ pr_err("can: allocation of receive list failed\n");
return NOTIFY_DONE;
}
BUG_ON(dev->ml_priv);
@@ -838,7 +837,7 @@ static int can_notifier(struct notifier_block *nb, unsigned long msg,
dev->ml_priv = NULL;
}
} else
- printk(KERN_ERR "can: notifier: receive list not "
+ pr_err("can: notifier: receive list not "
"found for dev %s\n", dev->name);
spin_unlock(&can_rcvlists_lock);
@@ -927,7 +926,7 @@ static __exit void can_exit(void)
/* remove created dev_rcv_lists from still registered CAN devices */
rcu_read_lock();
for_each_netdev_rcu(&init_net, dev) {
- if (dev->type == ARPHRD_CAN && dev->ml_priv){
+ if (dev->type == ARPHRD_CAN && dev->ml_priv) {
struct dev_rcv_lists *d = dev->ml_priv;
--
1.8.1.2
^ permalink raw reply related
* Re: [net-next.git 2/9] stmmac: add IEEE 1588-2002 PTP support
From: Richard Cochran @ 2013-03-10 12:31 UTC (permalink / raw)
To: Rayagond K; +Cc: Giuseppe CAVALLARO, netdev, bh74.an
In-Reply-To: <CAJ3bTp6tQ4L0vya9-eO0E+19Wk74uNJonbA9hqc+fgN77dcjOQ@mail.gmail.com>
On Fri, Mar 08, 2013 at 01:15:00PM +0530, Rayagond K wrote:
> On Fri, Mar 8, 2013 at 12:32 PM, Giuseppe CAVALLARO <peppe.cavallaro@st.com>wrote:
> > On 3/8/2013 7:34 AM, Richard Cochran wrote:
> >> What are these coarse/fine method? Can't we always use the fine one?
> >>
> >
> > This is explained in the "4.1.2 System Time Register Module" of Synopsys
> > Databook. Summarizing, the MAC can have an optional module and use this
> > coarse correction method. In the fine correction method, a slave clock’s
> > frequency drift with respect to the master clock is corrected over a period
> > of time instead of in one clock, as in coarse correction.
> >
> > Pls, Rayagond feels free to provide more details...
>
>
> Yes Synopsys core support two method for correcting the system time
> ie COARSE method and FINE method.
>
> In the fine correction method, a slave clock’s frequency drift with respect
> to the master clock (as defined in IEEE 1588) is corrected over a period of
> time instead of in one clock, as in coarse correction. This helps maintain
> linear time and does not introduce drastic changes (or a large jitter) in
> the reference time between PTP Sync message intervals.
So it sounds like to me that you only should offer the COARSE
method. Smoothly changing the frequency of the clock is the job of the
clock servo which is running in user space.
Although some clocks (like this one and the phyter, for example) have
the ability to smoothly correct a given offset over a given time, we
do not have any user/kernel API for making use of this feature.
Thanks,
Richard
^ permalink raw reply
* [PATCH] eicon: Fixed checkpatch warning
From: Andreea Hodea @ 2013-03-10 12:34 UTC (permalink / raw)
To: mac; +Cc: isdn, netdev, linux-kernel, Andreea Hodea
drivers/isdn/hardware/eicon/diva_didd.c:32:6: warning: symbol
'DRIVERRELEASE_DIDD' was not declared. Should it be static?
Signed-off-by: Andreea Hodea <hodea_andreea@yahoo.com>
---
drivers/isdn/hardware/eicon/diva_didd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/isdn/hardware/eicon/diva_didd.c b/drivers/isdn/hardware/eicon/diva_didd.c
index fab6ccf..21468be 100644
--- a/drivers/isdn/hardware/eicon/diva_didd.c
+++ b/drivers/isdn/hardware/eicon/diva_didd.c
@@ -29,7 +29,7 @@ static char *main_revision = "$Revision: 1.13.6.4 $";
static char *DRIVERNAME =
"Eicon DIVA - DIDD table (http://www.melware.net)";
static char *DRIVERLNAME = "divadidd";
-char *DRIVERRELEASE_DIDD = "2.0";
+static char *DRIVERRELEASE_DIDD = "2.0";
MODULE_DESCRIPTION("DIDD table driver for diva drivers");
MODULE_AUTHOR("Cytronics & Melware, Eicon Networks");
--
1.7.10.4
^ permalink raw reply related
* Re: [net-next.git 6/9] stmmac: add missing supported filters to get_ts_info
From: Richard Cochran @ 2013-03-10 12:36 UTC (permalink / raw)
To: Giuseppe CAVALLARO; +Cc: netdev, bh74.an, ayagond
In-Reply-To: <1362653419-1047-7-git-send-email-peppe.cavallaro@st.com>
On Thu, Mar 07, 2013 at 11:50:16AM +0100, Giuseppe CAVALLARO wrote:
> This patch updates the filters for ethtool's get_ts_info to return support for
> all filters which can be supported after having added the PTP support.
This code should ideally appear in the same patch where the filtering
support first appears.
Thanks,
Richard
^ permalink raw reply
* [PATCH] ath: changed kmalloc to kmemdup
From: Andrei Epure @ 2013-03-10 12:39 UTC (permalink / raw)
To: linville; +Cc: kvalo, linux-wireless, netdev, linux-kernel, Andrei Epure
Signed-off-by: Andrei Epure <epure.andrei@gmail.com>
---
drivers/net/wireless/ath/ath6kl/usb.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/wireless/ath/ath6kl/usb.c b/drivers/net/wireless/ath/ath6kl/usb.c
index 5fcd342..ffa1daa 100644
--- a/drivers/net/wireless/ath/ath6kl/usb.c
+++ b/drivers/net/wireless/ath/ath6kl/usb.c
@@ -856,11 +856,9 @@ static int ath6kl_usb_submit_ctrl_out(struct ath6kl_usb *ar_usb,
int ret;
if (size > 0) {
- buf = kmalloc(size, GFP_KERNEL);
+ buf = kmemdup(data, size, GFP_KERNEL);
if (buf == NULL)
return -ENOMEM;
-
- memcpy(buf, data, size);
}
/* note: if successful returns number of bytes transfered */
--
1.7.10.4
^ permalink raw reply related
* Re: [net-next.git 2/9] stmmac: add IEEE 1588-2002 PTP support
From: Richard Cochran @ 2013-03-10 12:40 UTC (permalink / raw)
To: Giuseppe CAVALLARO; +Cc: netdev, bh74.an, Rayagond K
In-Reply-To: <20130310122553.GA6407@netboy.at.omicron.at>
On Sun, Mar 10, 2013 at 01:25:54PM +0100, Richard Cochran wrote:
> So what I would like to see is a logical, understandable series of
> small steps, but when new code appears, it is the real, final form.
Here is the order of patches that has made sense for other cards:
1. add hw time stamping support (all possible kinds) via so_timestamping
2. add ptp hardware clock support via ptp_clock_register
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH] net: can: af_can.c: Fix checkpatch warnings
From: Oliver Hartkopp @ 2013-03-10 12:45 UTC (permalink / raw)
To: Valentin Ilie; +Cc: davem, netdev, linux-kernel
In-Reply-To: <1362918526-26730-1-git-send-email-valentin.ilie@gmail.com>
Hallo Valentin,
thanks for the idea for the cleanup.
But if you replace the the printk() statements it's worth to check if code
fits into a single line now, e.g.
On 10.03.2013 13:28, Valentin Ilie wrote:
r = NULL;
> @@ -749,7 +749,7 @@ int can_proto_register(const struct can_proto *cp)
> int err = 0;
>
> if (proto < 0 || proto >= CAN_NPROTO) {
> - printk(KERN_ERR "can: protocol number %d out of range\n",
> + pr_err("can: protocol number %d out of range\n",
> proto);
here " ,proto);" can obviously be move into the previous line while preserving
the 80 chars per line rule.
> return -EINVAL;
> }
> @@ -761,7 +761,7 @@ int can_proto_register(const struct can_proto *cp)
> mutex_lock(&proto_tab_lock);
>
> if (proto_tab[proto]) {
> - printk(KERN_ERR "can: protocol %d already registered\n",
> + pr_err("can: protocol %d already registered\n",
> proto);
here too
> err = -EBUSY;
> } else
> @@ -817,8 +817,7 @@ static int can_notifier(struct notifier_block *nb, unsigned long msg,
> /* create new dev_rcv_lists for this device */
> d = kzalloc(sizeof(*d), GFP_KERNEL);
> if (!d) {
> - printk(KERN_ERR
> - "can: allocation of receive list failed\n");
> + pr_err("can: allocation of receive list failed\n");
> return NOTIFY_DONE;
> }
As i've seen in former patches from Joe Perches OOM messages are obsolete.
You could write
if(!d)
return NOTIFY_DONE;
here instead.
Thanks,
Oliver
^ permalink raw reply
* Re: [PATCH] net: can: af_can.c: Fix checkpatch warnings
From: Daniel Mack @ 2013-03-10 13:05 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: Valentin Ilie, davem, netdev, linux-kernel
In-Reply-To: <513C8062.6030001@hartkopp.net>
On Sun, Mar 10, 2013 at 1:45 PM, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> Hallo Valentin,
>
> thanks for the idea for the cleanup.
>
> But if you replace the the printk() statements it's worth to check if code
> fits into a single line now, e.g.
>
> On 10.03.2013 13:28, Valentin Ilie wrote:
>
> r = NULL;
>> @@ -749,7 +749,7 @@ int can_proto_register(const struct can_proto *cp)
>> int err = 0;
>>
>> if (proto < 0 || proto >= CAN_NPROTO) {
>> - printk(KERN_ERR "can: protocol number %d out of range\n",
>> + pr_err("can: protocol number %d out of range\n",
>> proto);
>
>
> here " ,proto);" can obviously be move into the previous line while preserving
> the 80 chars per line rule.
>
>
>> return -EINVAL;
>> }
>> @@ -761,7 +761,7 @@ int can_proto_register(const struct can_proto *cp)
>> mutex_lock(&proto_tab_lock);
>>
>> if (proto_tab[proto]) {
>> - printk(KERN_ERR "can: protocol %d already registered\n",
>> + pr_err("can: protocol %d already registered\n",
>> proto);
>
>
> here too
>
>> err = -EBUSY;
>> } else
>> @@ -817,8 +817,7 @@ static int can_notifier(struct notifier_block *nb, unsigned long msg,
>> /* create new dev_rcv_lists for this device */
>> d = kzalloc(sizeof(*d), GFP_KERNEL);
>> if (!d) {
>> - printk(KERN_ERR
>> - "can: allocation of receive list failed\n");
>> + pr_err("can: allocation of receive list failed\n");
>> return NOTIFY_DONE;
>> }
>
>
> As i've seen in former patches from Joe Perches OOM messages are obsolete.
>
> You could write
>
> if(!d)
> return NOTIFY_DONE;
>
> here instead.
While at it, why not switch to dev_err() instead?
Daniel
^ permalink raw reply
* [PATCH] wireless:rtlwifi: replaced kmalloc+memcpy with kmemdup
From: Andrei Epure @ 2013-03-10 13:09 UTC (permalink / raw)
To: Larry.Finger, chaoming_li, linville
Cc: linux-wireless, netdev, linux-kernel, Andrei Epure
Signed-off-by: Andrei Epure <epure.andrei@gmail.com>
---
drivers/net/wireless/rtlwifi/usb.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/rtlwifi/usb.c b/drivers/net/wireless/rtlwifi/usb.c
index 156b527..b5c80b5 100644
--- a/drivers/net/wireless/rtlwifi/usb.c
+++ b/drivers/net/wireless/rtlwifi/usb.c
@@ -224,10 +224,9 @@ static void _usb_writeN_sync(struct rtl_priv *rtlpriv, u32 addr, void *data,
u8 *buffer;
wvalue = (u16)(addr & 0x0000ffff);
- buffer = kmalloc(len, GFP_ATOMIC);
+ buffer = kmemdup(data, len, GFP_ATOMIC);
if (!buffer)
return;
- memcpy(buffer, data, len);
usb_control_msg(udev, pipe, request, reqtype, wvalue,
index, buffer, len, 50);
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] net: can: af_can.c: Fix checkpatch warnings
From: Oliver Hartkopp @ 2013-03-10 13:11 UTC (permalink / raw)
To: Daniel Mack; +Cc: Valentin Ilie, davem, netdev, linux-kernel
In-Reply-To: <CACTFLAM4DtRGvEFQo5yXhR4Sjx8hKBJoNqARqA5U+9iQK=gkBg@mail.gmail.com>
On 10.03.2013 14:05, Daniel Mack wrote:
>>> @@ -817,8 +817,7 @@ static int can_notifier(struct notifier_block *nb, unsigned long msg,
>>> /* create new dev_rcv_lists for this device */
>>> d = kzalloc(sizeof(*d), GFP_KERNEL);
>>> if (!d) {
>>> - printk(KERN_ERR
>>> - "can: allocation of receive list failed\n");
>>> + pr_err("can: allocation of receive list failed\n");
>>> return NOTIFY_DONE;
>>> }
>>
>>
>> As i've seen in former patches from Joe Perches OOM messages are obsolete.
>>
>> You could write
>>
>> if(!d)
>> return NOTIFY_DONE;
>>
>> here instead.
>
> While at it, why not switch to dev_err() instead?
Hm, no.
1. pr_err() would be fine, as dev_err() is for drivers - and we're in
linux/net here and not in linux/drivers
2. When removing the entire printk() statement there's nothing to be modified left
Regards,
Oliver
^ permalink raw reply
* [PATCH] wireless:iwlwifi: changed kmalloc+memcpy with kmemdup
From: Andrei Epure @ 2013-03-10 13:22 UTC (permalink / raw)
To: johannes.berg-ral2JQCrhuEAvxtiuMwx3w,
wey-yi.w.guy-ral2JQCrhuEAvxtiuMwx3w, ilw-VuQAYsv1563Yd54FQh9/CA,
linville-2XuSBdqkA4R54TAoqtyWWQ
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andrei Epure
Signed-off-by: Andrei Epure <epure.andrei-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/net/wireless/iwlwifi/iwl-test.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/iwlwifi/iwl-test.c b/drivers/net/wireless/iwlwifi/iwl-test.c
index ce0c67b..9bc402c 100644
--- a/drivers/net/wireless/iwlwifi/iwl-test.c
+++ b/drivers/net/wireless/iwlwifi/iwl-test.c
@@ -271,7 +271,7 @@ static int iwl_test_fw_cmd(struct iwl_test *tst, struct nlattr **tb)
reply_len = le32_to_cpu(pkt->len_n_flags) & FH_RSCSR_FRAME_SIZE_MSK;
skb = iwl_test_alloc_reply(tst, reply_len + 20);
- reply_buf = kmalloc(reply_len, GFP_KERNEL);
+ reply_buf = kmemdup(&(pkt->hdr), reply_len, GFP_KERNEL);
if (!skb || !reply_buf) {
kfree_skb(skb);
kfree(reply_buf);
@@ -279,7 +279,6 @@ static int iwl_test_fw_cmd(struct iwl_test *tst, struct nlattr **tb)
}
/* The reply is in a page, that we cannot send to user space. */
- memcpy(reply_buf, &(pkt->hdr), reply_len);
iwl_free_resp(&cmd);
if (nla_put_u32(skb, IWL_TM_ATTR_COMMAND,
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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] ath: changed kmalloc to kmemdup
From: Kalle Valo @ 2013-03-10 13:27 UTC (permalink / raw)
To: Andrei Epure
Cc: linville-2XuSBdqkA4R54TAoqtyWWQ,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
ath6kl-devel-A+ZNKFmMK5xy9aJCnZT0Uw
In-Reply-To: <1362919199-16292-1-git-send-email-epure.andrei-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Andrei Epure <epure.andrei-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
> Signed-off-by: Andrei Epure <epure.andrei-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Thanks, I'll take this. But please prefix ath6kl patches with "ath6kl:"
in the future.
--
Kalle Valo
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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: [PATCHv2 net-next 01/15] net: add skb_dst_set_unref
From: Julian Anastasov @ 2013-03-10 13:37 UTC (permalink / raw)
To: David Miller; +Cc: horms, lvs-devel, netdev
In-Reply-To: <20130310.051735.1281635372029510938.davem@davemloft.net>
Hello,
On Sun, 10 Mar 2013, David Miller wrote:
> From: Julian Anastasov <ja@ssi.bg>
> Date: Sat, 9 Mar 2013 23:16:41 +0200
>
> > skb_dst_set_unref will use noref version even for
> > DST_NOCACHE entries because DST_NOCACHE means dst is not
> > cached in routing structures, still dst could be cached
> > by routing users and used to produce noref instances.
> >
> > Signed-off-by: Julian Anastasov <ja@ssi.bg>
>
> I'm fine with this approach, but I think the name of this
> interface could be better.
>
> In fact you could do something like:
>
> 1) Rename skb_dst_set_noref() to __skb_dst_set_noref() and add
> a new "bool force" parameter. DST_NOCACHE check is overriden
> when 'force' is true.
>
> 2) skb_dst_set_noref() is an inline that passes 'force' as false.
>
> 3) New interface skb_dst_set_noref_force() passes 'force' as true
> and will be used by your IPVS changes.
>
> Then all of the RCU checks etc. happen in one shared function.
The idea looks good, here is the implementation.
Can I use it in this form for next patchset versions?
net: add skb_dst_set_noref_force
Rename skb_dst_set_noref to __skb_dst_set_noref and
add force flag as suggested by David Miller. The new wrapper
skb_dst_set_noref_force will force dst entries that are not
cached to be attached as skb dst without taking reference
as long as provided dst is reclaimed after RCU grace period.
Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
include/linux/skbuff.h | 35 ++++++++++++++++++++++++++++++++++-
net/core/dst.c | 7 ++++---
2 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 821c7f4..e8ae1d6 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -570,7 +570,40 @@ static inline void skb_dst_set(struct sk_buff *skb, struct dst_entry *dst)
skb->_skb_refdst = (unsigned long)dst;
}
-extern void skb_dst_set_noref(struct sk_buff *skb, struct dst_entry *dst);
+extern void __skb_dst_set_noref(struct sk_buff *skb, struct dst_entry *dst,
+ bool force);
+
+/**
+ * skb_dst_set_noref - sets skb dst, hopefully, without taking reference
+ * @skb: buffer
+ * @dst: dst entry
+ *
+ * Sets skb dst, assuming a reference was not taken on dst.
+ * If dst entry is cached, we do not take reference and dst_release
+ * will be avoided by refdst_drop. If dst entry is not cached, we take
+ * reference, so that last dst_release can destroy the dst immediately.
+ */
+static inline void skb_dst_set_noref(struct sk_buff *skb, struct dst_entry *dst)
+{
+ __skb_dst_set_noref(skb, dst, false);
+}
+
+/**
+ * skb_dst_set_noref_force - sets skb dst, without taking reference
+ * @skb: buffer
+ * @dst: dst entry
+ *
+ * Sets skb dst, assuming a reference was not taken on dst.
+ * No reference is taken and no dst_release will be called. While for
+ * cached dsts deferred reclaim is a basic feature, for entries that are
+ * not cached it is caller's job to guarantee that last dst_release for
+ * provided dst happens when nobody uses it, eg. after a RCU grace period.
+ */
+static inline void skb_dst_set_noref_force(struct sk_buff *skb,
+ struct dst_entry *dst)
+{
+ __skb_dst_set_noref(skb, dst, true);
+}
/**
* skb_dst_is_noref - Test if skb dst isn't refcounted
diff --git a/net/core/dst.c b/net/core/dst.c
index 35fd12f..488d53c 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -320,20 +320,21 @@ void __dst_destroy_metrics_generic(struct dst_entry *dst, unsigned long old)
EXPORT_SYMBOL(__dst_destroy_metrics_generic);
/**
- * skb_dst_set_noref - sets skb dst, without a reference
+ * __skb_dst_set_noref - sets skb dst, without a reference
* @skb: buffer
* @dst: dst entry
+ * @force: if force is set, use noref version even for DST_NOCACHE entries
*
* Sets skb dst, assuming a reference was not taken on dst
* skb_dst_drop() should not dst_release() this dst
*/
-void skb_dst_set_noref(struct sk_buff *skb, struct dst_entry *dst)
+void __skb_dst_set_noref(struct sk_buff *skb, struct dst_entry *dst, bool force)
{
WARN_ON(!rcu_read_lock_held() && !rcu_read_lock_bh_held());
/* If dst not in cache, we must take a reference, because
* dst_release() will destroy dst as soon as its refcount becomes zero
*/
- if (unlikely(dst->flags & DST_NOCACHE)) {
+ if (unlikely(dst->flags & DST_NOCACHE && !force)) {
dst_hold(dst);
skb_dst_set(skb, dst);
} else {
--
1.7.3.4
^ permalink raw reply related
* [PATCH 2/3] bcm63xx_enet: use managed memory allocations
From: Jonas Gorski @ 2013-03-10 13:57 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Maxime Bizon, Florian Fainelli, Kevin Cernekee
In-Reply-To: <1362923869-21346-1-git-send-email-jogo@openwrt.org>
Signed-off-by: Jonas Gorski <jogo@openwrt.org>
---
drivers/net/ethernet/broadcom/bcm63xx_enet.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
index 0a43188..8397d1c 100644
--- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c
+++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
@@ -1742,7 +1742,8 @@ static int bcm_enet_probe(struct platform_device *pdev)
* if a slave is not present on hw */
bus->phy_mask = ~(1 << priv->phy_id);
- bus->irq = kmalloc(sizeof(int) * PHY_MAX_ADDR, GFP_KERNEL);
+ bus->irq = devm_kzalloc(&pdev->dev, sizeof(int) * PHY_MAX_ADDR,
+ GFP_KERNEL);
if (!bus->irq) {
ret = -ENOMEM;
goto out_free_mdio;
@@ -1803,10 +1804,8 @@ static int bcm_enet_probe(struct platform_device *pdev)
return 0;
out_unregister_mdio:
- if (priv->mii_bus) {
+ if (priv->mii_bus)
mdiobus_unregister(priv->mii_bus);
- kfree(priv->mii_bus->irq);
- }
out_free_mdio:
if (priv->mii_bus)
@@ -1847,7 +1846,6 @@ static int bcm_enet_remove(struct platform_device *pdev)
if (priv->has_phy) {
mdiobus_unregister(priv->mii_bus);
- kfree(priv->mii_bus->irq);
mdiobus_free(priv->mii_bus);
} else {
struct bcm63xx_enet_platform_data *pd;
--
1.7.10.4
^ permalink raw reply related
* [PATCH 1/3] bcm63xx_enet: use managed io memory allocations
From: Jonas Gorski @ 2013-03-10 13:57 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Maxime Bizon, Florian Fainelli, Kevin Cernekee
Signed-off-by: Jonas Gorski <jogo@openwrt.org>
---
drivers/net/ethernet/broadcom/bcm63xx_enet.c | 43 +++++---------------------
1 file changed, 7 insertions(+), 36 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
index 842f983..0a43188 100644
--- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c
+++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
@@ -1634,7 +1634,6 @@ static int bcm_enet_probe(struct platform_device *pdev)
struct resource *res_mem, *res_irq, *res_irq_rx, *res_irq_tx;
struct mii_bus *bus;
const char *clk_name;
- unsigned int iomem_size;
int i, ret;
/* stop if shared driver failed, assume driver->probe will be
@@ -1659,17 +1658,12 @@ static int bcm_enet_probe(struct platform_device *pdev)
if (ret)
goto out;
- iomem_size = resource_size(res_mem);
- if (!request_mem_region(res_mem->start, iomem_size, "bcm63xx_enet")) {
- ret = -EBUSY;
- goto out;
- }
-
- priv->base = ioremap(res_mem->start, iomem_size);
+ priv->base = devm_request_and_ioremap(&pdev->dev, res_mem);
if (priv->base == NULL) {
ret = -ENOMEM;
- goto out_release_mem;
+ goto out;
}
+
dev->irq = priv->irq = res_irq->start;
priv->irq_rx = res_irq_rx->start;
priv->irq_tx = res_irq_tx->start;
@@ -1689,7 +1683,7 @@ static int bcm_enet_probe(struct platform_device *pdev)
priv->mac_clk = clk_get(&pdev->dev, clk_name);
if (IS_ERR(priv->mac_clk)) {
ret = PTR_ERR(priv->mac_clk);
- goto out_unmap;
+ goto out;
}
clk_enable(priv->mac_clk);
@@ -1829,12 +1823,6 @@ out_uninit_hw:
out_put_clk_mac:
clk_disable(priv->mac_clk);
clk_put(priv->mac_clk);
-
-out_unmap:
- iounmap(priv->base);
-
-out_release_mem:
- release_mem_region(res_mem->start, iomem_size);
out:
free_netdev(dev);
return ret;
@@ -1848,7 +1836,6 @@ static int bcm_enet_remove(struct platform_device *pdev)
{
struct bcm_enet_priv *priv;
struct net_device *dev;
- struct resource *res;
/* stop netdevice */
dev = platform_get_drvdata(pdev);
@@ -1871,11 +1858,6 @@ static int bcm_enet_remove(struct platform_device *pdev)
bcm_enet_mdio_write_mii);
}
- /* release device resources */
- iounmap(priv->base);
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- release_mem_region(res->start, resource_size(res));
-
/* disable hw block clocks */
if (priv->phy_clk) {
clk_disable(priv->phy_clk);
@@ -1904,31 +1886,20 @@ struct platform_driver bcm63xx_enet_driver = {
static int bcm_enet_shared_probe(struct platform_device *pdev)
{
struct resource *res;
- unsigned int iomem_size;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res)
return -ENODEV;
- iomem_size = resource_size(res);
- if (!request_mem_region(res->start, iomem_size, "bcm63xx_enet_dma"))
- return -EBUSY;
-
- bcm_enet_shared_base = ioremap(res->start, iomem_size);
- if (!bcm_enet_shared_base) {
- release_mem_region(res->start, iomem_size);
+ bcm_enet_shared_base = devm_request_and_ioremap(&pdev->dev, res);
+ if (!bcm_enet_shared_base)
return -ENOMEM;
- }
+
return 0;
}
static int bcm_enet_shared_remove(struct platform_device *pdev)
{
- struct resource *res;
-
- iounmap(bcm_enet_shared_base);
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- release_mem_region(res->start, resource_size(res));
return 0;
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 3/3] bcm63xx_enet: properly prepare/unprepare clocks before/after usage
From: Jonas Gorski @ 2013-03-10 13:57 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Maxime Bizon, Florian Fainelli, Kevin Cernekee
In-Reply-To: <1362923869-21346-1-git-send-email-jogo@openwrt.org>
Use clk_prepare_enable/disable_unprepare calls in preparation for
switching to the generic clock framework.
Signed-off-by: Jonas Gorski <jogo@openwrt.org>
---
drivers/net/ethernet/broadcom/bcm63xx_enet.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
index 8397d1c..aff3866 100644
--- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c
+++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
@@ -1685,7 +1685,7 @@ static int bcm_enet_probe(struct platform_device *pdev)
ret = PTR_ERR(priv->mac_clk);
goto out;
}
- clk_enable(priv->mac_clk);
+ clk_prepare_enable(priv->mac_clk);
/* initialize default and fetch platform data */
priv->rx_ring_size = BCMENET_DEF_RX_DESC;
@@ -1714,7 +1714,7 @@ static int bcm_enet_probe(struct platform_device *pdev)
priv->phy_clk = NULL;
goto out_put_clk_mac;
}
- clk_enable(priv->phy_clk);
+ clk_prepare_enable(priv->phy_clk);
}
/* do minimal hardware init to be able to probe mii bus */
@@ -1815,12 +1815,12 @@ out_uninit_hw:
/* turn off mdc clock */
enet_writel(priv, 0, ENET_MIISC_REG);
if (priv->phy_clk) {
- clk_disable(priv->phy_clk);
+ clk_disable_unprepare(priv->phy_clk);
clk_put(priv->phy_clk);
}
out_put_clk_mac:
- clk_disable(priv->mac_clk);
+ clk_disable_unprepare(priv->mac_clk);
clk_put(priv->mac_clk);
out:
free_netdev(dev);
@@ -1858,10 +1858,10 @@ static int bcm_enet_remove(struct platform_device *pdev)
/* disable hw block clocks */
if (priv->phy_clk) {
- clk_disable(priv->phy_clk);
+ clk_disable_unprepare(priv->phy_clk);
clk_put(priv->phy_clk);
}
- clk_disable(priv->mac_clk);
+ clk_disable_unprepare(priv->mac_clk);
clk_put(priv->mac_clk);
platform_set_drvdata(pdev, NULL);
--
1.7.10.4
^ permalink raw reply related
* [PATCH linux-next] ipvs: ip_vs_fill_iph_skb: Silence warning that 'iph.thoff_reasm' may be used uninitialized
From: Tim Gardner @ 2013-03-10 15:09 UTC (permalink / raw)
To: linux-kernel
Cc: Tim Gardner, Wensong Zhang, Simon Horman, Julian Anastasov,
David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev, lvs-devel
If CONFIG_IP_VS_IPV6 is not defined, then ip_vs_fill_iph_skb() does not
initialize all of the fields in struct ip_vs_iphdr. Silence the 'may be used
uninitialized' warning in the IPV4 only case by initializing the only field
that is unique to IPV6.
net/netfilter/ipvs/ip_vs_pe_sip.c: In function 'ip_vs_sip_fill_param':
net/netfilter/ipvs/ip_vs_pe_sip.c:87:11: warning: 'iph.thoff_reasm' may be used uninitialized in this function [-Wuninitialized]
gcc version 4.6.3
Also update comment to correctly reference fragoffs.
Cc: Wensong Zhang <wensong@linux-vs.org>
Cc: Simon Horman <horms@verge.net.au>
Cc: Julian Anastasov <ja@ssi.bg>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: James Morris <jmorris@namei.org>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: netdev@vger.kernel.org
Cc: lvs-devel@vger.kernel.org
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
include/net/ip_vs.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 108ebe8..64ca32f 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -165,14 +165,16 @@ ip_vs_fill_ip4hdr(const void *nh, struct ip_vs_iphdr *iphdr)
static inline void
ip_vs_fill_iph_skb(int af, const struct sk_buff *skb, struct ip_vs_iphdr *iphdr)
{
+ /* May not get initialized in all cases */
+ iphdr->thoff_reasm = 0;
+
#ifdef CONFIG_IP_VS_IPV6
if (af == AF_INET6) {
const struct ipv6hdr *iph =
(struct ipv6hdr *)skb_network_header(skb);
iphdr->saddr.in6 = iph->saddr;
iphdr->daddr.in6 = iph->daddr;
- /* ipv6_find_hdr() updates len, flags, thoff_reasm */
- iphdr->thoff_reasm = 0;
+ /* ipv6_find_hdr() updates len, flags, and fragoffs */
iphdr->len = 0;
iphdr->flags = 0;
iphdr->protocol = ipv6_find_hdr(skb, &iphdr->len, -1,
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] wireless:iwlwifi: changed kmalloc+memcpy with kmemdup
From: Johannes Berg @ 2013-03-10 15:10 UTC (permalink / raw)
To: Andrei Epure
Cc: wey-yi.w.guy, ilw, linville, linux-wireless, netdev, linux-kernel
In-Reply-To: <1362921753-9554-1-git-send-email-epure.andrei@gmail.com>
Applied.
johannes
^ permalink raw reply
* Re: [PATCH] wireless:iwlwifi: changed kmalloc+memcpy with kmemdup
From: Johannes Berg @ 2013-03-10 15:18 UTC (permalink / raw)
To: Andrei Epure
Cc: wey-yi.w.guy, ilw, linville, linux-wireless, netdev, linux-kernel
In-Reply-To: <1362921753-9554-1-git-send-email-epure.andrei@gmail.com>
Applied.
johannes
^ permalink raw reply
* [PATCH net-next] Remove unused tw_cookie_values from tcp_timewait_sock
From: Christoph Paasch @ 2013-03-10 15:18 UTC (permalink / raw)
To: David Miller; +Cc: netdev, william.allen.simpson
tw_cookie_values is never used in the TCP-stack.
It was added by 435cf559f (TCPCT part 1d: define TCP cookie option,
extend existing struct's), but already at that time it was not used at
all, nor mentioned in the commit-message.
Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>
---
include/linux/tcp.h | 4 ----
1 file changed, 4 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index f28408c..515c374 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -361,10 +361,6 @@ struct tcp_timewait_sock {
#ifdef CONFIG_TCP_MD5SIG
struct tcp_md5sig_key *tw_md5_key;
#endif
- /* Few sockets in timewait have cookies; in that case, then this
- * object holds a reference to them (tw_cookie_values->kref).
- */
- struct tcp_cookie_values *tw_cookie_values;
};
static inline struct tcp_timewait_sock *tcp_twsk(const struct sock *sk)
--
1.8.1.227.g44fe835
^ permalink raw reply related
* Re: [PATCH] netconsole: release the spinlock before __netpoll_cleanup()
From: Veaceslav Falico @ 2013-03-10 15:25 UTC (permalink / raw)
To: David Miller; +Cc: nhorman, netdev, amwang
In-Reply-To: <20130307.161438.396456319050281566.davem@davemloft.net>
On Thu, Mar 07, 2013 at 04:14:38PM -0500, David Miller wrote:
>From: Veaceslav Falico <vfalico@redhat.com>
>Date: Thu, 7 Mar 2013 11:03:25 +0100
>
>> @@ -680,9 +681,21 @@ static int netconsole_netdev_event(struct
>> notifier_block *this,
>> * rtnl_lock already held
>> */
>> if (nt->np.dev) {
>> + /*
>> + * we still might sleep in
>> + * __netpoll_cleanup(), so release
>> + * the lock and restart
>
>Quite a bit of email corruption of this patch.
Sorry, somehow messed it.
>
>Also, this code block is probably too deeply indented to be sane,
>consider creating a small helper function to call instead.
It gets quite ugly if I try to move it to another function. However, maybe
something like that will work - it's effectively the same code, just that
I've moved the long part out of the if () { } block. Looks a lot more
readable, though one line still breaks 80chars limit. I've reworked the
subject/commit message too.
Subject: [PATCH] netconsole: don't call __netpoll_cleanup() while atomic
__netpoll_cleanup() is called in netconsole_netdev_event() while holding a
spinlock. Release/acquire the spinlock before/after it and restart the
loop.
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
drivers/net/netconsole.c | 22 +++++++++++++++-------
1 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 37add21..38eaa8c 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -666,6 +666,7 @@ static int netconsole_netdev_event(struct notifier_block *this,
goto done;
spin_lock_irqsave(&target_list_lock, flags);
+restart:
list_for_each_entry(nt, &target_list, list) {
netconsole_target_get(nt);
if (nt->np.dev == dev) {
@@ -679,14 +680,21 @@ static int netconsole_netdev_event(struct notifier_block *this,
/*
* rtnl_lock already held
*/
- if (nt->np.dev) {
- __netpoll_cleanup(&nt->np);
- dev_put(nt->np.dev);
- nt->np.dev = NULL;
+ if (!nt->np.dev) {
+ nt->enabled = 0;
+ stopped = true;
+ break;
}
- nt->enabled = 0;
- stopped = true;
- break;
+ /*
+ * we might sleep in __netpoll_cleanup()
+ */
+ spin_unlock_irqrestore(&target_list_lock, flags);
+ __netpoll_cleanup(&nt->np);
+ spin_lock_irqsave(&target_list_lock, flags);
+ dev_put(nt->np.dev);
+ nt->np.dev = NULL;
+ netconsole_target_put(nt);
+ goto restart;
}
}
netconsole_target_put(nt);
--
1.7.1
^ permalink raw reply related
* Re: hitting lockdep warning as of too early VF probe with 3.9-rc1
From: Jack Morgenstein @ 2013-03-10 15:28 UTC (permalink / raw)
To: Ming Lei
Cc: Or Gerlitz, Or Gerlitz, Greg Kroah-Hartman, David Miller,
Roland Dreier, netdev, Yan Burman, Liran Liss
In-Reply-To: <CACVXFVMcgMzwHrvimn2awQZCPP2m=_npTGap-m7Ru+jdCoqcyA@mail.gmail.com>
Hello, Ming, Greg, Roland, Dave, all...
>From a quick scan of ethernet drivers in Dave Miller's net-next git, I
notice that the following drivers (apart from the Mellanox mlx4 driver)
enable SRIOV during the PF probe:
cisco enic (function "enic_probe")
neterion vxge driver(function "vxge_probe")
Solarflare efx driver (function "efx_pci_probe", which invokes "efx_sriov_init")
emulex driver (function "be_probe" --> be_setup --> be_vf_setup)
It would seem that these drivers are susceptible to the nested probe/deadlock
race condition as well.
I believe that it is healthiest for everyone if the probe code in the kernel itself
would avoid such nested probe calls (rather than forcing vendors to deal
with this issue). The kernel code is certainly aware
(or could easily track) that it is invoking the a driver's probe function
while that same probe function has already been invoked and has not yet returned!
-Jack
On Thursday 07 March 2013 04:03, Ming Lei wrote:
> On Thu, Mar 7, 2013 at 4:54 AM, Or Gerlitz <or.gerlitz@gmail.com> wrote:
> > On Wed, Mar 6, 2013 at 4:43 AM, Ming Lei <ming.lei@canonical.com> wrote:
> >> You are adding one new PCI device inside another PCI device's probe(),
> >> so the new device will be probed, since PCI probe() is scheduled by
> >> work_on_cpu, then cause flush_work() called inside worker function,
> >> which might be a real deadlock.
> >
> > So if I understand correct, you recommend to somehow avoid this nested probing?
>
> Yes, you might need to avoid the nested probing in your driver.
>
> >
> >> I am wondering why this commit can cause the problem, since the PCI
> >> device will be probed with its driver if there is one driver for it. There is no
> >> any limit on when the driver should be loaded into system, either before
> >> device is added or after.
> >
> > FWIW to undertstanding the issue - the same driver (mlx4_core) is used
> > by the PF and VF, so the VF driver is already loaded at the time its
> > been added as new PCI device.
> >
> >> From driver core view, looks no wrong things are found.
> >
> > So this got me confused, you pointed on possible deadlock, are you
> > saying the deadlock wouldn't be the result of how the driver code is
> > going nor the commited we bisected?
>
> My commit only affects the driver loading path, but your warning
> is hit in driver probe path triggered by device addition, so the lockdep
> warning should still be triggered without my commit since the two paths
> are totally independent, right?
>
> Thanks,
> --
> Ming Lei
>
^ permalink raw reply
* [PATCH linux-next] SUNRPC: rpcrdma_register_default_external: Dynamically allocate ib_phys_buf
From: Tim Gardner @ 2013-03-10 15:39 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Tim Gardner, Trond Myklebust, J. Bruce Fields, David S. Miller,
Tom Tucker, Haggai Eran, Or Gerlitz, Shani Michaeli,
linux-nfs-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
rpcrdma_register_default_external() is several frames into
the call stack which goes deeper yet. You run the risk of stack
corruption by declaring such a large automatic variable,
so dynamically allocate the array of 'struct ib_phys_buf' objects in
order to silence the frame-larger-than warning.
net/sunrpc/xprtrdma/verbs.c: In function 'rpcrdma_register_default_external':
net/sunrpc/xprtrdma/verbs.c:1774:1: warning: the frame size of 1056 bytes is larger than 1024 bytes [-Wframe-larger-than=]
gcc version 4.6.3
Cc: Trond Myklebust <Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>
Cc: "J. Bruce Fields" <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
Cc: "David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Cc: Tom Tucker <tom-/Yg/VP3ZvrM@public.gmane.org>
Cc: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: Shani Michaeli <shanim-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Tim Gardner <tim.gardner-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
net/sunrpc/xprtrdma/verbs.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
index 93726560..0916467 100644
--- a/net/sunrpc/xprtrdma/verbs.c
+++ b/net/sunrpc/xprtrdma/verbs.c
@@ -1736,9 +1736,13 @@ rpcrdma_register_default_external(struct rpcrdma_mr_seg *seg,
int mem_priv = (writing ? IB_ACCESS_REMOTE_WRITE :
IB_ACCESS_REMOTE_READ);
struct rpcrdma_mr_seg *seg1 = seg;
- struct ib_phys_buf ipb[RPCRDMA_MAX_DATA_SEGS];
+ struct ib_phys_buf *ipb;
int len, i, rc = 0;
+ ipb = kmalloc(sizeof(*ipb) * RPCRDMA_MAX_DATA_SEGS, GFP_KERNEL);
+ if (!ipb)
+ return -ENOMEM;
+
if (*nsegs > RPCRDMA_MAX_DATA_SEGS)
*nsegs = RPCRDMA_MAX_DATA_SEGS;
for (len = 0, i = 0; i < *nsegs;) {
@@ -1770,6 +1774,7 @@ rpcrdma_register_default_external(struct rpcrdma_mr_seg *seg,
seg1->mr_len = len;
}
*nsegs = i;
+ kfree(ipb);
return rc;
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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
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