Re: [Qemu-devel] [PATCH 1/2] hw/net: add support for Allwinner EMAC Fast Ethernet controller

2014-01-12 Thread Edgar E. Iglesias
On Sat, Jan 11, 2014 at 08:48:12AM +1000, Peter Crosthwaite wrote:
 On Mon, Jan 6, 2014 at 4:12 PM, Stefan Hajnoczi stefa...@redhat.com wrote:
  On Mon, Jan 06, 2014 at 01:46:54PM +1000, Peter Crosthwaite wrote:
  On Mon, Jan 6, 2014 at 1:27 PM, Stefan Hajnoczi stefa...@redhat.com 
  wrote:
   On Thu, Jan 02, 2014 at 08:25:10PM +1000, Peter Crosthwaite wrote:
   Hi Beniamino,
  
   On Thu, Jan 2, 2014 at 7:18 PM, Beniamino Galvani b.galv...@gmail.com 
   wrote:
This patch adds support for the Fast Ethernet MAC found on Allwinner
SoCs, together with a basic emulation of Realtek RTL8201CP PHY.
   
  
   More a comment for net in general, but I think sooner or later we need
   to move towards a split between phy and mac on the device level.
   continuing the phy-within-mac philosophy is going to make the
   socification efforts awkward. Are MII and friends a busses (as in
   TYPE_BUS) in their own right, and connection of mac and phy has to
   happen on the board level?
  
   I see PHY and MAC split as advantageous because it allows code reuse and
   better testing.  The main thing I'd like to see is PHY device tests
   using tests/libqtest.h.
  
   If someone wants to implement it, great.  It would make it easier to add
   more NIC models in the future.
  
   Regarding SOCification and busses, I'm not sure.  Is it okay to just say
   a NIC has-a PHY (i.e. composition)?
  
 
  Generally speaking, in the (ARM) SoCification the MAC is part of the
  SoC which in the latest styling guidelines is a composite device. This
  composite is supposed to reflect the self contained SoC product which
  the PHY is usually not a part of. So we have two opposing compositions
  here:
 
  NIC = MAC + PHY
  SOC = CPUs + MAC + ...
 
  MAC can't be in both. So for SoCs the NIC concept needs to abandoned.
  After all the expansion of NIC as Network Interface Card is a little
  bit PCish. Your average SoC networking solution has no such card.
  Just an on chip MAC (same pacakge/die as CPU etc) connecting to a PHY
  via PCB traces.
 
  So I think long term, MII has to be a TYPE_BUS that is visible on the
  top level SoC device. Self contained NICs (as we know them today) are
  then also implementable as container devices (of MAC and PHY) that use
  this bus internally (in much the same way the SoC boards would attach
  external PHY to SoC).
 
  Okay, that makes sense.  Given the amount of emulated hardware in QEMU
  today, I think it would be okay to simply add new MAC/PHYs while still
  supporting the NICs of old.  If someone is enthusiastic about
  refactoring and testing existing NICs then great.  But I think it's more
  pragmatic to simply start working with a split MAC/PHY where that is
  beneficial.
 
 
 Alright,
 
 So lets make some plans. There is devil in the detail here. There was
 a previous attempt to do something similar by Grant early last year so
 cc as FYI.
 
 So the main question is whether or not this new interface is just for
 MDIO or is the full MII interface (both MDIO and packet data).
 
 My inclination is the latter, we want a new proper QOM bus that is
 both. What this would mean, is that these MAC-only devices wont be net
 devices at all. the -net args are instead applied to the PHY. This
 makes the most sense to me as its the phy that actually has copper
 connection to the external network, not MAC.
 
 MAC  TYPE_MII_BUS  PHY -net layer -- external
 network: -net foo,bar,baz

I don't really agree with this. You can do ethernet without a PHY,
it is in fact fairly common in the embedded switch space. You can also
have a single MDIO iface that is completely separate from any MAC take
care of many PHYs.

IMO, the MDIO bus should be separate from the data path.


 Another approach is to make both net devices in their own right. Phy
 has two net-layer-managed attachments, one for external network, and
 one point-to-point for the MII connecting to MAC. The MDIO bus is then
 a side channel which may or may not be QOMified (depending on effort
 levels). So you can still connect a standalone MAC to an external
 network, assuming the guest can handle no PHY (may in reality have
 limited use).
 
 MAC  net layer  PHY -net layer -- external network
  TYPE_MDIO_BUS 
 
 OR:
 
 MAC  net layer  external network
 
 
 The third approach (which is closest to current implementation) is to
 only have the phy do MDIO and still connect the MAC straight to an
 external network:
 
 MAC  net layer  external network
  \
   -- TYPE_MDIO_BUS  PHY
 
 I dont like this though, as its a little mismatched to real hw.
 Although it may be a good stepping stone to approaches 1 or 2.

I'd go for this third one and possibly do something about the
data path later if needed. Or possibly nr 2, not sure if I understood
that one correctly though.

Cheers,
Edgar



Re: [Qemu-devel] [PATCH 1/2] hw/net: add support for Allwinner EMAC Fast Ethernet controller

2014-01-12 Thread Peter Crosthwaite
On Sun, Jan 12, 2014 at 11:59 PM, Edgar E. Iglesias
edgar.igles...@gmail.com wrote:
 On Sat, Jan 11, 2014 at 08:48:12AM +1000, Peter Crosthwaite wrote:
 On Mon, Jan 6, 2014 at 4:12 PM, Stefan Hajnoczi stefa...@redhat.com wrote:
  On Mon, Jan 06, 2014 at 01:46:54PM +1000, Peter Crosthwaite wrote:
  On Mon, Jan 6, 2014 at 1:27 PM, Stefan Hajnoczi stefa...@redhat.com 
  wrote:
   On Thu, Jan 02, 2014 at 08:25:10PM +1000, Peter Crosthwaite wrote:
   Hi Beniamino,
  
   On Thu, Jan 2, 2014 at 7:18 PM, Beniamino Galvani 
   b.galv...@gmail.com wrote:
