| Message ID | cover.1548052878.git.baolin.wang@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add new device nodes for Spreadtrum SC9860 platform | expand |
On Mon, Jan 21, 2019 at 8:39 AM Baolin Wang <baolin.wang@linaro.org> wrote: > > This patch set adds charger and fuel gauge device nodes for Spreadtrum > SC2731 PMIC, it also removes redundant irq trigger setting for PMIC > devices and adds nvmem cells for ADC to calibrate the ADC channel scales. > > Baolin Wang (4): > arm64: dts: sprd: Remove PMIC INTC irq trigger type > arm64: dts: sprd: Add ADC calibration support > arm64: dts: sprd: Add SC2731 charger device > arm64: dts: sprd: Add SC27XX fuel gauge device I did not apply these because of the 'sc27xx' wildcard matching. Please fix those up and resend. Arnd
Hi Arnd, On Thu, 31 Jan 2019 at 00:46, Arnd Bergmann <arnd@arndb.de> wrote: > > On Mon, Jan 21, 2019 at 8:39 AM Baolin Wang <baolin.wang@linaro.org> wrote: > > > > This patch set adds charger and fuel gauge device nodes for Spreadtrum > > SC2731 PMIC, it also removes redundant irq trigger setting for PMIC > > devices and adds nvmem cells for ADC to calibrate the ADC channel scales. > > > > Baolin Wang (4): > > arm64: dts: sprd: Remove PMIC INTC irq trigger type > > arm64: dts: sprd: Add ADC calibration support > > arm64: dts: sprd: Add SC2731 charger device > > arm64: dts: sprd: Add SC27XX fuel gauge device > > I did not apply these because of the 'sc27xx' wildcard matching. > > Please fix those up and resend. > On Spreadtrum platform, we use one mfd driver [1] to populate the SC27XX series PMICs including SC2731, SC2721, SC2720 and SC2730. So we use sc27xx to be compatible with different PMICs' devices, otherwise it will be difficult to define the mfd cell arrays in mfd driver. Do you have any good suggestion? Thanks. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/mfd/sprd-sc27xx-spi.c
On Fri, Feb 01, 2019 at 08:05:30PM +0800, Baolin Wang wrote: > On Spreadtrum platform, we use one mfd driver [1] to populate the > SC27XX series PMICs including SC2731, SC2721, SC2720 and SC2730. So we > use sc27xx to be compatible with different PMICs' devices, otherwise > it will be difficult to define the mfd cell arrays in mfd driver. Do > you have any good suggestion? Thanks. You can just list all the individual device names in the of_match_table for the MFD and then it can bind to any of them. You can always map them onto the same behaviour in the MFD driver if they are identical from a software point of view.
Hi Mark, Sorry for late reply. On Fri, 1 Feb 2019 at 21:05, Mark Brown <broonie@kernel.org> wrote: > > On Fri, Feb 01, 2019 at 08:05:30PM +0800, Baolin Wang wrote: > > > On Spreadtrum platform, we use one mfd driver [1] to populate the > > SC27XX series PMICs including SC2731, SC2721, SC2720 and SC2730. So we > > use sc27xx to be compatible with different PMICs' devices, otherwise > > it will be difficult to define the mfd cell arrays in mfd driver. Do > > you have any good suggestion? Thanks. > > You can just list all the individual device names in the of_match_table > for the MFD and then it can bind to any of them. You can always map > them onto the same behaviour in the MFD driver if they are identical > from a software point of view. If I understood correctly, as you suggested, we should add new mfd_cell groups to list all different PMICs' device names. Something like: static const struct mfd_cell sprd_pmic_sc2731_devs[] = { { .name = "sc27xx-wdt", .of_compatible = "sprd,sc2731-wdt", }, { .name = "sc27xx-rtc", .of_compatible = "sprd,sc2731-rtc", }, { .name = "sc27xx-charger", .of_compatible = "sprd,sc2731-charger", }, { .name = "sc27xx-fast-chg", .of_compatible = "sprd,sc2731-fast-chg", }, { .name = "sc27xx-typec", .of_compatible = "sprd,sc2731-typec", }, { .name = "sc27xx-eic", .of_compatible = "sprd,sc2731-eic", }, ....... }; static const struct mfd_cell sprd_pmic_sc2730_devs[] = { { .name = "sc27xx-wdt", .of_compatible = "sprd,sc2730-wdt", }, { .name = "sc27xx-rtc", .of_compatible = "sprd,sc2730-rtc", }, { .name = "sc27xx-charger", .of_compatible = "sprd,sc2730-charger", }, { .name = "sc27xx-fast-chg", .of_compatible = "sprd,sc2730-fast-chg", }, { .name = "sc27xx-typec", .of_compatible = "sprd,sc2730-typec", }, { .name = "sc27xx-eic", .of_compatible = "sprd,sc2730-eic", }, ....... }; ...... But from my point, they are just some meaningless duplication, and will waste lots of code there.
On Tue, Feb 12, 2019 at 04:40:10PM +0800, Baolin Wang wrote: > On Fri, 1 Feb 2019 at 21:05, Mark Brown <broonie@kernel.org> wrote: > > You can just list all the individual device names in the of_match_table > > for the MFD and then it can bind to any of them. You can always map > > them onto the same behaviour in the MFD driver if they are identical > > from a software point of view. > If I understood correctly, as you suggested, we should add new > mfd_cell groups to list all different PMICs' device names. Something > like: I do think this is a good idea (registering the components of the MFD using mfd_cell), though it wasn't quite the point I was making. Having individual device names matters less for Linux-internal names like this. > But from my point, they are just some meaningless duplication, and > will waste lots of code there. I was more thinking of the of_match table that has the IDs that appear in DT - they're the one that's the ABI. Look at something like wm8994 where the driver has several IDs listed in the main table then selects function drivers based on that.
On Tue, 12 Feb 2019 at 20:20, Mark Brown <broonie@kernel.org> wrote: > > On Tue, Feb 12, 2019 at 04:40:10PM +0800, Baolin Wang wrote: > > On Fri, 1 Feb 2019 at 21:05, Mark Brown <broonie@kernel.org> wrote: > > > > You can just list all the individual device names in the of_match_table > > > for the MFD and then it can bind to any of them. You can always map > > > them onto the same behaviour in the MFD driver if they are identical > > > from a software point of view. > > > If I understood correctly, as you suggested, we should add new > > mfd_cell groups to list all different PMICs' device names. Something > > like: > > I do think this is a good idea (registering the components of the MFD > using mfd_cell), though it wasn't quite the point I was making. Having > individual device names matters less for Linux-internal names like this. > > > But from my point, they are just some meaningless duplication, and > > will waste lots of code there. > > I was more thinking of the of_match table that has the IDs that appear > in DT - they're the one that's the ABI. Look at something like wm8994 > where the driver has several IDs listed in the main table then selects > function drivers based on that. Yes, we can use id_table to populate the PMIC child devices, but some child devices need use the device node to get some resources which are described in DT, so we must specify the of_compabible member of mfd_cell. That means we should use compatible string of the of_match_table to populate the child devices. So the problem is still there, we should expand the mfd_cell groups to list all device names, but just some meaningless duplication. -- Baolin Wang Best Regards