RE: [PATCH 02/14] drivers: net: ldpaa: add support for probing based on the DTS

2020-03-18 Thread Ioana Ciornei
> Subject: Re: [PATCH 02/14] drivers: net: ldpaa: add support for probing based 
> on
> the DTS
> 
> On Thu, Mar 12, 2020 at 11:26 AM Ioana Ciornei 
> wrote:
> >
> > When CONFIG_DM_ETH is enabled DPAA2 network interfaces will now probe
> > based on DTS nodes with the "fsl,qoriq-mc-dpmac" compatible.
> > In this case, transform the ldpaa_eth driver into a UCLASS_ETH driver
> > and reuse the _open()/_tx()/_stop() functions already inplemented.
> >
> > For the moment, the ldpaa_eth driver will support both configurations:
> > with or without CONFIG_DM_ETH enabled. Any 'struct eth_device'
> > occurrence now has a matching 'struct udevice' made mutually exclusive
> > based on the state of CONFIG_DM_ETH.
> >
> > Signed-off-by: Florin Laurentiu Chiculita
> > 
> > Signed-off-by: Ioana Ciornei 
> > ---
> >  drivers/net/ldpaa_eth/ldpaa_eth.c | 230 +-
> >  drivers/net/ldpaa_eth/ldpaa_eth.h |   6 +
> >  2 files changed, 204 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/net/ldpaa_eth/ldpaa_eth.c
> > b/drivers/net/ldpaa_eth/ldpaa_eth.c
> > index a3b9c152b256..bbfe479ed1c9 100644
> > --- a/drivers/net/ldpaa_eth/ldpaa_eth.c
> > +++ b/drivers/net/ldpaa_eth/ldpaa_eth.c
> > @@ -12,6 +12,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >
> > @@ -19,6 +20,7 @@
> >  #include "ldpaa_eth.h"
> >
> >  #ifdef CONFIG_PHYLIB
> > +#ifndef CONFIG_DM_ETH
> 
> Please use positive logic here and throughout. Add the DM code here before the
> non-DM code.

Sure. Will change in v2.

