| Message ID | 20190318105823.2821-1-vadimp@mellanox.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | platform/x86: Mellanox: add new features | expand |
On Mon, Mar 18, 2019 at 12:58 PM Vadim Pasternak <vadimp@mellanox.com> 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. > 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 >
> -----Original Message----- > From: Andy Shevchenko <andy.shevchenko@gmail.com> > Sent: Monday, May 06, 2019 12:36 PM > To: Vadim Pasternak <vadimp@mellanox.com> > Cc: Darren Hart <dvhart@infradead.org>; Platform Driver <platform-driver- > x86@vger.kernel.org>; michealsh@mellanox.com > 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@mellanox.com> > 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" ? 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
On Mon, May 6, 2019 at 12:50 PM Vadim Pasternak <vadimp@mellanox.com> wrote: > > > > > -----Original Message----- > > From: Andy Shevchenko <andy.shevchenko@gmail.com> > > Sent: Monday, May 06, 2019 12:36 PM > > To: Vadim Pasternak <vadimp@mellanox.com> > > Cc: Darren Hart <dvhart@infradead.org>; Platform Driver <platform-driver- > > x86@vger.kernel.org>; michealsh@mellanox.com > > 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@mellanox.com> > > 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 > 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
> -----Original Message----- > From: Andy Shevchenko <andy.shevchenko@gmail.com> > Sent: Monday, May 06, 2019 12:58 PM > To: Vadim Pasternak <vadimp@mellanox.com> > Cc: Darren Hart <dvhart@infradead.org>; Platform Driver <platform-driver- > x86@vger.kernel.org>; michealsh@mellanox.com > 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@mellanox.com> > wrote: > > > > > > > > > -----Original Message----- > > > From: Andy Shevchenko <andy.shevchenko@gmail.com> > > > Sent: Monday, May 06, 2019 12:36 PM > > > To: Vadim Pasternak <vadimp@mellanox.com> > > > Cc: Darren Hart <dvhart@infradead.org>; Platform Driver > > > <platform-driver- x86@vger.kernel.org>; michealsh@mellanox.com > > > 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@mellanox.com> > > > 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
On Mon, May 6, 2019 at 1:12 PM Vadim Pasternak <vadimp@mellanox.com> wrote: > > > > I have added one patch on top, please, check if it's okay with you. > > http://git.infradead.org/linux-platform-drivers- > > x86.git/commit/771fb643f668527985addad2e40b4dc17bac9170 > 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]); The following commit adds an IS_ERR() check on top for long existing NULL check. The latter is what you are trying to do and effectively means double check for NULL. commit 99fef587ff98894426d9bf1f5b7336345052d4b3 Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Date: Mon Dec 3 20:21:41 2018 +0200 driver core: platform: Respect return code of platform_device_register_full()
> -----Original Message----- > From: Andy Shevchenko <andy.shevchenko@gmail.com> > Sent: Monday, May 06, 2019 3:07 PM > To: Vadim Pasternak <vadimp@mellanox.com> > Cc: Darren Hart <dvhart@infradead.org>; Platform Driver <platform-driver- > x86@vger.kernel.org>; michealsh@mellanox.com > Subject: Re: [PATCH v1 platform-next 0/2] platform/x86: Mellanox: add new > features > > On Mon, May 6, 2019 at 1:12 PM Vadim Pasternak <vadimp@mellanox.com> > wrote: > > > > > > I have added one patch on top, please, check if it's okay with you. > > > > http://git.infradead.org/linux-platform-drivers- > > > x86.git/commit/771fb643f668527985addad2e40b4dc17bac9170 > > > 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]); > > The following commit adds an IS_ERR() check on top for long existing NULL > check. > The latter is what you are trying to do and effectively means double check for > NULL. > > commit 99fef587ff98894426d9bf1f5b7336345052d4b3 > Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Date: Mon Dec 3 20:21:41 2018 +0200 > > driver core: platform: Respect return code of platform_device_register_full() O, yes, I see. So all is OK. > > -- > With Best Regards, > Andy Shevchenko
On Mon, May 6, 2019 at 3:12 PM Vadim Pasternak <vadimp@mellanox.com> wrote: > > On Mon, May 6, 2019 at 1:12 PM Vadim Pasternak <vadimp@mellanox.com> > > wrote: > > > > > > > > I have added one patch on top, please, check if it's okay with you. > > > > > > http://git.infradead.org/linux-platform-drivers- > > > > x86.git/commit/771fb643f668527985addad2e40b4dc17bac9170 > > > > > 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]); > > > > The following commit adds an IS_ERR() check on top for long existing NULL > > check. > > The latter is what you are trying to do and effectively means double check for > > NULL. > > > > commit 99fef587ff98894426d9bf1f5b7336345052d4b3 > > Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Date: Mon Dec 3 20:21:41 2018 +0200 > > > > driver core: platform: Respect return code of platform_device_register_full() > > O, yes, I see. > So all is OK. OK, I will fold it in.