Re: [PATCH net-next 1/1] ice: add TTY for GNSS module for E810T device

Hi Dave, Jakub, Richard,

On Tue, 2022-02-15 at 11:18 -0800, Tony Nguyen wrote:

Hi Richard,

On Mon, 2022-02-14 at 16:18 -0800, Richard Cochran wrote:

quoted

On Mon, Feb 14, 2022 at 03:15:36PM -0800, Tony Nguyen wrote:

quoted

diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h

b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
index fd8ee5b7f596..a23a9ea10751 100644

--- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h

@@ -1401,6 +1401,24 @@ struct ice_aqc_get_link_topo {

        u8 rsvd[9];
 };
 
+/* Read I2C (direct, 0x06E2) */
+struct ice_aqc_i2c {
+       struct ice_aqc_link_topo_addr topo_addr;
+       __le16 i2c_addr;
+       u8 i2c_params;
+#define ICE_AQC_I2C_DATA_SIZE_S                0
+#define ICE_AQC_I2C_DATA_SIZE_M                (0xF <<
ICE_AQC_I2C_DATA_SIZE_S)
+#define ICE_AQC_I2C_USE_REPEATED_START BIT(7)

Nit:  #define belongs at top of file, or at least outside of
structure definition.

These are the bits that directly relate to the fields for the HW-SW
structures. It's more readable and easier to correlate by keeping
these
defines next to the fields they relate to.

I haven't heard anything back. Are we ok with this convention? Just to
add this usage is fairly standard for our driver structures especially
in this ice_adminq.h file.

Thanks,
Tony

Thanks,
Tony

quoted

quoted

quoted

+       u8 rsvd;
+       __le16 i2c_bus_addr;
+       u8 rsvd2[4];
+};

Thanks,
Richard