> 
> >  static int init_phy(struct eth_device *dev)  {
> > struct ldpaa_eth_priv *priv = (struct ldpaa_eth_priv
> > *)dev->priv; @@ -62,6 +64,19 @@ static int init_phy(struct eth_device
> > *dev)
> >
> > return ret;
> >  }
> > +#else
> > +static void init_phy(struct udevice *dev) {
> > +   struct ldpaa_eth_priv *priv = dev_get_priv(dev);
> > +
> > +   priv->phy = dm_eth_phy_connect(dev);
> > +
> > +   if (!priv->phy)
> > +   return;
> > +
> > +   phy_config(priv->phy);
> > +}
> > +#endif
> >  #endif
> >
> >  #ifdef DEBUG
> > @@ -128,9 +143,15 @@ static void ldpaa_eth_get_dpni_counter(void)
> > }
> >  }
> >
> > +#ifdef CONFIG_DM_ETH
> > +static void ldpaa_eth_get_dpmac_counter(struct udevice *dev) {
> > +   struct ldpaa_eth_priv *priv = dev_get_priv(dev); #else
> >  static void ldpaa_eth_get_dpmac_counter(struct eth_device *net_dev)
> > {
> > struct ldpaa_eth_priv *priv = (struct ldpaa_eth_priv
> > *)net_dev->priv;
> > +#endif
> > int err = 0;
> > u64 value;
> >
> > @@ -263,9 +284,16 @@ error:
> > return;
> >  }
> >
> > +#ifdef CONFIG_DM_ETH
> > +static int ldpaa_eth_pull_dequeue_rx(struct udevice *dev,
> > +int flags, uchar **packetp) {
> > +   struct ldpaa_eth_priv *priv = dev_get_priv(dev); #else
> >  static int ldpaa_eth_pull_dequeue_rx(struct eth_device *dev)  {
> > struct ldpaa_eth_priv *priv = (struct ldpaa_eth_priv
> > *)dev->priv;
> > +#endif
> > const struct ldpaa_dq *dq;
> > const struct dpaa_fd *fd;
> > int i = 5, err = 0, status;
> > @@ -322,9 +350,15 @@ static int ldpaa_eth_pull_dequeue_rx(struct
> eth_device *dev)
> > return err;
> >  }
> >
> > +#ifdef CONFIG_DM_ETH
> > +static int ldpaa_eth_tx(struct udevice *dev, void *buf, int len) {
> > +   struct ldpaa_eth_priv *priv = dev_get_priv(dev); #else
> >  static int ldpaa_eth_tx(struct eth_device *net_dev, void *buf, int
> > len)  {
> > struct ldpaa_eth_priv *priv = (struct ldpaa_eth_priv
> > *)net_dev->priv;
> > +#endif
> > struct dpaa_fd fd;
> > u64 buffer_start;
> > int data_offset, err;
> > @@ -400,15 +434,32 @@ error:
> > return err;
> >  }
> >
> > +static struct phy_device *ldpaa_get_phydev(struct ldpaa_eth_priv
> > +*priv) { #ifdef CONFIG_DM_ETH
> > +   return priv->phy;
> > +#else
> > +#ifdef CONFIG_PHYLIB
> > +   struct phy_device *phydev = NULL;
> > +   int phy_num;
> > +
> > +   /* start the phy devices one by one and update the dpmac state */
> > +   for (phy_num = 0; phy_num < WRIOP_MAX_PHY_NUM; 

Re: [PATCH 02/14] drivers: net: ldpaa: add support for probing based on the DTS

2020-03-17 Thread Joe Hershberger
On Thu, Mar 12, 2020 at 11:26 AM Ioana Ciornei  wrote:
>
> When CONFIG_DM_ETH is enabled DPAA2 network interfaces will now probe
> based on DTS nodes with the "fsl,qoriq-mc-dpmac" compatible.
> In this case, transform the ldpaa_eth driver into a UCLASS_ETH driver
> and reuse the _open()/_tx()/_stop() functions already inplemented.
>
> For the moment, the ldpaa_eth driver will support both configurations:
> with or without CONFIG_DM_ETH enabled. Any 'struct eth_device' occurrence
> now has a matching 'struct udevice' made mutually exclusive based on the
> state of CONFIG_DM_ETH.
>
> Signed-off-by: Florin Laurentiu Chiculita 
> Signed-off-by: Ioana Ciornei 
> ---
>  drivers/net/ldpaa_eth/ldpaa_eth.c | 230 +-
>  drivers/net/ldpaa_eth/ldpaa_eth.h |   6 +
>  2 files changed, 204 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/net/ldpaa_eth/ldpaa_eth.c 
> b/drivers/net/ldpaa_eth/ldpaa_eth.c
> index a3b9c152b256..bbfe479ed1c9 100644
> --- a/drivers/net/ldpaa_eth/ldpaa_eth.c
> +++ b/drivers/net/ldpaa_eth/ldpaa_eth.c
> @@ -12,6 +12,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>
> @@ -19,6 +20,7 @@
>  #include "ldpaa_eth.h"
>
>  #ifdef CONFIG_PHYLIB
> +#ifndef CONFIG_DM_ETH

Please use positive logic here and throughout. Add the DM code here
before the non-DM code.

>  static int init_phy(struct eth_device *dev)
>  {
> struct ldpaa_eth_priv *priv = (struct ldpaa_eth_priv *)dev->priv;
> @@ -62,6 +64,19 @@ static int init_phy(struct eth_device *dev)
>
> return ret;
>  }
> +#else
> +static void init_phy(struct udevice *dev)
> +{
> +   struct ldpaa_eth_priv *priv = dev_get_priv(dev);
> +
> +   priv->phy = dm_eth_phy_connect(dev);
> +
> +   if (!priv->phy)
> +   return;
> +
> +   phy_config(priv->phy);
> +}
> +#endif
>  #endif
>
>  #ifdef DEBUG
> @@ -128,9 +143,15 @@ static void ldpaa_eth_get_dpni_counter(void)
> }
>  }
>
> +#ifdef CONFIG_DM_ETH
> +static void ldpaa_eth_get_dpmac_counter(struct udevice *dev)
> +{
> +   struct ldpaa_eth_priv *priv = dev_get_priv(dev);
> +#else
>  static void ldpaa_eth_get_dpmac_counter(struct eth_device *net_dev)
>  {
> struct ldpaa_eth_priv *priv = (struct ldpaa_eth_priv *)net_dev->priv;
> +#endif
> int err = 0;
> u64 value;
>
> @@ -263,9 +284,16 @@ error:
> return;
>  }
>
> +#ifdef CONFIG_DM_ETH
> +static int ldpaa_eth_pull_dequeue_rx(struct udevice *dev,
> +int flags, uchar **packetp)
> +{
> +   struct ldpaa_eth_priv *priv = dev_get_priv(dev);
> +#else
>  static int ldpaa_eth_pull_dequeue_rx(struct eth_device *dev)
>  {
> struct ldpaa_eth_priv *priv = (struct ldpaa_eth_priv *)dev->priv;
> +#endif
> const struct ldpaa_dq *dq;
> const struct dpaa_fd *fd;
> int i = 5, err = 0, status;
> @@ -322,9 +350,15 @@ static int ldpaa_eth_pull_dequeue_rx(struct eth_device 
> *dev)
> return err;
>  }
>
> +#ifdef CONFIG_DM_ETH
> +static int ldpaa_eth_tx(struct udevice *dev, void *buf, int len)
> +{
> +   struct ldpaa_eth_priv *priv = dev_get_priv(dev);
> +#else
>  static int ldpaa_eth_tx(struct eth_device *net_dev, void *buf, int len)
>  {
> struct ldpaa_eth_priv *priv = (struct ldpaa_eth_priv *)net_dev->priv;
> +#endif
> struct dpaa_fd fd;
> u64 buffer_start;
> int data_offset, err;
> @@ -400,15 +434,32 @@ error:
> return err;
>  }
>
> +static struct phy_device *ldpaa_get_phydev(struct ldpaa_eth_priv *priv)
> +{
> +#ifdef CONFIG_DM_ETH
> +   return priv->phy;
> +#else
> +#ifdef CONFIG_PHYLIB
> +   struct phy_device *phydev = NULL;
> +   int phy_num;
> +
> +   /* start the phy devices one by one and update the dpmac state */
> +   for (phy_num = 0; phy_num < WRIOP_MAX_PHY_NUM; phy_num++) {
> +   phydev = wriop_get_phy_dev(priv->dpmac_id, phy_num);
> +   if (phydev)
> +   return phydev;
> +   }
> +   return NULL;
> +#endif
> +#endif
> +}
> +
>  static int ldpaa_get_dpmac_state(struct ldpaa_eth_priv *priv,
>  struct dpmac_link_state *state)
>  {
> phy_interface_t enet_if;
> -   int phys_detected;
> -#ifdef CONFIG_PHYLIB
> struct phy_device *phydev = NULL;
> -   int err, phy_num;
> -#endif
> +   int err;
>
> /* let's start off with maximum capabilities */
> enet_if = wriop_get_enet_if(priv->dpmac_id);
> @@ -420,39 +471,28 @@ static int ldpaa_get_dpmac_state(struct ldpaa_eth_priv 
> *priv,
> state->rate = SPEED_1000;
> break;
> }
> -   state->up = 1;
>
> -   phys_detected = 0;
> -#ifdef CONFIG_PHYLIB
> +   state->up = 1;
> state->options |= DPMAC_LINK_OPT_AUTONEG;
> +   phydev = ldpaa_get_phydev(priv);
>
> -   /* start the phy devices one by one and update the dpmac state */
> -   for (phy_num = 0; phy_num <