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
