Re: [PATCH v4 02/11] mfd: Add support for Kontron sl28cpld management controller

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

 



Hi,

Am 2020-06-05 12:50, schrieb Mark Brown:
On Fri, Jun 05, 2020 at 07:57:09AM +0100, Lee Jones wrote:
On Thu, 04 Jun 2020, Michael Walle wrote:

> +	sl28cpld->regmap = devm_regmap_init_i2c(i2c, &sl28cpld_regmap_config);
> +	if (IS_ERR(sl28cpld->regmap))
> +		return PTR_ERR(sl28cpld->regmap);

This is now a shared memory allocator and not an MFD at all.

I'm clamping down on these type of drivers!

Please find a better way to accomplish this.

What is the concern with this?  Looking at the patch I'm guessing the
concern would be that the driver isn't instantiating any MFD children
and instead requiring them to be put in the DT?

Potentially using "simple-mfd" and "simple-regmap".

The former already exists and does what you want.  The latter doesn't
yet exist, but could solve your and lots of other contributor's
issues.

I have no idea what you are thinking of when you say "simple-regmap" so
it is difficult to comment.

I guess, Lee is suggesting to be able to create a regmap instance via
device tree (and populate its child nodes?). Like
  compatible = "syscon", "simple-mfd";
but for any regmap, not just MMIO.

But, there is more in my driver:
 (1) there is a version check
 (2) there is another function for which there is no suitable linux
     subsystem I'm aware of and thus which I'd like to us sysfs
     attributes for: This controller supports 16 non-volatile
     configuration bits. (this is still TBD)

I don't see what is different between this driver and for example the
gateworks-gsc.c. Just that mine doesn't use a global register set, but
local offsets and a base for each component. From a hardware
perspective its one device behind an I2C address providing different
functions across multiple driver subsystems.

Actually, I've tried to remove the devm_of_platform_populate() and
instead added the "simple-mfd" to my mfd node:
 compatible = "kontron,sl28cpld-r1", "simple-mfd";

I guess that doesn't work because the device is below the i2c bus?

--
-michael



[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