On 3/18/20 2:14 AM, Joe Hershberger wrote: > Hi Tom, > > On Tue, Mar 17, 2020 at 7:59 PM Tom Rini <tr...@konsulko.com> wrote: >> >> On Tue, Mar 17, 2020 at 07:54:51PM -0500, Joe Hershberger wrote: >>> On Tue, Mar 17, 2020 at 1:55 PM Tom Rini <tr...@konsulko.com> wrote: >>>> >>>> On Tue, Mar 17, 2020 at 07:53:58PM +0100, Marek Vasut wrote: >>>>> On 3/17/20 7:44 PM, Tom Rini wrote: >>>>>> On Tue, Mar 17, 2020 at 07:43:11PM +0100, Marek Vasut wrote: >>>>>>> On 3/17/20 7:42 PM, Tom Rini wrote: >>>>>>>> On Tue, Mar 17, 2020 at 07:39:49PM +0100, Marek Vasut wrote: >>>>>>>>> On 3/17/20 7:30 PM, Tom Rini wrote: >>>>>>>>>> On Tue, Mar 17, 2020 at 07:23:07PM +0100, Marek Vasut wrote: >>>>>>>>>>> On 3/17/20 7:10 PM, Masahiro Yamada wrote: >>>>>>>>>>>> On Sun, Mar 15, 2020 at 8:19 AM Marek Vasut >>>>>>>>>>>> <marek.va...@gmail.com> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>> Drop the example, for two reasons. First, it is tapping directly >>>>>>>>>>>>> into >>>>>>>>>>>>> the IO accessors of the SMC911x, while it should instead go >>>>>>>>>>>>> through >>>>>>>>>>>>> the net device API. Second, this makes conversion of the SMC911x >>>>>>>>>>>>> driver >>>>>>>>>>>>> to DM real hard. >>>>>>>>>>>>> >>>>>>>>>>>>> Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com> >>>>>>>>>>>>> Cc: Joe Hershberger <joe.hershber...@ni.com> >>>>>>>>>>>>> Cc: Tom Rini <tr...@konsulko.com> >>>>>>>>>>>>> --- >>>>>>>>>>>>> examples/standalone/Makefile | 1 - >>>>>>>>>>>>> examples/standalone/smc911x_eeprom.c | 379 >>>>>>>>>>>>> --------------------------- >>>>>>>>>>>>> 2 files changed, 380 deletions(-) >>>>>>>>>>>>> delete mode 100644 examples/standalone/smc911x_eeprom.c >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> Yeah, I was disturbed by this example code. >>>>>>>>>>>> >>>>>>>>>>>> I agree we should drop it. >>>>>>>>>>>> >>>>>>>>>>>> Reviewed-by: Masahiro Yamada <yamada.masah...@socionext.com> >>>>>>>>>>> >>>>>>>>>>> Well I dunno. Can this be rewritten on top of DM somehow ? Do we >>>>>>>>>>> even >>>>>>>>>>> have U-Boot application API to access DM EEPROM ? >>>>>>>>>> >>>>>>>>>> We should just drop it I think. The biggest surface we have today >>>>>>>>>> for >>>>>>>>>> external application is EFI application now, not U-Boot specific API. >>>>>>>>>> We can't drop the API but we don't expand it without very good >>>>>>>>>> reason. >>>>>>>>> >>>>>>>>> But this drops the ability to access the SMC911x EEPROM too. >>>>>>>>> So maybe we need some DM EEPROM implementation in the SMC911x driver ? >>>>>>>>> Does anyone have SMC911x with an external EEPROM ? >>>>>>>> >>>>>>>> All this does is drop an example. I don't see anything removing API >>>>>>>> code itself. >>>>>>> >>>>>>> Where did I say anything about API code ? >>>>>> >>>>>> Nowhere, which is my point. You're just dropping an example, not the >>>>>> ability to do $X. >>>>> >>>>> If $X is ability to access the EEPROM, then I am dropping $X here. >>>> >>>> No, you're dropping an example of doing $X. >>> >>> Correct. But the move to DM in the driver will drop the functions this >>> example was using, no? >> >> If it was using something that's not in <_exports.h> I don't see that as >> a problem. A standalone app could do whatever it likes with the >> hardware and needs to restore the hardware before passing control back >> to U-Boot (if it's doing that). > > My understanding (correct me if I'm wrong) is that the SCM911x driver > in question used to expose its eeprom through the external API, but > not as though there was a special API directly implemented in that > driver. > > I think the behavior change that Marek was concerned about was that > this particular device would not be accessible via the external eeprom > api any longer. One can have the driver expose some EEPROM interface, but not via the U-Boot application example. But then, the U-Boot application was poking registers directly anyway, which is awful.
-- Best regards, Marek Vasut