Hi--
On 11/3/21 4:02 AM, Wells Lu wrote:
> diff --git a/drivers/net/ethernet/sunplus/Kconfig b/drivers/net/ethernet/sunplus/Kconfig
> new file mode 100644
> index 0000000..a9e3a4c
> --- /dev/null
> +++ b/drivers/net/ethernet/sunplus/Kconfig
> @@ -0,0 +1,20 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Sunplus Ethernet device configuration
> +#
> +
> +config NET_VENDOR_SUNPLUS
> + tristate "Sunplus Dual 10M/100M Ethernet (with L2 switch) devices"
> + depends on ETHERNET && SOC_SP7021
> + select PHYLIB
> + select PINCTRL_SPPCTL
> + select COMMON_CLK_SP7021
> + select RESET_SUNPLUS
> + select NVMEM_SUNPLUS_OCOTP
> + help
> + If you have Sunplus dual 10M/100M Ethernet (with L2 switch)
> + devices, say Y.
> + The network device supports dual 10M/100M Ethernet interfaces,
> + or one 10/100M Ethernet interface with two LAN ports.
> + To compile this driver as a module, choose M here. The module
> + will be called sp_l2sw.
Please use NET_VENDOR_SUNPLUS in the same way that other
NET_VENDOR_wyxz kconfig symbols are used. It should just enable
or disable any specific device drivers under it.
--
~Randy
>
> Hi--
>
> On 11/3/21 4:02 AM, Wells Lu wrote:
> > diff --git a/drivers/net/ethernet/sunplus/Kconfig
> > b/drivers/net/ethernet/sunplus/Kconfig
> > new file mode 100644
> > index 0000000..a9e3a4c
> > --- /dev/null
> > +++ b/drivers/net/ethernet/sunplus/Kconfig
> > @@ -0,0 +1,20 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +#
> > +# Sunplus Ethernet device configuration #
> > +
> > +config NET_VENDOR_SUNPLUS
> > + tristate "Sunplus Dual 10M/100M Ethernet (with L2 switch) devices"
> > + depends on ETHERNET && SOC_SP7021
> > + select PHYLIB
> > + select PINCTRL_SPPCTL
> > + select COMMON_CLK_SP7021
> > + select RESET_SUNPLUS
> > + select NVMEM_SUNPLUS_OCOTP
> > + help
> > + If you have Sunplus dual 10M/100M Ethernet (with L2 switch)
> > + devices, say Y.
> > + The network device supports dual 10M/100M Ethernet interfaces,
> > + or one 10/100M Ethernet interface with two LAN ports.
> > + To compile this driver as a module, choose M here. The module
> > + will be called sp_l2sw.
>
> Please use NET_VENDOR_SUNPLUS in the same way that other
> NET_VENDOR_wyxz kconfig symbols are used. It should just enable or
> disable any specific device drivers under it.
>
>
> --
> ~Randy
I looked up Kconfig file of other vendors, but not sure what I should do.
Do I need to modify Kconfig file in the form as shown below?
# SPDX-License-Identifier: GPL-2.0
#
# Sunplus device configuration
#
config NET_VENDOR_SUNPLUS
bool "Sunplus devices"
default y
depends on ARCH_SUNPLUS
---help---
If you have a network (Ethernet) card belonging to this
class, say Y here.
Note that the answer to this question doesn't directly
affect the kernel: saying N will just cause the configurator
to skip all the questions about Sunplus cards. If you say Y,
you will be asked for your specific card in the following
questions.
if NET_VENDOR_SUNPLUS
config SP7021_EMAC
tristate "Sunplus Dual 10M/100M Ethernet (with L2 switch) devices"
depends on ETHERNET && SOC_SP7021
select PHYLIB
select PINCTRL_SPPCTL
select COMMON_CLK_SP7021
select RESET_SUNPLUS
select NVMEM_SUNPLUS_OCOTP
help
If you have Sunplus dual 10M/100M Ethernet (with L2 switch)
devices, say Y.
The network device supports dual 10M/100M Ethernet interfaces,
or one 10/100M Ethernet interface with two LAN ports.
To compile this driver as a module, choose M here. The module
will be called sp_l2sw.
endif # NET_VENDOR_SUNPLUS
Best regards,
Wells
> config NET_VENDOR_SUNPLUS
> bool "Sunplus devices"
> default y
> depends on ARCH_SUNPLUS
Does it actually depend on ARCH_SUNPLUS? What do you make use of?
Ideally, you want it to also build with COMPILE_TEST, so that the
driver gets build by 0-day and all the other build bots.
> ---help---
> If you have a network (Ethernet) card belonging to this
> class, say Y here.
>
> Note that the answer to this question doesn't directly
> affect the kernel: saying N will just cause the configurator
> to skip all the questions about Sunplus cards. If you say Y,
> you will be asked for your specific card in the following
> questions.
>
> if NET_VENDOR_SUNPLUS
>
> config SP7021_EMAC
> tristate "Sunplus Dual 10M/100M Ethernet (with L2 switch) devices"
> depends on ETHERNET && SOC_SP7021
Does it actually depend on SOC_SP7021 to build?
Andrew
Hi,
Thanks a lot for review.
>
> > config NET_VENDOR_SUNPLUS
> > bool "Sunplus devices"
> > default y
> > depends on ARCH_SUNPLUS
>
> Does it actually depend on ARCH_SUNPLUS? What do you make use of?
ARCH_SUNPLUS will be defined for Sunplus family series SoC.
Ethernet devices of Sunplus are designed and used for Sunplus SoC.
So far, only two SoC of Sunplus have the network device.
I'd like to show up the selection only for Sunplus SoC.
>
> Ideally, you want it to also build with COMPILE_TEST, so that the driver gets
> build by 0-day and all the other build bots.
I am not sure if this is mandatory or not.
Should I add COMPILE_TEST as below?
depends on ARCH_SUNPLUS | COMPILE_TEST
>
> > ---help---
> > If you have a network (Ethernet) card belonging to this
> > class, say Y here.
> >
> > Note that the answer to this question doesn't directly
> > affect the kernel: saying N will just cause the configurator
> > to skip all the questions about Sunplus cards. If you say Y,
> > you will be asked for your specific card in the following
> > questions.
> >
> > if NET_VENDOR_SUNPLUS
> >
> > config SP7021_EMAC
> > tristate "Sunplus Dual 10M/100M Ethernet (with L2 switch) devices"
> > depends on ETHERNET && SOC_SP7021
>
> Does it actually depend on SOC_SP7021 to build?
>
> Andrew
Yes, the device is now only for Sunplus SP7021 SoC.
Devices in each SoC may have a bit difference because of adding new
function or improving something.
On Thu, Nov 04, 2021 at 05:31:57AM +0000, Wells Lu 呂芳騰 wrote:
> Hi,
>
> Thanks a lot for review.
>
> >
> > > config NET_VENDOR_SUNPLUS
> > > bool "Sunplus devices"
> > > default y
> > > depends on ARCH_SUNPLUS
> >
> > Does it actually depend on ARCH_SUNPLUS? What do you make use of?
>
> ARCH_SUNPLUS will be defined for Sunplus family series SoC.
> Ethernet devices of Sunplus are designed and used for Sunplus SoC.
> So far, only two SoC of Sunplus have the network device.
> I'd like to show up the selection only for Sunplus SoC.
So it does not actually depend on ARCH_SUNPLUS. There are a few cases
where drivers have needed to call into arch specific code, which stops
them building for any other arch.
> > Ideally, you want it to also build with COMPILE_TEST, so that the driver gets
> > build by 0-day and all the other build bots.
>
> I am not sure if this is mandatory or not.
> Should I add COMPILE_TEST as below?
>
> depends on ARCH_SUNPLUS | COMPILE_TEST
Yes.
> Yes, the device is now only for Sunplus SP7021 SoC.
> Devices in each SoC may have a bit difference because of adding new
> function or improving something.
If it will compile with COMPILE_TEST on x86, mips, etc, you should
allow it to compile with COMPILE_TEST. You get better compile testing
that way.
Andrew
> On Thu, Nov 04, 2021 at 05:31:57AM +0000, Wells Lu 呂芳騰 wrote:
> > Hi,
> >
> > Thanks a lot for review.
> >
> > >
> > > > config NET_VENDOR_SUNPLUS
> > > > bool "Sunplus devices"
> > > > default y
> > > > depends on ARCH_SUNPLUS
> > >
> > > Does it actually depend on ARCH_SUNPLUS? What do you make use of?
> >
> > ARCH_SUNPLUS will be defined for Sunplus family series SoC.
> > Ethernet devices of Sunplus are designed and used for Sunplus SoC.
> > So far, only two SoC of Sunplus have the network device.
> > I'd like to show up the selection only for Sunplus SoC.
>
> So it does not actually depend on ARCH_SUNPLUS. There are a few cases where
> drivers have needed to call into arch specific code, which stops them building
> for any other arch.
>
> > > Ideally, you want it to also build with COMPILE_TEST, so that the
> > > driver gets build by 0-day and all the other build bots.
> >
> > I am not sure if this is mandatory or not.
> > Should I add COMPILE_TEST as below?
> >
> > depends on ARCH_SUNPLUS | COMPILE_TEST
>
> Yes.
>
> > Yes, the device is now only for Sunplus SP7021 SoC.
> > Devices in each SoC may have a bit difference because of adding new
> > function or improving something.
>
> If it will compile with COMPILE_TEST on x86, mips, etc, you should allow it to
> compile with COMPILE_TEST. You get better compile testing that way.
>
> Andrew
No, we only develop arm-based SoC, never for x86 or mips.
We never compile the driver for x86 or mips machine.
> No, we only develop arm-based SoC, never for x86 or mips.
> We never compile the driver for x86 or mips machine.
You don't, but the Linux community does build for those
architectures. Most people do tree wide refactoring work using
x86. Tree wide cleanups using x86, etc. Any changes like that could
touch your driver. The harder is it to build, the less build testing
it will get, and tree wide changes which break it are less likely to
get noticed. So you really do want it to compile cleanly for all
architectures. If it does not, it normally actually means you are
doing something wrong, something you need to fix anyway. So please do
build it for x86 and make sure it builds cleanly.
Andrew
> > No, we only develop arm-based SoC, never for x86 or mips.
> > We never compile the driver for x86 or mips machine.
>
> You don't, but the Linux community does build for those architectures. Most
> people do tree wide refactoring work using x86. Tree wide cleanups using x86,
> etc. Any changes like that could touch your driver. The harder is it to build, the
> less build testing it will get, and tree wide changes which break it are less likely
> to get noticed. So you really do want it to compile cleanly for all
> architectures. If it does not, it normally actually means you are doing
> something wrong, something you need to fix anyway. So please do build it for
> x86 and make sure it builds cleanly.
>
> Andrew
Ok, I understand.
I'll add COMPILE_TEST and compile driver for x86.
Thanks,