Re: [PATCH net-next v3 6/8] net: dsa: soce: Add basic support for SoC-e switch IP cores

[Andrew Lunn <andrew@lunn.ch>]

> +static void soce_sw_read_core_version(struct soce_dsa_local *local,
> +				      u8 *version, u8 *subversion,
> +				      u16 *revision)
> +{
> +	u32 regval;
> +
> +	regval = readl(local->base_addr + SOCE_CORE_VERSION_OFFSET);
> +	*version = (u8)(regval >> SOCE_CORE_VERSION_VERSION_SHIFT);
> +	*subversion = (u8)(regval >> SOCE_CORE_VERSION_SUBVERSION_SHIFT);
> +	*revision = (u16)regval;

FIELD_GET() would make this more readable. 

> +static int soce_sw_detect_features(struct soce_dsa_local *local,
> +				   u32 *numports)
> +{
> +	void __iomem *base = local->base_addr;
> +	u32 implemented_numports;
> +	u32 licensed_numports;
> +	u32 regval;
> +
> +	regval = readl(base + SOCE_LIC_FEATURES_OFFSET);
> +	licensed_numports = FIELD_GET(SOCE_LIC_FEATURES_NUM_PORTS_MASK, regval);
> +	if (!licensed_numports || licensed_numports > SOCE_MAX_NUM_PORTS)
> +		return -EINVAL;
> +
> +	regval = readl(base + SOCE_IMPL_FEATURES0_OFFSET);
> +	if (!(regval & SOCE_IMPL_FEATURES0_DSA))
> +		return -ENODEV;
> +
> +	implemented_numports =
> +		FIELD_GET(SOCE_IMPL_FEATURES0_NUM_PORTS_MASK, regval);
> +	if (implemented_numports < SOCE_MIN_NUM_PORTS ||
> +	    implemented_numports > licensed_numports)
> +		return -EINVAL;

Maybe add dev_err() here for all these error cases. It will help
somebody debug why there switch fails to probe.

	 Andrew