Thread (20 messages) 20 messages, 8 authors, 2024-08-30

Re: [PATCH v2 1/2] dt-bindings: pinctrl: airoha: Add EN7581 pinctrl controller

From: Conor Dooley <conor.dooley@microchip.com>
Date: 2024-08-30 11:02:02
Also in: linux-devicetree, linux-gpio, linux-mediatek

On Fri, Aug 30, 2024 at 12:55:32PM +0200, Lorenzo Bianconi wrote:
[...]
quoted
quoted
quoted
quoted
Hi Rob, thanks a lot for the hint, I hope we can finally find a solution
on how to implement this.

In Documentation the block is called GPIO Controller. As explained it does
expose pinctrl function AND pwm (with regs in the middle)

Is this semplification really needed? It does pose some problem driver
wise (on where to put the driver, in what subsystem) and also on the
Sorry, but no, dt-bindings do not affect the driver at all in such way.
Nothing changes in your driver in such aspect, no dilemma where to put
it (the same place as before).
Ok, from the proposed node structure, is it problematic to move the
gpio-controller and -cells in the pinctrl node? And also the pwm-cells
to the pwm node?
The move is just unnecessary and not neat. You design DTS based on your
drivers architecture and this is exactly what we want to avoid.
quoted
This is similar to how it's done by broadcom GPIO MFD [1] that also
There are 'reg' fields, which is the main problem here. I don't like
that arguments because it entirely misses the discussions - about that
binding or other bindings - happening prior to merge.
quoted
expose pinctrl and other device in the same register block as MFD
childs.

This would be the final node block.

                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>;
No resources here...
ack. iiuc, all the properties will be in the parent node (mfd) and we will
have just the compatible strings in the child ones, right? Something like:

		mfd@1fbf0200 {
			compatible = "airoha,en7581-gpio-mfd";
			reg = <0x0 0x1fbf0200 0x0 0xc0>;
			gpio-controller;
			#gpio-cells = <2>;

			...
			#pwm-cells = <3>;

			pio: pinctrl {
				compatible = "airoha,en7581-pinctrl";
			};

			pwm: pwm {
				compatible = "airoha,en7581-pwm";
			};
		};

Didn't Rob basically tell you how to do it earlier in the thread?
What you've got now makes no sense, the compatibles only exist in that
to probe drivers, which you can do from the mfd driver with
mfd_add_devices() or w/e that function is called.

Cheers,
Conor.

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help