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

[Andrew Lunn <andrew@xxxxxxx>]

On Sat, Aug 17, 2024 at 10:46:10PM +0200, Benjamin Larsson wrote:
> On 17/08/2024 00:52, Rob Herring wrote:
> > > Hi, this is by design because of the register placement in the gpio block
> > > and the fact that the pwm functionality is intermixed in there also. As
> > > example the following registers are all GPIOCTRL:
> > > 
> > > <0x0 0x1fbf0200 0x0 0x4>,
> > > <0x0 0x1fbf0220 0x0 0x4>,
> > > <0x0 0x1fbf0260 0x0 0x4>,
> > > <0x0 0x1fbf0264 0x0 0x4>,
> > > 
> > > To simplify the driver code logic the complexity is moved to the dts because
> > > of that.
> > DT to OS is an ABI. Don't put the complexity there. The driver is easy
> > to change.
> > 
> > Lot's of h/w blocks are just bit soup. This is not special. If a few
> > regions is helpful, then that would be fine.
> > 
> > Rob
> 
> Hi, the pwm functionality is to blame.
> 
> The following is the logic that populates the direction registers
> (GPIOCTRL).
> 
>     for (i = 0; i < ARRAY_SIZE(pinctrl->gpiochip.dir); i++) {
>         ptr = devm_platform_ioremap_resource(pdev, index++);
>         if (IS_ERR(ptr))
>             return dev_err_probe(dev, PTR_ERR(ptr),
>                          "failed to map gpio dir regs\n");
> 
>         pinctrl->gpiochip.dir[i] = ptr;
>     }
> 
> 
> As example in between 0x1fbf0200, 0x1fbf0220 and 0x1fbf0260 we have pwm
> related registers.
> 
> The gpio block could if I count it correctly be split into 8+ regions. The
> dts list contain 18 rows related to the gpio block. So the savings would be
> ca 10 rows but a register mapping list in the driver would be needed
> instead.

How messy are the GPIO and PWM registers? Are there N blocks of
independent GPIO registers? and M blocks of independent PWM registers?
By that, does one block of GPIO registers contain all you need for one
GPIO controller? One block of PWM registers give you all you need for
one PWM controller? Or are the registers for one GPIO controller
scattered all over the place?

Could you point at a public datasheet?

      Andrew