| Message ID | 1453067522-610-3-git-send-email-sjg@chromium.org |
|---|---|
| State | Accepted |
| Commit | c32a6fd07b1839e4a45729587ebc8e1c55601a4d |
| Delegated to: | Joe Hershberger |
| Headers | show |
Hi Simon, On Mon, Jan 18, 2016 at 5:51 AM, Simon Glass <sjg@chromium.org> wrote: > We should avoid weak functions with driver model. Existing boards that use > driver model don't need them, so let's kill them off. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > net/eth.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/eth.c b/net/eth.c > index 45fe6e3..d96d3a5 100644 > --- a/net/eth.c > +++ b/net/eth.c > @@ -96,6 +96,7 @@ static void eth_common_init(void) > phy_init(); > #endif > > +#ifndef CONFIG_DM_ETH > /* > * If board-specific initialization exists, call it. > * If not, call a CPU-specific one > @@ -107,10 +108,9 @@ static void eth_common_init(void) > if (cpu_eth_init(gd->bd) < 0) > printf("CPU Net Initialization Failed\n"); > } else { > -#ifndef CONFIG_DM_ETH > printf("Net Initialization Skipped\n"); > -#endif > } > +#endif > } > > #ifdef CONFIG_DM_ETH > -- I posted the same patch [1] before. But you and Joe mentioned that this is still needed on some cases. And even if it is not needed, we may still have a chance to insert some board-specific stuff into these two routines. [1] http://patchwork.ozlabs.org/patch/510391/ Regards, Bin
Hi Bin, On 17 January 2016 at 20:25, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Mon, Jan 18, 2016 at 5:51 AM, Simon Glass <sjg@chromium.org> wrote: >> We should avoid weak functions with driver model. Existing boards that use >> driver model don't need them, so let's kill them off. >> >> Signed-off-by: Simon Glass <sjg@chromium.org> >> --- >> >> net/eth.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/net/eth.c b/net/eth.c >> index 45fe6e3..d96d3a5 100644 >> --- a/net/eth.c >> +++ b/net/eth.c >> @@ -96,6 +96,7 @@ static void eth_common_init(void) >> phy_init(); >> #endif >> >> +#ifndef CONFIG_DM_ETH >> /* >> * If board-specific initialization exists, call it. >> * If not, call a CPU-specific one >> @@ -107,10 +108,9 @@ static void eth_common_init(void) >> if (cpu_eth_init(gd->bd) < 0) >> printf("CPU Net Initialization Failed\n"); >> } else { >> -#ifndef CONFIG_DM_ETH >> printf("Net Initialization Skipped\n"); >> -#endif >> } >> +#endif >> } >> >> #ifdef CONFIG_DM_ETH >> -- > > I posted the same patch [1] before. But you and Joe mentioned that > this is still needed on some cases. And even if it is not needed, we > may still have a chance to insert some board-specific stuff into these > two routines. > > [1] http://patchwork.ozlabs.org/patch/510391/ Well I think I was on your side :-) We really don't want this with driver model as calling weak functions is skirting around driver model. If there is a feature that is needed we should model it properly with a uclass (e.g. pinctrl, GPIOs). Regards, Simon
Hi Simon, On Mon, Jan 18, 2016 at 11:58 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Bin, > > On 17 January 2016 at 20:25, Bin Meng <bmeng.cn@gmail.com> wrote: >> Hi Simon, >> >> On Mon, Jan 18, 2016 at 5:51 AM, Simon Glass <sjg@chromium.org> wrote: >>> We should avoid weak functions with driver model. Existing boards that use >>> driver model don't need them, so let's kill them off. >>> >>> Signed-off-by: Simon Glass <sjg@chromium.org> >>> --- >>> >>> net/eth.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/net/eth.c b/net/eth.c >>> index 45fe6e3..d96d3a5 100644 >>> --- a/net/eth.c >>> +++ b/net/eth.c >>> @@ -96,6 +96,7 @@ static void eth_common_init(void) >>> phy_init(); >>> #endif >>> >>> +#ifndef CONFIG_DM_ETH >>> /* >>> * If board-specific initialization exists, call it. >>> * If not, call a CPU-specific one >>> @@ -107,10 +108,9 @@ static void eth_common_init(void) >>> if (cpu_eth_init(gd->bd) < 0) >>> printf("CPU Net Initialization Failed\n"); >>> } else { >>> -#ifndef CONFIG_DM_ETH >>> printf("Net Initialization Skipped\n"); >>> -#endif >>> } >>> +#endif >>> } >>> >>> #ifdef CONFIG_DM_ETH >>> -- >> >> I posted the same patch [1] before. But you and Joe mentioned that >> this is still needed on some cases. And even if it is not needed, we >> may still have a chance to insert some board-specific stuff into these >> two routines. >> >> [1] http://patchwork.ozlabs.org/patch/510391/ > > Well I think I was on your side :-) We really don't want this with > driver model as calling weak functions is skirting around driver > model. If there is a feature that is needed we should model it > properly with a uclass (e.g. pinctrl, GPIOs). > OK, unless Joe has different opinion :) Reviewed-by: Bin Meng <bmeng.cn@gmail.com> Regards, Bin
On Sun, Jan 17, 2016 at 10:29 PM, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Mon, Jan 18, 2016 at 11:58 AM, Simon Glass <sjg@chromium.org> wrote: >> Hi Bin, >> >> On 17 January 2016 at 20:25, Bin Meng <bmeng.cn@gmail.com> wrote: >>> Hi Simon, >>> >>> On Mon, Jan 18, 2016 at 5:51 AM, Simon Glass <sjg@chromium.org> wrote: >>>> We should avoid weak functions with driver model. Existing boards that use >>>> driver model don't need them, so let's kill them off. >>>> >>>> Signed-off-by: Simon Glass <sjg@chromium.org> >>>> --- >>>> >>>> net/eth.c | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/net/eth.c b/net/eth.c >>>> index 45fe6e3..d96d3a5 100644 >>>> --- a/net/eth.c >>>> +++ b/net/eth.c >>>> @@ -96,6 +96,7 @@ static void eth_common_init(void) >>>> phy_init(); >>>> #endif >>>> >>>> +#ifndef CONFIG_DM_ETH >>>> /* >>>> * If board-specific initialization exists, call it. >>>> * If not, call a CPU-specific one >>>> @@ -107,10 +108,9 @@ static void eth_common_init(void) >>>> if (cpu_eth_init(gd->bd) < 0) >>>> printf("CPU Net Initialization Failed\n"); >>>> } else { >>>> -#ifndef CONFIG_DM_ETH >>>> printf("Net Initialization Skipped\n"); >>>> -#endif >>>> } >>>> +#endif >>>> } >>>> >>>> #ifdef CONFIG_DM_ETH >>>> -- >>> >>> I posted the same patch [1] before. But you and Joe mentioned that >>> this is still needed on some cases. And even if it is not needed, we >>> may still have a chance to insert some board-specific stuff into these >>> two routines. >>> >>> [1] http://patchwork.ozlabs.org/patch/510391/ >> >> Well I think I was on your side :-) We really don't want this with >> driver model as calling weak functions is skirting around driver >> model. If there is a feature that is needed we should model it >> properly with a uclass (e.g. pinctrl, GPIOs). >> > > OK, unless Joe has different opinion :) Only problem I would have is if it is breaking other boards in the mean time until all clocking and pinmux stuff is supported... but like I said before (in your [1]), if a person finds they need this when porting an eth driver to DM and they don't have the needed other DM driver yet, this can easily be reverted when that situation comes up. > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> > > Regards, > Bin > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot
On Sun, Jan 17, 2016 at 3:51 PM, Simon Glass <sjg@chromium.org> wrote: > We should avoid weak functions with driver model. Existing boards that use > driver model don't need them, so let's kill them off. > > Signed-off-by: Simon Glass <sjg@chromium.org> Acked-by: Joe Hershberger <joe.hershberger@ni.com>
Hi Simon, https://patchwork.ozlabs.org/patch/569315/ was applied to u-boot-net.git. Thanks! -Joe
diff --git a/net/eth.c b/net/eth.c index 45fe6e3..d96d3a5 100644 --- a/net/eth.c +++ b/net/eth.c @@ -96,6 +96,7 @@ static void eth_common_init(void) phy_init(); #endif +#ifndef CONFIG_DM_ETH /* * If board-specific initialization exists, call it. * If not, call a CPU-specific one @@ -107,10 +108,9 @@ static void eth_common_init(void) if (cpu_eth_init(gd->bd) < 0) printf("CPU Net Initialization Failed\n"); } else { -#ifndef CONFIG_DM_ETH printf("Net Initialization Skipped\n"); -#endif } +#endif } #ifdef CONFIG_DM_ETH
We should avoid weak functions with driver model. Existing boards that use driver model don't need them, so let's kill them off. Signed-off-by: Simon Glass <sjg@chromium.org> --- net/eth.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)