* [PATCH v2 0/6] Configurable VLAN mode for NCSI driver @ 2022-06-10 16:59 Jiaqing Zhao 2022-06-10 16:59 ` [PATCH v2 1/6] net/ncsi: Fix value of NCSI_CAP_VLAN_ANY Jiaqing Zhao ` (6 more replies) 0 siblings, 7 replies; 17+ messages in thread From: Jiaqing Zhao @ 2022-06-10 16:59 UTC (permalink / raw) To: Samuel Mendoza-Jonas, David S . Miller, netdev, devicetree, linux-kernel, openbmc Cc: Jiaqing Zhao Currently kernel NCSI driver only supports the "VLAN + non-VLAN" mode (Mode #2), but this mode is an optional mode [1] defined in NCSI spec and some NCSI devices like Intel E810 Network Adapter [2] does not support that mode. This patchset adds a new "ncsi,vlan-mode" device tree property for configuring the VLAN mode of NCSI device. [1] Table 58 - VLAN Enable Modes https://www.dmtf.org/sites/default/files/standards/documents/DSP0222_1.0.0.pdf [2] 12.6.5.4.3 VLAN https://cdrdv2.intel.com/v1/dl/getContent/613875 v2: - Fix indentation Jiaqing Zhao (6): net/ncsi: Fix value of NCSI_CAP_VLAN_ANY net/ncsi: Rename NCSI_CAP_VLAN_NO to NCSI_CAP_VLAN_FILTERED net/ncsi: Enable VLAN filtering when callback is registered ftgmac100: Remove enable NCSI VLAN filtering dt-bindings: net: Add NCSI bindings net/ncsi: Support VLAN mode configuration .../devicetree/bindings/net/ncsi.yaml | 34 ++++++++++++++ MAINTAINERS | 2 + drivers/net/ethernet/faraday/ftgmac100.c | 3 -- include/dt-bindings/net/ncsi.h | 15 ++++++ net/ncsi/internal.h | 5 +- net/ncsi/ncsi-manage.c | 46 ++++++++++++++++--- 6 files changed, 93 insertions(+), 12 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/ncsi.yaml create mode 100644 include/dt-bindings/net/ncsi.h -- 2.34.1 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 1/6] net/ncsi: Fix value of NCSI_CAP_VLAN_ANY 2022-06-10 16:59 [PATCH v2 0/6] Configurable VLAN mode for NCSI driver Jiaqing Zhao @ 2022-06-10 16:59 ` Jiaqing Zhao 2022-06-10 16:59 ` [PATCH v2 2/6] net/ncsi: Rename NCSI_CAP_VLAN_NO to NCSI_CAP_VLAN_FILTERED Jiaqing Zhao ` (5 subsequent siblings) 6 siblings, 0 replies; 17+ messages in thread From: Jiaqing Zhao @ 2022-06-10 16:59 UTC (permalink / raw) To: Samuel Mendoza-Jonas, David S . Miller, netdev, devicetree, linux-kernel, openbmc Cc: Jiaqing Zhao According to the NCSI specification (DSP0222) [1], the value of allow Any VLAN + non-VLAN mode in Enable VLAN command should be 0x03. [1] https://www.dmtf.org/sites/default/files/standards/documents/DSP0222_1.0.0.pdf Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com> --- net/ncsi/internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h index 03757e76bb6b..bc4a00e41695 100644 --- a/net/ncsi/internal.h +++ b/net/ncsi/internal.h @@ -47,7 +47,7 @@ enum { NCSI_CAP_AEN_MASK = 0x07, NCSI_CAP_VLAN_ONLY = 0x01, /* Filter VLAN packet only */ NCSI_CAP_VLAN_NO = 0x02, /* Filter VLAN and non-VLAN */ - NCSI_CAP_VLAN_ANY = 0x04, /* Filter Any-and-non-VLAN */ + NCSI_CAP_VLAN_ANY = 0x03, /* Filter Any-and-non-VLAN */ NCSI_CAP_VLAN_MASK = 0x07 }; -- 2.34.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 2/6] net/ncsi: Rename NCSI_CAP_VLAN_NO to NCSI_CAP_VLAN_FILTERED 2022-06-10 16:59 [PATCH v2 0/6] Configurable VLAN mode for NCSI driver Jiaqing Zhao 2022-06-10 16:59 ` [PATCH v2 1/6] net/ncsi: Fix value of NCSI_CAP_VLAN_ANY Jiaqing Zhao @ 2022-06-10 16:59 ` Jiaqing Zhao 2022-06-10 16:59 ` [PATCH v2 3/6] net/ncsi: Enable VLAN filtering when callback is registered Jiaqing Zhao ` (4 subsequent siblings) 6 siblings, 0 replies; 17+ messages in thread From: Jiaqing Zhao @ 2022-06-10 16:59 UTC (permalink / raw) To: Samuel Mendoza-Jonas, David S . Miller, netdev, devicetree, linux-kernel, openbmc Cc: Jiaqing Zhao The NCSI_CAP_VLAN_NO actually stands for the "VLAN + non-VLAN" mode defined in NCSI spec, which accepts both VLAN-tagged packets that match the enabled VLAN filter settings and non-VLAN-tagged packets. It would be more clear to rename it to NCSI_CAP_VLAN_FILTERED. Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com> --- net/ncsi/internal.h | 2 +- net/ncsi/ncsi-manage.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h index bc4a00e41695..7f384f841019 100644 --- a/net/ncsi/internal.h +++ b/net/ncsi/internal.h @@ -46,7 +46,7 @@ enum { NCSI_CAP_AEN_HDS = 0x04, /* HNC driver status */ NCSI_CAP_AEN_MASK = 0x07, NCSI_CAP_VLAN_ONLY = 0x01, /* Filter VLAN packet only */ - NCSI_CAP_VLAN_NO = 0x02, /* Filter VLAN and non-VLAN */ + NCSI_CAP_VLAN_FILTERED = 0x02, /* Filter VLAN and non-VLAN */ NCSI_CAP_VLAN_ANY = 0x03, /* Filter Any-and-non-VLAN */ NCSI_CAP_VLAN_MASK = 0x07 }; diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c index 78814417d753..a8f7a2ff52a0 100644 --- a/net/ncsi/ncsi-manage.c +++ b/net/ncsi/ncsi-manage.c @@ -1098,7 +1098,7 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) nca.type = NCSI_PKT_CMD_DV; } else { nca.type = NCSI_PKT_CMD_EV; - nca.bytes[3] = NCSI_CAP_VLAN_NO; + nca.bytes[3] = NCSI_CAP_VLAN_FILTERED; } nd->state = ncsi_dev_state_config_sma; } else if (nd->state == ncsi_dev_state_config_sma) { -- 2.34.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 3/6] net/ncsi: Enable VLAN filtering when callback is registered 2022-06-10 16:59 [PATCH v2 0/6] Configurable VLAN mode for NCSI driver Jiaqing Zhao 2022-06-10 16:59 ` [PATCH v2 1/6] net/ncsi: Fix value of NCSI_CAP_VLAN_ANY Jiaqing Zhao 2022-06-10 16:59 ` [PATCH v2 2/6] net/ncsi: Rename NCSI_CAP_VLAN_NO to NCSI_CAP_VLAN_FILTERED Jiaqing Zhao @ 2022-06-10 16:59 ` Jiaqing Zhao 2022-06-10 16:59 ` [PATCH v2 4/6] ftgmac100: Remove enable NCSI VLAN filtering Jiaqing Zhao ` (3 subsequent siblings) 6 siblings, 0 replies; 17+ messages in thread From: Jiaqing Zhao @ 2022-06-10 16:59 UTC (permalink / raw) To: Samuel Mendoza-Jonas, David S . Miller, netdev, devicetree, linux-kernel, openbmc Cc: Jiaqing Zhao Sets NETIF_F_HW_VLAN_CTAG_FILTER flag to enable hardware VLAN filtering of NCSI when the ndo_vlan_rx_{add,kill}_vid callbacks are registered to the NCSI handlers. Previously it was done in the mac driver, this patch puts it to the NCSI drvier to make it more general. Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com> --- net/ncsi/ncsi-manage.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c index a8f7a2ff52a0..3fb95f29e3e2 100644 --- a/net/ncsi/ncsi-manage.c +++ b/net/ncsi/ncsi-manage.c @@ -1807,6 +1807,11 @@ struct ncsi_dev *ncsi_register_dev(struct net_device *dev, ndp->mlx_multi_host = true; } + /* Enable hardware VLAN filtering */ + if (dev->netdev_ops->ndo_vlan_rx_add_vid == ncsi_vlan_rx_add_vid && + dev->netdev_ops->ndo_vlan_rx_kill_vid == ncsi_vlan_rx_kill_vid) + dev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER; + return nd; } EXPORT_SYMBOL_GPL(ncsi_register_dev); -- 2.34.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 4/6] ftgmac100: Remove enable NCSI VLAN filtering 2022-06-10 16:59 [PATCH v2 0/6] Configurable VLAN mode for NCSI driver Jiaqing Zhao ` (2 preceding siblings ...) 2022-06-10 16:59 ` [PATCH v2 3/6] net/ncsi: Enable VLAN filtering when callback is registered Jiaqing Zhao @ 2022-06-10 16:59 ` Jiaqing Zhao 2022-06-10 16:59 ` [PATCH v2 5/6] dt-bindings: net: Add NCSI bindings Jiaqing Zhao ` (2 subsequent siblings) 6 siblings, 0 replies; 17+ messages in thread From: Jiaqing Zhao @ 2022-06-10 16:59 UTC (permalink / raw) To: Samuel Mendoza-Jonas, David S . Miller, netdev, devicetree, linux-kernel, openbmc Cc: Jiaqing Zhao Setting NETIF_F_HW_VLAN_CTAG_FILTER flag to enable NCSI VLAN filtering has been moved to the NCSI driver, the logic in ftgmac100 driver is no longer needed. Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com> --- drivers/net/ethernet/faraday/ftgmac100.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c index 5231818943c6..18821ca38795 100644 --- a/drivers/net/ethernet/faraday/ftgmac100.c +++ b/drivers/net/ethernet/faraday/ftgmac100.c @@ -1922,9 +1922,6 @@ static int ftgmac100_probe(struct platform_device *pdev) NETIF_F_GRO | NETIF_F_SG | NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_HW_VLAN_CTAG_TX; - if (priv->use_ncsi) - netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER; - /* AST2400 doesn't have working HW checksum generation */ if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac"))) netdev->hw_features &= ~NETIF_F_HW_CSUM; -- 2.34.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 5/6] dt-bindings: net: Add NCSI bindings 2022-06-10 16:59 [PATCH v2 0/6] Configurable VLAN mode for NCSI driver Jiaqing Zhao ` (3 preceding siblings ...) 2022-06-10 16:59 ` [PATCH v2 4/6] ftgmac100: Remove enable NCSI VLAN filtering Jiaqing Zhao @ 2022-06-10 16:59 ` Jiaqing Zhao 2022-06-10 23:19 ` Rob Herring 2022-06-10 16:59 ` [PATCH v2 6/6] net/ncsi: Support VLAN mode configuration Jiaqing Zhao 2022-06-10 20:09 ` [PATCH v2 0/6] Configurable VLAN mode for NCSI driver Jakub Kicinski 6 siblings, 1 reply; 17+ messages in thread From: Jiaqing Zhao @ 2022-06-10 16:59 UTC (permalink / raw) To: Samuel Mendoza-Jonas, David S . Miller, netdev, devicetree, linux-kernel, openbmc Cc: Jiaqing Zhao Add devicetree bindings for NCSI VLAN modes. This allows VLAN mode to be configured in devicetree. Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com> --- .../devicetree/bindings/net/ncsi.yaml | 34 +++++++++++++++++++ MAINTAINERS | 2 ++ include/dt-bindings/net/ncsi.h | 15 ++++++++ 3 files changed, 51 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/ncsi.yaml create mode 100644 include/dt-bindings/net/ncsi.h diff --git a/Documentation/devicetree/bindings/net/ncsi.yaml b/Documentation/devicetree/bindings/net/ncsi.yaml new file mode 100644 index 000000000000..ec76ae9a77a9 --- /dev/null +++ b/Documentation/devicetree/bindings/net/ncsi.yaml @@ -0,0 +1,34 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/net/ncsi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Network Controller Sideband Interface (NCSI) + +maintainers: + - Samuel Mendoza-Jonas <sam@mendozajonas.com> + +description: | + Bindings for the Network Controller Sideband Interface (NCSI) driver + +properties: + ncsi,vlan-mode: + description: VLAN mode used on the NCSI device. + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 1, 2, 3] + +examples: + - | + #include <dt-bindings/net/ncsi.h> + + mac0: ethernet@1e660000 { + compatible = "aspeed,ast2600-mac", "faraday,ftgmac100"; + reg = <0x1e660000 0x180>; + status = "okay"; + + use-ncsi; + ncsi,vlan-mode = <NCSI_VLAN_MODE_ANY>; + }; + +... diff --git a/MAINTAINERS b/MAINTAINERS index f468864fd268..199e4b5bceab 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -13534,6 +13534,8 @@ F: drivers/scsi/sun3_scsi_vme.c NCSI LIBRARY M: Samuel Mendoza-Jonas <sam@mendozajonas.com> S: Maintained +F: Documentation/devicetree/bindings/net/ncsi.yaml +F: include/dt-bindings/net/ncsi.h F: net/ncsi/ NCT6775 HARDWARE MONITOR DRIVER diff --git a/include/dt-bindings/net/ncsi.h b/include/dt-bindings/net/ncsi.h new file mode 100644 index 000000000000..d45790d97a21 --- /dev/null +++ b/include/dt-bindings/net/ncsi.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Device Tree constants for NCSI + */ + +#ifndef _DT_BINDINGS_NCSI_H +#define _DT_BINDINGS_NCSI_H + +/* VLAN Modes */ +#define NCSI_VLAN_MODE_DISABLED 0 +#define NCSI_VLAN_MODE_ONLY 1 +#define NCSI_VLAN_MODE_FILTERED 2 +#define NCSI_VLAN_MODE_ANY 3 + +#endif -- 2.34.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 5/6] dt-bindings: net: Add NCSI bindings 2022-06-10 16:59 ` [PATCH v2 5/6] dt-bindings: net: Add NCSI bindings Jiaqing Zhao @ 2022-06-10 23:19 ` Rob Herring 2022-06-11 3:09 ` Jiaqing Zhao 0 siblings, 1 reply; 17+ messages in thread From: Rob Herring @ 2022-06-10 23:19 UTC (permalink / raw) To: Jiaqing Zhao Cc: openbmc, netdev, devicetree, David S . Miller, Samuel Mendoza-Jonas, linux-kernel On Sat, 11 Jun 2022 00:59:39 +0800, Jiaqing Zhao wrote: > Add devicetree bindings for NCSI VLAN modes. This allows VLAN mode to > be configured in devicetree. > > Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com> > --- > .../devicetree/bindings/net/ncsi.yaml | 34 +++++++++++++++++++ > MAINTAINERS | 2 ++ > include/dt-bindings/net/ncsi.h | 15 ++++++++ > 3 files changed, 51 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/ncsi.yaml > create mode 100644 include/dt-bindings/net/ncsi.h > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ncsi.yaml: 'oneOf' conditional failed, one must be fixed: 'unevaluatedProperties' is a required property 'additionalProperties' is a required property hint: Either unevaluatedProperties or additionalProperties must be present from schema $id: http://devicetree.org/meta-schemas/core.yaml# /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ncsi.yaml: ignoring, error in schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ncsi.example.dtb: ethernet@1e660000: 'ncsi,vlan-mode' does not match any of the regexes: '^#.*', '^(at25|bm|devbus|dmacap|dsa|exynos|fsi[ab]|gpio-fan|gpio-key|gpio|gpmc|hdmi|i2c-gpio),.*', '^(keypad|m25p|max8952|max8997|max8998|mpmc),.*', '^(pinctrl-single|#pinctrl-single|PowerPC),.*', '^(pl022|pxa-mmc|rcar_sound|rotary-encoder|s5m8767|sdhci),.*', '^(simple-audio-card|st-plgpio|st-spics|ts),.*', '^100ask,.*', '^70mai,.*', '^8dev,.*', '^GEFanuc,.*', '^ORCL,.*', '^SUNW,.*', '^[a-zA-Z0-9#_][a-zA-Z0-9+\\-._@]{0,63}$', '^[a-zA-Z0-9+\\-._]*@[0-9a-zA-Z,]*$', '^abb,.*', '^abilis,.*', '^abracon,.*', '^abt,.*', '^acer,.*', '^acme,.*', '^actions,.*', '^active-semi,.*', '^ad,.*', '^adafruit,.*', '^adapteva,.*', '^adaptrum,.*', '^adh,.*', '^adi,.*', '^advantech,.*', '^aeroflexgaisler,.*', '^aesop,.*', '^airoha,.*', '^al,.*', '^alcatel,.*', '^allegro,.*', '^allo,.*', '^allwinner,.*', '^alphascale,.*', '^alps,.*', '^alt,.*', ' ^altr,.*', '^amarula,.*', '^amazon,.*', '^amcc,.*', '^amd,.*', '^amediatech,.*', '^amlogic,.*', '^ampere,.*', '^ampire,.*', '^ams,.*', '^amstaos,.*', '^analogix,.*', '^andestech,.*', '^anvo,.*', '^apm,.*', '^apple,.*', '^aptina,.*', '^arasan,.*', '^archermind,.*', '^arctic,.*', '^arcx,.*', '^aries,.*', '^arm,.*', '^armadeus,.*', '^arrow,.*', '^artesyn,.*', '^asahi-kasei,.*', '^asc,.*', '^asix,.*', '^aspeed,.*', '^asus,.*', '^atlas,.*', '^atmel,.*', '^auo,.*', '^auvidea,.*', '^avago,.*', '^avia,.*', '^avic,.*', '^avnet,.*', '^awinic,.*', '^axentia,.*', '^axis,.*', '^azoteq,.*', '^azw,.*', '^baikal,.*', '^bananapi,.*', '^beacon,.*', '^beagle,.*', '^bhf,.*', '^bitmain,.*', '^blutek,.*', '^boe,.*', '^bosch,.*', '^boundary,.*', '^brcm,.*', '^broadmobi,.*', '^bsh,.*', '^bticino,.*', '^buffalo,.*', '^bur,.*', '^calamp,.*', '^calaosystems,.*', '^calxeda,.*', '^canaan,.*', '^caninos,.*', '^capella,.*', '^cascoda,.*', '^catalyst,.*', '^cavium,.*', '^cdns,.*', '^cdtech,.*', '^cellwise,.*', '^c eva,.*', '^checkpoint,.*', '^chefree,.*', '^chipidea,.*', '^chipone,.*', '^chipspark,.*', '^chrontel,.*', '^chrp,.*', '^chunghwa,.*', '^chuwi,.*', '^ciaa,.*', '^cirrus,.*', '^cisco,.*', '^cloudengines,.*', '^cnm,.*', '^cnxt,.*', '^colorfly,.*', '^compulab,.*', '^congatec,.*', '^coreriver,.*', '^corpro,.*', '^cortina,.*', '^cosmic,.*', '^crane,.*', '^creative,.*', '^crystalfontz,.*', '^csky,.*', '^csq,.*', '^ctera,.*', '^ctu,.*', '^cubietech,.*', '^cui,.*', '^cypress,.*', '^cyx,.*', '^cznic,.*', '^dallas,.*', '^dataimage,.*', '^davicom,.*', '^dell,.*', '^delta,.*', '^denx,.*', '^devantech,.*', '^dfi,.*', '^dh,.*', '^difrnce,.*', '^digi,.*', '^digilent,.*', '^dioo,.*', '^dlc,.*', '^dlg,.*', '^dlink,.*', '^dmo,.*', '^domintech,.*', '^dongwoon,.*', '^dptechnics,.*', '^dragino,.*', '^ds,.*', '^dserve,.*', '^dynaimage,.*', '^ea,.*', '^ebang,.*', '^ebs-systart,.*', '^ebv,.*', '^eckelmann,.*', '^edimax,.*', '^edt,.*', '^eeti,.*', '^einfochips,.*', '^eink,.*', '^elan,.*', '^element14,.*', '^ elgin,.*', '^elida,.*', '^elimo,.*', '^elpida,.*', '^embest,.*', '^emlid,.*', '^emmicro,.*', '^empire-electronix,.*', '^emtrion,.*', '^enclustra,.*', '^endless,.*', '^ene,.*', '^energymicro,.*', '^engicam,.*', '^engleder,.*', '^epcos,.*', '^epfl,.*', '^epson,.*', '^esp,.*', '^est,.*', '^ettus,.*', '^eukrea,.*', '^everest,.*', '^everspin,.*', '^evervision,.*', '^exar,.*', '^excito,.*', '^exegin,.*', '^ezchip,.*', '^facebook,.*', '^fairphone,.*', '^faraday,.*', '^fastrax,.*', '^fcs,.*', '^feixin,.*', '^feiyang,.*', '^fii,.*', '^firefly,.*', '^focaltech,.*', '^forlinx,.*', '^frida,.*', '^friendlyarm,.*', '^fsl,.*', '^fujitsu,.*', '^fxtec,.*', '^gardena,.*', '^gateworks,.*', '^gcw,.*', '^ge,.*', '^geekbuying,.*', '^gef,.*', '^gemei,.*', '^geniatech,.*', '^giantec,.*', '^giantplus,.*', '^globalscale,.*', '^globaltop,.*', '^gmt,.*', '^goodix,.*', '^google,.*', '^grinn,.*', '^grmn,.*', '^gumstix,.*', '^gw,.*', '^hannstar,.*', '^haochuangyi,.*', '^haoyu,.*', '^hardkernel,.*', '^hideep,.*', '^himax,.*', '^hirschmann,.*', '^hisi,.*', '^hisilicon,.*', '^hit,.*', '^hitex,.*', '^holt,.*', '^holtek,.*', '^honestar,.*', '^honeywell,.*', '^hoperun,.*', '^hp,.*', '^hpe,.*', '^hsg,.*', '^huawei,.*', '^hugsun,.*', '^hwacom,.*', '^hycon,.*', '^hydis,.*', '^hynix,.*', '^hyundai,.*', '^i2se,.*', '^ibm,.*', '^icplus,.*', '^idt,.*', '^ifi,.*', '^ilitek,.*', '^imagis,.*', '^img,.*', '^imi,.*', '^incircuit,.*', '^inet-tek,.*', '^infineon,.*', '^inforce,.*', '^ingenic,.*', '^injoinic,.*', '^innolux,.*', '^inside-secure,.*', '^insignal,.*', '^inspur,.*', '^intel,.*', '^intercontrol,.*', '^invensense,.*', '^inversepath,.*', '^iom,.*', '^isee,.*', '^isil,.*', '^issi,.*', '^ite,.*', '^itead,.*', '^itian,.*', '^ivo,.*', '^iwave,.*', '^jdi,.*', '^jedec,.*', '^jesurun,.*', '^jethome,.*', '^jianda,.*', '^joz,.*', '^kam,.*', '^karo,.*', '^keithkoep,.*', '^keymile,.*', '^khadas,.*', '^kiebackpeter,.*', '^kinetic,.*', '^kingdisplay,.*', '^kingnovel,.*', '^kionix,.*', '^kobo,.*', '^kobol,.*', '^koe ,.*', '^kontron,.*', '^kosagi,.*', '^kvg,.*', '^kyo,.*', '^lacie,.*', '^laird,.*', '^lamobo,.*', '^lantiq,.*', '^lattice,.*', '^leadtek,.*', '^leez,.*', '^lego,.*', '^lemaker,.*', '^lenovo,.*', '^lg,.*', '^lgphilips,.*', '^libretech,.*', '^licheepi,.*', '^linaro,.*', '^linksprite,.*', '^linksys,.*', '^linutronix,.*', '^linux,.*', '^linx,.*', '^liteon,.*', '^litex,.*', '^lltc,.*', '^logicpd,.*', '^logictechno,.*', '^longcheer,.*', '^lontium,.*', '^loongson,.*', '^lsi,.*', '^lwn,.*', '^lxa,.*', '^m5stack,.*', '^macnica,.*', '^mantix,.*', '^mapleboard,.*', '^marvell,.*', '^maxbotix,.*', '^maxim,.*', '^mbvl,.*', '^mcube,.*', '^meas,.*', '^mecer,.*', '^mediatek,.*', '^megachips,.*', '^mele,.*', '^melexis,.*', '^melfas,.*', '^mellanox,.*', '^memsic,.*', '^menlo,.*', '^mentor,.*', '^meraki,.*', '^merrii,.*', '^micrel,.*', '^microchip,.*', '^microcrystal,.*', '^micron,.*', '^microsoft,.*', '^microsys,.*', '^mikroe,.*', '^mikrotik,.*', '^miniand,.*', '^minix,.*', '^miramems,.*', '^mitsubishi ,.*', '^miyoo,.*', '^mntre,.*', '^modtronix,.*', '^mosaixtech,.*', '^motorola,.*', '^moxa,.*', '^mpl,.*', '^mps,.*', '^mqmaker,.*', '^mrvl,.*', '^mscc,.*', '^msi,.*', '^mstar,.*', '^mti,.*', '^multi-inno,.*', '^mundoreader,.*', '^murata,.*', '^mxic,.*', '^mxicy,.*', '^myir,.*', '^national,.*', '^nec,.*', '^neonode,.*', '^netgear,.*', '^netlogic,.*', '^netron-dy,.*', '^netronix,.*', '^netxeon,.*', '^neweast,.*', '^newhaven,.*', '^nexbox,.*', '^nextthing,.*', '^ni,.*', '^nintendo,.*', '^nlt,.*', '^nokia,.*', '^nordic,.*', '^novtech,.*', '^nutsboard,.*', '^nuvoton,.*', '^nvd,.*', '^nvidia,.*', '^nxp,.*', '^oceanic,.*', '^ocs,.*', '^oct,.*', '^okaya,.*', '^oki,.*', '^olimex,.*', '^olpc,.*', '^oneplus,.*', '^onion,.*', '^onnn,.*', '^ontat,.*', '^opalkelly,.*', '^opencores,.*', '^openembed,.*', '^openrisc,.*', '^option,.*', '^oranth,.*', '^orisetech,.*', '^ortustech,.*', '^osddisplays,.*', '^osmc,.*', '^ouya,.*', '^overkiz,.*', '^ovti,.*', '^oxsemi,.*', '^ozzmaker,.*', '^panasonic,.*', '^ parade,.*', '^parallax,.*', '^pda,.*', '^pericom,.*', '^pervasive,.*', '^phicomm,.*', '^phytec,.*', '^picochip,.*', '^pine64,.*', '^pineriver,.*', '^pixcir,.*', '^plantower,.*', '^plathome,.*', '^plda,.*', '^plx,.*', '^ply,.*', '^pni,.*', '^pocketbook,.*', '^polaroid,.*', '^portwell,.*', '^poslab,.*', '^pov,.*', '^powertip,.*', '^powervr,.*', '^primux,.*', '^probox2,.*', '^prt,.*', '^pulsedlight,.*', '^purism,.*', '^qca,.*', '^qcom,.*', '^qemu,.*', '^qi,.*', '^qiaodian,.*', '^qihua,.*', '^qishenglong,.*', '^qnap,.*', '^radxa,.*', '^raidsonic,.*', '^ralink,.*', '^ramtron,.*', '^raspberrypi,.*', '^raydium,.*', '^rda,.*', '^realtek,.*', '^remarkable,.*', '^renesas,.*', '^rervision,.*', '^revotics,.*', '^rex,.*', '^richtek,.*', '^ricoh,.*', '^rikomagic,.*', '^riot,.*', '^riscv,.*', '^rockchip,.*', '^rocktech,.*', '^rohm,.*', '^ronbo,.*', '^roofull,.*', '^roseapplepi,.*', '^samsung,.*', '^samtec,.*', '^sancloud,.*', '^sandisk,.*', '^satoz,.*', '^sbs,.*', '^schindler,.*', '^seagate,.*', ' ^seeed,.*', '^seirobotics,.*', '^semtech,.*', '^senseair,.*', '^sensirion,.*', '^sensortek,.*', '^sercomm,.*', '^sff,.*', '^sgd,.*', '^sgmicro,.*', '^sgx,.*', '^sharp,.*', '^shimafuji,.*', '^shiratech,.*', '^si-en,.*', '^si-linux,.*', '^siemens,.*', '^sifive,.*', '^sigma,.*', '^sii,.*', '^sil,.*', '^silabs,.*', '^silan,.*', '^silead,.*', '^silergy,.*', '^silex-insight,.*', '^siliconfile,.*', '^siliconmitus,.*', '^silvaco,.*', '^simtek,.*', '^sinlinx,.*', '^sinovoip,.*', '^sinowealth,.*', '^sipeed,.*', '^sirf,.*', '^sis,.*', '^sitronix,.*', '^skov,.*', '^skyworks,.*', '^smartlabs,.*', '^smsc,.*', '^snps,.*', '^sochip,.*', '^socionext,.*', '^solidrun,.*', '^solomon,.*', '^sony,.*', '^spansion,.*', '^sparkfun,.*', '^spinalhdl,.*', '^sprd,.*', '^ssi,.*', '^sst,.*', '^sstar,.*', '^st,.*', '^st-ericsson,.*', '^starfive,.*', '^starry,.*', '^startek,.*', '^ste,.*', '^stericsson,.*', '^storlink,.*', '^storm,.*', '^storopack,.*', '^summit,.*', '^sunchip,.*', '^sundance,.*', '^sunplus,.*', '^s upermicro,.*', '^swir,.*', '^syna,.*', '^synology,.*', '^synopsys,.*', '^tbs,.*', '^tbs-biometrics,.*', '^tcg,.*', '^tcl,.*', '^tcs,.*', '^tdo,.*', '^team-source-display,.*', '^technexion,.*', '^technologic,.*', '^techstar,.*', '^teltonika,.*', '^tempo,.*', '^terasic,.*', '^tesla,.*', '^tfc,.*', '^thead,.*', '^thine,.*', '^thingyjp,.*', '^thundercomm,.*', '^ti,.*', '^tianma,.*', '^tlm,.*', '^tmt,.*', '^topeet,.*', '^topic,.*', '^toppoly,.*', '^topwise,.*', '^toradex,.*', '^toshiba,.*', '^toumaz,.*', '^tpk,.*', '^tplink,.*', '^tpo,.*', '^tq,.*', '^traverse,.*', '^tronfy,.*', '^tronsmart,.*', '^truly,.*', '^tsd,.*', '^tyan,.*', '^u-blox,.*', '^u-boot,.*', '^ubnt,.*', '^ucrobotics,.*', '^udoo,.*', '^ugoos,.*', '^uniwest,.*', '^upisemi,.*', '^urt,.*', '^usi,.*', '^utoo,.*', '^v3,.*', '^vaisala,.*', '^vamrs,.*', '^variscite,.*', '^vdl,.*', '^vertexcom,.*', '^via,.*', '^vicor,.*', '^videostrong,.*', '^virtio,.*', '^virtual,.*', '^vishay,.*', '^visionox,.*', '^vitesse,.*', '^vivante,.*', ' ^vivax,.*', '^vocore,.*', '^voipac,.*', '^vot,.*', '^vxt,.*', '^wanchanglong,.*', '^wand,.*', '^waveshare,.*', '^wd,.*', '^we,.*', '^welltech,.*', '^wetek,.*', '^wexler,.*', '^whwave,.*', '^wi2wi,.*', '^wiligear,.*', '^willsemi,.*', '^winbond,.*', '^wingtech,.*', '^winlink,.*', '^winstar,.*', '^wirelesstag,.*', '^wits,.*', '^wlf,.*', '^wm,.*', '^wobo,.*', '^x-powers,.*', '^xen,.*', '^xes,.*', '^xiaomi,.*', '^xillybus,.*', '^xingbangda,.*', '^xinpeng,.*', '^xiphera,.*', '^xlnx,.*', '^xnano,.*', '^xunlong,.*', '^xylon,.*', '^yadro,.*', '^yamaha,.*', '^yes-optoelectronics,.*', '^yic,.*', '^ylm,.*', '^yna,.*', '^yones-toptech,.*', '^ys,.*', '^ysoft,.*', '^zarlink,.*', '^zealz,.*', '^zeitec,.*', '^zidoo,.*', '^zii,.*', '^zinitix,.*', '^zkmagic,.*', '^zte,.*', '^zyxel,.*' From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/vendor-prefixes.yaml Documentation/devicetree/bindings/net/ncsi.example.dtb:0:0: /example-0/ethernet@1e660000: failed to match any schema with compatible: ['aspeed,ast2600-mac', 'faraday,ftgmac100'] Documentation/devicetree/bindings/net/ncsi.example.dtb:0:0: /example-0/ethernet@1e660000: failed to match any schema with compatible: ['aspeed,ast2600-mac', 'faraday,ftgmac100'] doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/ This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 5/6] dt-bindings: net: Add NCSI bindings 2022-06-10 23:19 ` Rob Herring @ 2022-06-11 3:09 ` Jiaqing Zhao 2022-06-13 15:28 ` Rob Herring 0 siblings, 1 reply; 17+ messages in thread From: Jiaqing Zhao @ 2022-06-11 3:09 UTC (permalink / raw) To: Rob Herring Cc: openbmc, netdev, devicetree, David S . Miller, Samuel Mendoza-Jonas, linux-kernel On 2022-06-11 07:19, Rob Herring wrote: > On Sat, 11 Jun 2022 00:59:39 +0800, Jiaqing Zhao wrote: >> Add devicetree bindings for NCSI VLAN modes. This allows VLAN mode to >> be configured in devicetree. >> >> Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com> >> --- >> .../devicetree/bindings/net/ncsi.yaml | 34 +++++++++++++++++++ >> MAINTAINERS | 2 ++ >> include/dt-bindings/net/ncsi.h | 15 ++++++++ >> 3 files changed, 51 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/net/ncsi.yaml >> create mode 100644 include/dt-bindings/net/ncsi.h >> > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > yamllint warnings/errors: > > dtschema/dtc warnings/errors: > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ncsi.yaml: 'oneOf' conditional failed, one must be fixed: > 'unevaluatedProperties' is a required property > 'additionalProperties' is a required property > hint: Either unevaluatedProperties or additionalProperties must be present > from schema $id: http://devicetree.org/meta-schemas/core.yaml# > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ncsi.yaml: ignoring, error in schema: > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ncsi.example.dtb: ethernet@1e660000: 'ncsi,vlan-mode' does not match any of the regexes > From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/vendor-prefixes.yaml I saw vendor-prefix.yaml says do not add non-vendor prefixes to the list. Since "ncsi" is not a vendor, may I ask what is the suggested replacement for 'ncsi,vlan-mode'? Will 'ncsi-vlan-mode' be fine? > Documentation/devicetree/bindings/net/ncsi.example.dtb:0:0: /example-0/ethernet@1e660000: failed to match any schema with compatible: ['aspeed,ast2600-mac', 'faraday,ftgmac100'] > Documentation/devicetree/bindings/net/ncsi.example.dtb:0:0: /example-0/ethernet@1e660000: failed to match any schema with compatible: ['aspeed,ast2600-mac', 'faraday,ftgmac100'] The ftgmac100 it depends on uses a txt document instead of an yaml schema. And I see there is other schemas having the same error, can this be ignored? And I've got one more question. The ncsi driver does not has its own compatible field, instead, it is enabled by setting the "use-ncsi" property of some specific mac drivers. Though currently only ftgmac100 supports ncsi in upstream kernel, it may be used by other mac drivers in the future. What do you think is a proper way for defining the ncsi schema? Having it in a separate yaml like this patch or add the properties to all the mac yamls that supports yaml? If the former way is preferred, how should the schema be defined without "compatible"? > doc reference errors (make refcheckdocs): > > See https://patchwork.ozlabs.org/patch/ > > This check can fail if there are any dependencies. The base for a patch > series is generally the most recent rc1. > > If you already ran 'make dt_binding_check' and didn't see the above > error(s), then make sure 'yamllint' is installed and dt-schema is up to > date: > > pip3 install dtschema --upgrade > > Please check and re-submit. > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 5/6] dt-bindings: net: Add NCSI bindings 2022-06-11 3:09 ` Jiaqing Zhao @ 2022-06-13 15:28 ` Rob Herring 2022-06-14 2:33 ` Jiaqing Zhao 0 siblings, 1 reply; 17+ messages in thread From: Rob Herring @ 2022-06-13 15:28 UTC (permalink / raw) To: Jiaqing Zhao Cc: OpenBMC Maillist, netdev, devicetree, David S . Miller, Samuel Mendoza-Jonas, linux-kernel On Fri, Jun 10, 2022 at 9:09 PM Jiaqing Zhao <jiaqing.zhao@linux.intel.com> wrote: > > On 2022-06-11 07:19, Rob Herring wrote: > > On Sat, 11 Jun 2022 00:59:39 +0800, Jiaqing Zhao wrote: > >> Add devicetree bindings for NCSI VLAN modes. This allows VLAN mode to > >> be configured in devicetree. > >> > >> Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com> > >> --- > >> .../devicetree/bindings/net/ncsi.yaml | 34 +++++++++++++++++++ > >> MAINTAINERS | 2 ++ > >> include/dt-bindings/net/ncsi.h | 15 ++++++++ > >> 3 files changed, 51 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/net/ncsi.yaml > >> create mode 100644 include/dt-bindings/net/ncsi.h > >> > > > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > > > yamllint warnings/errors: > > > > dtschema/dtc warnings/errors: > > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ncsi.yaml: 'oneOf' conditional failed, one must be fixed: > > 'unevaluatedProperties' is a required property > > 'additionalProperties' is a required property > > hint: Either unevaluatedProperties or additionalProperties must be present > > from schema $id: http://devicetree.org/meta-schemas/core.yaml# > > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ncsi.yaml: ignoring, error in schema: > > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ncsi.example.dtb: ethernet@1e660000: 'ncsi,vlan-mode' does not match any of the regexes > > From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/vendor-prefixes.yaml > > I saw vendor-prefix.yaml says do not add non-vendor prefixes to the list. Since "ncsi" is not a vendor, may I ask what is the suggested replacement for 'ncsi,vlan-mode'? Will 'ncsi-vlan-mode' be fine? I don't know. What is NCSI? Is it specific to certain MACs? Why do you need to set this up in DT? Network configuration is typically done in userspace, so putting VLAN config in DT doesn't seem right. All questions your commit message should answer. > > > Documentation/devicetree/bindings/net/ncsi.example.dtb:0:0: /example-0/ethernet@1e660000: failed to match any schema with compatible: ['aspeed,ast2600-mac', 'faraday,ftgmac100'] > > Documentation/devicetree/bindings/net/ncsi.example.dtb:0:0: /example-0/ethernet@1e660000: failed to match any schema with compatible: ['aspeed,ast2600-mac', 'faraday,ftgmac100'] > > The ftgmac100 it depends on uses a txt document instead of an yaml schema. And I see there is other schemas having the same error, can this be ignored? No. Don't add to the list. Once all the existing warnings (~40) are fixed, then this will be turned on by default. > > And I've got one more question. The ncsi driver does not has its own compatible field, instead, it is enabled by setting the "use-ncsi" property of some specific mac drivers. Though currently only ftgmac100 supports ncsi in upstream kernel, it may be used by other mac drivers in the future. What do you think is a proper way for defining the ncsi schema? Having it in a separate yaml like this patch or add the properties to all the mac yamls that supports yaml? If the former way is preferred, how should the schema be defined without "compatible"? If it is a function of driver support or not, then it doesn't belong in DT. Rob ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 5/6] dt-bindings: net: Add NCSI bindings 2022-06-13 15:28 ` Rob Herring @ 2022-06-14 2:33 ` Jiaqing Zhao 0 siblings, 0 replies; 17+ messages in thread From: Jiaqing Zhao @ 2022-06-14 2:33 UTC (permalink / raw) To: Rob Herring Cc: OpenBMC Maillist, netdev, devicetree, David S . Miller, Samuel Mendoza-Jonas, linux-kernel On 2022-06-13 23:28, Rob Herring wrote: > On Fri, Jun 10, 2022 at 9:09 PM Jiaqing Zhao > <jiaqing.zhao@linux.intel.com> wrote: >> >> On 2022-06-11 07:19, Rob Herring wrote: >>> On Sat, 11 Jun 2022 00:59:39 +0800, Jiaqing Zhao wrote: >>>> Add devicetree bindings for NCSI VLAN modes. This allows VLAN mode to >>>> be configured in devicetree. >>>> >>>> Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com> >>>> --- >>>> .../devicetree/bindings/net/ncsi.yaml | 34 +++++++++++++++++++ >>>> MAINTAINERS | 2 ++ >>>> include/dt-bindings/net/ncsi.h | 15 ++++++++ >>>> 3 files changed, 51 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/net/ncsi.yaml >>>> create mode 100644 include/dt-bindings/net/ncsi.h >>>> >>> >>> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' >>> on your patch (DT_CHECKER_FLAGS is new in v5.13): >>> >>> yamllint warnings/errors: >>> >>> dtschema/dtc warnings/errors: >>> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ncsi.yaml: 'oneOf' conditional failed, one must be fixed: >>> 'unevaluatedProperties' is a required property >>> 'additionalProperties' is a required property >>> hint: Either unevaluatedProperties or additionalProperties must be present >>> from schema $id: http://devicetree.org/meta-schemas/core.yaml# >>> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ncsi.yaml: ignoring, error in schema: >>> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ncsi.example.dtb: ethernet@1e660000: 'ncsi,vlan-mode' does not match any of the regexes >>> From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/vendor-prefixes.yaml >> >> I saw vendor-prefix.yaml says do not add non-vendor prefixes to the list. Since "ncsi" is not a vendor, may I ask what is the suggested replacement for 'ncsi,vlan-mode'? Will 'ncsi-vlan-mode' be fine? > > I don't know. What is NCSI? Is it specific to certain MACs? Why do you > need to set this up in DT? Network configuration is typically done in > userspace, so putting VLAN config in DT doesn't seem right. All > questions your commit message should answer. NCSI is a protocol that uses an external MAC+PHY like a PHY, the topology looks like: Packets + NCSI commands Packets MAC-------------------------External MAC---------PHY Some MACs like ftgmac100 driver supports using NCSI instead of PHY, the operation mode is configured by a DT option "use-ncsi". The NCSI external MAC has its own configuration, like VLAN filter mode of it, and all NCSI devices uses a generic driver. So I these external mac configurations to DT as they are device properties to kernel. Userspace is only able to configure the "internal" MAC. >>> Documentation/devicetree/bindings/net/ncsi.example.dtb:0:0: /example-0/ethernet@1e660000: failed to match any schema with compatible: ['aspeed,ast2600-mac', 'faraday,ftgmac100'] >>> Documentation/devicetree/bindings/net/ncsi.example.dtb:0:0: /example-0/ethernet@1e660000: failed to match any schema with compatible: ['aspeed,ast2600-mac', 'faraday,ftgmac100'] >> >> The ftgmac100 it depends on uses a txt document instead of an yaml schema. And I see there is other schemas having the same error, can this be ignored? > > No. Don't add to the list. Once all the existing warnings (~40) are > fixed, then this will be turned on by default. Sure, I'll put this to ftgmac100.txt instead of separate yaml file. >> >> And I've got one more question. The ncsi driver does not has its own compatible field, instead, it is enabled by setting the "use-ncsi" property of some specific mac drivers. Though currently only ftgmac100 supports ncsi in upstream kernel, it may be used by other mac drivers in the future. What do you think is a proper way for defining the ncsi schema? Having it in a separate yaml like this patch or add the properties to all the mac yamls that supports yaml? If the former way is preferred, how should the schema be defined without "compatible"? > > If it is a function of driver support or not, then it doesn't belong in DT. > > Rob It's a hardware operation mode of the external MAC, I think it's reasonable to put it in DT. There is also a previous patch adding NCSI MAC config "mlx,multi-host" in DT at https://lore.kernel.org/netdev/20200108234341.2590674-1-vijaykhemka@fb.com/T/ I referred this for my implementation, though it is undocumented. Jiaqing ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 6/6] net/ncsi: Support VLAN mode configuration 2022-06-10 16:59 [PATCH v2 0/6] Configurable VLAN mode for NCSI driver Jiaqing Zhao ` (4 preceding siblings ...) 2022-06-10 16:59 ` [PATCH v2 5/6] dt-bindings: net: Add NCSI bindings Jiaqing Zhao @ 2022-06-10 16:59 ` Jiaqing Zhao 2022-06-10 20:09 ` [PATCH v2 0/6] Configurable VLAN mode for NCSI driver Jakub Kicinski 6 siblings, 0 replies; 17+ messages in thread From: Jiaqing Zhao @ 2022-06-10 16:59 UTC (permalink / raw) To: Samuel Mendoza-Jonas, David S . Miller, netdev, devicetree, linux-kernel, openbmc Cc: Jiaqing Zhao NCSI specification defines 4 VLAN modes, currently kernel NCSI driver only supports the "VLAN + non-VLAN" mode (Mode #2), and there is no way to detect which modes are supported by the device. This patch adds support for configuring VLAN mode via the "ncsi,vlan-mode" devicetree node. Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com> --- net/ncsi/internal.h | 1 + net/ncsi/ncsi-manage.c | 41 ++++++++++++++++++++++++++++++++++------- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h index 7f384f841019..6a988c898a8d 100644 --- a/net/ncsi/internal.h +++ b/net/ncsi/internal.h @@ -334,6 +334,7 @@ struct ncsi_dev_priv { struct work_struct work; /* For channel management */ struct packet_type ptype; /* NCSI packet Rx handler */ struct list_head node; /* Form NCSI device list */ + u32 vlan_mode; /* VLAN mode */ #define NCSI_MAX_VLAN_VIDS 15 struct list_head vlan_vids; /* List of active VLAN IDs */ diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c index 3fb95f29e3e2..a398b0eb72b2 100644 --- a/net/ncsi/ncsi-manage.c +++ b/net/ncsi/ncsi-manage.c @@ -10,6 +10,7 @@ #include <linux/skbuff.h> #include <linux/of.h> #include <linux/platform_device.h> +#include <dt-bindings/net/ncsi.h> #include <net/ncsi.h> #include <net/net_namespace.h> @@ -1042,7 +1043,11 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) nd->state = ncsi_dev_state_config_oem_gma; break; case ncsi_dev_state_config_oem_gma: - nd->state = ncsi_dev_state_config_clear_vids; + /* Only set up hardware VLAN filters in filtered mode */ + if (ndp->vlan_mode == NCSI_VLAN_MODE_FILTERED) + nd->state = ncsi_dev_state_config_clear_vids; + else + nd->state = ncsi_dev_state_config_ev; ret = -1; #if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC) @@ -1094,11 +1099,15 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) nd->state = ncsi_dev_state_config_svf; /* Enable/Disable the VLAN filter */ } else if (nd->state == ncsi_dev_state_config_ev) { - if (list_empty(&ndp->vlan_vids)) { - nca.type = NCSI_PKT_CMD_DV; - } else { + if (ndp->vlan_mode == NCSI_VLAN_MODE_FILTERED && + !list_empty(&ndp->vlan_vids)) { nca.type = NCSI_PKT_CMD_EV; nca.bytes[3] = NCSI_CAP_VLAN_FILTERED; + } else if (ndp->vlan_mode == NCSI_VLAN_MODE_ANY) { + nca.type = NCSI_PKT_CMD_EV; + nca.bytes[3] = NCSI_CAP_VLAN_ANY; + } else { + nca.type = NCSI_PKT_CMD_DV; } nd->state = ncsi_dev_state_config_sma; } else if (nd->state == ncsi_dev_state_config_sma) { @@ -1800,15 +1809,33 @@ struct ncsi_dev *ncsi_register_dev(struct net_device *dev, ndp->ptype.dev = dev; dev_add_pack(&ndp->ptype); + /* Set default VLAN mode (filtered) */ + ndp->vlan_mode = NCSI_VLAN_MODE_FILTERED; + pdev = to_platform_device(dev->dev.parent); if (pdev) { np = pdev->dev.of_node; - if (np && of_get_property(np, "mlx,multi-host", NULL)) - ndp->mlx_multi_host = true; + if (np) { + u32 vlan_mode; + + if (!of_property_read_u32(np, "ncsi,vlan-mode", &vlan_mode)) { + if (vlan_mode > NCSI_VLAN_MODE_ANY || + vlan_mode == NCSI_VLAN_MODE_ONLY) + dev_warn(&pdev->dev, "NCSI: Unsupported VLAN mode %u", + vlan_mode); + else + ndp->vlan_mode = vlan_mode; + dev_info(&pdev->dev, "NCSI: Configured VLAN mode %u", + ndp->vlan_mode); + } + if (of_get_property(np, "mlx,multi-host", NULL)) + ndp->mlx_multi_host = true; + } } /* Enable hardware VLAN filtering */ - if (dev->netdev_ops->ndo_vlan_rx_add_vid == ncsi_vlan_rx_add_vid && + if (ndp->vlan_mode == NCSI_VLAN_MODE_FILTERED && + dev->netdev_ops->ndo_vlan_rx_add_vid == ncsi_vlan_rx_add_vid && dev->netdev_ops->ndo_vlan_rx_kill_vid == ncsi_vlan_rx_kill_vid) dev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER; -- 2.34.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 0/6] Configurable VLAN mode for NCSI driver 2022-06-10 16:59 [PATCH v2 0/6] Configurable VLAN mode for NCSI driver Jiaqing Zhao ` (5 preceding siblings ...) 2022-06-10 16:59 ` [PATCH v2 6/6] net/ncsi: Support VLAN mode configuration Jiaqing Zhao @ 2022-06-10 20:09 ` Jakub Kicinski 2022-06-11 3:25 ` Jiaqing Zhao 6 siblings, 1 reply; 17+ messages in thread From: Jakub Kicinski @ 2022-06-10 20:09 UTC (permalink / raw) To: Jiaqing Zhao Cc: Samuel Mendoza-Jonas, David S . Miller, netdev, devicetree, linux-kernel, openbmc On Sat, 11 Jun 2022 00:59:34 +0800 Jiaqing Zhao wrote: > Currently kernel NCSI driver only supports the "VLAN + non-VLAN" mode > (Mode #2), but this mode is an optional mode [1] defined in NCSI spec > and some NCSI devices like Intel E810 Network Adapter [2] does not > support that mode. This patchset adds a new "ncsi,vlan-mode" device > tree property for configuring the VLAN mode of NCSI device. > > [1] Table 58 - VLAN Enable Modes > https://www.dmtf.org/sites/default/files/standards/documents/DSP0222_1.0.0.pdf > [2] 12.6.5.4.3 VLAN > https://cdrdv2.intel.com/v1/dl/getContent/613875 Please don't post the same patches more than once a day. You posted the same patches 3 times within 15 minutes with no major difference :/ Why is "ncsi,vlan-mode" set via the device tree? Looks like something that can be configured at runtime. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 0/6] Configurable VLAN mode for NCSI driver 2022-06-10 20:09 ` [PATCH v2 0/6] Configurable VLAN mode for NCSI driver Jakub Kicinski @ 2022-06-11 3:25 ` Jiaqing Zhao 2022-06-11 4:45 ` Jakub Kicinski 0 siblings, 1 reply; 17+ messages in thread From: Jiaqing Zhao @ 2022-06-11 3:25 UTC (permalink / raw) To: Jakub Kicinski Cc: Samuel Mendoza-Jonas, David S . Miller, netdev, devicetree, linux-kernel, openbmc On 2022-06-11 04:09, Jakub Kicinski wrote: > On Sat, 11 Jun 2022 00:59:34 +0800 Jiaqing Zhao wrote: >> Currently kernel NCSI driver only supports the "VLAN + non-VLAN" mode >> (Mode #2), but this mode is an optional mode [1] defined in NCSI spec >> and some NCSI devices like Intel E810 Network Adapter [2] does not >> support that mode. This patchset adds a new "ncsi,vlan-mode" device >> tree property for configuring the VLAN mode of NCSI device. >> >> [1] Table 58 - VLAN Enable Modes >> https://www.dmtf.org/sites/default/files/standards/documents/DSP0222_1.0.0.pdf >> [2] 12.6.5.4.3 VLAN >> https://cdrdv2.intel.com/v1/dl/getContent/613875 > > Please don't post the same patches more than once a day. You posted the > same patches 3 times within 15 minutes with no major difference :/ Got it, sorry for misusing the mailing list. > Why is "ncsi,vlan-mode" set via the device tree? Looks like something > that can be configured at runtime. Actually this cannot be configured at runtime, the NCSI spec defines no command or register to determine which mode is supported by the device. If kernel want to enable VLAN on the NCSI device, either "Filtered tagged + Untagged" (current default) or "Any tagged + untagged" mode should be enabled, but unfortunately both of these two modes are documented to be optionally supported in the spec. And in real cases, there are devices that only supports one of them, or neither of them. So I added the device tree property to configure which mode to use. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 0/6] Configurable VLAN mode for NCSI driver 2022-06-11 3:25 ` Jiaqing Zhao @ 2022-06-11 4:45 ` Jakub Kicinski 2022-06-11 5:18 ` Jiaqing Zhao 0 siblings, 1 reply; 17+ messages in thread From: Jakub Kicinski @ 2022-06-11 4:45 UTC (permalink / raw) To: Jiaqing Zhao Cc: Samuel Mendoza-Jonas, David S . Miller, netdev, devicetree, linux-kernel, openbmc On Sat, 11 Jun 2022 11:25:03 +0800 Jiaqing Zhao wrote: > > Why is "ncsi,vlan-mode" set via the device tree? Looks like something > > that can be configured at runtime. > > Actually this cannot be configured at runtime, the NCSI spec defines no > command or register to determine which mode is supported by the device. To be clear I'm not saying that it should be auto-detected and auto-configured. Just that user space can issue a command to change the config. > If kernel want to enable VLAN on the NCSI device, either "Filtered tagged > + Untagged" (current default) or "Any tagged + untagged" mode should be > enabled, but unfortunately both of these two modes are documented to be > optionally supported in the spec. And in real cases, there are devices > that only supports one of them, or neither of them. So I added the device > tree property to configure which mode to use. But for a given device its driver knows what modes are supported. Is it not possible to make the VLAN mode passed thru ncsi-netlink? Better still, can't "Filtered tagged + Untagged" vs "Any tagged + untagged" be decided based on netdev features being enabled like it is for normal netdevs? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 0/6] Configurable VLAN mode for NCSI driver 2022-06-11 4:45 ` Jakub Kicinski @ 2022-06-11 5:18 ` Jiaqing Zhao 2022-06-11 5:44 ` Jakub Kicinski 0 siblings, 1 reply; 17+ messages in thread From: Jiaqing Zhao @ 2022-06-11 5:18 UTC (permalink / raw) To: Jakub Kicinski Cc: Samuel Mendoza-Jonas, David S . Miller, netdev, devicetree, linux-kernel, openbmc On 2022-06-11 12:45, Jakub Kicinski wrote: > On Sat, 11 Jun 2022 11:25:03 +0800 Jiaqing Zhao wrote: >>> Why is "ncsi,vlan-mode" set via the device tree? Looks like something >>> that can be configured at runtime. >> >> Actually this cannot be configured at runtime, the NCSI spec defines no >> command or register to determine which mode is supported by the device. > > To be clear I'm not saying that it should be auto-detected and > auto-configured. Just that user space can issue a command to change > the config. > >> If kernel want to enable VLAN on the NCSI device, either "Filtered tagged >> + Untagged" (current default) or "Any tagged + untagged" mode should be >> enabled, but unfortunately both of these two modes are documented to be >> optionally supported in the spec. And in real cases, there are devices >> that only supports one of them, or neither of them. So I added the device >> tree property to configure which mode to use. > > But for a given device its driver knows what modes are supported. > Is it not possible to make the VLAN mode passed thru ncsi-netlink? > > Better still, can't "Filtered tagged + Untagged" vs "Any tagged + > untagged" be decided based on netdev features being enabled like it > is for normal netdevs? All ncsi devices uses the same driver as they uses same command set, so the driver doesn't know what modes are supported. And in current driver, the vlan related parameters are configured when registering the device, adding an ncsi-netlink command to do so seems to be unsuitable. And adding a netlink command requires extra application in userspace to switch the mode. In my opinion, it would be more user-friendly to make it usable on boot. Netdev also does not work as the ncsi device itself does not have its own netdev, the netdev comes from the mac device. For different vlan modes, the netdev feature set of its parent mac device are the same. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 0/6] Configurable VLAN mode for NCSI driver 2022-06-11 5:18 ` Jiaqing Zhao @ 2022-06-11 5:44 ` Jakub Kicinski 2022-06-11 9:58 ` Jiaqing Zhao 0 siblings, 1 reply; 17+ messages in thread From: Jakub Kicinski @ 2022-06-11 5:44 UTC (permalink / raw) To: Jiaqing Zhao Cc: Samuel Mendoza-Jonas, David S . Miller, netdev, devicetree, linux-kernel, openbmc On Sat, 11 Jun 2022 13:18:51 +0800 Jiaqing Zhao wrote: > All ncsi devices uses the same driver as they uses same command set, > so the driver doesn't know what modes are supported. And in current > driver, the vlan related parameters are configured when registering > the device, adding an ncsi-netlink command to do so seems to be > unsuitable. Maybe you could draw a diagram? NC-SI is a bit confusing. > And adding a netlink command requires extra application in userspace > to switch the mode. In my opinion, it would be more user-friendly to > make it usable on boot. Unfortunately convenience is not reason to start adding system config into DT. > Netdev also does not work as the ncsi device itself does not have > its own netdev, the netdev comes from the mac device. For different > vlan modes, the netdev feature set of its parent mac device are the > same. You say that, yet the command handling already takes into account the VLAN list: if (list_empty(&ndp->vlan_vids)) { which come from the MAC netdev. What's wrong with setting the filtering mode based on NETIF_F_HW_VLAN_CTAG_FILTER ? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 0/6] Configurable VLAN mode for NCSI driver 2022-06-11 5:44 ` Jakub Kicinski @ 2022-06-11 9:58 ` Jiaqing Zhao 0 siblings, 0 replies; 17+ messages in thread From: Jiaqing Zhao @ 2022-06-11 9:58 UTC (permalink / raw) To: Jakub Kicinski Cc: Samuel Mendoza-Jonas, David S . Miller, netdev, devicetree, linux-kernel, openbmc On 2022-06-11 13:44, Jakub Kicinski wrote: > On Sat, 11 Jun 2022 13:18:51 +0800 Jiaqing Zhao wrote: >> All ncsi devices uses the same driver as they uses same command set, >> so the driver doesn't know what modes are supported. And in current >> driver, the vlan related parameters are configured when registering >> the device, adding an ncsi-netlink command to do so seems to be >> unsuitable. > > Maybe you could draw a diagram? NC-SI is a bit confusing. Yes I admit NC-SI is confusing as its design is not as straightforward as the MAC-PHY structure. In NC-SI, there are two macs like below. Packets + NCSI commands Packets MAC-------------------------External controller MAC---------PHY The NCSI commands are used to set the behavior of the External controller MAC, like it's MAC address filter, VLAN filters. Those filtered packets will be transferred back to the MAC. Unlike PHY has standard registers to determine its model and capabilities, NC-SI seems does not have such way. >> And adding a netlink command requires extra application in userspace >> to switch the mode. In my opinion, it would be more user-friendly to >> make it usable on boot. > > Unfortunately convenience is not reason to start adding system config > into DT. Currently there is already a DT config "use-ncsi" is used to choose using MDIO PHY or NCSI stack in the MAC driver with NCSI support like ftgmac100. That's why I choose adding another DT option here. >> Netdev also does not work as the ncsi device itself does not have >> its own netdev, the netdev comes from the mac device. For different >> vlan modes, the netdev feature set of its parent mac device are the >> same. > > You say that, yet the command handling already takes into account the > VLAN list: > > if (list_empty(&ndp->vlan_vids)) { > > which come from the MAC netdev. What's wrong with setting the filtering > mode based on NETIF_F_HW_VLAN_CTAG_FILTER ? When configuring the mac driver, there might be two net_device_ops sets for MDIO or NC-SI. When using NC-SI, some features need to be delegated to the external controller MAC, like VLAN hardware filtering, different ndo_vlan_rx_{add,kill}_vid callbacks need to be assigned. The filtering mode is an optional mode defined in NC-SI spec, some devices does not support it. In this case, to support VLAN, I would personally in favor of using the "Any VLAN" mode to let the external MAC pass all packets to the internal one, and let the internal MAC handle it either with its own hardware filter or software filter. In this case, the VLAN list in NC-SI driver (used for setting the external MAC filter) is not used. ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-06-14 2:55 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-06-10 16:59 [PATCH v2 0/6] Configurable VLAN mode for NCSI driver Jiaqing Zhao 2022-06-10 16:59 ` [PATCH v2 1/6] net/ncsi: Fix value of NCSI_CAP_VLAN_ANY Jiaqing Zhao 2022-06-10 16:59 ` [PATCH v2 2/6] net/ncsi: Rename NCSI_CAP_VLAN_NO to NCSI_CAP_VLAN_FILTERED Jiaqing Zhao 2022-06-10 16:59 ` [PATCH v2 3/6] net/ncsi: Enable VLAN filtering when callback is registered Jiaqing Zhao 2022-06-10 16:59 ` [PATCH v2 4/6] ftgmac100: Remove enable NCSI VLAN filtering Jiaqing Zhao 2022-06-10 16:59 ` [PATCH v2 5/6] dt-bindings: net: Add NCSI bindings Jiaqing Zhao 2022-06-10 23:19 ` Rob Herring 2022-06-11 3:09 ` Jiaqing Zhao 2022-06-13 15:28 ` Rob Herring 2022-06-14 2:33 ` Jiaqing Zhao 2022-06-10 16:59 ` [PATCH v2 6/6] net/ncsi: Support VLAN mode configuration Jiaqing Zhao 2022-06-10 20:09 ` [PATCH v2 0/6] Configurable VLAN mode for NCSI driver Jakub Kicinski 2022-06-11 3:25 ` Jiaqing Zhao 2022-06-11 4:45 ` Jakub Kicinski 2022-06-11 5:18 ` Jiaqing Zhao 2022-06-11 5:44 ` Jakub Kicinski 2022-06-11 9:58 ` Jiaqing Zhao
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).