Re: [PATCH net] net: dsa: mt7530: Use NULL instead of plain integer

[Sean Wang]

On Tue, 2018-04-03 at 10:02 -0700, Florian Fainelli wrote:
> On 04/02/2018 07:18 PM, Sean Wang wrote:
> > On Mon, 2018-04-02 at 16:24 -0700, Florian Fainelli wrote:
> >> We would be passing 0 instead of NULL as the rsp argument to
> >> mt7530_fdb_cmd(), fix that.
> >>
> > 
> > Acked-by: Sean Wang <sean.w...@mediatek.com>
> > 
> > BTW, does the part of the commit message should be updated with "passing
> > NULL instead of 0"?
> 
> I don't follow you, the commit message indicates what we were doing
> which implies it was wrong and fixes it. Would you want me to reword
> that part?

thanks for the explanation.

the commit message is clear and okay for me now

Re: [PATCH net] net: dsa: mt7530: Use NULL instead of plain integer

[Sean Wang]

On Mon, 2018-04-02 at 16:24 -0700, Florian Fainelli wrote:
> We would be passing 0 instead of NULL as the rsp argument to
> mt7530_fdb_cmd(), fix that.
> 

Acked-by: Sean Wang <sean.w...@mediatek.com>

BTW, does the part of the commit message should be updated with "passing
NULL instead of 0"?


