> On Thu, Oct 10, 2024 at 12:14 PM Christian Marangi <ansuelsmth@xxxxxxxxx> 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
Attachment:
signature.asc
Description: PGP signature
