| Message ID | 20191227023750.12559-3-olteanv@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | Improvements to SJA1105 DSA RX timestamping | expand |
Hi Vladimir,
I love your patch! Yet something to improve:
[auto build test ERROR on net-next/master]
[also build test ERROR on net/master linus/master v5.5-rc3 next-20191220]
[cannot apply to sparc-next/master]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Vladimir-Oltean/Improvements-to-SJA1105-DSA-RX-timestamping/20191227-104228
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 0914d2bb11cc182039084ac3b961dc359b647468
config: sparc64-randconfig-a001-20191226 (attached as .config)
compiler: sparc64-linux-gcc (GCC) 7.5.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.5.0 make.cross ARCH=sparc64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/net/dsa/sja1105/sja1105_ptp.c: In function 'sja1105_change_rxtstamping':
drivers/net/dsa/sja1105/sja1105_ptp.c:86:27: warning: unused variable 'ptp_data' [-Wunused-variable]
struct sja1105_ptp_data *ptp_data = &priv->ptp_data;
^~~~~~~~
drivers/net/dsa/sja1105/sja1105_ptp.c: In function 'sja1105_ptp_clock_unregister':
>> drivers/net/dsa/sja1105/sja1105_ptp.c:654:2: error: implicit declaration of function 'ptp_cancel_worker_sync'; did you mean 'cancel_work_sync'? [-Werror=implicit-function-declaration]
ptp_cancel_worker_sync(ptp_data->clock);
^~~~~~~~~~~~~~~~~~~~~~
cancel_work_sync
cc1: some warnings being treated as errors
vim +654 drivers/net/dsa/sja1105/sja1105_ptp.c
645
646 void sja1105_ptp_clock_unregister(struct dsa_switch *ds)
647 {
648 struct sja1105_private *priv = ds->priv;
649 struct sja1105_ptp_data *ptp_data = &priv->ptp_data;
650
651 if (IS_ERR_OR_NULL(ptp_data->clock))
652 return;
653
> 654 ptp_cancel_worker_sync(ptp_data->clock);
655 skb_queue_purge(&ptp_data->skb_rxtstamp_queue);
656 ptp_clock_unregister(ptp_data->clock);
657 ptp_data->clock = NULL;
658 }
659
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
On Fri, 27 Dec 2019 at 07:28, kbuild test robot <lkp@intel.com> wrote: > > Hi Vladimir, > > I love your patch! Yet something to improve: > > [auto build test ERROR on net-next/master] > [also build test ERROR on net/master linus/master v5.5-rc3 next-20191220] > [cannot apply to sparc-next/master] > [if your patch is applied to the wrong git tree, please drop us a note to help > improve the system. BTW, we also suggest to use '--base' option to specify the > base tree in git format-patch, please see https://stackoverflow.com/a/37406982] > > url: https://github.com/0day-ci/linux/commits/Vladimir-Oltean/Improvements-to-SJA1105-DSA-RX-timestamping/20191227-104228 > base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 0914d2bb11cc182039084ac3b961dc359b647468 > config: sparc64-randconfig-a001-20191226 (attached as .config) > compiler: sparc64-linux-gcc (GCC) 7.5.0 > reproduce: > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > GCC_VERSION=7.5.0 make.cross ARCH=sparc64 > > If you fix the issue, kindly add following tag > Reported-by: kbuild test robot <lkp@intel.com> > > All errors (new ones prefixed by >>): > > drivers/net/dsa/sja1105/sja1105_ptp.c: In function 'sja1105_change_rxtstamping': > drivers/net/dsa/sja1105/sja1105_ptp.c:86:27: warning: unused variable 'ptp_data' [-Wunused-variable] > struct sja1105_ptp_data *ptp_data = &priv->ptp_data; > ^~~~~~~~ Oops, this should have been part of 3/3 not 2/3... > drivers/net/dsa/sja1105/sja1105_ptp.c: In function 'sja1105_ptp_clock_unregister': > >> drivers/net/dsa/sja1105/sja1105_ptp.c:654:2: error: implicit declaration of function 'ptp_cancel_worker_sync'; did you mean 'cancel_work_sync'? [-Werror=implicit-function-declaration] And the attached .config.gz says that the error here is on the case where CONFIG_PTP_1588_CLOCK=n. Ok. > ptp_cancel_worker_sync(ptp_data->clock); > ^~~~~~~~~~~~~~~~~~~~~~ > cancel_work_sync > cc1: some warnings being treated as errors > > vim +654 drivers/net/dsa/sja1105/sja1105_ptp.c > > 645 > 646 void sja1105_ptp_clock_unregister(struct dsa_switch *ds) > 647 { > 648 struct sja1105_private *priv = ds->priv; > 649 struct sja1105_ptp_data *ptp_data = &priv->ptp_data; > 650 > 651 if (IS_ERR_OR_NULL(ptp_data->clock)) > 652 return; > 653 > > 654 ptp_cancel_worker_sync(ptp_data->clock); > 655 skb_queue_purge(&ptp_data->skb_rxtstamp_queue); > 656 ptp_clock_unregister(ptp_data->clock); > 657 ptp_data->clock = NULL; > 658 } > 659 > > --- > 0-DAY kernel test infrastructure Open Source Technology Center > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation Will send out a v2. Sorry, -Vladimir
diff --git a/drivers/net/dsa/sja1105/sja1105_ptp.c b/drivers/net/dsa/sja1105/sja1105_ptp.c index 038c83fbd9e8..d843c6395e52 100644 --- a/drivers/net/dsa/sja1105/sja1105_ptp.c +++ b/drivers/net/dsa/sja1105/sja1105_ptp.c @@ -83,6 +83,7 @@ static int sja1105_init_avb_params(struct sja1105_private *priv, static int sja1105_change_rxtstamping(struct sja1105_private *priv, bool on) { + struct sja1105_ptp_data *ptp_data = &priv->ptp_data; struct sja1105_general_params_entry *general_params; struct sja1105_table *table; int rc; @@ -367,22 +368,16 @@ static int sja1105_ptpclkval_write(struct sja1105_private *priv, u64 ticks, ptp_sts); } -#define rxtstamp_to_tagger(d) \ - container_of((d), struct sja1105_tagger_data, rxtstamp_work) -#define tagger_to_sja1105(d) \ - container_of((d), struct sja1105_private, tagger_data) - -static void sja1105_rxtstamp_work(struct work_struct *work) +static long sja1105_rxtstamp_work(struct ptp_clock_info *ptp) { - struct sja1105_tagger_data *tagger_data = rxtstamp_to_tagger(work); - struct sja1105_private *priv = tagger_to_sja1105(tagger_data); - struct sja1105_ptp_data *ptp_data = &priv->ptp_data; + struct sja1105_ptp_data *ptp_data = ptp_caps_to_data(ptp); + struct sja1105_private *priv = ptp_data_to_sja1105(ptp_data); struct dsa_switch *ds = priv->ds; struct sk_buff *skb; mutex_lock(&ptp_data->lock); - while ((skb = skb_dequeue(&tagger_data->skb_rxtstamp_queue)) != NULL) { + while ((skb = skb_dequeue(&ptp_data->skb_rxtstamp_queue)) != NULL) { struct skb_shared_hwtstamps *shwt = skb_hwtstamps(skb); u64 ticks, ts; int rc; @@ -404,6 +399,9 @@ static void sja1105_rxtstamp_work(struct work_struct *work) } mutex_unlock(&ptp_data->lock); + + /* Don't restart */ + return -1; } /* Called from dsa_skb_defer_rx_timestamp */ @@ -411,16 +409,16 @@ bool sja1105_port_rxtstamp(struct dsa_switch *ds, int port, struct sk_buff *skb, unsigned int type) { struct sja1105_private *priv = ds->priv; - struct sja1105_tagger_data *tagger_data = &priv->tagger_data; + struct sja1105_ptp_data *ptp_data = &priv->ptp_data; - if (!test_bit(SJA1105_HWTS_RX_EN, &tagger_data->state)) + if (!test_bit(SJA1105_HWTS_RX_EN, &priv->tagger_data.state)) return false; /* We need to read the full PTP clock to reconstruct the Rx * timestamp. For that we need a sleepable context. */ - skb_queue_tail(&tagger_data->skb_rxtstamp_queue, skb); - schedule_work(&tagger_data->rxtstamp_work); + skb_queue_tail(&ptp_data->skb_rxtstamp_queue, skb); + ptp_schedule_worker(ptp_data->clock, 0); return true; } @@ -628,11 +626,11 @@ int sja1105_ptp_clock_register(struct dsa_switch *ds) .adjtime = sja1105_ptp_adjtime, .gettimex64 = sja1105_ptp_gettimex, .settime64 = sja1105_ptp_settime, + .do_aux_work = sja1105_rxtstamp_work, .max_adj = SJA1105_MAX_ADJ_PPB, }; - skb_queue_head_init(&tagger_data->skb_rxtstamp_queue); - INIT_WORK(&tagger_data->rxtstamp_work, sja1105_rxtstamp_work); + skb_queue_head_init(&ptp_data->skb_rxtstamp_queue); spin_lock_init(&tagger_data->meta_lock); ptp_data->clock = ptp_clock_register(&ptp_data->caps, ds->dev); @@ -653,8 +651,8 @@ void sja1105_ptp_clock_unregister(struct dsa_switch *ds) if (IS_ERR_OR_NULL(ptp_data->clock)) return; - cancel_work_sync(&priv->tagger_data.rxtstamp_work); - skb_queue_purge(&priv->tagger_data.skb_rxtstamp_queue); + ptp_cancel_worker_sync(ptp_data->clock); + skb_queue_purge(&ptp_data->skb_rxtstamp_queue); ptp_clock_unregister(ptp_data->clock); ptp_data->clock = NULL; } diff --git a/drivers/net/dsa/sja1105/sja1105_ptp.h b/drivers/net/dsa/sja1105/sja1105_ptp.h index 470f44b76318..6f4a19eec709 100644 --- a/drivers/net/dsa/sja1105/sja1105_ptp.h +++ b/drivers/net/dsa/sja1105/sja1105_ptp.h @@ -30,6 +30,7 @@ struct sja1105_ptp_cmd { }; struct sja1105_ptp_data { + struct sk_buff_head skb_rxtstamp_queue; struct ptp_clock_info caps; struct ptp_clock *clock; struct sja1105_ptp_cmd cmd; diff --git a/include/linux/dsa/sja1105.h b/include/linux/dsa/sja1105.h index 897e799dbcb9..c0b6a603ea8c 100644 --- a/include/linux/dsa/sja1105.h +++ b/include/linux/dsa/sja1105.h @@ -37,8 +37,6 @@ * the structure defined in struct sja1105_private. */ struct sja1105_tagger_data { - struct sk_buff_head skb_rxtstamp_queue; - struct work_struct rxtstamp_work; struct sk_buff *stampable_skb; /* Protects concurrent access to the meta state machine * from taggers running on multiple ports on SMP systems
And move the queue of skb's waiting for RX timestamps into the ptp_data structure, since it isn't needed if PTP is not compiled. Signed-off-by: Vladimir Oltean <olteanv@gmail.com> --- drivers/net/dsa/sja1105/sja1105_ptp.c | 34 +++++++++++++-------------- drivers/net/dsa/sja1105/sja1105_ptp.h | 1 + include/linux/dsa/sja1105.h | 2 -- 3 files changed, 17 insertions(+), 20 deletions(-)