| Message ID | cover.1692376360.git.christophe.leroy@csgroup.eu |
|---|---|
| Headers | show |
| Series | Add support for QMC HDLC, framer infrastruture and PEF2256 framer (v4) | expand |
Hi, On 8/18/23 09:39, Christophe Leroy wrote: > +config SND_SOC_FRAMER > + tristate "Framer codec" > + depends on GENERIC_FRAMER > + help > + Enable support for the framer codec. > + The framer codec uses the generic framer infrastructure to transport > + some audio data over an analog E1/T1/J1 line. > + This codec allows to use some of the time slots available on the TDM > + bus on which the framer is connected to transport the audio data. > + Just curious: what controls the slot allocations/usages? Is that done in userspace? > + To compile this driver as a module, choose M here: the module > + will be called snd-soc-framer. thanks.
On Fri, 18 Aug 2023 18:39:15 +0200 Christophe Leroy wrote: > From: Herve Codina <herve.codina@bootlin.com> > > 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'
Le 19/08/2023 à 01:18, Randy Dunlap a écrit : > Hi, > > On 8/18/23 09:39, Christophe Leroy wrote: >> +config SND_SOC_FRAMER >> + tristate "Framer codec" >> + depends on GENERIC_FRAMER >> + help >> + Enable support for the framer codec. >> + The framer codec uses the generic framer infrastructure to transport >> + some audio data over an analog E1/T1/J1 line. >> + This codec allows to use some of the time slots available on the TDM >> + bus on which the framer is connected to transport the audio data. >> + > > Just curious: what controls the slot allocations/usages? > Is that done in userspace? For audio, this is done in userspace through alsalib. For IP over E1, a mask is provided with the userspace tool 'sethdlc' For the time being there is no sharing, either the E1 line is used for audio either the E1 line is used for IP over E1 (HDLC) Christophe
On Fri, Aug 18, 2023 at 06:39:15PM +0200, Christophe Leroy wrote: > From: Herve Codina <herve.codina@bootlin.com> > > 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 <herve.codina@bootlin.com> > Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu> > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> 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 000000000000..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 <herve.codina@bootlin.com> > + */ > + > +#ifndef __DRIVERS_FRAMER_H > +#define __DRIVERS_FRAMER_H > + > +#include <linux/err.h> > +#include <linux/mutex.h> > +#include <linux/notifier.h> > +#include <linux/of.h> > +#include <linux/device.h> > +#include <linux/workqueue.h> > + > +/** > + * 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 */ > +}; ...
On Fri, Aug 18, 2023 at 6:41 PM Christophe Leroy <christophe.leroy@csgroup.eu> wrote: > From: Herve Codina <herve.codina@bootlin.com> > > 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 <herve.codina@bootlin.com> > Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu> > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> I had these review comments, you must have missed them? https://lore.kernel.org/netdev/CACRpkdZQ9_f6+9CseV1L_wGphHujFPAYXMjJfjUrzSZRakOBzg@mail.gmail.com/ Yours, Linus Walleij
Hi Linus, Le 20/08/2023 à 23:06, Linus Walleij a écrit : > On Fri, Aug 18, 2023 at 6:41 PM Christophe Leroy > <christophe.leroy@csgroup.eu> wrote: > >> From: Herve Codina <herve.codina@bootlin.com> >> >> 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 <herve.codina@bootlin.com> >> Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > > I had these review comments, you must have missed them? > https://lore.kernel.org/netdev/CACRpkdZQ9_f6+9CseV1L_wGphHujFPAYXMjJfjUrzSZRakOBzg@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
Le 18/08/2023 à 18:39, Christophe Leroy a écrit : > From: Herve Codina <herve.codina@bootlin.com> > > QMC channels support runtime timeslots changes but nothing is done at > the QMC HDLC driver to handle these changes. > > Use existing IFACE ioctl in order to configure the timeslots to use. > > Signed-off-by: Herve Codina <herve.codina@bootlin.com> > Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu> > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- Hi, a few nits below, should there be a v5. > drivers/net/wan/fsl_qmc_hdlc.c | 169 ++++++++++++++++++++++++++++++++- > 1 file changed, 168 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wan/fsl_qmc_hdlc.c b/drivers/net/wan/fsl_qmc_hdlc.c > index 4f84ac5fc63e..4b8cb5761fd1 100644 > --- a/drivers/net/wan/fsl_qmc_hdlc.c > +++ b/drivers/net/wan/fsl_qmc_hdlc.c > @@ -32,6 +32,7 @@ struct qmc_hdlc { > struct qmc_hdlc_desc tx_descs[8]; > unsigned int tx_out; > struct qmc_hdlc_desc rx_descs[4]; > + u32 slot_map; > }; > > static inline struct qmc_hdlc *netdev_to_qmc_hdlc(struct net_device *netdev) > @@ -202,6 +203,162 @@ static netdev_tx_t qmc_hdlc_xmit(struct sk_buff *skb, struct net_device *netdev) > return NETDEV_TX_OK; > } > > +static int qmc_hdlc_xlate_slot_map(struct qmc_hdlc *qmc_hdlc, > + u32 slot_map, struct qmc_chan_ts_info *ts_info) > +{ > + u64 ts_mask_avail; > + unsigned int bit; > + unsigned int i; > + u64 ts_mask; > + u64 map = 0; This init looks useless. > + > + /* Tx and Rx masks must be identical */ > + if (ts_info->rx_ts_mask_avail != ts_info->tx_ts_mask_avail) { > + dev_err(qmc_hdlc->dev, "tx and rx available timeslots mismatch (0x%llx, 0x%llx)\n", > + ts_info->rx_ts_mask_avail, ts_info->tx_ts_mask_avail); > + return -EINVAL; > + } > + > + ts_mask_avail = ts_info->rx_ts_mask_avail; > + ts_mask = 0; > + map = slot_map; > + bit = 0; > + for (i = 0; i < 64; i++) { > + if (ts_mask_avail & BIT_ULL(i)) { > + if (map & BIT_ULL(bit)) > + ts_mask |= BIT_ULL(i); > + bit++; > + } > + } > + > + if (hweight64(ts_mask) != hweight64(map)) { > + dev_err(qmc_hdlc->dev, "Cannot translate timeslots 0x%llx -> (0x%llx,0x%llx)\n", > + map, ts_mask_avail, ts_mask); > + return -EINVAL; > + } > + > + ts_info->tx_ts_mask = ts_mask; > + ts_info->rx_ts_mask = ts_mask; > + return 0; > +} > + > +static int qmc_hdlc_xlate_ts_info(struct qmc_hdlc *qmc_hdlc, > + const struct qmc_chan_ts_info *ts_info, u32 *slot_map) > +{ > + u64 ts_mask_avail; > + unsigned int bit; > + unsigned int i; > + u64 ts_mask; > + u64 map = 0; This init looks useless. > + > + /* Tx and Rx masks must be identical */ > + if (ts_info->rx_ts_mask_avail != ts_info->tx_ts_mask_avail) { > + dev_err(qmc_hdlc->dev, "tx and rx available timeslots mismatch (0x%llx, 0x%llx)\n", > + ts_info->rx_ts_mask_avail, ts_info->tx_ts_mask_avail); > + return -EINVAL; > + } > + if (ts_info->rx_ts_mask != ts_info->tx_ts_mask) { > + dev_err(qmc_hdlc->dev, "tx and rx timeslots mismatch (0x%llx, 0x%llx)\n", > + ts_info->rx_ts_mask, ts_info->tx_ts_mask); > + return -EINVAL; > + } > + > + ts_mask_avail = ts_info->rx_ts_mask_avail; > + ts_mask = ts_info->rx_ts_mask; > + map = 0; > + bit = 0; > + for (i = 0; i < 64; i++) { > + if (ts_mask_avail & BIT_ULL(i)) { > + if (ts_mask & BIT_ULL(i)) > + map |= BIT_ULL(bit); > + bit++; > + } > + } > + > + if (hweight64(ts_mask) != hweight64(map)) { > + dev_err(qmc_hdlc->dev, "Cannot translate timeslots (0x%llx,0x%llx) -> 0x%llx\n", > + ts_mask_avail, ts_mask, map); > + return -EINVAL; > + } > + > + if (map >= BIT_ULL(32)) { > + dev_err(qmc_hdlc->dev, "Slot map out of 32bit (0x%llx,0x%llx) -> 0x%llx\n", > + ts_mask_avail, ts_mask, map); > + return -EINVAL; > + } > + > + *slot_map = map; > + return 0; > +} ... > +static int qmc_hdlc_ioctl(struct net_device *netdev, struct if_settings *ifs) > +{ > + struct qmc_hdlc *qmc_hdlc = netdev_to_qmc_hdlc(netdev); > + te1_settings te1; > + > + switch (ifs->type) { > + case IF_GET_IFACE: > + ifs->type = IF_IFACE_E1; > + if (ifs->size < sizeof(te1)) { > + if (!ifs->size) > + return 0; /* only type requested */ > + > + ifs->size = sizeof(te1); /* data size wanted */ > + return -ENOBUFS; > + } > + > + memset(&te1, 0, sizeof(te1)); > + > + /* Update slot_map */ > + te1.slot_map = qmc_hdlc->slot_map; > + > + if (copy_to_user(ifs->ifs_ifsu.te1, &te1, sizeof(te1))) ~~ Extra space. > + return -EFAULT; > + return 0; > + > + case IF_IFACE_E1: > + case IF_IFACE_T1: > + if (!capable(CAP_NET_ADMIN)) > + return -EPERM; > + > + if (netdev->flags & IFF_UP) > + return -EBUSY; > + > + if (copy_from_user(&te1, ifs->ifs_ifsu.te1, sizeof(te1))) > + return -EFAULT; > + > + return qmc_hdlc_set_iface(qmc_hdlc, ifs->type, &te1); > + > + default: > + return hdlc_ioctl(netdev, ifs); > + } > +} ...
Le 18/08/2023 à 18:39, Christophe Leroy a écrit : > From: Herve Codina <herve.codina@bootlin.com> > > 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 <herve.codina@bootlin.com> > Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu> > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- Hi, should there be a V5, some nits below. ... > +int framer_power_off(struct framer *framer) > +{ > + int ret; > + > + mutex_lock(&framer->mutex); > + if (framer->power_count == 1 && framer->ops->power_off) { > + ret = framer->ops->power_off(framer); ~~ Useless extra space > + if (ret < 0) { > + dev_err(&framer->dev, "framer poweroff failed --> %d\n", ret); > + mutex_unlock(&framer->mutex); > + return ret; > + } > + } > + --framer->power_count; > + mutex_unlock(&framer->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(&framer_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(&framer->dev); > + mutex_init(&framer->mutex); > + INIT_WORK(&framer->notify_status_work, framer_notify_status_work); > + INIT_DELAYED_WORK(&framer->polling_work, framer_polling_work); > + BLOCKING_INIT_NOTIFIER_HEAD(&framer->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(&framer->dev, "framer-%s.%d", dev_name(dev), id); > + if (ret) > + goto put_dev; > + > + /* framer-supply */ > + framer->pwr = regulator_get_optional(&framer->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(&framer->dev); > + if (ret) > + goto put_dev; > + > + if (pm_runtime_enabled(dev)) { > + pm_runtime_enable(&framer->dev); > + pm_runtime_no_callbacks(&framer->dev); > + } > + > + return framer; > + > +put_dev: > + put_device(&framer->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(&framer_provider_mutex); > + list_del(&framer_provider->list); > + of_node_put(framer_provider->dev->of_node); > + kfree(framer_provider); > + mutex_unlock(&framer_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(&framer_ida, framer->id); ida_free()? (ida_simple_remove() is deprecated) > + kfree(framer); > +} ...
On Mon, Aug 21, 2023 at 7:19 AM Christophe Leroy <christophe.leroy@csgroup.eu> wrote: > Le 20/08/2023 à 23:06, Linus Walleij a écrit : > > On Fri, Aug 18, 2023 at 6:41 PM Christophe Leroy > > <christophe.leroy@csgroup.eu> wrote: > > > >> From: Herve Codina <herve.codina@bootlin.com> > >> > >> 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 <herve.codina@bootlin.com> > >> Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu> > >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > > > > I had these review comments, you must have missed them? > > https://lore.kernel.org/netdev/CACRpkdZQ9_f6+9CseV1L_wGphHujFPAYXMjJfjUrzSZRakOBzg@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
On Fri, 18 Aug 2023, Christophe Leroy wrote: > From: Herve Codina <herve.codina@bootlin.com> > > The loop searching for a matching device based on its compatible > string is aborted when a matching disabled device is found. > This abort prevents to add devices as soon as one disabled device > is found. > > Continue searching for an other device instead of aborting on the > first disabled one fixes the issue. > > Fixes: 22380b65dc70 ("mfd: mfd-core: Ensure disabled devices are ignored without error") > Signed-off-by: Herve Codina <herve.codina@bootlin.com> > Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu> > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > drivers/mfd/mfd-core.c | 17 ++++++++++++----- > 1 file changed, 12 insertions(+), 5 deletions(-) This is too big of a change to be added this late in the cycle. Pushing review until after v6.5 is out.
On Mon, 21 Aug 2023 05:19:22 +0000 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
Hi Christophe, On Mon, 21 Aug 2023 07:40:26 +0200 Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote: > Le 18/08/2023 à 18:39, Christophe Leroy a écrit : > > From: Herve Codina <herve.codina@bootlin.com> > > > > QMC channels support runtime timeslots changes but nothing is done at > > the QMC HDLC driver to handle these changes. > > > > Use existing IFACE ioctl in order to configure the timeslots to use. > > > > Signed-off-by: Herve Codina <herve.codina@bootlin.com> > > Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu> > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > > --- > > Hi, > > a few nits below, should there be a v5. > > > drivers/net/wan/fsl_qmc_hdlc.c | 169 ++++++++++++++++++++++++++++++++- > > 1 file changed, 168 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/wan/fsl_qmc_hdlc.c b/drivers/net/wan/fsl_qmc_hdlc.c > > index 4f84ac5fc63e..4b8cb5761fd1 100644 > > --- a/drivers/net/wan/fsl_qmc_hdlc.c > > +++ b/drivers/net/wan/fsl_qmc_hdlc.c > > @@ -32,6 +32,7 @@ struct qmc_hdlc { > > struct qmc_hdlc_desc tx_descs[8]; > > unsigned int tx_out; > > struct qmc_hdlc_desc rx_descs[4]; > > + u32 slot_map; > > }; > > > > static inline struct qmc_hdlc *netdev_to_qmc_hdlc(struct net_device *netdev) > > @@ -202,6 +203,162 @@ static netdev_tx_t qmc_hdlc_xmit(struct sk_buff *skb, struct net_device *netdev) > > return NETDEV_TX_OK; > > } > > > > +static int qmc_hdlc_xlate_slot_map(struct qmc_hdlc *qmc_hdlc, > > + u32 slot_map, struct qmc_chan_ts_info *ts_info) > > +{ > > + u64 ts_mask_avail; > > + unsigned int bit; > > + unsigned int i; > > + u64 ts_mask; > > + u64 map = 0; > > This init looks useless. Will be removed in the next iteration. > > > + > > + /* Tx and Rx masks must be identical */ > > + if (ts_info->rx_ts_mask_avail != ts_info->tx_ts_mask_avail) { > > + dev_err(qmc_hdlc->dev, "tx and rx available timeslots mismatch (0x%llx, 0x%llx)\n", > > + ts_info->rx_ts_mask_avail, ts_info->tx_ts_mask_avail); > > + return -EINVAL; > > + } > > + > > + ts_mask_avail = ts_info->rx_ts_mask_avail; > > + ts_mask = 0; > > + map = slot_map; > > + bit = 0; > > + for (i = 0; i < 64; i++) { > > + if (ts_mask_avail & BIT_ULL(i)) { > > + if (map & BIT_ULL(bit)) > > + ts_mask |= BIT_ULL(i); > > + bit++; > > + } > > + } > > + > > + if (hweight64(ts_mask) != hweight64(map)) { > > + dev_err(qmc_hdlc->dev, "Cannot translate timeslots 0x%llx -> (0x%llx,0x%llx)\n", > > + map, ts_mask_avail, ts_mask); > > + return -EINVAL; > > + } > > + > > + ts_info->tx_ts_mask = ts_mask; > > + ts_info->rx_ts_mask = ts_mask; > > + return 0; > > +} > > + > > +static int qmc_hdlc_xlate_ts_info(struct qmc_hdlc *qmc_hdlc, > > + const struct qmc_chan_ts_info *ts_info, u32 *slot_map) > > +{ > > + u64 ts_mask_avail; > > + unsigned int bit; > > + unsigned int i; > > + u64 ts_mask; > > + u64 map = 0; > > This init looks useless. Will be remove in the next iteration. > > > + > > + /* Tx and Rx masks must be identical */ > > + if (ts_info->rx_ts_mask_avail != ts_info->tx_ts_mask_avail) { > > + dev_err(qmc_hdlc->dev, "tx and rx available timeslots mismatch (0x%llx, 0x%llx)\n", > > + ts_info->rx_ts_mask_avail, ts_info->tx_ts_mask_avail); > > + return -EINVAL; > > + } > > + if (ts_info->rx_ts_mask != ts_info->tx_ts_mask) { > > + dev_err(qmc_hdlc->dev, "tx and rx timeslots mismatch (0x%llx, 0x%llx)\n", > > + ts_info->rx_ts_mask, ts_info->tx_ts_mask); > > + return -EINVAL; > > + } > > + > > + ts_mask_avail = ts_info->rx_ts_mask_avail; > > + ts_mask = ts_info->rx_ts_mask; > > + map = 0; > > + bit = 0; > > + for (i = 0; i < 64; i++) { > > + if (ts_mask_avail & BIT_ULL(i)) { > > + if (ts_mask & BIT_ULL(i)) > > + map |= BIT_ULL(bit); > > + bit++; > > + } > > + } > > + > > + if (hweight64(ts_mask) != hweight64(map)) { > > + dev_err(qmc_hdlc->dev, "Cannot translate timeslots (0x%llx,0x%llx) -> 0x%llx\n", > > + ts_mask_avail, ts_mask, map); > > + return -EINVAL; > > + } > > + > > + if (map >= BIT_ULL(32)) { > > + dev_err(qmc_hdlc->dev, "Slot map out of 32bit (0x%llx,0x%llx) -> 0x%llx\n", > > + ts_mask_avail, ts_mask, map); > > + return -EINVAL; > > + } > > + > > + *slot_map = map; > > + return 0; > > +} > > ... > > > +static int qmc_hdlc_ioctl(struct net_device *netdev, struct if_settings *ifs) > > +{ > > + struct qmc_hdlc *qmc_hdlc = netdev_to_qmc_hdlc(netdev); > > + te1_settings te1; > > + > > + switch (ifs->type) { > > + case IF_GET_IFACE: > > + ifs->type = IF_IFACE_E1; > > + if (ifs->size < sizeof(te1)) { > > + if (!ifs->size) > > + return 0; /* only type requested */ > > + > > + ifs->size = sizeof(te1); /* data size wanted */ > > + return -ENOBUFS; > > + } > > + > > + memset(&te1, 0, sizeof(te1)); > > + > > + /* Update slot_map */ > > + te1.slot_map = qmc_hdlc->slot_map; > > + > > + if (copy_to_user(ifs->ifs_ifsu.te1, &te1, sizeof(te1))) > > ~~ > Extra space. Will be fixed in the next iteration. > > > + return -EFAULT; > > + return 0; > > + > > + case IF_IFACE_E1: > > + case IF_IFACE_T1: > > + if (!capable(CAP_NET_ADMIN)) > > + return -EPERM; > > + > > + if (netdev->flags & IFF_UP) > > + return -EBUSY; > > + > > + if (copy_from_user(&te1, ifs->ifs_ifsu.te1, sizeof(te1))) > > + return -EFAULT; > > + > > + return qmc_hdlc_set_iface(qmc_hdlc, ifs->type, &te1); > > + > > + default: > > + return hdlc_ioctl(netdev, ifs); > > + } > > +} > > ... > Thanks for the review, Best regards, Hervé
Hi Christophe, On Mon, 21 Aug 2023 08:02:10 +0200 Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote: > Le 18/08/2023 à 18:39, Christophe Leroy a écrit : > > From: Herve Codina <herve.codina@bootlin.com> > > > > 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 <herve.codina@bootlin.com> > > Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu> > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > > --- > > Hi, > > should there be a V5, some nits below. > > ... > > > +int framer_power_off(struct framer *framer) > > +{ > > + int ret; > > + > > + mutex_lock(&framer->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(&framer->dev, "framer poweroff failed --> %d\n", ret); > > + mutex_unlock(&framer->mutex); > > + return ret; > > + } > > + } > > + --framer->power_count; > > + mutex_unlock(&framer->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(&framer_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(&framer->dev); > > + mutex_init(&framer->mutex); > > + INIT_WORK(&framer->notify_status_work, framer_notify_status_work); > > + INIT_DELAYED_WORK(&framer->polling_work, framer_polling_work); > > + BLOCKING_INIT_NOTIFIER_HEAD(&framer->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(&framer->dev, "framer-%s.%d", dev_name(dev), id); > > + if (ret) > > + goto put_dev; > > + > > + /* framer-supply */ > > + framer->pwr = regulator_get_optional(&framer->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(&framer->dev); > > + if (ret) > > + goto put_dev; > > + > > + if (pm_runtime_enabled(dev)) { > > + pm_runtime_enable(&framer->dev); > > + pm_runtime_no_callbacks(&framer->dev); > > + } > > + > > + return framer; > > + > > +put_dev: > > + put_device(&framer->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(&framer_provider_mutex); > > + list_del(&framer_provider->list); > > + of_node_put(framer_provider->dev->of_node); > > + kfree(framer_provider); > > + mutex_unlock(&framer_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(&framer_ida, framer->id); > > ida_free()? > (ida_simple_remove() is deprecated) Yes > > > + kfree(framer); > > +} > > ... > Thanks for the review, Hervé
Hi Simon, On Sun, 20 Aug 2023 19:15:11 +0200 Simon Horman <horms@kernel.org> wrote: > On Fri, Aug 18, 2023 at 06:39:15PM +0200, Christophe Leroy wrote: > > From: Herve Codina <herve.codina@bootlin.com> > > > > 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 <herve.codina@bootlin.com> > > Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu> > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > > 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 000000000000..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 <herve.codina@bootlin.com> > > + */ > > + > > +#ifndef __DRIVERS_FRAMER_H > > +#define __DRIVERS_FRAMER_H > > + > > +#include <linux/err.h> > > +#include <linux/mutex.h> > > +#include <linux/notifier.h> > > +#include <linux/of.h> > > +#include <linux/device.h> > > +#include <linux/workqueue.h> > > + > > +/** > > + * 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 framer_status { > > + bool link_is_on; > > +}; > > + > > +/** > > + * framer_event - event available for notification > > nit: framer_event -> enum framer_event Will be fixed in the next iteration. > > A~d please document FRAMER_EVENT_STATUS in the kernel doc too. Will be documented in the next iteration. > > > + */ > > +enum framer_event { > > + FRAMER_EVENT_STATUS, /* Event notified on framer_status changes */ > > +}; > > ... Thanks for the review, Best regards, Hervé
On Fri, 18 Aug 2023 18:39:17 +0200, Christophe Leroy wrote: > The loop searching for a matching device based on its compatible > string is aborted when a matching disabled device is found. > This abort prevents to add devices as soon as one disabled device > is found. > > Continue searching for an other device instead of aborting on the > first disabled one fixes the issue. > > [...] Applied, thanks! [23/28] mfd: core: Ensure disabled devices are skiped without aborting commit: 36d139dc63db18eb95165fcc2bd3c670c948d605 -- Lee Jones [李琼斯]
From: Herve Codina <herve.codina@bootlin.com> Hi, TLDR: This is a resend of v3 with a build failure fix in patch 21. No other changes. I have a system where I need to handle an HDLC interface and some audio data. The HDLC data are transferred using a TDM bus on which a PEF2256 (E1/T1 framer) is present. The PEF2256 transfers data from/to the TDM bus to/from the E1 line. This PEF2256 is connected to a PowerQUICC SoC for the control path and the TDM is connected to the SoC (QMC component) for the data path. From the QMC HDLC driver, I need to handle HDLC data using the QMC, carrier detection using the PEF2256 (E1 line carrier) and set/get some PEF2256 configuration. The QMC HDLC driver considers the PEF2256 as a generic framer. It performs operations that involve the PEF2256 through the generic framer API. The audio data are exchanged with the PEF2256 using a CPU DAI connected to the TDM bus through the QMC and the PEF2256 needs to be seen as a codec in order to be linked to the CPU DAI. The codec handles the carrier detection using the PEF2256 and reports the carrier state to the ALSA subsystem using the ASoC jack detection. The codec, even if instantiated by the PEF2256 driver, considers the PEF2256 as a generic framer. The generic framer has: - 2 consumers (QMC HDLC drv and codec) - 1 provider (PEF2256) So, the design is the following: +------------------+ +---------+ | QMC | <- TDM -> | PEF2256 | <-> E1 +---------+ | +-------------+ | | | | CPU DAI | <-data--> | QMC channel | | | | +---------+ | +-------------+ | | | +--------------+ | +-------------+ | | | | QMC HDLC drv | <-data--> | QMC channel | | | | +--------------+ | +-------------+ | | | ^ +------------------+ | | | +--------+ +-------------+ | | +-> | framer | <-> | PEF2256 drv | <- local bus ->| | | | | | +---------+ +-> | | | | | +--------+ | +-------+ | +-------------------> | codec | | | +-------+ | +-------------+ Further more, the TDM timeslots used by the QMC HDLC driver need to be configured at runtime (QMC dynamic timeslots). Several weeks ago, I sent two series related to this topic: - Add the Lantiq PEF2256 audio support [1] - RFC Add support for QMC HDLC and PHY [2] This current series is a rework of these two series taking into account feedbacks previously received. In order to implement all of this, I do the following: 1) Perform some fixes (patches 1, 2, 3, 4) 2) Introduce the QMC HDLC driver (patches 5, 6, 7) 3) Add QMC dynamic timeslot support (patches 8 - 18) 4) Add timeslots change support in QMC HDLC (patch 19) 5) Introduce framer infrastructure (patch 20) 6) Add PEF2256 framer provider (patches 11, 22, 23, 24, 25) 7) Add framer codec as a framer consumer (patch 26) 8) Add framer support as a framer consumer in QMC HDLC (patch 27, 28) The series contains the full story and detailed modifications. If needed, the series can be split and/or commmits can be squashed. Let me know. Compare to the previous iteration https://lore.kernel.org/linux-kernel/20230726150225.483464-1-herve.codina@bootlin.com/ This v3 series mainly: - Fixes some implementation details. - Adds a new patch (patch 5) to remove existing inline keyword in the QMC driver. - Squashes patches related to the QMC HDLC binding together. Best regards, Hervé [1]: https://lore.kernel.org/all/20230417171601.74656-1-herve.codina@bootlin.com/ [2]: https://lore.kernel.org/all/20230323103154.264546-1-herve.codina@bootlin.com/ Changes v3 -> v4 - Fixes build failure with CONFIG_MODULES in patch 21 (net: wan: Add framer framework support) Changes v2 -> v3 - Patches 1, 2, 3, 4 Add 'Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>' - New patch Remove inline keyword from the existing registers accessors helpers - Patch 6 (patches 5, 27 in v2) Update the binding title Squash patch 27 - Patch 7 (patch 6 in v2) Remove the cast in netdev_to_qmc_hdlc() Add 'Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>' - Patch 8 (patch 7 in v2): No change - Patches 9, 10 (patches 8, 9 in v2) Add 'Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>' - Patch 11 (patch 10 in v2) Remove inline keyword from the introduced qmc_clrsetbits16() helper Add 'Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>' - Patches 12, 13, 14, 15, 16, 17, 18, 19, 20 Add 'Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>' - Patch 21 (patch 20 in v2) Remove unneeded framer NULL pointer check Add 'Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>' - Patch 22 (patch 21 in v2) Change sclkr and sclkx clocks description Remove the framer phandle property from the framer subnodes (ie. from framer-codec nodes) - Patch 23 (patch 22 in v2) Initialize 'disabled' variable at declaration Fix commit log Add 'Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>' - Patch 24 (patch 23 in v2) Remove inline keyword from the existing registers accessors helpers Use dev_warn_ratelimited() in default interrupt handler Add 'Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>' - Patch 25 (patch 24 in v2) Replace #include "linux/bitfield.h" by #include <linux/bitfield.h> Fold the pinctrl anonymous struct into the struct pef2256_pinctrl Update commit log Add 'Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>' - Patch 26 (patch 25 in v2) Add 'Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>' - Patch 27 (patch 26 in v2) Fix error message Changed the ch.max computation in framer_dai_hw_rule_channels_by_format() Add 'Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>' - Patch 28 Add 'Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>' Changes v1 -> v2 - Patches 1, 2 (New in v2) Fix __iomem addresses declaration - Patch 19 (17 in v1) Fix a compilation warning - Patch 26 (24 in v1) Fix a typo in Kconfig file Fix issues raised by sparse (make C=1) Herve Codina (28): soc: fsl: cpm1: tsa: Fix __iomem addresses declaration soc: fsl: cpm1: qmc: Fix __iomem addresses declaration soc: fsl: cpm1: qmc: Fix rx channel reset soc: fsl: cpm1: qmc: Extend the API to provide Rx status soc: fsl: cpm1: qmc: Remove inline function specifiers dt-bindings: net: Add support for QMC HDLC net: wan: Add support for QMC HDLC MAINTAINERS: Add the Freescale QMC HDLC driver entry soc: fsl: cpm1: qmc: Introduce available timeslots masks soc: fsl: cpm1: qmc: Rename qmc_setup_tsa* to qmc_init_tsa* soc: fsl: cpm1: qmc: Introduce qmc_chan_setup_tsa* soc: fsl: cpm1: qmc: Remove no more needed checks from qmc_check_chans() soc: fsl: cpm1: qmc: Check available timeslots in qmc_check_chans() soc: fsl: cpm1: qmc: Add support for disabling channel TSA entries soc: fsl: cpm1: qmc: Split Tx and Rx TSA entries setup soc: fsl: cpm1: qmc: Introduce is_tsa_64rxtx flag soc: fsl: cpm1: qmc: Handle timeslot entries at channel start() and stop() soc: fsl: cpm1: qmc: Remove timeslots handling from setup_chan() soc: fsl: cpm1: qmc: Introduce functions to change timeslots at runtime wan: qmc_hdlc: Add runtime timeslots changes support net: wan: Add framer framework support dt-bindings: net: Add the Lantiq PEF2256 E1/T1/J1 framer mfd: core: Ensure disabled devices are skiped without aborting net: wan: framer: Add support for the Lantiq PEF2256 framer pinctrl: Add support for the Lantic PEF2256 pinmux MAINTAINERS: Add the Lantiq PEF2256 driver entry ASoC: codecs: Add support for the framer codec net: wan: fsl_qmc_hdlc: Add framer support .../devicetree/bindings/net/fsl,qmc-hdlc.yaml | 46 + .../bindings/net/lantiq,pef2256.yaml | 219 +++++ MAINTAINERS | 17 + drivers/mfd/mfd-core.c | 17 +- drivers/net/wan/Kconfig | 14 + drivers/net/wan/Makefile | 3 + drivers/net/wan/framer/Kconfig | 35 + drivers/net/wan/framer/Makefile | 7 + drivers/net/wan/framer/framer-core.c | 886 ++++++++++++++++++ drivers/net/wan/framer/pef2256/Makefile | 8 + drivers/net/wan/framer/pef2256/pef2256-regs.h | 250 +++++ drivers/net/wan/framer/pef2256/pef2256.c | 880 +++++++++++++++++ drivers/net/wan/fsl_qmc_hdlc.c | 820 ++++++++++++++++ drivers/pinctrl/Kconfig | 14 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/pinctrl-pef2256-regs.h | 65 ++ drivers/pinctrl/pinctrl-pef2256.c | 308 ++++++ drivers/soc/fsl/qe/qmc.c | 501 ++++++++-- drivers/soc/fsl/qe/tsa.c | 22 +- include/linux/framer/framer-provider.h | 194 ++++ include/linux/framer/framer.h | 199 ++++ include/linux/framer/pef2256.h | 31 + include/soc/fsl/qe/qmc.h | 25 +- sound/soc/codecs/Kconfig | 15 + sound/soc/codecs/Makefile | 2 + sound/soc/codecs/framer-codec.c | 413 ++++++++ sound/soc/fsl/fsl_qmc_audio.c | 2 +- 27 files changed, 4872 insertions(+), 122 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/fsl,qmc-hdlc.yaml create mode 100644 Documentation/devicetree/bindings/net/lantiq,pef2256.yaml 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 drivers/net/wan/framer/pef2256/Makefile create mode 100644 drivers/net/wan/framer/pef2256/pef2256-regs.h create mode 100644 drivers/net/wan/framer/pef2256/pef2256.c create mode 100644 drivers/net/wan/fsl_qmc_hdlc.c create mode 100644 drivers/pinctrl/pinctrl-pef2256-regs.h create mode 100644 drivers/pinctrl/pinctrl-pef2256.c create mode 100644 include/linux/framer/framer-provider.h create mode 100644 include/linux/framer/framer.h create mode 100644 include/linux/framer/pef2256.h create mode 100644 sound/soc/codecs/framer-codec.c