Hi,
This series introduces the QMC HDLC support.
Patches were previously sent as part of a full feature series and were
previously reviewed in that context:
"Add support for QMC HDLC, framer infrastructure and PEF2256 framer" [1]
In order to ease the merge, the full feature series has been split and
needed parts were merged in v6.8-rc1:
- "Prepare the PowerQUICC QMC and TSA for the HDLC QMC driver" [2]
- "Add support for framer infrastructure and PEF2256 framer" [3]
This series contains patches related to the QMC HDLC part (QMC HDLC
driver):
- Introduce the QMC HDLC driver (patches 1 and 2)
- Add timeslots change support in QMC HDLC (patch 3)
- Add framer support as a framer consumer in QMC HDLC (patch 4)
Compare to the original full feature series, a modification was done on
patch 3 in order to use a coherent prefix in the commit title.
I kept the patches unsquashed as they were previously sent and reviewed.
Of course, I can squash them if needed.
Compared to the previous iteration:
https://lore.kernel.org/linux-kernel/[email protected]/
this v5 series mainly:
- Update some piece of code to improve readability.
- Release resources before printing error messages.
- Use 'struct device *dev' and dev_err_probe() in probe().
- Use guard() and scoped_guard() from cleanup.h for spinlock locking.
Best regards,
Hervé
[1]: https://lore.kernel.org/linux-kernel/[email protected]/
[2]: https://lore.kernel.org/linux-kernel/[email protected]/
[3]: https://lore.kernel.org/linux-kernel/[email protected]/
Changes v4 -> v5
- Patch 1
Update QMC_HDLC_RX_ERROR_FLAGS to improve readability.
Display an error message after releasing resources instead of
before.
Use 'struct device *dev' in probe().
Use dev_err_probe() in probe().
Do not print a message on -ENOMEM.
Use guard() and scoped_guard().
- Patch 3
Use '(). See' constructing in kernel-doc instead of '() (See ...'
Add 'Co-developed-by: Herve Codina <[email protected]>'
- Patch 4
Use 'struct device *dev' in probe().
Use dev_err_probe() in probe().
Use '%64pb' instead of '%*pb' in printk formats.
- Patch 5
Use 'struct device *dev' in probe().
Use guard()
Changes v3 -> v4
- Patch 1
Remove of.h and of_platform.h includes, add mod_devicetable.h.
Add a blank line in the includes list.
- Path 2
No changes.
- v3 patches 3 and 4 removed
- Patch 3 (new patch in v4)
Introduce bitmap_{scatter,gather}() based on the original patch done
by Andy Shevchenko.
Address comments already done on the original patch:
https://lore.kernel.org/lkml/[email protected]/
- Removed the returned values.
- Used 'unsigned int' for all indexes.
- Added a 'visual' description of the operations in kernel-doc.
- Described the relationship between bitmap_scatter() and
bitmap_gather().
- Moved bitmap_{scatter,gather}() to the bitmap.h file.
- Improved bitmap_{scatter,gather}() test.
- Reworked the commit log.
- Patch 4 (v3 patch 5)
Use bitmap_{scatter,gather}()
- Patches 5 (v3 patch 6)
No changes.
Changes v2 -> v3
- Patch 1
Remove 'inline' function specifier from .c file.
Fix a bug introduced when added WARN_ONCE(). The warn condition must
be desc->skb (descriptor used) instead of !desc->skb.
Remove a lock/unlock section locking the entire qmc_hdlc_xmit()
function.
- Patch 5
Use bitmap_from_u64() everywhere instead of bitmap_from_arr32() and
bitmap_from_arr64().
Changes v1 -> v2
- Patch 1
Use the same qmc_hdlc initialisation in qmc_hcld_recv_complete()
than the one present in qmc_hcld_xmit_complete().
Use WARN_ONCE()
- Patch 3 (new patch in v2)
Make bitmap_onto() available to users
- Patch 4 (new patch in v2)
Introduce bitmap_off()
- Patch 5 (patch 3 in v1)
Use bitmap_*() functions
- Patch 6 (patch 4 in v1)
No changes
Changes compare to the full feature series:
- Patch 3
Use 'net: wan: fsl_qmc_hdlc:' as commit title prefix
Patches extracted:
- Patch 1 : full feature series patch 7
- Patch 2 : full feature series patch 8
- Patch 3 : full feature series patch 20
- Patch 4 : full feature series patch 27
Andy Shevchenko (1):
lib/bitmap: Introduce bitmap_scatter() and bitmap_gather() helpers
Herve Codina (4):
net: wan: Add support for QMC HDLC
MAINTAINERS: Add the Freescale QMC HDLC driver entry
net: wan: fsl_qmc_hdlc: Add runtime timeslots changes support
net: wan: fsl_qmc_hdlc: Add framer support
MAINTAINERS | 7 +
drivers/net/wan/Kconfig | 12 +
drivers/net/wan/Makefile | 1 +
drivers/net/wan/fsl_qmc_hdlc.c | 787 +++++++++++++++++++++++++++++++++
include/linux/bitmap.h | 101 +++++
lib/test_bitmap.c | 42 ++
6 files changed, 950 insertions(+)
create mode 100644 drivers/net/wan/fsl_qmc_hdlc.c
--
2.43.0
Add framer support in the fsl_qmc_hdlc driver in order to be able to
signal carrier changes to the network stack based on the framer status
Also use this framer to provide information related to the E1/T1 line
interface on IF_GET_IFACE and configure the line interface according to
IF_IFACE_{E1,T1} information.
Signed-off-by: Herve Codina <[email protected]>
Reviewed-by: Christophe Leroy <[email protected]>
---
drivers/net/wan/fsl_qmc_hdlc.c | 239 ++++++++++++++++++++++++++++++++-
1 file changed, 234 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wan/fsl_qmc_hdlc.c b/drivers/net/wan/fsl_qmc_hdlc.c
index 80998ad89088..dd6b69a950c9 100644
--- a/drivers/net/wan/fsl_qmc_hdlc.c
+++ b/drivers/net/wan/fsl_qmc_hdlc.c
@@ -10,6 +10,7 @@
#include <linux/cleanup.h>
#include <linux/bitmap.h>
#include <linux/dma-mapping.h>
+#include <linux/framer/framer.h>
#include <linux/hdlc.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
@@ -29,6 +30,9 @@ struct qmc_hdlc {
struct device *dev;
struct qmc_chan *qmc_chan;
struct net_device *netdev;
+ struct framer *framer;
+ spinlock_t carrier_lock; /* Protect carrier detection */
+ struct notifier_block nb;
bool is_crc32;
spinlock_t tx_lock; /* Protect tx descriptors */
struct qmc_hdlc_desc tx_descs[8];
@@ -42,6 +46,192 @@ static struct qmc_hdlc *netdev_to_qmc_hdlc(struct net_device *netdev)
return dev_to_hdlc(netdev)->priv;
}
+static int qmc_hdlc_framer_set_carrier(struct qmc_hdlc *qmc_hdlc)
+{
+ struct framer_status framer_status;
+ int ret;
+
+ if (!qmc_hdlc->framer)
+ return 0;
+
+ guard(spinlock_irqsave)(&qmc_hdlc->carrier_lock);
+
+ ret = framer_get_status(qmc_hdlc->framer, &framer_status);
+ if (ret) {
+ dev_err(qmc_hdlc->dev, "get framer status failed (%d)\n", ret);
+ return ret;
+ }
+ if (framer_status.link_is_on)
+ netif_carrier_on(qmc_hdlc->netdev);
+ else
+ netif_carrier_off(qmc_hdlc->netdev);
+
+ return 0;
+}
+
+static int qmc_hdlc_framer_notifier(struct notifier_block *nb, unsigned long action,
+ void *data)
+{
+ struct qmc_hdlc *qmc_hdlc = container_of(nb, struct qmc_hdlc, nb);
+ int ret;
+
+ if (action != FRAMER_EVENT_STATUS)
+ return NOTIFY_DONE;
+
+ ret = qmc_hdlc_framer_set_carrier(qmc_hdlc);
+ return ret ? NOTIFY_DONE : NOTIFY_OK;
+}
+
+static int qmc_hdlc_framer_start(struct qmc_hdlc *qmc_hdlc)
+{
+ struct framer_status framer_status;
+ int ret;
+
+ if (!qmc_hdlc->framer)
+ return 0;
+
+ ret = framer_power_on(qmc_hdlc->framer);
+ if (ret) {
+ dev_err(qmc_hdlc->dev, "framer power-on failed (%d)\n", ret);
+ return ret;
+ }
+
+ /* Be sure that get_status is supported */
+ ret = framer_get_status(qmc_hdlc->framer, &framer_status);
+ if (ret) {
+ dev_err(qmc_hdlc->dev, "get framer status failed (%d)\n", ret);
+ goto framer_power_off;
+ }
+
+ qmc_hdlc->nb.notifier_call = qmc_hdlc_framer_notifier;
+ ret = framer_notifier_register(qmc_hdlc->framer, &qmc_hdlc->nb);
+ if (ret) {
+ dev_err(qmc_hdlc->dev, "framer notifier register failed (%d)\n", ret);
+ goto framer_power_off;
+ }
+
+ return 0;
+
+framer_power_off:
+ framer_power_off(qmc_hdlc->framer);
+ return ret;
+}
+
+static void qmc_hdlc_framer_stop(struct qmc_hdlc *qmc_hdlc)
+{
+ if (!qmc_hdlc->framer)
+ return;
+
+ framer_notifier_unregister(qmc_hdlc->framer, &qmc_hdlc->nb);
+ framer_power_off(qmc_hdlc->framer);
+}
+
+static int qmc_hdlc_framer_set_iface(struct qmc_hdlc *qmc_hdlc, int if_iface,
+ const te1_settings *te1)
+{
+ struct framer_config config;
+ int ret;
+
+ if (!qmc_hdlc->framer)
+ return 0;
+
+ ret = framer_get_config(qmc_hdlc->framer, &config);
+ if (ret)
+ return ret;
+
+ switch (if_iface) {
+ case IF_IFACE_E1:
+ config.iface = FRAMER_IFACE_E1;
+ break;
+ case IF_IFACE_T1:
+ config.iface = FRAMER_IFACE_T1;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ switch (te1->clock_type) {
+ case CLOCK_DEFAULT:
+ /* Keep current value */
+ break;
+ case CLOCK_EXT:
+ config.clock_type = FRAMER_CLOCK_EXT;
+ break;
+ case CLOCK_INT:
+ config.clock_type = FRAMER_CLOCK_INT;
+ break;
+ default:
+ return -EINVAL;
+ }
+ config.line_clock_rate = te1->clock_rate;
+
+ return framer_set_config(qmc_hdlc->framer, &config);
+}
+
+static int qmc_hdlc_framer_get_iface(struct qmc_hdlc *qmc_hdlc, int *if_iface, te1_settings *te1)
+{
+ struct framer_config config;
+ int ret;
+
+ if (!qmc_hdlc->framer) {
+ *if_iface = IF_IFACE_E1;
+ return 0;
+ }
+
+ ret = framer_get_config(qmc_hdlc->framer, &config);
+ if (ret)
+ return ret;
+
+ switch (config.iface) {
+ case FRAMER_IFACE_E1:
+ *if_iface = IF_IFACE_E1;
+ break;
+ case FRAMER_IFACE_T1:
+ *if_iface = IF_IFACE_T1;
+ break;
+ }
+
+ if (!te1)
+ return 0; /* Only iface type requested */
+
+ switch (config.clock_type) {
+ case FRAMER_CLOCK_EXT:
+ te1->clock_type = CLOCK_EXT;
+ break;
+ case FRAMER_CLOCK_INT:
+ te1->clock_type = CLOCK_INT;
+ break;
+ default:
+ return -EINVAL;
+ }
+ te1->clock_rate = config.line_clock_rate;
+ return 0;
+}
+
+static int qmc_hdlc_framer_init(struct qmc_hdlc *qmc_hdlc)
+{
+ int ret;
+
+ if (!qmc_hdlc->framer)
+ return 0;
+
+ ret = framer_init(qmc_hdlc->framer);
+ if (ret) {
+ dev_err(qmc_hdlc->dev, "framer init failed (%d)\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static void qmc_hdlc_framer_exit(struct qmc_hdlc *qmc_hdlc)
+{
+ if (!qmc_hdlc->framer)
+ return;
+
+ framer_exit(qmc_hdlc->framer);
+}
+
static int qmc_hdlc_recv_queue(struct qmc_hdlc *qmc_hdlc, struct qmc_hdlc_desc *desc, size_t size);
#define QMC_HDLC_RX_ERROR_FLAGS \
@@ -291,6 +481,12 @@ static int qmc_hdlc_set_iface(struct qmc_hdlc *qmc_hdlc, int if_iface, const te1
qmc_hdlc->slot_map = te1->slot_map;
+ ret = qmc_hdlc_framer_set_iface(qmc_hdlc, if_iface, te1);
+ if (ret) {
+ dev_err(qmc_hdlc->dev, "framer set iface failed %d\n", ret);
+ return ret;
+ }
+
return 0;
}
@@ -298,11 +494,16 @@ 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;
+ int ret;
switch (ifs->type) {
case IF_GET_IFACE:
- ifs->type = IF_IFACE_E1;
if (ifs->size < sizeof(te1)) {
+ /* Retrieve type only */
+ ret = qmc_hdlc_framer_get_iface(qmc_hdlc, &ifs->type, NULL);
+ if (ret)
+ return ret;
+
if (!ifs->size)
return 0; /* only type requested */
@@ -312,6 +513,11 @@ static int qmc_hdlc_ioctl(struct net_device *netdev, struct if_settings *ifs)
memset(&te1, 0, sizeof(te1));
+ /* Retrieve info from framer */
+ ret = qmc_hdlc_framer_get_iface(qmc_hdlc, &ifs->type, &te1);
+ if (ret)
+ return ret;
+
/* Update slot_map */
te1.slot_map = qmc_hdlc->slot_map;
@@ -345,10 +551,17 @@ static int qmc_hdlc_open(struct net_device *netdev)
int ret;
int i;
- ret = hdlc_open(netdev);
+ ret = qmc_hdlc_framer_start(qmc_hdlc);
if (ret)
return ret;
+ ret = hdlc_open(netdev);
+ if (ret)
+ goto framer_stop;
+
+ /* Update carrier */
+ qmc_hdlc_framer_set_carrier(qmc_hdlc);
+
chan_param.mode = QMC_HDLC;
/* HDLC_MAX_MRU + 4 for the CRC
* HDLC_MAX_MRU + 4 + 8 for the CRC and some extraspace needed by the QMC
@@ -397,6 +610,8 @@ static int qmc_hdlc_open(struct net_device *netdev)
}
hdlc_close:
hdlc_close(netdev);
+framer_stop:
+ qmc_hdlc_framer_stop(qmc_hdlc);
return ret;
}
@@ -432,6 +647,7 @@ static int qmc_hdlc_close(struct net_device *netdev)
}
hdlc_close(netdev);
+ qmc_hdlc_framer_stop(qmc_hdlc);
return 0;
}
@@ -480,6 +696,7 @@ static int qmc_hdlc_probe(struct platform_device *pdev)
qmc_hdlc->dev = dev;
spin_lock_init(&qmc_hdlc->tx_lock);
+ spin_lock_init(&qmc_hdlc->carrier_lock);
qmc_hdlc->qmc_chan = devm_qmc_chan_get_bychild(dev, dev->of_node);
if (IS_ERR(qmc_hdlc->qmc_chan))
@@ -502,9 +719,19 @@ static int qmc_hdlc_probe(struct platform_device *pdev)
if (ret)
return ret;
+ qmc_hdlc->framer = devm_framer_optional_get(dev, "fsl,framer");
+ if (IS_ERR(qmc_hdlc->framer))
+ return PTR_ERR(qmc_hdlc->framer);
+
+ ret = qmc_hdlc_framer_init(qmc_hdlc);
+ if (ret)
+ return ret;
+
qmc_hdlc->netdev = alloc_hdlcdev(qmc_hdlc);
- if (!qmc_hdlc->netdev)
- return -ENOMEM;
+ if (!qmc_hdlc->netdev) {
+ ret = -ENOMEM;
+ goto framer_exit;
+ }
hdlc = dev_to_hdlc(qmc_hdlc->netdev);
hdlc->attach = qmc_hdlc_attach;
@@ -519,11 +746,12 @@ static int qmc_hdlc_probe(struct platform_device *pdev)
}
platform_set_drvdata(pdev, qmc_hdlc);
-
return 0;
free_netdev:
free_netdev(qmc_hdlc->netdev);
+framer_exit:
+ qmc_hdlc_framer_exit(qmc_hdlc);
return ret;
}
@@ -533,6 +761,7 @@ static int qmc_hdlc_remove(struct platform_device *pdev)
unregister_hdlc_device(qmc_hdlc->netdev);
free_netdev(qmc_hdlc->netdev);
+ qmc_hdlc_framer_exit(qmc_hdlc);
return 0;
}
--
2.43.0
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 <[email protected]>
Reviewed-by: Christophe Leroy <[email protected]>
Acked-by: Jakub Kicinski <[email protected]>
---
drivers/net/wan/fsl_qmc_hdlc.c | 151 ++++++++++++++++++++++++++++++++-
1 file changed, 150 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wan/fsl_qmc_hdlc.c b/drivers/net/wan/fsl_qmc_hdlc.c
index b9e067101171..80998ad89088 100644
--- a/drivers/net/wan/fsl_qmc_hdlc.c
+++ b/drivers/net/wan/fsl_qmc_hdlc.c
@@ -8,6 +8,7 @@
*/
#include <linux/cleanup.h>
+#include <linux/bitmap.h>
#include <linux/dma-mapping.h>
#include <linux/hdlc.h>
#include <linux/mod_devicetable.h>
@@ -33,6 +34,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 struct qmc_hdlc *netdev_to_qmc_hdlc(struct net_device *netdev)
@@ -197,6 +199,144 @@ 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)
+{
+ DECLARE_BITMAP(ts_mask_avail, 64);
+ DECLARE_BITMAP(ts_mask, 64);
+ DECLARE_BITMAP(map, 64);
+
+ /* Tx and Rx available 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;
+ }
+
+ bitmap_from_u64(ts_mask_avail, ts_info->rx_ts_mask_avail);
+ bitmap_from_u64(map, slot_map);
+ bitmap_scatter(ts_mask, map, ts_mask_avail, 64);
+
+ if (bitmap_weight(ts_mask, 64) != bitmap_weight(map, 64)) {
+ dev_err(qmc_hdlc->dev, "Cannot translate timeslots %64pb -> (%64pb, %64pb)\n",
+ map, ts_mask_avail, ts_mask);
+ return -EINVAL;
+ }
+
+ bitmap_to_arr64(&ts_info->tx_ts_mask, ts_mask, 64);
+ ts_info->rx_ts_mask = ts_info->tx_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)
+{
+ DECLARE_BITMAP(ts_mask_avail, 64);
+ DECLARE_BITMAP(ts_mask, 64);
+ DECLARE_BITMAP(map, 64);
+ u32 array32[2];
+
+ /* Tx and Rx masks and available 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;
+ }
+
+ bitmap_from_u64(ts_mask_avail, ts_info->rx_ts_mask_avail);
+ bitmap_from_u64(ts_mask, ts_info->rx_ts_mask);
+ bitmap_gather(map, ts_mask, ts_mask_avail, 64);
+
+ if (bitmap_weight(ts_mask, 64) != bitmap_weight(map, 64)) {
+ dev_err(qmc_hdlc->dev, "Cannot translate timeslots (%64pb, %64pb) -> %64pb\n",
+ ts_mask_avail, ts_mask, map);
+ return -EINVAL;
+ }
+
+ bitmap_to_arr32(array32, map, 64);
+ if (array32[1]) {
+ dev_err(qmc_hdlc->dev, "Slot map out of 32bit (%64pb, %64pb) -> %64pb\n",
+ ts_mask_avail, ts_mask, map);
+ return -EINVAL;
+ }
+
+ *slot_map = array32[0];
+ return 0;
+}
+
+static int qmc_hdlc_set_iface(struct qmc_hdlc *qmc_hdlc, int if_iface, const te1_settings *te1)
+{
+ struct qmc_chan_ts_info ts_info;
+ int ret;
+
+ ret = qmc_chan_get_ts_info(qmc_hdlc->qmc_chan, &ts_info);
+ if (ret) {
+ dev_err(qmc_hdlc->dev, "get QMC channel ts info failed %d\n", ret);
+ return ret;
+ }
+ ret = qmc_hdlc_xlate_slot_map(qmc_hdlc, te1->slot_map, &ts_info);
+ if (ret)
+ return ret;
+
+ ret = qmc_chan_set_ts_info(qmc_hdlc->qmc_chan, &ts_info);
+ if (ret) {
+ dev_err(qmc_hdlc->dev, "set QMC channel ts info failed %d\n", ret);
+ return ret;
+ }
+
+ qmc_hdlc->slot_map = te1->slot_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)))
+ 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);
+ }
+}
+
static int qmc_hdlc_open(struct net_device *netdev)
{
struct qmc_hdlc *qmc_hdlc = netdev_to_qmc_hdlc(netdev);
@@ -322,12 +462,13 @@ static const struct net_device_ops qmc_hdlc_netdev_ops = {
.ndo_open = qmc_hdlc_open,
.ndo_stop = qmc_hdlc_close,
.ndo_start_xmit = hdlc_start_xmit,
- .ndo_siocwandev = hdlc_ioctl,
+ .ndo_siocwandev = qmc_hdlc_ioctl,
};
static int qmc_hdlc_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
+ struct qmc_chan_ts_info ts_info;
struct qmc_hdlc *qmc_hdlc;
struct qmc_chan_info info;
hdlc_device *hdlc;
@@ -353,6 +494,14 @@ static int qmc_hdlc_probe(struct platform_device *pdev)
return dev_err_probe(dev, -EINVAL, "QMC chan mode %d is not QMC_HDLC\n",
info.mode);
+ ret = qmc_chan_get_ts_info(qmc_hdlc->qmc_chan, &ts_info);
+ if (ret)
+ return dev_err_probe(dev, ret, "get QMC channel ts info failed\n");
+
+ ret = qmc_hdlc_xlate_ts_info(qmc_hdlc, &ts_info, &qmc_hdlc->slot_map);
+ if (ret)
+ return ret;
+
qmc_hdlc->netdev = alloc_hdlcdev(qmc_hdlc);
if (!qmc_hdlc->netdev)
return -ENOMEM;
--
2.43.0
From: Andy Shevchenko <[email protected]>
These helpers scatters or gathers a bitmap with the help of the mask
position bits parameter.
bitmap_scatter() does the following:
src: 0000000001011010
||||||
+------+|||||
| +----+||||
| |+----+|||
| || +-+||
| || | ||
mask: ...v..vv...v..vv
...0..11...0..10
dst: 0000001100000010
and bitmap_gather() performs this one:
mask: ...v..vv...v..vv
src: 0000001100000010
^ ^^ ^ 0
| || | 10
| || > 010
| |+--> 1010
| +--> 11010
+----> 011010
dst: 0000000000011010
bitmap_gather() can the seen as the reverse bitmap_scatter() operation.
The original work was done by Andy Shevchenko.
Signed-off-by: Andy Shevchenko <[email protected]>
Link: https://lore.kernel.org/lkml/[email protected]/
Co-developed-by: Herve Codina <[email protected]>
Signed-off-by: Herve Codina <[email protected]>
---
include/linux/bitmap.h | 101 +++++++++++++++++++++++++++++++++++++++++
lib/test_bitmap.c | 42 +++++++++++++++++
2 files changed, 143 insertions(+)
diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 99451431e4d6..6d3f6ff1b4d6 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -62,6 +62,8 @@ struct device;
* bitmap_shift_left(dst, src, n, nbits) *dst = *src << n
* bitmap_cut(dst, src, first, n, nbits) Cut n bits from first, copy rest
* bitmap_replace(dst, old, new, mask, nbits) *dst = (*old & ~(*mask)) | (*new & *mask)
+ * bitmap_scatter(dst, src, mask, nbits) *dst = map(dense, sparse)(src)
+ * bitmap_gather(dst, src, mask, nbits) *dst = map(sparse, dense)(src)
* bitmap_remap(dst, src, old, new, nbits) *dst = map(old, new)(src)
* bitmap_bitremap(oldbit, old, new, nbits) newbit = map(old, new)(oldbit)
* bitmap_onto(dst, orig, relmap, nbits) *dst = orig relative to relmap
@@ -487,6 +489,105 @@ static inline void bitmap_replace(unsigned long *dst,
__bitmap_replace(dst, old, new, mask, nbits);
}
+/**
+ * bitmap_scatter - Scatter a bitmap according to the given mask
+ * @dst: scattered bitmap
+ * @src: gathered bitmap
+ * @mask: mask representing bits to assign to in the scattered bitmap
+ * @nbits: number of bits in each of these bitmaps
+ *
+ * Scatters bitmap with sequential bits according to the given @mask.
+ *
+ * Example:
+ * If @src bitmap = 0x005a, with @mask = 0x1313, @dst will be 0x0302.
+ *
+ * Or in binary form
+ * @src @mask @dst
+ * 0000000001011010 0001001100010011 0000001100000010
+ *
+ * (Bits 0, 1, 2, 3, 4, 5 are copied to the bits 0, 1, 4, 8, 9, 12)
+ *
+ * A more 'visual' description of the operation:
+ * src: 0000000001011010
+ * ||||||
+ * +------+|||||
+ * | +----+||||
+ * | |+----+|||
+ * | || +-+||
+ * | || | ||
+ * mask: ...v..vv...v..vv
+ * ...0..11...0..10
+ * dst: 0000001100000010
+ *
+ * A relationship exists between bitmap_scatter() and bitmap_gather().
+ * bitmap_gather() can be seen as the 'reverse' bitmap_scatter() operation.
+ * See bitmap_scatter() for details related to this relationship.
+ */
+static inline void bitmap_scatter(unsigned long *dst, const unsigned long *src,
+ const unsigned long *mask, unsigned int nbits)
+{
+ unsigned int n = 0;
+ unsigned int bit;
+
+ bitmap_zero(dst, nbits);
+
+ for_each_set_bit(bit, mask, nbits)
+ __assign_bit(bit, dst, test_bit(n++, src));
+}
+
+/**
+ * bitmap_gather - Gather a bitmap according to given mask
+ * @dst: gathered bitmap
+ * @src: scattered bitmap
+ * @mask: mask representing bits to extract from in the scattered bitmap
+ * @nbits: number of bits in each of these bitmaps
+ *
+ * Gathers bitmap with sparse bits according to the given @mask.
+ *
+ * Example:
+ * If @src bitmap = 0x0302, with @mask = 0x1313, @dst will be 0x001a.
+ *
+ * Or in binary form
+ * @src @mask @dst
+ * 0000001100000010 0001001100010011 0000000000011010
+ *
+ * (Bits 0, 1, 4, 8, 9, 12 are copied to the bits 0, 1, 2, 3, 4, 5)
+ *
+ * A more 'visual' description of the operation:
+ * mask: ...v..vv...v..vv
+ * src: 0000001100000010
+ * ^ ^^ ^ 0
+ * | || | 10
+ * | || > 010
+ * | |+--> 1010
+ * | +--> 11010
+ * +----> 011010
+ * dst: 0000000000011010
+ *
+ * A relationship exists between bitmap_gather() and bitmap_scatter(). See
+ * bitmap_scatter() for the bitmap scatter detailed operations.
+ * Suppose scattered computed using bitmap_scatter(scattered, src, mask, n).
+ * The operation bitmap_gather(result, scattered, mask, n) leads to a result
+ * equal or equivalent to src.
+ *
+ * The result can be 'equivalent' because bitmap_scatter() and bitmap_gather()
+ * are not bijective.
+ * The result and src values are equivalent in that sense that a call to
+ * bitmap_scatter(res, src, mask, n) and a call to bitmap_scatter(res, result,
+ * mask, n) will lead to the same res value.
+ */
+static inline void bitmap_gather(unsigned long *dst, const unsigned long *src,
+ const unsigned long *mask, unsigned int nbits)
+{
+ unsigned int n = 0;
+ unsigned int bit;
+
+ bitmap_zero(dst, nbits);
+
+ for_each_set_bit(bit, mask, nbits)
+ __assign_bit(n++, dst, test_bit(bit, src));
+}
+
static inline void bitmap_next_set_region(unsigned long *bitmap,
unsigned int *rs, unsigned int *re,
unsigned int end)
diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
index 65f22c2578b0..6b2b33579f56 100644
--- a/lib/test_bitmap.c
+++ b/lib/test_bitmap.c
@@ -380,6 +380,47 @@ static void __init test_replace(void)
expect_eq_bitmap(bmap, exp3_1_0, nbits);
}
+static const unsigned long sg_mask[] __initconst = {
+ BITMAP_FROM_U64(0x000000000000035aULL),
+};
+
+static const unsigned long sg_src[] __initconst = {
+ BITMAP_FROM_U64(0x0000000000000667ULL),
+};
+
+static const unsigned long sg_gather_exp[] __initconst = {
+ BITMAP_FROM_U64(0x0000000000000029ULL),
+};
+
+static const unsigned long sg_scatter_exp[] __initconst = {
+ BITMAP_FROM_U64(0x000000000000021aULL),
+};
+
+static void __init test_bitmap_sg(void)
+{
+ unsigned int nbits = 64;
+ DECLARE_BITMAP(bmap_gather, 100);
+ DECLARE_BITMAP(bmap_scatter, 100);
+ DECLARE_BITMAP(bmap_tmp, 100);
+ DECLARE_BITMAP(bmap_res, 100);
+
+ /* Simple gather call */
+ bitmap_zero(bmap_gather, 100);
+ bitmap_gather(bmap_gather, sg_src, sg_mask, nbits);
+ expect_eq_bitmap(sg_gather_exp, bmap_gather, nbits);
+
+ /* Simple scatter call */
+ bitmap_zero(bmap_scatter, 100);
+ bitmap_scatter(bmap_scatter, sg_src, sg_mask, nbits);
+ expect_eq_bitmap(sg_scatter_exp, bmap_scatter, nbits);
+
+ /* Scatter/gather relationship */
+ bitmap_zero(bmap_tmp, 100);
+ bitmap_gather(bmap_tmp, bmap_scatter, sg_mask, nbits);
+ bitmap_scatter(bmap_res, bmap_tmp, sg_mask, nbits);
+ expect_eq_bitmap(bmap_scatter, bmap_res, nbits);
+}
+
#define PARSE_TIME 0x1
#define NO_LEN 0x2
@@ -1252,6 +1293,7 @@ static void __init selftest(void)
test_copy();
test_bitmap_region();
test_replace();
+ test_bitmap_sg();
test_bitmap_arr32();
test_bitmap_arr64();
test_bitmap_parse();
--
2.43.0
On Thu, Feb 29, 2024 at 03:15:51PM +0100, Herve Codina wrote:
> From: Andy Shevchenko <[email protected]>
>
> These helpers scatters or gathers a bitmap with the help of the mask
> position bits parameter.
>
> bitmap_scatter() does the following:
> src: 0000000001011010
> ||||||
> +------+|||||
> | +----+||||
> | |+----+|||
> | || +-+||
> | || | ||
> mask: ...v..vv...v..vv
> ...0..11...0..10
> dst: 0000001100000010
>
> and bitmap_gather() performs this one:
> mask: ...v..vv...v..vv
> src: 0000001100000010
> ^ ^^ ^ 0
> | || | 10
> | || > 010
> | |+--> 1010
> | +--> 11010
> +----> 011010
> dst: 0000000000011010
>
> bitmap_gather() can the seen as the reverse bitmap_scatter() operation.
> The original work was done by Andy Shevchenko.
As I said, it's too much credit :-)
..
> + * A relationship exists between bitmap_gather() and bitmap_scatter(). See
> + * bitmap_scatter() for the bitmap scatter detailed operations.
> + * Suppose scattered computed using bitmap_scatter(scattered, src, mask, n).
> + * The operation bitmap_gather(result, scattered, mask, n) leads to a result
> + * equal or equivalent to src.
> + *
> + * The result can be 'equivalent' because bitmap_scatter() and bitmap_gather()
> + * are not bijective.
> + * The result and src values are equivalent in that sense that a call to
> + * bitmap_scatter(res, src, mask, n) and a call to bitmap_scatter(res, result,
> + * mask, n) will lead to the same res value.
For better readability I wouldn't break API calls, hence
The result and src values are equivalent in that sense that a call to
bitmap_scatter(res, src, mask, n) and a call to
bitmap_scatter(res, result, mask, n) will lead to the same res value.
--
With Best Regards,
Andy Shevchenko
On Thu, Feb 29, 2024 at 03:15:52PM +0100, Herve Codina wrote:
> 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.
..
> + bitmap_scatter(ts_mask, map, ts_mask_avail, 64);
Wondering if we may have returned value more useful and hence having something like
n = bitmap_scatter(...);
> + if (bitmap_weight(ts_mask, 64) != bitmap_weight(map, 64)) {
if (n != ...) {
?
> + dev_err(qmc_hdlc->dev, "Cannot translate timeslots %64pb -> (%64pb, %64pb)\n",
> + map, ts_mask_avail, ts_mask);
> + return -EINVAL;
> + }
..
> + bitmap_gather(map, ts_mask, ts_mask_avail, 64);
> +
> + if (bitmap_weight(ts_mask, 64) != bitmap_weight(map, 64)) {
> + dev_err(qmc_hdlc->dev, "Cannot translate timeslots (%64pb, %64pb) -> %64pb\n",
> + ts_mask_avail, ts_mask, map);
> + return -EINVAL;
> + }
Ditto.
--
With Best Regards,
Andy Shevchenko
Hi Andy,
On Thu, 29 Feb 2024 17:20:59 +0200
Andy Shevchenko <[email protected]> wrote:
> On Thu, Feb 29, 2024 at 03:15:52PM +0100, Herve Codina wrote:
> > 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.
>
> ...
>
> > + bitmap_scatter(ts_mask, map, ts_mask_avail, 64);
>
> Wondering if we may have returned value more useful and hence having something like
>
> n = bitmap_scatter(...);
I thought about it.
In bitmap_{scatter,gather}(dst, src, mask, nbits), only returning the
weight of the third parameter (i.e. mask) can be efficient regarding to the
for_each_set_bit() loop done in the functions.
For dst parameter, we need to add a counter in the loop to count the number
of bit set depending on the test_bit() result. Will this be more efficient
than a call to bitmap_weight() ?
Also, in my case, the third parameter is ts_mask_avail and I don't need
its weight.
I thing users that need to have the dst or src weight should call
bitmap_weight() themselves as this is users context dependent.
bitmap_{scatter,gather}(dst, src, mask, nbits) can be improved later with
no impact to current users (except performance).
That's why I concluded to return nothing from bitmap_{scatter,gather} when
I took the old existing patches.
>
> > + if (bitmap_weight(ts_mask, 64) != bitmap_weight(map, 64)) {
>
> if (n != ...) {
>
> ?
>
> > + dev_err(qmc_hdlc->dev, "Cannot translate timeslots %64pb -> (%64pb, %64pb)\n",
> > + map, ts_mask_avail, ts_mask);
> > + return -EINVAL;
> > + }
>
> ...
>
> > + bitmap_gather(map, ts_mask, ts_mask_avail, 64);
> > +
> > + if (bitmap_weight(ts_mask, 64) != bitmap_weight(map, 64)) {
> > + dev_err(qmc_hdlc->dev, "Cannot translate timeslots (%64pb, %64pb) -> %64pb\n",
> > + ts_mask_avail, ts_mask, map);
> > + return -EINVAL;
> > + }
>
> Ditto.
>
Best regards,
Hervé
On Thu, Feb 29, 2024 at 03:15:48PM +0100, Herve Codina wrote:
> Hi,
>
> This series introduces the QMC HDLC support.
>
> Patches were previously sent as part of a full feature series and were
> previously reviewed in that context:
> "Add support for QMC HDLC, framer infrastructure and PEF2256 framer" [1]
>
> In order to ease the merge, the full feature series has been split and
> needed parts were merged in v6.8-rc1:
> - "Prepare the PowerQUICC QMC and TSA for the HDLC QMC driver" [2]
> - "Add support for framer infrastructure and PEF2256 framer" [3]
>
> This series contains patches related to the QMC HDLC part (QMC HDLC
> driver):
> - Introduce the QMC HDLC driver (patches 1 and 2)
> - Add timeslots change support in QMC HDLC (patch 3)
> - Add framer support as a framer consumer in QMC HDLC (patch 4)
>
> Compare to the original full feature series, a modification was done on
> patch 3 in order to use a coherent prefix in the commit title.
>
> I kept the patches unsquashed as they were previously sent and reviewed.
> Of course, I can squash them if needed.
I think it's a good series and next version will be final. The only question is
possible use of the returned values from bitmap_scatter()/bitmap_gather(), the
rest are minors.
Feel free to add
Reviewed-by: Andy Shevchenko <[email protected]>
to patches 4 and 5.
--
With Best Regards,
Andy Shevchenko
On Tue, Mar 05, 2024 at 12:02:26PM +0100, Herve Codina wrote:
> On Thu, 29 Feb 2024 17:23:32 +0200
> Andy Shevchenko <[email protected]> wrote:
..
> > I think it's a good series and next version will be final. The only question is
> > possible use of the returned values from bitmap_scatter()/bitmap_gather(), the
> > rest are minors.
>
> I replied about the reason why I didn't return any values from
> bitmap_{scatter,gather}() in that patch 4 discussion.
>
> Are you ok to keep them returning void in this series ?
Sure. We can amend that later if needed.
--
With Best Regards,
Andy Shevchenko