On Tue, 4 Apr 2023 09:23:36 +0200 Krzysztof Kozlowski krzysztof.kozlowski@linaro.org wrote:
On 04/04/2023 09:20, Herve Codina wrote:
I suggest that none of this (besides the child registration, which is achieved by a simple OF API call in this case) has anything to do with MFD. We are not requesting and initialising shared resources and we are not using the MFD API to register children. The pin control functionality clearly needs moving to Pinctrl and the rest, if you cannot find a suitable home for it *may be* suitable for Misc.
I am confused and I am not really sure to understand where to put my driver.
The core pef2256.c needs to:
- setup the pef2256
- add the children
To add the children it calls devm_of_platform_populate() to add the audio parts as several audio children can be available with the same compatible string.
I plan to move the pinctrl part to the pinctrl subsystem. With this done, the core pef2256.c will probably add the children using:
- a mfd_cell for the pinctrl part
- devm_of_platform_populate() for the audio children
The setup (E1 lines and TDM configuration) still needs to be done by the core pef2256.c. Moving this part only to Misc will break the hierarchy. The audio children depends on the core pef2256.c as this one do the setup. Having in the audio children and the part that do the setup in same hierarchy level is not correct. Audio children should be children of the part that do the setup.
So, the structure I have in mind:
- pef2256.c (MFD) implement and do the setup at probe() Add the children at probe():
- pef2256-pinctrl (pinctrl) added using mfd_add_devices()
- pef2256-codec (ASoC codec) added using devm_of_platform_populate()
Lee, with this in mind, can the core pef2256.c be a MFD driver ?
You do not use MFD here, so why do you want to keep it in MFD? If you disagree, please tell me where is the MFD code in your patch?
I don't want to absolutely use MFD. I just want to put my driver somewhere and I don't know the right location between MFD and Misc.
Basically, the driver needs to do (little simplified and error path removed):
static const struct mfd_cell pef2256_devs[] = { { .name = "lantiq-pef2256-pinctrl", }, };
static int pef2256_probe(struct platform_device *pdev) { struct pef2256 *pef2256; void __iomem *iomem; int ret; int irq;
pef2256 = devm_kzalloc(&pdev->dev, sizeof(*pef2256), GFP_KERNEL); if (!pef2256) return -ENOMEM;
pef2256->dev = &pdev->dev;
iomem = devm_platform_ioremap_resource(pdev, 0);
pef2256->regmap = devm_regmap_init_mmio(&pdev->dev, iomem, &pef2256_regmap_config);
pef2256->mclk = devm_clk_get_enabled(&pdev->dev, "mclk"); pef2256->sclkr = devm_clk_get_enabled(&pdev->dev, "sclkr"); pef2256->sclkx = devm_clk_get_enabled(&pdev->dev, "sclkx");
pef2256->reset_gpio = devm_gpiod_get_optional(&pdev->dev, "reset", GPIOD_OUT_LOW); if (pef2256->reset_gpio) { gpiod_set_value_cansleep(pef2256->reset_gpio, 1); udelay(10); gpiod_set_value_cansleep(pef2256->reset_gpio, 0); udelay(10); }
pef2556_of_parse(pef2256, np);
irq = platform_get_irq(pdev, 0); ret = devm_request_irq(pef2256->dev, irq, pef2256_irq_handler, 0, "pef2256", pef2256);
platform_set_drvdata(pdev, pef2256);
mfd_add_devices(pef2256->dev, PLATFORM_DEVID_NONE, pef2256_devs, ARRAY_SIZE(pef2256_devs), NULL, 0, NULL);
pef2256_setup(pef2256);
devm_of_platform_populate(pef2256->dev); return 0; }
Best regards, Krzysztof