| Message ID | 20190709232251.31746-5-agust@denx.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Joe Hershberger |
| Headers | show |
| Series | Extend mv88e61xx driver to support 88E6071 | expand |
On Tue, Jul 9, 2019 at 6:26 PM Anatolij Gustschin <agust@denx.de> wrote: > > MV88E61XX_88E6020_FAMILY option enables support for switch > devices 6250/6220/6020/6070/6071. > > Signed-off-by: Anatolij Gustschin <agust@denx.de> > --- > drivers/net/phy/Kconfig | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > index 2a3da068c9..097b499ba3 100644 > --- a/drivers/net/phy/Kconfig > +++ b/drivers/net/phy/Kconfig > @@ -50,6 +50,13 @@ config MV88E61XX_SWITCH > > if MV88E61XX_SWITCH > > +config MV88E61XX_88E6020_FAMILY > + bool "Marvell MV88E6020 family support." > + help > + The driver supports 6172/6176/6240/6352 devices in the > + default configuration. Select this option to enable support > + for 6250/6220/6020/6070/6071 switches. Is there a rhyme or reason to the model numbers here? This option seems oddly named, especially since the help doesn't have the model numbers in numerical order. Can you make it a choice instead? E.g... choice prompt "MV88E61XX device selection" default MV88E61XX_88E6172 config MV88E61XX_88E6020 bool "MV88E6020" [ ... ] config MV88E61XX_88E6172 bool "MV88E6172" [ ... ] config MV88E61XX_88E6352 bool "MV88E6352" help The driver supports 6172/6176/6240/6352 devices in the default configuration. Select the device to enable support for (e.g. 6020/6070/6071/6220/6250). endchoice > + > config MV88E61XX_CPU_PORT > int "CPU Port" > > -- > 2.17.1 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot
On Tue, 23 Jul 2019 04:26:17 +0000 Joe Hershberger joe.hershberger@ni.com wrote: ... > > +config MV88E61XX_88E6020_FAMILY > > + bool "Marvell MV88E6020 family support." > > + help > > + The driver supports 6172/6176/6240/6352 devices in the > > + default configuration. Select this option to enable support > > + for 6250/6220/6020/6070/6071 switches. > > Is there a rhyme or reason to the model numbers here? This option > seems oddly named, especially since the help doesn't have the model > numbers in numerical order. Can you make it a choice instead? We want to be able to use single U-Boot images which support different switches, a choice will make it not possible. -- Anatolij
On Thu, Jul 25, 2019 at 4:42 PM Anatolij Gustschin <agust@denx.de> wrote: > > On Tue, 23 Jul 2019 04:26:17 +0000 > Joe Hershberger joe.hershberger@ni.com wrote: > ... > > > +config MV88E61XX_88E6020_FAMILY > > > + bool "Marvell MV88E6020 family support." > > > + help > > > + The driver supports 6172/6176/6240/6352 devices in the > > > + default configuration. Select this option to enable support > > > + for 6250/6220/6020/6070/6071 switches. > > > > Is there a rhyme or reason to the model numbers here? This option > > seems oddly named, especially since the help doesn't have the model > > numbers in numerical order. Can you make it a choice instead? > > We want to be able to use single U-Boot images which support different > switches, a choice will make it not possible. I don't see how choice is any different than what you have here in that respect. You have a hard-coded selection among families. If you have a need to run the same binary on multiple switches, then this option should be removed and the pre-init should be detecting the target. -Joe > > -- > Anatolij > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot
On Mon, Jul 29, 2019 at 6:17 PM Joe Hershberger <joe.hershberger@ni.com> wrote: > > On Thu, Jul 25, 2019 at 4:42 PM Anatolij Gustschin <agust@denx.de> wrote: > > > > On Tue, 23 Jul 2019 04:26:17 +0000 > > Joe Hershberger joe.hershberger@ni.com wrote: > > ... > > > > +config MV88E61XX_88E6020_FAMILY > > > > + bool "Marvell MV88E6020 family support." > > > > + help > > > > + The driver supports 6172/6176/6240/6352 devices in the > > > > + default configuration. Select this option to enable support > > > > + for 6250/6220/6020/6070/6071 switches. > > > > > > Is there a rhyme or reason to the model numbers here? This option > > > seems oddly named, especially since the help doesn't have the model > > > numbers in numerical order. Can you make it a choice instead? > > > > We want to be able to use single U-Boot images which support different > > switches, a choice will make it not possible. > > I don't see how choice is any different than what you have here in > that respect. You have a hard-coded selection among families. If you > have a need to run the same binary on multiple switches, then this > option should be removed and the pre-init should be detecting the > target. Any response here? > > -Joe > > > > > -- > > Anatolij > > _______________________________________________ > > U-Boot mailing list > > U-Boot@lists.denx.de > > https://lists.denx.de/listinfo/u-boot
Hi Joe, On Tue, 3 Sep 2019 22:00:08 +0000 Joe Hershberger joe.hershberger@ni.com wrote: > On Mon, Jul 29, 2019 at 6:17 PM Joe Hershberger <joe.hershberger@ni.com> wrote: > > > > On Thu, Jul 25, 2019 at 4:42 PM Anatolij Gustschin <agust@denx.de> wrote: > > > > > > On Tue, 23 Jul 2019 04:26:17 +0000 > > > Joe Hershberger joe.hershberger@ni.com wrote: > > > ... > > > > > +config MV88E61XX_88E6020_FAMILY > > > > > + bool "Marvell MV88E6020 family support." > > > > > + help > > > > > + The driver supports 6172/6176/6240/6352 devices in the > > > > > + default configuration. Select this option to enable support > > > > > + for 6250/6220/6020/6070/6071 switches. > > > > > > > > Is there a rhyme or reason to the model numbers here? This option > > > > seems oddly named, especially since the help doesn't have the model > > > > numbers in numerical order. Can you make it a choice instead? > > > > > > We want to be able to use single U-Boot images which support different > > > switches, a choice will make it not possible. > > > > I don't see how choice is any different than what you have here in > > that respect. You have a hard-coded selection among families. If you > > have a need to run the same binary on multiple switches, then this > > option should be removed and the pre-init should be detecting the > > target. > > Any response here? Sorry for delay, I've been too busy with other stuff. v4 patch series was reworked to drop this config option. Thanks! -- Anatolij
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index 2a3da068c9..097b499ba3 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -50,6 +50,13 @@ config MV88E61XX_SWITCH if MV88E61XX_SWITCH +config MV88E61XX_88E6020_FAMILY + bool "Marvell MV88E6020 family support." + help + The driver supports 6172/6176/6240/6352 devices in the + default configuration. Select this option to enable support + for 6250/6220/6020/6070/6071 switches. + config MV88E61XX_CPU_PORT int "CPU Port"
MV88E61XX_88E6020_FAMILY option enables support for switch devices 6250/6220/6020/6070/6071. Signed-off-by: Anatolij Gustschin <agust@denx.de> --- drivers/net/phy/Kconfig | 7 +++++++ 1 file changed, 7 insertions(+)