This patch adds support for the Fast Ethernet MAC found on Allwinner
SoCs, together with a basic emulation of Realtek RTL8201CP PHY.
   
  
   More a comment for net in general, but I think sooner or later we need
   to move towards a split between phy and mac on the device level.
   continuing the phy-within-mac philosophy is going to make the
   socification efforts awkward. Are MII and friends a busses (as in
   TYPE_BUS) in their own right, and connection of mac and phy has to
   happen on the board level?
  
   I see PHY and MAC split as advantageous because it allows code reuse and
   better testing.  The main thing I'd like to see is PHY device tests
   using tests/libqtest.h.
  
   If someone wants to implement it, great.  It would make it easier to add
   more NIC models in the future.
  
   Regarding SOCification and busses, I'm not sure.  Is it okay to just say
   a NIC has-a PHY (i.e. composition)?
  
 
  Generally speaking, in the (ARM) SoCification the MAC is part of the
  SoC which in the latest styling guidelines is a composite device. This
  composite is supposed to reflect the self contained SoC product which
  the PHY is usually not a part of. So we have two opposing compositions
  here:
 
  NIC = MAC + PHY
  SOC = CPUs + MAC + ...
 
  MAC can't be in both. So for SoCs the NIC concept needs to abandoned.
  After all the expansion of NIC as Network Interface Card is a little
  bit PCish. Your average SoC networking solution has no such card.
  Just an on chip MAC (same pacakge/die as CPU etc) connecting to a PHY
  via PCB traces.
 
  So I think long term, MII has to be a TYPE_BUS that is visible on the
  top level SoC device. Self contained NICs (as we know them today) are
  then also implementable as container devices (of MAC and PHY) that use
  this bus internally (in much the same way the SoC boards would attach
  external PHY to SoC).
 
  Okay, that makes sense.  Given the amount of emulated hardware in QEMU
  today, I think it would be okay to simply add new MAC/PHYs while still
  supporting the NICs of old.  If someone is enthusiastic about
  refactoring and testing existing NICs then great.  But I think it's more
  pragmatic to simply start working with a split MAC/PHY where that is
  beneficial.
 

 Alright,

 So lets make some plans. There is devil in the detail here. There was
 a previous attempt to do something similar by Grant early last year so
 cc as FYI.

 So the main question is whether or not this new interface is just for
 MDIO or is the full MII interface (both MDIO and packet data).

 My inclination is the latter, we want a new proper QOM bus that is
 both. What this would mean, is that these MAC-only devices wont be net
 devices at all. the -net args are instead applied to the PHY. This
 makes the most sense to me as its the phy that actually has copper
 connection to the external network, not MAC.

 MAC  TYPE_MII_BUS  PHY -net layer -- external
 network: -net foo,bar,baz

 I don't really agree with this. You can do ethernet without a PHY,
 it is in fact fairly common in the embedded switch space. You can also
 have a single MDIO iface that is completely separate from any MAC take
 care of many PHYs.

 IMO, the MDIO bus should be separate from the data path.


 Another approach is to make both net devices in their own right. Phy
 has two net-layer-managed attachments, one for external network, and
 one point-to-point for the MII connecting to MAC. The MDIO bus is then
 a side channel which may or may not be QOMified (depending on effort
 levels). So you can still connect a standalone MAC to an external
 network, assuming the guest can handle no PHY (may in reality have
 limited use).

 MAC  net layer  PHY -net layer -- external network
  TYPE_MDIO_BUS 

 OR:

 MAC  net layer  external network


 The third approach (which is closest to current implementation) is to
 only have the phy do MDIO and still connect the MAC straight to an
 external network:

 MAC  net layer  external network
  \
   -- TYPE_MDIO_BUS  PHY

 I dont like this though, as its a little mismatched to real hw.
 Although it may be a good stepping stone to approaches 1 or 2.

 I'd go for this third one and possibly do something about the
 data path later if needed.

Yeh, so I think that really translates to do option 3 and go to option
2 later if needed. So option 3 looks 

Re: [Qemu-devel] [PATCH 1/2] hw/net: add support for Allwinner EMAC Fast Ethernet controller

2014-01-12 Thread Stefan Hajnoczi
On Mon, Jan 13, 2014 at 08:00:37AM +1000, Peter Crosthwaite wrote:
 On Sun, Jan 12, 2014 at 11:59 PM, Edgar E. Iglesias
 edgar.igles...@gmail.com wrote:
  On Sat, Jan 11, 2014 at 08:48:12AM +1000, Peter Crosthwaite wrote:
  On Mon, Jan 6, 2014 at 4:12 PM, Stefan Hajnoczi stefa...@redhat.com 
  wrote:
   On Mon, Jan 06, 2014 at 01:46:54PM +1000, Peter Crosthwaite wrote:
   On Mon, Jan 6, 2014 at 1:27 PM, Stefan Hajnoczi stefa...@redhat.com 
   wrote:
On Thu, Jan 02, 2014 at 08:25:10PM +1000, Peter Crosthwaite wrote:
Hi Beniamino,
   
On Thu, Jan 2, 2014 at 7:18 PM, Beniamino Galvani 
b.galv...@gmail.com wrote:
 This patch adds support for the Fast Ethernet MAC found on 
 Allwinner
 SoCs, together with a basic emulation of Realtek RTL8201CP PHY.

   
More a comment for net in general, but I think sooner or later we 
need
to move towards a split between phy and mac on the device level.
continuing the phy-within-mac philosophy is going to make the
socification efforts awkward. Are MII and friends a busses (as in
TYPE_BUS) in their own right, and connection of mac and phy has to
happen on the board level?
   
I see PHY and MAC split as advantageous because it allows code reuse 
and
better testing.  The main thing I'd like to see is PHY device tests
using tests/libqtest.h.
   
If someone wants to implement it, great.  It would make it easier to 
add
more NIC models in the future.
   
Regarding SOCification and busses, I'm not sure.  Is it okay to just 
say
a NIC has-a PHY (i.e. composition)?
   
  
   Generally speaking, in the (ARM) SoCification the MAC is part of the
   SoC which in the latest styling guidelines is a composite device. This
   composite is supposed to reflect the self contained SoC product which
   the PHY is usually not a part of. So we have two opposing compositions
   here:
  
   NIC = MAC + PHY
   SOC = CPUs + MAC + ...
  
   MAC can't be in both. So for SoCs the NIC concept needs to abandoned.
   After all the expansion of NIC as Network Interface Card is a little
   bit PCish. Your average SoC networking solution has no such card.
   Just an on chip MAC (same pacakge/die as CPU etc) connecting to a PHY
   via PCB traces.
  
   So I think long term, MII has to be a TYPE_BUS that is visible on the
   top level SoC device. Self contained NICs (as we know them today) are
   then also implementable as container devices (of MAC and PHY) that use
   this bus internally (in much the same way the SoC boards would attach
   external PHY to SoC).
  
   Okay, that makes sense.  Given the amount of emulated hardware in QEMU
   today, I think it would be okay to simply add new MAC/PHYs while still
   supporting the NICs of old.  If someone is enthusiastic about
   refactoring and testing existing NICs then great.  But I think it's more
   pragmatic to simply start working with a split MAC/PHY where that is
   beneficial.
  
 
  Alright,
 
  So lets make some plans. There is devil in the detail here. There was
  a previous attempt to do something similar by Grant early last year so
  cc as FYI.
 
  So the main question is whether or not this new interface is just for
  MDIO or is the full MII interface (both MDIO and packet data).
 
  My inclination is the latter, we want a new proper QOM bus that is
  both. What this would mean, is that these MAC-only devices wont be net
  devices at all. the -net args are instead applied to the PHY. This
  makes the most sense to me as its the phy that actually has copper
  connection to the external network, not MAC.
 
  MAC  TYPE_MII_BUS  PHY -net layer -- external
  network: -net foo,bar,baz
 
  I don't really agree with this. You can do ethernet without a PHY,
  it is in fact fairly common in the embedded switch space. You can also
  have a single MDIO iface that is completely separate from any MAC take
  care of many PHYs.
 
  IMO, the MDIO bus should be separate from the data path.
 
 
  Another approach is to make both net devices in their own right. Phy
  has two net-layer-managed attachments, one for external network, and
  one point-to-point for the MII connecting to MAC. The MDIO bus is then
  a side channel which may or may not be QOMified (depending on effort
  levels). So you can still connect a standalone MAC to an external
  network, assuming the guest can handle no PHY (may in reality have
  limited use).
 
  MAC  net layer  PHY -net layer -- external network
   TYPE_MDIO_BUS 
 
  OR:
 
  MAC  net layer  external network
 
 
  The third approach (which is closest to current implementation) is to
  only have the phy do MDIO and still connect the MAC straight to an
  external network:
 
  MAC  net layer  external network
   \
-- TYPE_MDIO_BUS  PHY
 
  I dont like this though, as its a little mismatched to real hw.
  Although it may be a good stepping stone to 

Re: [Qemu-devel] [PATCH 1/2] hw/net: add support for Allwinner EMAC Fast Ethernet controller

