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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Guenter Roeck <groeck7@xxxxxxxxx> On Behalf Of Guenter Roeck
> Sent: Monday, February 18, 2019 10:21 PM
> To: Vadim Pasternak <vadimp@xxxxxxxxxxxx>
> Cc: linux-hwmon@xxxxxxxxxxxxxxx
> 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.




[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux