Re: [PATCH v4 21/28] net: wan: Add framer framework support
Hi Simon,
On Sun, 20 Aug 2023 19:15:11 +0200
Simon Horman wrote:
> On Fri, Aug 18, 2023 at 06:39:15PM +0200, Christophe Leroy wrote:
> > From: Herve Codina
> >
> > A framer is a component in charge of an E1/T1 line interface.
> > Connected usually to a TDM bus, it converts TDM frames to/from E1/T1
> > frames. It also provides information related to the E1/T1 line.
> >
> > The framer framework provides a set of APIs for the framer drivers
> > (framer provider) to create/destroy a framer and APIs for the framer
> > users (framer consumer) to obtain a reference to the framer, and
> > use the framer.
> >
> > This basic implementation provides a framer abstraction for:
> > - power on/off the framer
> > - get the framer status (line state)
> > - be notified on framer status changes
> > - get/set the framer configuration
> >
> > Signed-off-by: Herve Codina
> > Reviewed-by: Christophe Leroy
> > Signed-off-by: Christophe Leroy
>
> Hi Christophe and Herve,
>
> some minor feedback from my side.
>
> ...
>
> > diff --git a/drivers/net/wan/framer/framer-core.c
> > b/drivers/net/wan/framer/framer-core.c
>
> ...
>
> > +/**
> > + * framer_create() - create a new framer
> > + * @dev: device that is creating the new framer
> > + * @node: device node of the framer. default to dev->of_node.
> > + * @ops: function pointers for performing framer operations
> > + *
> > + * Called to create a framer using framer framework.
> > + */
> > +struct framer *framer_create(struct device *dev, struct device_node *node,
> > +const struct framer_ops *ops)
> > +{
> > + int ret;
> > + int id;
> > + struct framer *framer;
>
> Please arrange local variable declarations for Networking code
> using reverse xmas tree order - longest line to shortest.
Yes, will be done in the next iteration.
>
> https://github.com/ecree-solarflare/xmastree is helpful here.
>
> ...
>
> > diff --git a/include/linux/framer/framer-provider.h
> > b/include/linux/framer/framer-provider.h
>
> ...
>
> > +/**
> > + * struct framer_ops - set of function pointers for performing framer
> > operations
> > + * @init: operation to be performed for initializing the framer
> > + * @exit: operation to be performed while exiting
> > + * @power_on: powering on the framer
> > + * @power_off: powering off the framer
> > + * @flags: OR-ed flags (FRAMER_FLAG_*) to ask for core functionality
> > + * - @FRAMER_FLAG_POLL_STATUS:
> > + *Ask the core to perfom a polling to get the framer status
> > and
>
> nit: perfom -> perform
Will be fixed in the next iteration.
>
> checkpatch.pl --codespell is your friend here
>
> > + *notify consumers on change.
> > + *The framer should call @framer_notify_status_change() when it
> > + *detects a status change. This is usally done using
> > interrutps.
>
> nit: usally -> usually
> interrutps -> interrupts
Will be fixed in the next iteration.
>
> ...
>
> > diff --git a/include/linux/framer/framer.h b/include/linux/framer/framer.h
> > new file mode 100644
> > index ..0bee7135142f
> > --- /dev/null
> > +++ b/include/linux/framer/framer.h
> > @@ -0,0 +1,199 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Generic framer header file
> > + *
> > + * Copyright 2023 CS GROUP France
> > + *
> > + * Author: Herve Codina
> > + */
> > +
> > +#ifndef __DRIVERS_FRAMER_H
> > +#define __DRIVERS_FRAMER_H
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +/**
> > + * enum framer_iface - Framer interface
>
> As this is a kernel-doc, please include documentation for
> the defined constants: FRAMER_IFACE_E1 and FRAMER_IFACE_T1.
>
> As flagged by: ./scripts/kernel-doc -none
Will be done in the next iteration.
>
> > + */
> > +enum framer_iface {
> > + FRAMER_IFACE_E1, /* E1 interface */
> > + FRAMER_IFACE_T1, /* T1 interface */
> > +};
> > +
> > +/**
> > + * enum framer_clock_mode - Framer clock mode
>
> Likewise here too.
>
> Also, nit: framer_clock_mode -> framer_clock_type
>
Will be updated (doc and change to framer_clock_type) in the next iteration.
> > + */
> > +enum framer_clock_type {
> > + FRAMER_CLOCK_EXT, /* External clock */
> > + FRAMER_CLOCK_INT, /* Internal clock */
> > +};
> > +
> > +/**
> > + * struct framer_configuration - Framer configuration
>
> nit: framer_configuration -> framer_config
Will be fixed in the next iteration.
>
> > + * @line_iface: Framer line interface
> > + * @clock_mode: Framer clock type
> > + * @clock_rate: Framer clock rate
> > + */
> > +struct framer_config {
> > + enum framer_iface iface;
> > + enum framer_clock_type clock_type;
> > + unsigned long line_clock_rate;
> > +};
> > +
> > +/**
> > + * struct framer_status - Framer status
> > + * @link_is_on: Framer link state. true, the link is on, false, the link
> > is off.
> > + */
> > +struct
Re: [PATCH v4 21/28] net: wan: Add framer framework support
Hi Christophe,
On Mon, 21 Aug 2023 08:02:10 +0200
Christophe JAILLET wrote:
> Le 18/08/2023 à 18:39, Christophe Leroy a écrit :
> > From: Herve Codina
> >
> > A framer is a component in charge of an E1/T1 line interface.
> > Connected usually to a TDM bus, it converts TDM frames to/from E1/T1
> > frames. It also provides information related to the E1/T1 line.
> >
> > The framer framework provides a set of APIs for the framer drivers
> > (framer provider) to create/destroy a framer and APIs for the framer
> > users (framer consumer) to obtain a reference to the framer, and
> > use the framer.
> >
> > This basic implementation provides a framer abstraction for:
> > - power on/off the framer
> > - get the framer status (line state)
> > - be notified on framer status changes
> > - get/set the framer configuration
> >
> > Signed-off-by: Herve Codina
> > Reviewed-by: Christophe Leroy
> > Signed-off-by: Christophe Leroy
> > ---
>
> Hi,
>
> should there be a V5, some nits below.
>
> ...
>
> > +int framer_power_off(struct framer *framer)
> > +{
> > + int ret;
> > +
> > + mutex_lock(>mutex);
> > + if (framer->power_count == 1 && framer->ops->power_off) {
> > + ret = framer->ops->power_off(framer);
>
> ~~
> Useless extra space
Will be remove in the next iteration.
>
> > + if (ret < 0) {
> > + dev_err(>dev, "framer poweroff failed -->
> > %d\n", ret);
> > + mutex_unlock(>mutex);
> > + return ret;
> > + }
> > + }
> > + --framer->power_count;
> > + mutex_unlock(>mutex);
> > + framer_pm_runtime_put(framer);
> > +
> > + if (framer->pwr)
> > + regulator_disable(framer->pwr);
> > +
> > + return 0;
> > +}
>
> ...
>
> > +struct framer *framer_create(struct device *dev, struct device_node *node,
> > +const struct framer_ops *ops)
> > +{
> > + int ret;
> > + int id;
> > + struct framer *framer;
> > +
> > + if (WARN_ON(!dev))
> > + return ERR_PTR(-EINVAL);
> > +
> > + /* get_status() is mandatory if the provider ask for polling status */
> > + if (WARN_ON((ops->flags & FRAMER_FLAG_POLL_STATUS) && !ops->get_status))
> > + return ERR_PTR(-EINVAL);
> > +
> > + framer = kzalloc(sizeof(*framer), GFP_KERNEL);
> > + if (!framer)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + id = ida_simple_get(_ida, 0, 0, GFP_KERNEL);
>
> ida_alloc()?
> (ida_simple_get() is deprecated)
Indeed, ida_alloc() and ida_free() will be used in the next iteration.
>
> > + if (id < 0) {
> > + dev_err(dev, "unable to get id\n");
> > + ret = id;
> > + goto free_framer;
> > + }
> > +
> > + device_initialize(>dev);
> > + mutex_init(>mutex);
> > + INIT_WORK(>notify_status_work, framer_notify_status_work);
> > + INIT_DELAYED_WORK(>polling_work, framer_polling_work);
> > + BLOCKING_INIT_NOTIFIER_HEAD(>notifier_list);
> > +
> > + framer->dev.class = framer_class;
> > + framer->dev.parent = dev;
> > + framer->dev.of_node = node ? node : dev->of_node;
> > + framer->id = id;
> > + framer->ops = ops;
> > +
> > + ret = dev_set_name(>dev, "framer-%s.%d", dev_name(dev), id);
> > + if (ret)
> > + goto put_dev;
> > +
> > + /* framer-supply */
> > + framer->pwr = regulator_get_optional(>dev, "framer");
> > + if (IS_ERR(framer->pwr)) {
> > + ret = PTR_ERR(framer->pwr);
> > + if (ret == -EPROBE_DEFER)
> > + goto put_dev;
> > +
> > + framer->pwr = NULL;
> > + }
> > +
> > + ret = device_add(>dev);
> > + if (ret)
> > + goto put_dev;
> > +
> > + if (pm_runtime_enabled(dev)) {
> > + pm_runtime_enable(>dev);
> > + pm_runtime_no_callbacks(>dev);
> > + }
> > +
> > + return framer;
> > +
> > +put_dev:
> > + put_device(>dev); /* calls framer_release() which frees
> > resources */
> > + return ERR_PTR(ret);
> > +
> > +free_framer:
> > + kfree(framer);
> > + return ERR_PTR(ret);
> > +}
>
> ...
>
> > +void framer_provider_of_unregister(struct framer_provider *framer_provider)
> > +{
> > + mutex_lock(_provider_mutex);
> > + list_del(_provider->list);
> > + of_node_put(framer_provider->dev->of_node);
> > + kfree(framer_provider);
> > + mutex_unlock(_provider_mutex);
>
> If it make sense, of_node_put() and kfree() could maybe be out of the
> mutex, in order to match how things are done in
> __framer_provider_of_register().
Yes, it makes sense.
Both of_node_put() and kfree() will be moved out of the mutex.
>
> > +}
>
> ...
>
> > +static void framer_release(struct device *dev)
> > +{
> > + struct framer *framer;
> > +
> > + framer = dev_to_framer(dev);
> > + regulator_put(framer->pwr);
> > + ida_simple_remove(_ida, framer->id);
>
> ida_free()?
> (ida_simple_remove() is deprecated)
Yes
>
> > + kfree(framer);
> > +}
>
> ...
>
Thanks for the
Re: [PATCH v4 21/28] net: wan: Add framer framework support
On Mon, 21 Aug 2023 05:19:22 + Christophe Leroy wrote: > As I said in the cover letter, this series only fixes critical build > failures that happened when CONFIG_MODULES is set. The purpose was to > allow robots to perform their job up to the end. Other feedback and > comments will be taken into account by Hervé when he is back from holidays. I missed this too, FTR this is unacceptable. Quoting documentation: **Do not** post your patches just to run them through the checks. You must ensure that your patches are ready by testing them locally before posting to the mailing list. The patchwork build bot instance gets overloaded very easily and netdev@vger really doesn't need more traffic if we can help it. See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#patchwork-checks
Re: [PATCH v4 21/28] net: wan: Add framer framework support
On Mon, Aug 21, 2023 at 7:19 AM Christophe Leroy wrote: > Le 20/08/2023 à 23:06, Linus Walleij a écrit : > > On Fri, Aug 18, 2023 at 6:41 PM Christophe Leroy > > wrote: > > > >> From: Herve Codina > >> > >> A framer is a component in charge of an E1/T1 line interface. > >> Connected usually to a TDM bus, it converts TDM frames to/from E1/T1 > >> frames. It also provides information related to the E1/T1 line. > >> > >> The framer framework provides a set of APIs for the framer drivers > >> (framer provider) to create/destroy a framer and APIs for the framer > >> users (framer consumer) to obtain a reference to the framer, and > >> use the framer. > >> > >> This basic implementation provides a framer abstraction for: > >> - power on/off the framer > >> - get the framer status (line state) > >> - be notified on framer status changes > >> - get/set the framer configuration > >> > >> Signed-off-by: Herve Codina > >> Reviewed-by: Christophe Leroy > >> Signed-off-by: Christophe Leroy > > > > I had these review comments, you must have missed them? > > https://lore.kernel.org/netdev/cacrpkdzq9_f6+9csev1l_wgphhujfpayxmjjfjurzszrako...@mail.gmail.com/ > > > > As I said in the cover letter, this series only fixes critical build > failures that happened when CONFIG_MODULES is set. The purpose was to > allow robots to perform their job up to the end. Other feedback and > comments will be taken into account by Hervé when he is back from holidays. Ah I see. I completely missed this. Yours, Linus Walleij
Re: [PATCH v4 21/28] net: wan: Add framer framework support
Le 18/08/2023 à 18:39, Christophe Leroy a écrit :
From: Herve Codina
A framer is a component in charge of an E1/T1 line interface.
Connected usually to a TDM bus, it converts TDM frames to/from E1/T1
frames. It also provides information related to the E1/T1 line.
The framer framework provides a set of APIs for the framer drivers
(framer provider) to create/destroy a framer and APIs for the framer
users (framer consumer) to obtain a reference to the framer, and
use the framer.
This basic implementation provides a framer abstraction for:
- power on/off the framer
- get the framer status (line state)
- be notified on framer status changes
- get/set the framer configuration
Signed-off-by: Herve Codina
Reviewed-by: Christophe Leroy
Signed-off-by: Christophe Leroy
---
Hi,
should there be a V5, some nits below.
...
+int framer_power_off(struct framer *framer)
+{
+ int ret;
+
+ mutex_lock(>mutex);
+ if (framer->power_count == 1 && framer->ops->power_off) {
+ ret = framer->ops->power_off(framer);
~~
Useless extra space
+ if (ret < 0) {
+ dev_err(>dev, "framer poweroff failed -->
%d\n", ret);
+ mutex_unlock(>mutex);
+ return ret;
+ }
+ }
+ --framer->power_count;
+ mutex_unlock(>mutex);
+ framer_pm_runtime_put(framer);
+
+ if (framer->pwr)
+ regulator_disable(framer->pwr);
+
+ return 0;
+}
...
+struct framer *framer_create(struct device *dev, struct device_node *node,
+const struct framer_ops *ops)
+{
+ int ret;
+ int id;
+ struct framer *framer;
+
+ if (WARN_ON(!dev))
+ return ERR_PTR(-EINVAL);
+
+ /* get_status() is mandatory if the provider ask for polling status */
+ if (WARN_ON((ops->flags & FRAMER_FLAG_POLL_STATUS) && !ops->get_status))
+ return ERR_PTR(-EINVAL);
+
+ framer = kzalloc(sizeof(*framer), GFP_KERNEL);
+ if (!framer)
+ return ERR_PTR(-ENOMEM);
+
+ id = ida_simple_get(_ida, 0, 0, GFP_KERNEL);
ida_alloc()?
(ida_simple_get() is deprecated)
+ if (id < 0) {
+ dev_err(dev, "unable to get id\n");
+ ret = id;
+ goto free_framer;
+ }
+
+ device_initialize(>dev);
+ mutex_init(>mutex);
+ INIT_WORK(>notify_status_work, framer_notify_status_work);
+ INIT_DELAYED_WORK(>polling_work, framer_polling_work);
+ BLOCKING_INIT_NOTIFIER_HEAD(>notifier_list);
+
+ framer->dev.class = framer_class;
+ framer->dev.parent = dev;
+ framer->dev.of_node = node ? node : dev->of_node;
+ framer->id = id;
+ framer->ops = ops;
+
+ ret = dev_set_name(>dev, "framer-%s.%d", dev_name(dev), id);
+ if (ret)
+ goto put_dev;
+
+ /* framer-supply */
+ framer->pwr = regulator_get_optional(>dev, "framer");
+ if (IS_ERR(framer->pwr)) {
+ ret = PTR_ERR(framer->pwr);
+ if (ret == -EPROBE_DEFER)
+ goto put_dev;
+
+ framer->pwr = NULL;
+ }
+
+ ret = device_add(>dev);
+ if (ret)
+ goto put_dev;
+
+ if (pm_runtime_enabled(dev)) {
+ pm_runtime_enable(>dev);
+ pm_runtime_no_callbacks(>dev);
+ }
+
+ return framer;
+
+put_dev:
+ put_device(>dev); /* calls framer_release() which frees
resources */
+ return ERR_PTR(ret);
+
+free_framer:
+ kfree(framer);
+ return ERR_PTR(ret);
+}
...
+void framer_provider_of_unregister(struct framer_provider *framer_provider)
+{
+ mutex_lock(_provider_mutex);
+ list_del(_provider->list);
+ of_node_put(framer_provider->dev->of_node);
+ kfree(framer_provider);
+ mutex_unlock(_provider_mutex);
If it make sense, of_node_put() and kfree() could maybe be out of the
mutex, in order to match how things are done in
__framer_provider_of_register().
+}
...
+static void framer_release(struct device *dev)
+{
+ struct framer *framer;
+
+ framer = dev_to_framer(dev);
+ regulator_put(framer->pwr);
+ ida_simple_remove(_ida, framer->id);
ida_free()?
(ida_simple_remove() is deprecated)
+ kfree(framer);
+}
...
Re: [PATCH v4 21/28] net: wan: Add framer framework support
Hi Linus, Le 20/08/2023 à 23:06, Linus Walleij a écrit : > On Fri, Aug 18, 2023 at 6:41 PM Christophe Leroy > wrote: > >> From: Herve Codina >> >> A framer is a component in charge of an E1/T1 line interface. >> Connected usually to a TDM bus, it converts TDM frames to/from E1/T1 >> frames. It also provides information related to the E1/T1 line. >> >> The framer framework provides a set of APIs for the framer drivers >> (framer provider) to create/destroy a framer and APIs for the framer >> users (framer consumer) to obtain a reference to the framer, and >> use the framer. >> >> This basic implementation provides a framer abstraction for: >> - power on/off the framer >> - get the framer status (line state) >> - be notified on framer status changes >> - get/set the framer configuration >> >> Signed-off-by: Herve Codina >> Reviewed-by: Christophe Leroy >> Signed-off-by: Christophe Leroy > > I had these review comments, you must have missed them? > https://lore.kernel.org/netdev/cacrpkdzq9_f6+9csev1l_wgphhujfpayxmjjfjurzszrako...@mail.gmail.com/ > As I said in the cover letter, this series only fixes critical build failures that happened when CONFIG_MODULES is set. The purpose was to allow robots to perform their job up to the end. Other feedback and comments will be taken into account by Hervé when he is back from holidays. Thanks Christophe
Re: [PATCH v4 21/28] net: wan: Add framer framework support
On Fri, Aug 18, 2023 at 6:41 PM Christophe Leroy wrote: > From: Herve Codina > > A framer is a component in charge of an E1/T1 line interface. > Connected usually to a TDM bus, it converts TDM frames to/from E1/T1 > frames. It also provides information related to the E1/T1 line. > > The framer framework provides a set of APIs for the framer drivers > (framer provider) to create/destroy a framer and APIs for the framer > users (framer consumer) to obtain a reference to the framer, and > use the framer. > > This basic implementation provides a framer abstraction for: > - power on/off the framer > - get the framer status (line state) > - be notified on framer status changes > - get/set the framer configuration > > Signed-off-by: Herve Codina > Reviewed-by: Christophe Leroy > Signed-off-by: Christophe Leroy I had these review comments, you must have missed them? https://lore.kernel.org/netdev/cacrpkdzq9_f6+9csev1l_wgphhujfpayxmjjfjurzszrako...@mail.gmail.com/ Yours, Linus Walleij
Re: [PATCH v4 21/28] net: wan: Add framer framework support
On Fri, Aug 18, 2023 at 06:39:15PM +0200, Christophe Leroy wrote:
> From: Herve Codina
>
> A framer is a component in charge of an E1/T1 line interface.
> Connected usually to a TDM bus, it converts TDM frames to/from E1/T1
> frames. It also provides information related to the E1/T1 line.
>
> The framer framework provides a set of APIs for the framer drivers
> (framer provider) to create/destroy a framer and APIs for the framer
> users (framer consumer) to obtain a reference to the framer, and
> use the framer.
>
> This basic implementation provides a framer abstraction for:
> - power on/off the framer
> - get the framer status (line state)
> - be notified on framer status changes
> - get/set the framer configuration
>
> Signed-off-by: Herve Codina
> Reviewed-by: Christophe Leroy
> Signed-off-by: Christophe Leroy
Hi Christophe and Herve,
some minor feedback from my side.
...
> diff --git a/drivers/net/wan/framer/framer-core.c
> b/drivers/net/wan/framer/framer-core.c
...
> +/**
> + * framer_create() - create a new framer
> + * @dev: device that is creating the new framer
> + * @node: device node of the framer. default to dev->of_node.
> + * @ops: function pointers for performing framer operations
> + *
> + * Called to create a framer using framer framework.
> + */
> +struct framer *framer_create(struct device *dev, struct device_node *node,
> + const struct framer_ops *ops)
> +{
> + int ret;
> + int id;
> + struct framer *framer;
Please arrange local variable declarations for Networking code
using reverse xmas tree order - longest line to shortest.
https://github.com/ecree-solarflare/xmastree is helpful here.
...
> diff --git a/include/linux/framer/framer-provider.h
> b/include/linux/framer/framer-provider.h
...
> +/**
> + * struct framer_ops - set of function pointers for performing framer
> operations
> + * @init: operation to be performed for initializing the framer
> + * @exit: operation to be performed while exiting
> + * @power_on: powering on the framer
> + * @power_off: powering off the framer
> + * @flags: OR-ed flags (FRAMER_FLAG_*) to ask for core functionality
> + * - @FRAMER_FLAG_POLL_STATUS:
> + *Ask the core to perfom a polling to get the framer status and
nit: perfom -> perform
checkpatch.pl --codespell is your friend here
> + *notify consumers on change.
> + *The framer should call @framer_notify_status_change() when it
> + *detects a status change. This is usally done using interrutps.
nit: usally -> usually
interrutps -> interrupts
...
> diff --git a/include/linux/framer/framer.h b/include/linux/framer/framer.h
> new file mode 100644
> index ..0bee7135142f
> --- /dev/null
> +++ b/include/linux/framer/framer.h
> @@ -0,0 +1,199 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Generic framer header file
> + *
> + * Copyright 2023 CS GROUP France
> + *
> + * Author: Herve Codina
> + */
> +
> +#ifndef __DRIVERS_FRAMER_H
> +#define __DRIVERS_FRAMER_H
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +/**
> + * enum framer_iface - Framer interface
As this is a kernel-doc, please include documentation for
the defined constants: FRAMER_IFACE_E1 and FRAMER_IFACE_T1.
As flagged by: ./scripts/kernel-doc -none
> + */
> +enum framer_iface {
> + FRAMER_IFACE_E1, /* E1 interface */
> + FRAMER_IFACE_T1, /* T1 interface */
> +};
> +
> +/**
> + * enum framer_clock_mode - Framer clock mode
Likewise here too.
Also, nit: framer_clock_mode -> framer_clock_type
> + */
> +enum framer_clock_type {
> + FRAMER_CLOCK_EXT, /* External clock */
> + FRAMER_CLOCK_INT, /* Internal clock */
> +};
> +
> +/**
> + * struct framer_configuration - Framer configuration
nit: framer_configuration -> framer_config
> + * @line_iface: Framer line interface
> + * @clock_mode: Framer clock type
> + * @clock_rate: Framer clock rate
> + */
> +struct framer_config {
> + enum framer_iface iface;
> + enum framer_clock_type clock_type;
> + unsigned long line_clock_rate;
> +};
> +
> +/**
> + * struct framer_status - Framer status
> + * @link_is_on: Framer link state. true, the link is on, false, the link is
> off.
> + */
> +struct framer_status {
> + bool link_is_on;
> +};
> +
> +/**
> + * framer_event - event available for notification
nit: framer_event -> enum framer_event
A~d please document FRAMER_EVENT_STATUS in the kernel doc too.
> + */
> +enum framer_event {
> + FRAMER_EVENT_STATUS,/* Event notified on framer_status changes */
> +};
...
Re: [PATCH v4 21/28] net: wan: Add framer framework support
On Fri, 18 Aug 2023 18:39:15 +0200 Christophe Leroy wrote: > From: Herve Codina > > A framer is a component in charge of an E1/T1 line interface. > Connected usually to a TDM bus, it converts TDM frames to/from E1/T1 > frames. It also provides information related to the E1/T1 line. Okay, progress is being made, now it builds patch by patch. Still some kdoc warnings remain (W=1 build only catches kdoc warnings in sources, you gotta run ./scripts/kernel-doc -none explicitly on the headers): include/linux/framer/framer.h:27: warning: Enum value 'FRAMER_IFACE_E1' not described in enum 'framer_iface' include/linux/framer/framer.h:27: warning: Enum value 'FRAMER_IFACE_T1' not described in enum 'framer_iface' include/linux/framer/framer.h:35: warning: expecting prototype for enum framer_clock_mode. Prototype was for enum framer_clock_type instead include/linux/framer/framer.h:47: warning: expecting prototype for struct framer_configuration. Prototype was for struct framer_config instead include/linux/framer/framer.h:60: warning: cannot understand function prototype: 'enum framer_event ' include/linux/framer/framer.h:89: warning: Function parameter or member 'notify_status_work' not described in 'framer' include/linux/framer/framer.h:89: warning: Function parameter or member 'notifier_list' not described in 'framer' include/linux/framer/framer.h:89: warning: Function parameter or member 'polling_work' not described in 'framer' -- pw-bot: cr
[PATCH v4 21/28] net: wan: Add framer framework support
From: Herve Codina
A framer is a component in charge of an E1/T1 line interface.
Connected usually to a TDM bus, it converts TDM frames to/from E1/T1
frames. It also provides information related to the E1/T1 line.
The framer framework provides a set of APIs for the framer drivers
(framer provider) to create/destroy a framer and APIs for the framer
users (framer consumer) to obtain a reference to the framer, and
use the framer.
This basic implementation provides a framer abstraction for:
- power on/off the framer
- get the framer status (line state)
- be notified on framer status changes
- get/set the framer configuration
Signed-off-by: Herve Codina
Reviewed-by: Christophe Leroy
Signed-off-by: Christophe Leroy
---
v4: Fixed wrong names in three EXPORT_SYMBOL()
---
drivers/net/wan/Kconfig| 2 +
drivers/net/wan/Makefile | 2 +
drivers/net/wan/framer/Kconfig | 19 +
drivers/net/wan/framer/Makefile| 6 +
drivers/net/wan/framer/framer-core.c | 886 +
include/linux/framer/framer-provider.h | 194 ++
include/linux/framer/framer.h | 199 ++
7 files changed, 1308 insertions(+)
create mode 100644 drivers/net/wan/framer/Kconfig
create mode 100644 drivers/net/wan/framer/Makefile
create mode 100644 drivers/net/wan/framer/framer-core.c
create mode 100644 include/linux/framer/framer-provider.h
create mode 100644 include/linux/framer/framer.h
diff --git a/drivers/net/wan/Kconfig b/drivers/net/wan/Kconfig
index 8de99f4b647b..31ab2136cdf1 100644
--- a/drivers/net/wan/Kconfig
+++ b/drivers/net/wan/Kconfig
@@ -95,6 +95,8 @@ config HDLC_X25
comment "X.25/LAPB support is disabled"
depends on HDLC && (LAPB!=m || HDLC!=m) && LAPB!=y
+source "drivers/net/wan/framer/Kconfig"
+
config PCI200SYN
tristate "Goramo PCI200SYN support"
depends on HDLC && PCI
diff --git a/drivers/net/wan/Makefile b/drivers/net/wan/Makefile
index f338f4830626..00e9b7ee1e01 100644
--- a/drivers/net/wan/Makefile
+++ b/drivers/net/wan/Makefile
@@ -14,6 +14,8 @@ obj-$(CONFIG_HDLC_FR) += hdlc_fr.o
obj-$(CONFIG_HDLC_PPP) += hdlc_ppp.o
obj-$(CONFIG_HDLC_X25) += hdlc_x25.o
+obj-y += framer/
+
obj-$(CONFIG_FARSYNC) += farsync.o
obj-$(CONFIG_LAPBETHER)+= lapbether.o
diff --git a/drivers/net/wan/framer/Kconfig b/drivers/net/wan/framer/Kconfig
new file mode 100644
index ..96ef1e7ba8eb
--- /dev/null
+++ b/drivers/net/wan/framer/Kconfig
@@ -0,0 +1,19 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# FRAMER
+#
+
+menu "Framer Subsystem"
+
+config GENERIC_FRAMER
+ bool "Framer Core"
+ help
+ Generic Framer support.
+
+ This framework is designed to provide a generic interface for framer
+ devices present in the kernel. This layer will have the generic
+ API by which framer drivers can create framer using the framer
+ framework and framer users can obtain reference to the framer.
+ All the users of this framework should select this config.
+
+endmenu
diff --git a/drivers/net/wan/framer/Makefile b/drivers/net/wan/framer/Makefile
new file mode 100644
index ..78dbd8e563d0
--- /dev/null
+++ b/drivers/net/wan/framer/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for the framer drivers.
+#
+
+obj-$(CONFIG_GENERIC_FRAMER) += framer-core.o
diff --git a/drivers/net/wan/framer/framer-core.c
b/drivers/net/wan/framer/framer-core.c
new file mode 100644
index ..1fb23a068010
--- /dev/null
+++ b/drivers/net/wan/framer/framer-core.c
@@ -0,0 +1,886 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Generic Framer framework.
+ *
+ * Copyright 2023 CS GROUP France
+ *
+ * Author: Herve Codina
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+static struct class *framer_class;
+static DEFINE_MUTEX(framer_provider_mutex);
+static LIST_HEAD(framer_provider_list);
+static DEFINE_IDA(framer_ida);
+
+#define dev_to_framer(a) (container_of((a), struct framer, dev))
+
+int framer_pm_runtime_get(struct framer *framer)
+{
+ int ret;
+
+ if (!pm_runtime_enabled(>dev))
+ return -EOPNOTSUPP;
+
+ ret = pm_runtime_get(>dev);
+ if (ret < 0 && ret != -EINPROGRESS)
+ pm_runtime_put_noidle(>dev);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(framer_pm_runtime_get);
+
+int framer_pm_runtime_get_sync(struct framer *framer)
+{
+ int ret;
+
+ if (!pm_runtime_enabled(>dev))
+ return -EOPNOTSUPP;
+
+ ret = pm_runtime_get_sync(>dev);
+ if (ret < 0)
+ pm_runtime_put_sync(>dev);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(framer_pm_runtime_get_sync);
+
+int framer_pm_runtime_put(struct framer *framer)
+{
+ if (!pm_runtime_enabled(>dev))
+ return -EOPNOTSUPP;