2014-01-10 Thread Beniamino Galvani
On Mon, Jan 06, 2014 at 02:12:27PM +0800, Stefan Hajnoczi wrote:
   More a comment for net in general, but I think sooner or later we need
   to move towards a split between phy and mac on the device level.
   continuing the phy-within-mac philosophy is going to make the
   socification efforts awkward. Are MII and friends a busses (as in
   TYPE_BUS) in their own right, and connection of mac and phy has to
   happen on the board level?
  
   I see PHY and MAC split as advantageous because it allows code reuse and
   better testing.  The main thing I'd like to see is PHY device tests
   using tests/libqtest.h.
  
   If someone wants to implement it, great.  It would make it easier to add
   more NIC models in the future.
  
   Regarding SOCification and busses, I'm not sure.  Is it okay to just say
   a NIC has-a PHY (i.e. composition)?
  
  
  Generally speaking, in the (ARM) SoCification the MAC is part of the
  SoC which in the latest styling guidelines is a composite device. This
  composite is supposed to reflect the self contained SoC product which
  the PHY is usually not a part of. So we have two opposing compositions
  here:
  
  NIC = MAC + PHY
  SOC = CPUs + MAC + ...
  
  MAC can't be in both. So for SoCs the NIC concept needs to abandoned.
  After all the expansion of NIC as Network Interface Card is a little
  bit PCish. Your average SoC networking solution has no such card.
  Just an on chip MAC (same pacakge/die as CPU etc) connecting to a PHY
  via PCB traces.
  
  So I think long term, MII has to be a TYPE_BUS that is visible on the
  top level SoC device. Self contained NICs (as we know them today) are
  then also implementable as container devices (of MAC and PHY) that use
  this bus internally (in much the same way the SoC boards would attach
  external PHY to SoC).
 
 Okay, that makes sense.  Given the amount of emulated hardware in QEMU
 today, I think it would be okay to simply add new MAC/PHYs while still
 supporting the NICs of old.  If someone is enthusiastic about
 refactoring and testing existing NICs then great.  But I think it's more
 pragmatic to simply start working with a split MAC/PHY where that is
 beneficial.

Regarding the patch, can I resubmit it with MAC and PHY modeled as a
single device? Or it's better to start thinking on how to implement
proper MAC/PHY split?

Beniamino



Re: [Qemu-devel] [PATCH 1/2] hw/net: add support for Allwinner EMAC Fast Ethernet controller

2014-01-10 Thread Peter Crosthwaite
On Sat, Jan 11, 2014 at 7:48 AM, Beniamino Galvani b.galv...@gmail.com wrote:
 On Mon, Jan 06, 2014 at 02:12:27PM +0800, Stefan Hajnoczi wrote:
   More a comment for net in general, but I think sooner or later we need
   to move towards a split between phy and mac on the device level.
   continuing the phy-within-mac philosophy is going to make the
   socification efforts awkward. Are MII and friends a busses (as in
   TYPE_BUS) in their own right, and connection of mac and phy has to
   happen on the board level?
  
   I see PHY and MAC split as advantageous because it allows code reuse and
   better testing.  The main thing I'd like to see is PHY device tests
   using tests/libqtest.h.
  
   If someone wants to implement it, great.  It would make it easier to add
   more NIC models in the future.
  
   Regarding SOCification and busses, I'm not sure.  Is it okay to just say
   a NIC has-a PHY (i.e. composition)?
  
 
  Generally speaking, in the (ARM) SoCification the MAC is part of the
  SoC which in the latest styling guidelines is a composite device. This
  composite is supposed to reflect the self contained SoC product which
  the PHY is usually not a part of. So we have two opposing compositions
  here:
 
  NIC = MAC + PHY
  SOC = CPUs + MAC + ...
 
  MAC can't be in both. So for SoCs the NIC concept needs to abandoned.
  After all the expansion of NIC as Network Interface Card is a little
  bit PCish. Your average SoC networking solution has no such card.
  Just an on chip MAC (same pacakge/die as CPU etc) connecting to a PHY
  via PCB traces.
 
  So I think long term, MII has to be a TYPE_BUS that is visible on the
  top level SoC device. Self contained NICs (as we know them today) are
  then also implementable as container devices (of MAC and PHY) that use
  this bus internally (in much the same way the SoC boards would attach
  external PHY to SoC).

 Okay, that makes sense.  Given the amount of emulated hardware in QEMU
 today, I think it would be okay to simply add new MAC/PHYs while still
 supporting the NICs of old.  If someone is enthusiastic about
 refactoring and testing existing NICs then great.  But I think it's more
 pragmatic to simply start working with a split MAC/PHY where that is
 beneficial.

 Regarding the patch, can I resubmit it with MAC and PHY modeled as a
 single device? Or it's better to start thinking on how to implement
 proper MAC/PHY split?


Resubmit as a single. Don't wait on the proposed fifo cleanups either.
I'm not going to block.

Regards,
Peter

 Beniamino




Re: [Qemu-devel] [PATCH 1/2] hw/net: add support for Allwinner EMAC Fast Ethernet controller

2014-01-10 Thread Peter Crosthwaite
On Mon, Jan 6, 2014 at 4:12 PM, Stefan Hajnoczi stefa...@redhat.com wrote:
 On Mon, Jan 06, 2014 at 01:46:54PM +1000, Peter Crosthwaite wrote:
 On Mon, Jan 6, 2014 at 1:27 PM, Stefan Hajnoczi stefa...@redhat.com wrote:
  On Thu, Jan 02, 2014 at 08:25:10PM +1000, Peter Crosthwaite wrote:
  Hi Beniamino,
 
  On Thu, Jan 2, 2014 at 7:18 PM, Beniamino Galvani b.galv...@gmail.com 
  wrote:
   This patch adds support for the Fast Ethernet MAC found on Allwinner
   SoCs, together with a basic emulation of Realtek RTL8201CP PHY.
  
 
  More a comment for net in general, but I think sooner or later we need
  to move towards a split between phy and mac on the device level.
  continuing the phy-within-mac philosophy is going to make the
  socification efforts awkward. Are MII and friends a busses (as in
  TYPE_BUS) in their own right, and connection of mac and phy has to
  happen on the board level?
 
  I see PHY and MAC split as advantageous because it allows code reuse and
  better testing.  The main thing I'd like to see is PHY device tests
  using tests/libqtest.h.
 
  If someone wants to implement it, great.  It would make it easier to add
  more NIC models in the future.
 
  Regarding SOCification and busses, I'm not sure.  Is it okay to just say
  a NIC has-a PHY (i.e. composition)?
 

 Generally speaking, in the (ARM) SoCification the MAC is part of the
 SoC which in the latest styling guidelines is a composite device. This
 composite is supposed to reflect the self contained SoC product which
 the PHY is usually not a part of. So we have two opposing compositions
 here:

 NIC = MAC + PHY
 SOC = CPUs + MAC + ...

 MAC can't be in both. So for SoCs the NIC concept needs to abandoned.
 After all the expansion of NIC as Network Interface Card is a little
 bit PCish. Your average SoC networking solution has no such card.
 Just an on chip MAC (same pacakge/die as CPU etc) connecting to a PHY
 via PCB traces.

 So I think long term, MII has to be a TYPE_BUS that is visible on the
 top level SoC device. Self contained NICs (as we know them today) are
 then also implementable as container devices (of MAC and PHY) that use
 this bus internally (in much the same way the SoC boards would attach
 external PHY to SoC).

 Okay, that makes sense.  Given the amount of emulated hardware in QEMU
 today, I think it would be okay to simply add new MAC/PHYs while still
 supporting the NICs of old.  If someone is enthusiastic about
 refactoring and testing existing NICs then great.  But I think it's more
 pragmatic to simply start working with a split MAC/PHY where that is
 beneficial.


Alright,

So lets make some plans. There is devil in the detail here. There was
a previous attempt to do something similar by Grant early last year so
cc as FYI.

So the main question is whether or not this new interface is just for
MDIO or is the full MII interface (both MDIO and packet data).

My inclination is the latter, we want a new proper QOM bus that is
both. What this would mean, is that these MAC-only devices wont be net
devices at all. the -net args are instead applied to the PHY. This
makes the most sense to me as its the phy that actually has copper
connection to the external network, not MAC.

MAC  TYPE_MII_BUS  PHY -net layer -- external
network: -net foo,bar,baz

