| Message ID | 20200608092719.10643-1-patrick.delaunay@st.com |
|---|---|
| State | Accepted |
| Commit | a08f2f7b944f6926843b26a216db58b8d02a19e1 |
| Headers | show |
| Series | net: dwc_eth_qos: add Kconfig option to select supported configuration | expand |
On Mon, Jun 08, 2020 at 11:27:19AM +0200, Patrick Delaunay wrote: > Add configuration flag to select the supported dwc driver configuration: > - CONFIG_DWC_ETH_QOS_TEGRA186 > - CONFIG_DWC_ETH_QOS_IMX > - CONFIG_DWC_ETH_QOS_STM32 > > See Linux driver ethernet/stmicro/stmmac and associated glue layers > for other configuration examples. > > This patch removes the not-selected compatibles and lets the linker remove > the unused functions to reduce the size of the driver. > > Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com> > --- > Hi, > > I propose this patch after Marek's remark in [1]. > > It is also linked to [2] to limit the STM32 glue for ST compatible. > > [1] net: dwc_eth_qos: Make clk_rx and clk_tx optional > http://patchwork.ozlabs.org/project/uboot/patch/20200512095603.29126-5-david.wu at rock-chips.com/#2432595 > > [2] net: dwc_eth_qos: update the compatible supported for STM32 > http://patchwork.ozlabs.org/project/uboot/list/?series=176917 > > > drivers/net/Kconfig | 27 ++++++++++++++++++++++++--- > drivers/net/dwc_eth_qos.c | 12 +++++++++--- > 2 files changed, 33 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig > index 0b08de0ef4..c4b3667d3b 100644 > --- a/drivers/net/Kconfig > +++ b/drivers/net/Kconfig > @@ -156,9 +156,30 @@ config DWC_ETH_QOS > help > This driver supports the Synopsys Designware Ethernet QOS (Quality > Of Service) IP block. The IP supports many options for bus type, > - clocking/reset structure, and feature list. This driver currently > - supports the specific configuration used in NVIDIA's Tegra186 chip, > - but should be extensible to other combinations quite easily. > + clocking/reset structure, and feature list. > + > +config DWC_ETH_QOS_IMX > + bool "Synopsys DWC Ethernet QOS device support for IMX" > + depends on DWC_ETH_QOS > + help > + The Synopsys Designware Ethernet QOS IP block with the specific > + configuration used in IMX soc. > + > +config DWC_ETH_QOS_STM32 > + bool "Synopsys DWC Ethernet QOS device support for STM32" > + depends on DWC_ETH_QOS > + default y if ARCH_STM32MP > + help > + The Synopsys Designware Ethernet QOS IP block with the specific > + configuration used in STM32MP soc. > + > +config DWC_ETH_QOS_TEGRA186 > + bool "Synopsys DWC Ethernet QOS device support for TEGRA186" > + depends on DWC_ETH_QOS > + default y if TEGRA186 > + help > + The Synopsys Designware Ethernet QOS IP block with specific > + configuration used in NVIDIA's Tegra186 chip. > > config E1000 > bool "Intel PRO/1000 Gigabit Ethernet support" I like this idea. But, is there a reason to have more than one of these enabled? Assuming not, we should do this as a choice I think so it'll be clear to the next SoC that they'll need to add this table right away. Thanks!
On 6/10/20 8:10 PM, Tom Rini wrote: > On Mon, Jun 08, 2020 at 11:27:19AM +0200, Patrick Delaunay wrote: > >> Add configuration flag to select the supported dwc driver configuration: >> - CONFIG_DWC_ETH_QOS_TEGRA186 >> - CONFIG_DWC_ETH_QOS_IMX >> - CONFIG_DWC_ETH_QOS_STM32 >> >> See Linux driver ethernet/stmicro/stmmac and associated glue layers >> for other configuration examples. >> >> This patch removes the not-selected compatibles and lets the linker remove >> the unused functions to reduce the size of the driver. >> >> Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com> >> --- >> Hi, >> >> I propose this patch after Marek's remark in [1]. >> >> It is also linked to [2] to limit the STM32 glue for ST compatible. >> >> [1] net: dwc_eth_qos: Make clk_rx and clk_tx optional >> http://patchwork.ozlabs.org/project/uboot/patch/20200512095603.29126-5-david.wu at rock-chips.com/#2432595 >> >> [2] net: dwc_eth_qos: update the compatible supported for STM32 >> http://patchwork.ozlabs.org/project/uboot/list/?series=176917 >> >> >> drivers/net/Kconfig | 27 ++++++++++++++++++++++++--- >> drivers/net/dwc_eth_qos.c | 12 +++++++++--- >> 2 files changed, 33 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig >> index 0b08de0ef4..c4b3667d3b 100644 >> --- a/drivers/net/Kconfig >> +++ b/drivers/net/Kconfig >> @@ -156,9 +156,30 @@ config DWC_ETH_QOS >> help >> This driver supports the Synopsys Designware Ethernet QOS (Quality >> Of Service) IP block. The IP supports many options for bus type, >> - clocking/reset structure, and feature list. This driver currently >> - supports the specific configuration used in NVIDIA's Tegra186 chip, >> - but should be extensible to other combinations quite easily. >> + clocking/reset structure, and feature list. >> + >> +config DWC_ETH_QOS_IMX >> + bool "Synopsys DWC Ethernet QOS device support for IMX" >> + depends on DWC_ETH_QOS >> + help >> + The Synopsys Designware Ethernet QOS IP block with the specific >> + configuration used in IMX soc. >> + >> +config DWC_ETH_QOS_STM32 >> + bool "Synopsys DWC Ethernet QOS device support for STM32" >> + depends on DWC_ETH_QOS >> + default y if ARCH_STM32MP >> + help >> + The Synopsys Designware Ethernet QOS IP block with the specific >> + configuration used in STM32MP soc. >> + >> +config DWC_ETH_QOS_TEGRA186 >> + bool "Synopsys DWC Ethernet QOS device support for TEGRA186" >> + depends on DWC_ETH_QOS >> + default y if TEGRA186 >> + help >> + The Synopsys Designware Ethernet QOS IP block with specific >> + configuration used in NVIDIA's Tegra186 chip. >> >> config E1000 >> bool "Intel PRO/1000 Gigabit Ethernet support" > > I like this idea. But, is there a reason to have more than one of these > enabled? Assuming not, we should do this as a choice I think so it'll > be clear to the next SoC that they'll need to add this table right away. Should we be able to enable different settings in U-Boot and in SPL , e.g. for the use case where the SPL is the same on different SoCs, while the U-Boot binary is not ?
On Wed, Jun 10, 2020 at 08:42:20PM +0200, Marek Vasut wrote: > On 6/10/20 8:10 PM, Tom Rini wrote: > > On Mon, Jun 08, 2020 at 11:27:19AM +0200, Patrick Delaunay wrote: > > > >> Add configuration flag to select the supported dwc driver configuration: > >> - CONFIG_DWC_ETH_QOS_TEGRA186 > >> - CONFIG_DWC_ETH_QOS_IMX > >> - CONFIG_DWC_ETH_QOS_STM32 > >> > >> See Linux driver ethernet/stmicro/stmmac and associated glue layers > >> for other configuration examples. > >> > >> This patch removes the not-selected compatibles and lets the linker remove > >> the unused functions to reduce the size of the driver. > >> > >> Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com> > >> --- > >> Hi, > >> > >> I propose this patch after Marek's remark in [1]. > >> > >> It is also linked to [2] to limit the STM32 glue for ST compatible. > >> > >> [1] net: dwc_eth_qos: Make clk_rx and clk_tx optional > >> http://patchwork.ozlabs.org/project/uboot/patch/20200512095603.29126-5-david.wu at rock-chips.com/#2432595 > >> > >> [2] net: dwc_eth_qos: update the compatible supported for STM32 > >> http://patchwork.ozlabs.org/project/uboot/list/?series=176917 > >> > >> > >> drivers/net/Kconfig | 27 ++++++++++++++++++++++++--- > >> drivers/net/dwc_eth_qos.c | 12 +++++++++--- > >> 2 files changed, 33 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig > >> index 0b08de0ef4..c4b3667d3b 100644 > >> --- a/drivers/net/Kconfig > >> +++ b/drivers/net/Kconfig > >> @@ -156,9 +156,30 @@ config DWC_ETH_QOS > >> help > >> This driver supports the Synopsys Designware Ethernet QOS (Quality > >> Of Service) IP block. The IP supports many options for bus type, > >> - clocking/reset structure, and feature list. This driver currently > >> - supports the specific configuration used in NVIDIA's Tegra186 chip, > >> - but should be extensible to other combinations quite easily. > >> + clocking/reset structure, and feature list. > >> + > >> +config DWC_ETH_QOS_IMX > >> + bool "Synopsys DWC Ethernet QOS device support for IMX" > >> + depends on DWC_ETH_QOS > >> + help > >> + The Synopsys Designware Ethernet QOS IP block with the specific > >> + configuration used in IMX soc. > >> + > >> +config DWC_ETH_QOS_STM32 > >> + bool "Synopsys DWC Ethernet QOS device support for STM32" > >> + depends on DWC_ETH_QOS > >> + default y if ARCH_STM32MP > >> + help > >> + The Synopsys Designware Ethernet QOS IP block with the specific > >> + configuration used in STM32MP soc. > >> + > >> +config DWC_ETH_QOS_TEGRA186 > >> + bool "Synopsys DWC Ethernet QOS device support for TEGRA186" > >> + depends on DWC_ETH_QOS > >> + default y if TEGRA186 > >> + help > >> + The Synopsys Designware Ethernet QOS IP block with specific > >> + configuration used in NVIDIA's Tegra186 chip. > >> > >> config E1000 > >> bool "Intel PRO/1000 Gigabit Ethernet support" > > > > I like this idea. But, is there a reason to have more than one of these > > enabled? Assuming not, we should do this as a choice I think so it'll > > be clear to the next SoC that they'll need to add this table right away. > > Should we be able to enable different settings in U-Boot and in SPL , > e.g. for the use case where the SPL is the same on different SoCs, while > the U-Boot binary is not ? You mean be more like barebox and Linux where the board-specific stuff is kicked up one level and we have a more generic binary? I don't know and there's so much work that would be required before having to move this from a Kconfig choice to just Kconfig options I don't see that as being a relevant question here. Or did I misunderstand the question?
On 6/10/20 8:52 PM, Tom Rini wrote: > On Wed, Jun 10, 2020 at 08:42:20PM +0200, Marek Vasut wrote: >> On 6/10/20 8:10 PM, Tom Rini wrote: >>> On Mon, Jun 08, 2020 at 11:27:19AM +0200, Patrick Delaunay wrote: >>> >>>> Add configuration flag to select the supported dwc driver configuration: >>>> - CONFIG_DWC_ETH_QOS_TEGRA186 >>>> - CONFIG_DWC_ETH_QOS_IMX >>>> - CONFIG_DWC_ETH_QOS_STM32 >>>> >>>> See Linux driver ethernet/stmicro/stmmac and associated glue layers >>>> for other configuration examples. >>>> >>>> This patch removes the not-selected compatibles and lets the linker remove >>>> the unused functions to reduce the size of the driver. >>>> >>>> Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com> >>>> --- >>>> Hi, >>>> >>>> I propose this patch after Marek's remark in [1]. >>>> >>>> It is also linked to [2] to limit the STM32 glue for ST compatible. >>>> >>>> [1] net: dwc_eth_qos: Make clk_rx and clk_tx optional >>>> http://patchwork.ozlabs.org/project/uboot/patch/20200512095603.29126-5-david.wu at rock-chips.com/#2432595 >>>> >>>> [2] net: dwc_eth_qos: update the compatible supported for STM32 >>>> http://patchwork.ozlabs.org/project/uboot/list/?series=176917 >>>> >>>> >>>> drivers/net/Kconfig | 27 ++++++++++++++++++++++++--- >>>> drivers/net/dwc_eth_qos.c | 12 +++++++++--- >>>> 2 files changed, 33 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig >>>> index 0b08de0ef4..c4b3667d3b 100644 >>>> --- a/drivers/net/Kconfig >>>> +++ b/drivers/net/Kconfig >>>> @@ -156,9 +156,30 @@ config DWC_ETH_QOS >>>> help >>>> This driver supports the Synopsys Designware Ethernet QOS (Quality >>>> Of Service) IP block. The IP supports many options for bus type, >>>> - clocking/reset structure, and feature list. This driver currently >>>> - supports the specific configuration used in NVIDIA's Tegra186 chip, >>>> - but should be extensible to other combinations quite easily. >>>> + clocking/reset structure, and feature list. >>>> + >>>> +config DWC_ETH_QOS_IMX >>>> + bool "Synopsys DWC Ethernet QOS device support for IMX" >>>> + depends on DWC_ETH_QOS >>>> + help >>>> + The Synopsys Designware Ethernet QOS IP block with the specific >>>> + configuration used in IMX soc. >>>> + >>>> +config DWC_ETH_QOS_STM32 >>>> + bool "Synopsys DWC Ethernet QOS device support for STM32" >>>> + depends on DWC_ETH_QOS >>>> + default y if ARCH_STM32MP >>>> + help >>>> + The Synopsys Designware Ethernet QOS IP block with the specific >>>> + configuration used in STM32MP soc. >>>> + >>>> +config DWC_ETH_QOS_TEGRA186 >>>> + bool "Synopsys DWC Ethernet QOS device support for TEGRA186" >>>> + depends on DWC_ETH_QOS >>>> + default y if TEGRA186 >>>> + help >>>> + The Synopsys Designware Ethernet QOS IP block with specific >>>> + configuration used in NVIDIA's Tegra186 chip. >>>> >>>> config E1000 >>>> bool "Intel PRO/1000 Gigabit Ethernet support" >>> >>> I like this idea. But, is there a reason to have more than one of these >>> enabled? Assuming not, we should do this as a choice I think so it'll >>> be clear to the next SoC that they'll need to add this table right away. >> >> Should we be able to enable different settings in U-Boot and in SPL , >> e.g. for the use case where the SPL is the same on different SoCs, while >> the U-Boot binary is not ? > > You mean be more like barebox and Linux where the board-specific stuff > is kicked up one level and we have a more generic binary? I don't know > and there's so much work that would be required before having to move > this from a Kconfig choice to just Kconfig options I don't see that as > being a relevant question here. > > Or did I misunderstand the question? More like automatically have a Kconfig option generate it's _SPL and _TPL variant.
On Wed, Jun 10, 2020 at 08:55:36PM +0200, Marek Vasut wrote: > On 6/10/20 8:52 PM, Tom Rini wrote: > > On Wed, Jun 10, 2020 at 08:42:20PM +0200, Marek Vasut wrote: > >> On 6/10/20 8:10 PM, Tom Rini wrote: > >>> On Mon, Jun 08, 2020 at 11:27:19AM +0200, Patrick Delaunay wrote: > >>> > >>>> Add configuration flag to select the supported dwc driver configuration: > >>>> - CONFIG_DWC_ETH_QOS_TEGRA186 > >>>> - CONFIG_DWC_ETH_QOS_IMX > >>>> - CONFIG_DWC_ETH_QOS_STM32 > >>>> > >>>> See Linux driver ethernet/stmicro/stmmac and associated glue layers > >>>> for other configuration examples. > >>>> > >>>> This patch removes the not-selected compatibles and lets the linker remove > >>>> the unused functions to reduce the size of the driver. > >>>> > >>>> Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com> > >>>> --- > >>>> Hi, > >>>> > >>>> I propose this patch after Marek's remark in [1]. > >>>> > >>>> It is also linked to [2] to limit the STM32 glue for ST compatible. > >>>> > >>>> [1] net: dwc_eth_qos: Make clk_rx and clk_tx optional > >>>> http://patchwork.ozlabs.org/project/uboot/patch/20200512095603.29126-5-david.wu at rock-chips.com/#2432595 > >>>> > >>>> [2] net: dwc_eth_qos: update the compatible supported for STM32 > >>>> http://patchwork.ozlabs.org/project/uboot/list/?series=176917 > >>>> > >>>> > >>>> drivers/net/Kconfig | 27 ++++++++++++++++++++++++--- > >>>> drivers/net/dwc_eth_qos.c | 12 +++++++++--- > >>>> 2 files changed, 33 insertions(+), 6 deletions(-) > >>>> > >>>> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig > >>>> index 0b08de0ef4..c4b3667d3b 100644 > >>>> --- a/drivers/net/Kconfig > >>>> +++ b/drivers/net/Kconfig > >>>> @@ -156,9 +156,30 @@ config DWC_ETH_QOS > >>>> help > >>>> This driver supports the Synopsys Designware Ethernet QOS (Quality > >>>> Of Service) IP block. The IP supports many options for bus type, > >>>> - clocking/reset structure, and feature list. This driver currently > >>>> - supports the specific configuration used in NVIDIA's Tegra186 chip, > >>>> - but should be extensible to other combinations quite easily. > >>>> + clocking/reset structure, and feature list. > >>>> + > >>>> +config DWC_ETH_QOS_IMX > >>>> + bool "Synopsys DWC Ethernet QOS device support for IMX" > >>>> + depends on DWC_ETH_QOS > >>>> + help > >>>> + The Synopsys Designware Ethernet QOS IP block with the specific > >>>> + configuration used in IMX soc. > >>>> + > >>>> +config DWC_ETH_QOS_STM32 > >>>> + bool "Synopsys DWC Ethernet QOS device support for STM32" > >>>> + depends on DWC_ETH_QOS > >>>> + default y if ARCH_STM32MP > >>>> + help > >>>> + The Synopsys Designware Ethernet QOS IP block with the specific > >>>> + configuration used in STM32MP soc. > >>>> + > >>>> +config DWC_ETH_QOS_TEGRA186 > >>>> + bool "Synopsys DWC Ethernet QOS device support for TEGRA186" > >>>> + depends on DWC_ETH_QOS > >>>> + default y if TEGRA186 > >>>> + help > >>>> + The Synopsys Designware Ethernet QOS IP block with specific > >>>> + configuration used in NVIDIA's Tegra186 chip. > >>>> > >>>> config E1000 > >>>> bool "Intel PRO/1000 Gigabit Ethernet support" > >>> > >>> I like this idea. But, is there a reason to have more than one of these > >>> enabled? Assuming not, we should do this as a choice I think so it'll > >>> be clear to the next SoC that they'll need to add this table right away. > >> > >> Should we be able to enable different settings in U-Boot and in SPL , > >> e.g. for the use case where the SPL is the same on different SoCs, while > >> the U-Boot binary is not ? > > > > You mean be more like barebox and Linux where the board-specific stuff > > is kicked up one level and we have a more generic binary? I don't know > > and there's so much work that would be required before having to move > > this from a Kconfig choice to just Kconfig options I don't see that as > > being a relevant question here. > > > > Or did I misunderstand the question? > > More like automatically have a Kconfig option generate it's _SPL and > _TPL variant. Ah. Well, that is rephrasing my first question. Would it ever make sense to have more than one of these enabled?
On 6/10/20 8:58 PM, Tom Rini wrote: > On Wed, Jun 10, 2020 at 08:55:36PM +0200, Marek Vasut wrote: >> On 6/10/20 8:52 PM, Tom Rini wrote: >>> On Wed, Jun 10, 2020 at 08:42:20PM +0200, Marek Vasut wrote: >>>> On 6/10/20 8:10 PM, Tom Rini wrote: >>>>> On Mon, Jun 08, 2020 at 11:27:19AM +0200, Patrick Delaunay wrote: >>>>> >>>>>> Add configuration flag to select the supported dwc driver configuration: >>>>>> - CONFIG_DWC_ETH_QOS_TEGRA186 >>>>>> - CONFIG_DWC_ETH_QOS_IMX >>>>>> - CONFIG_DWC_ETH_QOS_STM32 >>>>>> >>>>>> See Linux driver ethernet/stmicro/stmmac and associated glue layers >>>>>> for other configuration examples. >>>>>> >>>>>> This patch removes the not-selected compatibles and lets the linker remove >>>>>> the unused functions to reduce the size of the driver. >>>>>> >>>>>> Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com> >>>>>> --- >>>>>> Hi, >>>>>> >>>>>> I propose this patch after Marek's remark in [1]. >>>>>> >>>>>> It is also linked to [2] to limit the STM32 glue for ST compatible. >>>>>> >>>>>> [1] net: dwc_eth_qos: Make clk_rx and clk_tx optional >>>>>> http://patchwork.ozlabs.org/project/uboot/patch/20200512095603.29126-5-david.wu at rock-chips.com/#2432595 >>>>>> >>>>>> [2] net: dwc_eth_qos: update the compatible supported for STM32 >>>>>> http://patchwork.ozlabs.org/project/uboot/list/?series=176917 >>>>>> >>>>>> >>>>>> drivers/net/Kconfig | 27 ++++++++++++++++++++++++--- >>>>>> drivers/net/dwc_eth_qos.c | 12 +++++++++--- >>>>>> 2 files changed, 33 insertions(+), 6 deletions(-) >>>>>> >>>>>> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig >>>>>> index 0b08de0ef4..c4b3667d3b 100644 >>>>>> --- a/drivers/net/Kconfig >>>>>> +++ b/drivers/net/Kconfig >>>>>> @@ -156,9 +156,30 @@ config DWC_ETH_QOS >>>>>> help >>>>>> This driver supports the Synopsys Designware Ethernet QOS (Quality >>>>>> Of Service) IP block. The IP supports many options for bus type, >>>>>> - clocking/reset structure, and feature list. This driver currently >>>>>> - supports the specific configuration used in NVIDIA's Tegra186 chip, >>>>>> - but should be extensible to other combinations quite easily. >>>>>> + clocking/reset structure, and feature list. >>>>>> + >>>>>> +config DWC_ETH_QOS_IMX >>>>>> + bool "Synopsys DWC Ethernet QOS device support for IMX" >>>>>> + depends on DWC_ETH_QOS >>>>>> + help >>>>>> + The Synopsys Designware Ethernet QOS IP block with the specific >>>>>> + configuration used in IMX soc. >>>>>> + >>>>>> +config DWC_ETH_QOS_STM32 >>>>>> + bool "Synopsys DWC Ethernet QOS device support for STM32" >>>>>> + depends on DWC_ETH_QOS >>>>>> + default y if ARCH_STM32MP >>>>>> + help >>>>>> + The Synopsys Designware Ethernet QOS IP block with the specific >>>>>> + configuration used in STM32MP soc. >>>>>> + >>>>>> +config DWC_ETH_QOS_TEGRA186 >>>>>> + bool "Synopsys DWC Ethernet QOS device support for TEGRA186" >>>>>> + depends on DWC_ETH_QOS >>>>>> + default y if TEGRA186 >>>>>> + help >>>>>> + The Synopsys Designware Ethernet QOS IP block with specific >>>>>> + configuration used in NVIDIA's Tegra186 chip. >>>>>> >>>>>> config E1000 >>>>>> bool "Intel PRO/1000 Gigabit Ethernet support" >>>>> >>>>> I like this idea. But, is there a reason to have more than one of these >>>>> enabled? Assuming not, we should do this as a choice I think so it'll >>>>> be clear to the next SoC that they'll need to add this table right away. >>>> >>>> Should we be able to enable different settings in U-Boot and in SPL , >>>> e.g. for the use case where the SPL is the same on different SoCs, while >>>> the U-Boot binary is not ? >>> >>> You mean be more like barebox and Linux where the board-specific stuff >>> is kicked up one level and we have a more generic binary? I don't know >>> and there's so much work that would be required before having to move >>> this from a Kconfig choice to just Kconfig options I don't see that as >>> being a relevant question here. >>> >>> Or did I misunderstand the question? >> >> More like automatically have a Kconfig option generate it's _SPL and >> _TPL variant. > > Ah. Well, that is rephrasing my first question. Would it ever make > sense to have more than one of these enabled? For some sort of universal SPL, maybe ?
On Wed, Jun 10, 2020 at 09:01:39PM +0200, Marek Vasut wrote: > On 6/10/20 8:58 PM, Tom Rini wrote: > > On Wed, Jun 10, 2020 at 08:55:36PM +0200, Marek Vasut wrote: > >> On 6/10/20 8:52 PM, Tom Rini wrote: > >>> On Wed, Jun 10, 2020 at 08:42:20PM +0200, Marek Vasut wrote: > >>>> On 6/10/20 8:10 PM, Tom Rini wrote: > >>>>> On Mon, Jun 08, 2020 at 11:27:19AM +0200, Patrick Delaunay wrote: > >>>>> > >>>>>> Add configuration flag to select the supported dwc driver configuration: > >>>>>> - CONFIG_DWC_ETH_QOS_TEGRA186 > >>>>>> - CONFIG_DWC_ETH_QOS_IMX > >>>>>> - CONFIG_DWC_ETH_QOS_STM32 > >>>>>> > >>>>>> See Linux driver ethernet/stmicro/stmmac and associated glue layers > >>>>>> for other configuration examples. > >>>>>> > >>>>>> This patch removes the not-selected compatibles and lets the linker remove > >>>>>> the unused functions to reduce the size of the driver. > >>>>>> > >>>>>> Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com> > >>>>>> --- > >>>>>> Hi, > >>>>>> > >>>>>> I propose this patch after Marek's remark in [1]. > >>>>>> > >>>>>> It is also linked to [2] to limit the STM32 glue for ST compatible. > >>>>>> > >>>>>> [1] net: dwc_eth_qos: Make clk_rx and clk_tx optional > >>>>>> http://patchwork.ozlabs.org/project/uboot/patch/20200512095603.29126-5-david.wu at rock-chips.com/#2432595 > >>>>>> > >>>>>> [2] net: dwc_eth_qos: update the compatible supported for STM32 > >>>>>> http://patchwork.ozlabs.org/project/uboot/list/?series=176917 > >>>>>> > >>>>>> > >>>>>> drivers/net/Kconfig | 27 ++++++++++++++++++++++++--- > >>>>>> drivers/net/dwc_eth_qos.c | 12 +++++++++--- > >>>>>> 2 files changed, 33 insertions(+), 6 deletions(-) > >>>>>> > >>>>>> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig > >>>>>> index 0b08de0ef4..c4b3667d3b 100644 > >>>>>> --- a/drivers/net/Kconfig > >>>>>> +++ b/drivers/net/Kconfig > >>>>>> @@ -156,9 +156,30 @@ config DWC_ETH_QOS > >>>>>> help > >>>>>> This driver supports the Synopsys Designware Ethernet QOS (Quality > >>>>>> Of Service) IP block. The IP supports many options for bus type, > >>>>>> - clocking/reset structure, and feature list. This driver currently > >>>>>> - supports the specific configuration used in NVIDIA's Tegra186 chip, > >>>>>> - but should be extensible to other combinations quite easily. > >>>>>> + clocking/reset structure, and feature list. > >>>>>> + > >>>>>> +config DWC_ETH_QOS_IMX > >>>>>> + bool "Synopsys DWC Ethernet QOS device support for IMX" > >>>>>> + depends on DWC_ETH_QOS > >>>>>> + help > >>>>>> + The Synopsys Designware Ethernet QOS IP block with the specific > >>>>>> + configuration used in IMX soc. > >>>>>> + > >>>>>> +config DWC_ETH_QOS_STM32 > >>>>>> + bool "Synopsys DWC Ethernet QOS device support for STM32" > >>>>>> + depends on DWC_ETH_QOS > >>>>>> + default y if ARCH_STM32MP > >>>>>> + help > >>>>>> + The Synopsys Designware Ethernet QOS IP block with the specific > >>>>>> + configuration used in STM32MP soc. > >>>>>> + > >>>>>> +config DWC_ETH_QOS_TEGRA186 > >>>>>> + bool "Synopsys DWC Ethernet QOS device support for TEGRA186" > >>>>>> + depends on DWC_ETH_QOS > >>>>>> + default y if TEGRA186 > >>>>>> + help > >>>>>> + The Synopsys Designware Ethernet QOS IP block with specific > >>>>>> + configuration used in NVIDIA's Tegra186 chip. > >>>>>> > >>>>>> config E1000 > >>>>>> bool "Intel PRO/1000 Gigabit Ethernet support" > >>>>> > >>>>> I like this idea. But, is there a reason to have more than one of these > >>>>> enabled? Assuming not, we should do this as a choice I think so it'll > >>>>> be clear to the next SoC that they'll need to add this table right away. > >>>> > >>>> Should we be able to enable different settings in U-Boot and in SPL , > >>>> e.g. for the use case where the SPL is the same on different SoCs, while > >>>> the U-Boot binary is not ? > >>> > >>> You mean be more like barebox and Linux where the board-specific stuff > >>> is kicked up one level and we have a more generic binary? I don't know > >>> and there's so much work that would be required before having to move > >>> this from a Kconfig choice to just Kconfig options I don't see that as > >>> being a relevant question here. > >>> > >>> Or did I misunderstand the question? > >> > >> More like automatically have a Kconfig option generate it's _SPL and > >> _TPL variant. > > > > Ah. Well, that is rephrasing my first question. Would it ever make > > sense to have more than one of these enabled? > > For some sort of universal SPL, maybe ? So no, there's not a reason today then and it should be a choice. Thanks!
On 6/10/20 10:11 PM, Tom Rini wrote: [...] >>>>> You mean be more like barebox and Linux where the board-specific stuff >>>>> is kicked up one level and we have a more generic binary? I don't know >>>>> and there's so much work that would be required before having to move >>>>> this from a Kconfig choice to just Kconfig options I don't see that as >>>>> being a relevant question here. >>>>> >>>>> Or did I misunderstand the question? >>>> >>>> More like automatically have a Kconfig option generate it's _SPL and >>>> _TPL variant. >>> >>> Ah. Well, that is rephrasing my first question. Would it ever make >>> sense to have more than one of these enabled? >> >> For some sort of universal SPL, maybe ? > > So no, there's not a reason today then and it should be a choice. Can you provide some more detailed explanation why we shouldn't generate _SPL and _TPL variants of Kconfig entries for all Kconfig entries ? It would make things much simpler and permit configuring SPL/TPL the same way U-Boot is configured, with their own set of options.
On Wed, Jun 10, 2020 at 10:46:23PM +0200, Marek Vasut wrote: > On 6/10/20 10:11 PM, Tom Rini wrote: > [...] > >>>>> You mean be more like barebox and Linux where the board-specific stuff > >>>>> is kicked up one level and we have a more generic binary? I don't know > >>>>> and there's so much work that would be required before having to move > >>>>> this from a Kconfig choice to just Kconfig options I don't see that as > >>>>> being a relevant question here. > >>>>> > >>>>> Or did I misunderstand the question? > >>>> > >>>> More like automatically have a Kconfig option generate it's _SPL and > >>>> _TPL variant. > >>> > >>> Ah. Well, that is rephrasing my first question. Would it ever make > >>> sense to have more than one of these enabled? > >> > >> For some sort of universal SPL, maybe ? > > > > So no, there's not a reason today then and it should be a choice. > > Can you provide some more detailed explanation why we shouldn't generate > _SPL and _TPL variants of Kconfig entries for all Kconfig entries ? It > would make things much simpler and permit configuring SPL/TPL the same > way U-Boot is configured, with their own set of options. In the context of this particular thread? I don't see how it's helpful to say 3 times that we're in fact building for Tegra or STM32 or SoCFPGA when you can't build something that runs on more than one of those.
On 6/10/20 10:54 PM, Tom Rini wrote: > On Wed, Jun 10, 2020 at 10:46:23PM +0200, Marek Vasut wrote: >> On 6/10/20 10:11 PM, Tom Rini wrote: >> [...] >>>>>>> You mean be more like barebox and Linux where the board-specific stuff >>>>>>> is kicked up one level and we have a more generic binary? I don't know >>>>>>> and there's so much work that would be required before having to move >>>>>>> this from a Kconfig choice to just Kconfig options I don't see that as >>>>>>> being a relevant question here. >>>>>>> >>>>>>> Or did I misunderstand the question? >>>>>> >>>>>> More like automatically have a Kconfig option generate it's _SPL and >>>>>> _TPL variant. >>>>> >>>>> Ah. Well, that is rephrasing my first question. Would it ever make >>>>> sense to have more than one of these enabled? >>>> >>>> For some sort of universal SPL, maybe ? >>> >>> So no, there's not a reason today then and it should be a choice. >> >> Can you provide some more detailed explanation why we shouldn't generate >> _SPL and _TPL variants of Kconfig entries for all Kconfig entries ? It >> would make things much simpler and permit configuring SPL/TPL the same >> way U-Boot is configured, with their own set of options. > > In the context of this particular thread? I don't see how it's helpful > to say 3 times that we're in fact building for Tegra or STM32 or SoCFPGA > when you can't build something that runs on more than one of those. In general. Here I can imagine it is possible to build SoCFPGA+STM32 universal SD card image in fact.
On Wed, Jun 10, 2020 at 10:56:40PM +0200, Marek Vasut wrote: > On 6/10/20 10:54 PM, Tom Rini wrote: > > On Wed, Jun 10, 2020 at 10:46:23PM +0200, Marek Vasut wrote: > >> On 6/10/20 10:11 PM, Tom Rini wrote: > >> [...] > >>>>>>> You mean be more like barebox and Linux where the board-specific stuff > >>>>>>> is kicked up one level and we have a more generic binary? I don't know > >>>>>>> and there's so much work that would be required before having to move > >>>>>>> this from a Kconfig choice to just Kconfig options I don't see that as > >>>>>>> being a relevant question here. > >>>>>>> > >>>>>>> Or did I misunderstand the question? > >>>>>> > >>>>>> More like automatically have a Kconfig option generate it's _SPL and > >>>>>> _TPL variant. > >>>>> > >>>>> Ah. Well, that is rephrasing my first question. Would it ever make > >>>>> sense to have more than one of these enabled? > >>>> > >>>> For some sort of universal SPL, maybe ? > >>> > >>> So no, there's not a reason today then and it should be a choice. > >> > >> Can you provide some more detailed explanation why we shouldn't generate > >> _SPL and _TPL variants of Kconfig entries for all Kconfig entries ? It > >> would make things much simpler and permit configuring SPL/TPL the same > >> way U-Boot is configured, with their own set of options. > > > > In the context of this particular thread? I don't see how it's helpful > > to say 3 times that we're in fact building for Tegra or STM32 or SoCFPGA > > when you can't build something that runs on more than one of those. > > In general. > > Here I can imagine it is possible to build SoCFPGA+STM32 universal SD > card image in fact. So that's the case I brought up at first and you said no to. I don't see that in the foreseeable future but I don't feel so strongly about making this config area tidy enough to argue the point. So fine, leave it as separate options, the default y if ... is reasonable enough to ensure we get functional builds.
On 6/10/20 11:35 PM, Tom Rini wrote: > On Wed, Jun 10, 2020 at 10:56:40PM +0200, Marek Vasut wrote: >> On 6/10/20 10:54 PM, Tom Rini wrote: >>> On Wed, Jun 10, 2020 at 10:46:23PM +0200, Marek Vasut wrote: >>>> On 6/10/20 10:11 PM, Tom Rini wrote: >>>> [...] >>>>>>>>> You mean be more like barebox and Linux where the board-specific stuff >>>>>>>>> is kicked up one level and we have a more generic binary? I don't know >>>>>>>>> and there's so much work that would be required before having to move >>>>>>>>> this from a Kconfig choice to just Kconfig options I don't see that as >>>>>>>>> being a relevant question here. >>>>>>>>> >>>>>>>>> Or did I misunderstand the question? >>>>>>>> >>>>>>>> More like automatically have a Kconfig option generate it's _SPL and >>>>>>>> _TPL variant. >>>>>>> >>>>>>> Ah. Well, that is rephrasing my first question. Would it ever make >>>>>>> sense to have more than one of these enabled? >>>>>> >>>>>> For some sort of universal SPL, maybe ? >>>>> >>>>> So no, there's not a reason today then and it should be a choice. >>>> >>>> Can you provide some more detailed explanation why we shouldn't generate >>>> _SPL and _TPL variants of Kconfig entries for all Kconfig entries ? It >>>> would make things much simpler and permit configuring SPL/TPL the same >>>> way U-Boot is configured, with their own set of options. >>> >>> In the context of this particular thread? I don't see how it's helpful >>> to say 3 times that we're in fact building for Tegra or STM32 or SoCFPGA >>> when you can't build something that runs on more than one of those. >> >> In general. >> >> Here I can imagine it is possible to build SoCFPGA+STM32 universal SD >> card image in fact. > > So that's the case I brought up at first and you said no to. I think I don't understand this part. > I don't > see that in the foreseeable future but I don't feel so strongly about > making this config area tidy enough to argue the point. So fine, leave > it as separate options, the default y if ... is reasonable enough to > ensure we get functional builds. This patch is OK as-is. My point is more in the general direction of being able to configure SPL/TPL/U-Boot separately, without being forced to craft nasty ifdeffery in include/config/board.h if I need something enabled in SPL, but not in U-Boot, and vice versa. And for that the Kconfig should be able to somehow emit the _SPL/_TPL/U-Boot options of all symbols I think, so that we won't need separate entry for each.
On Wed, Jun 10, 2020 at 11:40:33PM +0200, Marek Vasut wrote: > On 6/10/20 11:35 PM, Tom Rini wrote: > > On Wed, Jun 10, 2020 at 10:56:40PM +0200, Marek Vasut wrote: > >> On 6/10/20 10:54 PM, Tom Rini wrote: > >>> On Wed, Jun 10, 2020 at 10:46:23PM +0200, Marek Vasut wrote: > >>>> On 6/10/20 10:11 PM, Tom Rini wrote: > >>>> [...] > >>>>>>>>> You mean be more like barebox and Linux where the board-specific stuff > >>>>>>>>> is kicked up one level and we have a more generic binary? I don't know > >>>>>>>>> and there's so much work that would be required before having to move > >>>>>>>>> this from a Kconfig choice to just Kconfig options I don't see that as > >>>>>>>>> being a relevant question here. > >>>>>>>>> > >>>>>>>>> Or did I misunderstand the question? > >>>>>>>> > >>>>>>>> More like automatically have a Kconfig option generate it's _SPL and > >>>>>>>> _TPL variant. > >>>>>>> > >>>>>>> Ah. Well, that is rephrasing my first question. Would it ever make > >>>>>>> sense to have more than one of these enabled? > >>>>>> > >>>>>> For some sort of universal SPL, maybe ? > >>>>> > >>>>> So no, there's not a reason today then and it should be a choice. > >>>> > >>>> Can you provide some more detailed explanation why we shouldn't generate > >>>> _SPL and _TPL variants of Kconfig entries for all Kconfig entries ? It > >>>> would make things much simpler and permit configuring SPL/TPL the same > >>>> way U-Boot is configured, with their own set of options. > >>> > >>> In the context of this particular thread? I don't see how it's helpful > >>> to say 3 times that we're in fact building for Tegra or STM32 or SoCFPGA > >>> when you can't build something that runs on more than one of those. > >> > >> In general. > >> > >> Here I can imagine it is possible to build SoCFPGA+STM32 universal SD > >> card image in fact. > > > > So that's the case I brought up at first and you said no to. > > I think I don't understand this part. You're talking about a binary that runs on more than one dissimilar SoC, yes? > > I don't > > see that in the foreseeable future but I don't feel so strongly about > > making this config area tidy enough to argue the point. So fine, leave > > it as separate options, the default y if ... is reasonable enough to > > ensure we get functional builds. > > This patch is OK as-is. Yes, since I'm no longer asking for changes to it, it's fine. > My point is more in the general direction of being able to configure > SPL/TPL/U-Boot separately, without being forced to craft nasty ifdeffery > in include/config/board.h if I need something enabled in SPL, but not in > U-Boot, and vice versa. And for that the Kconfig should be able to > somehow emit the _SPL/_TPL/U-Boot options of all symbols I think, so > that we won't need separate entry for each. I haven't seen a case where the nasty ifdeffery in a config header file wasn't basically either: - Now wrong (we _have_ the symbols today to say we don't want X in SPL) - Working around a case where we need to use $(SPL_TPL_) somewhere but didn't know that we could use $(SPL_TPL_) to fix the problem instead. - Now not useful (for example, disable CMD_xxx for SPL, but we've really sorted things out so now so doing that didn't help anything). Now I'm happy to admit that I just might be missing a case as I've only gotten as far as "undef CONFIG_[ABC]" and BOOTCOMMAND is possibly leading to embedding a long string where we really don't want it. Please point me at more undef cases that need to be resolved in some way. Thanks!
On 6/10/20 11:57 PM, Tom Rini wrote: [...] >> My point is more in the general direction of being able to configure >> SPL/TPL/U-Boot separately, without being forced to craft nasty ifdeffery >> in include/config/board.h if I need something enabled in SPL, but not in >> U-Boot, and vice versa. And for that the Kconfig should be able to >> somehow emit the _SPL/_TPL/U-Boot options of all symbols I think, so >> that we won't need separate entry for each. > > I haven't seen a case where the nasty ifdeffery in a config header file > wasn't basically either: > - Now wrong (we _have_ the symbols today to say we don't want X in SPL) > - Working around a case where we need to use $(SPL_TPL_) somewhere but > didn't know that we could use $(SPL_TPL_) to fix the problem instead. > - Now not useful (for example, disable CMD_xxx for SPL, but we've really > sorted things out so now so doing that didn't help anything). > > Now I'm happy to admit that I just might be missing a case as I've only > gotten as far as "undef CONFIG_[ABC]" and BOOTCOMMAND is possibly > leading to embedding a long string where we really don't want it. > Please point me at more undef cases that need to be resolved in some > way. I don't want to resolve these problems one-by-one , the obvious solution for them AND the growth of Kconfig files with multiple copies of the same symbol for SPL/TPL/U-Boot is the have Kconfig generate those symbols automatically for SPL/TPL/U-Boot and then let user pick the configuration as needed. That would make the undefs in include/configs/board.h go away easily and would reduce the duplication in Kconfig files.
On Thu, Jun 11, 2020 at 12:02:08AM +0200, Marek Vasut wrote: > On 6/10/20 11:57 PM, Tom Rini wrote: > [...] > > >> My point is more in the general direction of being able to configure > >> SPL/TPL/U-Boot separately, without being forced to craft nasty ifdeffery > >> in include/config/board.h if I need something enabled in SPL, but not in > >> U-Boot, and vice versa. And for that the Kconfig should be able to > >> somehow emit the _SPL/_TPL/U-Boot options of all symbols I think, so > >> that we won't need separate entry for each. > > > > I haven't seen a case where the nasty ifdeffery in a config header file > > wasn't basically either: > > - Now wrong (we _have_ the symbols today to say we don't want X in SPL) > > - Working around a case where we need to use $(SPL_TPL_) somewhere but > > didn't know that we could use $(SPL_TPL_) to fix the problem instead. > > - Now not useful (for example, disable CMD_xxx for SPL, but we've really > > sorted things out so now so doing that didn't help anything). > > > > Now I'm happy to admit that I just might be missing a case as I've only > > gotten as far as "undef CONFIG_[ABC]" and BOOTCOMMAND is possibly > > leading to embedding a long string where we really don't want it. > > Please point me at more undef cases that need to be resolved in some > > way. > > I don't want to resolve these problems one-by-one , the obvious solution > for them AND the growth of Kconfig files with multiple copies of the > same symbol for SPL/TPL/U-Boot is the have Kconfig generate those > symbols automatically for SPL/TPL/U-Boot and then let user pick the > configuration as needed. That would make the undefs in > include/configs/board.h go away easily and would reduce the duplication > in Kconfig files. I strongly disagree that having two or three times the number of symbols to select from will result in a better outcome. I see 386 SPL_ '^config ' lines (of which a number are 'default y' lines) and that's more than I thought there would be. There's also 6175 overall symbols today. I would be hopeful that the "lets look at the DTS files and see what we can tell about a platform" thread could help with configuring a good binary better. Because to me the worrisome entries are things like MMC_HS200_SUPPORT / SPL_MMC_HS200_SUPPORT as I would start by asking if the cases where we don't enable that in SPL are intention or oversight.
On 6/11/20 12:17 AM, Tom Rini wrote: > On Thu, Jun 11, 2020 at 12:02:08AM +0200, Marek Vasut wrote: >> On 6/10/20 11:57 PM, Tom Rini wrote: >> [...] >> >>>> My point is more in the general direction of being able to configure >>>> SPL/TPL/U-Boot separately, without being forced to craft nasty ifdeffery >>>> in include/config/board.h if I need something enabled in SPL, but not in >>>> U-Boot, and vice versa. And for that the Kconfig should be able to >>>> somehow emit the _SPL/_TPL/U-Boot options of all symbols I think, so >>>> that we won't need separate entry for each. >>> >>> I haven't seen a case where the nasty ifdeffery in a config header file >>> wasn't basically either: >>> - Now wrong (we _have_ the symbols today to say we don't want X in SPL) >>> - Working around a case where we need to use $(SPL_TPL_) somewhere but >>> didn't know that we could use $(SPL_TPL_) to fix the problem instead. >>> - Now not useful (for example, disable CMD_xxx for SPL, but we've really >>> sorted things out so now so doing that didn't help anything). >>> >>> Now I'm happy to admit that I just might be missing a case as I've only >>> gotten as far as "undef CONFIG_[ABC]" and BOOTCOMMAND is possibly >>> leading to embedding a long string where we really don't want it. >>> Please point me at more undef cases that need to be resolved in some >>> way. >> >> I don't want to resolve these problems one-by-one , the obvious solution >> for them AND the growth of Kconfig files with multiple copies of the >> same symbol for SPL/TPL/U-Boot is the have Kconfig generate those >> symbols automatically for SPL/TPL/U-Boot and then let user pick the >> configuration as needed. That would make the undefs in >> include/configs/board.h go away easily and would reduce the duplication >> in Kconfig files. > > I strongly disagree that having two or three times the number of symbols > to select from will result in a better outcome. I see 386 SPL_ > '^config ' lines (of which a number are 'default y' lines) and that's > more than I thought there would be. There's also 6175 overall symbols > today. I would be hopeful that the "lets look at the DTS files and see > what we can tell about a platform" thread could help with configuring a > good binary better. Because to me the worrisome entries are things like > MMC_HS200_SUPPORT / SPL_MMC_HS200_SUPPORT as I would start by asking if > the cases where we don't enable that in SPL are intention or oversight. Surely HS200 (and HS400/SDR104/UHS) , which wastes _A_LOT_ of boot time and a lot of space, is a good candidate for something to be disabled in SPL. Some core platform-specific symbols are fine with not having matching _SPL/_TPL counterpart, sure, but that's not the majority of the Kconfig symbols, right? Or maybe we need some new Kconfig keyword (instead of 'config') to describe symbols which do need _SPL/_TPL variants ?
Hi, > From: Tom Rini <trini at konsulko.com> > Sent: mercredi 10 juin 2020 23:58 > > On Wed, Jun 10, 2020 at 11:40:33PM +0200, Marek Vasut wrote: > > On 6/10/20 11:35 PM, Tom Rini wrote: > > > On Wed, Jun 10, 2020 at 10:56:40PM +0200, Marek Vasut wrote: > > >> On 6/10/20 10:54 PM, Tom Rini wrote: > > >>> On Wed, Jun 10, 2020 at 10:46:23PM +0200, Marek Vasut wrote: > > >>>> On 6/10/20 10:11 PM, Tom Rini wrote: > > >>>> [...] > > >>>>>>>>> You mean be more like barebox and Linux where the > > >>>>>>>>> board-specific stuff is kicked up one level and we have a > > >>>>>>>>> more generic binary? I don't know and there's so much work > > >>>>>>>>> that would be required before having to move this from a > > >>>>>>>>> Kconfig choice to just Kconfig options I don't see that as being a > relevant question here. > > >>>>>>>>> > > >>>>>>>>> Or did I misunderstand the question? > > >>>>>>>> > > >>>>>>>> More like automatically have a Kconfig option generate it's > > >>>>>>>> _SPL and _TPL variant. > > >>>>>>> > > >>>>>>> Ah. Well, that is rephrasing my first question. Would it > > >>>>>>> ever make sense to have more than one of these enabled? > > >>>>>> > > >>>>>> For some sort of universal SPL, maybe ? > > >>>>> > > >>>>> So no, there's not a reason today then and it should be a choice. > > >>>> > > >>>> Can you provide some more detailed explanation why we shouldn't > > >>>> generate _SPL and _TPL variants of Kconfig entries for all > > >>>> Kconfig entries ? It would make things much simpler and permit > > >>>> configuring SPL/TPL the same way U-Boot is configured, with their own > set of options. > > >>> > > >>> In the context of this particular thread? I don't see how it's > > >>> helpful to say 3 times that we're in fact building for Tegra or > > >>> STM32 or SoCFPGA when you can't build something that runs on more > than one of those. > > >> > > >> In general. > > >> > > >> Here I can imagine it is possible to build SoCFPGA+STM32 universal > > >> SD card image in fact. > > > > > > So that's the case I brought up at first and you said no to. > > > > I think I don't understand this part. > > You're talking about a binary that runs on more than one dissimilar SoC, yes? > > > > I don't > > > see that in the foreseeable future but I don't feel so strongly > > > about making this config area tidy enough to argue the point. So > > > fine, leave it as separate options, the default y if ... is > > > reasonable enough to ensure we get functional builds. > > > > This patch is OK as-is. > > Yes, since I'm no longer asking for changes to it, it's fine. I don't see possible use case where the 3 drivers configurations can be activated at the same time, Except, as Marek indicated it, the same U-Boot binary compiled to be executed for several architecture (STM32, IMX, TEGRA). But it is not possible (duplicate cote between arch) as each configuration is linked to archictecture variant (the synopsis IP is modified by SOC vendor). For me it should be possible to change the config option to select, even it is no more requested and the patch is OK as-is (I keep the possibility to have the 3 options activated). Regards Patrick
On 6/11/20 3:44 PM, Patrick DELAUNAY wrote: > Hi, > >> From: Tom Rini <trini at konsulko.com> >> Sent: mercredi 10 juin 2020 23:58 >> >> On Wed, Jun 10, 2020 at 11:40:33PM +0200, Marek Vasut wrote: >>> On 6/10/20 11:35 PM, Tom Rini wrote: >>>> On Wed, Jun 10, 2020 at 10:56:40PM +0200, Marek Vasut wrote: >>>>> On 6/10/20 10:54 PM, Tom Rini wrote: >>>>>> On Wed, Jun 10, 2020 at 10:46:23PM +0200, Marek Vasut wrote: >>>>>>> On 6/10/20 10:11 PM, Tom Rini wrote: >>>>>>> [...] >>>>>>>>>>>> You mean be more like barebox and Linux where the >>>>>>>>>>>> board-specific stuff is kicked up one level and we have a >>>>>>>>>>>> more generic binary? I don't know and there's so much work >>>>>>>>>>>> that would be required before having to move this from a >>>>>>>>>>>> Kconfig choice to just Kconfig options I don't see that as being a >> relevant question here. >>>>>>>>>>>> >>>>>>>>>>>> Or did I misunderstand the question? >>>>>>>>>>> >>>>>>>>>>> More like automatically have a Kconfig option generate it's >>>>>>>>>>> _SPL and _TPL variant. >>>>>>>>>> >>>>>>>>>> Ah. Well, that is rephrasing my first question. Would it >>>>>>>>>> ever make sense to have more than one of these enabled? >>>>>>>>> >>>>>>>>> For some sort of universal SPL, maybe ? >>>>>>>> >>>>>>>> So no, there's not a reason today then and it should be a choice. >>>>>>> >>>>>>> Can you provide some more detailed explanation why we shouldn't >>>>>>> generate _SPL and _TPL variants of Kconfig entries for all >>>>>>> Kconfig entries ? It would make things much simpler and permit >>>>>>> configuring SPL/TPL the same way U-Boot is configured, with their own >> set of options. >>>>>> >>>>>> In the context of this particular thread? I don't see how it's >>>>>> helpful to say 3 times that we're in fact building for Tegra or >>>>>> STM32 or SoCFPGA when you can't build something that runs on more >> than one of those. >>>>> >>>>> In general. >>>>> >>>>> Here I can imagine it is possible to build SoCFPGA+STM32 universal >>>>> SD card image in fact. >>>> >>>> So that's the case I brought up at first and you said no to. >>> >>> I think I don't understand this part. >> >> You're talking about a binary that runs on more than one dissimilar SoC, yes? >> >>>> I don't >>>> see that in the foreseeable future but I don't feel so strongly >>>> about making this config area tidy enough to argue the point. So >>>> fine, leave it as separate options, the default y if ... is >>>> reasonable enough to ensure we get functional builds. >>> >>> This patch is OK as-is. >> >> Yes, since I'm no longer asking for changes to it, it's fine. > > I don't see possible use case where the 3 drivers configurations can be activated at the same time, > Except, as Marek indicated it, the same U-Boot binary compiled to be executed for several architecture > (STM32, IMX, TEGRA). > > But it is not possible (duplicate cote between arch) as each configuration is linked to archictecture variant > (the synopsis IP is modified by SOC vendor). You can build Linux for all three and use the same zImage, so it should also be possible for U-Boot. > For me it should be possible to change the config option to select, > even it is no more requested and the patch is OK as-is (I keep the possibility to have the 3 options activated). The patch is OK.
On Mon, Jun 08, 2020 at 11:27:19AM +0200, Patrick Delaunay wrote: > Add configuration flag to select the supported dwc driver configuration: > - CONFIG_DWC_ETH_QOS_TEGRA186 > - CONFIG_DWC_ETH_QOS_IMX > - CONFIG_DWC_ETH_QOS_STM32 > > See Linux driver ethernet/stmicro/stmmac and associated glue layers > for other configuration examples. > > This patch removes the not-selected compatibles and lets the linker remove > the unused functions to reduce the size of the driver. > > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com> Applied to u-boot/master, thanks! -- Tom
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 0b08de0ef4..c4b3667d3b 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -156,9 +156,30 @@ config DWC_ETH_QOS help This driver supports the Synopsys Designware Ethernet QOS (Quality Of Service) IP block. The IP supports many options for bus type, - clocking/reset structure, and feature list. This driver currently - supports the specific configuration used in NVIDIA's Tegra186 chip, - but should be extensible to other combinations quite easily. + clocking/reset structure, and feature list. + +config DWC_ETH_QOS_IMX + bool "Synopsys DWC Ethernet QOS device support for IMX" + depends on DWC_ETH_QOS + help + The Synopsys Designware Ethernet QOS IP block with the specific + configuration used in IMX soc. + +config DWC_ETH_QOS_STM32 + bool "Synopsys DWC Ethernet QOS device support for STM32" + depends on DWC_ETH_QOS + default y if ARCH_STM32MP + help + The Synopsys Designware Ethernet QOS IP block with the specific + configuration used in STM32MP soc. + +config DWC_ETH_QOS_TEGRA186 + bool "Synopsys DWC Ethernet QOS device support for TEGRA186" + depends on DWC_ETH_QOS + default y if TEGRA186 + help + The Synopsys Designware Ethernet QOS IP block with specific + configuration used in NVIDIA's Tegra186 chip. config E1000 bool "Intel PRO/1000 Gigabit Ethernet support" diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index 3f4437069b..d4536a325f 100644 --- a/drivers/net/dwc_eth_qos.c +++ b/drivers/net/dwc_eth_qos.c @@ -2100,7 +2100,7 @@ static struct eqos_ops eqos_tegra186_ops = { .eqos_get_tick_clk_rate = eqos_get_tick_clk_rate_tegra186 }; -static const struct eqos_config eqos_tegra186_config = { +static const struct eqos_config __maybe_unused eqos_tegra186_config = { .reg_access_always_ok = false, .mdio_wait = 10, .swr_wait = 10, @@ -2127,7 +2127,7 @@ static struct eqos_ops eqos_stm32_ops = { .eqos_get_tick_clk_rate = eqos_get_tick_clk_rate_stm32 }; -static const struct eqos_config eqos_stm32_config = { +static const struct eqos_config __maybe_unused eqos_stm32_config = { .reg_access_always_ok = false, .mdio_wait = 10000, .swr_wait = 50, @@ -2154,7 +2154,7 @@ static struct eqos_ops eqos_imx_ops = { .eqos_get_tick_clk_rate = eqos_get_tick_clk_rate_imx }; -struct eqos_config eqos_imx_config = { +struct eqos_config __maybe_unused eqos_imx_config = { .reg_access_always_ok = false, .mdio_wait = 10000, .swr_wait = 50, @@ -2165,18 +2165,24 @@ struct eqos_config eqos_imx_config = { }; static const struct udevice_id eqos_ids[] = { +#if IS_ENABLED(CONFIG_DWC_ETH_QOS_TEGRA186) { .compatible = "nvidia,tegra186-eqos", .data = (ulong)&eqos_tegra186_config }, +#endif +#if IS_ENABLED(CONFIG_DWC_ETH_QOS_STM32) { .compatible = "snps,dwmac-4.20a", .data = (ulong)&eqos_stm32_config }, +#endif +#if IS_ENABLED(CONFIG_DWC_ETH_QOS_IMX) { .compatible = "fsl,imx-eqos", .data = (ulong)&eqos_imx_config }, +#endif { } };
Add configuration flag to select the supported dwc driver configuration: - CONFIG_DWC_ETH_QOS_TEGRA186 - CONFIG_DWC_ETH_QOS_IMX - CONFIG_DWC_ETH_QOS_STM32 See Linux driver ethernet/stmicro/stmmac and associated glue layers for other configuration examples. This patch removes the not-selected compatibles and lets the linker remove the unused functions to reduce the size of the driver. Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com> --- Hi, I propose this patch after Marek's remark in [1]. It is also linked to [2] to limit the STM32 glue for ST compatible. [1] net: dwc_eth_qos: Make clk_rx and clk_tx optional http://patchwork.ozlabs.org/project/uboot/patch/20200512095603.29126-5-david.wu at rock-chips.com/#2432595 [2] net: dwc_eth_qos: update the compatible supported for STM32 http://patchwork.ozlabs.org/project/uboot/list/?series=176917 drivers/net/Kconfig | 27 ++++++++++++++++++++++++--- drivers/net/dwc_eth_qos.c | 12 +++++++++--- 2 files changed, 33 insertions(+), 6 deletions(-)