| Message ID | 20230817141746.18726-2-karol.kolacinski@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ice: fix timestamping in reset process | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
On 8/17/23 16:17, Karol Kolacinski wrote: > The ice_pf_src_tmr_owned() macro exists to check the function capability This patch is send with you as an author. There should be a "From: Jacob Keller <jacob.e.keller@intel.com>" as first line. (git-send-email should do it for yourself, for testing: git log prior to sending should report Jake too) I remember that codewise v1 of this series was (about? :D) fine for me, will double check with v3. > bit indicating if the current function owns the PTP hardware clock. > > This is slightly shorter than the more verbose access via > hw.func_caps.ts_func_info.src_tmr_owned. Be consistent and use this > where possible rather than open coding its equivalent. > > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> > Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com> > --- > drivers/net/ethernet/intel/ice/ice_main.c | 2 +- > drivers/net/ethernet/intel/ice/ice_ptp.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c > index a6dd336d2500..b6858f04152c 100644 > --- a/drivers/net/ethernet/intel/ice/ice_main.c > +++ b/drivers/net/ethernet/intel/ice/ice_main.c > @@ -3185,7 +3185,7 @@ static irqreturn_t ice_misc_intr(int __always_unused irq, void *data) > > ena_mask &= ~PFINT_OICR_TSYN_EVNT_M; > > - if (hw->func_caps.ts_func_info.src_tmr_owned) { > + if (ice_pf_src_tmr_owned(pf)) { > /* Save EVENTs from GLTSYN register */ > pf->ptp.ext_ts_irq |= gltsyn_stat & > (GLTSYN_STAT_EVENT0_M | > diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c > index 97b8581ae931..0669ca905c46 100644 > --- a/drivers/net/ethernet/intel/ice/ice_ptp.c > +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c > @@ -2447,7 +2447,7 @@ void ice_ptp_reset(struct ice_pf *pf) > if (test_bit(ICE_PFR_REQ, pf->state)) > goto pfr; > > - if (!hw->func_caps.ts_func_info.src_tmr_owned) > + if (!ice_pf_src_tmr_owned(pf)) > goto reset_ts; > > err = ice_ptp_init_phc(hw);
On Thu, Aug 17, 2023 at 04:17:38PM +0200, Karol Kolacinski wrote: > The ice_pf_src_tmr_owned() macro exists to check the function capability > bit indicating if the current function owns the PTP hardware clock. This is first patch in the series, but I can't find mentioned macro. My net-next is based on 5b0a1414e0b0 ("Merge branch 'smc-features'") ➜ kernel git:(net-next) git grep ice_pf_src_tmr_owned shows nothing. On which branch is it based? Thanks > > This is slightly shorter than the more verbose access via > hw.func_caps.ts_func_info.src_tmr_owned. Be consistent and use this > where possible rather than open coding its equivalent. > > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> > Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com> > --- > drivers/net/ethernet/intel/ice/ice_main.c | 2 +- > drivers/net/ethernet/intel/ice/ice_ptp.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c > index a6dd336d2500..b6858f04152c 100644 > --- a/drivers/net/ethernet/intel/ice/ice_main.c > +++ b/drivers/net/ethernet/intel/ice/ice_main.c > @@ -3185,7 +3185,7 @@ static irqreturn_t ice_misc_intr(int __always_unused irq, void *data) > > ena_mask &= ~PFINT_OICR_TSYN_EVNT_M; > > - if (hw->func_caps.ts_func_info.src_tmr_owned) { > + if (ice_pf_src_tmr_owned(pf)) { > /* Save EVENTs from GLTSYN register */ > pf->ptp.ext_ts_irq |= gltsyn_stat & > (GLTSYN_STAT_EVENT0_M | > diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c > index 97b8581ae931..0669ca905c46 100644 > --- a/drivers/net/ethernet/intel/ice/ice_ptp.c > +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c > @@ -2447,7 +2447,7 @@ void ice_ptp_reset(struct ice_pf *pf) > if (test_bit(ICE_PFR_REQ, pf->state)) > goto pfr; > > - if (!hw->func_caps.ts_func_info.src_tmr_owned) > + if (!ice_pf_src_tmr_owned(pf)) > goto reset_ts; > > err = ice_ptp_init_phc(hw); > -- > 2.39.2 > >
On Sat, Aug 19, 2023 at 02:52:49PM +0300, Leon Romanovsky wrote: > On Thu, Aug 17, 2023 at 04:17:38PM +0200, Karol Kolacinski wrote: > > The ice_pf_src_tmr_owned() macro exists to check the function capability > > bit indicating if the current function owns the PTP hardware clock. > > This is first patch in the series, but I can't find mentioned macro. > My net-next is based on 5b0a1414e0b0 ("Merge branch 'smc-features'") > ➜ kernel git:(net-next) git grep ice_pf_src_tmr_owned > shows nothing. > > On which branch is it based? Hi Leon, My assumption is that it is based on the dev-queue branch of https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git
On Tue, Aug 22, 2023 at 09:02:11AM +0200, Simon Horman wrote: > On Sat, Aug 19, 2023 at 02:52:49PM +0300, Leon Romanovsky wrote: > > On Thu, Aug 17, 2023 at 04:17:38PM +0200, Karol Kolacinski wrote: > > > The ice_pf_src_tmr_owned() macro exists to check the function capability > > > bit indicating if the current function owns the PTP hardware clock. > > > > This is first patch in the series, but I can't find mentioned macro. > > My net-next is based on 5b0a1414e0b0 ("Merge branch 'smc-features'") > > ➜ kernel git:(net-next) git grep ice_pf_src_tmr_owned > > shows nothing. > > > > On which branch is it based? > > Hi Leon, > > My assumption is that it is based on the dev-queue branch of > https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git So should netdev readers review it or wait till Intel folks perform first pass on it? Thanks
On 8/22/23 16:13, Leon Romanovsky wrote: > On Tue, Aug 22, 2023 at 09:02:11AM +0200, Simon Horman wrote: >> On Sat, Aug 19, 2023 at 02:52:49PM +0300, Leon Romanovsky wrote: >>> On Thu, Aug 17, 2023 at 04:17:38PM +0200, Karol Kolacinski wrote: >>>> The ice_pf_src_tmr_owned() macro exists to check the function capability >>>> bit indicating if the current function owns the PTP hardware clock. >>> >>> This is first patch in the series, but I can't find mentioned macro. >>> My net-next is based on 5b0a1414e0b0 ("Merge branch 'smc-features'") >>> ➜ kernel git:(net-next) git grep ice_pf_src_tmr_owned >>> shows nothing. >>> >>> On which branch is it based? >> >> Hi Leon, >> >> My assumption is that it is based on the dev-queue branch of >> https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git > > So should netdev readers review it or wait till Intel folks perform > first pass on it? Most of the time Intel folks would be first to review, if only because of our pre-IWL processes or pure familiarity/interest in given piece. For this particular series, it is about right "codewise" since v1, so you are welcome for an insightful look at v3 (I didn't provided my RBs so far because of "metadata" issues :), will take a fresh look, but you don't need to wait). General idea for CC'ing netdev for IWL-targeted patches is to have open develompent process. Quality should be already as for netdev posting. Our VAL picks up patches for testing from here when Tony marks them so. That's what I could say for review process. "Maintainers stuff", I *guess*, is: after review&test Tony Requests netdev Maintainers to Pull (and throttles outgoing stuff by doing so to pace agreed upon). At that stage is a last moment for (late?) review, welcomed as always. > > Thanks > _______________________________________________ > Intel-wired-lan mailing list > Intel-wired-lan@osuosl.org > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
On Tue, Aug 22, 2023 at 04:44:29PM +0200, Przemek Kitszel wrote: > On 8/22/23 16:13, Leon Romanovsky wrote: > > On Tue, Aug 22, 2023 at 09:02:11AM +0200, Simon Horman wrote: > > > On Sat, Aug 19, 2023 at 02:52:49PM +0300, Leon Romanovsky wrote: > > > > On Thu, Aug 17, 2023 at 04:17:38PM +0200, Karol Kolacinski wrote: > > > > > The ice_pf_src_tmr_owned() macro exists to check the function capability > > > > > bit indicating if the current function owns the PTP hardware clock. > > > > > > > > This is first patch in the series, but I can't find mentioned macro. > > > > My net-next is based on 5b0a1414e0b0 ("Merge branch 'smc-features'") > > > > ➜ kernel git:(net-next) git grep ice_pf_src_tmr_owned > > > > shows nothing. > > > > > > > > On which branch is it based? > > > > > > Hi Leon, > > > > > > My assumption is that it is based on the dev-queue branch of > > > https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git > > > > So should netdev readers review it or wait till Intel folks perform > > first pass on it? > > Most of the time Intel folks would be first to review, if only because of > our pre-IWL processes or pure familiarity/interest in given piece. > > For this particular series, it is about right "codewise" since v1, so you > are welcome for an insightful look at v3 > (I didn't provided my RBs so far because of "metadata" issues :), > will take a fresh look, but you don't need to wait). > > > General idea for CC'ing netdev for IWL-targeted patches is to have open > develompent process. > Quality should be already as for netdev posting. > Our VAL picks up patches for testing from here when Tony marks them so. > > That's what I could say for review process. > > "Maintainers stuff", I *guess*, is: > after review&test Tony Requests netdev Maintainers to Pull > (and throttles outgoing stuff by doing so to pace agreed upon). > At that stage is a last moment for (late?) review, welcomed as always. It means that we (netdev@... ) will see "same" patches twice, am I right? Thanks > > > > > > > Thanks > > _______________________________________________ > > Intel-wired-lan mailing list > > Intel-wired-lan@osuosl.org > > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan > >
On 8/22/23 17:48, Leon Romanovsky wrote: > On Tue, Aug 22, 2023 at 04:44:29PM +0200, Przemek Kitszel wrote: >> On 8/22/23 16:13, Leon Romanovsky wrote: >>> On Tue, Aug 22, 2023 at 09:02:11AM +0200, Simon Horman wrote: >>>> On Sat, Aug 19, 2023 at 02:52:49PM +0300, Leon Romanovsky wrote: >>>>> On Thu, Aug 17, 2023 at 04:17:38PM +0200, Karol Kolacinski wrote: >>>>>> The ice_pf_src_tmr_owned() macro exists to check the function capability >>>>>> bit indicating if the current function owns the PTP hardware clock. >>>>> >>>>> This is first patch in the series, but I can't find mentioned macro. >>>>> My net-next is based on 5b0a1414e0b0 ("Merge branch 'smc-features'") >>>>> ➜ kernel git:(net-next) git grep ice_pf_src_tmr_owned >>>>> shows nothing. >>>>> >>>>> On which branch is it based? >>>> >>>> Hi Leon, >>>> >>>> My assumption is that it is based on the dev-queue branch of >>>> https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git >>> >>> So should netdev readers review it or wait till Intel folks perform >>> first pass on it? >> >> Most of the time Intel folks would be first to review, if only because of >> our pre-IWL processes or pure familiarity/interest in given piece. >> >> For this particular series, it is about right "codewise" since v1, so you >> are welcome for an insightful look at v3 >> (I didn't provided my RBs so far because of "metadata" issues :), >> will take a fresh look, but you don't need to wait). >> >> >> General idea for CC'ing netdev for IWL-targeted patches is to have open >> develompent process. >> Quality should be already as for netdev posting. >> Our VAL picks up patches for testing from here when Tony marks them so. >> >> That's what I could say for review process. >> >> "Maintainers stuff", I *guess*, is: >> after review&test Tony Requests netdev Maintainers to Pull >> (and throttles outgoing stuff by doing so to pace agreed upon). >> At that stage is a last moment for (late?) review, welcomed as always. > > It means that we (netdev@... ) will see "same" patches twice, am I right? That's true. > > Thanks > >> >> >> >>> >>> Thanks >>> _______________________________________________ >>> Intel-wired-lan mailing list >>> Intel-wired-lan@osuosl.org >>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan >> >>
On Tue, Aug 22, 2023 at 05:56:25PM +0200, Przemek Kitszel wrote: > On 8/22/23 17:48, Leon Romanovsky wrote: > > On Tue, Aug 22, 2023 at 04:44:29PM +0200, Przemek Kitszel wrote: > > > On 8/22/23 16:13, Leon Romanovsky wrote: > > > > On Tue, Aug 22, 2023 at 09:02:11AM +0200, Simon Horman wrote: > > > > > On Sat, Aug 19, 2023 at 02:52:49PM +0300, Leon Romanovsky wrote: > > > > > > On Thu, Aug 17, 2023 at 04:17:38PM +0200, Karol Kolacinski wrote: > > > > > > > The ice_pf_src_tmr_owned() macro exists to check the function capability > > > > > > > bit indicating if the current function owns the PTP hardware clock. > > > > > > > > > > > > This is first patch in the series, but I can't find mentioned macro. > > > > > > My net-next is based on 5b0a1414e0b0 ("Merge branch 'smc-features'") > > > > > > ➜ kernel git:(net-next) git grep ice_pf_src_tmr_owned > > > > > > shows nothing. > > > > > > > > > > > > On which branch is it based? > > > > > > > > > > Hi Leon, > > > > > > > > > > My assumption is that it is based on the dev-queue branch of > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git > > > > > > > > So should netdev readers review it or wait till Intel folks perform > > > > first pass on it? > > > > > > Most of the time Intel folks would be first to review, if only because of > > > our pre-IWL processes or pure familiarity/interest in given piece. > > > > > > For this particular series, it is about right "codewise" since v1, so you > > > are welcome for an insightful look at v3 > > > (I didn't provided my RBs so far because of "metadata" issues :), > > > will take a fresh look, but you don't need to wait). > > > > > > > > > General idea for CC'ing netdev for IWL-targeted patches is to have open > > > develompent process. > > > Quality should be already as for netdev posting. > > > Our VAL picks up patches for testing from here when Tony marks them so. > > > > > > That's what I could say for review process. > > > > > > "Maintainers stuff", I *guess*, is: > > > after review&test Tony Requests netdev Maintainers to Pull > > > (and throttles outgoing stuff by doing so to pace agreed upon). > > > At that stage is a last moment for (late?) review, welcomed as always. > > > > It means that we (netdev@... ) will see "same" patches twice, am I right? > > That's true. Can I suggest change in the process? 1. Perform validation before posting 2. Intel will post their patches to the netdev@ ML. 3. Tony will collect reviewed patches from netdev@ 4. Tony will send clean PRs (without patches) from time to time to netdev maintainers for acceptance. It will allow to all of us (Intel, Nvidia e.t.c) to have same submission flow without sacrificing open netdev@ review which will be done only once. Jakub/Dave, is it possible? Thanks > > > > > Thanks > > > > > > > > > > > > > > > > > > > Thanks > > > > _______________________________________________ > > > > Intel-wired-lan mailing list > > > > Intel-wired-lan@osuosl.org > > > > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan > > > > > > >
On Tue, 22 Aug 2023 19:06:51 +0300 Leon Romanovsky wrote: > Can I suggest change in the process? > 1. Perform validation before posting > 2. Intel will post their patches to the netdev@ ML. > 3. Tony will collect reviewed patches from netdev@ > 4. Tony will send clean PRs (without patches) from time to time to > netdev maintainers for acceptance. > > It will allow to all of us (Intel, Nvidia e.t.c) to have same submission > flow without sacrificing open netdev@ review which will be done only once. > > Jakub/Dave, is it possible? That sounds worse than what they are doing today. And I can't help but think that you're targeting them because I asked you to stop posting directly for net-next. Vendetta is not a good guide for process changes. Let's see what the 6.6 development stats look like. Then we'll have a discussion about what we can improve.
On Tue, Aug 22, 2023 at 09:53:01AM -0700, Jakub Kicinski wrote: > On Tue, 22 Aug 2023 19:06:51 +0300 Leon Romanovsky wrote: > > Can I suggest change in the process? > > 1. Perform validation before posting > > 2. Intel will post their patches to the netdev@ ML. > > 3. Tony will collect reviewed patches from netdev@ > > 4. Tony will send clean PRs (without patches) from time to time to > > netdev maintainers for acceptance. > > > > It will allow to all of us (Intel, Nvidia e.t.c) to have same submission > > flow without sacrificing open netdev@ review which will be done only once. > > > > Jakub/Dave, is it possible? > > That sounds worse than what they are doing today. And I can't help > but think that you're targeting them because I asked you to stop posting > directly for net-next. Vendetta is not a good guide for process changes. Are you real? I had this idea even BEFORE you started to be netdev@ maintainer and had put it on paper BEFORE your request. Should I add you to our internal conversation about it so you will be able to see dates by yourself? Thanks
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c index a6dd336d2500..b6858f04152c 100644 --- a/drivers/net/ethernet/intel/ice/ice_main.c +++ b/drivers/net/ethernet/intel/ice/ice_main.c @@ -3185,7 +3185,7 @@ static irqreturn_t ice_misc_intr(int __always_unused irq, void *data) ena_mask &= ~PFINT_OICR_TSYN_EVNT_M; - if (hw->func_caps.ts_func_info.src_tmr_owned) { + if (ice_pf_src_tmr_owned(pf)) { /* Save EVENTs from GLTSYN register */ pf->ptp.ext_ts_irq |= gltsyn_stat & (GLTSYN_STAT_EVENT0_M | diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c index 97b8581ae931..0669ca905c46 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp.c +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c @@ -2447,7 +2447,7 @@ void ice_ptp_reset(struct ice_pf *pf) if (test_bit(ICE_PFR_REQ, pf->state)) goto pfr; - if (!hw->func_caps.ts_func_info.src_tmr_owned) + if (!ice_pf_src_tmr_owned(pf)) goto reset_ts; err = ice_ptp_init_phc(hw);