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
From: Leon Romanovsky <leon@kernel.org> Date: 2021-02-15 06:08:23
On Mon, Feb 15, 2021 at 02:06:53PM +0900, Nobuhiro Iwamatsu wrote:
quoted hunk
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
I asked it before, but never received an answer.
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".
Thanks
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
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?
From: Leon Romanovsky <leon@kernel.org> Date: 2021-02-15 09:23:43
On Mon, Feb 15, 2021 at 04:28:09PM +0900, Nobuhiro Iwamatsu wrote:
Hi,
Thanks for your review.
On Mon, Feb 15, 2021 at 08:07:21AM +0200, Leon Romanovsky wrote:
quoted
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
Sorry, I sent the wrong patchset that didn't fix this point out.
quoted
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...
quoted
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?
This is correct understanding, just need to clean other entries in that
Kconfig that depends on STMMAC_ETH.
Thanks
On Mon, Feb 15, 2021 at 10:23 AM Leon Romanovsky [off-list ref] wrote:
On Mon, Feb 15, 2021 at 04:28:09PM +0900, Nobuhiro Iwamatsu wrote:
quoted
Sorry, I sent the wrong patchset that didn't fix this point out.
quoted
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...
quoted
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?
This is correct understanding, just need to clean other entries in that
Kconfig that depends on STMMAC_ETH.
'tristate' with no default sounds right. I see that some platforms have a
default according to the platform, which also makes sense but isn't
required. What I would suggest though is a dependency on the platform,
to make it easier to disable the front-end based on which platforms
are enabled. This would end up as
config DWMAC_VISCONTI
tristate "Toshiba Visconti DWMAC support"
depends on ARCH_VISCONTI || COMPILE_TEST
depends on OF && COMMON_CLK # only add this line if it's
required for compilation
default ARCH_VISCONTI
Arnd
Hi,
On Mon, Feb 15, 2021 at 11:22:33AM +0200, Leon Romanovsky wrote:
On Mon, Feb 15, 2021 at 04:28:09PM +0900, Nobuhiro Iwamatsu wrote:
quoted
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...
quoted
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?
This is correct understanding, just need to clean other entries in that
Kconfig that depends on STMMAC_ETH.
Hi,
On Mon, Feb 15, 2021 at 01:19:18PM +0100, Arnd Bergmann wrote:
On Mon, Feb 15, 2021 at 10:23 AM Leon Romanovsky [off-list ref] wrote:
quoted
On Mon, Feb 15, 2021 at 04:28:09PM +0900, Nobuhiro Iwamatsu wrote:
quoted
Sorry, I sent the wrong patchset that didn't fix this point out.
quoted
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...
quoted
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?
This is correct understanding, just need to clean other entries in that
Kconfig that depends on STMMAC_ETH.
'tristate' with no default sounds right. I see that some platforms have a
default according to the platform, which also makes sense but isn't
required. What I would suggest though is a dependency on the platform,
to make it easier to disable the front-end based on which platforms
are enabled. This would end up as
config DWMAC_VISCONTI
tristate "Toshiba Visconti DWMAC support"
depends on ARCH_VISCONTI || COMPILE_TEST
depends on OF && COMMON_CLK # only add this line if it's
required for compilation
default ARCH_VISCONTI
The fix at hand is the same as your suggestion.
Thank you for your comment.