If Clock Generation Unit and dplls are present on NIC board user shall
know its details.
Provide the devlink info callback with a new:
- fixed type object `cgu.id` - hardware variant of onboard CGU
- running type object `fw.cgu` - CGU firmware version
- running type object `fw.cgu.build` - CGU configuration build version
These information shall be known for debugging purposes.
Test (on NIC board with CGU)
$ devlink dev info <bus_name>/<dev_name> | grep cgu
cgu.id 8032
fw.cgu 6021
fw.cgu.build 0x1030001
Test (on NIC board without CGU)
$ devlink dev info <bus_name>/<dev_name> | grep cgu -c
0
Signed-off-by: Arkadiusz Kubalewski <[email protected]>
---
Documentation/networking/devlink/ice.rst | 14 +++++++++
drivers/net/ethernet/intel/ice/ice_devlink.c | 30 ++++++++++++++++++++
drivers/net/ethernet/intel/ice/ice_main.c | 5 +++-
drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 12 ++++----
drivers/net/ethernet/intel/ice/ice_type.h | 9 +++++-
5 files changed, 62 insertions(+), 8 deletions(-)
diff --git a/Documentation/networking/devlink/ice.rst b/Documentation/networking/devlink/ice.rst
index 10f282c2117c..3a54421c503d 100644
--- a/Documentation/networking/devlink/ice.rst
+++ b/Documentation/networking/devlink/ice.rst
@@ -23,6 +23,11 @@ The ``ice`` driver reports the following versions
- fixed
- K65390-000
- The Product Board Assembly (PBA) identifier of the board.
+ * - ``cgu.id``
+ - fixed
+ - 8032
+ - The Clock Generation Unit (CGU) hardware version identifier on the
+ board.
* - ``fw.mgmt``
- running
- 2.1.7
@@ -89,6 +94,15 @@ The ``ice`` driver reports the following versions
- running
- 0xee16ced7
- The first 4 bytes of the hash of the netlist module contents.
+ * - ``fw.cgu``
+ - running
+ - 6021
+ - Version of Clock Generation Unit (CGU) firmware.
+ * - ``fw.cgu.build``
+ - running
+ - 0x1030001
+ - Version of Clock Generation Unit (CGU) firmware configuration build.
+
Flash Update
============
diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
index bc44cc220818..06fe895739af 100644
--- a/drivers/net/ethernet/intel/ice/ice_devlink.c
+++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
@@ -193,6 +193,33 @@ ice_info_pending_netlist_build(struct ice_pf __always_unused *pf,
snprintf(ctx->buf, sizeof(ctx->buf), "0x%08x", netlist->hash);
}
+static void ice_info_cgu_id(struct ice_pf *pf, struct ice_info_ctx *ctx)
+{
+ if (ice_is_feature_supported(pf, ICE_F_CGU)) {
+ struct ice_hw *hw = &pf->hw;
+
+ snprintf(ctx->buf, sizeof(ctx->buf), "%u", hw->cgu.id);
+ }
+}
+
+static void ice_info_cgu_fw_version(struct ice_pf *pf, struct ice_info_ctx *ctx)
+{
+ if (ice_is_feature_supported(pf, ICE_F_CGU)) {
+ struct ice_hw *hw = &pf->hw;
+
+ snprintf(ctx->buf, sizeof(ctx->buf), "%u", hw->cgu.fw_ver);
+ }
+}
+
+static void ice_info_cgu_fw_build(struct ice_pf *pf, struct ice_info_ctx *ctx)
+{
+ if (ice_is_feature_supported(pf, ICE_F_CGU)) {
+ struct ice_hw *hw = &pf->hw;
+
+ snprintf(ctx->buf, sizeof(ctx->buf), "0x%x", hw->cgu.cfg_ver);
+ }
+}
+
#define fixed(key, getter) { ICE_VERSION_FIXED, key, getter, NULL }
#define running(key, getter) { ICE_VERSION_RUNNING, key, getter, NULL }
#define stored(key, getter, fallback) { ICE_VERSION_STORED, key, getter, fallback }
@@ -224,6 +251,7 @@ static const struct ice_devlink_version {
void (*fallback)(struct ice_pf *pf, struct ice_info_ctx *ctx);
} ice_devlink_versions[] = {
fixed(DEVLINK_INFO_VERSION_GENERIC_BOARD_ID, ice_info_pba),
+ fixed("cgu.id", ice_info_cgu_id),
running(DEVLINK_INFO_VERSION_GENERIC_FW_MGMT, ice_info_fw_mgmt),
running("fw.mgmt.api", ice_info_fw_api),
running("fw.mgmt.build", ice_info_fw_build),
@@ -235,6 +263,8 @@ static const struct ice_devlink_version {
running("fw.app.bundle_id", ice_info_ddp_pkg_bundle_id),
combined("fw.netlist", ice_info_netlist_ver, ice_info_pending_netlist_ver),
combined("fw.netlist.build", ice_info_netlist_build, ice_info_pending_netlist_build),
+ running("fw.cgu", ice_info_cgu_fw_version),
+ running("fw.cgu.build", ice_info_cgu_fw_build),
};
/**
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 6b28b95a7254..a3adc03bdd0a 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -4822,8 +4822,11 @@ static void ice_init_features(struct ice_pf *pf)
ice_gnss_init(pf);
if (ice_is_feature_supported(pf, ICE_F_CGU) ||
- ice_is_feature_supported(pf, ICE_F_PHY_RCLK))
+ ice_is_feature_supported(pf, ICE_F_PHY_RCLK)) {
+ ice_aq_get_cgu_info(&pf->hw, &pf->hw.cgu.id,
+ &pf->hw.cgu.cfg_ver, &pf->hw.cgu.fw_ver);
ice_dpll_init(pf);
+ }
/* Note: Flow director init failure is non-fatal to load */
if (ice_init_fdir(pf))
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
index 39b692945f73..90c1cc1e4401 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
@@ -3481,13 +3481,13 @@ bool ice_is_cgu_present(struct ice_hw *hw)
if (!ice_find_netlist_node(hw, ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL,
ICE_ACQ_GET_LINK_TOPO_NODE_NR_ZL30632_80032,
NULL)) {
- hw->cgu_part_number = ICE_ACQ_GET_LINK_TOPO_NODE_NR_ZL30632_80032;
+ hw->cgu.part_number = ICE_ACQ_GET_LINK_TOPO_NODE_NR_ZL30632_80032;
return true;
} else if (!ice_find_netlist_node(hw,
ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL,
ICE_ACQ_GET_LINK_TOPO_NODE_NR_SI5383_5384,
NULL)) {
- hw->cgu_part_number = ICE_ACQ_GET_LINK_TOPO_NODE_NR_SI5383_5384;
+ hw->cgu.part_number = ICE_ACQ_GET_LINK_TOPO_NODE_NR_SI5383_5384;
return true;
}
@@ -3507,7 +3507,7 @@ ice_cgu_get_pin_desc_e823(struct ice_hw *hw, bool input, int *size)
{
static const struct ice_cgu_pin_desc *t;
- if (hw->cgu_part_number ==
+ if (hw->cgu.part_number ==
ICE_ACQ_GET_LINK_TOPO_NODE_NR_ZL30632_80032) {
if (input) {
t = ice_e823_zl_cgu_inputs;
@@ -3516,7 +3516,7 @@ ice_cgu_get_pin_desc_e823(struct ice_hw *hw, bool input, int *size)
t = ice_e823_zl_cgu_outputs;
*size = ARRAY_SIZE(ice_e823_zl_cgu_outputs);
}
- } else if (hw->cgu_part_number ==
+ } else if (hw->cgu.part_number ==
ICE_ACQ_GET_LINK_TOPO_NODE_NR_SI5383_5384) {
if (input) {
t = ice_e823_si_cgu_inputs;
@@ -3778,10 +3778,10 @@ int ice_get_cgu_rclk_pin_info(struct ice_hw *hw, u8 *base_idx, u8 *pin_num)
case ICE_DEV_ID_E823C_SGMII:
*pin_num = ICE_E822_RCLK_PINS_NUM;
ret = 0;
- if (hw->cgu_part_number ==
+ if (hw->cgu.part_number ==
ICE_ACQ_GET_LINK_TOPO_NODE_NR_ZL30632_80032)
*base_idx = ZL_REF1P;
- else if (hw->cgu_part_number ==
+ else if (hw->cgu.part_number ==
ICE_ACQ_GET_LINK_TOPO_NODE_NR_SI5383_5384)
*base_idx = SI_REF1P;
else
diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
index 128bc4d326f9..814166d959ee 100644
--- a/drivers/net/ethernet/intel/ice/ice_type.h
+++ b/drivers/net/ethernet/intel/ice/ice_type.h
@@ -820,6 +820,13 @@ struct ice_mbx_data {
u16 async_watermark_val;
};
+struct ice_cgu_info {
+ u32 id;
+ u32 cfg_ver;
+ u32 fw_ver;
+ u8 part_number;
+};
+
/* Port hardware description */
struct ice_hw {
u8 __iomem *hw_addr;
@@ -963,7 +970,7 @@ struct ice_hw {
DECLARE_BITMAP(hw_ptype, ICE_FLOW_PTYPE_MAX);
u8 dvm_ena;
u16 io_expander_handle;
- u8 cgu_part_number;
+ struct ice_cgu_info cgu;
};
/* Statistics collected by each port, VSI, VEB, and S-channel */
--
2.31.1
On Wed, 12 Apr 2023 15:38:11 +0200 Arkadiusz Kubalewski wrote:
> If Clock Generation Unit and dplls are present on NIC board user shall
> know its details.
> Provide the devlink info callback with a new:
> - fixed type object `cgu.id` - hardware variant of onboard CGU
> - running type object `fw.cgu` - CGU firmware version
> - running type object `fw.cgu.build` - CGU configuration build version
>
> These information shall be known for debugging purposes.
>
> Test (on NIC board with CGU)
> $ devlink dev info <bus_name>/<dev_name> | grep cgu
> cgu.id 8032
> fw.cgu 6021
> fw.cgu.build 0x1030001
>
> Test (on NIC board without CGU)
> $ devlink dev info <bus_name>/<dev_name> | grep cgu -c
> 0
>
> Signed-off-by: Arkadiusz Kubalewski <[email protected]>
Is it flashed together with the rest of the FW components of the NIC?
Or the update method is different?
On Wed, Apr 12, 2023 at 03:38:11PM +0200, Arkadiusz Kubalewski wrote:
> If Clock Generation Unit and dplls are present on NIC board user shall
> know its details.
> Provide the devlink info callback with a new:
> - fixed type object `cgu.id` - hardware variant of onboard CGU
> - running type object `fw.cgu` - CGU firmware version
> - running type object `fw.cgu.build` - CGU configuration build version
>
> These information shall be known for debugging purposes.
>
> Test (on NIC board with CGU)
> $ devlink dev info <bus_name>/<dev_name> | grep cgu
> cgu.id 8032
> fw.cgu 6021
> fw.cgu.build 0x1030001
>
> Test (on NIC board without CGU)
> $ devlink dev info <bus_name>/<dev_name> | grep cgu -c
> 0
>
> Signed-off-by: Arkadiusz Kubalewski <[email protected]>
> ---
> Documentation/networking/devlink/ice.rst | 14 +++++++++
> drivers/net/ethernet/intel/ice/ice_devlink.c | 30 ++++++++++++++++++++
> drivers/net/ethernet/intel/ice/ice_main.c | 5 +++-
> drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 12 ++++----
> drivers/net/ethernet/intel/ice/ice_type.h | 9 +++++-
> 5 files changed, 62 insertions(+), 8 deletions(-)
<...>
> Flash Update
> ============
> diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
> index bc44cc220818..06fe895739af 100644
> --- a/drivers/net/ethernet/intel/ice/ice_devlink.c
> +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
> @@ -193,6 +193,33 @@ ice_info_pending_netlist_build(struct ice_pf __always_unused *pf,
> snprintf(ctx->buf, sizeof(ctx->buf), "0x%08x", netlist->hash);
> }
>
> +static void ice_info_cgu_id(struct ice_pf *pf, struct ice_info_ctx *ctx)
> +{
> + if (ice_is_feature_supported(pf, ICE_F_CGU)) {
> + struct ice_hw *hw = &pf->hw;
> +
> + snprintf(ctx->buf, sizeof(ctx->buf), "%u", hw->cgu.id);
> + }
Please use kernel coding style - success oriented flow
struct ice_hw *hw = &pf->hw;
if (!ice_is_feature_supported(pf, ICE_F_CGU))
return;
snprintf(ctx->buf, sizeof(ctx->buf), "%u", hw->cgu.id);
However, it will be nice to have these callbacks only if CGU is
supported, in such way you won't need any of ice_is_feature_supported()
checks.
Thanks
>From: Jakub Kicinski <[email protected]>
>Sent: Thursday, April 13, 2023 5:35 AM
>
>On Wed, 12 Apr 2023 15:38:11 +0200 Arkadiusz Kubalewski wrote:
>> If Clock Generation Unit and dplls are present on NIC board user shall
>> know its details.
>> Provide the devlink info callback with a new:
>> - fixed type object `cgu.id` - hardware variant of onboard CGU
>> - running type object `fw.cgu` - CGU firmware version
>> - running type object `fw.cgu.build` - CGU configuration build version
>>
>> These information shall be known for debugging purposes.
>>
>> Test (on NIC board with CGU)
>> $ devlink dev info <bus_name>/<dev_name> | grep cgu
>> cgu.id 8032
>> fw.cgu 6021
>> fw.cgu.build 0x1030001
>>
>> Test (on NIC board without CGU)
>> $ devlink dev info <bus_name>/<dev_name> | grep cgu -c
>> 0
>>
>> Signed-off-by: Arkadiusz Kubalewski <[email protected]>
>
>Is it flashed together with the rest of the FW components of the NIC?
>Or the update method is different?
Right now there is no mechanics for CGU firmware update at all, this is why I
mention that this is for now mostly for debugging purposes.
There are already some works ongoing to have CGU FW update possible, first with
Intel's nvmupdate packages and tools. But, for Linux we probably also gonna
need to support update through devlink, at least this seems right thing to do,
as there is already possibility to update NIC firmware with devlink.
Thank you!
Arkadiusz
>From: Leon Romanovsky <[email protected]>
>Sent: Thursday, April 13, 2023 3:17 PM
>
>On Wed, Apr 12, 2023 at 03:38:11PM +0200, Arkadiusz Kubalewski wrote:
>> If Clock Generation Unit and dplls are present on NIC board user shall
>> know its details.
>> Provide the devlink info callback with a new:
>> - fixed type object `cgu.id` - hardware variant of onboard CGU
>> - running type object `fw.cgu` - CGU firmware version
>> - running type object `fw.cgu.build` - CGU configuration build version
>>
>> These information shall be known for debugging purposes.
>>
>> Test (on NIC board with CGU)
>> $ devlink dev info <bus_name>/<dev_name> | grep cgu
>> cgu.id 8032
>> fw.cgu 6021
>> fw.cgu.build 0x1030001
>>
>> Test (on NIC board without CGU)
>> $ devlink dev info <bus_name>/<dev_name> | grep cgu -c
>> 0
>>
>> Signed-off-by: Arkadiusz Kubalewski <[email protected]>
>> ---
>> Documentation/networking/devlink/ice.rst | 14 +++++++++
>> drivers/net/ethernet/intel/ice/ice_devlink.c | 30 ++++++++++++++++++++
>> drivers/net/ethernet/intel/ice/ice_main.c | 5 +++-
>> drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 12 ++++----
>> drivers/net/ethernet/intel/ice/ice_type.h | 9 +++++-
>> 5 files changed, 62 insertions(+), 8 deletions(-)
>
><...>
>
>> Flash Update
>> ============
>> diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c
>b/drivers/net/ethernet/intel/ice/ice_devlink.c
>> index bc44cc220818..06fe895739af 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_devlink.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
>> @@ -193,6 +193,33 @@ ice_info_pending_netlist_build(struct ice_pf
>>__always_unused *pf,
>> snprintf(ctx->buf, sizeof(ctx->buf), "0x%08x", netlist->hash);
>> }
>>
>> +static void ice_info_cgu_id(struct ice_pf *pf, struct ice_info_ctx *ctx)
>> +{
>> + if (ice_is_feature_supported(pf, ICE_F_CGU)) {
>> + struct ice_hw *hw = &pf->hw;
>> +
>> + snprintf(ctx->buf, sizeof(ctx->buf), "%u", hw->cgu.id);
>> + }
>
>Please use kernel coding style - success oriented flow
>
>struct ice_hw *hw = &pf->hw;
>
>if (!ice_is_feature_supported(pf, ICE_F_CGU))
> return;
>
>snprintf(ctx->buf, sizeof(ctx->buf), "%u", hw->cgu.id);
>
>
>However, it will be nice to have these callbacks only if CGU is
>supported, in such way you won't need any of ice_is_feature_supported()
>checks.
>
>Thanks
Sure, I will fix as suggested in the next version.
Although most important is to achieve common understanding and agreement if
This way is the right one. Maybe those devlink id's shall be defined as a
part of "include/net/devlink.h", so other vendors could use it?
Also in such case probably naming might need to be unified.
Thank you!
Arkadiusz
On Thu, 13 Apr 2023 13:43:52 +0000 Kubalewski, Arkadiusz wrote:
> >Is it flashed together with the rest of the FW components of the NIC?
> >Or the update method is different?
>
> Right now there is no mechanics for CGU firmware update at all, this is why I
> mention that this is for now mostly for debugging purposes.
> There are already some works ongoing to have CGU FW update possible, first with
> Intel's nvmupdate packages and tools. But, for Linux we probably also gonna
> need to support update through devlink, at least this seems right thing to do,
> as there is already possibility to update NIC firmware with devlink.
Only FW versions which are updated with a single flashing operation /
are part of a single FW image should be reported under the device
instance. If the flash is separate you need to create a separate
devlink (sub)instance or something along those lines.
Differently put - the components in the API are components of the FW
image, not components of a board.
We had a similar discussion about "line cards" before.
On Thu, Apr 13, 2023 at 02:04:33PM +0000, Kubalewski, Arkadiusz wrote:
> >From: Leon Romanovsky <[email protected]>
> >Sent: Thursday, April 13, 2023 3:17 PM
> >
> >On Wed, Apr 12, 2023 at 03:38:11PM +0200, Arkadiusz Kubalewski wrote:
> >> If Clock Generation Unit and dplls are present on NIC board user shall
> >> know its details.
> >> Provide the devlink info callback with a new:
> >> - fixed type object `cgu.id` - hardware variant of onboard CGU
> >> - running type object `fw.cgu` - CGU firmware version
> >> - running type object `fw.cgu.build` - CGU configuration build version
> >>
> >> These information shall be known for debugging purposes.
> >>
> >> Test (on NIC board with CGU)
> >> $ devlink dev info <bus_name>/<dev_name> | grep cgu
> >> cgu.id 8032
> >> fw.cgu 6021
> >> fw.cgu.build 0x1030001
> >>
> >> Test (on NIC board without CGU)
> >> $ devlink dev info <bus_name>/<dev_name> | grep cgu -c
> >> 0
> >>
> >> Signed-off-by: Arkadiusz Kubalewski <[email protected]>
> >> ---
> >> Documentation/networking/devlink/ice.rst | 14 +++++++++
> >> drivers/net/ethernet/intel/ice/ice_devlink.c | 30 ++++++++++++++++++++
> >> drivers/net/ethernet/intel/ice/ice_main.c | 5 +++-
> >> drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 12 ++++----
> >> drivers/net/ethernet/intel/ice/ice_type.h | 9 +++++-
> >> 5 files changed, 62 insertions(+), 8 deletions(-)
> >
> ><...>
> >
> >> Flash Update
> >> ============
> >> diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c
> >b/drivers/net/ethernet/intel/ice/ice_devlink.c
> >> index bc44cc220818..06fe895739af 100644
> >> --- a/drivers/net/ethernet/intel/ice/ice_devlink.c
> >> +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
> >> @@ -193,6 +193,33 @@ ice_info_pending_netlist_build(struct ice_pf
> >>__always_unused *pf,
> >> snprintf(ctx->buf, sizeof(ctx->buf), "0x%08x", netlist->hash);
> >> }
> >>
> >> +static void ice_info_cgu_id(struct ice_pf *pf, struct ice_info_ctx *ctx)
> >> +{
> >> + if (ice_is_feature_supported(pf, ICE_F_CGU)) {
> >> + struct ice_hw *hw = &pf->hw;
> >> +
> >> + snprintf(ctx->buf, sizeof(ctx->buf), "%u", hw->cgu.id);
> >> + }
> >
> >Please use kernel coding style - success oriented flow
> >
> >struct ice_hw *hw = &pf->hw;
> >
> >if (!ice_is_feature_supported(pf, ICE_F_CGU))
> > return;
> >
> >snprintf(ctx->buf, sizeof(ctx->buf), "%u", hw->cgu.id);
> >
> >
> >However, it will be nice to have these callbacks only if CGU is
> >supported, in such way you won't need any of ice_is_feature_supported()
> >checks.
> >
> >Thanks
>
> Sure, I will fix as suggested in the next version.
> Although most important is to achieve common understanding and agreement if
> This way is the right one. Maybe those devlink id's shall be defined as a
> part of "include/net/devlink.h", so other vendors could use it?
Once second vendor materialize, it will be his responsibility to move
common code to devlink.h.
> Also in such case probably naming might need to be unified.
>
> Thank you!
> Arkadiusz
>From: Leon Romanovsky <[email protected]>
>Sent: Thursday, April 13, 2023 7:17 PM
>
>On Thu, Apr 13, 2023 at 02:04:33PM +0000, Kubalewski, Arkadiusz wrote:
>> >From: Leon Romanovsky <[email protected]>
>> >Sent: Thursday, April 13, 2023 3:17 PM
>> >
>> >On Wed, Apr 12, 2023 at 03:38:11PM +0200, Arkadiusz Kubalewski wrote:
>> >> If Clock Generation Unit and dplls are present on NIC board user shall
>> >> know its details.
>> >> Provide the devlink info callback with a new:
>> >> - fixed type object `cgu.id` - hardware variant of onboard CGU
>> >> - running type object `fw.cgu` - CGU firmware version
>> >> - running type object `fw.cgu.build` - CGU configuration build version
>> >>
>> >> These information shall be known for debugging purposes.
>> >>
>> >> Test (on NIC board with CGU)
>> >> $ devlink dev info <bus_name>/<dev_name> | grep cgu
>> >> cgu.id 8032
>> >> fw.cgu 6021
>> >> fw.cgu.build 0x1030001
>> >>
>> >> Test (on NIC board without CGU)
>> >> $ devlink dev info <bus_name>/<dev_name> | grep cgu -c
>> >> 0
>> >>
>> >> Signed-off-by: Arkadiusz Kubalewski <[email protected]>
>> >> ---
>> >> Documentation/networking/devlink/ice.rst | 14 +++++++++
>> >> drivers/net/ethernet/intel/ice/ice_devlink.c | 30
>> >>++++++++++++++++++++
>> >> drivers/net/ethernet/intel/ice/ice_main.c | 5 +++-
>> >> drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 12 ++++----
>> >> drivers/net/ethernet/intel/ice/ice_type.h | 9 +++++-
>> >> 5 files changed, 62 insertions(+), 8 deletions(-)
>> >
>> ><...>
>> >
>> >> Flash Update
>> >> ============
>> >> diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c
>> >b/drivers/net/ethernet/intel/ice/ice_devlink.c
>> >> index bc44cc220818..06fe895739af 100644
>> >> --- a/drivers/net/ethernet/intel/ice/ice_devlink.c
>> >> +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
>> >> @@ -193,6 +193,33 @@ ice_info_pending_netlist_build(struct ice_pf
>> >>__always_unused *pf,
>> >> snprintf(ctx->buf, sizeof(ctx->buf), "0x%08x", netlist-hash);
>> >> }
>> >>
>> >> +static void ice_info_cgu_id(struct ice_pf *pf, struct ice_info_ctx *ctx)
>> >> +{
>> >> + if (ice_is_feature_supported(pf, ICE_F_CGU)) {
>> >> + struct ice_hw *hw = &pf->hw;
>> >> +
>> >> + snprintf(ctx->buf, sizeof(ctx->buf), "%u", hw->cgu.id);
>> >> + }
>> >
>> >Please use kernel coding style - success oriented flow
>> >
>> >struct ice_hw *hw = &pf->hw;
>> >
>> >if (!ice_is_feature_supported(pf, ICE_F_CGU))
>> > return;
>> >
>> >snprintf(ctx->buf, sizeof(ctx->buf), "%u", hw->cgu.id);
>> >
>> >
>> >However, it will be nice to have these callbacks only if CGU is
>> >supported, in such way you won't need any of ice_is_feature_supported()
>> >checks.
>> >
>> >Thanks
>>
>> Sure, I will fix as suggested in the next version.
>> Although most important is to achieve common understanding and agreement if
>> This way is the right one. Maybe those devlink id's shall be defined as a
>> part of "include/net/devlink.h", so other vendors could use it?
>
>Once second vendor materialize, it will be his responsibility to move
>common code to devlink.h.
>
Makes sense, thanks for this explanation!
Arkadiusz
>> Also in such case probably naming might need to be unified.
>>
>> Thank you!
>> Arkadiusz
>From: Jakub Kicinski <[email protected]>
>Sent: Thursday, April 13, 2023 5:04 PM
>
>On Thu, 13 Apr 2023 13:43:52 +0000 Kubalewski, Arkadiusz wrote:
>> >Is it flashed together with the rest of the FW components of the NIC?
>> >Or the update method is different?
>>
>> Right now there is no mechanics for CGU firmware update at all, this is why I
>> mention that this is for now mostly for debugging purposes.
>> There are already some works ongoing to have CGU FW update possible, first
>>with
>> Intel's nvmupdate packages and tools. But, for Linux we probably also gonna
>> need to support update through devlink, at least this seems right thing to
>>do,
>> as there is already possibility to update NIC firmware with devlink.
>
>Only FW versions which are updated with a single flashing operation /
>are part of a single FW image should be reported under the device
>instance. If the flash is separate you need to create a separate
>devlink (sub)instance or something along those lines.
>
>Differently put - the components in the API are components of the FW
>image, not components of a board.
>
>We had a similar discussion about "line cards" before.
Well, this makes sense.
Although I double checked, and it seems I wasn't clear on previous explanation.
Once FW update is possible with Intel's nvmupdate tools, the devlink FW update
also going to update CGU firmware (part of nvm-flash region), so after all this
seems a right place for this info.
Thanks,
Arkadiusz
On Fri, 14 Apr 2023 10:04:05 +0000 Kubalewski, Arkadiusz wrote:
> Although I double checked, and it seems I wasn't clear on previous explanation.
> Once FW update is possible with Intel's nvmupdate tools, the devlink FW update
> also going to update CGU firmware (part of nvm-flash region), so after all this
> seems a right place for this info.
????️