Recommendations for a new MFD device driver?

Recommendations for a new MFD device driver?

[Sascha Hauer]

Hi All,

We are adding support for a new pretty typical MFD device, the MediaTek
MT6397. Initial patches are already posted. It's a PMIC which among other
things has regulators and a RTC. The same RTC is reused on another PMIC,
but with another register offset and another interrupt.

Now the question is where shall we put the register/irq resource
information?

1) Put it into the RTC device driver.
2) Put it into the .resource field of struct mfd_cell
3) Put it into the device tree using standard reg, interrupt properties and
   a) Let the RTC driver interpret these
   b) Let the MFD driver create resources in the .resource field of struct
      mfd_cell
   c) Let the MFD core create the resources

I have a tendency to 3, but I'm afraid that the resource informations
are duplicated too much in the device tree source files, because every
user would have to carry a full description of the mfd device. Maybe
that duplication could be reduced with some CPP magic, I don't know.

Maybe this is a solved problem and I'm just not picking a good example
from drivers/mfd.

Any thoughts?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: Recommendations for a new MFD device driver?

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1135 bytes --]

On Fri, Feb 06, 2015 at 11:10:06AM +0100, Sascha Hauer wrote:

> 1) Put it into the RTC device driver.
> 2) Put it into the .resource field of struct mfd_cell
> 3) Put it into the device tree using standard reg, interrupt properties and
>    a) Let the RTC driver interpret these
>    b) Let the MFD driver create resources in the .resource field of struct
>       mfd_cell
>    c) Let the MFD core create the resources

> I have a tendency to 3, but I'm afraid that the resource informations
> are duplicated too much in the device tree source files, because every
> user would have to carry a full description of the mfd device. Maybe
> that duplication could be reduced with some CPP magic, I don't know.

> Maybe this is a solved problem and I'm just not picking a good example
> from drivers/mfd.

The wm831x drivers take option 2 but are pre-DT, it still seems the most
sensible thing to me though - no need for the user to have to repeat
this information in every DT and easy to add new stuff if we need it.
You could use a .dtsi like we use for SoCs to reduce the duplication
required if you do decide to put things in the DT.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: Recommendations for a new MFD device driver?

[Yingjoe Chen]

On Fri, 2015-02-06 at 11:54 +0000, Mark Brown wrote:
> On Fri, Feb 06, 2015 at 11:10:06AM +0100, Sascha Hauer wrote:
> 
> > 1) Put it into the RTC device driver.
> > 2) Put it into the .resource field of struct mfd_cell
> > 3) Put it into the device tree using standard reg, interrupt properties and
> >    a) Let the RTC driver interpret these
> >    b) Let the MFD driver create resources in the .resource field of struct
> >       mfd_cell
> >    c) Let the MFD core create the resources
> 
> > I have a tendency to 3, but I'm afraid that the resource informations
> > are duplicated too much in the device tree source files, because every
> > user would have to carry a full description of the mfd device. Maybe
> > that duplication could be reduced with some CPP magic, I don't know.
> 
> > Maybe this is a solved problem and I'm just not picking a good example
> > from drivers/mfd.
> 
> The wm831x drivers take option 2 but are pre-DT, it still seems the most
> sensible thing to me though - no need for the user to have to repeat
> this information in every DT and easy to add new stuff if we need it.
> You could use a .dtsi like we use for SoCs to reduce the duplication
> required if you do decide to put things in the DT.

Hi,

I think we should create a dtsi file, say mt6397.dtsi, the regulator
device nodes are already too much to duplicate into every board using
mt6397.

But a new problem is where should we put it? MT6397 is used in both
mt8135-evbp1(armv7) & mt8173-evb(armv8) boards, so they both need
mt6397.dtsi. Should we put them in include/dt-bindings to avoid
duplicate it into both arch/arm/boot/dts and arch/arm64/boot/dts?

Joe.C

Re: Recommendations for a new MFD device driver?

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 728 bytes --]

On Fri, Feb 06, 2015 at 08:55:12PM +0800, Yingjoe Chen wrote:

> I think we should create a dtsi file, say mt6397.dtsi, the regulator
> device nodes are already too much to duplicate into every board using
> mt6397.

Please bear in mind that the regulator bindings are all about what is
being done to connect the PMIC to the rest of the system on a given
board; this means that it is almost always a bad idea to have a generic
include file for the PMIC since while many boards may be similar other
boards may differ and it's potentially unsafe to try to handle things
generally.  

It can be a good idea to have a .dsti file for the common elements of a
widely used reference design but that's a different thing.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: Recommendations for a new MFD device driver?

[Lee Jones]

On Fri, 06 Feb 2015, Sascha Hauer wrote:

> Hi All,
> 
> We are adding support for a new pretty typical MFD device, the MediaTek
> MT6397. Initial patches are already posted. It's a PMIC which among other
> things has regulators and a RTC. The same RTC is reused on another PMIC,
> but with another register offset and another interrupt.
> 
> Now the question is where shall we put the register/irq resource
> information?
> 
> 1) Put it into the RTC device driver.
> 2) Put it into the .resource field of struct mfd_cell
> 3) Put it into the device tree using standard reg, interrupt properties and
>    a) Let the RTC driver interpret these
>    b) Let the MFD driver create resources in the .resource field of struct
>       mfd_cell
>    c) Let the MFD core create the resources

I have no problem with 2, if that's the route you'd like to take.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: Recommendations for a new MFD device driver?

[Sascha Hauer]

On Tue, Feb 10, 2015 at 03:20:39PM +0800, Lee Jones wrote:
> On Fri, 06 Feb 2015, Sascha Hauer wrote:
> 
> > Hi All,
> > 
> > We are adding support for a new pretty typical MFD device, the MediaTek
> > MT6397. Initial patches are already posted. It's a PMIC which among other
> > things has regulators and a RTC. The same RTC is reused on another PMIC,
> > but with another register offset and another interrupt.
> > 
> > Now the question is where shall we put the register/irq resource
> > information?
> > 
> > 1) Put it into the RTC device driver.
> > 2) Put it into the .resource field of struct mfd_cell
> > 3) Put it into the device tree using standard reg, interrupt properties and
> >    a) Let the RTC driver interpret these
> >    b) Let the MFD driver create resources in the .resource field of struct
> >       mfd_cell
> >    c) Let the MFD core create the resources
> 
> I have no problem with 2, if that's the route you'd like to take.

Ok, thanks Lee and Mark for the input. I think we should go with option
2 then. While it's convenient to put the resources into the device tree
in the first place, it probably becomes cumbersome when Linux gets a
different idea how the subdevices of the PMIC should be arranged.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |