On 21/07/2023 08:26, [email protected] wrote:
> From: Boon Khai Ng <[email protected]>
>
> Currently, VLAN tag stripping is done by software driver in
> stmmac_rx_vlan(). This patch is to Add support for VLAN tag
> stripping by the MAC hardware and MAC drivers to support it.
> This is done by adding rx_hw_vlan() and set_hw_vlan_mode()
> callbacks at stmmac_ops struct which are called from upper
> software layer.
...
> if (priv->dma_cap.vlhash) {
> ndev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
> ndev->features |= NETIF_F_HW_VLAN_STAG_FILTER;
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 23d53ea04b24..bd7f3326a44c 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -543,6 +543,12 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
> plat->flags |= STMMAC_FLAG_TSO_EN;
> }
>
> + /* Rx VLAN HW Stripping */
> + if (of_property_read_bool(np, "snps,rx-vlan-offload")) {
> + dev_info(&pdev->dev, "RX VLAN HW Stripping\n");
Why? Drop.
Best regards,
Krzysztof
> -----Original Message-----
> From: Krzysztof Kozlowski <[email protected]>
> Sent: Friday, July 21, 2023 6:11 PM
> To: [email protected]; [email protected]; "Ng
> <boon.khai.ng"@intel.com; Giuseppe Cavallaro <[email protected]>;
> Alexandre Torgue <[email protected]>; Jose Abreu
> <[email protected]>; David S . Miller <[email protected]>; Eric
> Dumazet <[email protected]>; Jakub Kicinski <[email protected]>;
> Paolo Abeni <[email protected]>; Maxime Coquelin
> <[email protected]>; [email protected]; linux-stm32@st-
> md-mailman.stormreply.com; [email protected]; linux-
> [email protected]
> Cc: Ng, Boon Khai <[email protected]>; Shevchenko, Andriy
> <[email protected]>; Tham, Mun Yew
> <[email protected]>; Swee, Leong Ching
> <[email protected]>; G Thomas, Rohan
> <[email protected]>; Shevchenko Andriy
> <[email protected]>
> Subject: Re: [Enable Designware XGMAC VLAN Stripping Feature 2/2] net:
> stmmac: dwxgmac2: Add support for HW-accelerated VLAN Stripping
>
> On 21/07/2023 08:26, [email protected] wrote:
> > From: Boon Khai Ng <[email protected]>
> >
> > Currently, VLAN tag stripping is done by software driver in
> > stmmac_rx_vlan(). This patch is to Add support for VLAN tag stripping
> > by the MAC hardware and MAC drivers to support it.
> > This is done by adding rx_hw_vlan() and set_hw_vlan_mode() callbacks
> > at stmmac_ops struct which are called from upper software layer.
> ...
>
> > if (priv->dma_cap.vlhash) {
> > ndev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
> > ndev->features |= NETIF_F_HW_VLAN_STAG_FILTER; diff --
> git
> > a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > index 23d53ea04b24..bd7f3326a44c 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > @@ -543,6 +543,12 @@ stmmac_probe_config_dt(struct platform_device
> *pdev, u8 *mac)
> > plat->flags |= STMMAC_FLAG_TSO_EN;
> > }
> >
> > + /* Rx VLAN HW Stripping */
> > + if (of_property_read_bool(np, "snps,rx-vlan-offload")) {
> > + dev_info(&pdev->dev, "RX VLAN HW Stripping\n");
>
> Why? Drop.
>
This is an dts option export to dts for user to choose whether or not they
Want a Hardware stripping or a software stripping.
May I know what is the reason to drop this?
>
>
> Best regards,
> Krzysztof
On 21/07/2023 17:30, Ng, Boon Khai wrote:
>> git
>>> a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>> b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>> index 23d53ea04b24..bd7f3326a44c 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>> @@ -543,6 +543,12 @@ stmmac_probe_config_dt(struct platform_device
>> *pdev, u8 *mac)
>>> plat->flags |= STMMAC_FLAG_TSO_EN;
>>> }
>>>
>>> + /* Rx VLAN HW Stripping */
>>> + if (of_property_read_bool(np, "snps,rx-vlan-offload")) {
>>> + dev_info(&pdev->dev, "RX VLAN HW Stripping\n");
>>
>> Why? Drop.
>>
>
> This is an dts option export to dts for user to choose whether or not they
> Want a Hardware stripping or a software stripping.
>
> May I know what is the reason to drop this?
Because we do not print simple confirmation of DT properties parsing.
It's usually useless and obvious from DT.
To be clear - we talk about dev_info.
Best regards,
Krzysztof
On 7/21/2023 8:30 AM, Ng, Boon Khai wrote:
>> -----Original Message-----
>> From: Krzysztof Kozlowski <[email protected]>
>> Sent: Friday, July 21, 2023 6:11 PM
>> To: [email protected]; [email protected]; "Ng
>> <boon.khai.ng"@intel.com; Giuseppe Cavallaro <[email protected]>;
>> Alexandre Torgue <[email protected]>; Jose Abreu
>> <[email protected]>; David S . Miller <[email protected]>; Eric
>> Dumazet <[email protected]>; Jakub Kicinski <[email protected]>;
>> Paolo Abeni <[email protected]>; Maxime Coquelin
>> <[email protected]>; [email protected]; linux-stm32@st-
>> md-mailman.stormreply.com; [email protected]; linux-
>> [email protected]
>> Cc: Ng, Boon Khai <[email protected]>; Shevchenko, Andriy
>> <[email protected]>; Tham, Mun Yew
>> <[email protected]>; Swee, Leong Ching
>> <[email protected]>; G Thomas, Rohan
>> <[email protected]>; Shevchenko Andriy
>> <[email protected]>
>> Subject: Re: [Enable Designware XGMAC VLAN Stripping Feature 2/2] net:
>> stmmac: dwxgmac2: Add support for HW-accelerated VLAN Stripping
>>
>> On 21/07/2023 08:26, [email protected] wrote:
>>> From: Boon Khai Ng <[email protected]>
>>>
>>> Currently, VLAN tag stripping is done by software driver in
>>> stmmac_rx_vlan(). This patch is to Add support for VLAN tag stripping
>>> by the MAC hardware and MAC drivers to support it.
>>> This is done by adding rx_hw_vlan() and set_hw_vlan_mode() callbacks
>>> at stmmac_ops struct which are called from upper software layer.
>> ...
>>
>>> if (priv->dma_cap.vlhash) {
>>> ndev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
>>> ndev->features |= NETIF_F_HW_VLAN_STAG_FILTER; diff --
>> git
>>> a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>> b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>> index 23d53ea04b24..bd7f3326a44c 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>> @@ -543,6 +543,12 @@ stmmac_probe_config_dt(struct platform_device
>> *pdev, u8 *mac)
>>> plat->flags |= STMMAC_FLAG_TSO_EN;
>>> }
>>>
>>> + /* Rx VLAN HW Stripping */
>>> + if (of_property_read_bool(np, "snps,rx-vlan-offload")) {
>>> + dev_info(&pdev->dev, "RX VLAN HW Stripping\n");
>>
>> Why? Drop.
>>
>
> This is an dts option export to dts for user to choose whether or not they
> Want a Hardware stripping or a software stripping.
>
> May I know what is the reason to drop this?
Because the networking stack already exposes knobs for drivers to
advertise and control VLAN stripping/insertion on RX/TX using ethtool
and feature bits (NETIF_F_HW_VLAN_CTAG_RX, NETIF_F_HW_VLAN_CTAG_TX).
What you are doing here is encode a policy as a Device Tree property
rather than describe whether the hardware supports a given feature and
this is frowned upon.
--
Florian