Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 1/1] iproute2: rsvp classifier support for multiple actions
From: John Fastabend @ 2014-09-28 17:39 UTC (permalink / raw)
  To: Jamal Hadi Salim; +Cc: stephen, netdev
In-Reply-To: <1411922993-5828-1-git-send-email-jhs@emojatatu.com>

On 09/28/2014 09:49 AM, Jamal Hadi Salim wrote:
> From: Jamal Hadi Salim <jhs@mojatatu.com>
>
> Example setup:
>
> sudo tc qdisc del dev eth0 root handle 1:0 prio
> sudo tc qdisc add dev eth0 root handle 1:0 prio
>
> sudo tc filter add dev eth0 pref 10 proto ip parent 1:0 \
> rsvp session 10.0.0.1 ipproto icmp \
> classid 1:1  \
> action police rate 1kbit burst 90k pipe \
> action ok
>
> tc -s filter show dev eth0 parent 1:0
>
> filter protocol ip pref 10 rsvp
> filter protocol ip pref 10 rsvp fh 0x0001100a flowid 1:1 session
> 10.0.0.1 ipproto icmp
>          action order 1:  police 0x5 rate 1Kbit burst 23440b mtu 2Kb
> action pipe overhead 0b
> ref 1 bind 1
>          Action statistics:
>          Sent 98000 bytes 1000 pkt (dropped 0, overlimits 761 requeues 0)
>          backlog 0b 0p requeues 0
>
>          action order 2: gact action pass
>           random type none pass val 0
>           index 2 ref 1 bind 1 installed 60 sec used 3 sec
>          Action statistics:
>          Sent 74578 bytes 761 pkt (dropped 0, overlimits 0 requeues 0)
>          backlog 0b 0p requeues 0
>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> ---

Thanks, this fixes the rsvp syntax.


Tested-by: John Fastabend <john.r.fastabend@intel.com>


-- 
John Fastabend         Intel Corporation

^ permalink raw reply

* Re: tc rsvp filter show broke
From: Cong Wang @ 2014-09-28 17:15 UTC (permalink / raw)
  To: Jamal Hadi Salim; +Cc: John Fastabend, John Fastabend, netdev
In-Reply-To: <542823CE.9020301@mojatatu.com>

[-- Attachment #1: Type: text/plain, Size: 657 bytes --]

On Sun, Sep 28, 2014 at 8:05 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> On 09/26/14 19:25, John Fastabend wrote:
>>
>> On 09/26/2014 03:48 PM, Jamal Hadi Salim wrote:
>
>
>>> Hrm  - poking inside there and this seems to be one of those
>>> classifiers that hasnt been getting a lot of love. Doesnt
>>> support actions well for example.
>>>
>>> It seems to work for me:
>>
>>
>> This works for me as well try adding two or more policers and
>> check if they all get printed.
>>
>
> Multiple actions wont work for this classifier. I have time,
> will send patch.
>

Hmm, looks like there is a bug in police dump. Please
try the attached patch.

Thanks.

[-- Attachment #2: cls-fix.diff --]
[-- Type: text/plain, Size: 949 bytes --]

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index c28b0d3..1218900 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -576,16 +576,18 @@ int tcf_exts_dump(struct sk_buff *skb, struct tcf_exts *exts)
 			if (tcf_action_dump(skb, &exts->actions, 0, 0) < 0)
 				goto nla_put_failure;
 			nla_nest_end(skb, nest);
-		} else if (exts->police) {
-			struct tc_action *act = tcf_exts_first_act(exts);
-			nest = nla_nest_start(skb, exts->police);
-			if (nest == NULL || !act)
-				goto nla_put_failure;
-			if (tcf_action_dump_old(skb, act, 0, 0) < 0)
-				goto nla_put_failure;
-			nla_nest_end(skb, nest);
 		}
 	}
+
+	if (exts->police) {
+		struct tc_action *act = tcf_exts_first_act(exts);
+		nest = nla_nest_start(skb, exts->police);
+		if (nest == NULL || !act)
+			goto nla_put_failure;
+		if (tcf_action_dump_old(skb, act, 0, 0) < 0)
+			goto nla_put_failure;
+		nla_nest_end(skb, nest);
+	}
 	return 0;
 
 nla_put_failure:

^ permalink raw reply related

* Re: [PATCH v1 5/5] driver-core: add driver asynchronous probe support
From: Tom Gundersen @ 2014-09-28 17:07 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Greg KH, Dmitry Torokhov, Takashi Iwai, Tejun Heo,
	Arjan van de Ven, Robert Milasan, werner, Oleg Nesterov, hare,
	Benjamin Poirier, Santosh Rastapur, pmladek, dbueso,
	Luis R. Rodriguez, LKML, Tetsuo Handa, Joseph Salisbury,
	Kay Sievers, One Thousand Gnomes, Tim Gardner, Pierre Fersing,
	Andrew Morton, Nagalakshmi Nandigama,
	Praveen Krishnamoorthy <praveen
In-Reply-To: <1411768637-6809-6-git-send-email-mcgrof@do-not-panic.com>

Hi Luis,

Thanks for the patches and the detailed analysis.

Feel free to add

Acked-by: Tom Gundersen <teg@jklm.no>

Minor comments on the commit message below.

On Fri, Sep 26, 2014 at 11:57 PM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>
> Some init systems may wish to express the desire to have
> device drivers run their device driver's bus probe() run
> asynchronously. This implements support for this and
> allows userspace to request async probe as a preference
> through a generic shared device driver module parameter,
> async_probe. Implemention for async probe is supported
> through a module parameter given that since synchronous
> probe has been prevalent for years some userspace might
> exist which relies on the fact that the device driver will
> probe synchronously and the assumption that devices it
> provides will be immediately available after this.
>
> Some device driver might not be able to run async probe
> so we enable device drivers to annotate this to prevent
> this module parameter from having any effect on them.
>
> This implementation uses queue_work(system_unbound_wq)
> to queue async probes, this should enable probe to run
> slightly *faster* if the driver's probe path did not
> have much interaction with other workqueues otherwise
> it may run _slightly_ slower. Tests were done with cxgb4,
> which is known to take long on probe, both without
> having to run request_firmware() [0] and then by
> requiring it to use request_firmware() [1]. The
> difference in run time are only measurable in microseconds:
>
> =====================================================================|
> strategy                                fw (usec)       no-fw (usec) |
> ---------------------------------------------------------------------|
> synchronous                             24472569        1307563      |
> kthread                                 25066415.5      1309868.5    |
> queue_work(system_unbound_wq)           24913661.5      1307631      |
> ---------------------------------------------------------------------|
>
> In practice, in seconds, the difference is barely noticeable:
>
> =====================================================================|
> strategy                                fw (s)          no-fw (s)    |
> ---------------------------------------------------------------------|
> synchronous                             24.47           1.31         |
> kthread                                 25.07           1.31         |
> queue_work(system_unbound_wq)           24.91           1.31         |
> ---------------------------------------------------------------------|
>
> [0] http://ftp.suse.com/pub/people/mcgrof/async-probe/probe-cgxb4-no-firmware.png
> [1] http://ftp.suse.com/pub/people/mcgrof/async-probe/probe-cgxb4-firmware.png
>
> The rest of the commit log documents why this feature was implemented
> primarily first for systemd and things it should consider next.
>
> Systemd has a general timeout for all workers currently set to 180
> seconds after which it will send a sigkill signal. Systemd now has a
> warning which is issued once it reaches 1/3 of the timeout. The original
> motivation for the systemd timeout was to help track device drivers
> which do not use asynch firmware loading on init() and the timeout was
> originally set to 30 seconds.

Please note that the motivation for the timeout in systemd had nothing
to do with async firmware loading (that was just the case where
problems cropped up). The motivation was to not allow udev-workers to
stay around indefinitely, and hence put an upper-bound on
their duration (initially 180 s). At some point the bound was reduced
to 30 seconds to make sure module-loading would bail out before the
kernel's firmware loading timeout would bail out (60s I believe). That
is no longer relevant, which is why it was safe to reset the timeout
to 180 s.

> Since systemd + kernel are heavily tied in for the purposes of this
> patch it is assumed you have merged on systemd the following
> commits:
>
> 671174136525ddf208cdbe75d6d6bd159afa961f        udev: timeout - warn after a third of the timeout before killing
> b5338a19864ac3f5632aee48069a669479621dca        udev: timeout - increase timeout
> 2e92633dbae52f5ac9b7b2e068935990d475d2cd        udev: bump event timeout to 60 seconds
> be2ea723b1d023b3d385d3b791ee4607cbfb20ca        udev: remove userspace firmware loading support
> 9f20a8a376f924c8eb5423cfc1f98644fc1e2d1a        udev: fixup commit
> dd5eddd28a74a49607a8fffcaf960040dba98479        udev: unify event timeout handling
> 9719859c07aa13539ed2cd4b31972cd30f678543        udevd: add --event-timeout commandline option
>
> Since we bundle together serially driver init() and probe()
> on module initialiation systemd's imposed timeout  put a limit on the
> amount of time a driver init() and probe routines can take. There's a
> few overlooked issues with this and the timeout in general:
>
> 0) Not all drivers are killed, the signal is just sent and
>    the kill will only be acted upoon if the driver you loaded
>    happens to have some code path that either uses kthreads (which
>    as of 786235ee are now killable), or uses some code which checks for
>    fatal_signal_pending() on the kernel somewhere -- i.e: pci_read_vpd().

Shouldn't this be seen as something to be fixed in the kernel? I mean,
do we not want userspace to have the possibility to kill udev/modprobe
even disregarding the worker timeouts (say at shutdown, or before
switching from the initrd)?

> 1) Since systemd is the only one logging the sigkill debugging that
>    drivers are not loaded or in the worst case *failed to boot* because
>    of a sigkill has proven hard to debug.

Care to clarify this a bit? Are the udev logs somehow unclear? If you
think we can improve the logging from udev, please ping me about that
and I'll sort it out.

> 2) When and if the signal is received by the driver somehow
>    the driver may fail at different points in its initialization
>    and unless all error paths on the driver are implemented
>    perfectly this could mean leaving a device in a half
>    initialized state.
>
> 3) The timeout is penalizing device drivers that take long on
>    probe(), this wasn't the original motivation. Systemd seems
>    to have been under assumption that probe was asynchronous,
>    this perhaps is true as an *objective* and goal for *some
>    subsystems* but by no means is it true that we've been on a wide
>    crusade to ensure this for all device drivers. It may be a good
>    idea for *many* device drivers but penalizing them with a kill
>    for taking long on probe is simply unacceptable specially
>    when the timeout is completely arbitrary.

The point is really not to "penalize" anything, we just need to make
sure we put some sort of restrictions on our workers so they don't
hang around forever.

