Thread (74 messages) 74 messages, 8 authors, 2020-06-10

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

From: Lee Jones <hidden>
Date: 2020-06-10 18:02:49
Also in: linux-arm-kernel, linux-gpio, linux-hwmon, linux-pwm, linux-watchdog, lkml

On Wed, 10 Jun 2020, Rob Herring wrote:
On Wed, Jun 10, 2020 at 1:19 AM Lee Jones [off-list ref] wrote:
quoted
On Wed, 10 Jun 2020, Michael Walle wrote:
quoted
Am 2020-06-09 21:45, schrieb Lee Jones:
quoted
On Tue, 09 Jun 2020, Michael Walle wrote:
quoted
quoted
We do not need a 'simple-regmap' solution for your use-case.

Since your device's registers are segregated, just split up the
register map and allocate each sub-device with it's own slice.
I don't get it, could you make a device tree example for my
use-case? (see also above)
    &i2cbus {
        mfd-device@10 {
            compatible = "simple-mfd";
            reg = <10>;

            sub-device@10 {
                compatible = "vendor,sub-device";
                reg = <10>;
            };
   };

The Regmap config would be present in each of the child devices.

Each child device would call devm_regmap_init_i2c() in .probe().
Ah, I see. If I'm not wrong, this still means to create an i2c
device driver with the name "simple-mfd".
Yes, it does.
TBC, while fine for a driver to bind on 'simple-mfd', a DT compatible
with that alone is not fine.
'simple-mfd' essentially means:

  "This device doesn't do anything useful, but the children do."

When used with 'syscon' it means:

  "Memory for this device is shared between all children"

Adding more specific/descriptive compatible strings is conceptually
fine, but they should not be forced to bind to a real driver using
them.  Else we're creating drivers for the sake of creating drivers.

This is especially true with 'simple-mfd' is used without 'syscon'.
quoted
quoted
Besides that, I don't like this, because:
 - Rob already expressed its concerns with "simple-mfd" and so on.
Where did this take place?  I'd like to read up on this.
quoted
 - you need to duplicate the config in each sub device
You can have a share a single config.
quoted
 - which also means you are restricting the sub devices to be
   i2c only (unless you implement and duplicate other regmap configs,
   too). For this driver, SPI and MMIO may be viable options.
You could also have a shared implementation to choose between different
busses.
I think it is really the syscon mfd driver you want to generalize to
other buses. Though with a quick look at it, there's not really a
whole lot to share. The regmap lookup would be the main thing. You are
going to need a driver instance for each bus type.
On it.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help