RE: [PATCH platform-next v2 07/10] platform: mellanox: Introduce support of Nvidia smart switch

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 16670 bytes --]

On Mon, 13 Jan 2025, Vadim Pasternak wrote:
> > -----Original Message-----
> > From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > Sent: Monday, 13 January 2025 19:12
> > To: Vadim Pasternak <vadimp@nvidia.com>
> > Cc: Hans de Goede <hdegoede@redhat.com>; Michael Shych
> > <michaelsh@nvidia.com>; Ciju Rajan K <crajank@nvidia.com>; Felix Radensky
> > <fradensky@nvidia.com>; Oleksandr Shamray <oleksandrs@nvidia.com>;
> > platform-driver-x86@vger.kernel.org
> > Subject: Re: [PATCH platform-next v2 07/10] platform: mellanox: Introduce
> > support of Nvidia smart switch
> > 
> > On Mon, 13 Jan 2025, Vadim Pasternak wrote:
> > 
> > > Provide platform support for Nvidia Smart Switch SN4280.
> > >
> > > The Smart Switch equipped with:
> > > - Nvidia COME module based on AMD EPYC™ Embedded 3451 CPU.
> > > - Nvidia Spectrum-3 ASIC.
> > > - Four DPUs, each equipped with Nvidia BF3 ARM based processor and
> > >   with Lattice LFD2NX-40 FPGA device.
> > > - 28xQSFP-DD external ports.
> > > - Two power supplies.
> > > - Four cooling drawers.
> > >
> > > Introduce configuration structures for the new systems to allow proper
> > > activation of the required platform drivers.
> > >
> > > Reviewed-by: Ciju Rajan K <crajank@nvidia.com>
> > > Signed-off-by: Vadim Pasternak <vadimp@nvidia.com>
> > > ---
> > >  drivers/platform/mellanox/mlx-platform.c | 1965 +++++++++++++++++++-
> > --
> > >  1 file changed, 1711 insertions(+), 254 deletions(-)
> > >
> > > diff --git a/drivers/platform/mellanox/mlx-platform.c
> > b/drivers/platform/mellanox/mlx-platform.c
> > > index bd3bb06ff8f2..9d237852d3e0 100644
> > > --- a/drivers/platform/mellanox/mlx-platform.c
> > > +++ b/drivers/platform/mellanox/mlx-platform.c
> > > @@ -38,6 +38,7 @@
> > >  #define MLXPLAT_CPLD_LPC_REG_CPLD4_PN1_OFFSET	0x0b
> > >  #define MLXPLAT_CPLD_LPC_REG_RESET_GP1_OFFSET	0x17
> > >  #define MLXPLAT_CPLD_LPC_REG_RESET_GP2_OFFSET	0x19
> > > +#define MLXPLAT_CPLD_LPC_REG_RESET_GP3_OFFSET	0x1b
> > >  #define MLXPLAT_CPLD_LPC_REG_RESET_GP4_OFFSET	0x1c
> > >  #define MLXPLAT_CPLD_LPC_REG_RESET_CAUSE_OFFSET	0x1d
> > >  #define MLXPLAT_CPLD_LPC_REG_RST_CAUSE1_OFFSET	0x1e
> > > @@ -49,9 +50,11 @@
> > >  #define MLXPLAT_CPLD_LPC_REG_LED5_OFFSET	0x24
> > >  #define MLXPLAT_CPLD_LPC_REG_LED6_OFFSET	0x25
> > >  #define MLXPLAT_CPLD_LPC_REG_LED7_OFFSET	0x26
> > > +#define MLXPLAT_CPLD_LPC_REG_LED8_OFFSET	0x27
> > >  #define MLXPLAT_CPLD_LPC_REG_FAN_DIRECTION	0x2a
> > >  #define MLXPLAT_CPLD_LPC_REG_GP0_RO_OFFSET	0x2b
> > >  #define MLXPLAT_CPLD_LPC_REG_GPCOM0_OFFSET	0x2d
> > > +#define MLXPLAT_CPLD_LPC_REG_GP1_RO_OFFSET	0x2c
> > >  #define MLXPLAT_CPLD_LPC_REG_GP0_OFFSET		0x2e
> > >  #define MLXPLAT_CPLD_LPC_REG_GP_RST_OFFSET	0x2f
> > >  #define MLXPLAT_CPLD_LPC_REG_GP1_OFFSET		0x30
> > > @@ -71,12 +74,14 @@
> > >  #define MLXPLAT_CPLD_LPC_REG_AGGRCO_MASK_OFFSET	0x43
> > >  #define MLXPLAT_CPLD_LPC_REG_AGGRCX_OFFSET	0x44
> > >  #define MLXPLAT_CPLD_LPC_REG_AGGRCX_MASK_OFFSET 0x45
> > > +#define MLXPLAT_CPLD_LPC_REG_GP3_OFFSET		0x46
> > >  #define MLXPLAT_CPLD_LPC_REG_BRD_OFFSET		0x47
> > >  #define MLXPLAT_CPLD_LPC_REG_BRD_EVENT_OFFSET	0x48
> > >  #define MLXPLAT_CPLD_LPC_REG_BRD_MASK_OFFSET	0x49
> > >  #define MLXPLAT_CPLD_LPC_REG_GWP_OFFSET		0x4a
> > >  #define MLXPLAT_CPLD_LPC_REG_GWP_EVENT_OFFSET	0x4b
> > >  #define MLXPLAT_CPLD_LPC_REG_GWP_MASK_OFFSET	0x4c
> > > +#define MLXPLAT_CPLD_LPC_REG_GPI_MASK_OFFSET	0x4e
> > >  #define MLXPLAT_CPLD_LPC_REG_ASIC_HEALTH_OFFSET 0x50
> > >  #define MLXPLAT_CPLD_LPC_REG_ASIC_EVENT_OFFSET	0x51
> > >  #define MLXPLAT_CPLD_LPC_REG_ASIC_MASK_OFFSET	0x52
> > > @@ -88,15 +93,20 @@
> > >  #define MLXPLAT_CPLD_LPC_REG_PSU_OFFSET		0x58
> > >  #define MLXPLAT_CPLD_LPC_REG_PSU_EVENT_OFFSET	0x59
> > >  #define MLXPLAT_CPLD_LPC_REG_PSU_MASK_OFFSET	0x5a
> > > +#define MLXPLAT_CPLD_LPC_REG_PSU_AC_OFFSET	0x5e
> > >  #define MLXPLAT_CPLD_LPC_REG_PWR_OFFSET		0x64
> > >  #define MLXPLAT_CPLD_LPC_REG_PWR_EVENT_OFFSET	0x65
> > >  #define MLXPLAT_CPLD_LPC_REG_PWR_MASK_OFFSET	0x66
> > > +#define MLXPLAT_CPLD_LPC_REG_PSU_ALERT_OFFSET	0x6a
> > >  #define MLXPLAT_CPLD_LPC_REG_LC_IN_OFFSET	0x70
> > >  #define MLXPLAT_CPLD_LPC_REG_LC_IN_EVENT_OFFSET	0x71
> > >  #define MLXPLAT_CPLD_LPC_REG_LC_IN_MASK_OFFSET	0x72
> > >  #define MLXPLAT_CPLD_LPC_REG_FAN_OFFSET		0x88
> > >  #define MLXPLAT_CPLD_LPC_REG_FAN_EVENT_OFFSET	0x89
> > >  #define MLXPLAT_CPLD_LPC_REG_FAN_MASK_OFFSET	0x8a
> > > +#define MLXPLAT_CPLD_LPC_REG_FAN2_OFFSET	0x8b
> > > +#define MLXPLAT_CPLD_LPC_REG_FAN2_EVENT_OFFSET	0x8c
> > > +#define MLXPLAT_CPLD_LPC_REG_FAN2_MASK_OFFSET	0x8d
> > >  #define MLXPLAT_CPLD_LPC_REG_CPLD5_VER_OFFSET	0x8e
> > >  #define MLXPLAT_CPLD_LPC_REG_CPLD5_PN_OFFSET	0x8f
> > >  #define MLXPLAT_CPLD_LPC_REG_CPLD5_PN1_OFFSET	0x90
> > > @@ -128,10 +138,15 @@
> > >  #define MLXPLAT_CPLD_LPC_REG_LC_SD_EVENT_OFFSET	0xaa
> > >  #define MLXPLAT_CPLD_LPC_REG_LC_SD_MASK_OFFSET	0xab
> > >  #define MLXPLAT_CPLD_LPC_REG_LC_PWR_ON		0xb2
> > > +#define MLXPLAT_CPLD_LPC_REG_TACHO19_OFFSET	0xb4
> > > +#define MLXPLAT_CPLD_LPC_REG_TACHO20_OFFSET	0xb5
> > >  #define MLXPLAT_CPLD_LPC_REG_DBG1_OFFSET	0xb6
> > >  #define MLXPLAT_CPLD_LPC_REG_DBG2_OFFSET	0xb7
> > >  #define MLXPLAT_CPLD_LPC_REG_DBG3_OFFSET	0xb8
> > >  #define MLXPLAT_CPLD_LPC_REG_DBG4_OFFSET	0xb9
> > > +#define MLXPLAT_CPLD_LPC_REG_TACHO17_OFFSET	0xba
> > > +#define MLXPLAT_CPLD_LPC_REG_TACHO18_OFFSET	0xbb
> > > +#define MLXPLAT_CPLD_LPC_REG_ASIC_CAP_OFFSET	0xc1
> > >  #define MLXPLAT_CPLD_LPC_REG_GP4_RO_OFFSET	0xc2
> > >  #define MLXPLAT_CPLD_LPC_REG_SPI_CHNL_SELECT	0xc3
> > >  #define MLXPLAT_CPLD_LPC_REG_CPLD5_MVER_OFFSET	0xc4
> > > @@ -182,6 +197,9 @@
> > >  #define MLXPLAT_CPLD_LPC_REG_CONFIG1_OFFSET	0xfb
> > >  #define MLXPLAT_CPLD_LPC_REG_CONFIG2_OFFSET	0xfc
> > >  #define MLXPLAT_CPLD_LPC_REG_CONFIG3_OFFSET	0xfd
> > > +#define MLXPLAT_CPLD_LPC_REG_TACHO15_OFFSET	0xfe
> > > +#define MLXPLAT_CPLD_LPC_REG_TACHO16_OFFSET	0xff
> > > +
> > >  #define MLXPLAT_CPLD_LPC_IO_RANGE		0x100
> > >
> > >  #define MLXPLAT_CPLD_LPC_PIO_OFFSET		0x10000UL
> > > @@ -210,9 +228,15 @@
> > >  #define MLXPLAT_CPLD_AGGR_MASK_NG_DEF	0x04
> > >  #define MLXPLAT_CPLD_AGGR_MASK_COMEX	BIT(0)
> > >  #define MLXPLAT_CPLD_AGGR_MASK_LC	BIT(3)
> > > +#define MLXPLAT_CPLD_AGGR_MASK_DPU_BRD	BIT(4)
> > > +#define MLXPLAT_CPLD_AGGR_MASK_DPU_CORE	BIT(5)
> > >  #define MLXPLAT_CPLD_AGGR_MASK_MODULAR
> > 	(MLXPLAT_CPLD_AGGR_MASK_NG_DEF | \
> > >
> > MLXPLAT_CPLD_AGGR_MASK_COMEX | \
> > >  					 MLXPLAT_CPLD_AGGR_MASK_LC)
> > > +#define MLXPLAT_CPLD_AGGR_MASK_SMART_SW
> > 	(MLXPLAT_CPLD_AGGR_MASK_COMEX | \
> > > +
> > MLXPLAT_CPLD_AGGR_MASK_NG_DEF | \
> > > +
> > MLXPLAT_CPLD_AGGR_MASK_DPU_BRD | \
> > > +
> > MLXPLAT_CPLD_AGGR_MASK_DPU_CORE)
> > >  #define MLXPLAT_CPLD_AGGR_MASK_LC_PRSNT	BIT(0)
> > >  #define MLXPLAT_CPLD_AGGR_MASK_LC_RDY	BIT(1)
> > >  #define MLXPLAT_CPLD_AGGR_MASK_LC_PG	BIT(2)
> > > @@ -235,15 +259,24 @@
> > >  #define MLXPLAT_CPLD_PWR_MASK		GENMASK(1, 0)
> > >  #define MLXPLAT_CPLD_PSU_EXT_MASK	GENMASK(3, 0)
> > >  #define MLXPLAT_CPLD_PWR_EXT_MASK	GENMASK(3, 0)
> > > +#define MLXPLAT_CPLD_PSU_XDR_MASK	GENMASK(7, 0)
> > > +#define MLXPLAT_CPLD_PWR_XDR_MASK	GENMASK(7, 0)
> > >  #define MLXPLAT_CPLD_FAN_MASK		GENMASK(3, 0)
> > >  #define MLXPLAT_CPLD_ASIC_MASK		GENMASK(1, 0)
> > > +#define MLXPLAT_CPLD_ASIC_XDR_MASK	GENMASK(3, 0)
> > >  #define MLXPLAT_CPLD_FAN_NG_MASK	GENMASK(6, 0)
> > > +#define MLXPLAT_CPLD_FAN_XDR_MASK	GENMASK(7, 0)
> > >  #define MLXPLAT_CPLD_LED_LO_NIBBLE_MASK	GENMASK(7, 4)
> > >  #define MLXPLAT_CPLD_LED_HI_NIBBLE_MASK	GENMASK(3, 0)
> > >  #define MLXPLAT_CPLD_VOLTREG_UPD_MASK	GENMASK(5, 4)
> > >  #define MLXPLAT_CPLD_GWP_MASK		GENMASK(0, 0)
> > >  #define MLXPLAT_CPLD_EROT_MASK		GENMASK(1, 0)
> > >  #define MLXPLAT_CPLD_FU_CAP_MASK	GENMASK(1, 0)
> > > +#define MLXPLAT_CPLD_PSU_CAP_MASK	GENMASK(7, 0)
> > > +#define MLXPLAT_CPLD_FAN_CAP_MASK	GENMASK(7, 0)
> > > +#define MLXPLAT_CPLD_ASIC_CAP_MASK	GENMASK(7, 0)
> > > +#define MLXPLAT_CPLD_BIOS_STATUS_MASK	GENMASK(3, 1)
> > > +#define MLXPLAT_CPLD_DPU_MASK		GENMASK(3, 0)
> > >  #define MLXPLAT_CPLD_PWR_BUTTON_MASK	BIT(0)
> > >  #define MLXPLAT_CPLD_LATCH_RST_MASK	BIT(6)
> > >  #define MLXPLAT_CPLD_THERMAL1_PDB_MASK	BIT(3)
> > > @@ -267,6 +300,9 @@
> > >  /* Masks for aggregation for modular systems */
> > >  #define MLXPLAT_CPLD_LPC_LC_MASK	GENMASK(7, 0)
> > >
> > > +/* Masks for aggregation for smart switch systems */
> > > +#define MLXPLAT_CPLD_LPC_SM_SW_MASK	GENMASK(7, 0)
> > > +
> > >  #define MLXPLAT_CPLD_HALT_MASK		BIT(3)
> > >  #define MLXPLAT_CPLD_RESET_MASK		GENMASK(7, 1)
> > >
> > > @@ -297,15 +333,18 @@
> > >  #define MLXPLAT_CPLD_NR_NONE			-1
> > >  #define MLXPLAT_CPLD_PSU_DEFAULT_NR		10
> > >  #define MLXPLAT_CPLD_PSU_MSNXXXX_NR		4
> > > +#define MLXPLAT_CPLD_PSU_XDR_NR			3
> > >  #define MLXPLAT_CPLD_FAN1_DEFAULT_NR		11
> > >  #define MLXPLAT_CPLD_FAN2_DEFAULT_NR		12
> > >  #define MLXPLAT_CPLD_FAN3_DEFAULT_NR		13
> > >  #define MLXPLAT_CPLD_FAN4_DEFAULT_NR		14
> > >  #define MLXPLAT_CPLD_NR_ASIC			3
> > >  #define MLXPLAT_CPLD_NR_LC_BASE			34
> > > +#define MLXPLAT_CPLD_NR_DPU_BASE		18
> > >
> > >  #define MLXPLAT_CPLD_NR_LC_SET(nr)
> > 	(MLXPLAT_CPLD_NR_LC_BASE + (nr))
> > >  #define MLXPLAT_CPLD_LC_ADDR		0x32
> > > +#define MLXPLAT_CPLD_DPU_ADDR		0x68
> > >
> > >  /* Masks and default values for watchdogs */
> > >  #define MLXPLAT_CPLD_WD1_CLEAR_MASK	GENMASK(7, 1)
> > > @@ -320,6 +359,7 @@
> > >  #define MLXPLAT_CPLD_WD_DFLT_TIMEOUT	30
> > >  #define MLXPLAT_CPLD_WD3_DFLT_TIMEOUT	600
> > >  #define MLXPLAT_CPLD_WD_MAX_DEVS	2
> > > +#define MLXPLAT_CPLD_DPU_MAX_DEVS	4
> > >
> > >  #define MLXPLAT_CPLD_LPC_SYSIRQ		17
> > >
> > > @@ -346,6 +386,7 @@
> > >   * @pdev_io_regs - register access platform devices
> > >   * @pdev_fan - FAN platform devices
> > >   * @pdev_wd - array of watchdog platform devices
> > > + * pdev_dpu - array of Data Processor Unit platform devices
> > >   * @regmap: device register map
> > >   * @hotplug_resources: system hotplug resources
> > >   * @hotplug_resources_size: size of system hotplug resources
> > > @@ -360,6 +401,7 @@ struct mlxplat_priv {
> > >  	struct platform_device *pdev_io_regs;
> > >  	struct platform_device *pdev_fan;
> > >  	struct platform_device *pdev_wd[MLXPLAT_CPLD_WD_MAX_DEVS];
> > > +	struct platform_device
> > *pdev_dpu[MLXPLAT_CPLD_DPU_MAX_DEVS];
> > >  	void *regmap;
> > >  	struct resource *hotplug_resources;
> > >  	unsigned int hotplug_resources_size;
> > > @@ -626,6 +668,21 @@ static struct i2c_board_info
> > mlxplat_mlxcpld_pwr_ng800[] = {
> > >  	},
> > >  };
> > >
> > > +static struct i2c_board_info mlxplat_mlxcpld_xdr_pwr[] = {
> > > +	{
> > > +		I2C_BOARD_INFO("dps460", 0x5d),
> > > +	},
> > > +	{
> > > +		I2C_BOARD_INFO("dps460", 0x5c),
> > > +	},
> > > +	{
> > > +		I2C_BOARD_INFO("dps460", 0x5e),
> > > +	},
> > > +	{
> > > +		I2C_BOARD_INFO("dps460", 0x5f),
> > > +	},
> > > +};
> > > +
> > >  static struct i2c_board_info mlxplat_mlxcpld_fan[] = {
> > >  	{
> > >  		I2C_BOARD_INFO("24c32", 0x50),
> > > @@ -2370,205 +2427,665 @@ struct mlxreg_core_hotplug_platform_data
> > mlxplat_mlxcpld_rack_switch_data = {
> > >  	.mask_low = MLXPLAT_CPLD_LOW_AGGR_MASK_LOW,
> > >  };
> > >
> > > -/* Callback performs graceful shutdown after notification about power
> > button event */
> > > -static int
> > > -mlxplat_mlxcpld_l1_switch_pwr_events_handler(void *handle, enum
> > mlxreg_hotplug_kind kind,
> > > -					     u8 action)
> > > -{
> > > -	if (action) {
> > > -		dev_info(&mlxplat_dev->dev, "System shutdown due to short
> > press of power button");
> > > -		kernel_power_off();
> > > -	}
> > > -
> > > -	return 0;
> > > -}
> > > -
> > > -static struct mlxreg_core_hotplug_notifier
> > mlxplat_mlxcpld_l1_switch_pwr_events_notifier = {
> > > -	.user_handler = mlxplat_mlxcpld_l1_switch_pwr_events_handler,
> > > -};
> > > -
> > > -/* Platform hotplug for l1 switch systems family data */
> > > -static struct mlxreg_core_data
> > mlxplat_mlxcpld_l1_switch_pwr_events_items_data[] = {
> > > +/* Platform hotplug XDR and smart switch system family data */
> > > +static struct mlxreg_core_data mlxplat_mlxcpld_xdr_psu_items_data[] = {
> > >  	{
> > > -		.label = "power_button",
> > > -		.reg = MLXPLAT_CPLD_LPC_REG_PWRB_OFFSET,
> > > -		.mask = MLXPLAT_CPLD_PWR_BUTTON_MASK,
> > > +		.label = "psu1",
> > > +		.reg = MLXPLAT_CPLD_LPC_REG_PSU_OFFSET,
> > > +		.mask = BIT(0),
> > > +		.slot = 1,
> > > +		.capability = MLXPLAT_CPLD_LPC_REG_PSU_I2C_CAP_OFFSET,
> > > +		.capability_mask = MLXPLAT_CPLD_PSU_CAP_MASK,
> > >  		.hpdev.nr = MLXPLAT_CPLD_NR_NONE,
> > > -		.hpdev.action = MLXREG_HOTPLUG_DEVICE_NO_ACTION,
> > > -		.hpdev.notifier =
> > &mlxplat_mlxcpld_l1_switch_pwr_events_notifier,
> > >  	},
> > > -};
> > > -
> > > -/* Callback activates latch reset flow after notification about intrusion event
> > */
> > > -static int
> > > -mlxplat_mlxcpld_l1_switch_intrusion_events_handler(void *handle, enum
> > mlxreg_hotplug_kind kind,
> > > -						   u8 action)
> > > -{
> > > -	struct mlxplat_priv *priv = platform_get_drvdata(mlxplat_dev);
> > > -	u32 regval;
> > > -	int err;
> > > -
> > > -	err = regmap_read(priv->regmap,
> > MLXPLAT_CPLD_LPC_REG_GP1_OFFSET, &regval);
> > > -	if (err)
> > > -		goto fail_regmap_read;
> > > -
> > > -	if (action) {
> > > -		dev_info(&mlxplat_dev->dev, "Detected intrusion - system
> > latch is opened");
> > > -		err = regmap_write(priv->regmap,
> > MLXPLAT_CPLD_LPC_REG_GP1_OFFSET,
> > > -				   regval |
> > MLXPLAT_CPLD_LATCH_RST_MASK);
> > > -	} else {
> > > -		dev_info(&mlxplat_dev->dev, "System latch is properly
> > closed");
> > > -		err = regmap_write(priv->regmap,
> > MLXPLAT_CPLD_LPC_REG_GP1_OFFSET,
> > > -				   regval &
> > ~MLXPLAT_CPLD_LATCH_RST_MASK);
> > > -	}
> > > -
> > > -	if (err)
> > > -		goto fail_regmap_write;
> > > -
> > > -	return 0;
> > > -
> > > -fail_regmap_read:
> > > -fail_regmap_write:
> > > -	dev_err(&mlxplat_dev->dev, "Register access failed");
> > > -	return err;
> > > -}
> > > -
> > > -static struct mlxreg_core_hotplug_notifier
> > mlxplat_mlxcpld_l1_switch_intrusion_events_notifier = {
> > > -	.user_handler = mlxplat_mlxcpld_l1_switch_intrusion_events_handler,
> > > -};
> > > -
> > > -static struct mlxreg_core_data
> > mlxplat_mlxcpld_l1_switch_health_events_items_data[] = {
> > >  	{
> > > -		.label = "thermal1_pdb",
> > > -		.reg = MLXPLAT_CPLD_LPC_REG_BRD_OFFSET,
> > > -		.mask = MLXPLAT_CPLD_THERMAL1_PDB_MASK,
> > > +		.label = "psu2",
> > > +		.reg = MLXPLAT_CPLD_LPC_REG_PSU_OFFSET,
> > > +		.mask = BIT(1),
> > > +		.slot = 2,
> > > +		.capability = MLXPLAT_CPLD_LPC_REG_PSU_I2C_CAP_OFFSET,
> > > +		.capability_mask = MLXPLAT_CPLD_PSU_CAP_MASK,
> > >  		.hpdev.nr = MLXPLAT_CPLD_NR_NONE,
> > >  	},
> > >  	{
> > > -		.label = "thermal2_pdb",
> > > -		.reg = MLXPLAT_CPLD_LPC_REG_BRD_OFFSET,
> > > -		.mask = MLXPLAT_CPLD_THERMAL2_PDB_MASK,
> > > +		.label = "psu3",
> > > +		.reg = MLXPLAT_CPLD_LPC_REG_PSU_OFFSET,
> > > +		.mask = BIT(2),
> > > +		.slot = 3,
> > > +		.capability = MLXPLAT_CPLD_LPC_REG_PSU_I2C_CAP_OFFSET,
> > > +		.capability_mask = MLXPLAT_CPLD_PSU_CAP_MASK,
> > >  		.hpdev.nr = MLXPLAT_CPLD_NR_NONE,
> > >  	},
> > 
> > Why's the diff such a mess in this patch?
> > 
> > Are you perhaps doing two things in this patch, that is, reorganizing the
> > existing code/structs and adding new stuff? If that's the case, please try
> > to do the reorganization in another patch before this one so the diff
> > spaghetti is hopefully avoided.
> > 
> 
> I really don't understand why patch shows some lines as deleted.
> In patch only new lines where added, no one deleted.
> If I am doing just diff - it shows only new lines, but in patch it looks like
> some lines have been removed.

Okay, I'll play with various diff options then. Different diff strategies 
lock different lines.

> Probably some problem with ' git format-patch'.
> Have no idea how to handle it.

You could try with --minimal, --patience, or --histogram options which 
pick a different diff strategy, sometimes that might help (I can do that 
myself too after applying but if I would need to give review feedback a 
cleaner diff in email would be preferred :-)).

But if you cannot get it to avoid it generating the deleted lines, don't 
sweat it.

-- 
 i.