Another approach is to make both net devices in their own right. Phy
has two net-layer-managed attachments, one for external network, and
one point-to-point for the MII connecting to MAC. The MDIO bus is then
a side channel which may or may not be QOMified (depending on effort
levels). So you can still connect a standalone MAC to an external
network, assuming the guest can handle no PHY (may in reality have
limited use).

MAC  net layer  PHY -net layer -- external network
 TYPE_MDIO_BUS 

OR:

MAC  net layer  external network


The third approach (which is closest to current implementation) is to
only have the phy do MDIO and still connect the MAC straight to an
external network:

MAC  net layer  external network
 \
  -- TYPE_MDIO_BUS  PHY

I dont like this though, as its a little mismatched to real hw.
Although it may be a good stepping stone to approaches 1 or 2.

RFC

Regards,
Peter

 Stefan




Re: [Qemu-devel] [PATCH 1/2] hw/net: add support for Allwinner EMAC Fast Ethernet controller

2014-01-05 Thread Stefan Hajnoczi
On Thu, Jan 02, 2014 at 08:25:10PM +1000, Peter Crosthwaite wrote:
 Hi Beniamino,
 
 On Thu, Jan 2, 2014 at 7:18 PM, Beniamino Galvani b.galv...@gmail.com wrote:
  This patch adds support for the Fast Ethernet MAC found on Allwinner
  SoCs, together with a basic emulation of Realtek RTL8201CP PHY.
 
 
 More a comment for net in general, but I think sooner or later we need
 to move towards a split between phy and mac on the device level.
 continuing the phy-within-mac philosophy is going to make the
 socification efforts awkward. Are MII and friends a busses (as in
 TYPE_BUS) in their own right, and connection of mac and phy has to
 happen on the board level?

I see PHY and MAC split as advantageous because it allows code reuse and
better testing.  The main thing I'd like to see is PHY device tests
using tests/libqtest.h.

If someone wants to implement it, great.  It would make it easier to add
more NIC models in the future.

Regarding SOCification and busses, I'm not sure.  Is it okay to just say
a NIC has-a PHY (i.e. composition)?

Stefan



Re: [Qemu-devel] [PATCH 1/2] hw/net: add support for Allwinner EMAC Fast Ethernet controller

2014-01-05 Thread Peter Crosthwaite
On Mon, Jan 6, 2014 at 1:27 PM, Stefan Hajnoczi stefa...@redhat.com wrote:
 On Thu, Jan 02, 2014 at 08:25:10PM +1000, Peter Crosthwaite wrote:
 Hi Beniamino,

 On Thu, Jan 2, 2014 at 7:18 PM, Beniamino Galvani b.galv...@gmail.com 
 wrote:
  This patch adds support for the Fast Ethernet MAC found on Allwinner
  SoCs, together with a basic emulation of Realtek RTL8201CP PHY.
 

 More a comment for net in general, but I think sooner or later we need
 to move towards a split between phy and mac on the device level.
 continuing the phy-within-mac philosophy is going to make the
 socification efforts awkward. Are MII and friends a busses (as in
 TYPE_BUS) in their own right, and connection of mac and phy has to
 happen on the board level?

 I see PHY and MAC split as advantageous because it allows code reuse and
 better testing.  The main thing I'd like to see is PHY device tests
 using tests/libqtest.h.

 If someone wants to implement it, great.  It would make it easier to add
 more NIC models in the future.

 Regarding SOCification and busses, I'm not sure.  Is it okay to just say
 a NIC has-a PHY (i.e. composition)?


Generally speaking, in the (ARM) SoCification the MAC is part of the
SoC which in the latest styling guidelines is a composite device. This
composite is supposed to reflect the self contained SoC product which
the PHY is usually not a part of. So we have two opposing compositions
here:

NIC = MAC + PHY
SOC = CPUs + MAC + ...

MAC can't be in both. So for SoCs the NIC concept needs to abandoned.
After all the expansion of NIC as Network Interface Card is a little
bit PCish. Your average SoC networking solution has no such card.
Just an on chip MAC (same pacakge/die as CPU etc) connecting to a PHY
via PCB traces.

So I think long term, MII has to be a TYPE_BUS that is visible on the
top level SoC device. Self contained NICs (as we know them today) are
then also implementable as container devices (of MAC and PHY) that use
this bus internally (in much the same way the SoC boards would attach
external PHY to SoC).

Regards,
Peter

 Stefan




Re: [Qemu-devel] [PATCH 1/2] hw/net: add support for Allwinner EMAC Fast Ethernet controller

2014-01-05 Thread Stefan Hajnoczi
On Mon, Jan 06, 2014 at 01:46:54PM +1000, Peter Crosthwaite wrote:
 On Mon, Jan 6, 2014 at 1:27 PM, Stefan Hajnoczi stefa...@redhat.com wrote:
  On Thu, Jan 02, 2014 at 08:25:10PM +1000, Peter Crosthwaite wrote:
  Hi Beniamino,
 
  On Thu, Jan 2, 2014 at 7:18 PM, Beniamino Galvani b.galv...@gmail.com 
  wrote:
   This patch adds support for the Fast Ethernet MAC found on Allwinner
   SoCs, together with a basic emulation of Realtek RTL8201CP PHY.
  
 
  More a comment for net in general, but I think sooner or later we need
  to move towards a split between phy and mac on the device level.
  continuing the phy-within-mac philosophy is going to make the
  socification efforts awkward. Are MII and friends a busses (as in
  TYPE_BUS) in their own right, and connection of mac and phy has to
  happen on the board level?
 
  I see PHY and MAC split as advantageous because it allows code reuse and
  better testing.  The main thing I'd like to see is PHY device tests
  using tests/libqtest.h.
 
  If someone wants to implement it, great.  It would make it easier to add
  more NIC models in the future.
 
  Regarding SOCification and busses, I'm not sure.  Is it okay to just say
  a NIC has-a PHY (i.e. composition)?
 
 
 Generally speaking, in the (ARM) SoCification the MAC is part of the
 SoC which in the latest styling guidelines is a composite device. This
 composite is supposed to reflect the self contained SoC product which
 the PHY is usually not a part of. So we have two opposing compositions
 here:
 
 NIC = MAC + PHY
 SOC = CPUs + MAC + ...
 
 MAC can't be in both. So for SoCs the NIC concept needs to abandoned.
 After all the expansion of NIC as Network Interface Card is a little
 bit PCish. Your average SoC networking solution has no such card.
 Just an on chip MAC (same pacakge/die as CPU etc) connecting to a PHY
 via PCB traces.
 
 So I think long term, MII has to be a TYPE_BUS that is visible on the
 top level SoC device. Self contained NICs (as we know them today) are
 then also implementable as container devices (of MAC and PHY) that use
 this bus internally (in much the same way the SoC boards would attach
 external PHY to SoC).

Okay, that makes sense.  Given the amount of emulated hardware in QEMU
today, I think it would be okay to simply add new MAC/PHYs while still
supporting the NICs of old.  If someone is enthusiastic about
refactoring and testing existing NICs then great.  But I think it's more
pragmatic to simply start working with a split MAC/PHY where that is
beneficial.

Stefan



Re: [Qemu-devel] [PATCH 1/2] hw/net: add support for Allwinner EMAC Fast Ethernet controller

