> > If the community prefers readability
>
> Readability nearly always comes first. There is nothing performance
> critical here, MDIO is a slow bus. So the code should be readable,
> simple to understand.
>
Noted and thanks for the comments.
>
> , I will suggest to do the c45 setup in
> >
> If the community prefers readability
Readability nearly always comes first. There is nothing performance
critical here, MDIO is a slow bus. So the code should be readable,
simple to understand.
, I will suggest to do the c45 setup in
> both stmmac_mdio_read() and stmmac_mdio_write() 's
> I think there is too much passing variables around by reference than by
> value, to make this code easy to understand.
>
> Maybe a better structure would be
>
> static int stmmac_mdion_c45_read(struct stmmac_priv *priv, int phyaddr,
> int phyreg) {
>
> unsigned int reg_shift =
> Yes, the top 16 bit of the data register only valid when C45 is enable.
> It contains the Register address which MDIO c45 frame intended for.
I think there is too much passing variables around by reference than
by value, to make this code easy to understand.
Maybe a better structure would be
> > > > > > @@ -155,22 +171,26 @@ static int stmmac_mdio_read(struct
> > > > > > mii_bus *bus,
> > > > > int phyaddr, int phyreg)
> > > > > > struct stmmac_priv *priv = netdev_priv(ndev);
> > > > > > unsigned int mii_address = priv->hw->mii.addr;
> > > > > > unsigned int mii_data =
From: Andrew Lunn
> Yes, that is all clear. The stmmac_mdio_c45_setup() does part of this
> setup. There is also a write to mii_address which i snipped out when
> replying. But why do you need to write to the data registers during a
> read? C22 does not need this write. Are there some bits in
On Thu, Jul 04, 2019 at 06:05:23AM +, Voon, Weifeng wrote:
> > > > > @@ -155,22 +171,26 @@ static int stmmac_mdio_read(struct mii_bus
> > > > > *bus,
> > > > int phyaddr, int phyreg)
> > > > > struct stmmac_priv *priv = netdev_priv(ndev);
> > > > > unsigned int mii_address =
> > > > @@ -155,22 +171,26 @@ static int stmmac_mdio_read(struct mii_bus
> > > > *bus,
> > > int phyaddr, int phyreg)
> > > > struct stmmac_priv *priv = netdev_priv(ndev);
> > > > unsigned int mii_address = priv->hw->mii.addr;
> > > > unsigned int mii_data =
On Thu, Jul 04, 2019 at 01:33:16AM +, Voon, Weifeng wrote:
> > > @@ -155,22 +171,26 @@ static int stmmac_mdio_read(struct mii_bus *bus,
> > int phyaddr, int phyreg)
> > > struct stmmac_priv *priv = netdev_priv(ndev);
> > > unsigned int mii_address = priv->hw->mii.addr;
> > > unsigned int
> > @@ -155,22 +171,26 @@ static int stmmac_mdio_read(struct mii_bus *bus,
> int phyaddr, int phyreg)
> > struct stmmac_priv *priv = netdev_priv(ndev);
> > unsigned int mii_address = priv->hw->mii.addr;
> > unsigned int mii_data = priv->hw->mii.data;
> > - u32 v;
> > - int data;
>
On Wed, Jul 03, 2019 at 05:50:04PM +0800, Voon Weifeng wrote:
> @@ -155,22 +171,26 @@ static int stmmac_mdio_read(struct mii_bus *bus, int
> phyaddr, int phyreg)
> struct stmmac_priv *priv = netdev_priv(ndev);
> unsigned int mii_address = priv->hw->mii.addr;
> unsigned int
11 matches
Mail list logo