> Fixes: b8f126a8d543 ("net-next: dsa: add dsa support for Mediatek MT7530 
> switch")
> Signed-off-by: Florian Fainelli <f.faine...@gmail.com>
> ---
>  drivers/net/dsa/mt7530.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index 4e53c5ce23ff..a7f6c0a62fc7 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -917,7 +917,7 @@ mt7530_port_fdb_add(struct dsa_switch *ds, int port,
>  
>   mutex_lock(>reg_mutex);
>   mt7530_fdb_write(priv, vid, port_mask, addr, -1, STATIC_ENT);
> - ret = mt7530_fdb_cmd(priv, MT7530_FDB_WRITE, 0);
> + ret = mt7530_fdb_cmd(priv, MT7530_FDB_WRITE, NULL);
>   mutex_unlock(>reg_mutex);
>  
>   return ret;
> @@ -933,7 +933,7 @@ mt7530_port_fdb_del(struct dsa_switch *ds, int port,
>  
>   mutex_lock(>reg_mutex);
>   mt7530_fdb_write(priv, vid, port_mask, addr, -1, STATIC_EMP);
> - ret = mt7530_fdb_cmd(priv, MT7530_FDB_WRITE, 0);
> + ret = mt7530_fdb_cmd(priv, MT7530_FDB_WRITE, NULL);
>   mutex_unlock(>reg_mutex);
>  
>   return ret;
> @@ -1293,7 +1293,7 @@ mt7530_setup(struct dsa_switch *ds)
>   }
>  
>   /* Flush the FDB table */
> - ret = mt7530_fdb_cmd(priv, MT7530_FDB_FLUSH, 0);
> + ret = mt7530_fdb_cmd(priv, MT7530_FDB_FLUSH, NULL);
>   if (ret < 0)
>   return ret;
>  

Re: [net-next] phylib: Add device reset GPIO support causes DSA MT7530 acquires reset-gpios fails

[Sean Wang]

On Fri, 2017-12-15 at 11:10 +0100, Andrew Lunn wrote:
> On Fri, Dec 15, 2017 at 02:55:03PM +0800, Sean Wang wrote:
> > Hi Sergei,
> > 
> > Recently I found the patch commit bafbdd527d56 (phylib: Add device reset
> > GPIO support) would have the impact on MT7530 driver. Which causes the
> > DSA MT7530 device (it's the child node under mdio bus) gets the
> > reset-gpios fails because the same GPIO seems already be held in the
> > earlier mdiobus_register_device call patched through the commit.
> > 
> > do you have any idea how the commits also considers DSA case ?
> > 
> > I guessed the DSA lan9303, mv88e8 switch should have the same issue
> > since they have the same GPIO name as mdiobus_register_device required.
> 
> Hi Sean
> 
> Ah, not good :-(
> 
> I _think_ for the mv88e6xxx, we can remove the gpio reset code from
> the driver, and let the mdio core do it. I need to test to be sure.
> 
> Would that work for you?
> 
>   Andrew
> 

It probably can't. Because before the GPIO line is manipulated to reset,
certain power control should be handled such as power sources from
external PMIC to let devices actually enter the proper state.

So, I thought the kind of reset should be better controlled by the
specific driver, not by generic core.

Sean

[net-next] phylib: Add device reset GPIO support causes DSA MT7530 acquires reset-gpios fails

[Sean Wang]

Hi Sergei,

Recently I found the patch commit bafbdd527d56 (phylib: Add device reset
GPIO support) would have the impact on MT7530 driver. Which causes the
DSA MT7530 device (it's the child node under mdio bus) gets the
reset-gpios fails because the same GPIO seems already be held in the
earlier mdiobus_register_device call patched through the commit.

do you have any idea how the commits also considers DSA case ?

I guessed the DSA lan9303, mv88e8 switch should have the same issue
since they have the same GPIO name as mdiobus_register_device required.

drivers/net/dsa/lan9303-core.c:1303:chip->reset_gpio =
devm_gpiod_get_optional(chip->dev, "reset",
drivers/net/dsa/mv88e6xxx/chip.c:3936:  chip->reset =
devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
drivers/net/dsa/mt7530.c:1476:  priv->reset =
devm_gpiod_get_optional(>dev, "reset",

Below is the stack dump I added in of_get_named_gpiod_flags call. Which
apparently shows mdiobus_register_device would get gpio at first and
then DSA driver does.

[2.570607] [] (show_stack) from [] (dump_stack
+0x98/0xac)
[2.59] [] (dump_stack) from []
(of_get_named_gpiod_flags+0xe8/0x128)
[2.586239] [] (of_get_named_gpiod_flags) from []
(fwnode_get_named_gpiod+0x5c/0xd0)
[2.595648] [] (fwnode_get_named_gpiod) from []
(mdiobus_register_device+0x64/0xa8)
[2.604969] [] (mdiobus_register_device) from []
(mdio_device_register+0x34/0x8c)
[2.614119] [] (mdio_device_register) from []
(of_mdiobus_register+0x150/0x2c8)
[2.623097] [] (of_mdiobus_register) from []
(mtk_probe+0x6a4/0x820)
[2.631128] [] (mtk_probe) from []
(platform_drv_probe+0x5c/0xc0)
[2.638898] [] (platform_drv_probe) from []
(driver_probe_device+0x2f4/0x4d8)
[2.647702] [] (driver_probe_device) from []
(__driver_attach+0x10c/0x128)
[2.656248] [] (__driver_attach) from []
(bus_for_each_dev+0x78/0xac)
[2.664364] [] (bus_for_each_dev) from []
(driver_attach+0x2c/0x30)
[2.672307] [] (driver_attach) from []
(bus_add_driver+0x1e0/0x278)
[2.680250] [] (bus_add_driver) from []
(driver_register+0x88/0x108)would
[2.688277] [] (driver_register) from []
(__platform_driver_register+0x50/0x58)
[2.697256] [] (__platform_driver_register) from
[] (mtk_driver_init+0x24/0x28)
[2.706234] [] (mtk_driver_init) from []
(do_one_initcall+0x50/0x17c)
[2.714351] [] (do_one_initcall) from []
(kernel_init_freeable+0x154/0x1f4)
[2.722984] [] (kernel_init_freeable) from []
(kernel_init+0x18/0x124)
[2.731185] [] (kernel_init) from []
(ret_from_fork+0x14/0x2c)
[2.738719] of_get_named_gpiod_flags: parsed 'reset-gpios' property
of node '/ethernet@1b10/mdio-bus/switch@0[0]' - status (0)
[2.750631] mt7530 mdio-bus:00: GPIO lookup for consumer reset
[2.756443] mt7530 mdio-bus:00: using device tree for GPIO lookup
[2.762494] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
4.15.0-rc2-00819-g4d89425-dirty #6
[2.770514] Hardware name: Mediatek Cortex-A7 (Device Tree)
[2.776049] [] (unwind_backtrace) from []
(show_stack+0x20/0x24)
[2.783733] [] (show_stack) from [] (dump_stack
+0x98/0xac)
[2.790901] [] (dump_stack) from []
(of_get_named_gpiod_flags+0xe8/0x128)
[2.799359] [] (of_get_named_gpiod_flags) from []
(of_find_gpio+0x70/0xfc)
[2.807903] [] (of_find_gpio) from []
(gpiod_get_index+0xac/0x2a0)
[2.815759] [] (gpiod_get_index) from []
(devm_gpiod_get_index+0x5c/0x98)
[2.824220] [] (devm_gpiod_get_index) from []
(devm_gpiod_get_optional+0x20/0wouldx34)
[2.833370] [] (devm_gpiod_get_optional) from []
(mt7530_probe+0x170/0x1a0)
[2.842003] [] (mt7530_probe) from [] (mdio_probe
+0x40/0x64)
[2.849342] [] (mdio_probe) from []
(driver_probe_device+0x2f4/0x4d8)
[2.857455] [] (driver_probe_device) from []
(__device_attach_driver+0xc8/0x144)
[2.866518] [] (__device_attach_driver) from []
(bus_for_each_drv+0x70/0xa4)geert+rene...@glider.be
[2.875235] [] (bus_for_each_drv) from []
(__device_attach+0xc0/0x150)
[2.883434] [] (__device_attach) from []
(device_initial_probe+0x1c/0x20)
[2.891893] [] (device_initial_probe) from []
(bus_probe_device+0x94/0x9c)
[2.900440] [] (bus_probe_device) from []
(device_add+0x374/0x5c0)
[2.908297] [] (device_add) from []
(mdio_device_register+0x44/0x8c)
[2.916326] [] (mdio_device_register) from []
(of_mdiobus_register+0x150/0x2c8)
[2.925302] [] (of_mdiobus_register) from []
(mtk_probe+0x6a4/0x820)


and the related dts I used is as below 
would
 mdio: mdio-bus {
#address-cells = <1>;
#size-cells = <0>;

switch@0 {
compatible = "mediatek,mt7530";
#address-cells = <1>;
#size-cells = <0>;
reg = <0>;
reset-gpios = < 33 0>;
core-supply = <_vpa_reg>;
io-supply = <_vemc3v3_reg>;

ports {
   

Re: [PATCH net-next 2/3] net: dsa: mediatek: combine MediaTek tag with VLAN tag

[Sean Wang]

On Tue, 2017-12-12 at 09:28 +0100, Andrew Lunn wrote:
> On Tue, Dec 12, 2017 at 03:21:21PM +0800, Sean Wang wrote:
> > On Thu, 2017-12-07 at 16:30 +0100, Andrew Lunn wrote:
> > > > @@ -25,20 +28,37 @@ static struct sk_buff *mtk_tag_xmit(struct sk_buff 
> > > > *skb,
> > > >  {
> > > > struct dsa_port *dp = dsa_slave_to_port(dev);
> > > > u8 *mtk_tag;
> > > > +   bool is_vlan_skb = true;
> > > 
> > > ..
> > > 
> > > > +   /* Mark tag attribute on special tag insertion to notify 
> > > > hardware
> > > > +* whether that's a combined special tag with 802.1Q header.
> > > > +*/
> > > > +   mtk_tag[0] = is_vlan_skb ? MTK_HDR_XMIT_TAGGED_TPID_8100 :
> > > > +MTK_HDR_XMIT_UNTAGGED;
> > > > mtk_tag[1] = (1 << dp->index) & MTK_HDR_XMIT_DP_BIT_MASK;
> > > > -   mtk_tag[2] = 0;
> > > > -   mtk_tag[3] = 0;
> > > > +
> > > > +   /* Tag control information is kept for 802.1Q */
> > > > +   if (!is_vlan_skb) {
> > > > +   mtk_tag[2] = 0;
> > > > +   mtk_tag[3] = 0;
> > > > +   }
> > > >  
> > > > return skb;
> > > >  }
> > > 
> > > Hi Sean
> > > 
> > > So you can mark a packet for egress. What about ingress? How do you
> > > know the VLAN/PORT combination for packets the CPU receives? I would
> > > of expected a similar change to mtk_tag_rcv().
> > > 
> > >Andrew
> > 
> > Hi, Andrew
> > 
> > It's unnecessary for extra handling in mtk_tag_rcv() when VLAN tag is
> > present since it is able to put the VLAN tag after the special tag and
> > then follow the existing way to parse.
> 
> Hi Sean
> 
> O.K. Please mention this in the commit message. Since it was something
> i was expecting, it should be documented why it is not needed.
> 
>   Thanks
>   Andrew


Sure. I will add this in the commit message.

Sean



> 

Re: [PATCH net-next 1/3] net: dsa: mediatek: add VLAN support for MT7530

[Sean Wang]

On Tue, 2017-12-12 at 09:24 +0100, Felix Fietkau wrote:
> On 2017-12-07 07:06, sean.w...@mediatek.com wrote:
> > From: Sean Wang <sean.w...@mediatek.com>
> > 
> > MT7530 can treat each port as either VLAN-unware port or VLAN-ware port
> Shouldn't that be VLAN-unaware/VLAN-aware (in the code as well)?
> 
> - Felix
> 

Hi, Felix

Thank you. It is my misspelling problem. I will fix them all with this
including in the code.

Sean

Re: [PATCH net-next 2/3] net: dsa: mediatek: combine MediaTek tag with VLAN tag

[Sean Wang]

On Thu, 2017-12-07 at 16:30 +0100, Andrew Lunn wrote:
> > @@ -25,20 +28,37 @@ static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb,
> >  {
> > struct dsa_port *dp = dsa_slave_to_port(dev);
> > u8 *mtk_tag;
> > +   bool is_vlan_skb = true;
> 
> ..
> 
> > +   /* Mark tag attribute on special tag insertion to notify hardware
> > +* whether that's a combined special tag with 802.1Q header.
> > +*/
> > +   mtk_tag[0] = is_vlan_skb ? MTK_HDR_XMIT_TAGGED_TPID_8100 :
> > +MTK_HDR_XMIT_UNTAGGED;
> > mtk_tag[1] = (1 << dp->index) & MTK_HDR_XMIT_DP_BIT_MASK;
> > -   mtk_tag[2] = 0;
> > -   mtk_tag[3] = 0;
> > +
> > +   /* Tag control information is kept for 802.1Q */
> > +   if (!is_vlan_skb) {
> > +   mtk_tag[2] = 0;
> > +   mtk_tag[3] = 0;
> > +   }
> >  
> > return skb;
> >  }
> 
> Hi Sean
> 
> So you can mark a packet for egress. What about ingress? How do you
> know the VLAN/PORT combination for packets the CPU receives? I would
> of expected a similar change to mtk_tag_rcv().
> 
>Andrew

Hi, Andrew

It's unnecessary for extra handling in mtk_tag_rcv() when VLAN tag is
present since it is able to put the VLAN tag after the special tag and
then follow the existing way to parse.

Sean

Re: [PATCH net-next 1/3] net: dsa: mediatek: add VLAN support for MT7530

[Sean Wang]

Hi, Andrew

All sounds reasonable. All will be fixed in the next version.

Sean

On Thu, 2017-12-07 at 16:24 +0100, Andrew Lunn wrote:
> >  static void
> > +mt7530_port_set_vlan_unware(struct dsa_switch *ds, int port)
> > +{
> > +   struct mt7530_priv *priv = ds->priv;
> > +   int i;
> > +   bool all_user_ports_removed = true;
> 
> Hi Sean
> 
> Reverse Christmas tree please.
> 

will be fixed 

> > +static int
> > +mt7530_vlan_cmd(struct mt7530_priv *priv, enum mt7530_vlan_cmd cmd, u16 
> > vid)
> > +{
> > +   u32 val;
> > +   int ret;
> > +   struct mt7530_dummy_poll p;
> 
> Here too.
> 

will be fixed

> > +static int
> > +mt7530_port_vlan_prepare(struct dsa_switch *ds, int port,
> > +const struct switchdev_obj_port_vlan *vlan,
> > +struct switchdev_trans *trans)
> > +{
> > +   struct mt7530_priv *priv = ds->priv;
> > +
> > +   /* The port is being kept as VLAN-unware port when bridge is set up
> > +* with vlan_filtering not being set, Otherwise, the port and the
> > +* corresponding CPU port is required the setup for becoming a
> > +* VLAN-ware port.
> > +*/
> > +   if (!priv->ports[port].vlan_filtering)
> > +   return 0;
> > +
> > +   mt7530_port_set_vlan_ware(ds, port);
> > +   mt7530_port_set_vlan_ware(ds, MT7530_CPU_PORT);
> 
> A prepare function should just validate that it is possible to carry
> out the operation. It should not change any state. These two last
> lines probably don't belong here.
> 

okay, it will be moved into the proper place such as
mt7530_port_vlan_filtering

> > +
> > +   return 0;
> > +}
> > +
> > +static void
> > +mt7530_hw_vlan_add(struct mt7530_priv *priv,
> > +  struct mt7530_hw_vlan_entry *entry)
> > +{
> > +   u32 val;
> > +   u8 new_members;
> 
> Reverse Christmas tree. Please check the whole patch.
> 
 will be fixed

> > +static inline void INIT_MT7530_HW_ENTRY(struct mt7530_hw_vlan_entry *e,
> > +   int port, bool untagged)
> > +{
> > +   e->port = port;
> > +   e->untagged = untagged;
> > +}
> 
> All CAPITAL letters is for #defines. This is just a normal
> function. Please use lower case.
> 

will be fixed
> Andrew
> 

RE: [PATCH 02/15] drivers, net, ethernet: convert mtk_eth.dma_refcnt from atomic_t to refcount_t

[Sean Wang]

On Fri, 2017-10-20 at 10:37 +, Reshetova, Elena wrote:
> > On Fri, 2017-10-20 at 10:23 +0300, Elena Reshetova wrote:
> > > atomic_t variables are currently used to implement reference
> > > counters with the following properties:
> > >  - counter is initialized to 1 using atomic_set()
> > >  - a resource is freed upon counter reaching zero
> > >  - once counter reaches zero, its further
> > >increments aren't allowed
> > >  - counter schema uses basic atomic operations
> > >(set, inc, inc_not_zero, dec_and_test, etc.)
> > >
> > > Such atomic variables should be converted to a newly provided
> > > refcount_t type and API that prevents accidental counter overflows
> > > and underflows. This is important since overflows and underflows
> > > can lead to use-after-free situation and be exploitable.
> > >
> > > The variable mtk_eth.dma_refcnt is used as pure reference counter.
> > > Convert it to refcount_t and fix up the operations.
> > >
> > > Suggested-by: Kees Cook 
> > > Reviewed-by: David Windsor 
> > > Reviewed-by: Hans Liljestrand 
> > > Signed-off-by: Elena Reshetova 
> > > ---
> > >  drivers/net/ethernet/mediatek/mtk_eth_soc.c | 8 +---
> > >  drivers/net/ethernet/mediatek/mtk_eth_soc.h | 4 +++-
> > >  2 files changed, 8 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > index 5e81a72..54adfd9 100644
> > > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > @@ -1817,7 +1817,7 @@ static int mtk_open(struct net_device *dev)
> > >   struct mtk_eth *eth = mac->hw;
> > >
> > >   /* we run 2 netdevs on the same dma ring so we only bring it up once
> > */
> > > - if (!atomic_read(>dma_refcnt)) {
> > > + if (!refcount_read(>dma_refcnt)) {
> > >   int err = mtk_start_dma(eth);
> > >
> > >   if (err)
> > > @@ -1827,8 +1827,10 @@ static int mtk_open(struct net_device *dev)
> > >   napi_enable(>rx_napi);
> > >   mtk_tx_irq_enable(eth, MTK_TX_DONE_INT);
> > >   mtk_rx_irq_enable(eth, MTK_RX_DONE_INT);
> > > + refcount_set(>dma_refcnt, 1);
> > 
> > the existing driver seems to have a missing initial atomic_set for the
> > eth->dma_refcnt.
> > 
> > >   }
> > > - atomic_inc(>dma_refcnt);
> > > + else
> > > + refcount_inc(>dma_refcnt);
> > >
> > 
> > how about add the initial refcount_set into probe handler, and keep
> > logic else unchanged ?
> 
> Sure, I guess you mean mtk_probe() function? I can move the refcount_set to 
> be there
> and remove this change. 
> 
> Should I resend the modified patch to you (maybe then two of the ethernet 
> patches)?
> 
> Best Regards,
> Elena.

The entire series has been applies to net-next, I think I can make the
follow-ups patches relative to your work. 

Sean

> > 
> > >   phy_start(dev->phydev);
> > >   netif_start_queue(dev);
> > > @@ -1868,7 +1870,7 @@ static int mtk_stop(struct net_device *dev)
> > >   phy_stop(dev->phydev);
> > >
> > >   /* only shutdown DMA if this is the last user */
> > > - if (!atomic_dec_and_test(>dma_refcnt))
> > > + if (!refcount_dec_and_test(>dma_refcnt))
> > >   return 0;
> > >
> > >   mtk_tx_irq_disable(eth, MTK_TX_DONE_INT);
> > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> > b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> > > index 3d3c24a..a3af466 100644
> > > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> > > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> > > @@ -15,6 +15,8 @@
> > >  #ifndef MTK_ETH_H
> > >  #define MTK_ETH_H
> > >
> > > +#include 
> > > +
> > >  #define MTK_QDMA_PAGE_SIZE   2048
> > >  #define  MTK_MAX_RX_LENGTH   1536
> > >  #define MTK_TX_DMA_BUF_LEN   0x3fff
> > > @@ -632,7 +634,7 @@ struct mtk_eth {
> > >   struct regmap   *pctl;
> > >   u32 chip_id;
> > >   boolhwlro;
> > > - atomic_tdma_refcnt;
> > > + refcount_t  dma_refcnt;
> > >   struct mtk_tx_ring  tx_ring;
> > >   struct mtk_rx_ring
> > rx_ring[MTK_MAX_RX_RING_NUM];
> > >   struct mtk_rx_ring  rx_ring_qdma;
> > 
> 

Re: [PATCH 02/15] drivers, net, ethernet: convert mtk_eth.dma_refcnt from atomic_t to refcount_t

[Sean Wang]

On Fri, 2017-10-20 at 10:23 +0300, Elena Reshetova wrote:
> atomic_t variables are currently used to implement reference
> counters with the following properties:
>  - counter is initialized to 1 using atomic_set()
>  - a resource is freed upon counter reaching zero
>  - once counter reaches zero, its further
>increments aren't allowed
>  - counter schema uses basic atomic operations
>(set, inc, inc_not_zero, dec_and_test, etc.)
> 
> Such atomic variables should be converted to a newly provided
> refcount_t type and API that prevents accidental counter overflows
> and underflows. This is important since overflows and underflows
> can lead to use-after-free situation and be exploitable.
> 
> The variable mtk_eth.dma_refcnt is used as pure reference counter.
> Convert it to refcount_t and fix up the operations.
> 
> Suggested-by: Kees Cook 
> Reviewed-by: David Windsor 
> Reviewed-by: Hans Liljestrand 
> Signed-off-by: Elena Reshetova 
> ---
>  drivers/net/ethernet/mediatek/mtk_eth_soc.c | 8 +---
>  drivers/net/ethernet/mediatek/mtk_eth_soc.h | 4 +++-
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
> b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index 5e81a72..54adfd9 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -1817,7 +1817,7 @@ static int mtk_open(struct net_device *dev)
>   struct mtk_eth *eth = mac->hw;
>  
>   /* we run 2 netdevs on the same dma ring so we only bring it up once */
> - if (!atomic_read(>dma_refcnt)) {
> + if (!refcount_read(>dma_refcnt)) {
>   int err = mtk_start_dma(eth);
>  
>   if (err)
> @@ -1827,8 +1827,10 @@ static int mtk_open(struct net_device *dev)
>   napi_enable(>rx_napi);
>   mtk_tx_irq_enable(eth, MTK_TX_DONE_INT);
>   mtk_rx_irq_enable(eth, MTK_RX_DONE_INT);
> + refcount_set(>dma_refcnt, 1);

the existing driver seems to have a missing initial atomic_set for the
eth->dma_refcnt. 

>   }
> - atomic_inc(>dma_refcnt);
> + else
> + refcount_inc(>dma_refcnt);
>  

how about add the initial refcount_set into probe handler, and keep
logic else unchanged ? 

>   phy_start(dev->phydev);
>   netif_start_queue(dev);
> @@ -1868,7 +1870,7 @@ static int mtk_stop(struct net_device *dev)
>   phy_stop(dev->phydev);
>  
>   /* only shutdown DMA if this is the last user */
> - if (!atomic_dec_and_test(>dma_refcnt))
> + if (!refcount_dec_and_test(>dma_refcnt))
>   return 0;
>  
>   mtk_tx_irq_disable(eth, MTK_TX_DONE_INT);
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h 
> b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> index 3d3c24a..a3af466 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> @@ -15,6 +15,8 @@
>  #ifndef MTK_ETH_H
>  #define MTK_ETH_H
>  
> +#include 
> +
>  #define MTK_QDMA_PAGE_SIZE   2048
>  #define  MTK_MAX_RX_LENGTH   1536
>  #define MTK_TX_DMA_BUF_LEN   0x3fff
> @@ -632,7 +634,7 @@ struct mtk_eth {
>   struct regmap   *pctl;
>   u32 chip_id;
>   boolhwlro;
> - atomic_tdma_refcnt;
> + refcount_t  dma_refcnt;
>   struct mtk_tx_ring  tx_ring;
>   struct mtk_rx_ring  rx_ring[MTK_MAX_RX_RING_NUM];
>   struct mtk_rx_ring  rx_ring_qdma;

Re: [PATCH net] net: dsa: Initialize ds->cpu_port_mask earlier

[Sean Wang]

Hi, Florian

the solution is better and also can fix the problem I originally
reported. Thank for your immediate help :-)

Sean


On Mon, 2017-07-24 at 10:49 -0700, Florian Fainelli wrote:
> The mt7530 driver has its dsa_switch_ops::get_tag_protocol function
> check ds->cpu_port_mask to issue a warning in case the configured CPU
> port is not capable of supporting tags.
> 
> After commit 14be36c2c96c ("net: dsa: Initialize all CPU and enabled
> ports masks in dsa_ds_parse()") we slightly re-arranged the
> initialization such that this was no longer working. Just make sure that
> ds->cpu_port_mask is set prior to the first call to get_tag_protocol,
> thus restoring the expected contract. In case of error, the CPU port bit
> is cleared.
> 
> Fixes: 14be36c2c96c ("net: dsa: Initialize all CPU and enabled ports masks in 
> dsa_ds_parse()")
> Reported-by: Sean Wang <sean.w...@mediatek.com>
> Signed-off-by: Florian Fainelli <f.faine...@gmail.com>
> ---
> Sean, can you check this does fix the problem you originally reported
> correctly for you? Thanks!
> 
>  net/dsa/dsa2.c | 13 +++--
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
> index 56e46090526b..c442051d5a55 100644
> --- a/net/dsa/dsa2.c
> +++ b/net/dsa/dsa2.c
> @@ -509,21 +509,22 @@ static int dsa_cpu_parse(struct dsa_port *port, u32 
> index,
>   dst->cpu_dp->netdev = ethernet_dev;
>   }
>  
> + /* Initialize cpu_port_mask now for drv->setup()
> +  * to have access to a correct value, just like what
> +  * net/dsa/dsa.c::dsa_switch_setup_one does.
> +  */
> + ds->cpu_port_mask |= BIT(index);
> +
>   tag_protocol = ds->ops->get_tag_protocol(ds);
>   dst->tag_ops = dsa_resolve_tag_protocol(tag_protocol);
>   if (IS_ERR(dst->tag_ops)) {
>   dev_warn(ds->dev, "No tagger for this switch\n");
> + ds->cpu_port_mask &= ~BIT(index);
>   return PTR_ERR(dst->tag_ops);
>   }
>  
>   dst->rcv = dst->tag_ops->rcv;
>  
> - /* Initialize cpu_port_mask now for drv->setup()
> -  * to have access to a correct value, just like what
> -  * net/dsa/dsa.c::dsa_switch_setup_one does.
> -  */
> - ds->cpu_port_mask |= BIT(index);
> -
>   return 0;
>  }
>  

Re: [PATCH] net: ethernet: mediatek: remove useless code in mtk_poll_tx()

[Sean Wang]

On Tue, 2017-07-18 at 15:48 -0500, Gustavo A. R. Silva wrote:
> Remove useless local variable _condition_ and the code related.
> 
> Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
> ---
>  drivers/net/ethernet/mediatek/mtk_eth_soc.c | 5 +
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
> b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index b3d0c2e..7e95cf5 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -1027,7 +1027,6 @@ static int mtk_poll_tx(struct mtk_eth *eth, int budget)
>   unsigned int done[MTK_MAX_DEVS];
>   unsigned int bytes[MTK_MAX_DEVS];
>   u32 cpu, dma;
> - static int condition;
>   int total = 0, i;
>  
>   memset(done, 0, sizeof(done));
> @@ -1051,10 +1050,8 @@ static int mtk_poll_tx(struct mtk_eth *eth, int budget)
>   mac = 1;
>  
>   skb = tx_buf->skb;
> - if (!skb) {
> - condition = 1;
> + if (!skb)
>   break;
> - }
>  
>   if (skb != (struct sk_buff *)MTK_DMA_DUMMY_DESC) {
>   bytes[mac] += skb->len;

Acked-by: Sean Wang <sean.w...@mediatek.com>

Re: [PATCH net-next v2 1/4] dt-bindings: net: mediatek: add support for MediaTek MT7623 and MT7622 SoC

[Sean Wang]

On Mon, 2017-07-17 at 15:38 +0200, Andrew Lunn wrote:
> On Mon, Jul 17, 2017 at 06:06:22PM +0800, sean.w...@mediatek.com wrote:
> > From: Sean Wang <sean.w...@mediatek.com>
> > 
> > The patch adds the supplements in the dt-binding document for MediaTek
> > MT7622 SoC with extra SGMII system controller and relevant clock consumers
> > listed as the requirements for those SoCs equipped with the SGMII circuit.
> > Also, add the missing binding information for MT7623 SoC here which relies
> > on the fallback binding of MT2701.
> > 
> > Signed-off-by: Sean Wang <sean.w...@mediatek.com>
> > ---
> >  Documentation/devicetree/bindings/net/mediatek-net.txt | 12 +---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/mediatek-net.txt 
> > b/Documentation/devicetree/bindings/net/mediatek-net.txt
> > index c7194e8..1d1168b 100644
> > --- a/Documentation/devicetree/bindings/net/mediatek-net.txt
> > +++ b/Documentation/devicetree/bindings/net/mediatek-net.txt
> > @@ -7,24 +7,30 @@ have dual GMAC each represented by a child node..
> >  * Ethernet controller node
> >  
> >  Required properties:
> > -- compatible: Should be "mediatek,mt2701-eth"
> > +- compatible: Should be
> > +   "mediatek,mt2701-eth": for MT2701 SoC
> > +   "mediatek,mt7623-eth", "mediatek,mt2701-eth": for MT7623 SoC
> 
> Hi Sean
> 
> We appear to have "mediatek,mt2701-eth" twice.
> 

Hi, Andrew

mt7623-eth supported up to now currently rely on the fallback binding of
mt2701-eth

in fact, the logic could be commonly found in many driver such as mtk
sysirq such as [1] did, so i felt this usage should not make something
wrong.


[1] https://patchwork.kernel.org/patch/9739827/


>Andrew

Re: [PATCH net-next 2/4] net-next: mediatek: add platform data to adapt into various hardware

[Sean Wang]

On Wed, 2017-07-12 at 16:50 +0200, Andrew Lunn wrote:
> > +static int mtk_clk_enable(struct mtk_eth *eth)
> > +{
> > +   int clk, ret;
> > +
> > +   for (clk = 0; clk < MTK_CLK_MAX ; clk++) {
> > +   if (eth->clks[clk]) {
> > +   ret = clk_prepare_enable(eth->clks[clk]);
> > +   if (ret)
> > +   goto err_disable_clks;
> > +   }
> > +   }
> > +
> > +   return 0;
> > +
> > +err_disable_clks:
> > +   while (--clk >= 0) {
> > +   if (eth->clks[clk])
> > +   clk_disable_unprepare(eth->clks[clk]);
> > +   }
> > +
> > +   return ret;
> > +}
> 
> > +
> >  static int mtk_hw_init(struct mtk_eth *eth)
> >  {
> > int i, val;
> > @@ -1847,10 +1881,8 @@ static int mtk_hw_init(struct mtk_eth *eth)
> > pm_runtime_enable(eth->dev);
> > pm_runtime_get_sync(eth->dev);
> >  
> > -   clk_prepare_enable(eth->clks[MTK_CLK_ETHIF]);
> > -   clk_prepare_enable(eth->clks[MTK_CLK_ESW]);
> > -   clk_prepare_enable(eth->clks[MTK_CLK_GP1]);
> > -   clk_prepare_enable(eth->clks[MTK_CLK_GP2]);
> > +   mtk_clk_enable(eth);
> > +
> 
> mtk_clk_enable() returns an error code. It is probably a good idea to
> use it, especially if it could be EPRODE_DEFER.

okay, I will improve those clocks handling better along with Florian's
suggestion in the next version

Sean












> 
> Andrew

Re: [PATCH] net: ethernet: mediatek: remove useless code in mtk_probe()

[Sean Wang]

Hi,  Gustavo

It indeed is useless at the current time point.


but actually I will add new SoC support to the driver in the next week,
which requires the variable match :-(

Sean


On Fri, 2017-07-07 at 15:23 -0500, Gustavo A. R. Silva wrote:
> Remove useless local variables _match_, _soc_ and the code related.
> 
> Notice that
> 
> const struct of_device_id of_mtk_match[] = {
> { .compatible = "mediatek,mt2701-eth" },
> {},
> };
> 
> So match->data is NULL.
> 
> Suggested-by: Andrew Lunn 
> Signed-off-by: Gustavo A. R. Silva 
> ---
>  drivers/net/ethernet/mediatek/mtk_eth_soc.c | 5 -
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
> b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index adaaafc..b9a5a65 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -2401,15 +2401,10 @@ static int mtk_probe(struct platform_device *pdev)
>  {
>   struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   struct device_node *mac_np;
> - const struct of_device_id *match;
> - struct mtk_soc_data *soc;
>   struct mtk_eth *eth;
>   int err;
>   int i;
>  
> - match = of_match_device(of_mtk_match, >dev);
> - soc = (struct mtk_soc_data *)match->data;
> -
>   eth = devm_kzalloc(>dev, sizeof(*eth), GFP_KERNEL);
>   if (!eth)
>   return -ENOMEM;

Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

[Sean Wang]

Hi Andrew 


On Fri, 2017-03-24 at 15:19 +0100, Andrew Lunn wrote:
> On Tue, Mar 21, 2017 at 05:35:10PM +0800, sean.w...@mediatek.com wrote:
> 
> Hi Sean
> 
> > +   /* Lower Tx Driving */
> > +   for (i = 0 ; i < 6 ; i++)
> 
> Could MT7530_CPU_PORT be used here? 
> 

I should create meaningful definition instead of avoiding hard coding 

where 6 should be corrected into 5 which is meant for the number for

control register to configure TRGMII path


> > +   mt7530_write(priv, MT7530_TRGMII_TD_ODT(i),
> > +TD_DM_DRVP(8) | TD_DM_DRVN(8));
> > +
> > +   /* Setup MT7530 core clock */
> > +   if (!trgint) {
> > +   /* Disable MT7530 core clock */
> > +   core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
> > +
> > +   /* Disable MT7530 PLL, since phy_device has not yet been
> > +* created when this function is called. So we provide
> > +* core_write_mmd_indirect to complete this function
> > +*/
> > +   core_write_mmd_indirect(priv,
> > +   CORE_GSWPLL_GRP1,
> > +   MDIO_MMD_VEND2,
> > +   0);
> > +
> > +   /* Setup MT7530 core clock into 500Mhz */
> > +   core_write(priv, CORE_GSWPLL_GRP2,
> > +  RG_GSWPLL_POSDIV_500M(1) |
> > +  RG_GSWPLL_FBKDIV_500M(25));
> > +
> > +   /* Enable MT7530 PLL */
> > +   core_write(priv, CORE_GSWPLL_GRP1,
> > +  RG_GSWPLL_EN_PRE |
> > +  RG_GSWPLL_POSDIV_200M(2) |
> > +  RG_GSWPLL_FBKDIV_200M(32));
> > +
> > +   /* Enable MT7530 core clock */
> > +   core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
> > +   }
> > +
> > +   /* Setup the MT7530 TRGMII Tx Clock */
> > +   core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
> > +   core_write(priv, CORE_PLL_GROUP5, RG_LCDDS_PCW_NCPO1(ncpo1));
> > +   core_write(priv, CORE_PLL_GROUP6, RG_LCDDS_PCW_NCPO0(0));
> > +   core_write(priv, CORE_PLL_GROUP10, RG_LCDDS_SSC_DELTA(ssc_delta));
> > +   core_write(priv, CORE_PLL_GROUP11, RG_LCDDS_SSC_DELTA1(ssc_delta));
> > +   core_write(priv, CORE_PLL_GROUP4,
> > +  RG_SYSPLL_DDSFBK_EN | RG_SYSPLL_BIAS_EN |
> > +  RG_SYSPLL_BIAS_LPF_EN);
> > +   core_write(priv, CORE_PLL_GROUP2,
> > +  RG_SYSPLL_EN_NORMAL | RG_SYSPLL_VODEN |
> > +  RG_SYSPLL_POSDIV(1));
> > +   core_write(priv, CORE_PLL_GROUP7,
> > +  RG_LCDDS_PCW_NCPO_CHG | RG_LCCDS_C(3) |
> > +  RG_LCDDS_PWDB | RG_LCDDS_ISO_EN);
> > +   core_set(priv, CORE_TRGMII_GSW_CLK_CG,
> > +REG_GSWCK_EN | REG_TRGMIICK_EN);
> > +
> > +   if (!trgint)
> > +   for (i = 0 ; i < 5 ; i++)
> 
> Why only 5 here? All other similar loops are to 6.  Replacing 5 with a
> #define might help make this more readable.

the same as above, it should be corrected

> > +   mt7530_rmw(priv, MT7530_TRGMII_RD(i),
> > +  RD_TAP_MASK, RD_TAP(16));
> > +   else
> > +   mt7623_trgmii_set(priv, GSW_INTF_MODE, INTF_MODE_TRGMII);
> > +
> > +   return 0;
> > +}
> > +
> > +static int
> > +mt7623_pad_clk_setup(struct dsa_switch *ds)
> > +{
> > +   struct mt7530_priv *priv = ds->priv;
> > +   int i;
> > +
> > +   for (i = 0 ; i < 6; i++)
> 
> MT7530_CPU_PORT?


the same as above , it should be corrected
> > +
> > +/* Registers to mac forward conrol for unknown frames */
> 
> /conrol/control
> 

will fix it up
> > +
> > +/* Registor for port control */
> 
> Register
> 

will fix it up
> > +/* Regiser for TOP signal control */
> 
> Register
> 

will fix it up
> > +/* struct mt7530_priv -This is the main datasructure for holding the 
> > state
> 
> data structure
> 
> > + * of the driver
> > + * @dev:   The device pointer
> > + * @ds:The pointer to the dsa core structure
> > + * @bus:   The bus used for the device and built-in PHY
> > + * @ethsys:The regmap used for enabling the necessary PLL
> > + * @ethernet:  The regmap used for access TRGMII-based 
> > registers
> > + * @core_pwr:  The power supplied into the core
> > + * @io_pwr:The power supplied into the I/O
> > + * @mcm:   Flag for distinguishing if standalone IC or module
> > + * coupling
> > + * @reset: The descriptor for GPIO line tied to its reset pin
> > + * @phy_mode:  The xMII for cpu port used
> > + * @ports: Holding the state amongs ports
> 
> among

will fix it up

thank for your careful reviewing again!!



>   Andrew

Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

[Sean Wang]

Hi Andrew,

Add comment as below inline


On Fri, 2017-03-24 at 15:02 +0100, Andrew Lunn wrote:
> Hi Sean
> 
> > +   regmap = devm_regmap_init(ds->dev, NULL, priv,
> > + _regmap_config);
> > +   if (IS_ERR(regmap))
> > +   dev_warn(priv->dev, "phy regmap initialization failed");
> > +
> 
> Shouldn't this be a fatal error? If you keep going when there is an
> error, what happens when you actually try to use the regmap?
> 

Initial thought is that it is just for debugging purpose so if it fails,
it should break any functionality of switch and only have implication as
a warning message. 

Anyway, i will remove it in the next one since it seems better being
kept in private place as you suggested in the previous mail.  

> > +   phy_mode = of_get_phy_mode(ds->ports[ds->dst->cpu_port].dn);
> > +   if (phy_mode < 0) {
> > +   dev_err(priv->dev, "Can't find phy-mode for master device\n");
> > +   return phy_mode;
> > +   }
> > +   dev_info(priv->dev, "phy-mode for master device = %x\n", phy_mode);
> 
> dev_dbg? 
> 

Sorry. i forgot turned it into dev_dbg

> > +
> > +   id = mt7530_read(priv, MT7530_CREV);
> > +   id >>= CHIP_NAME_SHIFT;
> > +   if (id != MT7530_ID)
> > +   return -ENODEV;
> 
> It might be helpful to say what ID has been found, if it is not the
> supported ID.


I will fix it up to make users know what was going on 

thanks for taking your time on those reviewing again!


>   Andrew

Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

[Sean Wang]

Hi Florian,

Thank for taking your time on reviewing. Add comment as inline.

On Wed, 2017-03-22 at 11:39 -0700, Florian Fainelli wrote:
> On 03/21/2017 02:35 AM, sean.w...@mediatek.com wrote:
> > From: Sean Wang <sean.w...@mediatek.com>
> > 
> > MT7530 is a 7-ports Gigabit Ethernet Switch that could be found on
> > Mediatek router platforms such as MT7623A or MT7623N platform which
> > includes 7-port Gigabit Ethernet MAC and 5-port Gigabit Ethernet PHY.
> > Among these ports, The port from 0 to 4 are the user ports connecting
> > with the remote devices while the port 5 and 6 are the CPU ports
> > connecting into Mediatek Ethernet GMAC.
> > 
> > For port 6, it can communicate with the CPU via Mediatek Ethernet GMAC
> > through either the TRGMII or RGMII which could be controlled by phy-mode
> > in the dt-bindings to specify which mode is preferred to use. And for
> > port 5, only RGMII can be specified. However, currently, only port 6 is
> > being supported in this DSA driver.
> > 
> > The driver is made with the reference to qca8k and other existing DSA
> > driver. The most of the essential callbacks of the DSA are already
> > support in the driver, including tag insert for user port distinguishing,
> > port control, bridge offloading, STP setup and ethtool operation to allow
> > DSA to model each user port into a standalone netdevice as the other DSA
> > driver had done.
> 
> Overall, this looks pretty nice and clean, a few comments below
> 
> > 
> > Signed-off-by: Sean Wang <sean.w...@mediatek.com>
> > Signed-off-by: Landen Chao <landen.c...@mediatek.com>
> > ---
> 
> > +static void
> > +mt7530_fdb_read(struct mt7530_priv *priv, struct mt7530_fdb *fdb)
> > +{
> > +   u32 reg[3];
> > +   int i;
> > +
> > +   /* Read from ARL table into an array */
> > +   for (i = 0; i < 3; i++) {
> > +   reg[i] = mt7530_read(priv, MT7530_TSRA1 + (i * 4));
> > +
> > +   dev_dbg(priv->dev, "%s(%d) reg[%d]=0x%x\n",
> > +   __func__, __LINE__, i, reg[i]);
> > +   }
> > +
> > +   /* vid - 11:0 on reg[1] */
> > +   fdb->vid = (reg[1] >> 0) & 0xfff;
> > +   /* aging - 31:24 on reg[2] */
> > +   fdb->aging = (reg[2] >> 24) & 0xff;
> > +   /* portmask - 11:4 on reg[2] */
> > +   fdb->port_mask = (reg[2] >> 4) & 0xff;
> > +   /* mac - 31:0 on reg[0] and 31:16 on reg[1] */
> > +   fdb->mac[0] = (reg[0] >> 24) & 0xff;
> > +   fdb->mac[1] = (reg[0] >> 16) & 0xff;
> > +   fdb->mac[2] = (reg[0] >>  8) & 0xff;
> > +   fdb->mac[3] = (reg[0] >>  0) & 0xff;
> > +   fdb->mac[4] = (reg[1] >> 24) & 0xff;
> > +   fdb->mac[5] = (reg[1] >> 16) & 0xff;
> > +   /* noarp - 3:2 on reg[2] */
> > +   fdb->noarp = ((reg[2] >> 2) & 0x3) == STATIC_ENT;
> 
> Could you add some definitions for the bits and masks that you are
> shifting here?
> 

Okay, I'll make into proper macro for readability  

> > +}
> > +
> > +static void
> > +mt7530_fdb_write(struct mt7530_priv *priv, u16 vid,
> > +u8 port_mask, const u8 *mac,
> > +u8 aging, u8 type)
> > +{
> > +   u32 reg[3] = { 0 };
> > +   int i;
> > +
> > +   /* vid - 11:0 on reg[1] */
> > +   reg[1] |= (vid & 0xfff) << 0;
> > +   /* aging - 31:25 on reg[2] */
> > +   reg[2] |= (aging & 0xff) << 24;
> > +   /* portmask - 11:4 on reg[2] */
> > +   reg[2] |= (port_mask & 0xff) << 4;
> > +   /* type - 3 indicate that entry is static wouldn't
> > +* be aged out and 0 specified as erasing an entry
> > +*/
> > +   reg[2] |= (type & 0x3) << 2;
> > +   /* mac - 31:0 on reg[0] and 31:16 on reg[1] */
> > +   reg[1] |= mac[5] << 16;
> > +   reg[1] |= mac[4] << 24;
> > +   reg[0] |= mac[3] << 0;
> > +   reg[0] |= mac[2] << 8;
> > +   reg[0] |= mac[1] << 16;
> > +   reg[0] |= mac[0] << 24;
> > +
> > +   /* Wrirte array into the ARL table */
> > +   for (i = 0; i < 3; i++)
> > +   mt7530_write(priv, MT7530_ATA1 + (i * 4), reg[i]);
> > +}
> 
> Same here.
> 

As above. I will improve them.


> > +
> > +static int
> > +mt7530_pad_clk_setup(struct dsa_switch *ds, int mode)
> > +{
> > +   struct mt7530_priv *priv = ds->priv;
> > +   u32 ncpo1, ssc_delta, trgint, i;
> > +
> > +   switch (mode) {
> > +   case PHY_INTERFACE_

Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

[Sean Wang]

Hi Andrew,

The purpose for the regmap table registered is to 

provide a way which helps us to look up a specific 

register on the switch through regmap-debugfs.


And not all ranges of register is defined

so I only include the meaningful ones in a sparse way 

for the table.

Sean


On Thu, 2017-03-23 at 08:22 +0100, Andrew Lunn wrote:
> > +static int
> > +mt7623_trgmii_write(struct mt7530_priv *priv,  u32 reg, u32 val)
> > +{
> > +   int ret;
> > +
> > +   ret =  regmap_write(priv->ethernet, TRGMII_BASE(reg), val);
> > +   if (ret < 0)
> > +   dev_err(priv->dev,
> > +   "failed to priv write register\n");
> > +   return ret;
> > +}
> > +
> > +static u32
> > +mt7623_trgmii_read(struct mt7530_priv *priv, u32 reg)
> > +{
> > +   int ret;
> > +   u32 val;
> > +
> > +   ret = regmap_read(priv->ethernet, TRGMII_BASE(reg), );
> > +   if (ret < 0) {
> > +   dev_err(priv->dev,
> > +   "failed to priv read register\n");
> > +   return ret;
> > +   }
> > +
> > +   return val;
> > +}
> 
> Hi Sean
> 
> These appear to be the only two accessors which use the regmap.
> 
> > +static int
> > +mt7530_regmap_read(void *ctx, uint32_t reg, uint32_t *val)
> > +{
> > +   struct mt7530_priv *priv = (struct mt7530_priv *)ctx;
> > +
> > +   /* BIT(15) is used as indication for pseudo registers
> > +* which would be translated into the general MDIO
> > +* access to leverage the unique regmap sys interface.
> > +*/
> > +   if (reg & BIT(15))
> > +   *val = mdiobus_read_nested(priv->bus,
> > +  (reg & 0xf00) >> 8,
> > +  (reg & 0xff) >> 2);
> > +   else
> > +   *val = mt7530_read(priv, reg);
> > +
> > +   return 0;
> > +}
> 
> .
> 
> > +static const struct regmap_range mt7530_readable_ranges[] = {
> > +   regmap_reg_range(0x, 0x00ac), /* Global control */
> > +   regmap_reg_range(0x2000, 0x202c), /* Port Control - P0 */
> > +   regmap_reg_range(0x2100, 0x212c), /* Port Control - P1 */
> > +   regmap_reg_range(0x2200, 0x222c), /* Port Control - P2 */
> > +   regmap_reg_range(0x2300, 0x232c), /* Port Control - P3 */
> > +   regmap_reg_range(0x2400, 0x242c), /* Port Control - P4 */
> > +   regmap_reg_range(0x2500, 0x252c), /* Port Control - P5 */
> > +   regmap_reg_range(0x2600, 0x262c), /* Port Control - P6 */
> > +   regmap_reg_range(0x30e0, 0x30f8), /* Port MAC - SYS */
> > +   regmap_reg_range(0x3000, 0x3014), /* Port MAC - P0 */
> > +   regmap_reg_range(0x3100, 0x3114), /* Port MAC - P1 */
> > +   regmap_reg_range(0x3200, 0x3214), /* Port MAC - P2*/
> > +   regmap_reg_range(0x3300, 0x3314), /* Port MAC - P3*/
> > +   regmap_reg_range(0x3400, 0x3414), /* Port MAC - P4 */
> > +   regmap_reg_range(0x3500, 0x3514), /* Port MAC - P5 */
> > +   regmap_reg_range(0x3600, 0x3614), /* Port MAC - P6 */
> > +   regmap_reg_range(0x4000, 0x40d4), /* MIB - P0 */
> > +   regmap_reg_range(0x4100, 0x41d4), /* MIB - P1 */
> > +   regmap_reg_range(0x4200, 0x42d4), /* MIB - P2 */
> > +   regmap_reg_range(0x4300, 0x43d4), /* MIB - P3 */
> > +   regmap_reg_range(0x4400, 0x44d4), /* MIB - P4 */
> > +   regmap_reg_range(0x4500, 0x45d4), /* MIB - P5 */
> > +   regmap_reg_range(0x4600, 0x46d4), /* MIB - P6 */
> > +   regmap_reg_range(0x4fe0, 0x4ff4), /* SYS */
> > +   regmap_reg_range(0x7000, 0x700c), /* SYS 2 */
> > +   regmap_reg_range(0x7018, 0x7028), /* SYS 3 */
> > +   regmap_reg_range(0x7800, 0x7830), /* SYS 4 */
> > +   regmap_reg_range(0x7a00, 0x7a7c), /* TRGMII */
> > +   regmap_reg_range(0x8000, 0x8078), /* Psedo address for Phy - P0 */
> > +   regmap_reg_range(0x8100, 0x8178), /* Psedo address for Phy - P1 */
> > +   regmap_reg_range(0x8200, 0x8278), /* Psedo address for Phy - P2 */
> > +   regmap_reg_range(0x8300, 0x8378), /* Psedo address for Phy - P3 */
> > +   regmap_reg_range(0x8400, 0x8478), /* Psedo address for Phy - P4 */
> > +};
> 
> It looks like your regmap accessor are only used for 0x7a00 to 0x7a7c.
> 
> It is not clear why you even bother with a regmap. If you have it, why
> not use it for all registers within the regmap?
> 
> Andrew

Re: [PATCH net-next 4/4] net-next: dsa: add dsa support for Mediatek MT7530 switch

[Sean Wang]

On Tue, 2017-03-14 at 00:11 +0100, Andrew Lunn wrote:
> > +static int
> > +mt7530_setup(struct dsa_switch *ds)
> > +{
> > +   struct mt7530_priv *priv = ds->priv;
> > +   int ret, i, phy_mode;
> > +   u8  cpup_mask = 0;
> > +   u32 id, val;
> > +   struct regmap *regmap;
> > +
> > +   /* Make sure that cpu port specfied on the dt is appropriate */
> > +   if (!dsa_is_cpu_port(ds, MT7530_CPU_PORT)) {
> > +   dev_err(priv->dev, "port not matched with the CPU port\n");
> > +   return -EINVAL;
> > +   }
> > +
> > +   regmap = devm_regmap_init(ds->dev, NULL, priv,
> > + _regmap_config);
> > +   if (IS_ERR(regmap))
> > +   dev_warn(priv->dev, "phy regmap initialization failed");
> > +
> > +   phy_mode = of_get_phy_mode(ds->ports[ds->dst->cpu_port].dn);
> > +   if (phy_mode < 0) {
> > +   dev_err(priv->dev, "Can't find phy-mode for master device\n");
> > +   return phy_mode;
> > +   }
> > +   dev_info(priv->dev, "phy-mode for master device = %x\n", phy_mode);
> 
> Hi Sean
> 
> It is not documented in the binding that a phy-mode is mandatory for
> the cpu port.
> 
> Andrew

Hi Andrew,

thanks for your reviewing. I'll also add the missing part into the next
one. 
Sean

Re: [PATCH net-next 1/4] dt-bindings: net: dsa: add Mediatek MT7530 binding

[Sean Wang]

On Mon, 2017-03-13 at 09:47 -0700, Florian Fainelli wrote:
> On 03/13/2017 09:11 AM, sean.w...@mediatek.com wrote:
> > From: Sean Wang <sean.w...@mediatek.com>
> > 
> > Add device-tree binding for Mediatek MT7530 switch.
> > 
> > Cc: devicet...@vger.kernel.org
> > Signed-off-by: Sean Wang <sean.w...@mediatek.com>
> > ---
> >  .../devicetree/bindings/net/dsa/mt7530.txt | 94 
> > ++
> >  1 file changed, 94 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/dsa/mt7530.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/net/dsa/mt7530.txt 
> > b/Documentation/devicetree/bindings/net/dsa/mt7530.txt
> > new file mode 100644
> > index 000..0e50dbf
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/dsa/mt7530.txt
> > @@ -0,0 +1,94 @@
> > +Mediatek MT7530 Ethernet switch
> > +
> > +
> > +Required properties:
> > +
> > +- compatible: Must be compatible = "mediatek,mt7530";
> > +- #address-cells: Must be 1
> > +- #size-cells: Must be 0
> > +- mediatek,ethsys: Phandle to the syscon node that handles the reset.
> > +- mediatek,ethernet: Phandle to the syscon node that Mediatek ethernet 
> > driver
> > +   provides that handles the TRGMII setup used by the switch.
> > +   See
> > +   Documentation/devicetree/bindings/net/mediatek-net.txt for the detailed
> > +   setup on mediatek ethernet.
> 
> This seems redundant with the CPU port's ethernet phandle here.


Okay, it is indeed better to reuse the phandle embedded in the cpu
ports. I will reuse this in the next one.

By the ways, I have a question which is could current DSA framework
allows managing the fabric designated from "multiple cpu ports" to "user
ports" in any combination in brctl and in other existing commands?

For example.

I assume that there are two cpu port called 5, and 6.and there are five
user ports called 0, 1, 2 and 3. and the default fabric on the switch is
mapping from { 5 } <-> { 0, 1, 2, 3 ,4 } where members in the braces I
assumes they also can communicate with each other.

Is it feasible for changing the fabric into other combinations in the
runtime such as 
{5} <-> {0, 1, 2, 3} and {6} <-> {4}
{5} <-> {0, 1, 2} and {6} <-> {3, 4} or 
{6} <-> {0, 1} and {6} <-> {2, 3, 4} or

{6} <-> {0, 1, 2, 3 ,4} ?

After some trace code, I found it seemed that only one cpu port could be
supported via one dsa registration. 

Sean

Re: [PATCH net-next 2/4] net-next: dsa: add Mediatek tag RX/TX handler

[Sean Wang]

On Mon, 2017-03-13 at 12:59 -0400, Vivien Didelot wrote:
> Hi Sean,
> 
> sean.w...@mediatek.com writes:
> 
> > +   mtk_tag[1] = (1 << p->port) & MTK_HDR_XMIT_DP_BIT_MASK;
> 
> This won't apply, the port index in now stored in p->dp->index.
> 
> Thanks,
> 
> Vivien


Hi Vivien,

It seems that I need to upgrade to newer kernel to verify this

thanks for your review , I'll fix this in the next one.

Sean

Re: [PATCH net-next 2/4] net-next: dsa: add Mediatek tag RX/TX handler

[Sean Wang]

On Mon, 2017-03-13 at 09:35 -0700, Florian Fainelli wrote:
> On 03/13/2017 09:11 AM, sean.w...@mediatek.com wrote:
> > From: Sean Wang <sean.w...@mediatek.com>
> > 
> > Add the support for the 4-bytes tag for DSA port distinguishing inserted
> > allowing receiving and transmitting the packet via the particular port.
> > The tag is being added after the source MAC address in the ethernet
> > header.
> > 
> > Signed-off-by: Sean Wang <sean.w...@mediatek.com>
> > Signed-off-by: Landen Chao <landen.c...@mediatek.com>
> > ---
> >  include/net/dsa.h  |   1 +
> >  net/dsa/Kconfig|   2 +
> >  net/dsa/Makefile   |   1 +
> >  net/dsa/dsa.c  |   3 ++
> >  net/dsa/dsa_priv.h |   3 ++
> >  net/dsa/tag_mtk.c  | 121 
> > +
> >  6 files changed, 131 insertions(+)
> >  create mode 100644 net/dsa/tag_mtk.c
> > 
> > diff --git a/include/net/dsa.h b/include/net/dsa.h
> > index b122196..954cff2 100644
> > --- a/include/net/dsa.h
> > +++ b/include/net/dsa.h
> > @@ -27,6 +27,7 @@ enum dsa_tag_protocol {
> > DSA_TAG_PROTO_EDSA,
> > DSA_TAG_PROTO_BRCM,
> > DSA_TAG_PROTO_QCA,
> > +   DSA_TAG_PROTO_MTK,
> > DSA_TAG_LAST,   /* MUST BE LAST */
> >  };
> >  
> > diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
> > index 96e47c5..43b67e8 100644
> > --- a/net/dsa/Kconfig
> > +++ b/net/dsa/Kconfig
> > @@ -41,4 +41,6 @@ config NET_DSA_TAG_TRAILER
> >  config NET_DSA_TAG_QCA
> > bool
> >  
> > +config NET_DSA_TAG_MTK
> > +   bool
> >  endif
> > diff --git a/net/dsa/Makefile b/net/dsa/Makefile
> > index a3380ed..97c9891 100644
> > --- a/net/dsa/Makefile
> > +++ b/net/dsa/Makefile
> > @@ -8,3 +8,4 @@ dsa_core-$(CONFIG_NET_DSA_TAG_DSA) += tag_dsa.o
> >  dsa_core-$(CONFIG_NET_DSA_TAG_EDSA) += tag_edsa.o
> >  dsa_core-$(CONFIG_NET_DSA_TAG_TRAILER) += tag_trailer.o
> >  dsa_core-$(CONFIG_NET_DSA_TAG_QCA) += tag_qca.o
> > +dsa_core-$(CONFIG_NET_DSA_TAG_MTK) += tag_mtk.o
> > diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> > index 7899919..3586b1e 100644
> > --- a/net/dsa/dsa.c
> > +++ b/net/dsa/dsa.c
> > @@ -57,6 +57,9 @@ static struct sk_buff *dsa_slave_notag_xmit(struct 
> > sk_buff *skb,
> >  #ifdef CONFIG_NET_DSA_TAG_QCA
> > [DSA_TAG_PROTO_QCA] = _netdev_ops,
> >  #endif
> > +#ifdef CONFIG_NET_DSA_TAG_MTK
> > +   [DSA_TAG_PROTO_MTK] = _dsa_netdev_ops,
> > +#endif
> > [DSA_TAG_PROTO_NONE] = _ops,
> >  };
> >  
> > diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> > index 6cfd738..de61e8f 100644
> > --- a/net/dsa/dsa_priv.h
> > +++ b/net/dsa/dsa_priv.h
> > @@ -84,4 +84,7 @@ int dsa_slave_netdevice_event(struct notifier_block 
> > *unused,
> >  /* tag_qca.c */
> >  extern const struct dsa_device_ops qca_netdev_ops;
> >  
> > +/* tag_mtk.c */
> > +extern const struct dsa_device_ops mtk_dsa_netdev_ops;
> > +
> >  #endif
> > diff --git a/net/dsa/tag_mtk.c b/net/dsa/tag_mtk.c
> > new file mode 100644
> > index 000..a2dc014
> > --- /dev/null
> > +++ b/net/dsa/tag_mtk.c
> > @@ -0,0 +1,121 @@
> > +/*
> > + * Mediatek DSA Tag support
> > + * Copyright (C) 2017 Landen Chao <landen.c...@mediatek.com>
> > + *   Sean Wang <sean.w...@mediatek.com>
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 and
> > + * only version 2 as published by the Free Software Foundation.
> > + *
> > + * 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 
> > +#include "dsa_priv.h"
> > +
> > +#define MTK_HDR_LEN4
> > +#define MTK_HDR_RECV_SOURCE_PORT_MASK  GENMASK(2, 0)
> > +#define MTK_HDR_XMIT_DP_BIT_MASK   GENMASK(5, 0)
> > +
> > +static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb,
> > +   struct net_device *dev)
> > +{
> > +   struct dsa_slave_priv *p = netdev_priv(dev);
> > +   u8 *mtk_tag;
> > +
> > +   if (skb_cow_head(skb, MTK_HDR_LEN) < 0)
> > +   goto out_free;
> > +
> > +   skb_push(skb, MTK_HDR_LEN);
> > +
> > +   memmove(skb->data, skb->data + MTK_HDR_LEN, 2 * ETH_ALEN);
> > +
> > +   /* Build the tag after the MAC Source Address */
> > +   mtk_tag = skb->data + 2 * ETH_ALEN;
> > +
> > +   /* Set the ingress opcode, traffic class, tag enforcment is
> > +* deprecated
> > +*/
> 
> Sounds like this comment came from tag_brcm.c does it really apply here
> as well?
> 
> Other than that:


It seem a copy-paste error accidentally , i will fix up them in the next
version.  The tag only carried port information only , not complicated
as tag_brcm.c is done.


> Reviewed-by: Florian Fainelli <f.faine...@gmail.com>

Re: [PATCH net-next 1/4] dt-bindings: net: dsa: add Mediatek MT7530 binding

[Sean Wang]

On Mon, 2017-03-13 at 17:36 +0100, Andrew Lunn wrote:
> > +- mediatek,reset-pin: Phandle to the pinctrl node used for the reset. Which
> > +   must be required if the property mediatek,mcm of specified as
> > +   "disabled". See
> > +   Documentation/devicetree/bindings/pinctrl/pinctrl-mt65xx.txt for
> > +   the mediatek pintcrl setting for the details.
> 
> Hi Sean
> 
> This appears to be a plan GPIO line. Marvell has the same. It would be
> nice to be consistent with the naming. From
> Documentation/devicetree/bindings/net/dsa/marvell.txt
> 
> Optional properties:
> 
> - reset-gpios   : Should be a gpio specifier for a reset line
> 
>   Andrew


Hi Andrew,

I'll change the property into the consistent way, and usage for 

GPIO control be also used in devm_gpiod_* instead as you suggested

for patch 4.

Sean

Re: [PATCH net-next v2 1/3] net: ethernet: mediatek: add extension of phy-mode for TRGMII

[Sean Wang]

Date: Thu, 22 Sep 2016 14:30:53 +0300, Sergei Shtylyov 
<sergei.shtyl...@cogentembedded.com> wrote:
>>Hello.
>
>On 9/22/2016 5:33 AM, sean.w...@mediatek.com wrote:
>
>> From: Sean Wang <sean.w...@mediatek.com>
>>
>> adds PHY-mode "trgmii" as an extension for the operation mode of the 
>> PHY interface for PHY_INTERFACE_MODE_TRGMII.

.. deleted

>>  switch (of_get_phy_mode(np)) {
>> +case PHY_INTERFACE_MODE_TRGMII:
>> +mac->trgmii = true;
>>  case PHY_INTERFACE_MODE_RGMII_TXID:
>>  case PHY_INTERFACE_MODE_RGMII_RXID:
>>  case PHY_INTERFACE_MODE_RGMII_ID:
>> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h 
>> b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
>> index 7c5e534..e3b9525 100644
>> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
>> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
>> @@ -529,6 +529,8 @@ struct mtk_eth {
>>   * @hw: Backpointer to our main datastruture
>>   * @hw_stats:   Packet statistics counter
>>   * @phy_dev:The attached PHY if available
>> + * @trgmii  Indicate if the MAC uses TRGMII connected to internal
>> +switch
>>   */
>>  struct mtk_mac {
>>  int id;
>> @@ -539,6 +541,7 @@ struct mtk_mac {
>>  struct phy_device   *phy_dev;
>>  __be32  hwlro_ip[MTK_MAX_LRO_IP_CNT];
>>  int hwlro_ip_cnt;
>> +booltrgmii;
>
> I don't see where this is used.

I set trgmii as below
switch (of_get_phy_mode(np)) {
case PHY_INTERFACE_MODE_TRGMII:
mac->trgmii = true;
case PHY_INTERFACE_MODE_RGMII_TXID:

>[...]
>> diff --git a/include/linux/phy.h b/include/linux/phy.h index 
>> 2d24b28..e25f183 100644
>> --- a/include/linux/phy.h
>> +++ b/include/linux/phy.h
>> @@ -80,6 +80,7 @@ typedef enum {
>>  PHY_INTERFACE_MODE_XGMII,
>>  PHY_INTERFACE_MODE_MOCA,
>>  PHY_INTERFACE_MODE_QSGMII,
>> +PHY_INTERFACE_MODE_TRGMII,
>>  PHY_INTERFACE_MODE_MAX,
>>  } phy_interface_t;
>>
>> @@ -123,6 +124,8 @@ static inline const char *phy_modes(phy_interface_t 
>> interface)
>>  return "moca";
>>  case PHY_INTERFACE_MODE_QSGMII:
>>  return "qsgmii";
>> +case PHY_INTERFACE_MODE_TRGMII:
>> +return "trgmii";
>>  default:
>>  return "unknown";
>>  }
>
>I think this should be done in a separate phylib patch.

this patch is applied, so I am so little confused how to do this.
next time I will note placing modification for generic layer
into separate patch.

>
>MBR, Sergei
>

Re: [PATCH net-next] Documentation: devicetree: revise ethernet device-tree binding about TRGMII

[Sean Wang]

Date: Thu, 22 Sep 2016 19:48:47 +0300, Sergei Shtylyov 
<sergei.shtyl...@cogentembedded.com> wrote:
>On 09/22/2016 07:16 PM, sean.w...@mediatek.com wrote:
>
>> From: Sean Wang <sean.w...@mediatek.com>
>>
>> fix typo in mediatek-net.txt and add phy-mode "trgmii" to ethernet.txt
>
>These changes are unrelated to each other, so there should be 2 separate 
>patches. And have the patches I reviewed been merged already, why are you 
>sending an incremental patch?
>

okay, I will make them into distinct patchs.

I saw they had been applied so I created an incremental
patch based on codebase after applied.

>> Cc: devicet...@vger.kernel.org
>> Reported-by: Sergei Shtylyov <sergei.shtyl...@cogentembedded.com>
>> Signed-off-by: Sean Wang <sean.w...@mediatek.com>
>[...]
>
>MBR, Sergei
>
>

Re: [PATCH net-next v2 3/3] net: ethernet: mediatek: add the dts property to set if TRGMII supported on GMAC0

[Sean Wang]

Date: Thu, 22 Sep 2016 14:28:36 +0300, Sergei Shtylyov 
<sergei.shtyl...@cogentembedded.com> wrote:
>Hello.
>
>On 9/22/2016 5:33 AM, sean.w...@mediatek.com wrote:
>
>> From: Sean Wang <sean.w...@mediatek.com>
>>
>> Add the dts property for the capability if TRGMII supported on GAMC0
>>
>> Signed-off-by: Sean Wang <sean.w...@mediatek.com>
>> ---
>>  Documentation/devicetree/bindings/net/mediatek-net.txt | 5 -
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/mediatek-net.txt 
>> b/Documentation/devicetree/bindings/net/mediatek-net.txt
>> index 6103e55..7111278 100644
>> --- a/Documentation/devicetree/bindings/net/mediatek-net.txt
>> +++ b/Documentation/devicetree/bindings/net/mediatek-net.txt
>> @@ -31,7 +31,10 @@ Optional properties:
>>  Required properties:
>>  - compatible: Should be "mediatek,eth-mac"
>>  - reg: The number of the MAC
>> -- phy-handle: see ethernet.txt file in the same directory.
>> +- phy-handle: see ethernet.txt file in the same directory and
>> +the phy-mode "trgmii" required being provided when reg
>
>   Since you've modified the generic parser of the "phy-mode" to add your 
>"trgmii", you also need to update ethernet.txt...
>

>> +is equal to 0 and the MAC uses fixed-link to connect
>> +with inernal switch such as MT7530.
>
>   Internal.
>

thank you for taking time and effort to review this
I will fix it up

Re: [PATCH net-next 3/3] net: ethernet: mediatek: add the dts property to set if TRGMII supported on GMAC0

[Sean Wang]

Date: Wed, 21 Sep 2016 16:17:20 +0200, Andrew Lunn <and...@lunn.ch> wrote:
>On Wed, Sep 21, 2016 at 02:16:30PM +0800, Sean Wang wrote:
>> Date: Tue, 20 Sep 2016 21:37:58 +0200, Andrew Lunn <and...@lunn.ch> wrote:
>> >On Tue, Sep 20, 2016 at 03:59:20PM +0800, sean.w...@mediatek.com wrote:
>> >> From: Sean Wang <sean.w...@mediatek.com>
>> >>
>> >> Add the dts property for the capability if TRGMII supported on GAMC0
>> >>
>> >> Signed-off-by: Sean Wang <sean.w...@mediatek.com>

 deleted

>> >In this case the switch is an MDIO device, not an PHY. It will not
>> >have an phy-mode. It cannot have a phy mode, it is not a PHY.
>> >
>> >Or am i missing something here?
>> >
>> >Thanks
>> >
>> 
>> 1)
>> 
>> The switch driver is not supported for DSA so far yet 
>> but DSA is good thing and I will try make it happen
>> in the near future.
>
>O.K. But if i understand correctly, the TRGMII is so you can use the
>switch. So it needs to work when you have DSA.
>

yes, you are right. TRGMII for now is dedicated for switch
and furthermore it needs doing calibration between the host and
the switch before it works, that I expect to put
the logic of calibration into setup callback of DSA driver.


>> And another question about DSA, that is
>> if I use DSA for switch, how to know the relationship
>> between MAC and DSA ? such like I could know relationship 
>> between MAC and PHY by phy-handle.
>
>It will look like what i stated above. But i missed the cpu node in
>the ports, which is what you are asking about. There will also be a
>node like:
>
>port@6 {
> reg = <6>;
> label = "cpu";
> ethernet = <>;
> };
>
>And this is how you couple the MAC to DSA.

thanks, it is answerig my question : i can get the relationship from 
the node of cpu port pointing to what MAC it runs for.

>> The cause I ask is becasue I think it's good if the topology
>> about MAC/PHYs/Switch is known just by dts files.
>> 
>> 2)
>> 
>> The phy-mode I mention is for fixed-link. For current MAC driver, 
>> it just uses fixed phy to adapt into the part of switch, so the 
>> device tree looks something like the below. 
>> 
>>  {
>> status = "okay";
>> gmac0: mac@0 {
>> compatible = "mediatek,eth-mac";
>> reg = <0>;
>> phy-mode = "trgmii";
>> fixed-link {
>> speed = <1000>;
>> full-duplex;
>> pause;
>> };
>> };
>> 
>> gmac1: mac@1 {
>> compatible = "mediatek,eth-mac";
>> reg = <1>;
>> phy-handle = <>;
>> };
>
>
>static int mtk_phy_connect(struct mtk_mac *mac)
>{
>struct mtk_eth *eth = mac->hw;
>struct device_node *np;
>u32 val;
>
>np = of_parse_phandle(mac->of_node, "phy-handle", 0);
>if (!np && of_phy_is_fixed_link(mac->of_node))
>if (!of_phy_register_fixed_link(mac->of_node))
>np = of_node_get(mac->of_node);
>   ...
>...
>mtk_phy_connect_node(eth, mac, np);
>
>
>So in the case of a fixed-phy, you do look in the MAC node, and when
>there is a phy-handle, you look in the PHY node.
>
>So this does work

yes , it is all

>
>   Andrew

Re: [PATCH net-next 1/3] net: ethernet: mediatek: add extension of phy-mode for TRGMII

[Sean Wang]

Date: Tue, 20 Sep 2016 14:23:24 -0700, Florian Fainelli <f.faine...@gmail.com> 
wrote:
>On 09/20/2016 12:59 AM, sean.w...@mediatek.com wrote:
>> From: Sean Wang <sean.w...@mediatek.com>
>> 
>> adds PHY-mode "trgmii" as an extension for the operation
>> mode of the PHY interface, TRGMII can be compatible with
>> RGMII, so the extended mode doesn't really have effects on
>> the target MAC and PHY, is used as the indication if the
>> current MAC is connected to an internal switch or external
>> PHY respectively by the given configuration on the board and
>> then to perform the corresponding setup on TRGMII hardware
>> module.
>
>Based on my googling, it seems like Turbo RGMII is a Mediatek-specific
>thing for now, but this could become standard and used by other vendors
>at some point, so I would be inclined to just extend the phy-mode
>property to support trgmii as another interface type.
>
>If you do so, do you also mind proposing an update to the Device Tree
>specification:
>
>https://www.devicetree.org/specifications/
>
>Thanks!

I am willing to do the these thing

1)
in the next version, I will extend rgmii mode as
another interface type as PHY_INTERFACE_MODE_TRGMII
defined in linux/phy.h instead of extension only inside
the current driver. This change also helps to save some code.

2)
I send another separate patch for updating the Device Tree
specification about TRGMII adding description

are these all okay for you?

Re: [PATCH net-next 3/3] net: ethernet: mediatek: add the dts property to set if TRGMII supported on GMAC0

[Sean Wang]

Date: Tue, 20 Sep 2016 21:37:58 +0200, Andrew Lunn <and...@lunn.ch> wrote:
>On Tue, Sep 20, 2016 at 03:59:20PM +0800, sean.w...@mediatek.com wrote:
>> From: Sean Wang <sean.w...@mediatek.com>
>>
>> Add the dts property for the capability if TRGMII supported on GAMC0
>>
>> Signed-off-by: Sean Wang <sean.w...@mediatek.com>
>> ---
>>  Documentation/devicetree/bindings/net/mediatek-net.txt | 5 -
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/mediatek-net.txt 
>> b/Documentation/devicetree/bindings/net/mediatek-net.txt
>> index 6103e55..32f79d8 100644
>> --- a/Documentation/devicetree/bindings/net/mediatek-net.txt
>> +++ b/Documentation/devicetree/bindings/net/mediatek-net.txt
>> @@ -31,7 +31,10 @@ Optional properties:
>>  Required properties:
>>  - compatible: Should be "mediatek,eth-mac"
>>  - reg: The number of the MAC
>> -- phy-handle: see ethernet.txt file in the same directory.
>> +- phy-handle: see ethernet.txt file in the same directory and
>> + the additional phy-mode "tgrmii" is provided in order to connect
>> + with the internal switch MT7530 which is only applicable when reg
>> + is equal to 0.
>
>Humm. How is the switch connected? Is it on the MDIO bus?

the switch is connected to MDIO bus

>If it is on the mdio bus, the binding is going to look something like:
>
>eth: ethernet@1b10 {
>compatible = "mediatek,mt7623-eth";
>reg = <0 0x1b10 0 0x2>;
>clocks = < CLK_TOP_ETHIF_SEL>,
> < CLK_ETHSYS_ESW>,
> < CLK_ETHSYS_GP2>,
> < CLK_ETHSYS_GP1>;
>clock-names = "ethif", "esw", "gp2", "gp1";
>interrupts =   GIC_SPI 199 IRQ_TYPE_LEVEL_LOW
>  GIC_SPI 198 IRQ_TYPE_LEVEL_LOW>;
>power-domains = < MT2701_POWER_DOMAIN_ETH>;
>resets = < MT2701_ETHSYS_ETH_RST>;
>reset-names = "eth";
>mediatek,ethsys = <>;
>mediatek,pctl = <_pctl_a>;
>#address-cells = <1>;
>#size-cells = <0>;
>
>gmac1: mac@0 {
>compatible = "mediatek,eth-mac";
>reg = <0>;
>};
>
>gmac2: mac@1 {
>compatible = "mediatek,eth-mac";
>reg = <1>;
>};
>
>mdio-bus {
>   reg = <1>;
>   #address-cells = <1>;
>   #size-cells = <0>;
>
>   switch0: switch0@0 {
>   compatible = "marvell,mv88e6085";
>   #address-cells = <1>;
>   #size-cells = <0>;
>   reg = <0>;
>   dsa,member = <0 0>;
>
>   ports {
>   #address-cells = <1>;
>   #size-cells = <0>;
>   port@0 {
>   reg = <0>;
>   label = "lan0";
>...
>...
>In this case the switch is an MDIO device, not an PHY. It will not
>have an phy-mode. It cannot have a phy mode, it is not a PHY.
>
>Or am i missing something here?
>
>Thanks
>

1)

The switch driver is not supported for DSA so far yet 
but DSA is good thing and I will try make it happen
in the near future.

And another question about DSA, that is
if I use DSA for switch, how to know the relationship
between MAC and DSA ? such like I could know relationship 
between MAC and PHY by phy-handle.

The cause I ask is becasue I think it's good if the topology
about MAC/PHYs/Switch is known just by dts files.

2)

The phy-mode I mention is for fixed-link. For current MAC driver, 
it just uses fixed phy to adapt into the part of switch, so the 
device tree looks something like the below. 

 {
status = "okay";
gmac0: mac@0 {
compatible = "mediatek,eth-mac";
reg = <0>;
phy-mode = "trgmii";
fixed-link {
speed = <1000>;
full-duplex;
pause;
};
};

gmac1: mac@1 {
compatible = "mediatek,eth-mac";
reg = <1>;
phy-handle = <>;
};

Re: [PATCH net v2 9/9] net: ethernet: mediatek: fix error handling inside mtk_mdio_init

[Sean Wang]

Date: Mon, 29 Aug 2016 15:15:58 +0200,Andrew Lunn wrote:
>On Mon, Aug 29, 2016 at 01:03:23PM +0800, sean.w...@mediatek.com wrote:
>> From: Sean Wang <sean.w...@mediatek.com>
>> 
>> return -ENODEV if no child is found in MDIO bus.
>
>Hi Sean
>
>Why is it an error not to have any children on the bus?
>
>Say i have a fibre optical module connected to the MAC. It is unlikely
>to have an MII interface, so i would not list it on the bus. With this
>change, if i have the mdio-bus node in my device tree, i don't get a
>working system. Without this change, it simply does not instantiate
>the MDIO device, and returns without an error.
>
>I think this patch should be dropped, or maybe a comment adding, why
>the current code returns 0 at this point.
>
>Andrew
>
Hi Andrew,

Sorry, i didn't add the comment enough and well on the patch for let you can't 
see 
what i am done

the patch I just want to let driver know if device tree is defined well at the 
earlier time

although original driver still works without this patch 

the original logic on the driver is 

1) returning -ENODEV if no mdio_bus defined in the device tree. 

2) returning -ENODEV inside ndo_init callback if no phy_dev detected on the bus

--
after with the patch, the logic becomes

1) returning -ENODEV if no mdio_bus defined in the device tree

2) returning -ENODEV if no phy_dev defined in the device tree 
a. that is used to check if the device tree about phy_dev is defined well
b. and it could be aligned with the above 1) is doing

3) returning -ENODEV inside ndo_init callback if no phy device detected on the 
bus 
(that is used to test if phy_device is workable or encounters real phy problems)

so add 2) help to make distinguish if it is a device tree definition 
problem or a real phy problem at the earlier time

this is my whole thought

thanks,
Sean

>> 
>> Signed-off-by: Sean Wang <sean.w...@mediatek.com>
>> Acked-by: John Crispin <j...@phrozen.org>
>> ---
>>  drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
>> b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> index f741c6a..e48b2a4 100644
>> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> @@ -304,7 +304,7 @@ static int mtk_mdio_init(struct mtk_eth *eth)
>>  }
>>  
>>  if (!of_device_is_available(mii_np)) {
>> -ret = 0;
>> +ret = -ENODEV;
>>  goto err_put_node;
>>  }
>>  
>> -- 
>> 1.9.1
>

Re: [RESEND PATCH net 06/10] net: ethernet: mediatek: fix the loss

[Sean Wang]

Date: Fri, 26 Aug 2016 16:17:59 +0200, Andrew Lunn wrote:
>> Hi Andrew,
>> 
>> Here pinctrl is used to setup what function the group of the pins is
>> for.
>
>Agreed.
> 
>> The group of the pins could be configured for the function provided 
>> by the SoC, such as general purpose I/O or specific function such as
>> ethernet depending on what products or boards you design for various 
>> customers or vendors. Thanks for device tree introducing, it is easy 
>> to find what resources the board needs including the pins usage is 
>> also defined here.
>
>All clear. However, if the ethernet driver has loaded, it means the
>device tree says the ethernet should be loaded, unless it happens to
>be on some discoverable bus. And so the device tree node for the
>ethernet should also contain the needed pinctrl properties.  The core
>driver code should of seen these properties and already enabled the
>correct pinctrl state before the driver probes.
>
>This is how every other driver works. Like i said, i don't think i've
>seen any other driver do its own pinctrl. So i just need a simple
>description, what is different here, why does this driver need to do
>it, when no other does?
>
>Andrew
>

You are right
all that I need about pinctrl are all being done with core driver 
as you said, so the patch I did seems the redundant work and i will remove 
it from the patch set.

thanks for your patient and careful reviewing and that also helps me getting 
familiar with based driver with pinctrl more :)

Sean

Re: [RESEND PATCH net 07/10] net: ethernet: mediatek: fix issue of driver removal with interface is up

[Sean Wang]

Date: Thu, 25 Aug 2016 15:35:34 +0200, John Crispin wrote:
>On 25/08/2016 12:44, Sean Wang wrote:
>> 1) mtk_stop() must be called to stop for freeing DMA resources
>> acquired and restoring state changed by mtk_open() when module
>> removal.
>> 
>> 2) group clock disabled related function into mtk_hw_deinit which
>> could be reused with others functionality such as the whole ethernet
>> reset that would be posted in the later series of patches.
>> 
>
>Hi Sean,
>
>these are 2 unrelated changes so they really need to go into two
>separate patches. i also think that change 1) would better fit into the
>future series making use of that functionality.
>
>   John
>

okay. splitting makes sense more

I will leave 1) here 
and move 2) into the better place that is one part of the future series 
about whole ethernet reset, I will post them after the current series is done

thanks for your effort on reviewing

>> Signed-off-by: Sean Wang <sean.w...@mediatek.com>
>> ---
>>  drivers/net/ethernet/mediatek/mtk_eth_soc.c | 22 ++
>>  1 file changed, 18 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
>> b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> index 0a4c782..c573475 100644
>> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> @@ -1478,6 +1478,16 @@ static int __init mtk_hw_init(struct mtk_eth *eth)
>>  return 0;
>>  }
>>  
>> +static int mtk_hw_deinit(struct mtk_eth *eth)
>> +{
>> +clk_disable_unprepare(eth->clk_esw);
>> +clk_disable_unprepare(eth->clk_gp1);
>> +clk_disable_unprepare(eth->clk_gp2);
>> +clk_disable_unprepare(eth->clk_ethif);
>> +
>> +return 0;
>> +}
>> +
>>  static int __init mtk_init(struct net_device *dev)
>>  {
>>  struct mtk_mac *mac = netdev_priv(dev);
>> @@ -1919,11 +1929,15 @@ err_free_dev:
>>  static int mtk_remove(struct platform_device *pdev)
>>  {
>>  struct mtk_eth *eth = platform_get_drvdata(pdev);
>> +int i;
>>  
>> -clk_disable_unprepare(eth->clk_ethif);
>> -clk_disable_unprepare(eth->clk_esw);
>> -clk_disable_unprepare(eth->clk_gp1);
>> -clk_disable_unprepare(eth->clk_gp2);
>> +/* stop all devices to make sure that dma is properly shut down */
>> +for (i = 0; i < MTK_MAC_COUNT; i++) {
>> +if (!eth->netdev[i])
>> +continue;
>> +mtk_stop(eth->netdev[i]);
>> +}
>> +mtk_hw_deinit(eth);
>>  
>>  netif_napi_del(>tx_napi);
>>  netif_napi_del(>rx_napi);
>> 

Re: [RESEND PATCH net 02/10] net: ethernet: mediatek: fix incorrect

[Sean Wang]

On Date: Thu, 25 Aug 2016 15:49:10 +0200, John Crispin wrote:
>On 25/08/2016 12:44, Sean Wang wrote:
>> If the return value of devm_clk_get is EPROBE_DEFER, we should
>> defer probing the driver. The change is verified and works based
>> on 4.8-rc1 staying with the latest clk-next code for MT7623.
>> 
>> Signed-off-by: Sean Wang <sean.w...@mediatek.com>
>> ---
>>  drivers/net/ethernet/mediatek/mtk_eth_soc.c | 11 +--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
...
>> +PTR_ERR(eth->clk_gp2) == -EPROBE_DEFER)
>> +return -EPROBE_DEFER;
>> +else
>> +return -ENODEV;
>> +}
>
>Hi Sean,
>
>this looks a bit tedious. maybe a better solution would be to add an
>array to struct mtk_eth for the clocks and an enum for the index
>mapping. that would allow the usage of loops to work out if all clocks
>are fine. the following code calling clk_prepare_enable() could then
>also be turned into a loop
>
>   John

The suggestion is better, so I will use your suggested way to 
to implement the logic in the next version.

>
>>  
>>  clk_prepare_enable(eth->clk_ethif);
>>  clk_prepare_enable(eth->clk_esw);
>> 

Re: [RESEND PATCH net 06/10] net: ethernet: mediatek: fix the loss

[Sean Wang]

On Thu, 25 Aug 2016 15:30:34 +0200, Andrew Lunn wrote:
>On Thu, Aug 25, 2016 at 06:44:57PM +0800, Sean Wang wrote:
>> ommited the setting about pin-mux which results in incorrect signals
>> being routed on GMAC2.
>
>Hi Sean
>
>Please could you explain this some more. I don't know too much about
>pinctrl, but i've never seen a driver have to do anything with it. The
>core driver code handles it all, selecting the default state. See
>seeing this here makes me wonder if it is correct.
>
>Thanks
>   Andrew
>
>>

Hi Andrew,

Here pinctrl is used to setup what function the group of the pins is for.

The group of the pins could be configured for the function provided 
by the SoC, such as general purpose I/O or specific function such as
ethernet depending on what products or boards you design for various 
customers or vendors. Thanks for device tree introducing, it is easy 
to find what resources the board needs including the pins usage is 
also defined here.
 
Pins are limited resource that is also meant for the cost 
so that it is common way seen on embedded system.

thanks,
Sean

>> +eth->pins = devm_pinctrl_get(>dev);
>> +if (IS_ERR(eth->pins)) {
>> +dev_err(>dev, "cannot get pinctrl\n");
>> +return PTR_ERR(eth->pins);
>> +}
>> +
>> +eth->ephy_default =
>> +pinctrl_lookup_state(eth->pins, "default");
>> +if (IS_ERR(eth->ephy_default)) {
>> +dev_err(>dev, "cannot get pinctrl state\n");
>> +return PTR_ERR(eth->ephy_default);
>> +}
>> +
>>  clk_prepare_enable(eth->clk_ethif);
>>  clk_prepare_enable(eth->clk_esw);
>>  clk_prepare_enable(eth->clk_gp1);
>> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h 
>> b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
>> index f82e3ac..13d3f1b 100644
>> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
>> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
>> @@ -404,6 +404,9 @@ struct mtk_eth {
>>  struct clk  *clk_esw;
>>  struct clk  *clk_gp1;
>>  struct clk  *clk_gp2;
>> +struct pinctrl  *pins;
>> +struct pinctrl_state*ephy_default;
>> +
>>  struct mii_bus  *mii_bus;
>>  struct work_struct  pending_work;
>>  };
>> -- 
>> 1.9.1

[RESEND PATCH net 05/10] net: ethernet: mediatek: fix logic unbalance between probe and remove

[Sean Wang]

original mdio_cleanup is not in the symmetric place against where
mdio_init is, so relocate mdio_cleanup to the right one.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 9883dac..5bd31f8 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1504,7 +1504,6 @@ static void mtk_uninit(struct net_device *dev)
struct mtk_eth *eth = mac->hw;
 
phy_disconnect(mac->phy_dev);
-   mtk_mdio_cleanup(eth);
mtk_irq_disable(eth, ~0);
 }
 
@@ -1915,6 +1914,7 @@ static int mtk_remove(struct platform_device *pdev)
netif_napi_del(>tx_napi);
netif_napi_del(>rx_napi);
mtk_cleanup(eth);
+   mtk_mdio_cleanup(eth);
 
return 0;
 }
-- 
1.9.1

[RESEND PATCH net 02/10] net: ethernet: mediatek: fix incorrect return value of devm_clk_get with EPROBE_DEFER

[Sean Wang]

If the return value of devm_clk_get is EPROBE_DEFER, we should
defer probing the driver. The change is verified and works based
on 4.8-rc1 staying with the latest clk-next code for MT7623.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 6e4a6ca..02b048f 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1851,8 +1851,15 @@ static int mtk_probe(struct platform_device *pdev)
eth->clk_gp1 = devm_clk_get(>dev, "gp1");
eth->clk_gp2 = devm_clk_get(>dev, "gp2");
if (IS_ERR(eth->clk_esw) || IS_ERR(eth->clk_gp1) ||
-   IS_ERR(eth->clk_gp2) || IS_ERR(eth->clk_ethif))
-   return -ENODEV;
+   IS_ERR(eth->clk_gp2) || IS_ERR(eth->clk_ethif)) {
+   if (PTR_ERR(eth->clk_esw) == -EPROBE_DEFER ||
+   PTR_ERR(eth->clk_gp1) == -EPROBE_DEFER ||
+   PTR_ERR(eth->clk_gp1) == -EPROBE_DEFER ||
+   PTR_ERR(eth->clk_gp2) == -EPROBE_DEFER)
+   return -EPROBE_DEFER;
+   else
+   return -ENODEV;
+   }
 
clk_prepare_enable(eth->clk_ethif);
clk_prepare_enable(eth->clk_esw);
-- 
1.9.1

[RESEND PATCH net 03/10] net: ethernet: mediatek: fix API usage with skb_free_frag

[Sean Wang]

use skb_free_frag() instead of legacy put_page()

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 02b048f..1b131a1 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -864,7 +864,7 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
/* receive data */
skb = build_skb(data, ring->frag_size);
if (unlikely(!skb)) {
-   put_page(virt_to_head_page(new_data));
+   skb_free_frag(new_data);
netdev->stats.rx_dropped++;
goto release_desc;
}
-- 
1.9.1

[RESEND PATCH net 00/10] net: ethernet: mediatek: a couple of fixes

[Sean Wang]

a couple of fixes come out from integrating with linux-4.8 rc1
they all are verified and workable on linux-4.8 rc1

Sean Wang (10):
  net: ethernet: mediatek: fix fails from TX housekeeping due to
incorrect port setup
  net: ethernet: mediatek: fix incorrect return value of devm_clk_get
with EPROBE_DEFER
  net: ethernet: mediatek: fix API usage with skb_free_frag
  net: ethernet: mediatek: remove redundant free_irq for
devm_request_irq allocated irq
  net: ethernet: mediatek: fix logic unbalance between probe and remove
  net: ethernet: mediatek: fix the loss of pin-mux setting for GMAC2
  net: ethernet: mediatek: fix issue of driver removal with interface is
up
  net: ethernet: mediatek: fix the missing of_node_put() after node is
used done inside mtk_mdio_init
  net: ethernet: mediatek: use devm_mdiobus_alloc instead of
mdiobus_alloc inside mtk_mdio_init
  net: ethernet: mediatek: fix error handling inside mtk_mdio_init

 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 74 +++--
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |  3 ++
 2 files changed, 52 insertions(+), 25 deletions(-)

-- 
1.9.1

[RESEND PATCH net 10/10] net: ethernet: mediatek: fix error handling inside mtk_mdio_init

[Sean Wang]

return -ENODEV if no child is found in MDIO bus.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 05d85da..2d547c2 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -300,7 +300,7 @@ static int mtk_mdio_init(struct mtk_eth *eth)
}
 
if (!of_device_is_available(mii_np)) {
-   err = 0;
+   err = -ENODEV;
goto err_put_node;
}
 
-- 
1.9.1

[RESEND PATCH net 07/10] net: ethernet: mediatek: fix issue of driver removal with interface is up

[Sean Wang]

1) mtk_stop() must be called to stop for freeing DMA resources
acquired and restoring state changed by mtk_open() when module
removal.

2) group clock disabled related function into mtk_hw_deinit which
could be reused with others functionality such as the whole ethernet
reset that would be posted in the later series of patches.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 22 ++
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 0a4c782..c573475 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1478,6 +1478,16 @@ static int __init mtk_hw_init(struct mtk_eth *eth)
return 0;
 }
 
+static int mtk_hw_deinit(struct mtk_eth *eth)
+{
+   clk_disable_unprepare(eth->clk_esw);
+   clk_disable_unprepare(eth->clk_gp1);
+   clk_disable_unprepare(eth->clk_gp2);
+   clk_disable_unprepare(eth->clk_ethif);
+
+   return 0;
+}
+
 static int __init mtk_init(struct net_device *dev)
 {
struct mtk_mac *mac = netdev_priv(dev);
@@ -1919,11 +1929,15 @@ err_free_dev:
 static int mtk_remove(struct platform_device *pdev)
 {
struct mtk_eth *eth = platform_get_drvdata(pdev);
+   int i;
 
-   clk_disable_unprepare(eth->clk_ethif);
-   clk_disable_unprepare(eth->clk_esw);
-   clk_disable_unprepare(eth->clk_gp1);
-   clk_disable_unprepare(eth->clk_gp2);
+   /* stop all devices to make sure that dma is properly shut down */
+   for (i = 0; i < MTK_MAC_COUNT; i++) {
+   if (!eth->netdev[i])
+   continue;
+   mtk_stop(eth->netdev[i]);
+   }
+   mtk_hw_deinit(eth);
 
netif_napi_del(>tx_napi);
netif_napi_del(>rx_napi);
-- 
1.9.1

[RESEND PATCH net 04/10] net: ethernet: mediatek: remove redundant free_irq for devm_request_irq allocated irq

[Sean Wang]

these irqs are not used for shared irq and disabled during ethernet stops.
irq requested by devm_request_irq is safe to be freed automatically on
driver detach.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 1b131a1..9883dac 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1506,8 +1506,6 @@ static void mtk_uninit(struct net_device *dev)
phy_disconnect(mac->phy_dev);
mtk_mdio_cleanup(eth);
mtk_irq_disable(eth, ~0);
-   free_irq(eth->irq[1], dev);
-   free_irq(eth->irq[2], dev);
 }
 
 static int mtk_do_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
-- 
1.9.1

[RESEND PATCH net 06/10] net: ethernet: mediatek: fix the loss of pin-mux setting for GMAC2

[Sean Wang]

ommited the setting about pin-mux which results in incorrect signals
being routed on GMAC2.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 14 ++
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |  3 +++
 2 files changed, 17 insertions(+)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 5bd31f8..0a4c782 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1415,6 +1415,7 @@ static int __init mtk_hw_init(struct mtk_eth *eth)
usleep_range(10, 20);
reset_control_deassert(eth->rstc);
usleep_range(10, 20);
+   pinctrl_select_state(eth->pins, eth->ephy_default);
 
/* Set GE2 driving and slew rate */
regmap_write(eth->pctl, GPIO_DRV_SEL10, 0xa00);
@@ -1858,6 +1859,19 @@ static int mtk_probe(struct platform_device *pdev)
return -ENODEV;
}
 
+   eth->pins = devm_pinctrl_get(>dev);
+   if (IS_ERR(eth->pins)) {
+   dev_err(>dev, "cannot get pinctrl\n");
+   return PTR_ERR(eth->pins);
+   }
+
+   eth->ephy_default =
+   pinctrl_lookup_state(eth->pins, "default");
+   if (IS_ERR(eth->ephy_default)) {
+   dev_err(>dev, "cannot get pinctrl state\n");
+   return PTR_ERR(eth->ephy_default);
+   }
+
clk_prepare_enable(eth->clk_ethif);
clk_prepare_enable(eth->clk_esw);
clk_prepare_enable(eth->clk_gp1);
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index f82e3ac..13d3f1b 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -404,6 +404,9 @@ struct mtk_eth {
struct clk  *clk_esw;
struct clk  *clk_gp1;
struct clk  *clk_gp2;
+   struct pinctrl  *pins;
+   struct pinctrl_state*ephy_default;
+
struct mii_bus  *mii_bus;
struct work_struct  pending_work;
 };
-- 
1.9.1

[RESEND PATCH net 01/10] net: ethernet: mediatek: fix fails from TX housekeeping due to incorrect port setup

[Sean Wang]

which net device the SKB is complete for depends on the forward port
on txd4 on the corresponding TX descriptor, but the information isn't
set up well in case of  SKB fragments that would lead to watchdog timeout
from the upper layer, so fix it up.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 1801fd8..6e4a6ca 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -587,14 +587,15 @@ static int mtk_tx_map(struct sk_buff *skb, struct 
net_device *dev,
dma_addr_t mapped_addr;
unsigned int nr_frags;
int i, n_desc = 1;
-   u32 txd4 = 0;
+   u32 txd4 = 0, fport;
 
itxd = ring->next_free;
if (itxd == ring->last_free)
return -ENOMEM;
 
/* set the forward port */
-   txd4 |= (mac->id + 1) << TX_DMA_FPORT_SHIFT;
+   fport = (mac->id + 1) << TX_DMA_FPORT_SHIFT;
+   txd4 |= fport;
 
tx_buf = mtk_desc_to_tx_buf(ring, itxd);
memset(tx_buf, 0, sizeof(*tx_buf));
@@ -652,7 +653,7 @@ static int mtk_tx_map(struct sk_buff *skb, struct 
net_device *dev,
WRITE_ONCE(txd->txd3, (TX_DMA_SWC |
   TX_DMA_PLEN0(frag_map_size) |
   last_frag * TX_DMA_LS0));
-   WRITE_ONCE(txd->txd4, 0);
+   WRITE_ONCE(txd->txd4, fport);
 
tx_buf->skb = (struct sk_buff *)MTK_DMA_DUMMY_DESC;
tx_buf = mtk_desc_to_tx_buf(ring, txd);
-- 
1.9.1

[RESEND PATCH net 08/10] net: ethernet: mediatek: fix the missing of_node_put() after node is used done inside mtk_mdio_init

[Sean Wang]

This patch adds the missing of_node_put() after finishing the usage
of of_get_child_by_name.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index c573475..e3baa63 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -320,6 +320,7 @@ static int mtk_mdio_init(struct mtk_eth *eth)
err = of_mdiobus_register(eth->mii_bus, mii_np);
if (err)
goto err_free_bus;
+   of_node_put(mii_np);
 
return 0;
 
-- 
1.9.1

[RESEND PATCH net 09/10] net: ethernet: mediatek: use devm_mdiobus_alloc instead of mdiobus_alloc inside mtk_mdio_init

[Sean Wang]

a lot of parts in the driver uses devm_* APIs to gain benefits from the
device resource management, so devm_mdiobus_alloc is also used instead
of mdiobus_alloc to have more elegant code flow.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 13 +
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index e3baa63..05d85da 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -304,7 +304,7 @@ static int mtk_mdio_init(struct mtk_eth *eth)
goto err_put_node;
}
 
-   eth->mii_bus = mdiobus_alloc();
+   eth->mii_bus = devm_mdiobus_alloc(eth->dev);
if (!eth->mii_bus) {
err = -ENOMEM;
goto err_put_node;
@@ -318,18 +318,9 @@ static int mtk_mdio_init(struct mtk_eth *eth)
 
snprintf(eth->mii_bus->id, MII_BUS_ID_SIZE, "%s", mii_np->name);
err = of_mdiobus_register(eth->mii_bus, mii_np);
-   if (err)
-   goto err_free_bus;
-   of_node_put(mii_np);
-
-   return 0;
-
-err_free_bus:
-   mdiobus_free(eth->mii_bus);
 
 err_put_node:
of_node_put(mii_np);
-   eth->mii_bus = NULL;
return err;
 }
 
@@ -339,8 +330,6 @@ static void mtk_mdio_cleanup(struct mtk_eth *eth)
return;
 
mdiobus_unregister(eth->mii_bus);
-   of_node_put(eth->mii_bus->dev.of_node);
-   mdiobus_free(eth->mii_bus);
 }
 
 static inline void mtk_irq_disable(struct mtk_eth *eth, u32 mask)
-- 
1.9.1

[PATCH net-next v3 0/2] Add enhancements to RX path

[Sean Wang]

This patch set fix gives some enhancements about RX path handling.
and thanks for Sergei Shtylyov helps reviewing during v2 to v3.

v1 -> v2: Fix message typos and add coverletter
  
v2 -> v3: 
Split from the previous series for submitting add enhancements 
as a series targeting 'net-next' and add indents before comments. 

Sean Wang (2):
  net: ethernet: mediatek: enhance RX path by reducing the frequency of
the memory barrier used
  net: ethernet: mediatek: enhance RX path by aggregating more SKBs
into NAPI

 drivers/net/ethernet/mediatek/mtk_eth_soc.c |   24 
 1 file changed, 12 insertions(+), 12 deletions(-)

-- 
1.7.9.5

[PATCH net-next v3 2/2] net: ethernet: mediatek: enhance RX path by aggregating more SKBs into NAPI

[Sean Wang]

The patch adds support for aggregating more SKBs feed into NAPI in
order to get more benefits from generic receive offload (GRO) by
peeking at the RX ring status and moving more packets right before
returning from NAPI RX polling handler if NAPI budgets are still
available.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |   15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 589ff50..d340673 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -902,9 +902,6 @@ release_desc:
wmb();
mtk_w32(eth, ring->calc_idx, MTK_QRX_CRX_IDX0);
 
-   if (done < budget)
-   mtk_w32(eth, MTK_RX_DONE_INT, MTK_QMTK_INT_STATUS);
-
return done;
 }
 
@@ -1023,8 +1020,10 @@ static int mtk_napi_rx(struct napi_struct *napi, int 
budget)
struct mtk_eth *eth = container_of(napi, struct mtk_eth, rx_napi);
u32 status, mask;
int rx_done = 0;
+   int remain_budget = budget;
 
mtk_handle_status_irq(eth);
+poll_again:
mtk_w32(eth, MTK_RX_DONE_INT, MTK_QMTK_INT_STATUS);
rx_done = mtk_poll_rx(napi, budget, eth);
 
@@ -1035,14 +1034,14 @@ static int mtk_napi_rx(struct napi_struct *napi, int 
budget)
 "done rx %d, intr 0x%08x/0x%x\n",
 rx_done, status, mask);
}
-
-   if (rx_done == budget)
+   if (rx_done == remain_budget)
return budget;
 
status = mtk_r32(eth, MTK_QMTK_INT_STATUS);
-   if (status & MTK_RX_DONE_INT)
-   return budget;
-
+   if (status & MTK_RX_DONE_INT) {
+   remain_budget -= rx_done;
+   goto poll_again;
+   }
napi_complete(napi);
mtk_irq_enable(eth, MTK_RX_DONE_INT);
 
-- 
1.7.9.5

[PATCH net-next v3 1/2] net: ethernet: mediatek: enhance RX path by reducing the frequency of the memory barrier used

[Sean Wang]

The patch makes moving wmb() to outside the loop that could help
RX path handling more faster although that RX descriptors aren't
freed for DMA to use as soon as possible, but based on my experiment
and the result shows it still can reach about 943Mbpis without
performance drop that is tested based on the setup with one port
using Giga PHY and 256 RX descriptors for DMA to move.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |   11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index b782330..589ff50 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -893,14 +893,15 @@ release_desc:
rxd->rxd2 = RX_DMA_PLEN0(ring->buf_size);
 
ring->calc_idx = idx;
-   /* make sure that all changes to the dma ring are flushed before
-* we continue
-*/
-   wmb();
-   mtk_w32(eth, ring->calc_idx, MTK_QRX_CRX_IDX0);
done++;
}
 
+   /* make sure that all changes to the dma ring are flushed before
+* we continue
+*/
+   wmb();
+   mtk_w32(eth, ring->calc_idx, MTK_QRX_CRX_IDX0);
+
if (done < budget)
mtk_w32(eth, MTK_RX_DONE_INT, MTK_QMTK_INT_STATUS);
 
-- 
1.7.9.5

[PATCH net v3 1/3] net: ethernet: mediatek: fix RMII mode and add REVMII supported by GMAC

[Sean Wang]

The patch fixes up the incorrect setup of reduced MII (RMII) on GMAC
and adds the supplement for the setup of reverse MII (REVMII) on GMAC
, and rearranges the error handling for invalid PHY argument.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |   15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 88b04dd..f19b8b9 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -245,12 +245,16 @@ static int mtk_phy_connect(struct mtk_mac *mac)
case PHY_INTERFACE_MODE_MII:
ge_mode = 1;
break;
-   case PHY_INTERFACE_MODE_RMII:
+   case PHY_INTERFACE_MODE_REVMII:
ge_mode = 2;
break;
+   case PHY_INTERFACE_MODE_RMII:
+   if (!mac->id)
+   goto err_phy;
+   ge_mode = 3;
+   break;
default:
-   dev_err(eth->dev, "invalid phy_mode\n");
-   return -1;
+   goto err_phy;
}
 
/* put the gmac into the right mode */
@@ -272,6 +276,11 @@ static int mtk_phy_connect(struct mtk_mac *mac)
of_node_put(np);
 
return 0;
+
+err_phy:
+   of_node_put(np);
+   dev_err(eth->dev, "invalid phy_mode\n");
+   return -EINVAL;
 }
 
 static int mtk_mdio_init(struct mtk_eth *eth)
-- 
1.7.9.5

[PATCH net v3 0/3] Fix warning and issue

[Sean Wang]

This patch set fixes the following warning and issues

v1 -> v2: Fix message typos and add coverletter

v2 -> v3: Split from the previous series for submitting bug fixes 
as a series targeting 'net'

Sean Wang (3):
  net: ethernet: mediatek: fix RMII mode and add REVMII supported by
GMAC
  net: ethernet: mediatek: fix flow control settings on GMAC0 is not
being enabled properly
  net: ethernet: mediatek: fix runtime warning raised by inconsistent
struct device pointers passed to DMA API

 drivers/net/ethernet/mediatek/mtk_eth_soc.c |   46 +--
 1 file changed, 30 insertions(+), 16 deletions(-)

-- 
1.7.9.5

[PATCH net v3 2/3] net: ethernet: mediatek: fix flow control settings on GMAC0 is not being enabled properly

[Sean Wang]

Commit 08ef55c6f257acf3bdc6940813f80e8f0f5d90ec
("net-next: mediatek: fix gigabit and flow control advertisement")
had supported proper flow control settings for GMAC1. But for GMAC0,

1.GMAC0 shares the common logic with GMAC1 inside mtk_phy_link_adjust()
to adapt various settings for the target phy.

2.GMAC0 uses fixed-phy to connect to a builtin gigabit switch with
fixed link speed as commit 0c72c50f6f93b0c3daa9ea35d89ab3a933c7b5a0
("net-next: mediatek: add fixed-phy support") describes.

3.However, fixed-phy doesn't enable SUPPORTED_Pause & SUPPORTED_Asym_Pause
supported flag on default that would cause mtk_phy_link_adjust() not to
enable flow control setting on GMAC0 properly and cause packet dropped
when high traffic.

Due to these reasons, the patch adds SUPPORTED_Pause & SUPPORTED_Asym_Pause
supported flags on fixed-phy used by the driver to have proper handling on
the both GMAC with the shared common logic.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index f19b8b9..eb59a73 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -267,6 +267,11 @@ static int mtk_phy_connect(struct mtk_mac *mac)
mac->phy_dev->autoneg = AUTONEG_ENABLE;
mac->phy_dev->speed = 0;
mac->phy_dev->duplex = 0;
+
+   if (of_phy_is_fixed_link(mac->of_node))
+   mac->phy_dev->supported |=
+   SUPPORTED_Pause | SUPPORTED_Asym_Pause;
+
mac->phy_dev->supported &= PHY_GBIT_FEATURES | SUPPORTED_Pause |
   SUPPORTED_Asym_Pause;
mac->phy_dev->advertising = mac->phy_dev->supported |
-- 
1.7.9.5

[PATCH net v3 3/3] net: ethernet: mediatek: fix runtime warning raised by inconsistent struct device pointers passed to DMA API

[Sean Wang]

Runtime warning occurs if DMA-API debug feature is enabled that would be
raised by pointers passed to DMA API as arguments to inconsistent struct
device objects, so that the patch makes them usage aligned between DMA
operations such as dma_map_*() and dma_unmap_*() to eliminate the warning.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |   26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index eb59a73..b782330 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -557,15 +557,15 @@ static inline struct mtk_tx_buf 
*mtk_desc_to_tx_buf(struct mtk_tx_ring *ring,
return >buf[idx];
 }
 
-static void mtk_tx_unmap(struct device *dev, struct mtk_tx_buf *tx_buf)
+static void mtk_tx_unmap(struct mtk_eth *eth, struct mtk_tx_buf *tx_buf)
 {
if (tx_buf->flags & MTK_TX_FLAGS_SINGLE0) {
-   dma_unmap_single(dev,
+   dma_unmap_single(eth->dev,
 dma_unmap_addr(tx_buf, dma_addr0),
 dma_unmap_len(tx_buf, dma_len0),
 DMA_TO_DEVICE);
} else if (tx_buf->flags & MTK_TX_FLAGS_PAGE0) {
-   dma_unmap_page(dev,
+   dma_unmap_page(eth->dev,
   dma_unmap_addr(tx_buf, dma_addr0),
   dma_unmap_len(tx_buf, dma_len0),
   DMA_TO_DEVICE);
@@ -610,9 +610,9 @@ static int mtk_tx_map(struct sk_buff *skb, struct 
net_device *dev,
if (skb_vlan_tag_present(skb))
txd4 |= TX_DMA_INS_VLAN | skb_vlan_tag_get(skb);
 
-   mapped_addr = dma_map_single(>dev, skb->data,
+   mapped_addr = dma_map_single(eth->dev, skb->data,
 skb_headlen(skb), DMA_TO_DEVICE);
-   if (unlikely(dma_mapping_error(>dev, mapped_addr)))
+   if (unlikely(dma_mapping_error(eth->dev, mapped_addr)))
return -ENOMEM;
 
WRITE_ONCE(itxd->txd1, mapped_addr);
@@ -638,10 +638,10 @@ static int mtk_tx_map(struct sk_buff *skb, struct 
net_device *dev,
 
n_desc++;
frag_map_size = min(frag_size, MTK_TX_DMA_BUF_LEN);
-   mapped_addr = skb_frag_dma_map(>dev, frag, offset,
+   mapped_addr = skb_frag_dma_map(eth->dev, frag, offset,
   frag_map_size,
   DMA_TO_DEVICE);
-   if (unlikely(dma_mapping_error(>dev, mapped_addr)))
+   if (unlikely(dma_mapping_error(eth->dev, mapped_addr)))
goto err_dma;
 
if (i == nr_frags - 1 &&
@@ -694,7 +694,7 @@ err_dma:
tx_buf = mtk_desc_to_tx_buf(ring, itxd);
 
/* unmap dma */
-   mtk_tx_unmap(>dev, tx_buf);
+   mtk_tx_unmap(eth, tx_buf);
 
itxd->txd3 = TX_DMA_LS0 | TX_DMA_OWNER_CPU;
itxd = mtk_qdma_phys_to_virt(ring, itxd->txd2);
@@ -850,11 +850,11 @@ static int mtk_poll_rx(struct napi_struct *napi, int 
budget,
netdev->stats.rx_dropped++;
goto release_desc;
}
-   dma_addr = dma_map_single(>netdev[mac]->dev,
+   dma_addr = dma_map_single(eth->dev,
  new_data + NET_SKB_PAD,
  ring->buf_size,
  DMA_FROM_DEVICE);
-   if (unlikely(dma_mapping_error(>dev, dma_addr))) {
+   if (unlikely(dma_mapping_error(eth->dev, dma_addr))) {
skb_free_frag(new_data);
netdev->stats.rx_dropped++;
goto release_desc;
@@ -869,7 +869,7 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
}
skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
 
-   dma_unmap_single(>dev, trxd.rxd1,
+   dma_unmap_single(eth->dev, trxd.rxd1,
 ring->buf_size, DMA_FROM_DEVICE);
pktlen = RX_DMA_GET_PLEN0(trxd.rxd2);
skb->dev = netdev;
@@ -951,7 +951,7 @@ static int mtk_poll_tx(struct mtk_eth *eth, int budget)
done[mac]++;
budget--;
}
-   mtk_tx_unmap(eth->dev, tx_buf);
+   mtk_tx_unmap(eth, tx_buf);
 
ring->last_free = desc;
atomic_inc(>free_count);
@@ -1106,7 +1106,7

[PATCH v2 3/5] net: ethernet: mediatek: fix runtime warning raised by inconsistent struct device pointers passed to DMA API

[Sean Wang]

Runtime warning occurs if DMA-API debug feature is enabled that would be
raised by pointers passed to DMA API as arguments to inconsistent struct
device objects, so that the patch makes them usage aligned between dma
operations such as dma_map_*() and dma_unmap_*() to eliminate the warning.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |   26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index eb59a73..b782330 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -557,15 +557,15 @@ static inline struct mtk_tx_buf 
*mtk_desc_to_tx_buf(struct mtk_tx_ring *ring,
return >buf[idx];
 }
 
-static void mtk_tx_unmap(struct device *dev, struct mtk_tx_buf *tx_buf)
+static void mtk_tx_unmap(struct mtk_eth *eth, struct mtk_tx_buf *tx_buf)
 {
if (tx_buf->flags & MTK_TX_FLAGS_SINGLE0) {
-   dma_unmap_single(dev,
+   dma_unmap_single(eth->dev,
 dma_unmap_addr(tx_buf, dma_addr0),
 dma_unmap_len(tx_buf, dma_len0),
 DMA_TO_DEVICE);
} else if (tx_buf->flags & MTK_TX_FLAGS_PAGE0) {
-   dma_unmap_page(dev,
+   dma_unmap_page(eth->dev,
   dma_unmap_addr(tx_buf, dma_addr0),
   dma_unmap_len(tx_buf, dma_len0),
   DMA_TO_DEVICE);
@@ -610,9 +610,9 @@ static int mtk_tx_map(struct sk_buff *skb, struct 
net_device *dev,
if (skb_vlan_tag_present(skb))
txd4 |= TX_DMA_INS_VLAN | skb_vlan_tag_get(skb);
 
-   mapped_addr = dma_map_single(>dev, skb->data,
+   mapped_addr = dma_map_single(eth->dev, skb->data,
 skb_headlen(skb), DMA_TO_DEVICE);
-   if (unlikely(dma_mapping_error(>dev, mapped_addr)))
+   if (unlikely(dma_mapping_error(eth->dev, mapped_addr)))
return -ENOMEM;
 
WRITE_ONCE(itxd->txd1, mapped_addr);
@@ -638,10 +638,10 @@ static int mtk_tx_map(struct sk_buff *skb, struct 
net_device *dev,
 
n_desc++;
frag_map_size = min(frag_size, MTK_TX_DMA_BUF_LEN);
-   mapped_addr = skb_frag_dma_map(>dev, frag, offset,
+   mapped_addr = skb_frag_dma_map(eth->dev, frag, offset,
   frag_map_size,
   DMA_TO_DEVICE);
-   if (unlikely(dma_mapping_error(>dev, mapped_addr)))
+   if (unlikely(dma_mapping_error(eth->dev, mapped_addr)))
goto err_dma;
 
if (i == nr_frags - 1 &&
@@ -694,7 +694,7 @@ err_dma:
tx_buf = mtk_desc_to_tx_buf(ring, itxd);
 
/* unmap dma */
-   mtk_tx_unmap(>dev, tx_buf);
+   mtk_tx_unmap(eth, tx_buf);
 
itxd->txd3 = TX_DMA_LS0 | TX_DMA_OWNER_CPU;
itxd = mtk_qdma_phys_to_virt(ring, itxd->txd2);
@@ -850,11 +850,11 @@ static int mtk_poll_rx(struct napi_struct *napi, int 
budget,
netdev->stats.rx_dropped++;
goto release_desc;
}
-   dma_addr = dma_map_single(>netdev[mac]->dev,
+   dma_addr = dma_map_single(eth->dev,
  new_data + NET_SKB_PAD,
  ring->buf_size,
  DMA_FROM_DEVICE);
-   if (unlikely(dma_mapping_error(>dev, dma_addr))) {
+   if (unlikely(dma_mapping_error(eth->dev, dma_addr))) {
skb_free_frag(new_data);
netdev->stats.rx_dropped++;
goto release_desc;
@@ -869,7 +869,7 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
}
skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
 
-   dma_unmap_single(>dev, trxd.rxd1,
+   dma_unmap_single(eth->dev, trxd.rxd1,
 ring->buf_size, DMA_FROM_DEVICE);
pktlen = RX_DMA_GET_PLEN0(trxd.rxd2);
skb->dev = netdev;
@@ -951,7 +951,7 @@ static int mtk_poll_tx(struct mtk_eth *eth, int budget)
done[mac]++;
budget--;
}
-   mtk_tx_unmap(eth->dev, tx_buf);
+   mtk_tx_unmap(eth, tx_buf);
 
ring->last_free = desc;
atomic_inc(>free_count);
@@ -1106,7 +1106,7

[PATCH v2 5/5] net: ethernet: mediatek: enhance RX path by aggregating more skbs into NAPI

[Sean Wang]

The patch adds support for aggregating more skbs feed into NAPI in
order to get more benifits from generic receive offload (GRO) by
peeking at the RX ring status and moving more packets right before
returning from NAPI RX polling handler if NAPI budgets are still
available.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |   15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 53e24c1..742b9ec 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -902,9 +902,6 @@ release_desc:
wmb();
mtk_w32(eth, ring->calc_idx, MTK_QRX_CRX_IDX0);
 
-   if (done < budget)
-   mtk_w32(eth, MTK_RX_DONE_INT, MTK_QMTK_INT_STATUS);
-
return done;
 }
 
@@ -1023,8 +1020,10 @@ static int mtk_napi_rx(struct napi_struct *napi, int 
budget)
struct mtk_eth *eth = container_of(napi, struct mtk_eth, rx_napi);
u32 status, mask;
int rx_done = 0;
+   int remain_budget = budget;
 
mtk_handle_status_irq(eth);
+poll_again:
mtk_w32(eth, MTK_RX_DONE_INT, MTK_QMTK_INT_STATUS);
rx_done = mtk_poll_rx(napi, budget, eth);
 
@@ -1035,14 +1034,14 @@ static int mtk_napi_rx(struct napi_struct *napi, int 
budget)
 "done rx %d, intr 0x%08x/0x%x\n",
 rx_done, status, mask);
}
-
-   if (rx_done == budget)
+   if (rx_done == remain_budget)
return budget;
 
status = mtk_r32(eth, MTK_QMTK_INT_STATUS);
-   if (status & MTK_RX_DONE_INT)
-   return budget;
-
+   if (status & MTK_RX_DONE_INT) {
+   remain_budget -= rx_done;
+   goto poll_again;
+   }
napi_complete(napi);
mtk_irq_enable(eth, MTK_RX_DONE_INT);
 
-- 
1.7.9.5

[PATCH v2 1/5] net: ethernet: mediatek: add REVMII and fix RMII mode supported by GMAC

[Sean Wang]

The patch adds the supplement for the setup of reverse MII (REVMII)
on GMAC, fixes up the incorrect setup of reduced MII (RMII) on GMAC
and rearranges the error handling for invalid phy argument.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |   15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 88b04dd..f19b8b9 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -245,12 +245,16 @@ static int mtk_phy_connect(struct mtk_mac *mac)
case PHY_INTERFACE_MODE_MII:
ge_mode = 1;
break;
-   case PHY_INTERFACE_MODE_RMII:
+   case PHY_INTERFACE_MODE_REVMII:
ge_mode = 2;
break;
+   case PHY_INTERFACE_MODE_RMII:
+   if (!mac->id)
+   goto err_phy;
+   ge_mode = 3;
+   break;
default:
-   dev_err(eth->dev, "invalid phy_mode\n");
-   return -1;
+   goto err_phy;
}
 
/* put the gmac into the right mode */
@@ -272,6 +276,11 @@ static int mtk_phy_connect(struct mtk_mac *mac)
of_node_put(np);
 
return 0;
+
+err_phy:
+   of_node_put(np);
+   dev_err(eth->dev, "invalid phy_mode\n");
+   return -EINVAL;
 }
 
 static int mtk_mdio_init(struct mtk_eth *eth)
-- 
1.7.9.5

[PATCH v2 2/5] net: ethernet: mediatek: fix flow control settings on GMAC0 is not being enabled properly

[Sean Wang]

Commit 08ef55c6f257acf3bdc6940813f80e8f0f5d90ec
("net-next: mediatek: fix gigabit and flow control advertisement")
had supported proper flow control settings for GMAC1. But for GMAC0,

1.GMAC0 shares the common logic with GMAC1 inside mtk_phy_link_adjust()
to adapt various settings for the target phy.

2.GMAC0 uses fixed-phy to connect to a builtin gigabit switch with
fixed link speed as commit 0c72c50f6f93b0c3daa9ea35d89ab3a933c7b5a0
("net-next: mediatek: add fixed-phy support") describes.

3.However, fixed-phy doesn't enable SUPPORTED_Pause & SUPPORTED_Asym_Pause
supported flag on default that would cause mtk_phy_link_adjust() not to
enable flow control setting on GMAC0 properly and cause packet dropped
when high traffic.

Due to these reasons, the patch adds SUPPORTED_Pause & SUPPORTED_Asym_Pause
supported flags on fixed-phy used by driver to have proper handling on
the both GMAC with the shared common logic.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index f19b8b9..eb59a73 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -267,6 +267,11 @@ static int mtk_phy_connect(struct mtk_mac *mac)
mac->phy_dev->autoneg = AUTONEG_ENABLE;
mac->phy_dev->speed = 0;
mac->phy_dev->duplex = 0;
+
+   if (of_phy_is_fixed_link(mac->of_node))
+   mac->phy_dev->supported |=
+   SUPPORTED_Pause | SUPPORTED_Asym_Pause;
+
mac->phy_dev->supported &= PHY_GBIT_FEATURES | SUPPORTED_Pause |
   SUPPORTED_Asym_Pause;
mac->phy_dev->advertising = mac->phy_dev->supported |
-- 
1.7.9.5

[PATCH v2 0/5] Fix warning, issue and give some enhancements

[Sean Wang]

This patch set fixes the following warning and issues and gives some 
enhancements about RX path handling.

v1 -> v2: fix message typos and add coverletter

Sean Wang (5):
  net: ethernet: mediatek: add REVMII and fix RMII mode supported by
GMAC
  net: ethernet: mediatek: fix flow control settings on GMAC0 is not
being enabled properly
  net: ethernet: mediatek: fix runtime warning raised by inconsistent
struct device pointers passed to DMA API
  net: ethernet: mediatek: enhance RX path by reducing the frequency of
the memory barrier used
  net: ethernet: mediatek: enhance RX path by aggregating more skbs
into NAPI

 drivers/net/ethernet/mediatek/mtk_eth_soc.c |   70 ---
 1 file changed, 42 insertions(+), 28 deletions(-)

-- 
1.7.9.5

[PATCH v2 4/5] net: ethernet: mediatek: enhance RX path by reducing the frequency of the memory barrier used

[Sean Wang]

The patch makes moving wmb() to outside the loop that could help
RX path handling more faster although that RX descriptors aren't
freed for DMA to use as soon as possible, but based on my experiment
and the result show it still can reach about 943Mbpis without
performance drop that is tested based on the setup with one port
using gigaphy and 256 RX descriptors for DMA to move.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |   11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index b782330..53e24c1 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -893,14 +893,15 @@ release_desc:
rxd->rxd2 = RX_DMA_PLEN0(ring->buf_size);
 
ring->calc_idx = idx;
-   /* make sure that all changes to the dma ring are flushed before
-* we continue
-*/
-   wmb();
-   mtk_w32(eth, ring->calc_idx, MTK_QRX_CRX_IDX0);
done++;
}
 
+/* make sure that all changes to the dma ring are flushed before
+ * we continue
+ */
+   wmb();
+   mtk_w32(eth, ring->calc_idx, MTK_QRX_CRX_IDX0);
+
if (done < budget)
mtk_w32(eth, MTK_RX_DONE_INT, MTK_QMTK_INT_STATUS);
 
-- 
1.7.9.5

[PATCH 5/5] net: ethernet: mediatek: enable rx path by aggregrating more skbs

[Sean Wang]

The patch adds support for aggregrating more skbs feed into NAPI in
order to get benifits from generic receive offload (GRO) by peeking
at the RX ring status and moving more packet right before returning
from NAPI rx handler if NAPI budgets is still available.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |   15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 53e24c1..742b9ec 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -902,9 +902,6 @@ release_desc:
wmb();
mtk_w32(eth, ring->calc_idx, MTK_QRX_CRX_IDX0);
 
-   if (done < budget)
-   mtk_w32(eth, MTK_RX_DONE_INT, MTK_QMTK_INT_STATUS);
-
return done;
 }
 
@@ -1023,8 +1020,10 @@ static int mtk_napi_rx(struct napi_struct *napi, int 
budget)
struct mtk_eth *eth = container_of(napi, struct mtk_eth, rx_napi);
u32 status, mask;
int rx_done = 0;
+   int remain_budget = budget;
 
mtk_handle_status_irq(eth);
+poll_again:
mtk_w32(eth, MTK_RX_DONE_INT, MTK_QMTK_INT_STATUS);
rx_done = mtk_poll_rx(napi, budget, eth);
 
@@ -1035,14 +1034,14 @@ static int mtk_napi_rx(struct napi_struct *napi, int 
budget)
 "done rx %d, intr 0x%08x/0x%x\n",
 rx_done, status, mask);
}
-
-   if (rx_done == budget)
+   if (rx_done == remain_budget)
return budget;
 
status = mtk_r32(eth, MTK_QMTK_INT_STATUS);
-   if (status & MTK_RX_DONE_INT)
-   return budget;
-
+   if (status & MTK_RX_DONE_INT) {
+   remain_budget -= rx_done;
+   goto poll_again;
+   }
napi_complete(napi);
mtk_irq_enable(eth, MTK_RX_DONE_INT);
 
-- 
1.7.9.5

[PATCH 4/5] net: ethernet: mediatek: enable rx path by reducing memory barrier

[Sean Wang]

The patch makes moving wmb() to outside the loop that could help
rx path handling more faster although that rx descriptors aren't
freed for DMA to use as soon as possible, but based on my experiment
and the result show it still can reach about 943Mbpis tested on the
setup with one port using gigaphy and 256 rx descriptors for
DMA to move.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |   11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index b782330..53e24c1 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -893,14 +893,15 @@ release_desc:
rxd->rxd2 = RX_DMA_PLEN0(ring->buf_size);
 
ring->calc_idx = idx;
-   /* make sure that all changes to the dma ring are flushed before
-* we continue
-*/
-   wmb();
-   mtk_w32(eth, ring->calc_idx, MTK_QRX_CRX_IDX0);
done++;
}
 
+/* make sure that all changes to the dma ring are flushed before
+ * we continue
+ */
+   wmb();
+   mtk_w32(eth, ring->calc_idx, MTK_QRX_CRX_IDX0);
+
if (done < budget)
mtk_w32(eth, MTK_RX_DONE_INT, MTK_QMTK_INT_STATUS);
 
-- 
1.7.9.5

[PATCH 1/5] net: ethernet: mediatek: add REVMII and fix RMII modes supported by GMAC

[Sean Wang]

The patch adds the supplement for the setup of reverse MII (REVMII)
on GMAC, fixes up incorrect setup of reduced mii (RMII) on GMAC and
rearranges the error handling path for invalid phy argument.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |   15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 88b04dd..f19b8b9 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -245,12 +245,16 @@ static int mtk_phy_connect(struct mtk_mac *mac)
case PHY_INTERFACE_MODE_MII:
ge_mode = 1;
break;
-   case PHY_INTERFACE_MODE_RMII:
+   case PHY_INTERFACE_MODE_REVMII:
ge_mode = 2;
break;
+   case PHY_INTERFACE_MODE_RMII:
+   if (!mac->id)
+   goto err_phy;
+   ge_mode = 3;
+   break;
default:
-   dev_err(eth->dev, "invalid phy_mode\n");
-   return -1;
+   goto err_phy;
}
 
/* put the gmac into the right mode */
@@ -272,6 +276,11 @@ static int mtk_phy_connect(struct mtk_mac *mac)
of_node_put(np);
 
return 0;
+
+err_phy:
+   of_node_put(np);
+   dev_err(eth->dev, "invalid phy_mode\n");
+   return -EINVAL;
 }
 
 static int mtk_mdio_init(struct mtk_eth *eth)
-- 
1.7.9.5

[PATCH 3/5] net: ethernet: mediatek: fixed runtime warning raised by inconsistent struct device pointers passed to DMA API

[Sean Wang]

Runtime warning occurs if DMA-API debug feature is enabled that would be
raised by pointers passed to DMA API as arguments to inconsistent struct
device objects, so that the patch makes them usage aligned between dma
operations such as dma_map_*() and dma_unmap_*() to eliminate the warning.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |   26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index eb59a73..b782330 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -557,15 +557,15 @@ static inline struct mtk_tx_buf 
*mtk_desc_to_tx_buf(struct mtk_tx_ring *ring,
return >buf[idx];
 }
 
-static void mtk_tx_unmap(struct device *dev, struct mtk_tx_buf *tx_buf)
+static void mtk_tx_unmap(struct mtk_eth *eth, struct mtk_tx_buf *tx_buf)
 {
if (tx_buf->flags & MTK_TX_FLAGS_SINGLE0) {
-   dma_unmap_single(dev,
+   dma_unmap_single(eth->dev,
 dma_unmap_addr(tx_buf, dma_addr0),
 dma_unmap_len(tx_buf, dma_len0),
 DMA_TO_DEVICE);
} else if (tx_buf->flags & MTK_TX_FLAGS_PAGE0) {
-   dma_unmap_page(dev,
+   dma_unmap_page(eth->dev,
   dma_unmap_addr(tx_buf, dma_addr0),
   dma_unmap_len(tx_buf, dma_len0),
   DMA_TO_DEVICE);
@@ -610,9 +610,9 @@ static int mtk_tx_map(struct sk_buff *skb, struct 
net_device *dev,
if (skb_vlan_tag_present(skb))
txd4 |= TX_DMA_INS_VLAN | skb_vlan_tag_get(skb);
 
-   mapped_addr = dma_map_single(>dev, skb->data,
+   mapped_addr = dma_map_single(eth->dev, skb->data,
 skb_headlen(skb), DMA_TO_DEVICE);
-   if (unlikely(dma_mapping_error(>dev, mapped_addr)))
+   if (unlikely(dma_mapping_error(eth->dev, mapped_addr)))
return -ENOMEM;
 
WRITE_ONCE(itxd->txd1, mapped_addr);
@@ -638,10 +638,10 @@ static int mtk_tx_map(struct sk_buff *skb, struct 
net_device *dev,
 
n_desc++;
frag_map_size = min(frag_size, MTK_TX_DMA_BUF_LEN);
-   mapped_addr = skb_frag_dma_map(>dev, frag, offset,
+   mapped_addr = skb_frag_dma_map(eth->dev, frag, offset,
   frag_map_size,
   DMA_TO_DEVICE);
-   if (unlikely(dma_mapping_error(>dev, mapped_addr)))
+   if (unlikely(dma_mapping_error(eth->dev, mapped_addr)))
goto err_dma;
 
if (i == nr_frags - 1 &&
@@ -694,7 +694,7 @@ err_dma:
tx_buf = mtk_desc_to_tx_buf(ring, itxd);
 
/* unmap dma */
-   mtk_tx_unmap(>dev, tx_buf);
+   mtk_tx_unmap(eth, tx_buf);
 
itxd->txd3 = TX_DMA_LS0 | TX_DMA_OWNER_CPU;
itxd = mtk_qdma_phys_to_virt(ring, itxd->txd2);
@@ -850,11 +850,11 @@ static int mtk_poll_rx(struct napi_struct *napi, int 
budget,
netdev->stats.rx_dropped++;
goto release_desc;
}
-   dma_addr = dma_map_single(>netdev[mac]->dev,
+   dma_addr = dma_map_single(eth->dev,
  new_data + NET_SKB_PAD,
  ring->buf_size,
  DMA_FROM_DEVICE);
-   if (unlikely(dma_mapping_error(>dev, dma_addr))) {
+   if (unlikely(dma_mapping_error(eth->dev, dma_addr))) {
skb_free_frag(new_data);
netdev->stats.rx_dropped++;
goto release_desc;
@@ -869,7 +869,7 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
}
skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
 
-   dma_unmap_single(>dev, trxd.rxd1,
+   dma_unmap_single(eth->dev, trxd.rxd1,
 ring->buf_size, DMA_FROM_DEVICE);
pktlen = RX_DMA_GET_PLEN0(trxd.rxd2);
skb->dev = netdev;
@@ -951,7 +951,7 @@ static int mtk_poll_tx(struct mtk_eth *eth, int budget)
done[mac]++;
budget--;
}
-   mtk_tx_unmap(eth->dev, tx_buf);
+   mtk_tx_unmap(eth, tx_buf);
 
ring->last_free = desc;
atomic_inc(>free_count);
@@ -1106,7 +1106,7

[PATCH 2/5] net: ethernet: mediatek: fixed flow control settings on GMAC0 is not being enabled properly

[Sean Wang]

Commit 08ef55c6f257acf3bdc6940813f80e8f0f5d90ec
("net-next: mediatek: fix gigabit and flow control advertisement")
had supported proper flow control settings for GMAC1. But for GMAC0,

1.GMAC0 shares the common logic with GMAC1 inside mtk_phy_link_adjust()
to adapt various settings for the target phy.

2.GMAC1 uses fixed-phy to connect to a builtin gigabit switch with
fixed link speed as commit 0c72c50f6f93b0c3daa9ea35d89ab3a933c7b5a0
("net-next: mediatek: add fixed-phy support") describes.

3.However, fixed-phy doesn't enable SUPPORTED_Pause & SUPPORTED_Asym_Pause
supported flag on default that would cause mtk_phy_link_adjust() not to
enable flow control setting on GMAC0 properly and cause packet dropped
when high traffic.

Due to these reasons, the patch adds SUPPORTED_Pause & SUPPORTED_Asym_Pause
supported flags on fixed-phy used by driver to have proper handling on
the both GMAC with the shared common logic.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index f19b8b9..eb59a73 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -267,6 +267,11 @@ static int mtk_phy_connect(struct mtk_mac *mac)
mac->phy_dev->autoneg = AUTONEG_ENABLE;
mac->phy_dev->speed = 0;
mac->phy_dev->duplex = 0;
+
+   if (of_phy_is_fixed_link(mac->of_node))
+   mac->phy_dev->supported |=
+   SUPPORTED_Pause | SUPPORTED_Asym_Pause;
+
mac->phy_dev->supported &= PHY_GBIT_FEATURES | SUPPORTED_Pause |
   SUPPORTED_Asym_Pause;
mac->phy_dev->advertising = mac->phy_dev->supported |
-- 
1.7.9.5

[PATCH 2/2] net: ethernet: mediatek: add the missing of_node_put() after node is used done

[Sean Wang]

This patch adds the missing of_node_put() after finishing the usage
of of_parse_phandle() or of_node_get() used by fixed_phy.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index f5d2745..88b04dd 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -269,6 +269,8 @@ static int mtk_phy_connect(struct mtk_mac *mac)
ADVERTISED_Autoneg;
phy_start_aneg(mac->phy_dev);
 
+   of_node_put(np);
+
return 0;
 }
 
-- 
1.7.9.5

[PATCH 1/2] net: ethernet: mediatek: fixed that initializing u64_stats_sync is missing

[Sean Wang]

To fix runtime warning with lockdep is enabled due that u64_stats_sync
is not initialized well, so add it.

Signed-off-by: Sean Wang <sean.w...@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 3a4726e..f5d2745 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1748,6 +1748,7 @@ static int mtk_add_mac(struct mtk_eth *eth, struct 
device_node *np)
goto free_netdev;
}
spin_lock_init(>hw_stats->stats_lock);
+   u64_stats_init(>hw_stats->syncp);
mac->hw_stats->reg_offset = id * MTK_STAT_OFFSET;
 
SET_NETDEV_DEV(eth->netdev[id], eth->dev);
-- 
1.7.9.5