> 4) The driver core calls probe for *all* devices that a driver can
>    claim and it does so serially, so if a device driver will need
>    to probe 3 devices and if probe on the device driver is synchronous
>    the amount of time that module loading will take will be:
>
>    driver load time = init() + probe for 3 devices serially
>
>    The timeout ultimatley ends up limiting the number of devices that
>    *any* device driver can support based on the following formula:
>
>    number_devices =          systemd_timeout
>                       -------------------------------------
>                          max known probe time for driver
>
>    Lastly since the error value passed down is the value of
>    the probe for the last device probed the module will fail
>    to load and all devices will fail to be available.
>
> In the Linux kernel we don't want to work around the timeout,
> instead systemd must be changed to take all the above into
> consideration when issuing any kills on device drivers, ideally
> the sigkill should be considered to be ignored at least for
> kmod. In addition to this we help systemd by giving it what it
> originally considered was there and enable it to ask device
> drivers to use asynchronous probe. This patch addresses that
> feature.
>
> Systemd should consider enabling async probe on device drivers
> it loads through systemd-udev but probably does not want to
> enable it for modules loaded through systemd-modules-load
> (modules-load.d). At least on my booting enablign async probe
> for all modules fails to boot as such in order to make this
> a bit more useful we whitelist a few buses where it should be
> at least in theory safe to try to enable async probe. This
> way even if systemd tried to ask to enable async probe for all
> its device drivers the kernel won't blindly do this. We also
> have the sync_probe flag which device drivers can themselves
> enable *iff* its known the device driver should never async
> probe.
>
> In order to help *test* things folks can use the bus.safe_mod_async_probe=1
> kernel parameter which will work as if userspace would have
> requested all modules to load with async probe. Daring folks can
> also use bus.force_mod_async_probe=1 which will enable asynch probe
> even on buses not tested in any way yet, if you use that though
> you're on your own.
>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Arjan van de Ven <arjan@linux.intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: Joseph Salisbury <joseph.salisbury@canonical.com>
> Cc: Kay Sievers <kay@vrfy.org>
> Cc: One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk>
> Cc: Tim Gardner <tim.gardner@canonical.com>
> Cc: Pierre Fersing <pierre-fersing@pierref.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: Benjamin Poirier <bpoirier@suse.de>
> Cc: Nagalakshmi Nandigama <nagalakshmi.nandigama@avagotech.com>
> Cc: Praveen Krishnamoorthy <praveen.krishnamoorthy@avagotech.com>
> Cc: Sreekanth Reddy <sreekanth.reddy@avagotech.com>
> Cc: Abhijit Mahajan <abhijit.mahajan@avagotech.com>
> Cc: Casey Leedom <leedom@chelsio.com>
> Cc: Hariprasad S <hariprasad@chelsio.com>
> Cc: Santosh Rastapur <santosh@chelsio.com>
> Cc: MPT-FusionLinux.pdl@avagotech.com
> Cc: linux-scsi@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> ---
>  drivers/base/base.h    |   6 +++
>  drivers/base/bus.c     | 137 +++++++++++++++++++++++++++++++++++++++++++++++--
>  drivers/base/dd.c      |   7 +++
>  include/linux/module.h |   2 +
>  kernel/module.c        |  12 ++++-
>  5 files changed, 159 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/base/base.h b/drivers/base/base.h
> index 251c5d3..24836f1 100644
> --- a/drivers/base/base.h
> +++ b/drivers/base/base.h
> @@ -43,11 +43,17 @@ struct subsys_private {
>  };
>  #define to_subsys_private(obj) container_of(obj, struct subsys_private, subsys.kobj)
>
> +struct driver_attach_work {
> +       struct work_struct work;
> +       struct device_driver *driver;
> +};
> +
>  struct driver_private {
>         struct kobject kobj;
>         struct klist klist_devices;
>         struct klist_node knode_bus;
>         struct module_kobject *mkobj;
> +       struct driver_attach_work *attach_work;
>         struct device_driver *driver;
>  };
>  #define to_driver(obj) container_of(obj, struct driver_private, kobj)
> diff --git a/drivers/base/bus.c b/drivers/base/bus.c
> index a5f41e4..41e321e6 100644
> --- a/drivers/base/bus.c
> +++ b/drivers/base/bus.c
> @@ -85,6 +85,7 @@ static void driver_release(struct kobject *kobj)
>         struct driver_private *drv_priv = to_driver(kobj);
>
>         pr_debug("driver: '%s': %s\n", kobject_name(kobj), __func__);
> +       kfree(drv_priv->attach_work);
>         kfree(drv_priv);
>  }
>
> @@ -662,10 +663,125 @@ static void remove_driver_private(struct device_driver *drv)
>         struct driver_private *priv = drv->p;
>
>         kobject_put(&priv->kobj);
> +       kfree(priv->attach_work);
>         kfree(priv);
>         drv->p = NULL;
>  }
>
> +static void driver_attach_workfn(struct work_struct *work)
> +{
> +       int ret;
> +       struct driver_attach_work *attach_work =
> +               container_of(work, struct driver_attach_work, work);
> +       struct device_driver *drv = attach_work->driver;
> +       ktime_t calltime, delta, rettime;
> +       unsigned long long duration;
> +
> +       calltime = ktime_get();
> +
> +       ret = driver_attach(drv);
> +       if (ret != 0) {
> +               remove_driver_private(drv);
> +               bus_put(drv->bus);
> +       }
> +
> +       rettime = ktime_get();
> +       delta = ktime_sub(rettime, calltime);
> +       duration = (unsigned long long) ktime_to_ns(delta) >> 10;
> +
> +       pr_debug("bus: '%s': add driver %s attach completed after %lld usecs\n",
> +                drv->bus->name, drv->name, duration);
> +}
> +
> +int bus_driver_async_probe(struct device_driver *drv)
> +{
> +       struct driver_private *priv = drv->p;
> +
> +       priv->attach_work = kzalloc(sizeof(struct driver_attach_work),
> +                                   GFP_KERNEL);
> +       if (!priv->attach_work)
> +               return -ENOMEM;
> +
> +       priv->attach_work->driver = drv;
> +       INIT_WORK(&priv->attach_work->work, driver_attach_workfn);
> +
> +       /* Keep this as pr_info() until this is prevalent */
> +       pr_info("bus: '%s': probe for driver %s is run asynchronously\n",
> +                drv->bus->name, drv->name);
> +
> +       queue_work(system_unbound_wq, &priv->attach_work->work);
> +
> +       return 0;
> +}
> +
> +/*
> + */
> +static bool safe_mod_async = false;
> +module_param_named(safe_mod_async_probe, safe_mod_async, bool, 0400);
> +MODULE_PARM_DESC(safe_mod_async_probe,
> +                "Enable async probe on all modules safely");
> +
> +static bool force_mod_async = false;
> +module_param_named(force_mod_async_probe, force_mod_async, bool, 0400);
> +MODULE_PARM_DESC(force_mod_async_probe,
> +                "Force async probe on all modules");
> +
> +/**
> + * drv_enable_async_probe - evaluates if async probe should be used
> + * @drv: device driver to evaluate
> + * @bus: the bus for the device driver
> + *
> + * The driver core supports enabling asynchronous probe on device drivers
> + * by requiring userspace to pass the module parameter "async_probe".
> + * Currently only modules are enabled to use this feature. If a device
> + * driver is known to not work properly with asynchronous probe they
> + * can force disable asynchronous probe from being enabled through
> + * userspace by adding setting sync_probe to true on the @drv. We require
> + * async probe to be requested from userspace given that we have historically
> + * supported synchronous probe and some userspaces may exist which depend
> + * on this functionality. Userspace may wish to use asynchronous probe for
> + * most device drivers but since this can fail boot in practice we only
> + * enable it currently for a set of buses.
> + *
> + * If you'd like to test enabling async probe for all buses whitelisted
> + * you can enable the safe_mod_async_probe module parameter. Note that its
> + * not a good idea to always enable this, in particular you probably don't
> + * want drivers under modules-load.d to use this. This module parameter should
> + * only be used to help test. If you'd like to test even futher you can
> + * use force_mod_async_probe, that will force enable async probe on all
> + * drivers, regardless if its bus type, it should however be used with
> + * caution.
> + */
> +static bool drv_enable_async_probe(struct device_driver *drv,
> +                                  struct bus_type *bus)
> +{
> +       struct module *mod;
> +
> +       if (!drv->owner || drv->sync_probe)
> +               return false;
> +
> +       if (force_mod_async)
> +               return true;
> +
> +       mod = drv->owner;
> +       if (!safe_mod_async && !mod->async_probe_requested)
> +               return false;
> +
> +       /* For now lets avoid stupid bug reports */
> +       if (!strcmp(bus->name, "pci") ||
> +           !strcmp(bus->name, "pci_express") ||
> +           !strcmp(bus->name, "hid") ||
> +           !strcmp(bus->name, "sdio") ||
> +           !strcmp(bus->name, "gameport") ||
> +           !strcmp(bus->name, "mmc") ||
> +           !strcmp(bus->name, "i2c") ||
> +           !strcmp(bus->name, "platform") ||
> +           !strcmp(bus->name, "usb"))
> +               return true;
> +
> +       return false;
> +}
> +
>  /**
>   * bus_add_driver - Add a driver to the bus.
>   * @drv: driver.
> @@ -675,6 +791,7 @@ int bus_add_driver(struct device_driver *drv)
>         struct bus_type *bus;
>         struct driver_private *priv;
>         int error = 0;
> +       bool async_probe = false;
>
>         bus = bus_get(drv->bus);
>         if (!bus)
> @@ -696,11 +813,19 @@ int bus_add_driver(struct device_driver *drv)
>         if (error)
>                 goto out_unregister;
>
> +       async_probe = drv_enable_async_probe(drv, bus);
> +
>         klist_add_tail(&priv->knode_bus, &bus->p->klist_drivers);
>         if (drv->bus->p->drivers_autoprobe) {
> -               error = driver_attach(drv);
> -               if (error)
> -                       goto out_unregister;
> +               if (async_probe) {
> +                       error = bus_driver_async_probe(drv);
> +                       if (error)
> +                               goto out_unregister;
> +               } else {
> +                       error = driver_attach(drv);
> +                       if (error)
> +                               goto out_unregister;
> +               }
>         }
>         module_add_driver(drv->owner, drv);
>
> @@ -1267,6 +1392,12 @@ EXPORT_SYMBOL_GPL(subsys_virtual_register);
>
>  int __init buses_init(void)
>  {
> +       if (unlikely(safe_mod_async))
> +               pr_info("Enabled safe_mod_async -- you may run into issues\n");
> +
> +       if (unlikely(force_mod_async))
> +               pr_info("Enabling force_mod_async -- you're on your own!\n");
> +
>         bus_kset = kset_create_and_add("bus", &bus_uevent_ops, NULL);
>         if (!bus_kset)
>                 return -ENOMEM;
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index e4ffbcf..7999aba 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -507,6 +507,13 @@ static void __device_release_driver(struct device *dev)
>
>         drv = dev->driver;
>         if (drv) {
> +               if (drv->owner && !drv->sync_probe) {
> +                       struct module *mod = drv->owner;
> +                       struct driver_private *priv = drv->p;
> +
> +                       if (mod->async_probe_requested)
> +                               flush_work(&priv->attach_work->work);
> +               }
>                 pm_runtime_get_sync(dev);
>
>                 driver_sysfs_remove(dev);
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 71f282a..1e9e017 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -271,6 +271,8 @@ struct module {
>         bool sig_ok;
>  #endif
>
> +       bool async_probe_requested;
> +
>         /* symbols that will be GPL-only in the near future. */
>         const struct kernel_symbol *gpl_future_syms;
>         const unsigned long *gpl_future_crcs;
> diff --git a/kernel/module.c b/kernel/module.c
> index 88f3d6c..31d71ff 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -3175,8 +3175,16 @@ out:
>  static int unknown_module_param_cb(char *param, char *val, const char *modname,
>                                    void *arg)
>  {
> +       int ret;
> +       struct module *mod = arg;
> +
> +       if (strcmp(param, "async_probe") == 0) {
> +               mod->async_probe_requested = true;
> +               return 0;
> +       }
> +
>         /* Check for magic 'dyndbg' arg */
> -       int ret = ddebug_dyndbg_module_param_cb(param, val, modname);
> +       ret = ddebug_dyndbg_module_param_cb(param, val, modname);
>         if (ret != 0)
>                 pr_warn("%s: unknown parameter '%s' ignored\n", modname, param);
>         return 0;
> @@ -3278,7 +3286,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
>
>         /* Module is ready to execute: parsing args may do that. */
>         after_dashes = parse_args(mod->name, mod->args, mod->kp, mod->num_kp,
> -                                 -32768, 32767, NULL,
> +                                 -32768, 32767, mod,
>                                   unknown_module_param_cb);
>         if (IS_ERR(after_dashes)) {
>                 err = PTR_ERR(after_dashes);
> --
> 2.1.0
>

^ permalink raw reply

* [PATCH 1/1] iproute2: rsvp classifier support for multiple actions
From: Jamal Hadi Salim @ 2014-09-28 16:49 UTC (permalink / raw)
  To: stephen; +Cc: john.fastabend, netdev, Jamal Hadi Salim

From: Jamal Hadi Salim <jhs@mojatatu.com>

Example setup:

sudo tc qdisc del dev eth0 root handle 1:0 prio
sudo tc qdisc add dev eth0 root handle 1:0 prio

sudo tc filter add dev eth0 pref 10 proto ip parent 1:0 \
rsvp session 10.0.0.1 ipproto icmp \
classid 1:1  \
action police rate 1kbit burst 90k pipe \
action ok

tc -s filter show dev eth0 parent 1:0

filter protocol ip pref 10 rsvp
filter protocol ip pref 10 rsvp fh 0x0001100a flowid 1:1 session
10.0.0.1 ipproto icmp
        action order 1:  police 0x5 rate 1Kbit burst 23440b mtu 2Kb
action pipe overhead 0b
ref 1 bind 1
        Action statistics:
        Sent 98000 bytes 1000 pkt (dropped 0, overlimits 761 requeues 0)
        backlog 0b 0p requeues 0

        action order 2: gact action pass
         random type none pass val 0
         index 2 ref 1 bind 1 installed 60 sec used 3 sec
        Action statistics:
        Sent 74578 bytes 761 pkt (dropped 0, overlimits 0 requeues 0)
        backlog 0b 0p requeues 0

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 tc/f_rsvp.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tc/f_rsvp.c b/tc/f_rsvp.c
index 44d3bdf..14f24b1 100644
--- a/tc/f_rsvp.c
+++ b/tc/f_rsvp.c
@@ -261,6 +261,13 @@ static int rsvp_parse_opt(struct filter_util *qu, char *handle, int argc, char *
 			}
 			pinfo.tunnelhdr = tid;
 			pinfo_ok++;
+		} else if (matches(*argv, "action") == 0) {
+			NEXT_ARG();
+			if (parse_action(&argc, &argv, TCA_RSVP_ACT, n)) {
+				fprintf(stderr, "Illegal \"action\"\n");
+				return -1;
+			}
+			continue;
 		} else if (matches(*argv, "police") == 0) {
 			NEXT_ARG();
 			if (parse_police(&argc, &argv, TCA_RSVP_POLICE, n)) {
@@ -385,6 +392,10 @@ static int rsvp_print_opt(struct filter_util *qu, FILE *f, struct rtattr *opt, _
 		SPRINT_BUF(b2);
 		fprintf(f, "sender [NONE]%s ", sprint_spi(&pinfo->spi, 0, b2));
 	}
+
+	if (tb[TCA_RSVP_ACT]) {
+		tc_print_action(f, tb[TCA_RSVP_ACT]);
+	}
 	if (tb[TCA_RSVP_POLICE])
 		tc_print_police(f, tb[TCA_RSVP_POLICE]);
 	return 0;
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH net-next] mlx4: optimize xmit path
From: Eric Dumazet @ 2014-09-28 16:03 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: Alexei Starovoitov, David S. Miller, Jesper Dangaard Brouer,
	Eric Dumazet, John Fastabend, Linux Netdev List, Amir Vadai,
	Or Gerlitz, saeedm, Yevgeny Petrilin, idos
In-Reply-To: <CAJ3xEMiVXonPkJsve2UuWtTs2wqBnV_jrYMuJ1yvccBTeAFM2w@mail.gmail.com>

On Sun, 2014-09-28 at 17:35 +0300, Or Gerlitz wrote:
> On Sun, Sep 28, 2014 at 1:56 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > From: Eric Dumazet <edumazet@google.com>
> >
> > First I implemented skb->xmit_more support, and pktgen throughput
> > went from ~5Mpps to ~10Mpps.
> >
> > Then, looking closely at this driver I found false sharing problems that
> > should be addressed by this patch, as my pktgen now reaches 14.7 Mpps
> > on a single TX queue, with a burst factor of 8.
> >
> > So this patch in a whole permits to improve raw performance on a single
> > TX queue from about 5 Mpps to 14.7 Mpps.
> 
> Eric,
> 
> cool!! the team here will take a look this week. I assume we might
> want to break the fifteen changes into multiple patches...
> 
> Thanks again for all your great work

Another problem I noticed is the false sharing on
prot_stats.tso_packets.

Please add following fix to your queue.

Thanks !

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_port.c b/drivers/net/ethernet/mellanox/mlx4/en_port.c
index c2cfb05e7290..5bd33e580b22 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_port.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_port.c
@@ -150,14 +150,17 @@ int mlx4_en_DUMP_ETH_STATS(struct mlx4_en_dev *mdev, u8 port, u8 reset)
 	priv->port_stats.tx_chksum_offload = 0;
 	priv->port_stats.queue_stopped = 0;
 	priv->port_stats.wake_queue = 0;
+	priv->port_stats.tso_packets = 0;
 
 	for (i = 0; i < priv->tx_ring_num; i++) {
-		stats->tx_packets += priv->tx_ring[i]->packets;
-		stats->tx_bytes += priv->tx_ring[i]->bytes;
-		priv->port_stats.tx_chksum_offload += priv->tx_ring[i]->tx_csum;
-		priv->port_stats.queue_stopped +=
-			priv->tx_ring[i]->queue_stopped;
-		priv->port_stats.wake_queue += priv->tx_ring[i]->wake_queue;
+		struct mlx4_en_tx_ring *ring = priv->tx_ring[i];
+
+		stats->tx_packets += ring->packets;
+		stats->tx_bytes += ring->bytes;
+		priv->port_stats.tx_chksum_offload += ring->tx_csum;
+		priv->port_stats.queue_stopped += ring->queue_stopped;
+		priv->port_stats.wake_queue += ring->wake_queue;
+		priv->port_stats.tso_packets += ring->tso_packets;
 	}
 
 	stats->rx_errors = be64_to_cpu(mlx4_en_stats->PCS) +
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index c44f4237b9be..7bb156e99894 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -839,7 +839,8 @@ netdev_tx_t mlx4_en_xmit(struct sk_buff *skb, struct net_device *dev)
 		 * note that we already verified that it is linear */
 		memcpy(tx_desc->lso.header, skb->data, lso_header_size);
 
-		priv->port_stats.tso_packets++;
+		ring->tso_packets++;
+
 		i = ((skb->len - lso_header_size) / skb_shinfo(skb)->gso_size) +
 			!!((skb->len - lso_header_size) % skb_shinfo(skb)->gso_size);
 		tx_info->nr_bytes = skb->len + (i - 1) * lso_header_size;
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
index 6a4fc2394cf2..007645c4edc0 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
@@ -277,6 +277,7 @@ struct mlx4_en_tx_ring {
 	unsigned long bytes;
 	unsigned long packets;
 	unsigned long tx_csum;
+	unsigned long tso_packets;
 	unsigned long queue_stopped;
 	unsigned long wake_queue;
 	struct mlx4_bf bf;

^ permalink raw reply related

* Re: tc rsvp filter show broke
From: Jamal Hadi Salim @ 2014-09-28 15:05 UTC (permalink / raw)
  To: John Fastabend, John Fastabend; +Cc: Cong Wang, netdev
In-Reply-To: <5425F5CC.4070507@intel.com>

On 09/26/14 19:25, John Fastabend wrote:
> On 09/26/2014 03:48 PM, Jamal Hadi Salim wrote:

>> Hrm  - poking inside there and this seems to be one of those
>> classifiers that hasnt been getting a lot of love. Doesnt
>> support actions well for example.
>>
>> It seems to work for me:
>
> This works for me as well try adding two or more policers and
> check if they all get printed.
>

Multiple actions wont work for this classifier. I have time,
will send patch.

cheers,
jamal

^ permalink raw reply

* Re: [PATCH v1 5/5] driver-core: add driver asynchronous probe support
From: Tejun Heo @ 2014-09-28 15:03 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: gregkh, dmitry.torokhov, tiwai, arjan, teg, rmilasan, werner,
	oleg, hare, bpoirier, santosh, pmladek, dbueso, mcgrof,
	linux-kernel, Tetsuo Handa, Joseph Salisbury, Kay Sievers,
	One Thousand Gnomes, Tim Gardner, Pierre Fersing, Andrew Morton,
	Nagalakshmi Nandigama, Praveen Krishnamoorthy, Sreekanth Reddy,
	Abhijit Mahajan, Casey Leedom, Hariprasad S <harip
In-Reply-To: <1411768637-6809-6-git-send-email-mcgrof@do-not-panic.com>

Hello,

On Fri, Sep 26, 2014 at 02:57:17PM -0700, Luis R. Rodriguez wrote:
...
> Systemd should consider enabling async probe on device drivers
> it loads through systemd-udev but probably does not want to
> enable it for modules loaded through systemd-modules-load
> (modules-load.d). At least on my booting enablign async probe
> for all modules fails to boot as such in order to make this

Did you find out why boot failed with those modules?

> a bit more useful we whitelist a few buses where it should be
> at least in theory safe to try to enable async probe. This
> way even if systemd tried to ask to enable async probe for all
> its device drivers the kernel won't blindly do this. We also
> have the sync_probe flag which device drivers can themselves
> enable *iff* its known the device driver should never async
> probe.
> 
> In order to help *test* things folks can use the bus.safe_mod_async_probe=1
> kernel parameter which will work as if userspace would have
> requested all modules to load with async probe. Daring folks can
> also use bus.force_mod_async_probe=1 which will enable asynch probe
> even on buses not tested in any way yet, if you use that though
> you're on your own.

If those two knobs are meant for debugging, let's please make that
fact immediately evident.  e.g. Make them ugly boot params like
"__DEVEL__driver_force_mod_async_probe".  Devel/debug options ending
up becoming stable interface are really nasty.

> +struct driver_attach_work {
> +	struct work_struct work;
> +	struct device_driver *driver;
> +};
> +
>  struct driver_private {
>  	struct kobject kobj;
>  	struct klist klist_devices;
>  	struct klist_node knode_bus;
>  	struct module_kobject *mkobj;
> +	struct driver_attach_work *attach_work;
>  	struct device_driver *driver;
>  };

How many bytes are we saving by allocating it separately?  Can't we
just embed it in driver_private?

> +static void driver_attach_workfn(struct work_struct *work)
> +{
> +	int ret;
> +	struct driver_attach_work *attach_work =
> +		container_of(work, struct driver_attach_work, work);
> +	struct device_driver *drv = attach_work->driver;
> +	ktime_t calltime, delta, rettime;
> +	unsigned long long duration;

This could just be a personal preference but I think it's easier to
read if local vars w/ initializers come before the ones w/o.

> +
> +	calltime = ktime_get();
> +
> +	ret = driver_attach(drv);
> +	if (ret != 0) {
> +		remove_driver_private(drv);
> +		bus_put(drv->bus);
> +	}
> +
> +	rettime = ktime_get();
> +	delta = ktime_sub(rettime, calltime);
> +	duration = (unsigned long long) ktime_to_ns(delta) >> 10;
> +
> +	pr_debug("bus: '%s': add driver %s attach completed after %lld usecs\n",
> +		 drv->bus->name, drv->name, duration);

Why do we have the above printout for async path but not sync path?
It's kinda weird for the code path to diverge like this.  Shouldn't
the only difference be the context probes are running from?

...
> +static bool drv_enable_async_probe(struct device_driver *drv,
> +				   struct bus_type *bus)
> +{
> +	struct module *mod;
> +
> +	if (!drv->owner || drv->sync_probe)
> +		return false;
> +
> +	if (force_mod_async)
> +		return true;
> +
> +	mod = drv->owner;
> +	if (!safe_mod_async && !mod->async_probe_requested)
> +		return false;
> +
> +	/* For now lets avoid stupid bug reports */
> +	if (!strcmp(bus->name, "pci") ||
> +	    !strcmp(bus->name, "pci_express") ||
> +	    !strcmp(bus->name, "hid") ||
> +	    !strcmp(bus->name, "sdio") ||
> +	    !strcmp(bus->name, "gameport") ||
> +	    !strcmp(bus->name, "mmc") ||
> +	    !strcmp(bus->name, "i2c") ||
> +	    !strcmp(bus->name, "platform") ||
> +	    !strcmp(bus->name, "usb"))
> +		return true;

Ugh... things like this tend to become permanent.  Do we really need
this?  And how are we gonna find out what's broken why w/o bug
reports?

> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index e4ffbcf..7999aba 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -507,6 +507,13 @@ static void __device_release_driver(struct device *dev)
>  
>  	drv = dev->driver;
>  	if (drv) {
> +		if (drv->owner && !drv->sync_probe) {
> +			struct module *mod = drv->owner;
> +			struct driver_private *priv = drv->p;
> +
> +			if (mod->async_probe_requested)
> +				flush_work(&priv->attach_work->work);

This can be unconditional flus_work(&priv->attach_work) if attach_work
isn't separately allocated.

>  static int unknown_module_param_cb(char *param, char *val, const char *modname,
>  				   void *arg)
>  {
> +	int ret;
> +	struct module *mod = arg;

Ditto with the order of definitions.

> +	if (strcmp(param, "async_probe") == 0) {
> +		mod->async_probe_requested = true;
> +		return 0;
> +	}

Generally looks good to me.

Thanks a lot for doing this! :)

-- 
tejun

^ permalink raw reply

* Re: [PATCH net-next] mlx4: optimize xmit path
From: Or Gerlitz @ 2014-09-28 14:35 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Alexei Starovoitov, David S. Miller, Jesper Dangaard Brouer,
	Eric Dumazet, John Fastabend, Linux Netdev List, Amir Vadai,
	Or Gerlitz, saeedm, Yevgeny Petrilin, idos
In-Reply-To: <1411858593.15768.51.camel@edumazet-glaptop2.roam.corp.google.com>

On Sun, Sep 28, 2014 at 1:56 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> First I implemented skb->xmit_more support, and pktgen throughput
> went from ~5Mpps to ~10Mpps.
>
> Then, looking closely at this driver I found false sharing problems that
> should be addressed by this patch, as my pktgen now reaches 14.7 Mpps
> on a single TX queue, with a burst factor of 8.
>
> So this patch in a whole permits to improve raw performance on a single
> TX queue from about 5 Mpps to 14.7 Mpps.

Eric,

cool!! the team here will take a look this week. I assume we might
want to break the fifteen changes into multiple patches...

Thanks again for all your great work

Or.

^ permalink raw reply

* Re: [PATCH RFC] drivers: parport: Ask user for irqreturn_t value
From: Matwey V. Kornilov @ 2014-09-28 14:30 UTC (permalink / raw)
  To: Greg KH
  Cc: linux-parport, arnd, Jean Delvare, linux-i2c, dmitry.torokhov,
	linux-input, t.sailer, linux-hams, netdev, giometti, linuxpps,
	m.chehab, linux-media, linux-kernel, Matwey V. Kornilov
In-Reply-To: <1411891335-3677-1-git-send-email-matwey-TTlVxmypnbovJsYlp49lxw@public.gmane.org>

Forget it, I invented something strange.

2014-09-28 12:02 GMT+04:00 Matwey V. Kornilov <matwey-TTlVxmypnbovJsYlp49lxw@public.gmane.org>:
> Current parport_irq_handler behaviour is not correct when IRQ is shared.
> LDDv3 on page 279 requires us:
>
> "Be sure to return IRQ_NONE whenever your handler is called and finds
> the device is not interrupting."
>
> This is not the case of parport_irq_handler.
> Current implementation of IRQ handling use this to optimize shared IRQ processing.
> When an action returns IRQ_HANDLED no other actions are processed.
>
> Modify callback function void (*irq_func)(void *) to irqreturn_t (*irq_func)(void *) and return the value in parport_irq_handler
>
> This also fixes https://bugzilla.kernel.org/show_bug.cgi?id=85221
>
> Signed-off-by: Matwey V. Kornilov <matwey-TTlVxmypnbovJsYlp49lxw@public.gmane.org>
> ---
>  drivers/char/ppdev.c                       |  4 +++-
>  drivers/i2c/busses/i2c-parport.c           |  5 ++++-
>  drivers/input/joystick/walkera0701.c       |  6 ++++--
>  drivers/net/hamradio/baycom_par.c          |  4 +++-
>  drivers/net/plip/plip.c                    |  8 +++++---
>  drivers/parport/share.c                    |  6 ++----
>  drivers/pps/clients/pps_parport.c          |  6 +++---
>  drivers/staging/media/lirc/lirc_parallel.c | 12 +++++++-----
>  include/linux/parport.h                    | 12 ++++++++----
>  9 files changed, 39 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/char/ppdev.c b/drivers/char/ppdev.c
> index ae0b42b..61fec51 100644
> --- a/drivers/char/ppdev.c
> +++ b/drivers/char/ppdev.c
> @@ -264,7 +264,7 @@ static ssize_t pp_write (struct file * file, const char __user * buf,
>         return bytes_written;
>  }
>
> -static void pp_irq (void *private)
> +static irqreturn_t pp_irq (void *private)
>  {
>         struct pp_struct *pp = private;
>
> @@ -275,6 +275,8 @@ static void pp_irq (void *private)
>
>         atomic_inc (&pp->irqc);
>         wake_up_interruptible (&pp->irq_wait);
> +
> +       return IRQ_HANDLED;
>  }
>
>  static int register_device (int minor, struct pp_struct *pp)
> diff --git a/drivers/i2c/busses/i2c-parport.c b/drivers/i2c/busses/i2c-parport.c
> index a27aae2..7549e10 100644
> --- a/drivers/i2c/busses/i2c-parport.c
> +++ b/drivers/i2c/busses/i2c-parport.c
> @@ -151,7 +151,7 @@ static const struct i2c_algo_bit_data parport_algo_data = {
>
>  /* ----- I2c and parallel port call-back functions and structures --------- */
>
> -static void i2c_parport_irq(void *data)
> +static irqreturn_t i2c_parport_irq(void *data)
>  {
>         struct i2c_par *adapter = data;
>         struct i2c_client *ara = adapter->ara;
> @@ -159,9 +159,12 @@ static void i2c_parport_irq(void *data)
>         if (ara) {
>                 dev_dbg(&ara->dev, "SMBus alert received\n");
>                 i2c_handle_smbus_alert(ara);
> +               return IRQ_HANDLED;
>         } else
>                 dev_dbg(&adapter->adapter.dev,
>                         "SMBus alert received but no ARA client!\n");
> +
> +       return IRQ_NONE;
>  }
>
>  static void i2c_parport_attach(struct parport *port)
> diff --git a/drivers/input/joystick/walkera0701.c b/drivers/input/joystick/walkera0701.c
> index b76ac58..f3903a6 100644
> --- a/drivers/input/joystick/walkera0701.c
> +++ b/drivers/input/joystick/walkera0701.c
> @@ -124,7 +124,7 @@ static inline int read_ack(struct pardevice *p)
>  }
>
>  /* falling edge, prepare to BIN value calculation */
> -static void walkera0701_irq_handler(void *handler_data)
> +static irqreturn_t walkera0701_irq_handler(void *handler_data)
>  {
>         u64 pulse_time;
>         struct walkera_dev *w = handler_data;
> @@ -136,7 +136,7 @@ static void walkera0701_irq_handler(void *handler_data)
>         /* cancel timer, if in handler or active do resync */
>         if (unlikely(0 != hrtimer_try_to_cancel(&w->timer))) {
>                 w->counter = NO_SYNC;
> -               return;
> +               return IRQ_HANDLED;
>         }
>
>         if (w->counter < NO_SYNC) {
> @@ -166,6 +166,8 @@ static void walkera0701_irq_handler(void *handler_data)
>                 w->counter = 0;
>
>         hrtimer_start(&w->timer, ktime_set(0, BIN_SAMPLE), HRTIMER_MODE_REL);
> +
> +       return IRQ_HANDLED;
>  }
>
>  static enum hrtimer_restart timer_handler(struct hrtimer
> diff --git a/drivers/net/hamradio/baycom_par.c b/drivers/net/hamradio/baycom_par.c
> index acb6369..1fd9a7b 100644
> --- a/drivers/net/hamradio/baycom_par.c
> +++ b/drivers/net/hamradio/baycom_par.c
> @@ -269,7 +269,7 @@ static __inline__ void par96_rx(struct net_device *dev, struct baycom_state *bc)
>
>  /* --------------------------------------------------------------------- */
>
> -static void par96_interrupt(void *dev_id)
> +static irqreturn_t par96_interrupt(void *dev_id)
>  {
>         struct net_device *dev = dev_id;
>         struct baycom_state *bc = netdev_priv(dev);
> @@ -292,6 +292,8 @@ static void par96_interrupt(void *dev_id)
>         hdlcdrv_transmitter(dev, &bc->hdrv);
>         hdlcdrv_receiver(dev, &bc->hdrv);
>          local_irq_disable();
> +
> +       return IRQ_HANDLED;
>  }
>
>  /* --------------------------------------------------------------------- */
> diff --git a/drivers/net/plip/plip.c b/drivers/net/plip/plip.c
> index 040b897..00a8b3a 100644
> --- a/drivers/net/plip/plip.c
> +++ b/drivers/net/plip/plip.c
> @@ -143,7 +143,7 @@ static void plip_bh(struct work_struct *work);
>  static void plip_timer_bh(struct work_struct *work);
>
>  /* Interrupt handler */
> -static void plip_interrupt(void *dev_id);
> +static irqreturn_t plip_interrupt(void *dev_id);
>
>  /* Functions for DEV methods */
>  static int plip_tx_packet(struct sk_buff *skb, struct net_device *dev);
> @@ -900,7 +900,7 @@ plip_error(struct net_device *dev, struct net_local *nl,
>  }
>
>  /* Handle the parallel port interrupts. */
> -static void
> +static irqreturn_t
>  plip_interrupt(void *dev_id)
>  {
>         struct net_device *dev = dev_id;
> @@ -919,7 +919,7 @@ plip_interrupt(void *dev_id)
>                 if ((dev->irq != -1) && (net_debug > 1))
>                         printk(KERN_DEBUG "%s: spurious interrupt\n", dev->name);
>                 spin_unlock_irqrestore (&nl->lock, flags);
> -               return;
> +               return IRQ_NONE;
>         }
>
>         if (net_debug > 3)
> @@ -948,6 +948,8 @@ plip_interrupt(void *dev_id)
>         }
>
>         spin_unlock_irqrestore(&nl->lock, flags);
> +
> +       return IRQ_HANDLED;
>  }
>
>  static int
> diff --git a/drivers/parport/share.c b/drivers/parport/share.c
> index 3fa6624..1fd8dc2 100644
> --- a/drivers/parport/share.c
> +++ b/drivers/parport/share.c
> @@ -523,7 +523,7 @@ void parport_remove_port(struct parport *port)
>  struct pardevice *
>  parport_register_device(struct parport *port, const char *name,
>                         int (*pf)(void *), void (*kf)(void *),
> -                       void (*irq_func)(void *),
> +                       irqreturn_t (*irq_func)(void *),
>                         int flags, void *handle)
>  {
>         struct pardevice *tmp;
> @@ -1006,9 +1006,7 @@ irqreturn_t parport_irq_handler(int irq, void *dev_id)
>  {
>         struct parport *port = dev_id;
>
> -       parport_generic_irq(port);
> -
> -       return IRQ_HANDLED;
> +       return parport_generic_irq(port);
>  }
>
>  /* Exported symbols for modules. */
> diff --git a/drivers/pps/clients/pps_parport.c b/drivers/pps/clients/pps_parport.c
> index 38a8bbe..9acf376 100644
> --- a/drivers/pps/clients/pps_parport.c
> +++ b/drivers/pps/clients/pps_parport.c
> @@ -64,7 +64,7 @@ static inline int signal_is_set(struct parport *port)
>  }
>
>  /* parport interrupt handler */
> -static void parport_irq(void *handle)
> +static irqreturn_t parport_irq(void *handle)
>  {
>         struct pps_event_time ts_assert, ts_clear;
>         struct pps_client_pp *dev = handle;
> @@ -122,7 +122,7 @@ out_assert:
>         /* fire assert event */
>         pps_event(dev->pps, &ts_assert,
>                         PPS_CAPTUREASSERT, NULL);
> -       return;
> +       return IRQ_HANDLED;
>
>  out_both:
>         /* fire assert event */
> @@ -131,7 +131,7 @@ out_both:
>         /* fire clear event */
>         pps_event(dev->pps, &ts_clear,
>                         PPS_CAPTURECLEAR, NULL);
> -       return;
> +       return IRQ_HANDLED;
>  }
>
>  static void parport_attach(struct parport *port)
> diff --git a/drivers/staging/media/lirc/lirc_parallel.c b/drivers/staging/media/lirc/lirc_parallel.c
> index 672858a..63fdc27 100644
> --- a/drivers/staging/media/lirc/lirc_parallel.c
> +++ b/drivers/staging/media/lirc/lirc_parallel.c
> @@ -219,7 +219,7 @@ static void rbuf_write(int signal)
>         wptr = nwptr;
>  }
>
> -static void lirc_lirc_irq_handler(void *blah)
> +static irqreturn_t lirc_lirc_irq_handler(void *blah)
>  {
>         struct timeval tv;
>         static struct timeval lasttv;
> @@ -230,10 +230,10 @@ static void lirc_lirc_irq_handler(void *blah)
>         unsigned int timeout;
>
>         if (!is_open)
> -               return;
> +               return IRQ_NONE;
>
>         if (!is_claimed)
> -               return;
> +               return IRQ_NONE;
>
>  #if 0
>         /* disable interrupt */
> @@ -241,7 +241,7 @@ static void lirc_lirc_irq_handler(void *blah)
>           out(LIRC_PORT_IRQ, in(LIRC_PORT_IRQ) & (~LP_PINTEN));
>  #endif
>         if (check_pselecd && (in(1) & LP_PSELECD))
> -               return;
> +               return IRQ_NONE;
>
>  #ifdef LIRC_TIMER
>         if (init) {
> @@ -265,7 +265,7 @@ static void lirc_lirc_irq_handler(void *blah)
>                          */
>                         timer = init_lirc_timer();
>                         /* enable_irq(irq); */
> -                       return;
> +                       return IRQ_HANDLED;
>                 }
>                 init = 1;
>         }
> @@ -314,6 +314,8 @@ static void lirc_lirc_irq_handler(void *blah)
>           enable_irq(irq);
>           out(LIRC_PORT_IRQ, in(LIRC_PORT_IRQ)|LP_PINTEN);
>         */
> +
> +       return IRQ_HANDLED;
>  }
>
>  /*** file operations ***/
> diff --git a/include/linux/parport.h b/include/linux/parport.h
> index c22f125..2ab774c 100644
> --- a/include/linux/parport.h
> +++ b/include/linux/parport.h
> @@ -141,7 +141,7 @@ struct pardevice {
>         int (*preempt)(void *);
>         void (*wakeup)(void *);
>         void *private;
> -       void (*irq_func)(void *);
> +       irqreturn_t (*irq_func)(void *);
>         unsigned int flags;
>         struct pardevice *next;
>         struct pardevice *prev;
> @@ -298,7 +298,7 @@ extern void parport_put_port (struct parport *);
>  struct pardevice *parport_register_device(struct parport *port,
>                           const char *name,
>                           int (*pf)(void *), void (*kf)(void *),
> -                         void (*irq_func)(void *),
> +                         irqreturn_t (*irq_func)(void *),
>                           int flags, void *handle);
>
>  /* parport_unregister unlinks a device from the chain. */
> @@ -429,13 +429,17 @@ extern void parport_daisy_deselect_all (struct parport *port);
>  extern int parport_daisy_select (struct parport *port, int daisy, int mode);
>
>  /* Lowlevel drivers _can_ call this support function to handle irqs.  */
> -static inline void parport_generic_irq(struct parport *port)
> +static inline irqreturn_t parport_generic_irq(struct parport *port)
>  {
> +       irqreturn_t ret = IRQ_NONE;
> +
>         parport_ieee1284_interrupt (port);
>         read_lock(&port->cad_lock);
>         if (port->cad && port->cad->irq_func)
> -               port->cad->irq_func(port->cad->private);
> +               ret = port->cad->irq_func(port->cad->private);
>         read_unlock(&port->cad_lock);
> +
> +       return ret;
>  }
>
>  /* Prototypes from parport_procfs */
> --
> 2.1.0
>



-- 
With best regards,
Matwey V. Kornilov.
Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia

^ permalink raw reply

* RE: [PATCH net] tcp: Fix TLP implementation in case receive window limits send
From: Itzcak Pechtalt @ 2014-09-28 14:26 UTC (permalink / raw)
  To: Neal Cardwell, Nandita Dukkipati; +Cc: Yuchung Cheng, netdev@vger.kernel.org
In-Reply-To: <CADVnQy=KofBtktTj+ZW6ZzbkoAxdq_Ks8sehdtQNbO00EbvfdQ@mail.gmail.com>



> -----Original Message-----
> From: Neal Cardwell [mailto:ncardwell@google.com]
> Sent: Wednesday, September 24, 2014 10:25 PM
> To: Nandita Dukkipati
> Cc: Yuchung Cheng; Itzcak Pechtalt; netdev@vger.kernel.org
> Subject: Re: [PATCH net] tcp: Fix TLP implementation in case receive
> window limits send
> 
> On Wed, Sep 24, 2014 at 3:00 PM, Nandita Dukkipati
> <nanditad@google.com> wrote:
> >> On Tue, Sep 23, 2014 at 9:28 AM, Itzcak Pechtalt
> >> <itzcak@flashnetworks.com> wrote:
> >>> From: Itzcak Pechtalt <itzcak@flashnetworks.com>
> >>>
> >>> TCP Tail loss probe (TLP) algorithm implementation has some problem.
> >>> According to paper (draft-dukkipati-tcpm-tcp-loss-probe-0 ):
> >>> In case recive window of receiver limits send of new packet in probe
> >>> time than a retransmit of last packet send should be done.
> >>>
> >>> Actually, return code from tcp_write_xmit is not checked and only
> >>> RTO is scheduled, So, it will take more time for reovery in this
> >>> case than without TLP.
> >>>
> >>> Signed-off-by: Itzcak Pechtalt <itzcak@flashnetworks.com>
> >> Acked-by: Yuchung Cheng <ycheng@google.com>
> > Acked-by: Nandita Dukkipati <nanditad@google.com>
> 
> Acked-by: Neal Cardwell <ncardwell@google.com>
> 
> BTW, Itzcak, have you been able to construct a test case where this patch
> now allows a TLP in a case where the sender is limited by the receive
> window?
> 
> Often we will be prevented from doing any kind of TLP if we are limited by
> the receive window, due to these lines in
> tcp_schedule_loss_probe():
> 
>   if ((tp->snd_cwnd > tcp_packets_in_flight(tp)) &&
>        tcp_send_head(sk))
>           return false;
> 
> But there is something else going on as well, since I haven't been able to
> force a TLP even when cwnd == rwin == tcp_packets_in_flight() == 10. But I
> haven't had much time to spend on it.
> 
> neal

The scenario will be  in case the sender process calls "send" per packet size data, 
so when tcp_schedule_loss_probe  is called there is no additional data 
(tcp_send_head(sk)) returns false),  but further calls to send fail due to receive 
window limit  (The same is right if data bulk size ended exactly with receive 
window limit).

But really I can't understand what is the goal of the above 3 lines of code.
If send was stopped from any reason, why not schedule TLP timer?
Anyway next send will clear the timer by tcp_event_new_data_sent function.

What is your opinion about another patch to remove these 3 lines of code?

Itzcak

  

^ permalink raw reply

* [PATCH] Fix getsockopt(SO_PEERNAME) buffer size against potential future buffer overflow
From: Samuel Thibault @ 2014-09-28 13:55 UTC (permalink / raw)
  To: linux-kernel, davem, netdev; +Cc: akpm

In net/core/sock.c's sock_getsockopt, the address buffer size is
hardcoded to 128. It happens that sizeof(struct sockaddr_storage) is
indeed 128, but that is just luck and would probably not get updated
whenever sockaddr_storage would grow. This patch makes it simply use
sockaddr_storage instead.

Signed-off-by:  Samuel Thibault <samuel.thibault@ens-lyon.org>

--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1155,13 +1155,13 @@ int sock_getsockopt(struct socket *sock,
 
 	case SO_PEERNAME:
 	{
-		char address[128];
+		struct sockaddr_storage address;
 
-		if (sock->ops->getname(sock, (struct sockaddr *)address, &lv, 2))
+		if (sock->ops->getname(sock, (struct sockaddr *)&address, &lv, 2))
 			return -ENOTCONN;
 		if (lv < len)
 			return -EINVAL;
-		if (copy_to_user(optval, address, len))
+		if (copy_to_user(optval, &address, len))
 			return -EFAULT;
 		goto lenout;
 	}

^ permalink raw reply

* Re: [PATCH net-next] mlx4: optimize xmit path
From: Eric Dumazet @ 2014-09-28 12:42 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: Alexei Starovoitov, David S. Miller, Jesper Dangaard Brouer,
	Eric Dumazet, John Fastabend, Linux Netdev List, Amir Vadai,
	Or Gerlitz
In-Reply-To: <1411858593.15768.51.camel@edumazet-glaptop2.roam.corp.google.com>

On Sat, 2014-09-27 at 15:56 -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>

>  
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---


>  
>  struct mlx4_en_tx_ring {
> +			/* cache line used and dirtied in tx completion
> +			 * (mlx4_en_free_tx_buf())
> +			 */
> +	u32			last_nr_txbb;
> +	u32			cons;
> +
> +			/* cache line used and dirtied in mlx4_en_xmit()
> +			 */
> +	u32			prod ____cacheline_aligned_in_smp;
> +	unsigned long		bytes;
> +	unsigned long		packets;
> +	unsigned long		tx_csum;
> +	struct mlx4_bf		bf;
> +	unsigned long		queue_stopped;
> +	unsigned long		wake_queue;
> +


Hmm, I forgot to move wake_queue in the first cache line.

I also forgot to include <linux/prefetch.h>

I'll send a v2.

^ permalink raw reply

* Re: [PATCH] wireless: Remove unnessary write over of register 11 in rt2800lib.c
From: nick @ 2014-09-28 12:34 UTC (permalink / raw)
  To: Denis Kirjanov, Stanislaw Gruszka
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	users-poMEt7QlJxcwIE2E9O76wjtx2kNaKg5H,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5427FE15.7080100-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>



On 14-09-28 08:24 AM, nick wrote:
> 
> 
> On 14-09-28 04:05 AM, Denis Kirjanov wrote:
>> Or someone with the hardware has to test and check that...
>>
>> On 9/28/14, Stanislaw Gruszka <sgruszka-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>>> On Sat, Sep 27, 2014 at 05:11:41PM -0400, Nicholas Krause wrote:
>>>> This removes the unnecessary  overwrite of register 11 in the function,
>>>> rt2800_config_channel as we are already writing a correct value to the
>>>> register with rt2800_rfcsr_write(rt2x00dev,11.rfcsr).
>>> What make you sure that overwrite is unnecessary ?
>>>
>>>>
>>>> Signed-off-by: Nicholas Krause <yocto6-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> ---
>>>>  drivers/net/wireless/rt2x00/rt2800lib.c | 2 --
>>>>  1 file changed, 2 deletions(-)
>>>>
>>>> diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c
>>>> b/drivers/net/wireless/rt2x00/rt2800lib.c
>>>> index 893c9d5..fb0ae38 100644
>>>> --- a/drivers/net/wireless/rt2x00/rt2800lib.c
>>>> +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
>>>> @@ -2787,8 +2787,6 @@ static void rt2800_config_channel_rf55xx(struct
>>>> rt2x00_dev *rt2x00dev,
>>>>
>>>>  	if (rf->channel <= 14) {
>>>>  		rt2800_rfcsr_write(rt2x00dev, 10, 0x90);
>>>> -		/* FIXME: RF11 owerwrite ? */
>>>> -		rt2800_rfcsr_write(rt2x00dev, 11, 0x4A);
>>> We need a prove that remove is correct, i.e. some reference to
>>> vendor code or spec that current writing register twice with
>>> different values is wrong.
>>>
>>> NACK for the patch.
>>>
>>> Stanislaw
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
> I will see if I can either find the vendor code or spec in order to do this correctly. Thanks for the reply
> through.
> Nick 
> 
In addition someone banned me from the list yet again. I am getting rather annoyed with this and can someone 
please remove it.
Cheers Nick

^ permalink raw reply

* Re: [PATCH] wireless: Remove unnessary write over of register 11 in rt2800lib.c
From: nick @ 2014-09-28 12:24 UTC (permalink / raw)
  To: Denis Kirjanov, Stanislaw Gruszka
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	users-poMEt7QlJxcwIE2E9O76wjtx2kNaKg5H,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAOJe8K1nkeVjNpNoUBxuJv-YXj3Wr_oH86yR31U7w3Jq5j7oxw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>



On 14-09-28 04:05 AM, Denis Kirjanov wrote:
> Or someone with the hardware has to test and check that...
> 
> On 9/28/14, Stanislaw Gruszka <sgruszka-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> On Sat, Sep 27, 2014 at 05:11:41PM -0400, Nicholas Krause wrote:
>>> This removes the unnecessary  overwrite of register 11 in the function,
>>> rt2800_config_channel as we are already writing a correct value to the
>>> register with rt2800_rfcsr_write(rt2x00dev,11.rfcsr).
>> What make you sure that overwrite is unnecessary ?
>>
>>>
>>> Signed-off-by: Nicholas Krause <yocto6-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> ---
>>>  drivers/net/wireless/rt2x00/rt2800lib.c | 2 --
>>>  1 file changed, 2 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c
>>> b/drivers/net/wireless/rt2x00/rt2800lib.c
>>> index 893c9d5..fb0ae38 100644
>>> --- a/drivers/net/wireless/rt2x00/rt2800lib.c
>>> +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
>>> @@ -2787,8 +2787,6 @@ static void rt2800_config_channel_rf55xx(struct
>>> rt2x00_dev *rt2x00dev,
>>>
>>>  	if (rf->channel <= 14) {
>>>  		rt2800_rfcsr_write(rt2x00dev, 10, 0x90);
>>> -		/* FIXME: RF11 owerwrite ? */
>>> -		rt2800_rfcsr_write(rt2x00dev, 11, 0x4A);
>> We need a prove that remove is correct, i.e. some reference to
>> vendor code or spec that current writing register twice with
>> different values is wrong.
>>
>> NACK for the patch.
>>
>> Stanislaw
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
I will see if I can either find the vendor code or spec in order to do this correctly. Thanks for the reply
through.
Nick 

^ permalink raw reply

* Re: IPv6 FIB related crash with MACVLANs in 3.9.11+ kernel.
From: Hannes Frederic Sowa @ 2014-09-28 12:11 UTC (permalink / raw)
  To: Hongmei Li, netdev
In-Reply-To: <loom.20140926T001736-777@post.gmane.org>

Hi Hongmei,

On Fri, Sep 26, 2014, at 00:24, Hongmei Li wrote:
> Hi Hannes and Ben,
> 
> I encounter the same kernel panic.
> Do you guys have any progress on this issue?

Unfortunately not, not looked into it for a long time.

Can you send me a fast reproducer script? I have better machines to
reproduce this issue nowadays.

Thanks,
Hannes

^ permalink raw reply

* Re: [PATCH] wireless: Remove unnessary write over of register 11 in rt2800lib.c
From: Stanislaw Gruszka @ 2014-09-28  8:35 UTC (permalink / raw)
  To: Denis Kirjanov
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	users-poMEt7QlJxcwIE2E9O76wjtx2kNaKg5H,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Nicholas Krause
In-Reply-To: <CAOJe8K1nkeVjNpNoUBxuJv-YXj3Wr_oH86yR31U7w3Jq5j7oxw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Sun, Sep 28, 2014 at 12:05:30PM +0400, Denis Kirjanov wrote:
> Or someone with the hardware has to test and check that...

... on all possible combinations of channel, bandwidth, signal strength, modulation type, ...

Stanislaw

^ permalink raw reply

* Re: [rt2x00-users] [PATCH] wireless: Remove unnessary write over of register 11 in rt2800lib.c
From: Denis Kirjanov @ 2014-09-28  8:05 UTC (permalink / raw)
  To: Stanislaw Gruszka
  Cc: Nicholas Krause, IvDoorn-Re5JQEeQqe8AvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	users-poMEt7QlJxcwIE2E9O76wjtx2kNaKg5H,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20140928070923.GA2087-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Or someone with the hardware has to test and check that...

On 9/28/14, Stanislaw Gruszka <sgruszka-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Sat, Sep 27, 2014 at 05:11:41PM -0400, Nicholas Krause wrote:
>> This removes the unnecessary  overwrite of register 11 in the function,
>> rt2800_config_channel as we are already writing a correct value to the
>> register with rt2800_rfcsr_write(rt2x00dev,11.rfcsr).
> What make you sure that overwrite is unnecessary ?
>
>>
>> Signed-off-by: Nicholas Krause <yocto6-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  drivers/net/wireless/rt2x00/rt2800lib.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c
>> b/drivers/net/wireless/rt2x00/rt2800lib.c
>> index 893c9d5..fb0ae38 100644
>> --- a/drivers/net/wireless/rt2x00/rt2800lib.c
>> +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
>> @@ -2787,8 +2787,6 @@ static void rt2800_config_channel_rf55xx(struct
>> rt2x00_dev *rt2x00dev,
>>
>>  	if (rf->channel <= 14) {
>>  		rt2800_rfcsr_write(rt2x00dev, 10, 0x90);
>> -		/* FIXME: RF11 owerwrite ? */
>> -		rt2800_rfcsr_write(rt2x00dev, 11, 0x4A);
> We need a prove that remove is correct, i.e. some reference to
> vendor code or spec that current writing register twice with
> different values is wrong.
>
> NACK for the patch.
>
> Stanislaw
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
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

* [PATCH RFC] drivers: parport: Ask user for irqreturn_t value
From: Matwey V. Kornilov @ 2014-09-28  8:02 UTC (permalink / raw)
  To: gregkh
  Cc: linux-parport, arnd, jdelvare, linux-i2c, dmitry.torokhov,
	linux-input, t.sailer, linux-hams, netdev, giometti, linuxpps,
	m.chehab, linux-media, linux-kernel, Matwey V. Kornilov

Current parport_irq_handler behaviour is not correct when IRQ is shared.
LDDv3 on page 279 requires us:

"Be sure to return IRQ_NONE whenever your handler is called and finds
the device is not interrupting."

This is not the case of parport_irq_handler.
Current implementation of IRQ handling use this to optimize shared IRQ processing.
When an action returns IRQ_HANDLED no other actions are processed.

Modify callback function void (*irq_func)(void *) to irqreturn_t (*irq_func)(void *) and return the value in parport_irq_handler

This also fixes https://bugzilla.kernel.org/show_bug.cgi?id=85221

Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
---
 drivers/char/ppdev.c                       |  4 +++-
 drivers/i2c/busses/i2c-parport.c           |  5 ++++-
 drivers/input/joystick/walkera0701.c       |  6 ++++--
 drivers/net/hamradio/baycom_par.c          |  4 +++-
 drivers/net/plip/plip.c                    |  8 +++++---
 drivers/parport/share.c                    |  6 ++----
 drivers/pps/clients/pps_parport.c          |  6 +++---
 drivers/staging/media/lirc/lirc_parallel.c | 12 +++++++-----
 include/linux/parport.h                    | 12 ++++++++----
 9 files changed, 39 insertions(+), 24 deletions(-)

diff --git a/drivers/char/ppdev.c b/drivers/char/ppdev.c
index ae0b42b..61fec51 100644
--- a/drivers/char/ppdev.c
+++ b/drivers/char/ppdev.c
@@ -264,7 +264,7 @@ static ssize_t pp_write (struct file * file, const char __user * buf,
 	return bytes_written;
 }
 
-static void pp_irq (void *private)
+static irqreturn_t pp_irq (void *private)
 {
 	struct pp_struct *pp = private;
 
@@ -275,6 +275,8 @@ static void pp_irq (void *private)
 
 	atomic_inc (&pp->irqc);
 	wake_up_interruptible (&pp->irq_wait);
+	
+	return IRQ_HANDLED;
 }
 
 static int register_device (int minor, struct pp_struct *pp)
diff --git a/drivers/i2c/busses/i2c-parport.c b/drivers/i2c/busses/i2c-parport.c
index a27aae2..7549e10 100644
--- a/drivers/i2c/busses/i2c-parport.c
+++ b/drivers/i2c/busses/i2c-parport.c
@@ -151,7 +151,7 @@ static const struct i2c_algo_bit_data parport_algo_data = {
 
 /* ----- I2c and parallel port call-back functions and structures --------- */
 
-static void i2c_parport_irq(void *data)
+static irqreturn_t i2c_parport_irq(void *data)
 {
 	struct i2c_par *adapter = data;
 	struct i2c_client *ara = adapter->ara;
@@ -159,9 +159,12 @@ static void i2c_parport_irq(void *data)
 	if (ara) {
 		dev_dbg(&ara->dev, "SMBus alert received\n");
 		i2c_handle_smbus_alert(ara);
+		return IRQ_HANDLED;
 	} else
 		dev_dbg(&adapter->adapter.dev,
 			"SMBus alert received but no ARA client!\n");
+
+	return IRQ_NONE;
 }
 
 static void i2c_parport_attach(struct parport *port)
diff --git a/drivers/input/joystick/walkera0701.c b/drivers/input/joystick/walkera0701.c
index b76ac58..f3903a6 100644
--- a/drivers/input/joystick/walkera0701.c
+++ b/drivers/input/joystick/walkera0701.c
@@ -124,7 +124,7 @@ static inline int read_ack(struct pardevice *p)
 }
 
 /* falling edge, prepare to BIN value calculation */
-static void walkera0701_irq_handler(void *handler_data)
+static irqreturn_t walkera0701_irq_handler(void *handler_data)
 {
 	u64 pulse_time;
 	struct walkera_dev *w = handler_data;
@@ -136,7 +136,7 @@ static void walkera0701_irq_handler(void *handler_data)
 	/* cancel timer, if in handler or active do resync */
 	if (unlikely(0 != hrtimer_try_to_cancel(&w->timer))) {
 		w->counter = NO_SYNC;
-		return;
+		return IRQ_HANDLED;
 	}
 
 	if (w->counter < NO_SYNC) {
@@ -166,6 +166,8 @@ static void walkera0701_irq_handler(void *handler_data)
 		w->counter = 0;
 
 	hrtimer_start(&w->timer, ktime_set(0, BIN_SAMPLE), HRTIMER_MODE_REL);
+
+	return IRQ_HANDLED;
 }
 
 static enum hrtimer_restart timer_handler(struct hrtimer
diff --git a/drivers/net/hamradio/baycom_par.c b/drivers/net/hamradio/baycom_par.c
index acb6369..1fd9a7b 100644
--- a/drivers/net/hamradio/baycom_par.c
+++ b/drivers/net/hamradio/baycom_par.c
@@ -269,7 +269,7 @@ static __inline__ void par96_rx(struct net_device *dev, struct baycom_state *bc)
 
 /* --------------------------------------------------------------------- */
 
-static void par96_interrupt(void *dev_id)
+static irqreturn_t par96_interrupt(void *dev_id)
 {
 	struct net_device *dev = dev_id;
 	struct baycom_state *bc = netdev_priv(dev);
@@ -292,6 +292,8 @@ static void par96_interrupt(void *dev_id)
 	hdlcdrv_transmitter(dev, &bc->hdrv);
 	hdlcdrv_receiver(dev, &bc->hdrv);
         local_irq_disable();
+
+	return IRQ_HANDLED;
 }
 
 /* --------------------------------------------------------------------- */
diff --git a/drivers/net/plip/plip.c b/drivers/net/plip/plip.c
index 040b897..00a8b3a 100644
--- a/drivers/net/plip/plip.c
+++ b/drivers/net/plip/plip.c
@@ -143,7 +143,7 @@ static void plip_bh(struct work_struct *work);
 static void plip_timer_bh(struct work_struct *work);
 
 /* Interrupt handler */
-static void plip_interrupt(void *dev_id);
+static irqreturn_t plip_interrupt(void *dev_id);
 
 /* Functions for DEV methods */
 static int plip_tx_packet(struct sk_buff *skb, struct net_device *dev);
@@ -900,7 +900,7 @@ plip_error(struct net_device *dev, struct net_local *nl,
 }
 
 /* Handle the parallel port interrupts. */
-static void
+static irqreturn_t
 plip_interrupt(void *dev_id)
 {
 	struct net_device *dev = dev_id;
@@ -919,7 +919,7 @@ plip_interrupt(void *dev_id)
 		if ((dev->irq != -1) && (net_debug > 1))
 			printk(KERN_DEBUG "%s: spurious interrupt\n", dev->name);
 		spin_unlock_irqrestore (&nl->lock, flags);
-		return;
+		return IRQ_NONE;
 	}
 
 	if (net_debug > 3)
@@ -948,6 +948,8 @@ plip_interrupt(void *dev_id)
 	}
 
 	spin_unlock_irqrestore(&nl->lock, flags);
+
+	return IRQ_HANDLED;
 }
 
 static int
diff --git a/drivers/parport/share.c b/drivers/parport/share.c
index 3fa6624..1fd8dc2 100644
--- a/drivers/parport/share.c
+++ b/drivers/parport/share.c
@@ -523,7 +523,7 @@ void parport_remove_port(struct parport *port)
 struct pardevice *
 parport_register_device(struct parport *port, const char *name,
 			int (*pf)(void *), void (*kf)(void *),
-			void (*irq_func)(void *), 
+			irqreturn_t (*irq_func)(void *),
 			int flags, void *handle)
 {
 	struct pardevice *tmp;
@@ -1006,9 +1006,7 @@ irqreturn_t parport_irq_handler(int irq, void *dev_id)
 {
 	struct parport *port = dev_id;
 
-	parport_generic_irq(port);
-
-	return IRQ_HANDLED;
+	return parport_generic_irq(port);
 }
 
 /* Exported symbols for modules. */
diff --git a/drivers/pps/clients/pps_parport.c b/drivers/pps/clients/pps_parport.c
index 38a8bbe..9acf376 100644
--- a/drivers/pps/clients/pps_parport.c
+++ b/drivers/pps/clients/pps_parport.c
@@ -64,7 +64,7 @@ static inline int signal_is_set(struct parport *port)
 }
 
 /* parport interrupt handler */
-static void parport_irq(void *handle)
+static irqreturn_t parport_irq(void *handle)
 {
 	struct pps_event_time ts_assert, ts_clear;
 	struct pps_client_pp *dev = handle;
@@ -122,7 +122,7 @@ out_assert:
 	/* fire assert event */
 	pps_event(dev->pps, &ts_assert,
 			PPS_CAPTUREASSERT, NULL);
-	return;
+	return IRQ_HANDLED;
 
 out_both:
 	/* fire assert event */
@@ -131,7 +131,7 @@ out_both:
 	/* fire clear event */
 	pps_event(dev->pps, &ts_clear,
 			PPS_CAPTURECLEAR, NULL);
-	return;
+	return IRQ_HANDLED;
 }
 
 static void parport_attach(struct parport *port)
diff --git a/drivers/staging/media/lirc/lirc_parallel.c b/drivers/staging/media/lirc/lirc_parallel.c
index 672858a..63fdc27 100644
--- a/drivers/staging/media/lirc/lirc_parallel.c
+++ b/drivers/staging/media/lirc/lirc_parallel.c
@@ -219,7 +219,7 @@ static void rbuf_write(int signal)
 	wptr = nwptr;
 }
 
-static void lirc_lirc_irq_handler(void *blah)
+static irqreturn_t lirc_lirc_irq_handler(void *blah)
 {
 	struct timeval tv;
 	static struct timeval lasttv;
@@ -230,10 +230,10 @@ static void lirc_lirc_irq_handler(void *blah)
 	unsigned int timeout;
 
 	if (!is_open)
-		return;
+		return IRQ_NONE;
 
 	if (!is_claimed)
-		return;
+		return IRQ_NONE;
 
 #if 0
 	/* disable interrupt */
@@ -241,7 +241,7 @@ static void lirc_lirc_irq_handler(void *blah)
 	  out(LIRC_PORT_IRQ, in(LIRC_PORT_IRQ) & (~LP_PINTEN));
 #endif
 	if (check_pselecd && (in(1) & LP_PSELECD))
-		return;
+		return IRQ_NONE;
 
 #ifdef LIRC_TIMER
 	if (init) {
@@ -265,7 +265,7 @@ static void lirc_lirc_irq_handler(void *blah)
 			 */
 			timer = init_lirc_timer();
 			/* enable_irq(irq); */
-			return;
+			return IRQ_HANDLED;
 		}
 		init = 1;
 	}
@@ -314,6 +314,8 @@ static void lirc_lirc_irq_handler(void *blah)
 	  enable_irq(irq);
 	  out(LIRC_PORT_IRQ, in(LIRC_PORT_IRQ)|LP_PINTEN);
 	*/
+
+	return IRQ_HANDLED;
 }
 
 /*** file operations ***/
diff --git a/include/linux/parport.h b/include/linux/parport.h
index c22f125..2ab774c 100644
--- a/include/linux/parport.h
+++ b/include/linux/parport.h
@@ -141,7 +141,7 @@ struct pardevice {
 	int (*preempt)(void *);
 	void (*wakeup)(void *);
 	void *private;
-	void (*irq_func)(void *);
+	irqreturn_t (*irq_func)(void *);
 	unsigned int flags;
 	struct pardevice *next;
 	struct pardevice *prev;
@@ -298,7 +298,7 @@ extern void parport_put_port (struct parport *);
 struct pardevice *parport_register_device(struct parport *port, 
 			  const char *name,
 			  int (*pf)(void *), void (*kf)(void *),
-			  void (*irq_func)(void *), 
+			  irqreturn_t (*irq_func)(void *),
 			  int flags, void *handle);
 
 /* parport_unregister unlinks a device from the chain. */
@@ -429,13 +429,17 @@ extern void parport_daisy_deselect_all (struct parport *port);
 extern int parport_daisy_select (struct parport *port, int daisy, int mode);
 
 /* Lowlevel drivers _can_ call this support function to handle irqs.  */
-static inline void parport_generic_irq(struct parport *port)
+static inline irqreturn_t parport_generic_irq(struct parport *port)
 {
+	irqreturn_t ret = IRQ_NONE;
+
 	parport_ieee1284_interrupt (port);
 	read_lock(&port->cad_lock);
 	if (port->cad && port->cad->irq_func)
-		port->cad->irq_func(port->cad->private);
+		ret = port->cad->irq_func(port->cad->private);
 	read_unlock(&port->cad_lock);
+
+	return ret;
 }
 
 /* Prototypes from parport_procfs */
-- 
2.1.0


^ permalink raw reply related

* Re: [rt2x00-users] [PATCH] wireless: Remove unnessary write over of register 11 in rt2800lib.c
From: Stanislaw Gruszka @ 2014-09-28  7:09 UTC (permalink / raw)
  To: Nicholas Krause; +Cc: IvDoorn, netdev, linux-wireless, users, linux-kernel
In-Reply-To: <1411852301-26857-1-git-send-email-yocto6@gmail.com>

On Sat, Sep 27, 2014 at 05:11:41PM -0400, Nicholas Krause wrote:
> This removes the unnecessary  overwrite of register 11 in the function,
> rt2800_config_channel as we are already writing a correct value to the
> register with rt2800_rfcsr_write(rt2x00dev,11.rfcsr).
What make you sure that overwrite is unnecessary ?

> 
> Signed-off-by: Nicholas Krause <yocto6@gmail.com>
> ---
>  drivers/net/wireless/rt2x00/rt2800lib.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
> index 893c9d5..fb0ae38 100644
> --- a/drivers/net/wireless/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
> @@ -2787,8 +2787,6 @@ static void rt2800_config_channel_rf55xx(struct rt2x00_dev *rt2x00dev,
>  
>  	if (rf->channel <= 14) {
>  		rt2800_rfcsr_write(rt2x00dev, 10, 0x90);
> -		/* FIXME: RF11 owerwrite ? */
> -		rt2800_rfcsr_write(rt2x00dev, 11, 0x4A);
We need a prove that remove is correct, i.e. some reference to 
vendor code or spec that current writing register twice with
different values is wrong.

NACK for the patch.

Stanislaw

^ permalink raw reply

* Re: SKB Documentation Ideas and How to Write Docs for Kernel
From: Mike Galbraith @ 2014-09-28  6:29 UTC (permalink / raw)
  To: nick
  Cc: davem, edumazet, mst, therbert, willemb, fw, Paul.Durrant, tgraf,
	vyasevic, herbert, JBeulich, mszeredi, netdev, linux-kernel
In-Reply-To: <542770B8.3000102@gmail.com>

On Sat, 2014-09-27 at 22:21 -0400, nick wrote: 
> Hello to the maintainers of the file,skb.c ,
> I am wondering about making it easier for new developers including myself by writing a file in the docs on networking
> related to skb_buff and it's various methods of use to developing in the kernel's various networking subsystems. If one of the maintainers would like to help give me feedback on my writing of these docs this would be helpful and make it 
> easier for everyone as from my experience skb_buff is one of the most asked about things in the networking lists and
> would allow people more thing for other work, including the maintainers of lots of the networking code. I understand
> you guys are very busy so feel free to respond whenever you get free time during the next few weeks.

Observation #2, this time public: despite having been told repeatedly
that this is not the way to help, step #1 in your work flow continues to
be to pester people to help YOU.  This repeated pattern of attempts by
you to TAP the community to help YOU really needs to stop.

If you want to work on the linux kernel, get off your butt and get to
work, but whatever you do, STOP trying to engage developers to become
your private tutors.  This list is NOT about developing Nick Krause.

-Mike

^ permalink raw reply

* [PATCH v2 4/4] net: fec: ptp: Enalbe PPS ouput based on ptp clock
From: Luwei Zhou @ 2014-09-28  4:20 UTC (permalink / raw)
  To: davem, richardcochran
  Cc: netdev, shawn.guo, bhutchings, R49496, b38611, b20596, stephen
In-Reply-To: <1411878011-21767-1-git-send-email-b45643@freescale.com>

FEC ptp timer has 4 channel compare/trigger function. It can be used to
enable pps output.
The pulse would be ouput high exactly on N second. The pulse ouput high
on compare event mode is used to produce pulse per second.  The pulse
width would be one cycle based on ptp timer clock source.Since 31-bit
ptp hardware timer is used, the timer will wrap more than 2 seconds. We
need to reload the compare compare event about every 1 second.

Signed-off-by: Luwei Zhou <b45643@freescale.com>
---
 drivers/net/ethernet/freescale/fec.h      |   7 +
 drivers/net/ethernet/freescale/fec_main.c |   2 +
 drivers/net/ethernet/freescale/fec_ptp.c  | 217 +++++++++++++++++++++++++++++-
 3 files changed, 224 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index b2e263e..1dbc014 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -485,12 +485,19 @@ struct fec_enet_private {
 
 	/* ptp clock period in ns*/
 	unsigned int ptp_inc;
+
+	/* pps  */
+	int pps_channel;
+	unsigned int reload_period;
+	int pps_enable;
+	unsigned int next_counter;
 };
 
 void fec_ptp_init(struct platform_device *pdev);
 void fec_ptp_start_cyclecounter(struct net_device *ndev);
 int fec_ptp_set(struct net_device *ndev, struct ifreq *ifr);
 int fec_ptp_get(struct net_device *ndev, struct ifreq *ifr);
+uint fec_ptp_check_pps_event(struct fec_enet_private *fep);
 
 /****************************************************************************/
 #endif /* FEC_H */
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 2c73434..5342f45 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1571,6 +1571,8 @@ fec_enet_interrupt(int irq, void *dev_id)
 		complete(&fep->mdio_done);
 	}
 
+	fec_ptp_check_pps_event(fep);
+
 	return ret;
 }
 
diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
index 036d956..4ac2ff7 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -61,6 +61,24 @@
 #define FEC_T_INC_CORR_MASK             0x00007f00
 #define FEC_T_INC_CORR_OFFSET           8
 
+#define FEC_T_CTRL_PINPER		0x00000080
+#define FEC_T_TF0_MASK			0x00000001
+#define FEC_T_TF0_OFFSET		0
+#define FEC_T_TF1_MASK			0x00000002
+#define FEC_T_TF1_OFFSET		1
+#define FEC_T_TF2_MASK			0x00000004
+#define FEC_T_TF2_OFFSET		2
+#define FEC_T_TF3_MASK			0x00000008
+#define FEC_T_TF3_OFFSET		3
+#define FEC_T_TDRE_MASK			0x00000001
+#define FEC_T_TDRE_OFFSET		0
+#define FEC_T_TMODE_MASK		0x0000003C
+#define FEC_T_TMODE_OFFSET		2
+#define FEC_T_TIE_MASK			0x00000040
+#define FEC_T_TIE_OFFSET		6
+#define FEC_T_TF_MASK			0x00000080
+#define FEC_T_TF_OFFSET			7
+
 #define FEC_ATIME_CTRL		0x400
 #define FEC_ATIME		0x404
 #define FEC_ATIME_EVT_OFFSET	0x408
@@ -69,9 +87,160 @@
 #define FEC_ATIME_INC		0x414
 #define FEC_TS_TIMESTAMP	0x418
 
+#define FEC_TGSR		0x604
+#define FEC_TCSR(n)		(0x608 + n * 0x08)
+#define FEC_TCCR(n)		(0x60C + n * 0x08)
+#define MAX_TIMER_CHANNEL	3
+#define FEC_TMODE_TOGGLE	0x05
+#define FEC_HIGH_PULSE		0x0F
+
 #define FEC_CC_MULT	(1 << 31)
 #define FEC_COUNTER_PERIOD	(1 << 31)
 #define FEC_T_NSEC_PER_SEC	(1000000000UL)
+#define PPS_OUPUT_RELOAD_PERIOD	FEC_T_NSEC_PER_SEC
+
+/**
+ * The default PPS ouput pin configuration.
+ * User can set/get the PPS output channel
+ * via PTP_PIN_SET/GETFUNC ptp interface.
+ */
+static struct ptp_pin_desc fec_pps_pin = {
+	.name = "fec_pps_pin_config",
+	.index = 0,
+	.func = PTP_PF_PPS,
+	.chan = 0,
+};
+
+/**
+ * fec_ptp_enable_pps
+ * @fep: the fec_enet_private structure handle
+ * @enable: enable the channel pps output
+ *
+ * This function enble the PPS ouput on the timer channel.
+ */
+static int fec_ptp_enable_pps(struct fec_enet_private *fep, uint enable)
+{
+	unsigned long flags;
+	u32 val, tempval;
+	int inc;
+	struct timespec ts;
+	u64 ns;
+	u32 remainder;
+	struct ptp_pin_desc *pin_config = fep->ptp_caps.pin_config;
+	val = 0;
+
+	if (pin_config->func != PTP_PF_PPS) {
+		dev_err(&fep->pdev->dev, "No pps pin function\n");
+		return -EINVAL;
+	}
+
+	if (!(fep->hwts_tx_en || fep->hwts_rx_en)) {
+		dev_err(&fep->pdev->dev, "No ptp stack is running\n");
+		return -EINVAL;
+	}
+
+	if (fep->pps_enable == enable)
+		return 0;
+
+	fep->pps_channel = pin_config->chan;
+	fep->reload_period = PPS_OUPUT_RELOAD_PERIOD;
+	inc = fep->ptp_inc;
+
+	spin_lock_irqsave(&fep->tmreg_lock, flags);
+
+	if (enable) {
+		/* clear capture or output compare interrupt status if have.
+		 */
+		writel(FEC_T_TF_MASK, fep->hwp + FEC_TCSR(fep->pps_channel));
+
+		/* It is recommended to doulbe check the TMODE field in the
+		 * TCSR register to be cleared before the first compare counter
+		 * is written into TCCR register. Just add a double check.
+		 */
+		val = readl(fep->hwp + FEC_TCSR(fep->pps_channel));
+		do {
+			val &= ~(FEC_T_TMODE_MASK);
+			writel(val, fep->hwp + FEC_TCSR(fep->pps_channel));
+			val = readl(fep->hwp + FEC_TCSR(fep->pps_channel));
+		} while (val & FEC_T_TMODE_MASK);
+
+		/* Dummy read counter to update the counter */
+		timecounter_read(&fep->tc);
+		/* We want to find the first compare event in the next
+		 * second point. So we need to know what the ptp time
+		 * is now and how many nanoseconds is ahead to get next second.
+		 * The remaining nanosecond ahead before the next second would be
+		 * FEC_T_NSEC_PER_SEC - ts.tv_nsec. Add the remaining nanoseconds
+		 * to current timer would be next second.
+		 */
+		tempval = readl(fep->hwp + FEC_ATIME_CTRL);
+		tempval |= FEC_T_CTRL_CAPTURE;
+		writel(tempval, fep->hwp + FEC_ATIME_CTRL);
+
+		tempval = readl(fep->hwp + FEC_ATIME);
+		/* Convert the ptp local counter to 1588 timestamp */
+		ns = timecounter_cyc2time(&fep->tc, tempval);
+		ts.tv_sec = div_u64_rem(ns, 1000000000ULL, &remainder);
+		ts.tv_nsec = remainder;
+
+		/* The tempval is  less than 3 seconds, and  so val is less than
+		 * 4 seconds. No overflow for 32bit calculation.
+		 */
+		val = FEC_T_NSEC_PER_SEC - (u32)ts.tv_nsec + tempval;
+
+		/* Need to consider the situation that the current time is
+		 * very close to the second point, which means FEC_T_NSEC_PER_SEC
+		 * - ts.tv_nsec is close to be zero(For example 20ns); Since the timer
+		 * is still running when we calculate the first compare event, it is
+		 * possible that the remaining nanoseonds run out before the compare
+		 * counter is calculated and written into TCCR register. To avoid
+		 * this possibility, we will set the compare event to be the next
+		 * of next second. The current setting is 31-bit timer and wrap
+		 * around over 2 seconds. So it is okay to set the next of next
+		 * seond for the timer.
+		 */
+		val += FEC_T_NSEC_PER_SEC;
+
+		/* We add (2 *FEC_T_NSEC_PER_SEC - (u32)ts.tv_nsec) to current
+		 * ptp counter, which maybe cause 32-bit wrap. Since the
+		 * (FEC_T_NSEC_PER_SEC - (u32)ts.tv_nsec) is less than 2 second.
+		 * We can ensure the wrap will not cause issue. If the offset
+		 * is bigger than fep->cc.mask would be a error.
+		 */
+		val &= fep->cc.mask;
+		writel(val, fep->hwp + FEC_TCCR(fep->pps_channel));
+
+		/* Calculate the second the compare event timestamp */
+		fep->next_counter = (val + fep->reload_period) & fep->cc.mask;
+
+		/* * Enable compare event when overflow */
+		val = readl(fep->hwp + FEC_ATIME_CTRL);
+		val |= FEC_T_CTRL_PINPER;
+		writel(val, fep->hwp + FEC_ATIME_CTRL);
+
+		/* Compare channel setting. */
+		val = readl(fep->hwp + FEC_TCSR(fep->pps_channel));
+		val |= (1 << FEC_T_TF_OFFSET | 1 << FEC_T_TIE_OFFSET);
+		val &= ~(1 << FEC_T_TDRE_OFFSET);
+		val &= ~(FEC_T_TMODE_MASK);
+		val |= (FEC_HIGH_PULSE << FEC_T_TMODE_OFFSET);
+		writel(val, fep->hwp + FEC_TCSR(fep->pps_channel));
+
+		/* Write the second compare event timestamp and calculate
+		 * the third timestamp. Refer the TCCR register detail in the spec.
+		 */
+		writel(fep->next_counter, fep->hwp + FEC_TCCR(fep->pps_channel));
+		fep->next_counter = (fep->next_counter + fep->reload_period) & fep->cc.mask;
+	} else {
+		writel(0, fep->hwp + FEC_TCSR(fep->pps_channel));
+	}
+
+	fep->pps_enable = enable;
+	spin_unlock_irqrestore(&fep->tmreg_lock, flags);
+
+	return 0;
+}
+
 /**
  * fec_ptp_read - read raw cycle counter (to be used by time counter)
  * @cc: the cyclecounter structure
@@ -315,6 +484,15 @@ static int fec_ptp_settime(struct ptp_clock_info *ptp,
 static int fec_ptp_enable(struct ptp_clock_info *ptp,
 			  struct ptp_clock_request *rq, int on)
 {
+	struct fec_enet_private *fep =
+	    container_of(ptp, struct fec_enet_private, ptp_caps);
+	int ret = 0;
+
+	if (rq->type == PTP_CLK_REQ_PPS) {
+		ret = fec_ptp_enable_pps(fep, on);
+
+		return ret;
+	}
 	return -EOPNOTSUPP;
 }
 
@@ -428,8 +606,10 @@ void fec_ptp_init(struct platform_device *pdev)
 	fep->ptp_caps.n_alarm = 0;
 	fep->ptp_caps.n_ext_ts = 0;
 	fep->ptp_caps.n_per_out = 0;
-	fep->ptp_caps.n_pins = 0;
-	fep->ptp_caps.pps = 0;
+	fep->ptp_caps.n_pps_chan = MAX_TIMER_CHANNEL;
+	fep->ptp_caps.n_pins = 1;
+	fep->ptp_caps.pin_config = &fec_pps_pin;
+	fep->ptp_caps.pps = 1;
 	fep->ptp_caps.adjfreq = fec_ptp_adjfreq;
 	fep->ptp_caps.adjtime = fec_ptp_adjtime;
 	fep->ptp_caps.gettime = fec_ptp_gettime;
@@ -453,3 +633,36 @@ void fec_ptp_init(struct platform_device *pdev)
 
 	schedule_delayed_work(&fep->time_keep, HZ);
 }
+
+/**
+ * fec_ptp_check_pps_event
+ * @fep: the fec_enet_private structure handle
+ *
+ * This function check the pps event and reload the timer compare counter.
+ */
+uint fec_ptp_check_pps_event(struct fec_enet_private *fep)
+{
+	u32 val;
+	u8 channel = fep->pps_channel;
+	struct ptp_clock_event event;
+
+	val = readl(fep->hwp + FEC_TCSR(channel));
+	if (val & FEC_T_TF_MASK) {
+		/* Write the next next compare(not the next according the spec)
+		 * value to the register
+		 */
+		writel(fep->next_counter, fep->hwp + FEC_TCCR(channel));
+		do {
+			writel(val, fep->hwp + FEC_TCSR(channel));
+		} while (readl(fep->hwp + FEC_TCSR(channel)) & FEC_T_TF_MASK);
+
+		/* Update the counter; */
+		fep->next_counter = (fep->next_counter + fep->reload_period) & fep->cc.mask;
+
+		event.type = PTP_CLOCK_PPS;
+		ptp_clock_event(fep->ptp_clock, &event);
+		return 1;
+	}
+
+	return 0;
+}
-- 
1.9.1

^ permalink raw reply related

* [PATCH v2 3/4] ptp: Add PTP_PF_PPS enumeration to ptp_pin_function.
From: Luwei Zhou @ 2014-09-28  4:20 UTC (permalink / raw)
  To: davem, richardcochran
  Cc: netdev, shawn.guo, bhutchings, R49496, b38611, b20596, stephen
In-Reply-To: <1411878011-21767-1-git-send-email-b45643@freescale.com>

PTP_PIN_SET/GETFUNC interface can be used to specify the channel index
when multi-channel timer is used to ouput PPS. Need to add PPS corresponding
pin function enumeration in the ptp.

Signed-off-by: Luwei Zhou <b45643@freescale.com>
---
 drivers/ptp/ptp_chardev.c        | 8 ++++++++
 include/linux/ptp_clock_kernel.h | 2 ++
 include/uapi/linux/ptp_clock.h   | 1 +
 3 files changed, 11 insertions(+)

diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index f8a7609..a746e5a 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -48,6 +48,10 @@ static int ptp_disable_pinfunc(struct ptp_clock_info *ops,
 		break;
 	case PTP_PF_PHYSYNC:
 		break;
+	case PTP_PF_PPS:
+		rq.type = PTP_CLK_REQ_PPS;
+		err = ops->enable(ops, &rq, 0);
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -85,6 +89,10 @@ int ptp_set_pinfunc(struct ptp_clock *ptp, unsigned int pin,
 		if (chan >= info->n_per_out)
 			return -EINVAL;
 		break;
+	case PTP_PF_PPS:
+		if (chan >= info->n_pps_chan)
+			return -EINVAL;
+		break;
 	case PTP_PF_PHYSYNC:
 		if (chan != 0)
 			return -EINVAL;
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index 0d8ff3f..2a5ad1c 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -49,6 +49,7 @@ struct ptp_clock_request {
  * @n_alarm:   The number of programmable alarms.
  * @n_ext_ts:  The number of external time stamp channels.
  * @n_per_out: The number of programmable periodic signals.
+ * @n_pps_chan: The number of programmable channles for pps output.
  * @n_pins:    The number of programmable pins.
  * @pps:       Indicates whether the clock supports a PPS callback.
  * @pin_config: Array of length 'n_pins'. If the number of
@@ -99,6 +100,7 @@ struct ptp_clock_info {
 	int n_alarm;
 	int n_ext_ts;
 	int n_per_out;
+	int n_pps_chan;
 	int n_pins;
 	int pps;
 	struct ptp_pin_desc *pin_config;
diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
index f0b7bfe..57ba7e7 100644
--- a/include/uapi/linux/ptp_clock.h
+++ b/include/uapi/linux/ptp_clock.h
@@ -86,6 +86,7 @@ enum ptp_pin_function {
 	PTP_PF_EXTTS,
 	PTP_PF_PEROUT,
 	PTP_PF_PHYSYNC,
+	PTP_PF_PPS,
 };
 
 struct ptp_pin_desc {
-- 
1.9.1

^ permalink raw reply related

* [PATCH v2 2/4] net: fec: ptp: Use hardware algorithm to adjust PTP counter.
From: Luwei Zhou @ 2014-09-28  4:20 UTC (permalink / raw)
  To: davem, richardcochran
  Cc: netdev, shawn.guo, bhutchings, R49496, b38611, b20596, stephen
In-Reply-To: <1411878011-21767-1-git-send-email-b45643@freescale.com>

The FEC IP supports hardware adjustment for ptp timer. Refer to the description of
ENET_ATCOR and ENET_ATINC registers in the spec about the hardware adjustment. This
patch uses hardware support to adjust the ptp offset and frequency on the slave side.

Signed-off-by: Luwei Zhou <b45643@freescale.com>
Signed-off-by: Frank Li <Frank.Li@freescale.com>
Signed-off-by: Fugang Duan <b38611@freescale.com>
---
 drivers/net/ethernet/freescale/fec.h     |  3 ++
 drivers/net/ethernet/freescale/fec_ptp.c | 66 ++++++++++++++++++++++++++------
 2 files changed, 57 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 354a309..b2e263e 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -482,6 +482,9 @@ struct fec_enet_private {
 	unsigned int tx_pkts_itr;
 	unsigned int tx_time_itr;
 	unsigned int itr_clk_rate;
+
+	/* ptp clock period in ns*/
+	unsigned int ptp_inc;
 };
 
 void fec_ptp_init(struct platform_device *pdev);
diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
index 8016bdd..036d956 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -71,6 +71,7 @@
 
 #define FEC_CC_MULT	(1 << 31)
 #define FEC_COUNTER_PERIOD	(1 << 31)
+#define FEC_T_NSEC_PER_SEC	(1000000000UL)
 /**
  * fec_ptp_read - read raw cycle counter (to be used by time counter)
  * @cc: the cyclecounter structure
@@ -145,32 +146,59 @@ void fec_ptp_start_cyclecounter(struct net_device *ndev)
  */
 static int fec_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
 {
-	u64 diff;
 	unsigned long flags;
 	int neg_adj = 0;
-	u32 mult = FEC_CC_MULT;
+	u32 i, tmp;
+	u32 corr_inc, corr_period;
+	u32 corr_ns;
+	u64 lhs, rhs;
 
 	struct fec_enet_private *fep =
 	    container_of(ptp, struct fec_enet_private, ptp_caps);
 
+	if (ppb == 0)
+		return 0;
+
 	if (ppb < 0) {
 		ppb = -ppb;
 		neg_adj = 1;
 	}
 
-	diff = mult;
-	diff *= ppb;
-	diff = div_u64(diff, 1000000000ULL);
+	/* In theory, corr_inc/corr_period = ppb/FEC_T_NSEC_PER_SEC;
+	 * Try to find the corr_inc  between 1 to fep->ptp_inc to
+	 * meet adjustment requirement.
+	 */
+	lhs = FEC_T_NSEC_PER_SEC;
+	rhs = (u64)ppb * (u64)fep->ptp_inc;
+	for (i = 1; i <= fep->ptp_inc; i++) {
+		if (lhs >= rhs) {
+			corr_inc = i;
+			corr_period = div_u64(lhs, rhs);
+			break;
+		}
+		lhs += FEC_T_NSEC_PER_SEC;
+	}
+	/* Not found? Set it to high value - double speed
+	 * correct in every clock step.
+	 */
+	if (i > fep->ptp_inc) {
+		corr_inc = fep->ptp_inc;
+		corr_period = 1;
+	}
+
+	if (neg_adj)
+		corr_ns = fep->ptp_inc - corr_inc;
+	else
+		corr_ns = fep->ptp_inc + corr_inc;
 
 	spin_lock_irqsave(&fep->tmreg_lock, flags);
-	/*
-	 * dummy read to set cycle_last in tc to now.
-	 * So use adjusted mult to calculate when next call
-	 * timercounter_read.
-	 */
-	timecounter_read(&fep->tc);
 
-	fep->cc.mult = neg_adj ? mult - diff : mult + diff;
+	tmp = readl(fep->hwp + FEC_ATIME_INC) & FEC_T_INC_MASK;
+	tmp |= corr_ns << FEC_T_INC_CORR_OFFSET;
+	writel(tmp, fep->hwp + FEC_ATIME_INC);
+	writel(corr_period, fep->hwp + FEC_ATIME_CORR);
+	/* dummy read to update the timer. */
+	timecounter_read(&fep->tc);
 
 	spin_unlock_irqrestore(&fep->tmreg_lock, flags);
 
@@ -190,12 +218,19 @@ static int fec_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 	    container_of(ptp, struct fec_enet_private, ptp_caps);
 	unsigned long flags;
 	u64 now;
+	u32 counter;
 
 	spin_lock_irqsave(&fep->tmreg_lock, flags);
 
 	now = timecounter_read(&fep->tc);
 	now += delta;
 
+	/* Get the timer value based on adjusted timestamp.
+	 * Update the counter with the masked value.
+	 */
+	counter = now & fep->cc.mask;
+	writel(counter, fep->hwp + FEC_ATIME);
+
 	/* reset the timecounter */
 	timecounter_init(&fep->tc, &fep->cc, now);
 
@@ -246,6 +281,7 @@ static int fec_ptp_settime(struct ptp_clock_info *ptp,
 
 	u64 ns;
 	unsigned long flags;
+	u32 counter;
 
 	mutex_lock(&fep->ptp_clk_mutex);
 	/* Check the ptp clock */
@@ -256,8 +292,13 @@ static int fec_ptp_settime(struct ptp_clock_info *ptp,
 
 	ns = ts->tv_sec * 1000000000ULL;
 	ns += ts->tv_nsec;
+	/* Get the timer value based on timestamp.
+	 * Update the counter with the masked value.
+	 */
+	counter = ns & fep->cc.mask;
 
 	spin_lock_irqsave(&fep->tmreg_lock, flags);
+	writel(counter, fep->hwp + FEC_ATIME);
 	timecounter_init(&fep->tc, &fep->cc, ns);
 	spin_unlock_irqrestore(&fep->tmreg_lock, flags);
 	mutex_unlock(&fep->ptp_clk_mutex);
@@ -396,6 +437,7 @@ void fec_ptp_init(struct platform_device *pdev)
 	fep->ptp_caps.enable = fec_ptp_enable;
 
 	fep->cycle_speed = clk_get_rate(fep->clk_ptp);
+	fep->ptp_inc = FEC_T_NSEC_PER_SEC / fep->cycle_speed;
 
 	spin_lock_init(&fep->tmreg_lock);
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH v2 1/4] net: fec: ptp: Use the 31-bit ptp timer.
From: Luwei Zhou @ 2014-09-28  4:20 UTC (permalink / raw)
  To: davem, richardcochran
  Cc: netdev, shawn.guo, bhutchings, R49496, b38611, b20596, stephen
In-Reply-To: <1411878011-21767-1-git-send-email-b45643@freescale.com>

When ptp switches from software adjustment to hardware ajustment, linux ptp can't converge.
It is caused by the IP limit. Hardware adjustment logcial have issue when ptp counter
runs over 0x80000000(31 bit counter). The internal IP reference manual already remove 32bit
free-running count support. This patch replace the 32-bit PTP timer with 31-bit.

Signed-off-by: Luwei Zhou <b45643@freescale.com>
Signed-off-by: Frank Li <Frank.Li@freescale.com>
---
 drivers/net/ethernet/freescale/fec_ptp.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
index cca3617..8016bdd 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -70,6 +70,7 @@
 #define FEC_TS_TIMESTAMP	0x418
 
 #define FEC_CC_MULT	(1 << 31)
+#define FEC_COUNTER_PERIOD	(1 << 31)
 /**
  * fec_ptp_read - read raw cycle counter (to be used by time counter)
  * @cc: the cyclecounter structure
@@ -113,14 +114,15 @@ void fec_ptp_start_cyclecounter(struct net_device *ndev)
 	/* 1ns counter */
 	writel(inc << FEC_T_INC_OFFSET, fep->hwp + FEC_ATIME_INC);
 
-	/* use free running count */
-	writel(0, fep->hwp + FEC_ATIME_EVT_PERIOD);
+	/* use 31-bit timer counter */
+	writel(FEC_COUNTER_PERIOD, fep->hwp + FEC_ATIME_EVT_PERIOD);
 
-	writel(FEC_T_CTRL_ENABLE, fep->hwp + FEC_ATIME_CTRL);
+	writel(FEC_T_CTRL_ENABLE | FEC_T_CTRL_PERIOD_RST,
+		fep->hwp + FEC_ATIME_CTRL);
 
 	memset(&fep->cc, 0, sizeof(fep->cc));
 	fep->cc.read = fec_ptp_read;
-	fep->cc.mask = CLOCKSOURCE_MASK(32);
+	fep->cc.mask = CLOCKSOURCE_MASK(31);
 	fep->cc.shift = 31;
 	fep->cc.mult = FEC_CC_MULT;
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH v2 0/4] net: fec: ptp: Add FEC PPS ouput support.
From: Luwei Zhou @ 2014-09-28  4:20 UTC (permalink / raw)
  To: davem, richardcochran
  Cc: netdev, shawn.guo, bhutchings, R49496, b38611, b20596, stephen

These patches use hardeware adjustment to ptp timer and add pps ouput
support.

Change from v1 to v2:
	- Fix the potential 32-bit multiplication overflow issue.
	- Optimize the hareware adjustment code to improve efficiency as Richard suggested
	- Use ptp PTP_PIN_SET/GETFUNC interface to set PPS channel not device tree
	  and add PTP_PF_PPS enumeration
	- Modify comments style
	
Luwei Zhou (4):
  net: fec: ptp: Use the 31-bit ptp timer.
  net: fec: ptp: Use hardware algorithm to adjust PTP counter.
  ptp: Add PTP_PF_PPS enumeration to ptp_pin_function.
  net: fec: ptp: Enalbe PPS ouput based on ptp clock

 drivers/net/ethernet/freescale/fec.h      |  10 +
 drivers/net/ethernet/freescale/fec_main.c |   2 +
 drivers/net/ethernet/freescale/fec_ptp.c  | 293 ++++++++++++++++++++++++++++--
 drivers/ptp/ptp_chardev.c                 |   8 +
 include/linux/ptp_clock_kernel.h          |   2 +
 include/uapi/linux/ptp_clock.h            |   1 +
 6 files changed, 298 insertions(+), 18 deletions(-)

-- 
1.9.1

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox