Re: [PATCH 2/4] net: stmmac: Add Toshiba Visconti SoCs glue driver

Hi,

Thanks for your review.

On Mon, Feb 15, 2021 at 08:07:21AM +0200, Leon Romanovsky wrote:

On Mon, Feb 15, 2021 at 02:06:53PM +0900, Nobuhiro Iwamatsu wrote:

quoted

Add dwmac-visconti to the stmmac driver in Toshiba Visconti ARM SoCs.
This patch contains only the basic function of the device. There is no
clock control, PM, etc. yet. These will be added in the future.

Signed-off-by: Nobuhiro Iwamatsu <redacted>
---
 drivers/net/ethernet/stmicro/stmmac/Kconfig   |   8 +
 drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
 .../ethernet/stmicro/stmmac/dwmac-visconti.c  | 285 ++++++++++++++++++
 3 files changed, 294 insertions(+)
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-visconti.c

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 53f14c5a9e02..55ba67a550b9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig

@@ -219,6 +219,14 @@ config DWMAC_INTEL_PLAT
 	  This selects the Intel platform specific glue layer support for
 	  the stmmac device driver. This driver is used for the Intel Keem Bay
 	  SoC.
+
+config DWMAC_VISCONTI
+	bool "Toshiba Visconti DWMAC support"
+	def_bool y

Sorry, I sent the wrong patchset that didn't fix this point out.

I asked it before, but never received an answer.

I have received your point out and have sent an email with the content
to remove this line. But it may not have arrived yet...

Why did you use "def_bool y" and not "default y"? Isn't it supposed to be
"depends on STMMAC_ETH"? And probably it shouldn't be set as a default as "y".

The reason why "def_bool y" was set is that the wrong fix was left when
debugging. Also, I don't think it is necessary to set "default y".
This is also incorrect because it says "bool" Toshiba Visconti DWMAC
support "". I change it to trustate in the new patch.

And this driver is enabled when STMMAC_PLATFORM was Y. And STMMAC_PLATFORM
depends on STMMAC_ETH.
So I understand that STMMAC_ETH does not need to be dependents. Is this
understanding wrong?

Thanks

Best regards,
  Nobuhiro

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel