query for hwmon: (mlxreg-fan) patch

query for hwmon: (mlxreg-fan) patch

[Vadim Pasternak]

Hi Guenter,

I would like to ask you about the way of sending the patch for
drivers/hwmon/mlxreg-fan.c.

This patch makes use of new "capability" register, but required commit
"platform_data/mlxreg: Add capability field to core platform data"
from platform for-next branch:
http://git.infradead.org/linux-platform-drivers-x86.git/blobdiff/9b28aa1d0eae1be1016c8f4ba504545caff01da3..946e4e02b11889cb161b15ff4712a8ba21a50eb6:/include/linux/platform_data/mlxreg.h

Is it possibly to send a patch with such dependency? Or I should wait until
"platform_data/mlxreg: Add capability field to core platform data" is got
to upstream?

Thanks,
Vadim.

Re: query for hwmon: (mlxreg-fan) patch

[Guenter Roeck]

On 2/18/19 12:07 PM, Vadim Pasternak wrote:
> Hi Guenter,
> 
> I would like to ask you about the way of sending the patch for
> drivers/hwmon/mlxreg-fan.c.
> 
> This patch makes use of new "capability" register, but required commit
> "platform_data/mlxreg: Add capability field to core platform data"
> from platform for-next branch:
> http://git.infradead.org/linux-platform-drivers-x86.git/blobdiff/9b28aa1d0eae1be1016c8f4ba504545caff01da3..946e4e02b11889cb161b15ff4712a8ba21a50eb6:/include/linux/platform_data/mlxreg.h
> 
> Is it possibly to send a patch with such dependency? Or I should wait until
> "platform_data/mlxreg: Add capability field to core platform data" is got
> to upstream?
> 

A single series submitted through one maintainer, with acks from all
the other maintainers, is always the easiest. Otherwise one maintainer
would have to create an immmutable branch with the essential patch(es)
needed by all the others. Or, yes, you could wait for the infrastructure
to be in place.

Makes me wonder, though: If you are open to doing that, why the complex,
confusing, and risky mlxreg_wdt_check_watchdog_type() in the watchdog
driver (instead of providing a means for the driver to get the HW version
directly from the parent) ?

Guenter

RE: query for hwmon: (mlxreg-fan) patch

[Vadim Pasternak]

> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Monday, February 18, 2019 10:21 PM
> To: Vadim Pasternak <vadimp@mellanox.com>
> Cc: linux-hwmon@vger.kernel.org
> Subject: Re: query for hwmon: (mlxreg-fan) patch
> 
> On 2/18/19 12:07 PM, Vadim Pasternak wrote:
> > Hi Guenter,
> >
> > I would like to ask you about the way of sending the patch for
> > drivers/hwmon/mlxreg-fan.c.
> >
> > This patch makes use of new "capability" register, but required commit
> > "platform_data/mlxreg: Add capability field to core platform data"
> > from platform for-next branch:
> > http://git.infradead.org/linux-platform-drivers-x86.git/blobdiff/9b28a
> >
> a1d0eae1be1016c8f4ba504545caff01da3..946e4e02b11889cb161b15ff4712a8b
> a2
> > 1a50eb6:/include/linux/platform_data/mlxreg.h
> >
> > Is it possibly to send a patch with such dependency? Or I should wait
> > until
> > "platform_data/mlxreg: Add capability field to core platform data" is
> > got to upstream?
> >
> 
> A single series submitted through one maintainer, with acks from all the other
> maintainers, is always the easiest. Otherwise one maintainer would have to
> create an immmutable branch with the essential patch(es) needed by all the
> others. Or, yes, you could wait for the infrastructure to be in place.

OK. I see.
Will follow you input for the future patches.

I used this field in mlx-platform and mlxre-hotplug drivers and also
need it for mlxreg-fan and leds-mlxreg driver.
And I thought that putting all in a single patchset will complicate
submission. Will learn from this mistake.

> 
> Makes me wonder, though: If you are open to doing that, why the complex,
> confusing, and risky mlxreg_wdt_check_watchdog_type() in the watchdog driver
> (instead of providing a means for the driver to get the HW version directly from
> the parent) ?

Well.
All this info really should be available in mlx-platform, which is a parent.
It has to provide configuration like:
static struct mlxreg_core_data mlxplat_mlxcpld_wd_main_regs_type2[] = {
	{
		.label = "action",
		.reg = MLXPLAT_CPLD_LPC_REG_WD2_ACT_OFFSET,
		.mask = MLXPLAT_CPLD_WD_RESET_ACT_MASK,
		.bit = 0,
	},
	{
		.label = "ping",
		.reg = MLXPLAT_CPLD_LPC_REG_WD2_ACT_OFFSET,
		.mask = MLXPLAT_CPLD_WD_RESET_ACT_MASK,
		.bit = 0,
	},
	...
};

It however will require to add some special field to "struct mlxreg_core_data"
for passing it to mlx_wdt.
We'll think about it.
Thanks for this input.

> 
> Guenter

Thank you very much,
Vadim.