Re: [Intel-wired-lan] [PATCH net 1/5] igb: reject invalid external timestamp requests for 82580-based HW
On Mon, Mar 10, 2025 at 03:16:36PM -0700, Jacob Keller wrote:
> The igb_ptp_feature_enable_82580 function correctly checks that unknown
> flags are not passed to the function. However, it does not actually check
> PTP_RISING_EDGE or PTP_FALLING_EDGE when configuring the external timestamp
> function.
>
> The data sheet for the 82580 product says:
>
> Upon a change in the input level of one of the SDP pins that was
> configured to detect Time stamp events using the TSSDP register, a time
> stamp of the system time is captured into one of the two auxiliary time
> stamp registers (AUXSTMPL/H0 or AUXSTMPL/H1).
>
> For example to define timestamping of events in the AUXSTMPL0 and
> AUXSTMPH0 registers, Software should:
>
> 1. Set the TSSDP.AUX0_SDP_SEL field to select the SDP pin that detects
> the level change and set the TSSDP.AUX0_TS_SDP_EN bit to 1.
>
> 2. Set the TSAUXC.EN_TS0 bit to 1 to enable timestamping
>
> The same paragraph is in the i350 and i354 data sheets.
>
> The wording implies that the time stamps are captured at any level change.
> There does not appear to be any way to only timestamp one edge of the
> signal.
>
> Reject requests which do not set both PTP_RISING_EDGE and PTP_FALLING_EDGE
> when operating under PTP_STRICT_FLAGS mode via PTP_EXTTS_REQUEST2.
>
> Fixes: 38970eac41db ("igb: support EXTTS on 82580/i354/i350")
> Signed-off-by: Jacob Keller
> ---
> drivers/net/ethernet/intel/igb/igb_ptp.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c
> b/drivers/net/ethernet/intel/igb/igb_ptp.c
> index
> f9457055612004c10f74379122063e8136fe7d76..b89ef4538a18d7ca11325ddc15944a878f4d807e
> 100644
> --- a/drivers/net/ethernet/intel/igb/igb_ptp.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
> @@ -509,6 +509,11 @@ static int igb_ptp_feature_enable_82580(struct
> ptp_clock_info *ptp,
> PTP_STRICT_FLAGS))
> return -EOPNOTSUPP;
>
> + /* Both the rising and falling edge are timstamped */
> + if (rq->extts.flags & PTP_STRICT_FLAGS &&
> + (rq->extts.flags & PTP_EXTTS_EDGES) != PTP_EXTTS_EDGES)
> + return -EOPNOTSUPP;
> +
> if (on) {
> pin = ptp_find_pin(igb->ptp_clock, PTP_PF_EXTTS,
> rq->extts.index);
Thanks for fixing
Reviewed-by: Michal Swiatkowski
In igb_ptp_feature_enable_i210() there is the same check for both edges
but also PTP_ENABLE_FEATURE is tested. There is no need for it here, or
it is redundant even in i210?
>
> --
> 2.48.1.397.gec9d649cc640
[Intel-wired-lan] [PATCH iwl-next v2 2/3] ice: refactor ice_sbq_msg_dev enum
From: Karol Kolacinski
Rename ice_sbq_msg_dev to ice_sbq_dev_id to reflect the meaning of this
type more precisely. This enum type describes RDA (Remote Device Access)
client ids, accessible over SB (Side Band) interface.
Rename enum elements to make a driver namespace more cleaner and
consistent with other definitions within SB
Remove unused 'rmn_x' entries, specific to unsupported E824 device.
Adjust clients '2' and '13' names (phy_0 and phy_0_peer respectively) to
be compliant with EAS doc. According to the specification, regardless of
the complex entity (single or dual), when accessing its own ports,
they're accessed always as 'phy_0' client. And referred as 'phy_0_peer'
when handling ports conneced to the other complex.
Reviewed-by: Simon Horman
Reviewed-by: Przemek Kitszel
Signed-off-by: Karol Kolacinski
Co-developed-by: Grzegorz Nitka
Signed-off-by: Grzegorz Nitka
---
drivers/net/ethernet/intel/ice/ice_common.c | 2 +-
drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 20 ++--
drivers/net/ethernet/intel/ice/ice_sbq_cmd.h | 11 ---
3 files changed, 15 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c
b/drivers/net/ethernet/intel/ice/ice_common.c
index edb1d0f7e187..8f0c72df2a44 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -3434,7 +3434,7 @@ int ice_aq_get_fec_stats(struct ice_hw *hw, u16 pcs_quad,
u16 pcs_port,
msg.msg_addr_low = lower_16_bits(reg_offset);
msg.msg_addr_high = receiver_id;
msg.opcode = ice_sbq_msg_rd;
- msg.dest_dev = rmn_0;
+ msg.dest_dev = ice_sbq_dev_phy_0;
err = ice_sbq_rw_reg(hw, &msg, flag);
if (err)
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
index a5df081ffc19..eb1893dd8979 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
@@ -240,7 +240,7 @@ static int ice_read_cgu_reg_e82x(struct ice_hw *hw, u32
addr, u32 *val)
{
struct ice_sbq_msg_input cgu_msg = {
.opcode = ice_sbq_msg_rd,
- .dest_dev = cgu,
+ .dest_dev = ice_sbq_dev_cgu,
.msg_addr_low = addr
};
int err;
@@ -272,7 +272,7 @@ static int ice_write_cgu_reg_e82x(struct ice_hw *hw, u32
addr, u32 val)
{
struct ice_sbq_msg_input cgu_msg = {
.opcode = ice_sbq_msg_wr,
- .dest_dev = cgu,
+ .dest_dev = ice_sbq_dev_cgu,
.msg_addr_low = addr,
.data = val
};
@@ -919,16 +919,16 @@ static void ice_ptp_cfg_sync_delay(const struct ice_hw
*hw, u32 delay)
*
* Return: destination sideband queue PHY device.
*/
-static enum ice_sbq_msg_dev ice_ptp_get_dest_dev_e825(struct ice_hw *hw,
- u8 port)
+static enum ice_sbq_dev_id ice_ptp_get_dest_dev_e825(struct ice_hw *hw,
+u8 port)
{
/* On a single complex E825, PHY 0 is always destination device phy_0
* and PHY 1 is phy_0_peer.
*/
if (port >= hw->ptp.ports_per_phy)
- return eth56g_phy_1;
+ return ice_sbq_dev_phy_0_peer;
else
- return eth56g_phy_0;
+ return ice_sbq_dev_phy_0;
}
/**
@@ -2758,7 +2758,7 @@ static void ice_fill_phy_msg_e82x(struct ice_hw *hw,
msg->msg_addr_high = P_Q1_H(P_4_BASE + offset, phy_port);
}
- msg->dest_dev = rmn_0;
+ msg->dest_dev = ice_sbq_dev_phy_0;
}
/**
@@ -3081,7 +3081,7 @@ static int ice_fill_quad_msg_e82x(struct ice_hw *hw,
if (quad >= ICE_GET_QUAD_NUM(hw->ptp.num_lports))
return -EINVAL;
- msg->dest_dev = rmn_0;
+ msg->dest_dev = ice_sbq_dev_phy_0;
if (!(quad % ICE_GET_QUAD_NUM(hw->ptp.ports_per_phy)))
addr = Q_0_BASE + offset;
@@ -4800,7 +4800,7 @@ static int ice_read_phy_reg_e810(struct ice_hw *hw, u32
addr, u32 *val)
msg.msg_addr_low = lower_16_bits(addr);
msg.msg_addr_high = upper_16_bits(addr);
msg.opcode = ice_sbq_msg_rd;
- msg.dest_dev = rmn_0;
+ msg.dest_dev = ice_sbq_dev_phy_0;
err = ice_sbq_rw_reg(hw, &msg, ICE_AQ_FLAG_RD);
if (err) {
@@ -4830,7 +4830,7 @@ static int ice_write_phy_reg_e810(struct ice_hw *hw, u32
addr, u32 val)
msg.msg_addr_low = lower_16_bits(addr);
msg.msg_addr_high = upper_16_bits(addr);
msg.opcode = ice_sbq_msg_wr;
- msg.dest_dev = rmn_0;
+ msg.dest_dev = ice_sbq_dev_phy_0;
msg.data = val;
err = ice_sbq_rw_reg(hw, &msg, ICE_AQ_FLAG_RD);
diff --git a/drivers/net/ethernet/intel/ice/ice_sbq_cmd.h
b/drivers/net/ethernet/intel/ice/ice_sbq_cmd.h
index 3b0054faf70c..183dd5457d6a 100644
--- a/drivers/net/ethernet/intel/ice/ice_sbq_cmd.h
+++ b/driv
[Intel-wired-lan] [PATCH iwl-next v2] i40e: fix MMIO write access to an invalid page in i40e_clear_hw
When the device sends a specific input, an integer underflow can occur, leading
to MMIO write access to an invalid page.
Prevent the integer underflow by changing the type of related variables.
Signed-off-by: Kyungwook Boo
Link:
https://lore.kernel.org/lkml/ffc91764-1142-4ba2-91b6-8c773f6f7...@gmail.com/T/
---
Changes in v2:
- Formatting properly
- Fix variable shadowing
- Link to v1:
https://lore.kernel.org/netdev/55acc5dc-8d5a-45bc-a59c-9304071e4...@gmail.com/
---
drivers/net/ethernet/intel/i40e/i40e_common.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c
b/drivers/net/ethernet/intel/i40e/i40e_common.c
index 370b4bddee44..b11c35e307ca 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_common.c
@@ -817,10 +817,11 @@ int i40e_pf_reset(struct i40e_hw *hw)
void i40e_clear_hw(struct i40e_hw *hw)
{
u32 num_queues, base_queue;
- u32 num_pf_int;
- u32 num_vf_int;
+ s32 num_pf_int;
+ s32 num_vf_int;
u32 num_vfs;
- u32 i, j;
+ s32 i;
+ u32 j;
u32 val;
u32 eol = 0x7ff;
---
base-commit: 4d872d51bc9d7b899c1f61534e3dbde72613f627
Best regards,
Kyungwook Boo
Re: [Intel-wired-lan] [PATCH iwl-net] idpf: fix adapter NULL pointer dereference on reboot
On 3/9/2025 11:22 PM, Simon Horman wrote: On Thu, Mar 06, 2025 at 04:39:56PM -0800, Emil Tantilov wrote: Driver calls idpf_remove() from idpf_shutdown(), which can end up calling idpf_remove() again when disabling SRIOV. echo 1 > /sys/class/net//device/sriov_numvfs reboot BUG: kernel NULL pointer dereference, address: 0020 ... RIP: 0010:idpf_remove+0x22/0x1f0 [idpf] ... ? idpf_remove+0x22/0x1f0 [idpf] ? idpf_remove+0x1e4/0x1f0 [idpf] pci_device_remove+0x3f/0xb0 device_release_driver_internal+0x19f/0x200 pci_stop_bus_device+0x6d/0x90 pci_stop_and_remove_bus_device+0x12/0x20 pci_iov_remove_virtfn+0xbe/0x120 sriov_disable+0x34/0xe0 idpf_sriov_configure+0x58/0x140 [idpf] idpf_remove+0x1b9/0x1f0 [idpf] idpf_shutdown+0x12/0x30 [idpf] pci_device_shutdown+0x35/0x60 device_shutdown+0x156/0x200 ... Replace the direct idpf_remove() call in idpf_shutdown() with idpf_vc_core_deinit() and idpf_deinit_dflt_mbx(), which perform the bulk of the cleanup, such as stopping the init task, freeing IRQs, destroying the vports and freeing the mailbox. Hi Emil, I think it would be worth adding some commentary on the rest of the clean-up performed by idpf_remove() and why it is correct The main reason behind the change is to avoid calling sriov_disable(), which ends up calling idpf_remove() again via pci_device_remove(). The idpf_remove() will crash in that situation as it attempts to access adapter pointer, which was already freed. to no longer do so directly from a call to idpf_remove() from idpf_shutdown() (IOW, it isn't clear to me :). I assume you are asking what portion of the idpf_remove() will not be present in idpf_shutdown() as result? Aside from not calling sriov_disable(), there is a small cleanup of stale netdevs and the destruction of WQs, which did not seem like would be needed on shutdown. Then again, I was not able to find documentation on what steps are required for shutdown and mostly checked on how other drivers handle it (where there is no 1:1 overlap between shutdown and remove), and applied similar steps to idpf. Ideally I do not wish to do more than is needed for that flow. ...
Re: [Intel-wired-lan] [PATCH iwl-net] idpf: fix adapter NULL pointer dereference on reboot
On 3/6/2025 9:58 PM, Michal Swiatkowski wrote:
On Thu, Mar 06, 2025 at 04:39:56PM -0800, Emil Tantilov wrote:
Driver calls idpf_remove() from idpf_shutdown(), which can end up
calling idpf_remove() again when disabling SRIOV.
The same is done in other drivers (ice, iavf). Why here it is a problem?
I am asking because heaving one function to remove is pretty handy.
Maybe the problem can be fixed by some changes in idpf_remove() instead?
It was indeed handy, until we ran into the crash. I did look into fixing
it in idpf_remove(), but I don't think I have a lot of options. I can
simply check and exit on adapter being NULL, but this types of checks
are usually frowned upon, so I looked into alternatives.
The main difference between idpf and ice is that idpf will load on both
VF and PF devices. From what I can tell, the VFs created by ice are
supported by iavf (0x1889 device id). With VFs created, on idpf, we end
up calling into idpf_remove() twice. First on shutdown and then again
when idpf_remove calls into sriov_disable(), because the VF devices have
the same driver, hence the same remove routine.
echo 1 > /sys/class/net//device/sriov_numvfs
reboot
BUG: kernel NULL pointer dereference, address: 0020
...
RIP: 0010:idpf_remove+0x22/0x1f0 [idpf]
...
? idpf_remove+0x22/0x1f0 [idpf]
? idpf_remove+0x1e4/0x1f0 [idpf]
pci_device_remove+0x3f/0xb0
device_release_driver_internal+0x19f/0x200
pci_stop_bus_device+0x6d/0x90
pci_stop_and_remove_bus_device+0x12/0x20
pci_iov_remove_virtfn+0xbe/0x120
sriov_disable+0x34/0xe0
idpf_sriov_configure+0x58/0x140 [idpf]
idpf_remove+0x1b9/0x1f0 [idpf]
idpf_shutdown+0x12/0x30 [idpf]
pci_device_shutdown+0x35/0x60
device_shutdown+0x156/0x200
...
Replace the direct idpf_remove() call in idpf_shutdown() with
idpf_vc_core_deinit() and idpf_deinit_dflt_mbx(), which perform
the bulk of the cleanup, such as stopping the init task, freeing IRQs,
destroying the vports and freeing the mailbox.
Reported-by: Yuying Ma
Fixes: e850efed5e15 ("idpf: add module register and probe functionality")
Reviewed-by: Madhu Chittim
Signed-off-by: Emil Tantilov
---
drivers/net/ethernet/intel/idpf/idpf_main.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c
b/drivers/net/ethernet/intel/idpf/idpf_main.c
index b6c515d14cbf..bec4a02c5373 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_main.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_main.c
@@ -87,7 +87,11 @@ static void idpf_remove(struct pci_dev *pdev)
*/
static void idpf_shutdown(struct pci_dev *pdev)
{
- idpf_remove(pdev);
+ struct idpf_adapter *adapter = pci_get_drvdata(pdev);
+
+ cancel_delayed_work_sync(&adapter->vc_event_task);
+ idpf_vc_core_deinit(adapter);
+ idpf_deinit_dflt_mbx(adapter);
if (system_state == SYSTEM_POWER_OFF)
pci_set_power_state(pdev, PCI_D3hot);
--
2.17.2
Re: [Intel-wired-lan] [PATCH bpf-next v12 5/5] igc: Add launch time support to XDP ZC
> -Original Message-
> From: Song, Yoong Siang
> Sent: Friday, March 7, 2025 3:21 PM
>
> On Friday, March 7, 2025 9:25 PM, Bouska, Zdenek
> wrote:
>
> [...]
>
> >> @@ -2996,7 +3035,13 @@ static void igc_xdp_xmit_zc(struct igc_ring *ring)
> >>ntu = ring->next_to_use;
> >>budget = igc_desc_unused(ring);
> >>
> >> - while (xsk_tx_peek_desc(pool, &xdp_desc) && budget--) {
> >> + /* Packets with launch time require one data descriptor and one context
> >> + * descriptor. When the launch time falls into the next Qbv cycle, we
> >> + * may need to insert an empty packet, which requires two more
> >> + * descriptors. Therefore, to be safe, we always ensure we have at least
> >> + * 4 descriptors available.
> >> + */
> >> + while (xsk_tx_peek_desc(pool, &xdp_desc) && budget >= 4) {
> >
> >I think that here is a bug: some frames could be missed if budget < 4.
> >I was able to reproduce it by sending 10x 60 B frames with minimal IPG
> >(672 ns between starts of frames) on 1Gbit/s. Always 1026 frames were not
> >sent
> >and were missing a AF_XDP competition. Interesting was that then even when I
> >sent more
> >frames for hours it still was 1026 frames not sent and missing competition.
> >
> >Bug seems to be fixed when I change this line to:
> >
> > while (budget >= 4 && xsk_tx_peek_desc(pool, &xdp_desc)) {
> >
> >Do you think this is a good fix?
> >
> >I think this bug is also in original code base, but I was only able to
> >reproduce
> >it with launch time.
> >
>
> Thank you for pointing out this issue and for providing a detailed
> explanation of your findings. I personally agree with your proposed fix
> that make sure there is enough budget in the driver, before go peek the xsk
> descriptor. Do you plan to submit bug fix patch to iwl-net?
>
Yes, I plan to submit bug fix patch.
Best regards,
Zdenek Bouska
--
Siemens, s.r.o
Foundational Technologies
Re: [Intel-wired-lan] [PATCH iwl-next] ice: use DSN instead of PCI BDF for ice_adapter index
Subject: Re: [PATCH iwl-next] ice: use DSN instead of PCI BDF for ice_adapter index regarding -net vs -next, no one have complained that this bug hurts + return (unsigned long)pci_get_dsn(pdev); How do you ensure there is no xarray index collision then you cut the number like this? The reduction occurs only on "32b" systems, which are unlikely to have this device. And any mixing of the upper and lower 4B part still could collide. It is also probably necessary to check if all devices supported by the driver have DSN capability enabled. I will double check on the SoC you have in mind. Regards, Sergey
Re: [Intel-wired-lan] [PATCH net 5/5] ptp: ocp: reject unsupported periodic output flags
On 10/03/2025 22:16, Jacob Keller wrote:
The ptp_ocp_signal_from_perout() function supports PTP_PEROUT_DUTY_CYCLE
and PTP_PEROUT_PHASE. It does not support PTP_PEROUT_ONE_SHOT, but does not
reject a request with such an unsupported flag.
Add the appropriate check to ensure that unsupported requests are rejected
both for PTP_PEROUT_ONE_SHOT as well as any future flags.
Fixes: 1aa66a3a135a ("ptp: ocp: Program the signal generators via
PTP_CLK_REQ_PEROUT")
Signed-off-by: Jacob Keller
---
drivers/ptp/ptp_ocp.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index
b651087f426f50a73229ca57634fc5d6912e0a87..4a87af0980d695a9ab1b23e2544f620759ccb892
100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -2090,6 +2090,10 @@ ptp_ocp_signal_from_perout(struct ptp_ocp *bp, int gen,
{
struct ptp_ocp_signal s = { };
+ if (req->flags & ~(PTP_PEROUT_DUTY_CYCLE |
+ PTP_PEROUT_PHASE))
+ return -EOPNOTSUPP;
+
s.polarity = bp->signal[gen].polarity;
s.period = ktime_set(req->period.sec, req->period.nsec);
if (!s.period)
Thanks,
Reviewed-by: Vadim Fedorenko
[Intel-wired-lan] [PATCH iwl-next 02/10] ice: rename TSPLL and CGU functions and definitions
Rename TSPLL and CGU functions, definitions etc. to match the file name
and have constistent naming scheme.
Reviewed-by: Michal Kubiak
Reviewed-by: Milena Olech
Signed-off-by: Karol Kolacinski
---
drivers/net/ethernet/intel/ice/ice_common.c | 28 +-
drivers/net/ethernet/intel/ice/ice_common.h | 36 +-
drivers/net/ethernet/intel/ice/ice_ptp.c | 2 +-
.../net/ethernet/intel/ice/ice_ptp_consts.h | 16 +-
drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 4 +-
drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 11 +-
drivers/net/ethernet/intel/ice/ice_tspll.c| 350 +-
drivers/net/ethernet/intel/ice/ice_tspll.h| 32 +-
drivers/net/ethernet/intel/ice/ice_type.h | 20 +-
9 files changed, 244 insertions(+), 255 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c
b/drivers/net/ethernet/intel/ice/ice_common.c
index 3119bec3da33..d7333105ecd8 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -2298,12 +2298,12 @@ ice_parse_1588_func_caps(struct ice_hw *hw, struct
ice_hw_func_caps *func_p,
info->clk_freq = FIELD_GET(ICE_TS_CLK_FREQ_M, number);
info->clk_src = ((number & ICE_TS_CLK_SRC_M) != 0);
} else {
- info->clk_freq = ICE_TIME_REF_FREQ_156_250;
+ info->clk_freq = ICE_TSPLL_FREQ_156_250;
info->clk_src = ICE_CLK_SRC_TCXO;
}
- if (info->clk_freq < NUM_ICE_TIME_REF_FREQ) {
- info->time_ref = (enum ice_time_ref_freq)info->clk_freq;
+ if (info->clk_freq < NUM_ICE_TSPLL_FREQ) {
+ info->time_ref = (enum ice_tspll_freq)info->clk_freq;
} else {
/* Unknown clock frequency, so assume a (probably incorrect)
* default to avoid out-of-bounds look ups of frequency
@@ -2311,7 +2311,7 @@ ice_parse_1588_func_caps(struct ice_hw *hw, struct
ice_hw_func_caps *func_p,
*/
ice_debug(hw, ICE_DBG_INIT, "1588 func caps: unknown clock
frequency %u\n",
info->clk_freq);
- info->time_ref = ICE_TIME_REF_FREQ_25_000;
+ info->time_ref = ICE_TSPLL_FREQ_25_000;
}
ice_debug(hw, ICE_DBG_INIT, "func caps: ieee_1588 = %u\n",
@@ -6113,17 +6113,17 @@ u32 ice_get_link_speed(u16 index)
}
/**
- * ice_read_cgu_reg_e82x - Read a CGU register
- * @hw: pointer to the HW struct
+ * ice_read_cgu_reg - Read a CGU register
+ * @hw: Pointer to the HW struct
* @addr: Register address to read
- * @val: storage for register value read
+ * @val: Storage for register value read
*
* Read the contents of a register of the Clock Generation Unit. Only
- * applicable to E822 devices.
+ * applicable to E82X devices.
*
* Return: 0 on success, other error codes when failed to read from CGU.
*/
-int ice_read_cgu_reg_e82x(struct ice_hw *hw, u32 addr, u32 *val)
+int ice_read_cgu_reg(struct ice_hw *hw, u32 addr, u32 *val)
{
struct ice_sbq_msg_input cgu_msg = {
.opcode = ice_sbq_msg_rd,
@@ -6145,17 +6145,17 @@ int ice_read_cgu_reg_e82x(struct ice_hw *hw, u32 addr,
u32 *val)
}
/**
- * ice_write_cgu_reg_e82x - Write a CGU register
- * @hw: pointer to the HW struct
+ * ice_write_cgu_reg - Write a CGU register
+ * @hw: Pointer to the HW struct
* @addr: Register address to write
- * @val: value to write into the register
+ * @val: Value to write into the register
*
* Write the specified value to a register of the Clock Generation Unit. Only
- * applicable to E822 devices.
+ * applicable to E82X devices.
*
* Return: 0 on success, other error codes when failed to write to CGU.
*/
-int ice_write_cgu_reg_e82x(struct ice_hw *hw, u32 addr, u32 val)
+int ice_write_cgu_reg(struct ice_hw *hw, u32 addr, u32 val)
{
struct ice_sbq_msg_input cgu_msg = {
.opcode = ice_sbq_msg_wr,
diff --git a/drivers/net/ethernet/intel/ice/ice_common.h
b/drivers/net/ethernet/intel/ice/ice_common.h
index 47e17fa443a1..fee5b373af8c 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.h
+++ b/drivers/net/ethernet/intel/ice/ice_common.h
@@ -39,8 +39,8 @@
#define FEC_RECEIVER_ID_PCS0 (0x33 << FEC_RECV_ID_SHIFT)
#define FEC_RECEIVER_ID_PCS1 (0x34 << FEC_RECV_ID_SHIFT)
-#define NAC_CGU_DWORD9 0x24
-union nac_cgu_dword9 {
+#define ICE_CGU_R9 0x24
+union ice_cgu_r9 {
struct {
u32 time_ref_freq_sel : 3;
u32 clk_eref1_en : 1;
@@ -62,24 +62,24 @@ union nac_cgu_dword9 {
u32 val;
};
-#define NAC_CGU_DWORD16_E825C 0x40
-union nac_cgu_dword16_e825c {
+#define ICE_CGU_R16 0x40
+union ice_cgu_r16 {
struct {
u32 synce_remndr : 6;
u32 synce_phlmt_en : 1;
u32 misc13 : 17;
- u32 tspll_ck_refclkfreq : 8;
+ u32 ck_refclkfreq : 8;
};
u32 val;
};
-#define NAC_CGU_DWORD19 0x4c
-union nac_cgu_dword19 {
+#d
[Intel-wired-lan] [PATCH iwl-next 01/10] ice: move TSPLL functions to a separate file
Collect TSPLL related functions and definitions and move them to
a separate file to have all TSPLL functionality in one place.
Move CGU related functions and definitions to ice_common.*
Reviewed-by: Michal Kubiak
Reviewed-by: Milena Olech
Signed-off-by: Karol Kolacinski
---
drivers/net/ethernet/intel/ice/Makefile | 2 +-
drivers/net/ethernet/intel/ice/ice.h | 1 +
drivers/net/ethernet/intel/ice/ice_cgu_regs.h | 181 -
drivers/net/ethernet/intel/ice/ice_common.c | 61 ++
drivers/net/ethernet/intel/ice/ice_common.h | 176 +
drivers/net/ethernet/intel/ice/ice_ptp.c | 1 -
.../net/ethernet/intel/ice/ice_ptp_consts.h | 161 -
drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 542 ---
drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 43 --
drivers/net/ethernet/intel/ice/ice_tspll.c| 643 ++
drivers/net/ethernet/intel/ice/ice_tspll.h| 43 ++
11 files changed, 925 insertions(+), 929 deletions(-)
delete mode 100644 drivers/net/ethernet/intel/ice/ice_cgu_regs.h
create mode 100644 drivers/net/ethernet/intel/ice/ice_tspll.c
create mode 100644 drivers/net/ethernet/intel/ice/ice_tspll.h
diff --git a/drivers/net/ethernet/intel/ice/Makefile
b/drivers/net/ethernet/intel/ice/Makefile
index 9e0d9f710441..d0f9c9492363 100644
--- a/drivers/net/ethernet/intel/ice/Makefile
+++ b/drivers/net/ethernet/intel/ice/Makefile
@@ -53,7 +53,7 @@ ice-$(CONFIG_PCI_IOV) += \
ice_vf_mbx.o\
ice_vf_vsi_vlan_ops.o \
ice_vf_lib.o
-ice-$(CONFIG_PTP_1588_CLOCK) += ice_ptp.o ice_ptp_hw.o ice_dpll.o
+ice-$(CONFIG_PTP_1588_CLOCK) += ice_ptp.o ice_ptp_hw.o ice_dpll.o ice_tspll.o
ice-$(CONFIG_DCB) += ice_dcb.o ice_dcb_nl.o ice_dcb_lib.o
ice-$(CONFIG_RFS_ACCEL) += ice_arfs.o
ice-$(CONFIG_XDP_SOCKETS) += ice_xsk.o
diff --git a/drivers/net/ethernet/intel/ice/ice.h
b/drivers/net/ethernet/intel/ice/ice.h
index 2694951a0b1d..ceab989d3f7e 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -67,6 +67,7 @@
#include "ice_sriov.h"
#include "ice_vf_mbx.h"
#include "ice_ptp.h"
+#include "ice_tspll.h"
#include "ice_fdir.h"
#include "ice_xsk.h"
#include "ice_arfs.h"
diff --git a/drivers/net/ethernet/intel/ice/ice_cgu_regs.h
b/drivers/net/ethernet/intel/ice/ice_cgu_regs.h
deleted file mode 100644
index 10d9d74f3545..
--- a/drivers/net/ethernet/intel/ice/ice_cgu_regs.h
+++ /dev/null
@@ -1,181 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/* Copyright (C) 2018-2021, Intel Corporation. */
-
-#ifndef _ICE_CGU_REGS_H_
-#define _ICE_CGU_REGS_H_
-
-#define NAC_CGU_DWORD9 0x24
-union nac_cgu_dword9 {
- struct {
- u32 time_ref_freq_sel : 3;
- u32 clk_eref1_en : 1;
- u32 clk_eref0_en : 1;
- u32 time_ref_en : 1;
- u32 time_sync_en : 1;
- u32 one_pps_out_en : 1;
- u32 clk_ref_synce_en : 1;
- u32 clk_synce1_en : 1;
- u32 clk_synce0_en : 1;
- u32 net_clk_ref1_en : 1;
- u32 net_clk_ref0_en : 1;
- u32 clk_synce1_amp : 2;
- u32 misc6 : 1;
- u32 clk_synce0_amp : 2;
- u32 one_pps_out_amp : 2;
- u32 misc24 : 12;
- };
- u32 val;
-};
-
-#define NAC_CGU_DWORD16_E825C 0x40
-union nac_cgu_dword16_e825c {
- struct {
- u32 synce_remndr : 6;
- u32 synce_phlmt_en : 1;
- u32 misc13 : 17;
- u32 tspll_ck_refclkfreq : 8;
- };
- u32 val;
-};
-
-#define NAC_CGU_DWORD19 0x4c
-union nac_cgu_dword19 {
- struct {
- u32 tspll_fbdiv_intgr : 8;
- u32 fdpll_ulck_thr : 5;
- u32 misc15 : 3;
- u32 tspll_ndivratio : 4;
- u32 tspll_iref_ndivratio : 3;
- u32 misc19 : 1;
- u32 japll_ndivratio : 4;
- u32 japll_iref_ndivratio : 3;
- u32 misc27 : 1;
- };
- u32 val;
-};
-
-#define NAC_CGU_DWORD22 0x58
-union nac_cgu_dword22 {
- struct {
- u32 fdpll_frac_div_out_nc : 2;
- u32 fdpll_lock_int_for : 1;
- u32 synce_hdov_int_for : 1;
- u32 synce_lock_int_for : 1;
- u32 fdpll_phlead_slip_nc : 1;
- u32 fdpll_acc1_ovfl_nc : 1;
- u32 fdpll_acc2_ovfl_nc : 1;
- u32 synce_status_nc : 6;
- u32 fdpll_acc1f_ovfl : 1;
- u32 misc18 : 1;
- u32 fdpllclk_div : 4;
- u32 time1588clk_div : 4;
- u32 synceclk_div : 4;
- u32 synceclk_sel_div2 : 1;
- u32 fdpllclk_sel_div2 : 1;
- u32 time1588clk_sel_div2 : 1;
- u32 misc3 : 1;
- };
- u32 val;
-};
-
-#define NAC_CGU_DWORD23_E825C 0x5C
-union nac_cgu_dword23_e825c {
- struct {
- u3
[Intel-wired-lan] [PATCH iwl-next 07/10] ice: add multiple TSPLL helpers
Add helpers for checking TSPLL params, disabling sticky bits,
configuring TSPLL and getting default clock frequency to simplify
the code flows.
Reviewed-by: Milena Olech
Signed-off-by: Karol Kolacinski
---
drivers/net/ethernet/intel/ice/ice_tspll.c | 152 ++---
1 file changed, 104 insertions(+), 48 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_tspll.c
b/drivers/net/ethernet/intel/ice/ice_tspll.c
index 4d4d0a74310b..dce5164ebd9b 100644
--- a/drivers/net/ethernet/intel/ice/ice_tspll.c
+++ b/drivers/net/ethernet/intel/ice/ice_tspll.c
@@ -68,6 +68,54 @@ static const char *ice_tspll_clk_freq_str(enum
ice_tspll_freq clk_freq)
}
}
+/**
+ * ice_tspll_default_freq - Return default frequency for a MAC type
+ * @mac_type: MAC type
+ */
+static enum ice_tspll_freq ice_tspll_default_freq(enum ice_mac_type mac_type)
+{
+ switch (mac_type) {
+ case ICE_MAC_GENERIC:
+ return ICE_TSPLL_FREQ_25_000;
+ case ICE_MAC_GENERIC_3K_E825:
+ return ICE_TSPLL_FREQ_156_250;
+ default:
+ return -ERANGE;
+ }
+}
+
+/**
+ * ice_tspll_check_params - Check if TSPLL params are correct
+ * @hw: Pointer to the HW struct
+ * @clk_freq: Clock frequency to program
+ * @clk_src: Clock source to select (TIME_REF or TCXO)
+ */
+static bool ice_tspll_check_params(struct ice_hw *hw,
+ enum ice_tspll_freq clk_freq,
+ enum ice_clk_src clk_src)
+{
+ if (clk_freq >= NUM_ICE_TSPLL_FREQ) {
+ dev_warn(ice_hw_to_dev(hw), "Invalid TSPLL frequency %u\n",
+clk_freq);
+ return false;
+ }
+
+ if (clk_src >= NUM_ICE_CLK_SRC) {
+ dev_warn(ice_hw_to_dev(hw), "Invalid clock source %u\n",
+clk_src);
+ return false;
+ }
+
+ if ((hw->mac_type == ICE_MAC_GENERIC_3K_E825 ||
+clk_src == ICE_CLK_SRC_TCXO) &&
+ clk_freq != ice_tspll_default_freq(hw->mac_type)) {
+ dev_warn(ice_hw_to_dev(hw), "Unsupported frequency for this
clock source\n");
+ return false;
+ }
+
+ return true;
+}
+
/**
* ice_tspll_clk_src_str - Convert time_ref_src to string
* @clk_src: Clock source
@@ -126,24 +174,6 @@ static int ice_tspll_cfg_e82x(struct ice_hw *hw, enum
ice_tspll_freq clk_freq,
{
u32 val, r9, r24;
- if (clk_freq >= NUM_ICE_TSPLL_FREQ) {
- dev_warn(ice_hw_to_dev(hw), "Invalid TIME_REF frequency %u\n",
-clk_freq);
- return -EINVAL;
- }
-
- if (clk_src >= NUM_ICE_CLK_SRC) {
- dev_warn(ice_hw_to_dev(hw), "Invalid clock source %u\n",
-clk_src);
- return -EINVAL;
- }
-
- if (clk_src == ICE_CLK_SRC_TCXO && clk_freq != ICE_TSPLL_FREQ_25_000) {
- dev_warn(ice_hw_to_dev(hw),
-"TCXO only supports 25 MHz frequency\n");
- return -EINVAL;
- }
-
ICE_READ_CGU_REG_OR_DIE(hw, ICE_CGU_R9, &r9);
ICE_READ_CGU_REG_OR_DIE(hw, ICE_CGU_R24, &r24);
ICE_READ_CGU_REG_OR_DIE(hw, ICE_CGU_RO_BWM_LF, &val);
@@ -253,23 +283,6 @@ static int ice_tspll_cfg_e825c(struct ice_hw *hw, enum
ice_tspll_freq clk_freq,
{
u32 val, r9, r23;
- if (clk_freq >= NUM_ICE_TSPLL_FREQ) {
- dev_warn(ice_hw_to_dev(hw), "Invalid TIME_REF frequency %u\n",
-clk_freq);
- return -EINVAL;
- }
-
- if (clk_src >= NUM_ICE_CLK_SRC) {
- dev_warn(ice_hw_to_dev(hw), "Invalid clock source %u\n",
-clk_src);
- return -EINVAL;
- }
-
- if (clk_freq != ICE_TSPLL_FREQ_156_250) {
- dev_warn(ice_hw_to_dev(hw), "Adapter only supports 156.25 MHz
frequency\n");
- return -EINVAL;
- }
-
ICE_READ_CGU_REG_OR_DIE(hw, ICE_CGU_R9, &r9);
ICE_READ_CGU_REG_OR_DIE(hw, ICE_CGU_R23, &r23);
ICE_READ_CGU_REG_OR_DIE(hw, ICE_CGU_RO_LOCK, &val);
@@ -394,6 +407,52 @@ int ice_tspll_cfg_pps_out_e825c(struct ice_hw *hw, bool
enable)
return ice_write_cgu_reg(hw, ICE_CGU_R9, val);
}
+/**
+ * ice_tspll_cfg - Configure the Clock Generation Unit TSPLL
+ * @hw: Pointer to the HW struct
+ * @clk_freq: Clock frequency to program
+ * @clk_src: Clock source to select (TIME_REF, or TCXO)
+ *
+ * Configure the Clock Generation Unit with the desired clock frequency and
+ * time reference, enabling the TSPLL which drives the PTP hardware clock.
+ *
+ * Return: 0 on success, -ERANGE on unsupported MAC type, other negative error
+ * codes when failed to configure CGU.
+ */
+static int ice_tspll_cfg(struct ice_hw *hw, enum ice_tspll_freq clk_freq,
+enum ice_clk_src clk_src)
+{
+ switch (hw->mac_type) {
+ case ICE_MAC_GENERIC:
+ retur
[Intel-wired-lan] [PATCH iwl-next 03/10] ice: use designated initializers for TSPLL consts
Instead of multiple comments, use designated initializers for TSPLL
consts.
Adjust ice_tspll_params_e82x fields sizes.
Remove ice_tspll_params_e825 definitions as according to EDS (Electrical
Design Specification) doc, E825 devices support only 156.25 MHz TSPLL
frequency for both TCXO and TIME_REF clock source.
Reviewed-by: Michal Kubiak
Reviewed-by: Milena Olech
Signed-off-by: Karol Kolacinski
---
drivers/net/ethernet/intel/ice/ice_tspll.c | 199 -
drivers/net/ethernet/intel/ice/ice_tspll.h | 29 +--
2 files changed, 45 insertions(+), 183 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_tspll.c
b/drivers/net/ethernet/intel/ice/ice_tspll.c
index 210be7861313..992c22a34ca9 100644
--- a/drivers/net/ethernet/intel/ice/ice_tspll.c
+++ b/drivers/net/ethernet/intel/ice/ice_tspll.c
@@ -4,164 +4,42 @@
static const struct
ice_tspll_params_e82x e82x_tspll_params[NUM_ICE_TSPLL_FREQ] = {
- /* ICE_TSPLL_FREQ_25_000 -> 25 MHz */
- {
- /* refclk_pre_div */
- 1,
- /* feedback_div */
- 197,
- /* frac_n_div */
- 2621440,
- /* post_pll_div */
- 6,
+ [ICE_TSPLL_FREQ_25_000] = {
+ .refclk_pre_div = 1,
+ .post_pll_div = 6,
+ .feedback_div = 197,
+ .frac_n_div = 2621440,
},
-
- /* ICE_TSPLL_FREQ_122_880 -> 122.88 MHz */
- {
- /* refclk_pre_div */
- 5,
- /* feedback_div */
- 223,
- /* frac_n_div */
- 524288,
- /* post_pll_div */
- 7,
- },
-
- /* ICE_TSPLL_FREQ_125_000 -> 125 MHz */
- {
- /* refclk_pre_div */
- 5,
- /* feedback_div */
- 223,
- /* frac_n_div */
- 524288,
- /* post_pll_div */
- 7,
- },
-
- /* ICE_TSPLL_FREQ_153_600 -> 153.6 MHz */
- {
- /* refclk_pre_div */
- 5,
- /* feedback_div */
- 159,
- /* frac_n_div */
- 1572864,
- /* post_pll_div */
- 6,
- },
-
- /* ICE_TSPLL_FREQ_156_250 -> 156.25 MHz */
- {
- /* refclk_pre_div */
- 5,
- /* feedback_div */
- 159,
- /* frac_n_div */
- 1572864,
- /* post_pll_div */
- 6,
- },
-
- /* ICE_TSPLL_FREQ_245_760 -> 245.76 MHz */
- {
- /* refclk_pre_div */
- 10,
- /* feedback_div */
- 223,
- /* frac_n_div */
- 524288,
- /* post_pll_div */
- 7,
- },
-};
-
-static const struct
-ice_tspll_params_e825c e825c_tspll_params[NUM_ICE_TSPLL_FREQ] = {
- /* ICE_TSPLL_FREQ_25_000 -> 25 MHz */
- {
- /* ck_refclkfreq */
- 0x19,
- /* ndivratio */
- 1,
- /* fbdiv_intgr */
- 320,
- /* fbdiv_frac */
- 0,
- /* ref1588_ck_div */
- 0,
+ [ICE_TSPLL_FREQ_122_880] = {
+ .refclk_pre_div = 5,
+ .post_pll_div = 7,
+ .feedback_div = 223,
+ .frac_n_div = 524288
},
-
- /* ICE_TSPLL_FREQ_122_880 -> 122.88 MHz */
- {
- /* ck_refclkfreq */
- 0x29,
- /* ndivratio */
- 3,
- /* fbdiv_intgr */
- 195,
- /* fbdiv_frac */
- 1342177280UL,
- /* ref1588_ck_div */
- 0,
+ [ICE_TSPLL_FREQ_125_000] = {
+ .refclk_pre_div = 5,
+ .post_pll_div = 7,
+ .feedback_div = 223,
+ .frac_n_div = 524288
},
-
- /* ICE_TSPLL_FREQ_125_000 -> 125 MHz */
- {
- /* ck_refclkfreq */
- 0x3E,
- /* ndivratio */
- 2,
- /* fbdiv_intgr */
- 128,
- /* fbdiv_frac */
- 0,
- /* ref1588_ck_div */
- 0,
+ [ICE_TSPLL_FREQ_153_600] = {
+ .refclk_pre_div = 5,
+ .post_pll_div = 6,
+ .feedback_div = 159,
+ .frac_n_div = 1572864
},
-
- /* ICE_TSPLL_FREQ_153_600 -> 153.6 MHz */
- {
- /* ck_refclkfreq */
- 0x33,
- /* ndivratio */
- 3,
- /* fbdiv_intgr */
- 156,
- /* fbdiv_frac */
- 1073741824UL,
- /* ref1588_ck_div */
- 0,
- },
-
- /* ICE_TSPLL_FREQ_156_250 -> 156.25 MHz */
- {
-
[Intel-wired-lan] [PATCH iwl-next 04/10] ice: add TSPLL log config helper
Add a helper function to print new/current TSPLL config. This helps
avoid unnecessary casts from u8 to enums.
Reviewed-by: Michal Kubiak
Reviewed-by: Milena Olech
Signed-off-by: Karol Kolacinski
---
drivers/net/ethernet/intel/ice/ice_tspll.c | 54 --
1 file changed, 30 insertions(+), 24 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_tspll.c
b/drivers/net/ethernet/intel/ice/ice_tspll.c
index 992c22a34ca9..4c929fca510e 100644
--- a/drivers/net/ethernet/intel/ice/ice_tspll.c
+++ b/drivers/net/ethernet/intel/ice/ice_tspll.c
@@ -86,6 +86,26 @@ static const char *ice_tspll_clk_src_str(enum ice_clk_src
clk_src)
}
}
+/**
+ * ice_tspll_log_cfg - Log current/new TSPLL configuration
+ * @hw: Pointer to the HW struct
+ * @enable: CGU enabled/disabled
+ * @clk_src: Current clock source
+ * @tspll_freq: Current clock frequency
+ * @lock: CGU lock status
+ * @new_cfg: true if this is a new config
+ */
+static void ice_tspll_log_cfg(struct ice_hw *hw, bool enable, u8 clk_src,
+ u8 tspll_freq, bool lock, bool new_cfg)
+{
+ dev_dbg(ice_hw_to_dev(hw),
+ "%s TSPLL configuration -- %s, src %s, freq %s, PLL %s\n",
+ new_cfg ? "New" : "Current", enable ? "enabled" : "disabled",
+ ice_tspll_clk_src_str((enum ice_clk_src)clk_src),
+ ice_tspll_clk_freq_str((enum ice_tspll_freq)tspll_freq),
+ lock ? "locked" : "unlocked");
+}
+
/**
* ice_tspll_cfg_e82x - Configure the Clock Generation Unit TSPLL
* @hw: Pointer to the HW struct
@@ -141,12 +161,9 @@ static int ice_tspll_cfg_e82x(struct ice_hw *hw, enum
ice_tspll_freq clk_freq,
if (err)
return err;
- /* Log the current clock configuration */
- ice_debug(hw, ICE_DBG_PTP, "Current TSPLL configuration -- %s, clk_src
%s, clk_freq %s, PLL %s\n",
- dw24.ts_pll_enable ? "enabled" : "disabled",
- ice_tspll_clk_src_str(dw24.time_ref_sel),
- ice_tspll_clk_freq_str(dw9.time_ref_freq_sel),
- bwm_lf.plllock_true_lock_cri ? "locked" : "unlocked");
+ ice_tspll_log_cfg(hw, dw24.ts_pll_enable, dw24.time_ref_sel,
+ dw9.time_ref_freq_sel, bwm_lf.plllock_true_lock_cri,
+ false);
/* Disable the PLL before changing the clock source or frequency */
if (dw24.ts_pll_enable) {
@@ -219,12 +236,8 @@ static int ice_tspll_cfg_e82x(struct ice_hw *hw, enum
ice_tspll_freq clk_freq,
return -EBUSY;
}
- /* Log the current clock configuration */
- ice_debug(hw, ICE_DBG_PTP, "New TSPLL configuration -- %s, clk_src %s,
clk_freq %s, PLL %s\n",
- dw24.ts_pll_enable ? "enabled" : "disabled",
- ice_tspll_clk_src_str(dw24.time_ref_sel),
- ice_tspll_clk_freq_str(dw9.time_ref_freq_sel),
- bwm_lf.plllock_true_lock_cri ? "locked" : "unlocked");
+ ice_tspll_log_cfg(hw, dw24.ts_pll_enable, clk_src, clk_freq, true,
+ true);
return 0;
}
@@ -318,12 +331,9 @@ static int ice_tspll_cfg_e825c(struct ice_hw *hw, enum
ice_tspll_freq clk_freq,
if (err)
return err;
- /* Log the current clock configuration */
- ice_debug(hw, ICE_DBG_PTP, "Current TSPLL configuration -- %s, clk_src
%s, clk_freq %s, PLL %s\n",
- dw24.ts_pll_enable ? "enabled" : "disabled",
- ice_tspll_clk_src_str(dw23.time_ref_sel),
- ice_tspll_clk_freq_str(dw9.time_ref_freq_sel),
- ro_lock.plllock_true_lock_cri ? "locked" : "unlocked");
+ ice_tspll_log_cfg(hw, dw24.ts_pll_enable, dw23.time_ref_sel,
+ dw9.time_ref_freq_sel,
+ ro_lock.plllock_true_lock_cri, false);
/* Disable the PLL before changing the clock source or frequency */
if (dw23.ts_pll_enable) {
@@ -417,12 +427,8 @@ static int ice_tspll_cfg_e825c(struct ice_hw *hw, enum
ice_tspll_freq clk_freq,
return -EBUSY;
}
- /* Log the current clock configuration */
- ice_debug(hw, ICE_DBG_PTP, "New TSPLL configuration -- %s, clk_src %s,
clk_freq %s, PLL %s\n",
- dw24.ts_pll_enable ? "enabled" : "disabled",
- ice_tspll_clk_src_str(dw23.time_ref_sel),
- ice_tspll_clk_freq_str(dw9.time_ref_freq_sel),
- ro_lock.plllock_true_lock_cri ? "locked" : "unlocked");
+ ice_tspll_log_cfg(hw, dw23.ts_pll_enable, clk_src, clk_freq, true,
+ true);
return 0;
}
--
2.48.1
[Intel-wired-lan] [PATCH iwl-next 08/10] ice: wait before enabling TSPLL
To ensure proper operation, wait for 10 to 20 microseconds before
enabling TSPLL.
Adjust wait time after enabling TSPLL from 1-5 ms to 1-2 ms.
Those values are empirical and tested on multiple HW configurations.
Reviewed-by: Milena Olech
Signed-off-by: Karol Kolacinski
---
drivers/net/ethernet/intel/ice/ice_tspll.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_tspll.c
b/drivers/net/ethernet/intel/ice/ice_tspll.c
index dce5164ebd9b..62da095d32ef 100644
--- a/drivers/net/ethernet/intel/ice/ice_tspll.c
+++ b/drivers/net/ethernet/intel/ice/ice_tspll.c
@@ -222,12 +222,15 @@ static int ice_tspll_cfg_e82x(struct ice_hw *hw, enum
ice_tspll_freq clk_freq,
r24 |= FIELD_PREP(ICE_CGU_R23_R24_TIME_REF_SEL, clk_src);
ICE_WRITE_CGU_REG_OR_DIE(hw, ICE_CGU_R24, r24);
+ /* Wait to ensure everything is stable */
+ usleep_range(10, 20);
+
/* Finally, enable the PLL */
r24 |= ICE_CGU_R23_R24_TSPLL_ENABLE;
ICE_WRITE_CGU_REG_OR_DIE(hw, ICE_CGU_R24, r24);
- /* Wait to verify if the PLL locks */
- usleep_range(1000, 5000);
+ /* Wait at least 1 ms to verify if the PLL locks */
+ usleep_range(USEC_PER_MSEC, 2 * USEC_PER_MSEC);
ICE_READ_CGU_REG_OR_DIE(hw, ICE_CGU_RO_BWM_LF, &val);
if (!(val & ICE_CGU_RO_BWM_LF_TRUE_LOCK)) {
@@ -348,12 +351,15 @@ static int ice_tspll_cfg_e825c(struct ice_hw *hw, enum
ice_tspll_freq clk_freq,
/* Clear the R24 register. */
ICE_WRITE_CGU_REG_OR_DIE(hw, ICE_CGU_R24, 0);
+ /* Wait to ensure everything is stable */
+ usleep_range(10, 20);
+
/* Finally, enable the PLL */
r23 |= ICE_CGU_R23_R24_TSPLL_ENABLE;
ICE_WRITE_CGU_REG_OR_DIE(hw, ICE_CGU_R23, r23);
- /* Wait to verify if the PLL locks */
- usleep_range(1000, 5000);
+ /* Wait at least 1 ms to verify if the PLL locks */
+ usleep_range(USEC_PER_MSEC, 2 * USEC_PER_MSEC);
ICE_READ_CGU_REG_OR_DIE(hw, ICE_CGU_RO_LOCK, &val);
if (!(val & ICE_CGU_RO_LOCK_TRUE_LOCK)) {
--
2.48.1
[Intel-wired-lan] [PATCH iwl-next 09/10] ice: fall back to TCXO on TSPLL lock fail
TSPLL can fail when trying to lock to TIME_REF as a clock source, e.g.
when the external clock source is not stable or connected to the board.
To continue operation after failure, try to lock again to internal TCXO
and inform user about this.
Reviewed-by: Milena Olech
Signed-off-by: Karol Kolacinski
---
drivers/net/ethernet/intel/ice/ice_tspll.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_tspll.c
b/drivers/net/ethernet/intel/ice/ice_tspll.c
index 62da095d32ef..37fcfdd5e032 100644
--- a/drivers/net/ethernet/intel/ice/ice_tspll.c
+++ b/drivers/net/ethernet/intel/ice/ice_tspll.c
@@ -487,5 +487,17 @@ int ice_tspll_init(struct ice_hw *hw)
/* Configure the TSPLL using the parameters from the function
* capabilities.
*/
- return ice_tspll_cfg(hw, tspll_freq, clk_src);
+ err = ice_tspll_cfg(hw, tspll_freq, clk_src);
+ if (err) {
+ dev_warn(ice_hw_to_dev(hw), "Failed to lock TSPLL to predefined
frequency. Retrying with fallback frequency.\n");
+
+ /* Try to lock to internal TCXO as a fallback. */
+ tspll_freq = ice_tspll_default_freq(hw->mac_type);
+ clk_src = ICE_CLK_SRC_TCXO;
+ err = ice_tspll_cfg(hw, tspll_freq, clk_src);
+ if (err)
+ dev_warn(ice_hw_to_dev(hw), "Failed to lock TSPLL to
fallback frequency.\n");
+ }
+
+ return err;
}
--
2.48.1
Re: [Intel-wired-lan] [PATCH iwl-next v2 2/4] igb: Link queues to NAPI instances
On Mon Mar 10 2025, Joe Damato wrote: > On Fri, Mar 07, 2025 at 02:03:44PM -0800, Tony Nguyen wrote: >> On 2/17/2025 3:31 AM, Kurt Kanzenbach wrote: >> >> ... >> >> > diff --git a/drivers/net/ethernet/intel/igb/igb_xsk.c >> > b/drivers/net/ethernet/intel/igb/igb_xsk.c >> > index >> > 157d43787fa0b55a74714f69e9e7903b695fcf0a..a5ad090dfe94b6afc8194fe39d28cdd51c7067b0 >> > 100644 >> > --- a/drivers/net/ethernet/intel/igb/igb_xsk.c >> > +++ b/drivers/net/ethernet/intel/igb/igb_xsk.c >> > @@ -45,6 +45,7 @@ static void igb_txrx_ring_disable(struct igb_adapter >> > *adapter, u16 qid) >> >synchronize_net(); >> >/* Rx/Tx share the same napi context. */ >> > + igb_set_queue_napi(adapter, qid, NULL); >> >napi_disable(&rx_ring->q_vector->napi); >> >igb_clean_tx_ring(tx_ring); >> > @@ -78,6 +79,7 @@ static void igb_txrx_ring_enable(struct igb_adapter >> > *adapter, u16 qid) >> >/* Rx/Tx share the same napi context. */ >> >napi_enable(&rx_ring->q_vector->napi); >> > + igb_set_queue_napi(adapter, qid, &rx_ring->q_vector->napi); >> > } >> > struct xsk_buff_pool *igb_xsk_pool(struct igb_adapter *adapter, >> >> I believe Joe's fix/changes [1] need to be done here as well? >> >> Thanks, >> Tony >> >> [1] >> https://lore.kernel.org/intel-wired-lan/9ddf6293-6cb0-47ea-a0e7-cad7d33c2...@intel.com/T/#m863614df1fb3d1980ad09016b1c9ef4e2f0b074e > > Yes, the code above should be dropped. Sorry I missed that during > review - thanks for catching that, Tony. > > Kurt: when you respin this to fix what Tony mentioned, can you also > run the test mentioned above? Hi Tony & Joe, Hm. I did run the test and it succeeded. I'll take a look at it next week when I'm back in the office. Thanks, Kurt signature.asc Description: PGP signature
[Intel-wired-lan] [PATCH iwl-next 10/10] ice: move TSPLL init calls to ice_ptp.c
Initialize TSPLL after initializing PHC in ice_ptp.c instead of calling
for each product in PHC init in ice_ptp_hw.c.
Reviewed-by: Michal Kubiak
Reviewed-by: Milena Olech
Signed-off-by: Karol Kolacinski
---
drivers/net/ethernet/intel/ice/ice_ptp.c| 11 ++
drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 24 +
drivers/net/ethernet/intel/ice/ice_tspll.c | 5 +
3 files changed, 17 insertions(+), 23 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c
b/drivers/net/ethernet/intel/ice/ice_ptp.c
index fdeb20ac831c..5fbd77e0cb17 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -2864,6 +2864,10 @@ static int ice_ptp_rebuild_owner(struct ice_pf *pf)
if (err)
return err;
+ err = ice_tspll_init(hw);
+ if (err)
+ return err;
+
/* Acquire the global hardware lock */
if (!ice_ptp_lock(hw)) {
err = -EBUSY;
@@ -3038,6 +3042,13 @@ static int ice_ptp_init_owner(struct ice_pf *pf)
return err;
}
+ err = ice_tspll_init(hw);
+ if (err) {
+ dev_err(ice_pf_to_dev(pf), "Failed to initialize CGU, status
%d\n",
+ err);
+ return err;
+ }
+
/* Acquire the global hardware lock */
if (!ice_ptp_lock(hw)) {
err = -EBUSY;
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
index 689feac7baf9..ba97a52917af 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
@@ -2107,22 +2107,6 @@ static void ice_sb_access_ena_eth56g(struct ice_hw *hw,
bool enable)
wr32(hw, PF_SB_REM_DEV_CTL, val);
}
-/**
- * ice_ptp_init_phc_e825 - Perform E825 specific PHC initialization
- * @hw: pointer to HW struct
- *
- * Perform E825-specific PTP hardware clock initialization steps.
- *
- * Return: 0 on success, negative error code otherwise.
- */
-static int ice_ptp_init_phc_e825(struct ice_hw *hw)
-{
- ice_sb_access_ena_eth56g(hw, true);
-
- /* Initialize the Clock Generation Unit */
- return ice_tspll_init(hw);
-}
-
/**
* ice_ptp_read_tx_hwtstamp_status_eth56g - Get TX timestamp status
* @hw: pointer to the HW struct
@@ -2784,7 +2768,6 @@ static int ice_ptp_set_vernier_wl(struct ice_hw *hw)
*/
static int ice_ptp_init_phc_e82x(struct ice_hw *hw)
{
- int err;
u32 val;
/* Enable reading switch and PHY registers over the sideband queue */
@@ -2794,11 +2777,6 @@ static int ice_ptp_init_phc_e82x(struct ice_hw *hw)
val |= (PF_SB_REM_DEV_CTL_SWITCH_READ | PF_SB_REM_DEV_CTL_PHY0);
wr32(hw, PF_SB_REM_DEV_CTL, val);
- /* Initialize the Clock Generation Unit */
- err = ice_tspll_init(hw);
- if (err)
- return err;
-
/* Set window length for all the ports */
return ice_ptp_set_vernier_wl(hw);
}
@@ -5580,7 +5558,7 @@ int ice_ptp_init_phc(struct ice_hw *hw)
case ICE_MAC_GENERIC:
return ice_ptp_init_phc_e82x(hw);
case ICE_MAC_GENERIC_3K_E825:
- return ice_ptp_init_phc_e825(hw);
+ return 0;
default:
return -EOPNOTSUPP;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_tspll.c
b/drivers/net/ethernet/intel/ice/ice_tspll.c
index 37fcfdd5e032..17c23b29b53c 100644
--- a/drivers/net/ethernet/intel/ice/ice_tspll.c
+++ b/drivers/net/ethernet/intel/ice/ice_tspll.c
@@ -474,6 +474,11 @@ int ice_tspll_init(struct ice_hw *hw)
enum ice_clk_src clk_src;
int err;
+ /* Only E822, E823 and E825 products support TSPLL */
+ if (hw->mac_type != ICE_MAC_GENERIC &&
+ hw->mac_type != ICE_MAC_GENERIC_3K_E825)
+ return 0;
+
tspll_freq = (enum ice_tspll_freq)ts_info->time_ref;
clk_src = (enum ice_clk_src)ts_info->clk_src;
if (!ice_tspll_check_params(hw, tspll_freq, clk_src))
--
2.48.1
Re: [Intel-wired-lan] [PATCH iwl-next] ice: use DSN instead of PCI BDF for ice_adapter index
Mon, Mar 10, 2025 at 09:40:16AM +0100, przemyslaw.kits...@intel.com wrote: >> Subject: Re: [PATCH iwl-next] ice: use DSN instead of PCI BDF for >> ice_adapter index > >regarding -net vs -next, no one have complained that this bug hurts Wait, so we are now waiting for someone to hit the bug and complain, before we do fix? Does not make any sense to me. > >> > + return (unsigned long)pci_get_dsn(pdev); >> >> > How do you ensure there is no xarray index collision then you cut the >> > number like this? > >The reduction occurs only on "32b" systems, which are unlikely to have >this device. And any mixing of the upper and lower 4B part still could >collide. Passtrough to 32 bit qemu machine? Even how unlikely is that, you are risking a user to hit a bug for newly introduced code without good reason. Why? > >> >> It is also probably necessary to check if all devices supported by the >> driver have DSN capability enabled. > >I will double check on the SoC you have in mind. > >> >> Regards, >> Sergey >
[Intel-wired-lan] [PATCH iwl-next v2 3/3] ice: enable timesync operation on 2xNAC E825 devices
From: Karol Kolacinski
According to the E825C specification, SBQ address for ports on a single
complex is device 2 for PHY 0 and device 13 for PHY1.
For accessing ports on a dual complex E825C (so called 2xNAC mode),
the driver should use destination device 2 (referred as phy_0) for
the current complex PHY and device 13 (referred as phy_0_peer) for
peer complex PHY.
Differentiate SBQ destination device by checking if current PF port
number is on the same PHY as target port number.
Adjust 'ice_get_lane_number' function to provide unique port number for
ports from PHY1 in 'dual' mode config (by adding fixed offset for PHY1
ports). Cache this value in ice_hw struct.
Introduce ice_get_primary_hw wrapper to get access to timesync register
not available from second NAC.
Reviewed-by: Przemek Kitszel
Signed-off-by: Karol Kolacinski
Co-developed-by: Grzegorz Nitka
Signed-off-by: Grzegorz Nitka
---
drivers/net/ethernet/intel/ice/ice.h| 60 -
drivers/net/ethernet/intel/ice/ice_common.c | 6 ++-
drivers/net/ethernet/intel/ice/ice_ptp.c| 49 -
drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 39 +++---
drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 5 --
drivers/net/ethernet/intel/ice/ice_type.h | 1 +
6 files changed, 134 insertions(+), 26 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice.h
b/drivers/net/ethernet/intel/ice/ice.h
index 2694951a0b1d..e42572ae7631 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -193,8 +193,6 @@
#define ice_pf_to_dev(pf) (&((pf)->pdev->dev))
-#define ice_pf_src_tmr_owned(pf)
((pf)->hw.func_caps.ts_func_info.src_tmr_owned)
-
enum ice_feature {
ICE_F_DSCP,
ICE_F_PHY_RCLK,
@@ -1046,4 +1044,62 @@ static inline void ice_clear_rdma_cap(struct ice_pf *pf)
}
extern const struct xdp_metadata_ops ice_xdp_md_ops;
+
+/**
+ * ice_is_dual - Check if given config is multi-NAC
+ * @hw: pointer to HW structure
+ *
+ * Return: true if the device is running in mutli-NAC (Network
+ * Acceleration Complex) configuration variant, false otherwise
+ * (always false for non-E825 devices).
+ */
+static inline bool ice_is_dual(struct ice_hw *hw)
+{
+ return hw->mac_type == ICE_MAC_GENERIC_3K_E825 &&
+ (hw->dev_caps.nac_topo.mode & ICE_NAC_TOPO_DUAL_M);
+}
+
+/**
+ * ice_is_primary - Check if given device belongs to the primary complex
+ * @hw: pointer to HW structure
+ *
+ * Check if given PF/HW is running on primary complex in multi-NAC
+ * configuration.
+ *
+ * Return: true if the device is dual, false otherwise (always true
+ * for non-E825 devices).
+ */
+static inline bool ice_is_primary(struct ice_hw *hw)
+{
+ return hw->mac_type != ICE_MAC_GENERIC_3K_E825 ||
+ !ice_is_dual(hw) ||
+ (hw->dev_caps.nac_topo.mode & ICE_NAC_TOPO_PRIMARY_M);
+}
+
+/**
+ * ice_pf_src_tmr_owned - Check if a primary timer is owned by PF
+ * @pf: pointer to PF structure
+ *
+ * Return: true if PF owns primary timer, false otherwise.
+ */
+static inline bool ice_pf_src_tmr_owned(struct ice_pf *pf)
+{
+ return pf->hw.func_caps.ts_func_info.src_tmr_owned &&
+ ice_is_primary(&pf->hw);
+}
+
+/**
+ * ice_get_primary_hw - Get pointer to primary ice_hw structure
+ * @pf: pointer to PF structure
+ *
+ * Return: A pointer to ice_hw structure with access to timesync
+ * register space.
+ */
+static inline struct ice_hw *ice_get_primary_hw(struct ice_pf *pf)
+{
+ if (!pf->adapter->ctrl_pf)
+ return &pf->hw;
+ else
+ return &pf->adapter->ctrl_pf->hw;
+}
#endif /* _ICE_H_ */
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c
b/drivers/net/ethernet/intel/ice/ice_common.c
index 8f0c72df2a44..547b16f9fb17 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -1135,6 +1135,8 @@ int ice_init_hw(struct ice_hw *hw)
}
}
+ hw->lane_num = ice_get_phy_lane_number(hw);
+
return 0;
err_unroll_fltr_mgmt_struct:
ice_cleanup_fltr_mgmt_struct(hw);
@@ -4091,10 +4093,12 @@ int ice_get_phy_lane_number(struct ice_hw *hw)
continue;
if (hw->pf_id == lport) {
+ if (hw->mac_type == ICE_MAC_GENERIC_3K_E825 &&
+ ice_is_dual(hw) && !ice_is_primary(hw))
+ lane += ICE_PORTS_PER_QUAD;
kfree(options);
return lane;
}
-
lport++;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c
b/drivers/net/ethernet/intel/ice/ice_ptp.c
index cdd76ecb2196..85b614135694 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -208,6 +208,9 @@ u64 ice_ptp_read_src_clk_reg(struct ice_pf *pf,
u32 hi, lo, lo2;
u8 tmr_idx;
+ if (!ice_is_primary(hw))
[Intel-wired-lan] [PATCH iwl-next v2 1/3] ice: remove SW side band access workaround for E825
From: Karol Kolacinski
Due to the bug in FW/NVM autoload mechanism (wrong default
SB_REM_DEV_CTL register settings), the access to peer PHY and CGU
clients was disabled by default.
As the workaround solution, the register value was overwritten by the
driver at the probe or reset handling.
Remove workaround as it's not needed anymore. The fix in autoload
procedure has been provided with NVM 3.80 version.
Reviewed-by: Michal Swiatkowski
Reviewed-by: Przemek Kitszel
Signed-off-by: Karol Kolacinski
Signed-off-by: Grzegorz Nitka
---
drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 23 -
1 file changed, 23 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
index 89bb8461284a..a5df081ffc19 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
@@ -2630,25 +2630,6 @@ int ice_start_phy_timer_eth56g(struct ice_hw *hw, u8
port)
return 0;
}
-/**
- * ice_sb_access_ena_eth56g - Enable SB devices (PHY and others) access
- * @hw: pointer to HW struct
- * @enable: Enable or disable access
- *
- * Enable sideband devices (PHY and others) access.
- */
-static void ice_sb_access_ena_eth56g(struct ice_hw *hw, bool enable)
-{
- u32 val = rd32(hw, PF_SB_REM_DEV_CTL);
-
- if (enable)
- val |= BIT(eth56g_phy_0) | BIT(cgu) | BIT(eth56g_phy_1);
- else
- val &= ~(BIT(eth56g_phy_0) | BIT(cgu) | BIT(eth56g_phy_1));
-
- wr32(hw, PF_SB_REM_DEV_CTL, val);
-}
-
/**
* ice_ptp_init_phc_e825 - Perform E825 specific PHC initialization
* @hw: pointer to HW struct
@@ -2659,8 +2640,6 @@ static void ice_sb_access_ena_eth56g(struct ice_hw *hw,
bool enable)
*/
static int ice_ptp_init_phc_e825(struct ice_hw *hw)
{
- ice_sb_access_ena_eth56g(hw, true);
-
/* Initialize the Clock Generation Unit */
return ice_init_cgu_e82x(hw);
}
@@ -2747,8 +2726,6 @@ static void ice_ptp_init_phy_e825(struct ice_hw *hw)
params->num_phys = 2;
ptp->ports_per_phy = 4;
ptp->num_lports = params->num_phys * ptp->ports_per_phy;
-
- ice_sb_access_ena_eth56g(hw, true);
}
/* E822 family functions
--
2.39.3
[Intel-wired-lan] [PATCH iwl-next v2 0/3] E825C timesync dual NAC support
This patch series adds full support for timesync operations for E8225C devices which are configured in so called 2xNAC mode (Network Acceleration Complex). 2xNAC mode is the mode in which IO die is housing two complexes and each of them has its own PHY connected to it. The complex which controls time transmitter is referred as primary complex. The series solves known configuration issues in dual config mode: - side-band queue (SBQ) addressing when configuring the ports on the PHY on secondary NAC - access to timesync config from the second NAC as only one PF in primary NAC controls time transmitter clock v1->v2: - fixed ice_pf_src_tmr_owned function doc - fixed type for lane_num field in ice_hw struct Karol Kolacinski (3): ice: remove SW side band access workaround for E825 ice: refactor ice_sbq_msg_dev enum ice: enable timesync operation on 2xNAC E825 devices drivers/net/ethernet/intel/ice/ice.h | 60 +- drivers/net/ethernet/intel/ice/ice_common.c | 8 +- drivers/net/ethernet/intel/ice/ice_ptp.c | 49 +--- drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 82 ++-- drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 5 -- drivers/net/ethernet/intel/ice/ice_sbq_cmd.h | 11 +-- drivers/net/ethernet/intel/ice/ice_type.h| 1 + 7 files changed, 149 insertions(+), 67 deletions(-) base-commit: daa2036c311e81ee32f88257e3dfd4985f79 -- 2.39.3
Re: [Intel-wired-lan] [PATCH iwl-next v1 1/3] ice: remove SW side band access workaround for E825
> -Original Message- > From: Intel-wired-lan On Behalf Of > Nitka, Grzegorz > Sent: Tuesday, March 4, 2025 2:04 PM > To: Paul Menzel ; Kolacinski, Karol > > Cc: intel-wired-...@lists.osuosl.org; net...@vger.kernel.org; Kitszel, > Przemyslaw ; Michal Swiatkowski > > Subject: Re: [Intel-wired-lan] [PATCH iwl-next v1 1/3] ice: remove SW side > band access workaround for E825 > > > -Original Message- > > From: Paul Menzel > > Sent: Friday, February 21, 2025 10:16 PM > > To: Nitka, Grzegorz ; Kolacinski, Karol > > > > Cc: intel-wired-...@lists.osuosl.org; net...@vger.kernel.org; Kitszel, > > Przemyslaw ; Michal Swiatkowski > > > > Subject: Re: [Intel-wired-lan] [PATCH iwl-next v1 1/3] ice: remove SW side > > band access workaround for E825 > > > > Dear Grzegorz, dear Karol, > > > > > > Thank you for your patch. > > > > Am 21.02.25 um 13:31 schrieb Grzegorz Nitka: > > > From: Karol Kolacinski > > > > > > Due to the bug in FW/NVM autoload mechanism (wrong default > > > SB_REM_DEV_CTL register settings), the access to peer PHY and CGU > > > clients was disabled by default. > > > > I’d add a blank line between the paragraphs. > > > > > As the workaround solution, the register value was overwritten by the > > > driver at the probe or reset handling. > > > Remove workaround as it's not needed anymore. The fix in autoload > > > procedure has been provided with NVM 3.80 version. > > > > Is this compatible with Linux’ no regression policy? People might only > > update the Linux kernel and not the firmware. > > > > How did you test this, and how can others test this? > > > > > Reviewed-by: Michal Swiatkowski > > > Reviewed-by: Przemek Kitszel > > > Signed-off-by: Karol Kolacinski > > > Signed-off-by: Grzegorz Nitka > > > --- > > > drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 23 - > > > 1 file changed, 23 deletions(-) > > > > > > Kind regards, > > > > Paul > > > > > > Dear Paul, first of all thank you for your review and apologize for late > response (simply, I was out previous week). > > Removing that workaround was a conscious decision. > Rationale for that is, that the 'problematic' workaround was provided > in very early product development stage (~ year ago). Now, especially > when E825-C product was just officially announced, the customer shall > use official, approved SW ingredients. > > Here are more details on this: > - introduction to E825-C devices was provided in kernel 6.6, to allow > selected customers for early E825-C enablement. At that time E825-C > product family was in early phase (kind of Alpha), and one of the > consequences, from today's perspective, is that it included 'unwanted' > workarounds like this. > > - this change applies to E825-C products only, which is SoC product, not > a regular NIC card. What I'd like to emphasize here, it requires even more > customer support from Intel company in terms of providing matching > SW stack (like BIOS, firmware, drivers etc.). > > I see two possibilities now: > 1) if the regression policy you mentioned is inviolable, keep the workaround >in the kernel code like it is today. Maybe we could add NVM version > checker >and apply register updates for older NVMs only. >But, in my opinion, it would lead to keeping a dead code. There shouldn't > be >official FW (I hope I won't regret these words) on the market which > requires >this workaround. > > 2) remove the workaround like it's implemented in this patch and improve >commit message to clarify all potential doubts/questions from the user >perspective. > > What's your thoughts? > > Kind regards > > Grzegorz > I've just submitted v2 of this series, but no changes in this area yet (except adding blank line suggestion) I'm waiting for feedback or confirmation if above justification is sufficient. I'll submit one more if needed. Regards Grzegorz
[Intel-wired-lan] [PATCH net 1/5] igb: reject invalid external timestamp requests for 82580-based HW
The igb_ptp_feature_enable_82580 function correctly checks that unknown
flags are not passed to the function. However, it does not actually check
PTP_RISING_EDGE or PTP_FALLING_EDGE when configuring the external timestamp
function.
The data sheet for the 82580 product says:
Upon a change in the input level of one of the SDP pins that was
configured to detect Time stamp events using the TSSDP register, a time
stamp of the system time is captured into one of the two auxiliary time
stamp registers (AUXSTMPL/H0 or AUXSTMPL/H1).
For example to define timestamping of events in the AUXSTMPL0 and
AUXSTMPH0 registers, Software should:
1. Set the TSSDP.AUX0_SDP_SEL field to select the SDP pin that detects
the level change and set the TSSDP.AUX0_TS_SDP_EN bit to 1.
2. Set the TSAUXC.EN_TS0 bit to 1 to enable timestamping
The same paragraph is in the i350 and i354 data sheets.
The wording implies that the time stamps are captured at any level change.
There does not appear to be any way to only timestamp one edge of the
signal.
Reject requests which do not set both PTP_RISING_EDGE and PTP_FALLING_EDGE
when operating under PTP_STRICT_FLAGS mode via PTP_EXTTS_REQUEST2.
Fixes: 38970eac41db ("igb: support EXTTS on 82580/i354/i350")
Signed-off-by: Jacob Keller
---
drivers/net/ethernet/intel/igb/igb_ptp.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c
b/drivers/net/ethernet/intel/igb/igb_ptp.c
index
f9457055612004c10f74379122063e8136fe7d76..b89ef4538a18d7ca11325ddc15944a878f4d807e
100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -509,6 +509,11 @@ static int igb_ptp_feature_enable_82580(struct
ptp_clock_info *ptp,
PTP_STRICT_FLAGS))
return -EOPNOTSUPP;
+ /* Both the rising and falling edge are timstamped */
+ if (rq->extts.flags & PTP_STRICT_FLAGS &&
+ (rq->extts.flags & PTP_EXTTS_EDGES) != PTP_EXTTS_EDGES)
+ return -EOPNOTSUPP;
+
if (on) {
pin = ptp_find_pin(igb->ptp_clock, PTP_PF_EXTTS,
rq->extts.index);
--
2.48.1.397.gec9d649cc640
[Intel-wired-lan] [PATCH net 3/5] net: lan743x: reject unsupported external timestamp requests
The lan743x_ptp_io_event_cap_en() function checks that the given request
sets only one of PTP_RISING_EDGE or PTP_FALLING_EDGE, but not both.
However, this driver does not check whether other flags (such as
PTP_EXT_OFF) are set, nor whether any future unrecognized flags are set.
Fix this by adding the appropriate check to the lan743x_ptp_io_extts()
function.
Fixes: 60942c397af6 ("net: lan743x: Add support for PTP-IO Event Input External
Timestamp (extts)")
Signed-off-by: Jacob Keller
---
drivers/net/ethernet/microchip/lan743x_ptp.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/ethernet/microchip/lan743x_ptp.c
b/drivers/net/ethernet/microchip/lan743x_ptp.c
index
4a777b449ecd03ac0d7576f8570f1a7794fb3d06..0be44dcb339387e9756f36f909f65c20870bc49b
100644
--- a/drivers/net/ethernet/microchip/lan743x_ptp.c
+++ b/drivers/net/ethernet/microchip/lan743x_ptp.c
@@ -942,6 +942,12 @@ static int lan743x_ptp_io_extts(struct lan743x_adapter
*adapter, int on,
extts = &ptp->extts[index];
+ if (extts_request->flags & ~(PTP_ENABLE_FEATURE |
+PTP_RISING_EDGE |
+PTP_FALLING_EDGE |
+PTP_STRICT_FLAGS))
+ return -EOPNOTSUPP;
+
if (on) {
extts_pin = ptp_find_pin(ptp->ptp_clock, PTP_PF_EXTTS, index);
if (extts_pin < 0)
--
2.48.1.397.gec9d649cc640
[Intel-wired-lan] [PATCH net 4/5] broadcom: fix supported flag check in periodic output function
In bcm_ptp_perout_locked, the driver rejects requests which have
PTP_PEROUT_PHASE set. This appears to be an attempt to reject any
unsupported flags. Unfortunately, this only checks one flag, but does not
protect against PTP_PEROUT_ONE_SHOT, or any future flags which may be
added.
Fix the check to ensure that no flag other than the supported
PTP_PEROUT_DUTY_CYCLE is set.
Fixes: 7bfe91efd525 ("net: phy: Add support for 1PPS out and external
timestamps")
Signed-off-by: Jacob Keller
---
drivers/net/phy/bcm-phy-ptp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/bcm-phy-ptp.c b/drivers/net/phy/bcm-phy-ptp.c
index
208e8f561e0696e64bd5e842b66d88c65d70bfc0..eba8b5fb1365f4e43331e479e8e2f3c4b590ab96
100644
--- a/drivers/net/phy/bcm-phy-ptp.c
+++ b/drivers/net/phy/bcm-phy-ptp.c
@@ -597,7 +597,8 @@ static int bcm_ptp_perout_locked(struct bcm_ptp_private
*priv,
period = BCM_MAX_PERIOD_8NS;/* write nonzero value */
- if (req->flags & PTP_PEROUT_PHASE)
+ /* Reject unsupported flags */
+ if (req->flags & ~PTP_PEROUT_DUTY_CYCLE)
return -EOPNOTSUPP;
if (req->flags & PTP_PEROUT_DUTY_CYCLE)
--
2.48.1.397.gec9d649cc640
[Intel-wired-lan] [PATCH net 0/5] net: ptp: fix egregious supported flag checks
In preparation for adding .supported_extts_flags and .supported_perout_flags to the ptp_clock_info structure, fix a couple of places where drivers get existing flag gets grossly incorrect. The igb driver claims 82580 supports strictly validating PTP_RISING_EDGE and PTP_FALLING_EDGE, but doesn't actually check the flags. Fix the driver to require that the request match both edges, as this is implied by the datasheet description. The renesas driver also claims to support strict flag checking, but does not actually check the flags either. I do not have the data sheet for this device, so I do not know what edge it timestamps. For simplicity, just reject all requests with PTP_STRICT_FLAGS. This essentially prevents the PTP_EXTTS_REQUEST2 ioctl from working. Updating to correctly validate the flags will require someone who has the hardware to confirm the behavior. The lan743x driver supports (and strictly validates) that the request is either PTP_RISING_EDGE or PTP_FALLING_EDGE but not both. However, it does not check the flags are one of the known valid flags. Thus, requests for PTP_EXT_OFF (and any future flag) will be accepted and misinterpreted. Add the appropriate check to reject unsupported PTP_EXT_OFF requests and future proof against new flags. The broadcom PHY driver checks that PTP_PEROUT_PHASE is not set. This appears to be an attempt at rejecting unsupported flags. It is not robust against flag additions such as the PTP_PEROUT_ONE_SHOT, or anything added in the future. Fix this by instead checking against the negation of the supported PTP_PEROUT_DUTY_CYCLE instead. The ptp_ocp driver supports PTP_PEROUT_PHASE and PTP_PEROUT_DUTY_CYCLE, but does not check unsupported flags. Add the appropriate check to ensure PTP_PEROUT_ONE_SHOT and any future flags are rejected as unsupported. These are changes compile-tested, but I do not have hardware to validate the behavior. There are a number of other drivers which enable periodic output or external timestamp requests, but which do not check flags at all. We could go through each of these drivers one-by-one and meticulously add a flag check. Instead, these drivers will be covered only by the upcoming .supported_extts_flags and .supported_perout_flags checks in a net-next series. Signed-off-by: Jacob Keller --- Jacob Keller (5): igb: reject invalid external timestamp requests for 82580-based HW renesas: reject PTP_STRICT_FLAGS as unsupported net: lan743x: reject unsupported external timestamp requests broadcom: fix supported flag check in periodic output function ptp: ocp: reject unsupported periodic output flags drivers/net/ethernet/intel/igb/igb_ptp.c | 5 + drivers/net/ethernet/microchip/lan743x_ptp.c | 6 ++ drivers/net/ethernet/renesas/ravb_ptp.c | 3 +-- drivers/net/phy/bcm-phy-ptp.c| 3 ++- drivers/ptp/ptp_ocp.c| 4 5 files changed, 18 insertions(+), 3 deletions(-) --- base-commit: 992ee3ed6e9fdd0be83a7daa5ff738e3cf86047f change-id: 20250310-jk-net-fixes-supported-extts-flags-e8434d8e2552 Best regards, -- Jacob Keller
[Intel-wired-lan] [PATCH net 5/5] ptp: ocp: reject unsupported periodic output flags
The ptp_ocp_signal_from_perout() function supports PTP_PEROUT_DUTY_CYCLE
and PTP_PEROUT_PHASE. It does not support PTP_PEROUT_ONE_SHOT, but does not
reject a request with such an unsupported flag.
Add the appropriate check to ensure that unsupported requests are rejected
both for PTP_PEROUT_ONE_SHOT as well as any future flags.
Fixes: 1aa66a3a135a ("ptp: ocp: Program the signal generators via
PTP_CLK_REQ_PEROUT")
Signed-off-by: Jacob Keller
---
drivers/ptp/ptp_ocp.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index
b651087f426f50a73229ca57634fc5d6912e0a87..4a87af0980d695a9ab1b23e2544f620759ccb892
100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -2090,6 +2090,10 @@ ptp_ocp_signal_from_perout(struct ptp_ocp *bp, int gen,
{
struct ptp_ocp_signal s = { };
+ if (req->flags & ~(PTP_PEROUT_DUTY_CYCLE |
+ PTP_PEROUT_PHASE))
+ return -EOPNOTSUPP;
+
s.polarity = bp->signal[gen].polarity;
s.period = ktime_set(req->period.sec, req->period.nsec);
if (!s.period)
--
2.48.1.397.gec9d649cc640
