> > +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
Hi Andrew,
This file is targeted for dual licensing which is GPL-2.0 OR BSD-3-Clause.
This is the reason why we are using wrappers around the functions so that
all the function call is generic.
Regards,
Weifeng
