RE: [PATCH v1 platform-next 0/2] platform/x86: Mellanox: add new features

[Vadim Pasternak <vadimp@xxxxxxxxxxxx>]

> -----Original Message-----
> From: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
> Sent: Monday, May 06, 2019 12:58 PM
> To: Vadim Pasternak <vadimp@xxxxxxxxxxxx>
> Cc: Darren Hart <dvhart@xxxxxxxxxxxxx>; Platform Driver <platform-driver-
> x86@xxxxxxxxxxxxxxx>; michealsh@xxxxxxxxxxxx
> Subject: Re: [PATCH v1 platform-next 0/2] platform/x86: Mellanox: add new
> features
> 
> On Mon, May 6, 2019 at 12:50 PM Vadim Pasternak <vadimp@xxxxxxxxxxxx>
> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
> > > Sent: Monday, May 06, 2019 12:36 PM
> > > To: Vadim Pasternak <vadimp@xxxxxxxxxxxx>
> > > Cc: Darren Hart <dvhart@xxxxxxxxxxxxx>; Platform Driver
> > > <platform-driver- x86@xxxxxxxxxxxxxxx>; michealsh@xxxxxxxxxxxx
> > > Subject: Re: [PATCH v1 platform-next 0/2] platform/x86: Mellanox:
> > > add new features
> > >
> > > On Mon, Mar 18, 2019 at 12:58 PM Vadim Pasternak
> > > <vadimp@xxxxxxxxxxxx>
> > > wrote:
> > > >
> > > > This patchset:
> > > > - Adds support for tachometer speed capability register.
> > > > - Adds support for Mellanox watchdog driver activation.
> > > >
> > >
> > > Pushed to my review and testing queue, thanks!
> > >
> > > I have added one patch on top, please, check if it's okay with you.
> >
> > Hi Andy,
> >
> > Thanks.
> >
> > You mean on top of patch
> > "platform/mellanox: Add TmFifo driver for Mellanox BlueField Soc" ?
> >
> 
> I meant that I would like to amend your patch, see this fixup:
> http://git.infradead.org/linux-platform-drivers-
> x86.git/commit/771fb643f668527985addad2e40b4dc17bac9170
> 

Andy,

I am not sure about this:
        for (i = MLXPLAT_CPLD_WD_MAX_DEVS - 1; i >= 0 ; i--) {
-               if (mlxplat_wd_data[i])
-                       platform_device_unregister(priv->pdev_wd[i]);
-       }
+               platform_device_unregister(priv->pdev_wd[i]);

For some systems we have only one watchdog instance:
mlxplat_wd_data[0] = &mlxplat_mlxcpld_wd_set_type1[0];
while for others two instances
	for (i = 0; i < ARRAY_SIZE(mlxplat_mlxcpld_wd_set_type2); i++)
		mlxplat_wd_data[i] = &mlxplat_mlxcpld_wd_set_type2[i];

So, in the first case we will have NULL for
platform_device_unregister(priv->pdev_wd[1]);

> > If yes, it's OK with me.
> >
> > Thanks,
> > Vadim
> >
> > >
> > > > Vadim Pasternak (2):
> > > >   platform/x86: mlx-platform: Add support for tachometer speed register
> > > >   platform/x86: mlx-platform: Add mlx-wdt platform driver
> > > > activation
> > > >
> > > >  drivers/platform/x86/mlx-platform.c | 232
> > > +++++++++++++++++++++++++++++++++++-
> > > >  1 file changed, 230 insertions(+), 2 deletions(-)
> > > >
> > > > --
> > > > 2.11.0
> > > >
> > >
> > >
> > > --
> > > With Best Regards,
> > > Andy Shevchenko
> 
> 
> 
> --
> With Best Regards,
> Andy Shevchenko