Re: [PATCH 1/6] stmmac: Define CSUM offload engine Types

[Giuseppe CAVALLARO <peppe.cavallaro@st.com>]

Hello Deepak

On 3/2/2012 1:55 PM, Deepak SIKRI wrote:
> This patch explicitly defines the CSUM offload engine type which need
> (not mandatory) to be passed from the platform code.
> STMMAC core supports two check sum offload engine types- Type-1 & Type-2.
> Also, there are STMMAC cores that do not have the check sum offload
> capabilities.
> The behaviour of Type-1 & Type-2 cores related to provision of checksum
> increases the packet length for Type-1 cores by 2, as the checksum is appended
> at the end of data packet and the same is made accountable in the DMA status.
> The STMMAC cores beyond Version-3.5 provide HW interface registers which allows
> the user to read the HW capabilities, while to support the previous cores the
> information related to HW capabilities has to be provided from the platform
> code.
> The Type-1 cores which do not have the HW register interface need this
> information.

this patch is useful but I've some notes.

Pay attention that, new GMAC devices have the HW capability register to
understand if the rx_coe_type is TYPE1 or TYPE2.
These values should always override the ones come from the platform.
This check is missing in your code.
Can I ask you to rename csum_off_engine_type as rx_coe_type to be
aligned to the name convention used inside the driver.

Regards
Peppe

> Signed-off-by: Deepak Sikri <deepak.sikri@st.com>
> ---
>  include/linux/stmmac.h |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index 0dddc9e..aa0d99e 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -28,6 +28,18 @@
>  
>  #include <linux/platform_device.h>
>  
> +/* Checksum offload engine Types */
> +/* STMMAC core supports two check sum offloading engine types
> + * Type-1 & Type-2
> + * These are configurable portion of the MAC core and hence could be
> + * also made off. 
> + * The Type-0 Macro defined below covers the core which do not support
> + * the checksum offloading.
> + */
> +#define STMMAC_CSUM_T0	0
> +#define STMMAC_CSUM_T1	1
> +#define STMMAC_CSUM_T2	2
> +
>  /* Platfrom data for platform device structure's platform_data field */
>  
>  struct stmmac_mdio_bus_data {
> @@ -57,5 +69,6 @@ struct plat_stmmacenet_data {
>  	void (*exit)(struct platform_device *pdev);
>  	void *custom_cfg;
>  	void *bsp_priv;
> +	int csum_off_engine_type;
>  };
>  #endif