| Message ID | 20241001-en7581-pinctrl-v5-3-dc1ce542b6c6@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add mfd, pinctrl and pwm support to EN7581 SoC | expand |
On Tue, 01 Oct 2024, Lorenzo Bianconi wrote: > From: Christian Marangi <ansuelsmth@gmail.com> > > Support for Airoha EN7581 Multi Function Device that > expose PINCTRL functionality and PWM functionality. The device is a jumble of pinctrl registers, some of which can oscillate. This is *still* not an MFD. If you wish to spread this functionality over 2 drivers, use syscon to obtain the registers and simple-mfd to automatically probe the drivers. > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > --- > drivers/mfd/Kconfig | 8 ++++ > drivers/mfd/Makefile | 2 + > drivers/mfd/airoha-en7581-gpio-mfd.c | 72 +++++++++++++++++++++++++++++++++++ > include/linux/mfd/airoha-en7581-mfd.h | 9 +++++ > 4 files changed, 91 insertions(+) > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index f9325bcce1b9..eca221351ab7 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -32,6 +32,14 @@ config MFD_ADP5585 > the core APIs _only_, you have to select individual components like > the GPIO and PWM functions under the corresponding menus. > > +config MFD_AIROHA_EN7581 > + bool "Airoha EN7581 Multi Function Device" > + depends on (ARCH_AIROHA || COMPILE_TEST) && OF > + select MFD_CORE > + help > + Support for Airoha EN7581 Multi Function Device that > + expose PINCTRL functionality and PWM functionality. > + > config MFD_ALTERA_A10SR > bool "Altera Arria10 DevKit System Resource chip" > depends on ARCH_INTEL_SOCFPGA && SPI_MASTER=y && OF > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > index 2a9f91e81af8..be8448e81a5b 100644 > --- a/drivers/mfd/Makefile > +++ b/drivers/mfd/Makefile > @@ -257,6 +257,8 @@ obj-$(CONFIG_INTEL_SOC_PMIC_CHTWC) += intel_soc_pmic_chtwc.o > obj-$(CONFIG_INTEL_SOC_PMIC_CHTDC_TI) += intel_soc_pmic_chtdc_ti.o > obj-$(CONFIG_INTEL_SOC_PMIC_MRFLD) += intel_soc_pmic_mrfld.o > > +obj-$(CONFIG_MFD_AIROHA_EN7581) += airoha-en7581-gpio-mfd.o > + > obj-$(CONFIG_MFD_ALTERA_A10SR) += altera-a10sr.o > obj-$(CONFIG_MFD_ALTERA_SYSMGR) += altera-sysmgr.o > obj-$(CONFIG_MFD_STPMIC1) += stpmic1.o > diff --git a/drivers/mfd/airoha-en7581-gpio-mfd.c b/drivers/mfd/airoha-en7581-gpio-mfd.c > new file mode 100644 > index 000000000000..88407ce5747e > --- /dev/null > +++ b/drivers/mfd/airoha-en7581-gpio-mfd.c > @@ -0,0 +1,72 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * MFD driver for Airoha EN7581 > + */ > + > +#include <linux/io.h> > +#include <linux/of.h> > +#include <linux/mfd/airoha-en7581-mfd.h> > +#include <linux/mfd/core.h> > +#include <linux/module.h> > + > +static struct resource airoha_mfd_pinctrl_intr[] = { > + { > + .flags = IORESOURCE_IRQ, > + }, > +}; > + > +static const struct mfd_cell airoha_mfd_devs[] = { > + { > + .name = "pinctrl-airoha", > + .resources = airoha_mfd_pinctrl_intr, > + .num_resources = ARRAY_SIZE(airoha_mfd_pinctrl_intr), > + }, { > + .name = "pwm-airoha", > + }, > +}; > + > +static int airoha_mfd_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct airoha_mfd *mfd; > + int irq; > + > + mfd = devm_kzalloc(dev, sizeof(*mfd), GFP_KERNEL); > + if (!mfd) > + return -ENOMEM; > + > + platform_set_drvdata(pdev, mfd); > + > + mfd->base = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(mfd->base)) > + return PTR_ERR(mfd->base); > + > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) > + return irq; > + > + airoha_mfd_pinctrl_intr[0].start = irq; > + airoha_mfd_pinctrl_intr[0].end = irq; > + > + return devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, airoha_mfd_devs, > + ARRAY_SIZE(airoha_mfd_devs), NULL, 0, > + NULL); > +} > + > +static const struct of_device_id airoha_mfd_of_match[] = { > + { .compatible = "airoha,en7581-gpio-sysctl" }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, airoha_mfd_of_match); > + > +static struct platform_driver airoha_mfd_driver = { > + .probe = airoha_mfd_probe, > + .driver = { > + .name = KBUILD_MODNAME, > + .of_match_table = airoha_mfd_of_match, > + }, > +}; > +module_platform_driver(airoha_mfd_driver); > + > +MODULE_AUTHOR("Christian Marangi <ansuelsmth@gmail.com>"); > +MODULE_DESCRIPTION("Driver for Airoha EN7581 MFD"); > diff --git a/include/linux/mfd/airoha-en7581-mfd.h b/include/linux/mfd/airoha-en7581-mfd.h > new file mode 100644 > index 000000000000..25e73952a777 > --- /dev/null > +++ b/include/linux/mfd/airoha-en7581-mfd.h > @@ -0,0 +1,9 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _LINUX_INCLUDE_MFD_AIROHA_EN7581_MFD_H_ > +#define _LINUX_INCLUDE_MFD_AIROHA_EN7581_MFD_H_ > + > +struct airoha_mfd { > + void __iomem *base; > +}; > + > +#endif /* _LINUX_INCLUDE_MFD_AIROHA_EN7581_MFD_H_ */ > > -- > 2.46.2 >
On Wed, Oct 02, 2024 at 02:25:18PM +0100, Lee Jones wrote: > On Tue, 01 Oct 2024, Lorenzo Bianconi wrote: > > > From: Christian Marangi <ansuelsmth@gmail.com> > > > > Support for Airoha EN7581 Multi Function Device that > > expose PINCTRL functionality and PWM functionality. > > The device is a jumble of pinctrl registers, some of which can oscillate. > > This is *still* not an MFD. > > If you wish to spread this functionality over 2 drivers, use syscon to > obtain the registers and simple-mfd to automatically probe the drivers. > Hi Lee, let me summarize the situation so it's more clear why this additional mfd driver. There were various iteration for these 2 driver (pinctrl and PWM). Due to the fact that these 2 are placed in the same register block with the PWM register in the middle, we proposed various .yaml schema that could better model it. The first idea was to map the single register used by the 2 driver. pio: pinctrl@1fa20214 { compatible = "airoha,en7581-pinctrl"; reg = <0x0 0x1fa20214 0x0 0x30>, <0x0 0x1fa2027c 0x0 0x8>, <0x0 0x1fbf0234 0x0 0x4>, <0x0 0x1fbf0268 0x0 0x4>, <0x0 0x1fa2001c 0x0 0x50>, <0x0 0x1fa2018c 0x0 0x4>, <0x0 0x1fbf0204 0x0 0x4>, <0x0 0x1fbf0270 0x0 0x4>, <0x0 0x1fbf0200 0x0 0x4>, <0x0 0x1fbf0220 0x0 0x4>, <0x0 0x1fbf0260 0x0 0x4>, <0x0 0x1fbf0264 0x0 0x4>, <0x0 0x1fbf0214 0x0 0x4>, <0x0 0x1fbf0278 0x0 0x4>, <0x0 0x1fbf0208 0x0 0x4>, <0x0 0x1fbf027c 0x0 0x4>, <0x0 0x1fbf0210 0x0 0x4>, <0x0 0x1fbf028c 0x0 0x4>, <0x0 0x1fbf0290 0x0 0x4>, <0x0 0x1fbf0294 0x0 0x4>, <0x0 0x1fbf020c 0x0 0x4>, <0x0 0x1fbf0280 0x0 0x4>, <0x0 0x1fbf0284 0x0 0x4>, <0x0 0x1fbf0288 0x0 0x4>; gpio-controller; #gpio-cells = <2>; gpio-ranges = <&pio 0 13 47>; ... }; pwm@1fbf0224 { compatible = "airoha,en7581-pwm"; reg = <0x1fbf0224 0x10>, <0x1fbf0238 0x28>, <0x1fbf0298 0x8>; #pwm-cells = <3>; }; This was quickly rejected as it introduced way more complication to workaround the overlapping addresses. (the device should map the entire register space) The second proposal was a parent+child implementation with the pinctrl parent and the PWM child by referencing a syscon from the parent. pio: pinctrl@1fbf0200 { compatible = "airoha,en7581-pinctrl", "simple-mfd", "syscon"; reg = <0x1fbf0200 0x0 0xbc>; airoha,chip-scu = <&chip_scu>; .... pwm { compatible = "airoha,en7581-pwm"; ... }; }; Also this second proposal was rejected by device tree folks as the device implement both pinctrl and PWM in the register space and they are not actually separate devices. There was also an additional proposal with the entire register map in a dedicated node with syscon and pwm + pinctrl using it. This was also rejected by device tree folks. (node that have only a syscon are a nono) It was suggested that this is a case of MFD (multi-functional-device) As suggested we proposed a simple-mfd implementation following common pattern. Parent node with simple-mfd and syscon compatible and 2 child nodes, one with pinctrl and the other with PWM with each his own compatible. mfd@1fbf0200 { compatible = "airoha,en7581-gpio-mfd"; reg = <0x0 0x1fbf0200 0x0 0xc0>; interrupt-parent = <&gic>; interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>; pio: pinctrl { compatible = "airoha,en7581-pinctrl"; gpio-controller; #gpio-cells = <2>; interrupt-controller; #interrupt-cells = <2>; }; pwm: pwm { compatible = "airoha,en7581-pwm"; #pwm-cells = <3>; status = "disabled"; }; }; Also this was rejected by device tree folks as the property for pinctrl and pwm needed to be in the MFD node and there should't be child node with single compatible. This comes from the fact that DT needs to describe how the HW is modelled and not how the drivers are implemented. Finally Rob agreed and added the Reviwed-by on the current implementation with single MFD node with pinctrl and pwm. Also Conor and Krzysztof agreed on this solution for the task. mfd@1fbf0200 { compatible = "airoha,en7581-gpio-sysctl"; reg = <0x1fbf0200 0xc0>; interrupt-parent = <&gic>; interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>; gpio-controller; #gpio-cells = <2>; interrupt-controller; #interrupt-cells = <2>; #pwm-cells = <3>; pinctrl { ... }; }; With the following implementation, the only way to probe the additional driver is with a specialized mfd driver that probe the 2 driver by name and we can't really use a simple-mfd implementation as that requires child nodes with compatibles. Sorry for the long message and I honestly hope we can find together a common path for this between driver and Documentation. Is it clear now why we had to ultimely had to implement and model things this way? -- Ansuel
On Oct 02, Lee Jones wrote: > On Tue, 01 Oct 2024, Lorenzo Bianconi wrote: > > > From: Christian Marangi <ansuelsmth@gmail.com> > > > > Support for Airoha EN7581 Multi Function Device that > > expose PINCTRL functionality and PWM functionality. > > The device is a jumble of pinctrl registers, some of which can oscillate. > > This is *still* not an MFD. > > If you wish to spread this functionality over 2 drivers, use syscon to > obtain the registers and simple-mfd to automatically probe the drivers. Hi Lee, IIUC you are suggesting two possible approaches here: 1- have a single driver implementing both pinctrl and pwm functionalities. This approach will not let us reuse the code for future devices that have just one of them in common, like pwm (but we can live with that). 2- use a device node like the one below (something similar to [0]) system-controller@1fbf0200 { compatible = "syscon", "simple-mfd"; reg = <0x0 0x1fbf0200 0x0 0xc0>; interrupt-parent = <&gic>; interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>; gpio-controller; #gpio-cells = <2>; interrupt-controller; #interrupt-cells = <2>; pio: pinctrl { compatible = "airoha,en7581-pinctrl"; [ some pinctrl properties here ] }; #pwm-cells = <3>; pwm { compatible = "airoha,en7581-pwm"; }; }; Please correct me if I am wrong, but using syscon/simple-mfd as compatible string for the 'parent' device, will require to introduce the compatible strings even for the child devices in order to probe them, correct? If so, as pointed out by Christian, this is something nacked by Rob/Krzysztof/Conor (this is the main reason why we introduced a full mfd driver here). @Rob, Krzysztof, Conor: am I right? I guess we need to find a middle ground here between mfd and dts to support this uncommon hw device. Regards, Lorenzo [0] https://elixir.bootlin.com/linux/v6.11.2/source/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi#L269 > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > --- > > drivers/mfd/Kconfig | 8 ++++ > > drivers/mfd/Makefile | 2 + > > drivers/mfd/airoha-en7581-gpio-mfd.c | 72 +++++++++++++++++++++++++++++++++++ > > include/linux/mfd/airoha-en7581-mfd.h | 9 +++++ > > 4 files changed, 91 insertions(+) > > > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > > index f9325bcce1b9..eca221351ab7 100644 > > --- a/drivers/mfd/Kconfig > > +++ b/drivers/mfd/Kconfig > > @@ -32,6 +32,14 @@ config MFD_ADP5585 > > the core APIs _only_, you have to select individual components like > > the GPIO and PWM functions under the corresponding menus. > > > > +config MFD_AIROHA_EN7581 > > + bool "Airoha EN7581 Multi Function Device" > > + depends on (ARCH_AIROHA || COMPILE_TEST) && OF > > + select MFD_CORE > > + help > > + Support for Airoha EN7581 Multi Function Device that > > + expose PINCTRL functionality and PWM functionality. > > + > > config MFD_ALTERA_A10SR > > bool "Altera Arria10 DevKit System Resource chip" > > depends on ARCH_INTEL_SOCFPGA && SPI_MASTER=y && OF > > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > > index 2a9f91e81af8..be8448e81a5b 100644 > > --- a/drivers/mfd/Makefile > > +++ b/drivers/mfd/Makefile > > @@ -257,6 +257,8 @@ obj-$(CONFIG_INTEL_SOC_PMIC_CHTWC) += intel_soc_pmic_chtwc.o > > obj-$(CONFIG_INTEL_SOC_PMIC_CHTDC_TI) += intel_soc_pmic_chtdc_ti.o > > obj-$(CONFIG_INTEL_SOC_PMIC_MRFLD) += intel_soc_pmic_mrfld.o > > > > +obj-$(CONFIG_MFD_AIROHA_EN7581) += airoha-en7581-gpio-mfd.o > > + > > obj-$(CONFIG_MFD_ALTERA_A10SR) += altera-a10sr.o > > obj-$(CONFIG_MFD_ALTERA_SYSMGR) += altera-sysmgr.o > > obj-$(CONFIG_MFD_STPMIC1) += stpmic1.o > > diff --git a/drivers/mfd/airoha-en7581-gpio-mfd.c b/drivers/mfd/airoha-en7581-gpio-mfd.c > > new file mode 100644 > > index 000000000000..88407ce5747e > > --- /dev/null > > +++ b/drivers/mfd/airoha-en7581-gpio-mfd.c > > @@ -0,0 +1,72 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * MFD driver for Airoha EN7581 > > + */ > > + > > +#include <linux/io.h> > > +#include <linux/of.h> > > +#include <linux/mfd/airoha-en7581-mfd.h> > > +#include <linux/mfd/core.h> > > +#include <linux/module.h> > > + > > +static struct resource airoha_mfd_pinctrl_intr[] = { > > + { > > + .flags = IORESOURCE_IRQ, > > + }, > > +}; > > + > > +static const struct mfd_cell airoha_mfd_devs[] = { > > + { > > + .name = "pinctrl-airoha", > > + .resources = airoha_mfd_pinctrl_intr, > > + .num_resources = ARRAY_SIZE(airoha_mfd_pinctrl_intr), > > + }, { > > + .name = "pwm-airoha", > > + }, > > +}; > > + > > +static int airoha_mfd_probe(struct platform_device *pdev) > > +{ > > + struct device *dev = &pdev->dev; > > + struct airoha_mfd *mfd; > > + int irq; > > + > > + mfd = devm_kzalloc(dev, sizeof(*mfd), GFP_KERNEL); > > + if (!mfd) > > + return -ENOMEM; > > + > > + platform_set_drvdata(pdev, mfd); > > + > > + mfd->base = devm_platform_ioremap_resource(pdev, 0); > > + if (IS_ERR(mfd->base)) > > + return PTR_ERR(mfd->base); > > + > > + irq = platform_get_irq(pdev, 0); > > + if (irq < 0) > > + return irq; > > + > > + airoha_mfd_pinctrl_intr[0].start = irq; > > + airoha_mfd_pinctrl_intr[0].end = irq; > > + > > + return devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, airoha_mfd_devs, > > + ARRAY_SIZE(airoha_mfd_devs), NULL, 0, > > + NULL); > > +} > > + > > +static const struct of_device_id airoha_mfd_of_match[] = { > > + { .compatible = "airoha,en7581-gpio-sysctl" }, > > + { /* sentinel */ } > > +}; > > +MODULE_DEVICE_TABLE(of, airoha_mfd_of_match); > > + > > +static struct platform_driver airoha_mfd_driver = { > > + .probe = airoha_mfd_probe, > > + .driver = { > > + .name = KBUILD_MODNAME, > > + .of_match_table = airoha_mfd_of_match, > > + }, > > +}; > > +module_platform_driver(airoha_mfd_driver); > > + > > +MODULE_AUTHOR("Christian Marangi <ansuelsmth@gmail.com>"); > > +MODULE_DESCRIPTION("Driver for Airoha EN7581 MFD"); > > diff --git a/include/linux/mfd/airoha-en7581-mfd.h b/include/linux/mfd/airoha-en7581-mfd.h > > new file mode 100644 > > index 000000000000..25e73952a777 > > --- /dev/null > > +++ b/include/linux/mfd/airoha-en7581-mfd.h > > @@ -0,0 +1,9 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef _LINUX_INCLUDE_MFD_AIROHA_EN7581_MFD_H_ > > +#define _LINUX_INCLUDE_MFD_AIROHA_EN7581_MFD_H_ > > + > > +struct airoha_mfd { > > + void __iomem *base; > > +}; > > + > > +#endif /* _LINUX_INCLUDE_MFD_AIROHA_EN7581_MFD_H_ */ > > > > -- > > 2.46.2 > > > > -- > Lee Jones [李琼斯]
On Wed, 09 Oct 2024, Lorenzo Bianconi wrote: > On Oct 02, Lee Jones wrote: > > On Tue, 01 Oct 2024, Lorenzo Bianconi wrote: > > > > > From: Christian Marangi <ansuelsmth@gmail.com> > > > > > > Support for Airoha EN7581 Multi Function Device that > > > expose PINCTRL functionality and PWM functionality. > > > > The device is a jumble of pinctrl registers, some of which can oscillate. > > > > This is *still* not an MFD. > > > > If you wish to spread this functionality over 2 drivers, use syscon to > > obtain the registers and simple-mfd to automatically probe the drivers. > > Hi Lee, > > IIUC you are suggesting two possible approaches here: > > 1- have a single driver implementing both pinctrl and pwm functionalities. > This approach will not let us reuse the code for future devices that > have just one of them in common, like pwm (but we can live with that). If you can have one without the other, then they are separate devices. > 2- use a device node like the one below (something similar to [0]) > > system-controller@1fbf0200 { > compatible = "syscon", "simple-mfd"; > reg = <0x0 0x1fbf0200 0x0 0xc0>; > > interrupt-parent = <&gic>; > interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>; > > gpio-controller; > #gpio-cells = <2>; > > interrupt-controller; > #interrupt-cells = <2>; > > pio: pinctrl { > compatible = "airoha,en7581-pinctrl"; > > [ some pinctrl properties here ] > }; > > #pwm-cells = <3>; > > pwm { > compatible = "airoha,en7581-pwm"; > }; > }; > > Please correct me if I am wrong, but using syscon/simple-mfd as compatible > string for the 'parent' device, will require to introduce the compatible strings > even for the child devices in order to probe them, correct? > If so, as pointed out by Christian, this is something nacked by Rob/Krzysztof/Conor > (this is the main reason why we introduced a full mfd driver here). > > @Rob, Krzysztof, Conor: am I right? I don't see why separate functionality shouldn't have separate compatible strings, even if the registers are together. Register layout and functionality separation are not related.
On Wed, 09 Oct 2024, Lee Jones wrote: > On Wed, 09 Oct 2024, Lorenzo Bianconi wrote: > > > On Oct 02, Lee Jones wrote: > > > On Tue, 01 Oct 2024, Lorenzo Bianconi wrote: > > > > > > > From: Christian Marangi <ansuelsmth@gmail.com> > > > > > > > > Support for Airoha EN7581 Multi Function Device that > > > > expose PINCTRL functionality and PWM functionality. > > > > > > The device is a jumble of pinctrl registers, some of which can oscillate. > > > > > > This is *still* not an MFD. > > > > > > If you wish to spread this functionality over 2 drivers, use syscon to > > > obtain the registers and simple-mfd to automatically probe the drivers. > > > > Hi Lee, > > > > IIUC you are suggesting two possible approaches here: > > > > 1- have a single driver implementing both pinctrl and pwm functionalities. > > This approach will not let us reuse the code for future devices that > > have just one of them in common, like pwm (but we can live with that). > > If you can have one without the other, then they are separate devices. > > > 2- use a device node like the one below (something similar to [0]) > > > > system-controller@1fbf0200 { > > compatible = "syscon", "simple-mfd"; > > reg = <0x0 0x1fbf0200 0x0 0xc0>; > > > > interrupt-parent = <&gic>; > > interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>; > > > > gpio-controller; > > #gpio-cells = <2>; > > > > interrupt-controller; > > #interrupt-cells = <2>; > > > > pio: pinctrl { > > compatible = "airoha,en7581-pinctrl"; > > > > [ some pinctrl properties here ] > > }; > > > > #pwm-cells = <3>; > > > > pwm { > > compatible = "airoha,en7581-pwm"; > > }; > > }; > > > > Please correct me if I am wrong, but using syscon/simple-mfd as compatible > > string for the 'parent' device, will require to introduce the compatible strings > > even for the child devices in order to probe them, correct? > > If so, as pointed out by Christian, this is something nacked by Rob/Krzysztof/Conor > > (this is the main reason why we introduced a full mfd driver here). > > > > @Rob, Krzysztof, Conor: am I right? > > I don't see why separate functionality shouldn't have separate > compatible strings, even if the registers are together. Register layout > and functionality separation are not related. We've been happy to support both pinctrl and pwm devices before: git grep "\-pinctrl\|\-pwm" -- drivers/mfd git grep "\-pinctrl\|\-pwm" -- arch/*/boot/dts git grep "\-pinctrl" -- arch/*/boot/dts | wc -l 602 git grep "\-pwm" -- arch/*/boot/dts | wc -l 856 What makes this particular device different to all of the others?
On Wed, Oct 09, 2024 at 11:55:50AM +0100, Lee Jones wrote: > On Wed, 09 Oct 2024, Lee Jones wrote: > > > On Wed, 09 Oct 2024, Lorenzo Bianconi wrote: > > > > > On Oct 02, Lee Jones wrote: > > > > On Tue, 01 Oct 2024, Lorenzo Bianconi wrote: > > > > > > > > > From: Christian Marangi <ansuelsmth@gmail.com> > > > > > > > > > > Support for Airoha EN7581 Multi Function Device that > > > > > expose PINCTRL functionality and PWM functionality. > > > > > > > > The device is a jumble of pinctrl registers, some of which can oscillate. > > > > > > > > This is *still* not an MFD. > > > > > > > > If you wish to spread this functionality over 2 drivers, use syscon to > > > > obtain the registers and simple-mfd to automatically probe the drivers. > > > > > > Hi Lee, > > > > > > IIUC you are suggesting two possible approaches here: > > > > > > 1- have a single driver implementing both pinctrl and pwm functionalities. > > > This approach will not let us reuse the code for future devices that > > > have just one of them in common, like pwm (but we can live with that). > > > > If you can have one without the other, then they are separate devices. > > > > > 2- use a device node like the one below (something similar to [0]) > > > > > > system-controller@1fbf0200 { > > > compatible = "syscon", "simple-mfd"; > > > reg = <0x0 0x1fbf0200 0x0 0xc0>; > > > > > > interrupt-parent = <&gic>; > > > interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>; > > > > > > gpio-controller; > > > #gpio-cells = <2>; > > > > > > interrupt-controller; > > > #interrupt-cells = <2>; > > > > > > pio: pinctrl { > > > compatible = "airoha,en7581-pinctrl"; > > > > > > [ some pinctrl properties here ] > > > }; > > > > > > #pwm-cells = <3>; > > > > > > pwm { > > > compatible = "airoha,en7581-pwm"; > > > }; > > > }; > > > > > > Please correct me if I am wrong, but using syscon/simple-mfd as compatible > > > string for the 'parent' device, will require to introduce the compatible strings > > > even for the child devices in order to probe them, correct? > > > If so, as pointed out by Christian, this is something nacked by Rob/Krzysztof/Conor > > > (this is the main reason why we introduced a full mfd driver here). > > > > > > @Rob, Krzysztof, Conor: am I right? > > > > I don't see why separate functionality shouldn't have separate > > compatible strings, even if the registers are together. Register layout > > and functionality separation are not related. > > We've been happy to support both pinctrl and pwm devices before: > > git grep "\-pinctrl\|\-pwm" -- drivers/mfd > git grep "\-pinctrl\|\-pwm" -- arch/*/boot/dts > > git grep "\-pinctrl" -- arch/*/boot/dts | wc -l > 602 > git grep "\-pwm" -- arch/*/boot/dts | wc -l > 856 > > What makes this particular device different to all of the others? > Hi Lee, this would be the final DTS following the "simple-mfd" pattern. Can you confirm it's correct? mfd: system-controller@1fbf0200 { compatible = "syscon", "simple-mfd"; reg = <0x0 0x1fbf0200 0x0 0xc0>; interrupt-parent = <&gic>; interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>; gpio-controller; #gpio-cells = <2>; interrupt-controller; #interrupt-cells = <2>; gpio-ranges = <&mfd 0 13 47>; #pwm-cells = <3>; pio: pinctrl { compatible = "airoha,en7581-pinctrl"; mdio_pins: mdio-pins { mux { function = "mdio"; groups = "mdio"; }; conf { pins = "gpio2"; output-high; }; }; pcie0_rst_pins: pcie0-rst-pins { conf { pins = "pcie_reset0"; drive-open-drain = <1>; }; }; pcie1_rst_pins: pcie1-rst-pins { conf { pins = "pcie_reset1"; drive-open-drain = <1>; }; }; }; pwm { compatible = "airoha,en7581-pwm"; }; };
On Thu, Oct 10, 2024 at 12:14:01PM +0200, Christian Marangi wrote: > On Wed, Oct 09, 2024 at 11:55:50AM +0100, Lee Jones wrote: > > On Wed, 09 Oct 2024, Lee Jones wrote: > > > > > On Wed, 09 Oct 2024, Lorenzo Bianconi wrote: > > > > > > > On Oct 02, Lee Jones wrote: > > > > > On Tue, 01 Oct 2024, Lorenzo Bianconi wrote: > > > > > > > > > > > From: Christian Marangi <ansuelsmth@gmail.com> > > > > > > > > > > > > Support for Airoha EN7581 Multi Function Device that > > > > > > expose PINCTRL functionality and PWM functionality. > > > > > > > > > > The device is a jumble of pinctrl registers, some of which can oscillate. > > > > > > > > > > This is *still* not an MFD. > > > > > > > > > > If you wish to spread this functionality over 2 drivers, use syscon to > > > > > obtain the registers and simple-mfd to automatically probe the drivers. > > > > > > > > Hi Lee, > > > > > > > > IIUC you are suggesting two possible approaches here: > > > > > > > > 1- have a single driver implementing both pinctrl and pwm functionalities. > > > > This approach will not let us reuse the code for future devices that > > > > have just one of them in common, like pwm (but we can live with that). > > > > > > If you can have one without the other, then they are separate devices. > > > > > > > 2- use a device node like the one below (something similar to [0]) > > > > > > > > system-controller@1fbf0200 { > > > > compatible = "syscon", "simple-mfd"; > > > > reg = <0x0 0x1fbf0200 0x0 0xc0>; > > > > > > > > interrupt-parent = <&gic>; > > > > interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>; > > > > > > > > gpio-controller; > > > > #gpio-cells = <2>; > > > > > > > > interrupt-controller; > > > > #interrupt-cells = <2>; > > > > > > > > pio: pinctrl { > > > > compatible = "airoha,en7581-pinctrl"; > > > > > > > > [ some pinctrl properties here ] > > > > }; > > > > > > > > #pwm-cells = <3>; > > > > > > > > pwm { > > > > compatible = "airoha,en7581-pwm"; > > > > }; > > > > }; > > > > > > > > Please correct me if I am wrong, but using syscon/simple-mfd as compatible > > > > string for the 'parent' device, will require to introduce the compatible strings > > > > even for the child devices in order to probe them, correct? > > > > If so, as pointed out by Christian, this is something nacked by Rob/Krzysztof/Conor > > > > (this is the main reason why we introduced a full mfd driver here). > > > > > > > > @Rob, Krzysztof, Conor: am I right? > > > > > > I don't see why separate functionality shouldn't have separate > > > compatible strings, even if the registers are together. Register layout > > > and functionality separation are not related. > > > > We've been happy to support both pinctrl and pwm devices before: > > > > git grep "\-pinctrl\|\-pwm" -- drivers/mfd > > git grep "\-pinctrl\|\-pwm" -- arch/*/boot/dts > > > > git grep "\-pinctrl" -- arch/*/boot/dts | wc -l > > 602 > > git grep "\-pwm" -- arch/*/boot/dts | wc -l > > 856 > > > > What makes this particular device different to all of the others? > > > > Hi Lee, > > this would be the final DTS following the "simple-mfd" pattern. > > Can you confirm it's correct? > > mfd: system-controller@1fbf0200 { > compatible = "syscon", "simple-mfd"; > reg = <0x0 0x1fbf0200 0x0 0xc0>; > > interrupt-parent = <&gic>; > interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>; > > gpio-controller; > #gpio-cells = <2>; > > interrupt-controller; > #interrupt-cells = <2>; > > gpio-ranges = <&mfd 0 13 47>; > > #pwm-cells = <3>; > > pio: pinctrl { > compatible = "airoha,en7581-pinctrl"; > > mdio_pins: mdio-pins { > mux { > function = "mdio"; > groups = "mdio"; > }; > > conf { > pins = "gpio2"; > output-high; > }; > }; > > pcie0_rst_pins: pcie0-rst-pins { > conf { > pins = "pcie_reset0"; > drive-open-drain = <1>; > }; > }; > > pcie1_rst_pins: pcie1-rst-pins { > conf { > pins = "pcie_reset1"; > drive-open-drain = <1>; > }; > }; > }; > > pwm { > compatible = "airoha,en7581-pwm"; > }; > }; > Also asking to Rob, Krzysztof and Conor. Is this DTS OK for you?
On Thu, 10 Oct 2024, Christian Marangi wrote: > On Wed, Oct 09, 2024 at 11:55:50AM +0100, Lee Jones wrote: > > On Wed, 09 Oct 2024, Lee Jones wrote: > > > > > On Wed, 09 Oct 2024, Lorenzo Bianconi wrote: > > > > > > > On Oct 02, Lee Jones wrote: > > > > > On Tue, 01 Oct 2024, Lorenzo Bianconi wrote: > > > > > > > > > > > From: Christian Marangi <ansuelsmth@gmail.com> > > > > > > > > > > > > Support for Airoha EN7581 Multi Function Device that > > > > > > expose PINCTRL functionality and PWM functionality. > > > > > > > > > > The device is a jumble of pinctrl registers, some of which can oscillate. > > > > > > > > > > This is *still* not an MFD. > > > > > > > > > > If you wish to spread this functionality over 2 drivers, use syscon to > > > > > obtain the registers and simple-mfd to automatically probe the drivers. > > > > > > > > Hi Lee, > > > > > > > > IIUC you are suggesting two possible approaches here: > > > > > > > > 1- have a single driver implementing both pinctrl and pwm functionalities. > > > > This approach will not let us reuse the code for future devices that > > > > have just one of them in common, like pwm (but we can live with that). > > > > > > If you can have one without the other, then they are separate devices. > > > > > > > 2- use a device node like the one below (something similar to [0]) > > > > > > > > system-controller@1fbf0200 { > > > > compatible = "syscon", "simple-mfd"; > > > > reg = <0x0 0x1fbf0200 0x0 0xc0>; > > > > > > > > interrupt-parent = <&gic>; > > > > interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>; > > > > > > > > gpio-controller; > > > > #gpio-cells = <2>; > > > > > > > > interrupt-controller; > > > > #interrupt-cells = <2>; > > > > > > > > pio: pinctrl { > > > > compatible = "airoha,en7581-pinctrl"; > > > > > > > > [ some pinctrl properties here ] > > > > }; > > > > > > > > #pwm-cells = <3>; > > > > > > > > pwm { > > > > compatible = "airoha,en7581-pwm"; > > > > }; > > > > }; > > > > > > > > Please correct me if I am wrong, but using syscon/simple-mfd as compatible > > > > string for the 'parent' device, will require to introduce the compatible strings > > > > even for the child devices in order to probe them, correct? > > > > If so, as pointed out by Christian, this is something nacked by Rob/Krzysztof/Conor > > > > (this is the main reason why we introduced a full mfd driver here). > > > > > > > > @Rob, Krzysztof, Conor: am I right? > > > > > > I don't see why separate functionality shouldn't have separate > > > compatible strings, even if the registers are together. Register layout > > > and functionality separation are not related. > > > > We've been happy to support both pinctrl and pwm devices before: > > > > git grep "\-pinctrl\|\-pwm" -- drivers/mfd > > git grep "\-pinctrl\|\-pwm" -- arch/*/boot/dts > > > > git grep "\-pinctrl" -- arch/*/boot/dts | wc -l > > 602 > > git grep "\-pwm" -- arch/*/boot/dts | wc -l > > 856 > > > > What makes this particular device different to all of the others? > > > > Hi Lee, > > this would be the final DTS following the "simple-mfd" pattern. > > Can you confirm it's correct? I can't confirm that it's 100% correct, but it looks okay to me. > mfd: system-controller@1fbf0200 { Not sure about the mfd: label though. What is the device? > compatible = "syscon", "simple-mfd"; > reg = <0x0 0x1fbf0200 0x0 0xc0>; > > interrupt-parent = <&gic>; > interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>; > > gpio-controller; > #gpio-cells = <2>; > > interrupt-controller; > #interrupt-cells = <2>; > > gpio-ranges = <&mfd 0 13 47>; > > #pwm-cells = <3>; > > pio: pinctrl { > compatible = "airoha,en7581-pinctrl"; > > mdio_pins: mdio-pins { > mux { > function = "mdio"; > groups = "mdio"; > }; > > conf { > pins = "gpio2"; > output-high; > }; > }; > > pcie0_rst_pins: pcie0-rst-pins { > conf { > pins = "pcie_reset0"; > drive-open-drain = <1>; > }; > }; > > pcie1_rst_pins: pcie1-rst-pins { > conf { > pins = "pcie_reset1"; > drive-open-drain = <1>; > }; > }; > }; > > pwm { > compatible = "airoha,en7581-pwm"; > }; > }; > > -- > Ansuel
On Thu, Oct 10, 2024 at 12:14 PM Christian Marangi <ansuelsmth@gmail.com> wrote: > mfd: system-controller@1fbf0200 { Drop the mfd: thing, you probably don't want to reference the syscon node directly in the device tree. If you still give it a label just say en7581_syscon: system-controller... > compatible = "syscon", "simple-mfd"; > reg = <0x0 0x1fbf0200 0x0 0xc0>; > > interrupt-parent = <&gic>; > interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>; > > gpio-controller; > #gpio-cells = <2>; > > interrupt-controller; > #interrupt-cells = <2>; > > gpio-ranges = <&mfd 0 13 47>; I think you want a separate GPIO node inside the system controller: en7581_gpio: gpio { compatible = "airhoa,en7581-gpio"; interrupt-parent = <&gic>; interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>; gpio-controller; #gpio-cells = <2>; interrupt-controller; #interrupt-cells = <2>; gpio-ranges = <&en7581_pinctrl 0 13 47>; }; So users pick GPIOs: foo-gpios = <&en7581_gpio ...>; Notice that the gpio-ranges should refer to the pin controller node. > > #pwm-cells = <3>; Shouldn't this be inside the pwm node? en7581_pwm: pwm { compatible = "airoha,en7581-pwm"; #pwm-cells = <3>; }; So PWM users can pick a PWM with pwms = <&en7581_pwm ...>; > pio: pinctrl { I would use the label en7581_pinctrl: > compatible = "airoha,en7581-pinctrl"; > > mdio_pins: mdio-pins { > mux { > function = "mdio"; > groups = "mdio"; > }; > > conf { > pins = "gpio2"; > output-high; > }; > }; > > pcie0_rst_pins: pcie0-rst-pins { > conf { > pins = "pcie_reset0"; > drive-open-drain = <1>; > }; > }; > > pcie1_rst_pins: pcie1-rst-pins { > conf { > pins = "pcie_reset1"; > drive-open-drain = <1>; > }; > }; > }; > > pwm { > compatible = "airoha,en7581-pwm"; > }; > }; This will make subdevices probe and you can put the pure GPIO driver in drivers/gpio/gpio-en7581.c Yours, Linus Walleij
> On Thu, Oct 10, 2024 at 12:14 PM Christian Marangi <ansuelsmth@gmail.com> wrote: > > > mfd: system-controller@1fbf0200 { > > Drop the mfd: thing, you probably don't want to reference the syscon > node directly > in the device tree. If you still give it a label just say > en7581_syscon: system-controller... ack, I am fine with it. > > > compatible = "syscon", "simple-mfd"; > > reg = <0x0 0x1fbf0200 0x0 0xc0>; > > > > interrupt-parent = <&gic>; > > interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>; > > > > gpio-controller; > > #gpio-cells = <2>; > > > > interrupt-controller; > > #interrupt-cells = <2>; > > > > gpio-ranges = <&mfd 0 13 47>; > > I think you want a separate GPIO node inside the system controller: > > en7581_gpio: gpio { > compatible = "airhoa,en7581-gpio"; > interrupt-parent = <&gic>; > interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>; > > gpio-controller; > #gpio-cells = <2>; > > interrupt-controller; > #interrupt-cells = <2>; > > gpio-ranges = <&en7581_pinctrl 0 13 47>; > }; So far I implemented the gpio functionalities in the en7581 pinctrl driver (as it is done for other mtk pinctrl drivers) but I am fine to reuse the gpio-en7523 driver for it. Do you prefer this second approach? > > So users pick GPIOs: > > foo-gpios = <&en7581_gpio ...>; > > Notice that the gpio-ranges should refer to the pin controller > node. > > > > > #pwm-cells = <3>; > > Shouldn't this be inside the pwm node? > > en7581_pwm: pwm { > compatible = "airoha,en7581-pwm"; > #pwm-cells = <3>; > }; > > So PWM users can pick a PWM with pwms = <&en7581_pwm ...>; ack, I am fine with it. > > > pio: pinctrl { > > I would use the label en7581_pinctrl: ack, I am fine with it. > > > compatible = "airoha,en7581-pinctrl"; > > > > mdio_pins: mdio-pins { > > mux { > > function = "mdio"; > > groups = "mdio"; > > }; > > > > conf { > > pins = "gpio2"; > > output-high; > > }; > > }; > > > > pcie0_rst_pins: pcie0-rst-pins { > > conf { > > pins = "pcie_reset0"; > > drive-open-drain = <1>; > > }; > > }; > > > > pcie1_rst_pins: pcie1-rst-pins { > > conf { > > pins = "pcie_reset1"; > > drive-open-drain = <1>; > > }; > > }; > > }; > > > > pwm { > > compatible = "airoha,en7581-pwm"; > > }; > > }; > > This will make subdevices probe and you can put the pure GPIO > driver in drivers/gpio/gpio-en7581.c We could actually reuse gpio-en7523 driver (removing the gpio part from en7581 pinctrl driver) and extend it to support irq_chip. I do not have a strong opinion about it. Regards, Lorenzo > > Yours, > Linus Walleij
On Thu, Oct 10, 2024 at 11:41 PM Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > On Thu, Oct 10, 2024 at 12:14 PM Christian Marangi <ansuelsmth@gmail.com> wrote: > > I think you want a separate GPIO node inside the system controller: > > > > en7581_gpio: gpio { > > compatible = "airhoa,en7581-gpio"; > > interrupt-parent = <&gic>; > > interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>; > > > > gpio-controller; > > #gpio-cells = <2>; > > > > interrupt-controller; > > #interrupt-cells = <2>; > > > > gpio-ranges = <&en7581_pinctrl 0 13 47>; > > }; > > So far I implemented the gpio functionalities in the en7581 pinctrl driver > (as it is done for other mtk pinctrl drivers) but I am fine to reuse the > gpio-en7523 driver for it. Do you prefer this second approach? It's fine to combine GPIO and pin control into the same node, especially if you will have a combined driver for it, then it's more or less mandatory. I only wrote this looking at ansuel's sketch. > > This will make subdevices probe and you can put the pure GPIO > > driver in drivers/gpio/gpio-en7581.c > > We could actually reuse gpio-en7523 driver (removing the gpio part from en7581 > pinctrl driver) and extend it to support irq_chip. I do not have a strong > opinion about it. Code reuse is always preferred, if possible. Yours, Linus Walleij
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index f9325bcce1b9..eca221351ab7 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -32,6 +32,14 @@ config MFD_ADP5585 the core APIs _only_, you have to select individual components like the GPIO and PWM functions under the corresponding menus. +config MFD_AIROHA_EN7581 + bool "Airoha EN7581 Multi Function Device" + depends on (ARCH_AIROHA || COMPILE_TEST) && OF + select MFD_CORE + help + Support for Airoha EN7581 Multi Function Device that + expose PINCTRL functionality and PWM functionality. + config MFD_ALTERA_A10SR bool "Altera Arria10 DevKit System Resource chip" depends on ARCH_INTEL_SOCFPGA && SPI_MASTER=y && OF diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index 2a9f91e81af8..be8448e81a5b 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -257,6 +257,8 @@ obj-$(CONFIG_INTEL_SOC_PMIC_CHTWC) += intel_soc_pmic_chtwc.o obj-$(CONFIG_INTEL_SOC_PMIC_CHTDC_TI) += intel_soc_pmic_chtdc_ti.o obj-$(CONFIG_INTEL_SOC_PMIC_MRFLD) += intel_soc_pmic_mrfld.o +obj-$(CONFIG_MFD_AIROHA_EN7581) += airoha-en7581-gpio-mfd.o + obj-$(CONFIG_MFD_ALTERA_A10SR) += altera-a10sr.o obj-$(CONFIG_MFD_ALTERA_SYSMGR) += altera-sysmgr.o obj-$(CONFIG_MFD_STPMIC1) += stpmic1.o diff --git a/drivers/mfd/airoha-en7581-gpio-mfd.c b/drivers/mfd/airoha-en7581-gpio-mfd.c new file mode 100644 index 000000000000..88407ce5747e --- /dev/null +++ b/drivers/mfd/airoha-en7581-gpio-mfd.c @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * MFD driver for Airoha EN7581 + */ + +#include <linux/io.h> +#include <linux/of.h> +#include <linux/mfd/airoha-en7581-mfd.h> +#include <linux/mfd/core.h> +#include <linux/module.h> + +static struct resource airoha_mfd_pinctrl_intr[] = { + { + .flags = IORESOURCE_IRQ, + }, +}; + +static const struct mfd_cell airoha_mfd_devs[] = { + { + .name = "pinctrl-airoha", + .resources = airoha_mfd_pinctrl_intr, + .num_resources = ARRAY_SIZE(airoha_mfd_pinctrl_intr), + }, { + .name = "pwm-airoha", + }, +}; + +static int airoha_mfd_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct airoha_mfd *mfd; + int irq; + + mfd = devm_kzalloc(dev, sizeof(*mfd), GFP_KERNEL); + if (!mfd) + return -ENOMEM; + + platform_set_drvdata(pdev, mfd); + + mfd->base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(mfd->base)) + return PTR_ERR(mfd->base); + + irq = platform_get_irq(pdev, 0); + if (irq < 0) + return irq; + + airoha_mfd_pinctrl_intr[0].start = irq; + airoha_mfd_pinctrl_intr[0].end = irq; + + return devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, airoha_mfd_devs, + ARRAY_SIZE(airoha_mfd_devs), NULL, 0, + NULL); +} + +static const struct of_device_id airoha_mfd_of_match[] = { + { .compatible = "airoha,en7581-gpio-sysctl" }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, airoha_mfd_of_match); + +static struct platform_driver airoha_mfd_driver = { + .probe = airoha_mfd_probe, + .driver = { + .name = KBUILD_MODNAME, + .of_match_table = airoha_mfd_of_match, + }, +}; +module_platform_driver(airoha_mfd_driver); + +MODULE_AUTHOR("Christian Marangi <ansuelsmth@gmail.com>"); +MODULE_DESCRIPTION("Driver for Airoha EN7581 MFD"); diff --git a/include/linux/mfd/airoha-en7581-mfd.h b/include/linux/mfd/airoha-en7581-mfd.h new file mode 100644 index 000000000000..25e73952a777 --- /dev/null +++ b/include/linux/mfd/airoha-en7581-mfd.h @@ -0,0 +1,9 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _LINUX_INCLUDE_MFD_AIROHA_EN7581_MFD_H_ +#define _LINUX_INCLUDE_MFD_AIROHA_EN7581_MFD_H_ + +struct airoha_mfd { + void __iomem *base; +}; + +#endif /* _LINUX_INCLUDE_MFD_AIROHA_EN7581_MFD_H_ */