2014-01-04 Thread Beniamino Galvani
On Sat, Jan 04, 2014 at 10:56:13AM +1000, Peter Crosthwaite wrote:
 On Thu, Jan 2, 2014 at 7:18 PM, Beniamino Galvani b.galv...@gmail.com wrote:
  This patch adds support for the Fast Ethernet MAC found on Allwinner
  SoCs, together with a basic emulation of Realtek RTL8201CP PHY.
 
  Since there is no public documentation of the Allwinner controller, the
  implementation is based on Linux kernel driver.
 
  Signed-off-by: Beniamino Galvani b.galv...@gmail.com
  ---
   default-configs/arm-softmmu.mak |1 +
   hw/net/Makefile.objs|1 +
   hw/net/allwinner_emac.c |  466 
  +++
   include/hw/net/allwinner_emac.h |  204 +
   4 files changed, 672 insertions(+)
   create mode 100644 hw/net/allwinner_emac.c
   create mode 100644 include/hw/net/allwinner_emac.h
 
  diff --git a/default-configs/arm-softmmu.mak 
  b/default-configs/arm-softmmu.mak
  index ce1d620..f3513fa 100644
  --- a/default-configs/arm-softmmu.mak
  +++ b/default-configs/arm-softmmu.mak
  @@ -27,6 +27,7 @@ CONFIG_SSI_SD=y
   CONFIG_SSI_M25P80=y
   CONFIG_LAN9118=y
   CONFIG_SMC91C111=y
  +CONFIG_ALLWINNER_EMAC=y
   CONFIG_DS1338=y
   CONFIG_PFLASH_CFI01=y
   CONFIG_PFLASH_CFI02=y
  diff --git a/hw/net/Makefile.objs b/hw/net/Makefile.objs
  index 951cca3..75e80c2 100644
  --- a/hw/net/Makefile.objs
  +++ b/hw/net/Makefile.objs
  @@ -18,6 +18,7 @@ common-obj-$(CONFIG_OPENCORES_ETH) += opencores_eth.o
   common-obj-$(CONFIG_XGMAC) += xgmac.o
   common-obj-$(CONFIG_MIPSNET) += mipsnet.o
   common-obj-$(CONFIG_XILINX_AXI) += xilinx_axienet.o
  +common-obj-$(CONFIG_ALLWINNER_EMAC) += allwinner_emac.o
 
   common-obj-$(CONFIG_CADENCE) += cadence_gem.o
   common-obj-$(CONFIG_STELLARIS_ENET) += stellaris_enet.o
  diff --git a/hw/net/allwinner_emac.c b/hw/net/allwinner_emac.c
  new file mode 100644
  index 000..9791be4
  --- /dev/null
  +++ b/hw/net/allwinner_emac.c
  @@ -0,0 +1,466 @@
  +/*
  + * Emulation of Allwinner EMAC Fast Ethernet controller and
  + * Realtek RTL8201CP PHY
  + *
  + * Copyright (C) 2013 Beniamino Galvani b.galv...@gmail.com
  + *
  + * This program is free software; you can redistribute it and/or modify
  + * it under the terms of the GNU General Public License as published by
  + * the Free Software Foundation; either version 2 of the License, or
  + * (at your option) any later version.
  + *
  + * This program is distributed in the hope that it will be useful,
  + * but WITHOUT ANY WARRANTY; without even the implied warranty of
  + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
  + * GNU General Public License for more details.
  + *
  + */
  +#include hw/sysbus.h
  +#include net/net.h
  +#include hw/net/allwinner_emac.h
  +#include zlib.h
  +
  +#undef AW_EMAC_DEBUG
  +
  +#ifdef AW_EMAC_DEBUG
  +#define debug(...)  \
  +do {\
  +fprintf(stderr,  allwinner_emac : %s: , __func__);\
  +fprintf(stderr, ## __VA_ARGS__);\
  +} while (0)
  +#else
  +#define debug(...) do {} while (0)
  +#endif
  +
  +static void mii_set_link(AwEmacMii *mii, bool link_ok)
  +{
  +if (link_ok) {
  +mii-bmsr |= MII_BMSR_LINK_ST;
  +mii-anlpar |= MII_ANAR_TXFD | MII_ANAR_TX | MII_ANAR_10FD |
  +   MII_ANAR_10 | MII_ANAR_CSMACD;
  +} else {
  +mii-bmsr = ~MII_BMSR_LINK_ST;
  +mii-anlpar = MII_ANAR_TX;
  +}
  +mii-link_ok = link_ok;
  +}
  +
  +static void mii_reset(AwEmacMii *mii)
  +{
  +mii-bmcr = MII_BMCR_FD | MII_BMCR_AUTOEN | MII_BMCR_SPEED;
  +mii-bmsr = MII_BMSR_100TX_FD | MII_BMSR_100TX_HD | MII_BMSR_10T_FD |
  +MII_BMSR_10T_HD | MII_BMSR_MFPS | MII_BMSR_AUTONEG;
  +mii-anar = MII_ANAR_TXFD | MII_ANAR_TX | MII_ANAR_10FD | MII_ANAR_10 |
  +MII_ANAR_CSMACD;
  +mii-anlpar = MII_ANAR_TX;
  +mii_set_link(mii, mii-link_ok);
  +}
  +
  +static uint16_t mii_read(AwEmacMii *mii, uint8_t phy_addr, uint8_t reg)
  +{
  +uint16_t ret = 0x;
  +
  +if (phy_addr == BOARD_PHY_ADDRESS) {
  +switch (reg) {
  +case MII_BMCR:
  +ret = mii-bmcr;
  +break;
  +case MII_BMSR:
  +ret = mii-bmsr;
  +break;
  +case MII_PHYID1:
  +ret = RTL8201CP_PHYID1;
  +break;
  +case MII_PHYID2:
  +ret = RTL8201CP_PHYID2;
  +break;
  +case MII_ANAR:
  +ret = mii-anar;
  +break;
  +case MII_ANLPAR:
  +ret = mii-anlpar;
  +break;
  +default:
  +debug(unknown mii register %x\n, reg);
  +ret = 0;
  +}
  +}
  +return ret;
  +}
  +
  +static void mii_write(AwEmacMii *mii, uint8_t phy_addr, uint8_t reg,
  +  uint16_t value)
  +{
  +if (phy_addr == 

Re: [Qemu-devel] [PATCH 1/2] hw/net: add support for Allwinner EMAC Fast Ethernet controller

2014-01-03 Thread Beniamino Galvani
On Fri, Jan 03, 2014 at 11:26:01AM +1000, Peter Crosthwaite wrote:
   +static const VMStateDescription vmstate_tx_fifo = {
   +.name = allwinner_emac_tx_fifo,
   +.version_id = 1,
   +.minimum_version_id = 1,
   +.fields = (VMStateField[]) {
   +VMSTATE_UINT32_ARRAY(data, AwEmacTxFifo, FIFO_SIZE  2),
   +VMSTATE_INT32(length, AwEmacTxFifo),
   +VMSTATE_INT32(offset, AwEmacTxFifo),
   +VMSTATE_END_OF_LIST()
   +}
   +};
 
  This might warrant a dup of fifo8 as fifo32 if you want to clean this
  up. I think I have a patch handy that might help, will send tmrw.
  Check util/fifo8.c for fifo8 example.
 
  Yes, probably AwEmacTxFifo can be replaced with Fifo32.
 

I need to obtain a pointer to the fifo backing buffer after the frame
has been copied completely in order to pass it to qemu_send_packet().
The generic fifo implementation doesn't allow that (unless I access
directly the structure member, but I suppose that the intent is to
treat the fifo object as opaque).

Maybe a function fifo_get_data() could be added to the generic
implementation? Otherwise I can go on using my custom implementation.

Beniamino



Re: [Qemu-devel] [PATCH 1/2] hw/net: add support for Allwinner EMAC Fast Ethernet controller

2014-01-03 Thread Peter Crosthwaite
On Thu, Jan 2, 2014 at 7:18 PM, Beniamino Galvani b.galv...@gmail.com wrote:
 This patch adds support for the Fast Ethernet MAC found on Allwinner
 SoCs, together with a basic emulation of Realtek RTL8201CP PHY.

 Since there is no public documentation of the Allwinner controller, the
 implementation is based on Linux kernel driver.

 Signed-off-by: Beniamino Galvani b.galv...@gmail.com
 ---
  default-configs/arm-softmmu.mak |1 +
  hw/net/Makefile.objs|1 +
  hw/net/allwinner_emac.c |  466 
 +++
  include/hw/net/allwinner_emac.h |  204 +
  4 files changed, 672 insertions(+)
  create mode 100644 hw/net/allwinner_emac.c
  create mode 100644 include/hw/net/allwinner_emac.h

 diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
 index ce1d620..f3513fa 100644
 --- a/default-configs/arm-softmmu.mak
 +++ b/default-configs/arm-softmmu.mak
 @@ -27,6 +27,7 @@ CONFIG_SSI_SD=y
  CONFIG_SSI_M25P80=y
  CONFIG_LAN9118=y
  CONFIG_SMC91C111=y
 +CONFIG_ALLWINNER_EMAC=y
  CONFIG_DS1338=y
  CONFIG_PFLASH_CFI01=y
  CONFIG_PFLASH_CFI02=y
 diff --git a/hw/net/Makefile.objs b/hw/net/Makefile.objs
 index 951cca3..75e80c2 100644
 --- a/hw/net/Makefile.objs
 +++ b/hw/net/Makefile.objs
 @@ -18,6 +18,7 @@ common-obj-$(CONFIG_OPENCORES_ETH) += opencores_eth.o
  common-obj-$(CONFIG_XGMAC) += xgmac.o
  common-obj-$(CONFIG_MIPSNET) += mipsnet.o
  common-obj-$(CONFIG_XILINX_AXI) += xilinx_axienet.o
 +common-obj-$(CONFIG_ALLWINNER_EMAC) += allwinner_emac.o

  common-obj-$(CONFIG_CADENCE) += cadence_gem.o
  common-obj-$(CONFIG_STELLARIS_ENET) += stellaris_enet.o
 diff --git a/hw/net/allwinner_emac.c b/hw/net/allwinner_emac.c
 new file mode 100644
 index 000..9791be4
 --- /dev/null
 +++ b/hw/net/allwinner_emac.c
 @@ -0,0 +1,466 @@
 +/*
 + * Emulation of Allwinner EMAC Fast Ethernet controller and
 + * Realtek RTL8201CP PHY
 + *
 + * Copyright (C) 2013 Beniamino Galvani b.galv...@gmail.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 + * GNU General Public License for more details.
 + *
 + */
 +#include hw/sysbus.h
 +#include net/net.h
 +#include hw/net/allwinner_emac.h
 +#include zlib.h
 +
 +#undef AW_EMAC_DEBUG
 +
 +#ifdef AW_EMAC_DEBUG
 +#define debug(...)  \
 +do {\
 +fprintf(stderr,  allwinner_emac : %s: , __func__);\
 +fprintf(stderr, ## __VA_ARGS__);\
 +} while (0)
 +#else
 +#define debug(...) do {} while (0)
 +#endif
 +
 +static void mii_set_link(AwEmacMii *mii, bool link_ok)
 +{
 +if (link_ok) {
 +mii-bmsr |= MII_BMSR_LINK_ST;
 +mii-anlpar |= MII_ANAR_TXFD | MII_ANAR_TX | MII_ANAR_10FD |
 +   MII_ANAR_10 | MII_ANAR_CSMACD;
 +} else {
 +mii-bmsr = ~MII_BMSR_LINK_ST;
 +mii-anlpar = MII_ANAR_TX;
 +}
 +mii-link_ok = link_ok;
 +}
 +
 +static void mii_reset(AwEmacMii *mii)
 +{
 +mii-bmcr = MII_BMCR_FD | MII_BMCR_AUTOEN | MII_BMCR_SPEED;
 +mii-bmsr = MII_BMSR_100TX_FD | MII_BMSR_100TX_HD | MII_BMSR_10T_FD |
 +MII_BMSR_10T_HD | MII_BMSR_MFPS | MII_BMSR_AUTONEG;
 +mii-anar = MII_ANAR_TXFD | MII_ANAR_TX | MII_ANAR_10FD | MII_ANAR_10 |
 +MII_ANAR_CSMACD;
 +mii-anlpar = MII_ANAR_TX;
 +mii_set_link(mii, mii-link_ok);
 +}
 +
 +static uint16_t mii_read(AwEmacMii *mii, uint8_t phy_addr, uint8_t reg)
 +{
 +uint16_t ret = 0x;
 +
 +if (phy_addr == BOARD_PHY_ADDRESS) {
 +switch (reg) {
 +case MII_BMCR:
 +ret = mii-bmcr;
 +break;
 +case MII_BMSR:
 +ret = mii-bmsr;
 +break;
 +case MII_PHYID1:
 +ret = RTL8201CP_PHYID1;
 +break;
 +case MII_PHYID2:
 +ret = RTL8201CP_PHYID2;
 +break;
 +case MII_ANAR:
 +ret = mii-anar;
 +break;
 +case MII_ANLPAR:
 +ret = mii-anlpar;
 +break;
 +default:
 +debug(unknown mii register %x\n, reg);
 +ret = 0;
 +}
 +}
 +return ret;
 +}
 +
 +static void mii_write(AwEmacMii *mii, uint8_t phy_addr, uint8_t reg,
 +  uint16_t value)
 +{
 +if (phy_addr == BOARD_PHY_ADDRESS) {
 +switch (reg) {
 +case MII_BMCR:
 +if (value  MII_BMCR_RESET) {
 +mii_reset(mii);
 +} else {
 +mii-bmcr = value;
 +   

Re: [Qemu-devel] [PATCH 1/2] hw/net: add support for Allwinner EMAC Fast Ethernet controller

2014-01-02 Thread Peter Crosthwaite
Hi Beniamino,

On Thu, Jan 2, 2014 at 7:18 PM, Beniamino Galvani b.galv...@gmail.com wrote:
 This patch adds support for the Fast Ethernet MAC found on Allwinner
 SoCs, together with a basic emulation of Realtek RTL8201CP PHY.


More a comment for net in general, but I think sooner or later we need
to move towards a split between phy and mac on the device level.
continuing the phy-within-mac philosophy is going to make the
socification efforts awkward. Are MII and friends a busses (as in
TYPE_BUS) in their own right, and connection of mac and phy has to
happen on the board level?

 Since there is no public documentation of the Allwinner controller, the
 implementation is based on Linux kernel driver.

 Signed-off-by: Beniamino Galvani b.galv...@gmail.com
 ---
  default-configs/arm-softmmu.mak |1 +
  hw/net/Makefile.objs|1 +
  hw/net/allwinner_emac.c |  466 
 +++
  include/hw/net/allwinner_emac.h |  204 +
  4 files changed, 672 insertions(+)
  create mode 100644 hw/net/allwinner_emac.c
  create mode 100644 include/hw/net/allwinner_emac.h

 diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
 index ce1d620..f3513fa 100644
 --- a/default-configs/arm-softmmu.mak
 +++ b/default-configs/arm-softmmu.mak
 @@ -27,6 +27,7 @@ CONFIG_SSI_SD=y
  CONFIG_SSI_M25P80=y
  CONFIG_LAN9118=y
  CONFIG_SMC91C111=y
 +CONFIG_ALLWINNER_EMAC=y
  CONFIG_DS1338=y
  CONFIG_PFLASH_CFI01=y
  CONFIG_PFLASH_CFI02=y
 diff --git a/hw/net/Makefile.objs b/hw/net/Makefile.objs
 index 951cca3..75e80c2 100644
 --- a/hw/net/Makefile.objs
 +++ b/hw/net/Makefile.objs
 @@ -18,6 +18,7 @@ common-obj-$(CONFIG_OPENCORES_ETH) += opencores_eth.o
  common-obj-$(CONFIG_XGMAC) += xgmac.o
  common-obj-$(CONFIG_MIPSNET) += mipsnet.o
  common-obj-$(CONFIG_XILINX_AXI) += xilinx_axienet.o
 +common-obj-$(CONFIG_ALLWINNER_EMAC) += allwinner_emac.o

  common-obj-$(CONFIG_CADENCE) += cadence_gem.o
  common-obj-$(CONFIG_STELLARIS_ENET) += stellaris_enet.o
 diff --git a/hw/net/allwinner_emac.c b/hw/net/allwinner_emac.c
 new file mode 100644
 index 000..9791be4
 --- /dev/null
 +++ b/hw/net/allwinner_emac.c
 @@ -0,0 +1,466 @@
 +/*
 + * Emulation of Allwinner EMAC Fast Ethernet controller and
 + * Realtek RTL8201CP PHY
 + *
 + * Copyright (C) 2013 Beniamino Galvani b.galv...@gmail.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 + * GNU General Public License for more details.
 + *
 + */
 +#include hw/sysbus.h
 +#include net/net.h
 +#include hw/net/allwinner_emac.h
 +#include zlib.h
 +
 +#undef AW_EMAC_DEBUG
 +
 +#ifdef AW_EMAC_DEBUG
 +#define debug(...)  \
 +do {\
 +fprintf(stderr,  allwinner_emac : %s: , __func__);\
 +fprintf(stderr, ## __VA_ARGS__);\
 +} while (0)
 +#else
 +#define debug(...) do {} while (0)
 +#endif

For debug macros, its better to use a regular if (DEBUG_SYMBOL) so the
body of the macro always gets compile tested. We have had incidences
where major change patterns break stripped debug instrumentation
because the code is compiled out.

 +
 +static void mii_set_link(AwEmacMii *mii, bool link_ok)
 +{
 +if (link_ok) {
 +mii-bmsr |= MII_BMSR_LINK_ST;
 +mii-anlpar |= MII_ANAR_TXFD | MII_ANAR_TX | MII_ANAR_10FD |
 +   MII_ANAR_10 | MII_ANAR_CSMACD;
 +} else {
 +mii-bmsr = ~MII_BMSR_LINK_ST;
 +mii-anlpar = MII_ANAR_TX;
 +}
 +mii-link_ok = link_ok;
 +}
 +
 +static void mii_reset(AwEmacMii *mii)
 +{
 +mii-bmcr = MII_BMCR_FD | MII_BMCR_AUTOEN | MII_BMCR_SPEED;
 +mii-bmsr = MII_BMSR_100TX_FD | MII_BMSR_100TX_HD | MII_BMSR_10T_FD |
 +MII_BMSR_10T_HD | MII_BMSR_MFPS | MII_BMSR_AUTONEG;
 +mii-anar = MII_ANAR_TXFD | MII_ANAR_TX | MII_ANAR_10FD | MII_ANAR_10 |
 +MII_ANAR_CSMACD;
 +mii-anlpar = MII_ANAR_TX;
 +mii_set_link(mii, mii-link_ok);
 +}
 +
 +static uint16_t mii_read(AwEmacMii *mii, uint8_t phy_addr, uint8_t reg)
 +{
 +uint16_t ret = 0x;
 +
 +if (phy_addr == BOARD_PHY_ADDRESS) {
 +switch (reg) {
 +case MII_BMCR:
 +ret = mii-bmcr;
 +break;
 +case MII_BMSR:
 +ret = mii-bmsr;
 +break;
 +case MII_PHYID1:
 +ret = RTL8201CP_PHYID1;
 +break;
 +case MII_PHYID2:
 +ret = RTL8201CP_PHYID2;
 +break;
 +case MII_ANAR:
 +  

Re: [Qemu-devel] [PATCH 1/2] hw/net: add support for Allwinner EMAC Fast Ethernet controller

2014-01-02 Thread Beniamino Galvani
On Thu, Jan 02, 2014 at 08:25:10PM +1000, Peter Crosthwaite wrote:
  +#undef AW_EMAC_DEBUG
  +
  +#ifdef AW_EMAC_DEBUG
  +#define debug(...)  \
  +do {\
  +fprintf(stderr,  allwinner_emac : %s: , __func__);\
  +fprintf(stderr, ## __VA_ARGS__);\
  +} while (0)
  +#else
  +#define debug(...) do {} while (0)
  +#endif
 
 For debug macros, its better to use a regular if (DEBUG_SYMBOL) so the
 body of the macro always gets compile tested. We have had incidences
 where major change patterns break stripped debug instrumentation
 because the code is compiled out.

Ok.

 
  +
  +static void mii_set_link(AwEmacMii *mii, bool link_ok)
  +{
  +if (link_ok) {
  +mii-bmsr |= MII_BMSR_LINK_ST;
  +mii-anlpar |= MII_ANAR_TXFD | MII_ANAR_TX | MII_ANAR_10FD |
  +   MII_ANAR_10 | MII_ANAR_CSMACD;
  +} else {
  +mii-bmsr = ~MII_BMSR_LINK_ST;
  +mii-anlpar = MII_ANAR_TX;
  +}
  +mii-link_ok = link_ok;
  +}
  +
  +static void mii_reset(AwEmacMii *mii)
  +{
  +mii-bmcr = MII_BMCR_FD | MII_BMCR_AUTOEN | MII_BMCR_SPEED;
  +mii-bmsr = MII_BMSR_100TX_FD | MII_BMSR_100TX_HD | MII_BMSR_10T_FD |
  +MII_BMSR_10T_HD | MII_BMSR_MFPS | MII_BMSR_AUTONEG;
  +mii-anar = MII_ANAR_TXFD | MII_ANAR_TX | MII_ANAR_10FD | MII_ANAR_10 |
  +MII_ANAR_CSMACD;
  +mii-anlpar = MII_ANAR_TX;
  +mii_set_link(mii, mii-link_ok);
  +}
  +
  +static uint16_t mii_read(AwEmacMii *mii, uint8_t phy_addr, uint8_t reg)
  +{
  +uint16_t ret = 0x;
  +
  +if (phy_addr == BOARD_PHY_ADDRESS) {
  +switch (reg) {
  +case MII_BMCR:
  +ret = mii-bmcr;
  +break;
  +case MII_BMSR:
  +ret = mii-bmsr;
  +break;
  +case MII_PHYID1:
  +ret = RTL8201CP_PHYID1;
  +break;
  +case MII_PHYID2:
  +ret = RTL8201CP_PHYID2;
  +break;
  +case MII_ANAR:
  +ret = mii-anar;
  +break;
  +case MII_ANLPAR:
  +ret = mii-anlpar;
  +break;
  +default:
  +debug(unknown mii register %x\n, reg);
  +ret = 0;
  +}
  +}
  +return ret;
  +}
  +
  +static void mii_write(AwEmacMii *mii, uint8_t phy_addr, uint8_t reg,
  +  uint16_t value)
  +{
  +if (phy_addr == BOARD_PHY_ADDRESS) {
  +switch (reg) {
  +case MII_BMCR:
  +if (value  MII_BMCR_RESET) {
  +mii_reset(mii);
  +} else {
  +mii-bmcr = value;
  +}
  +break;
  +case MII_BMSR:
  +case MII_PHYID1:
  +case MII_PHYID2:
  +case MII_ANLPAR:
  +qemu_log_mask(LOG_GUEST_ERROR, %s: write to mii register 
  %x\n,
  +  __func__, reg);
  +break;
  +case MII_ANAR:
  +mii-anar = value;
  +break;
  +default:
  +debug(unknown mii register %x\n, reg);
  +}
  +}
  +}
  +
  +static void aw_emac_update_irq(AwEmacState *s)
  +{
  +qemu_set_irq(s-irq, (s-int_sta  s-int_ctl) != 0);
  +}
  +
  +static int aw_emac_can_receive(NetClientState *nc)
  +{
  +AwEmacState *s = qemu_get_nic_opaque(nc);
  +
  +return (s-ctl  EMAC_CTL_RX_EN)  (s-num_rx  MAX_RX);
 
 If you return false from a can_recieve(), you need to explictly flush
 queued packets (qemu_flush_queued_packets()) when the blocking
 condition is lifted, heres a good example a bugfix patch addressing
 this issue for another mac:
 
 http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg02255.html

Ok.

  +}
  +
  +static ssize_t aw_emac_receive(NetClientState *nc, const uint8_t *buf,
  +   size_t size)
  +{
  +AwEmacState *s = qemu_get_nic_opaque(nc);
  +uint32_t *fifo;
  +uint32_t crc;
  +char *dest;
  +
  +if (s-num_rx = MAX_RX) {
  +debug(rx queue full - packed dropped\n);
  +return -1;
  +}
  +
  +if (size + RX_HDR_SIZE  FIFO_SIZE) {
  +debug(packet too big\n);
  +return -1;
  +}
  +
  +fifo = s-rx_fifos[(s-first_rx + s-num_rx) % MAX_RX];
  +dest = (char *)fifo[2];
  +s-num_rx++;
  +
  +memcpy(dest, buf, size);
  +
  +/* Fill to minimum frame length */
  +if (size  60) {
  +memset(dest + size, 0, 60 - size);
  +size = 60;
  +}
  +
  +/* Append FCS */
  +crc = crc32(~0, buf, size);
  +memcpy(dest + size, crc, 4);
  +
  +fifo[0] = EMAC_UNDOCUMENTED_MAGIC;
  +fifo[1] = EMAC_RX_HEADER(size + 4, EMAC_RX_IO_DATA_STATUS_OK);
  +
  +/* Set rx interrupt flag */
  +s-int_sta |= EMAC_INT_RX;
  +aw_emac_update_irq(s);
  +
  +return size;
  +}
  +
  +static void 

Re: [Qemu-devel] [PATCH 1/2] hw/net: add support for Allwinner EMAC Fast Ethernet controller

2014-01-02 Thread Peter Crosthwaite
On Fri, Jan 3, 2014 at 12:58 AM, Beniamino Galvani b.galv...@gmail.com wrote:
 On Thu, Jan 02, 2014 at 08:25:10PM +1000, Peter Crosthwaite wrote:
  +#undef AW_EMAC_DEBUG
  +
  +#ifdef AW_EMAC_DEBUG
  +#define debug(...)  \
  +do {\
  +fprintf(stderr,  allwinner_emac : %s: , __func__);\
  +fprintf(stderr, ## __VA_ARGS__);\
  +} while (0)
  +#else
  +#define debug(...) do {} while (0)
  +#endif

 For debug macros, its better to use a regular if (DEBUG_SYMBOL) so the
 body of the macro always gets compile tested. We have had incidences
 where major change patterns break stripped debug instrumentation
 because the code is compiled out.

 Ok.


  +
  +static void mii_set_link(AwEmacMii *mii, bool link_ok)
  +{
  +if (link_ok) {
  +mii-bmsr |= MII_BMSR_LINK_ST;
  +mii-anlpar |= MII_ANAR_TXFD | MII_ANAR_TX | MII_ANAR_10FD |
  +   MII_ANAR_10 | MII_ANAR_CSMACD;
  +} else {
  +mii-bmsr = ~MII_BMSR_LINK_ST;
  +mii-anlpar = MII_ANAR_TX;
  +}
  +mii-link_ok = link_ok;
  +}
  +
  +static void mii_reset(AwEmacMii *mii)
  +{
  +mii-bmcr = MII_BMCR_FD | MII_BMCR_AUTOEN | MII_BMCR_SPEED;
  +mii-bmsr = MII_BMSR_100TX_FD | MII_BMSR_100TX_HD | MII_BMSR_10T_FD |
  +MII_BMSR_10T_HD | MII_BMSR_MFPS | MII_BMSR_AUTONEG;
  +mii-anar = MII_ANAR_TXFD | MII_ANAR_TX | MII_ANAR_10FD | MII_ANAR_10 
  |
  +MII_ANAR_CSMACD;
  +mii-anlpar = MII_ANAR_TX;
  +mii_set_link(mii, mii-link_ok);
  +}
  +
  +static uint16_t mii_read(AwEmacMii *mii, uint8_t phy_addr, uint8_t reg)
  +{
  +uint16_t ret = 0x;
  +
  +if (phy_addr == BOARD_PHY_ADDRESS) {
  +switch (reg) {
  +case MII_BMCR:
  +ret = mii-bmcr;
  +break;
  +case MII_BMSR:
  +ret = mii-bmsr;
  +break;
  +case MII_PHYID1:
  +ret = RTL8201CP_PHYID1;
  +break;
  +case MII_PHYID2:
  +ret = RTL8201CP_PHYID2;
  +break;
  +case MII_ANAR:
  +ret = mii-anar;
  +break;
  +case MII_ANLPAR:
  +ret = mii-anlpar;
  +break;
  +default:
  +debug(unknown mii register %x\n, reg);
  +ret = 0;
  +}
  +}
  +return ret;
  +}
  +
  +static void mii_write(AwEmacMii *mii, uint8_t phy_addr, uint8_t reg,
  +  uint16_t value)
  +{
  +if (phy_addr == BOARD_PHY_ADDRESS) {
  +switch (reg) {
  +case MII_BMCR:
  +if (value  MII_BMCR_RESET) {
  +mii_reset(mii);
  +} else {
  +mii-bmcr = value;
  +}
  +break;
  +case MII_BMSR:
  +case MII_PHYID1:
  +case MII_PHYID2:
  +case MII_ANLPAR:
  +qemu_log_mask(LOG_GUEST_ERROR, %s: write to mii register 
  %x\n,
  +  __func__, reg);
  +break;
  +case MII_ANAR:
  +mii-anar = value;
  +break;
  +default:
  +debug(unknown mii register %x\n, reg);
  +}
  +}
  +}
  +
  +static void aw_emac_update_irq(AwEmacState *s)
  +{
  +qemu_set_irq(s-irq, (s-int_sta  s-int_ctl) != 0);
  +}
  +
  +static int aw_emac_can_receive(NetClientState *nc)
  +{
  +AwEmacState *s = qemu_get_nic_opaque(nc);
  +
  +return (s-ctl  EMAC_CTL_RX_EN)  (s-num_rx  MAX_RX);

 If you return false from a can_recieve(), you need to explictly flush
 queued packets (qemu_flush_queued_packets()) when the blocking
 condition is lifted, heres a good example a bugfix patch addressing
 this issue for another mac:

 http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg02255.html

 Ok.

  +}
  +
  +static ssize_t aw_emac_receive(NetClientState *nc, const uint8_t *buf,
  +   size_t size)
  +{
  +AwEmacState *s = qemu_get_nic_opaque(nc);
  +uint32_t *fifo;
  +uint32_t crc;
  +char *dest;
  +
  +if (s-num_rx = MAX_RX) {
  +debug(rx queue full - packed dropped\n);
  +return -1;
  +}
  +
  +if (size + RX_HDR_SIZE  FIFO_SIZE) {
  +debug(packet too big\n);
  +return -1;
  +}
  +
  +fifo = s-rx_fifos[(s-first_rx + s-num_rx) % MAX_RX];
  +dest = (char *)fifo[2];
  +s-num_rx++;
  +
  +memcpy(dest, buf, size);
  +
  +/* Fill to minimum frame length */
  +if (size  60) {
  +memset(dest + size, 0, 60 - size);
  +size = 60;
  +}
  +
  +/* Append FCS */
  +crc = crc32(~0, buf, size);
  +memcpy(dest + size, crc, 4);
  +
  +fifo[0] = EMAC_UNDOCUMENTED_MAGIC;
  +fifo[1] = EMAC_RX_HEADER(size + 4, EMAC_RX_IO_DATA_STATUS_OK);
  +
  +/* Set rx interrupt flag */
  +s-int_sta |= EMAC_INT_RX;
  +