Messages in this thread | | | Date | Wed, 19 Jun 2019 05:07:29 +0200 | From | Andrew Lunn <> | Subject | Re: [RFC net-next 1/5] net: stmmac: introduce IEEE 802.1Qbv configuration functionalities |
| |
On Wed, Jun 19, 2019 at 05:36:14AM +0800, Voon Weifeng wrote:
Hi Voon
> +static int est_poll_srwo(void *ioaddr) > +{ > + /* Poll until the EST GCL Control[SRWO] bit clears. > + * Total wait = 12 x 50ms ~= 0.6s. > + */ > + unsigned int retries = 12; > + unsigned int value; > + > + do { > + value = TSN_RD32(ioaddr + MTL_EST_GCL_CTRL); > + if (!(value & MTL_EST_GCL_CTRL_SRWO)) > + return 0; > + msleep(50); > + } while (--retries); > + > + return -ETIMEDOUT;
Maybe use one of the readx_poll_timeout() macros?
> +static int est_read_gce(void *ioaddr, unsigned int row, > + unsigned int *gates, unsigned int *ti_nsec, > + unsigned int dbgb, unsigned int dbgm) > +{ > + struct tsn_hw_cap *cap = &dw_tsn_hwcap; > + unsigned int ti_wid = cap->ti_wid; > + unsigned int gates_mask; > + unsigned int ti_mask; > + unsigned int value; > + int ret; > + > + gates_mask = (1 << cap->txqcnt) - 1; > + ti_mask = (1 << ti_wid) - 1; > + > + ret = est_read_gcl_config(ioaddr, &value, row, 0, dbgb, dbgm); > + if (ret) { > + TSN_ERR("Read GCE failed! row=%u\n", row);
It is generally not a good idea to put wrappers around the kernel print functions. It would be better if all these functions took struct stmmac_priv *priv rather than ioaddr, so you could then do
netdev_err(priv->dev, "Read GCE failed! row=%u\n", row);
> + /* Ensure that HW is not in the midst of GCL transition */ > + value = TSN_RD32(ioaddr + MTL_EST_CTRL);
Also, don't put wrapper around readl()/writel().
> + value &= ~MTL_EST_CTRL_SSWL; > + > + /* MTL_EST_CTRL value has been read earlier, if TILS value > + * differs, we update here. > + */ > + if (tils != dw_tsn_hwtunable[TSN_HWTUNA_TX_EST_TILS]) { > + value &= ~MTL_EST_CTRL_TILS; > + value |= (tils << MTL_EST_CTRL_TILS_SHIFT); > + > + TSN_WR32(value, ioaddr + MTL_EST_CTRL); > + dw_tsn_hwtunable[TSN_HWTUNA_TX_EST_TILS] = tils; > + } > + > + return 0; > +} > + > +static int est_set_ov(void *ioaddr, > + const unsigned int *ptov, > + const unsigned int *ctov) > +{ > + unsigned int value; > + > + if (!dw_tsn_feat_en[TSN_FEAT_ID_EST]) > + return -ENOTSUPP; > + > + value = TSN_RD32(ioaddr + MTL_EST_CTRL); > + value &= ~MTL_EST_CTRL_SSWL; > + > + if (ptov) { > + if (*ptov > EST_PTOV_MAX) { > + TSN_WARN("EST: invalid PTOV(%u), max=%u\n", > + *ptov, EST_PTOV_MAX);
It looks like most o the TSN_WARN should actually be netdev_dbg().
Andrew
| |