Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <a0353e85-8604-a268-5776-2f28b092e57b@linaro.org>
Date:   Fri, 21 Oct 2022 09:38:23 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.0
Subject: Re: [PATCH net-next v5 2/5] net: ipqess: introduce the Qualcomm
 IPQESS driver
Content-Language: en-US
To:     Maxime Chevallier <maxime.chevallier@bootlin.com>,
        davem@davemloft.net, Rob Herring <robh+dt@kernel.org>
Cc:     netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        devicetree@vger.kernel.org, thomas.petazzoni@bootlin.com,
        Andrew Lunn <andrew@lunn.ch>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        linux-arm-kernel@lists.infradead.org,
        Vladimir Oltean <vladimir.oltean@nxp.com>,
        Luka Perkov <luka.perkov@sartura.hr>,
        Robert Marko <robert.marko@sartura.hr>
References: <20221021124556.100445-1-maxime.chevallier@bootlin.com>
 <20221021124556.100445-3-maxime.chevallier@bootlin.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <20221021124556.100445-3-maxime.chevallier@bootlin.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 21/10/2022 08:45, Maxime Chevallier wrote:
> The Qualcomm IPQESS controller is a simple 1G Ethernet controller found
> on the IPQ4019 chip. This controller has some specificities, in that the
> IPQ4019 platform that includes that controller also has an internal
> switch, based on the QCA8K IP.

Thank you for your patch. There is something to discuss/improve.

> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5c6ce094e55e..46fdd3c523c5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17037,6 +17037,12 @@ L:	netdev@vger.kernel.org
>  S:	Maintained
>  F:	drivers/net/ethernet/qualcomm/emac/
>  
> +QUALCOMM IPQESS ETHERNET DRIVER
> +M:	Maxime Chevallier <maxime.chevallier@bootlin.com>
> +L:	netdev@vger.kernel.org
> +S:	Maintained
> +F:	drivers/net/ethernet/qualcomm/ipqess/

Add also bindings.

> +
>  QUALCOMM ETHQOS ETHERNET DRIVER
>  M:	Vinod Koul <vkoul@kernel.org>
>  R:	Bhupesh Sharma <bhupesh.sharma@linaro.org>
> diff --git a/drivers/net/ethernet/qualcomm/Kconfig b/drivers/net/ethernet/qualcomm/Kconfig
> index a4434eb38950..a723ddbea248 100644
> --- a/drivers/net/ethernet/qualcomm/Kconfig
> +++ b/drivers/net/ethernet/qualcomm/Kconfig
> @@ -60,6 +60,17 @@ config QCOM_EMAC
>  	  low power, Receive-Side Scaling (RSS), and IEEE 1588-2008
>  	  Precision Clock Synchronization Protocol.
>  
> +config QCOM_IPQ4019_ESS_EDMA
> +	tristate "Qualcomm Atheros IPQ4019 ESS EDMA support"
> +	depends on OF

I think this is present only on systems where Qualcomm IPQ4019 SoCs is
the main SoC (AP)? If so, I propose not to offer this to non-Qualcomm
SoC builds, because they cannot use it and it makes life of
distro-vendors configurators difficult:

	depends on ARCH_QCOM || COMPILE_TEST

> +	select PHYLINK
> +	help
> +	  This driver supports the Qualcomm Atheros IPQ40xx built-in
> +	  ESS EDMA ethernet controller.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ipqess.
> +
>  source "drivers/net/ethernet/qualcomm/rmnet/Kconfig"


Best regards,
Krzysztof