mbox series

[00/12] net: smc911x: Convert to DM

Message ID 20200315165843.81753-1-marek.vasut+renesas@gmail.com
Headers show
Series net: smc911x: Convert to DM | expand

Message

Marek Vasut March 15, 2020, 4:58 p.m. UTC
Perform DM conversion of the SMC911x driver. Note that as I do not have
any hardware with this chip, I only compile-tested this conversion. But
it seems Yamada-san has one, so please test.

Note that the DT compatible is set only for 9115 , so this might need
to be changed.

Note that this is compile-tested, but not hardware tested, so please do
send me patches, thanks.

Marek Vasut (12):
  net: smc911x: Remove pkt_data_{push,pull}
  net: smc911x: Replace malloc()+memset() with calloc()
  net: smc911x: Rename smc911x_rx() to smc911x_recv()
  net: smc911x: Invert the logic in smc911x_miiphy_{read,write}()
  net: smc911x: Fix potential memleak() in init fail path
  net: smc911x: Inline all functions from header file
  net: smc911x: Drop weak alias from 32bit accessors
  net: smc911x: Convert IO accessors to {read,write}{w,l}()
  net: smc911x: Pass around driver private data
  net: smc911x: Clean up the status handling in smc911x_recv()
  net: smc911x: Split non-DM specific bits from common code
  net: smc911x: Add DM support

 board/renesas/blanche/blanche.c      |   2 +
 drivers/net/smc911x.c                | 523 +++++++++++++++++++++------
 drivers/net/smc911x.h                | 157 --------
 examples/standalone/Makefile         |   5 +-
 examples/standalone/smc911x_eeprom.c | 150 ++++++++
 5 files changed, 577 insertions(+), 260 deletions(-)

Cc: Joe Hershberger <joe.hershberger at ni.com>
Cc: Masahiro Yamada <yamada.masahiro at socionext.com>

Comments

Masahiro Yamada March 17, 2020, 6:17 a.m. UTC | #1
Hi Marek,


On Mon, Mar 16, 2020 at 1:59 AM Marek Vasut <marek.vasut at gmail.com> wrote:
>
> Perform DM conversion of the SMC911x driver. Note that as I do not have
> any hardware with this chip, I only compile-tested this conversion. But
> it seems Yamada-san has one, so please test.

With a quick test on my board, it worked for me.

I will leave comments to each patch, but
they are mostly nit-picking.

Thanks for the cleanups and the conversion.




>
> Note that the DT compatible is set only for 9115 , so this might need
> to be changed.
>
> Note that this is compile-tested, but not hardware tested, so please do
> send me patches, thanks.
>
> Marek Vasut (12):
>   net: smc911x: Remove pkt_data_{push,pull}
>   net: smc911x: Replace malloc()+memset() with calloc()
>   net: smc911x: Rename smc911x_rx() to smc911x_recv()
>   net: smc911x: Invert the logic in smc911x_miiphy_{read,write}()
>   net: smc911x: Fix potential memleak() in init fail path
>   net: smc911x: Inline all functions from header file
>   net: smc911x: Drop weak alias from 32bit accessors
>   net: smc911x: Convert IO accessors to {read,write}{w,l}()
>   net: smc911x: Pass around driver private data
>   net: smc911x: Clean up the status handling in smc911x_recv()
>   net: smc911x: Split non-DM specific bits from common code
>   net: smc911x: Add DM support
>
>  board/renesas/blanche/blanche.c      |   2 +
>  drivers/net/smc911x.c                | 523 +++++++++++++++++++++------
>  drivers/net/smc911x.h                | 157 --------
>  examples/standalone/Makefile         |   5 +-
>  examples/standalone/smc911x_eeprom.c | 150 ++++++++
>  5 files changed, 577 insertions(+), 260 deletions(-)
>
> Cc: Joe Hershberger <joe.hershberger at ni.com>
> Cc: Masahiro Yamada <yamada.masahiro at socionext.com>
>
> --
> 2.25.0
>
Marek Vasut March 21, 2020, 4:52 p.m. UTC | #2
On 3/17/20 7:17 AM, Masahiro Yamada wrote:
> Hi Marek,

Hi,

> On Mon, Mar 16, 2020 at 1:59 AM Marek Vasut <marek.vasut at gmail.com> wrote:
>>
>> Perform DM conversion of the SMC911x driver. Note that as I do not have
>> any hardware with this chip, I only compile-tested this conversion. But
>> it seems Yamada-san has one, so please test.
> 
> With a quick test on my board, it worked for me.
> 
> I will leave comments to each patch, but
> they are mostly nit-picking.
> 
> Thanks for the cleanups and the conversion.

I fixed most of those, thanks for testing.

I also got access to the V2H blanche and there it works too, so maybe
some future revision of this could be queued up for next.
Adam Ford March 24, 2020, 1:41 a.m. UTC | #3
On Sat, Mar 21, 2020 at 11:57 AM Marek Vasut <marek.vasut at gmail.com> wrote:
>
> On 3/17/20 7:17 AM, Masahiro Yamada wrote:
> > Hi Marek,
>
> Hi,
>
> > On Mon, Mar 16, 2020 at 1:59 AM Marek Vasut <marek.vasut at gmail.com> wrote:
> >>
> >> Perform DM conversion of the SMC911x driver. Note that as I do not have

Thanks for taking this on.

> >> any hardware with this chip, I only compile-tested this conversion. But
> >> it seems Yamada-san has one, so please test.

I have a DM3730 with this chip.  Without DM_ETH, the
omap3_logic_defconfig doesn't compile with the following errors:

drivers/net/smc911x.c: In function ?smc911x_initialize?:
drivers/net/smc911x.c:477:2: error: ?dev? undeclared (first use in
this function)
  477 |  dev = kzalloc(sizeof(*priv), GFP_KERNEL);
      |  ^~~
drivers/net/smc911x.c:477:2: note: each undeclared identifier is
reported only once for each function it appears in
drivers/net/smc911x.c:477:8: warning: implicit declaration of function
?kzalloc?; did you mean ?calloc?? [-Wimplicit-function-declaration]
  477 |  dev = kzalloc(sizeof(*priv), GFP_KERNEL);
      |        ^~~~~~~
      |        calloc
drivers/net/smc911x.c:477:31: error: ?GFP_KERNEL? undeclared (first
use in this function)
  477 |  dev = kzalloc(sizeof(*priv), GFP_KERNEL);
      |                               ^~~~~~~~~~
make[1]: *** [scripts/Makefile.build:279: drivers/net/smc911x.o] Error 1

> >
> > With a quick test on my board, it worked for me.
> >
> > I will leave comments to each patch, but
> > they are mostly nit-picking.
> >
> > Thanks for the cleanups and the conversion.
>
> I fixed most of those, thanks for testing.

If I enable DM_ETH, I need to make a few changes to the device tree,
but with those changes, I was able to get the Ethernet working.
The OMAP3 boards have the Ethernet node attached to their GPMC bus
which currently doesn't have a driver.  Making the GPMC compatible
with 'simple-bus' wasn't sufficient. With only that change, the system
would hang while trying to probe the network interface.

What I ended up having to do was add the following to my root node:

ethernet at 08000000 {
     compatible = "smsc,lan9221","smsc,lan9115";
     reg = <0x08000000>;
     bank-width = <2>;
     vddvario-supply = <&vddvario>;
     vdd33a-supply = <&vdd33a>;
     reg-io-width = <4>;
     smsc,save-mac-address;
};

I didn't look to see if I needed all those entries, but I was able to
get an IP address and ping a different computer.
If there is a future revision to fix the default build errors, I can
add similar comments with my tested-by if you want to CC me.

adam
>
> I also got access to the V2H blanche and there it works too, so maybe
> some future revision of this could be queued up for next.
>
> --
> Best regards,
> Marek Vasut
Marek Vasut March 25, 2020, 3:32 p.m. UTC | #4
On 3/24/20 2:41 AM, Adam Ford wrote:
> On Sat, Mar 21, 2020 at 11:57 AM Marek Vasut <marek.vasut at gmail.com> wrote:
>>
>> On 3/17/20 7:17 AM, Masahiro Yamada wrote:
>>> Hi Marek,
>>
>> Hi,
>>
>>> On Mon, Mar 16, 2020 at 1:59 AM Marek Vasut <marek.vasut at gmail.com> wrote:
>>>>
>>>> Perform DM conversion of the SMC911x driver. Note that as I do not have
> 
> Thanks for taking this on.
> 
>>>> any hardware with this chip, I only compile-tested this conversion. But
>>>> it seems Yamada-san has one, so please test.
> 
> I have a DM3730 with this chip.  Without DM_ETH, the
> omap3_logic_defconfig doesn't compile with the following errors:
> 
> drivers/net/smc911x.c: In function ?smc911x_initialize?:
> drivers/net/smc911x.c:477:2: error: ?dev? undeclared (first use in
> this function)
>   477 |  dev = kzalloc(sizeof(*priv), GFP_KERNEL);
>       |  ^~~
> drivers/net/smc911x.c:477:2: note: each undeclared identifier is
> reported only once for each function it appears in
> drivers/net/smc911x.c:477:8: warning: implicit declaration of function
> ?kzalloc?; did you mean ?calloc?? [-Wimplicit-function-declaration]
>   477 |  dev = kzalloc(sizeof(*priv), GFP_KERNEL);
>       |        ^~~~~~~
>       |        calloc
> drivers/net/smc911x.c:477:31: error: ?GFP_KERNEL? undeclared (first
> use in this function)
>   477 |  dev = kzalloc(sizeof(*priv), GFP_KERNEL);
>       |                               ^~~~~~~~~~
> make[1]: *** [scripts/Makefile.build:279: drivers/net/smc911x.o] Error 1

Well, replace the kzalloc with calloc.

>>> With a quick test on my board, it worked for me.
>>>
>>> I will leave comments to each patch, but
>>> they are mostly nit-picking.
>>>
>>> Thanks for the cleanups and the conversion.
>>
>> I fixed most of those, thanks for testing.
> 
> If I enable DM_ETH, I need to make a few changes to the device tree,
> but with those changes, I was able to get the Ethernet working.
> The OMAP3 boards have the Ethernet node attached to their GPMC bus
> which currently doesn't have a driver.  Making the GPMC compatible
> with 'simple-bus' wasn't sufficient. With only that change, the system
> would hang while trying to probe the network interface.
> 
> What I ended up having to do was add the following to my root node:
> 
> ethernet at 08000000 {
>      compatible = "smsc,lan9221","smsc,lan9115";
>      reg = <0x08000000>;
>      bank-width = <2>;
>      vddvario-supply = <&vddvario>;
>      vdd33a-supply = <&vdd33a>;
>      reg-io-width = <4>;
>      smsc,save-mac-address;
> };
> 
> I didn't look to see if I needed all those entries, but I was able to
> get an IP address and ping a different computer.
> If there is a future revision to fix the default build errors, I can
> add similar comments with my tested-by if you want to CC me.

I'm about to send a V3.