[PATCH] [RFC] net: smc911x: Drop the standalone EEPROM example

[Marek Vasut]

On 3/18/20 2:14 AM, Joe Hershberger wrote:
> Hi Tom,
> 
> On Tue, Mar 17, 2020 at 7:59 PM Tom Rini <trini at 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 <trini at 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.vasut at 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+renesas at gmail.com>
>>>>>>>>>>>>> Cc: Joe Hershberger <joe.hershberger at ni.com>
>>>>>>>>>>>>> Cc: Tom Rini <trini at 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.masahiro at 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