Re: [PATCH 1/3] pinctrl: rockchip: fix RK3328 pinmux bits

[Huang-Huang Bao <i@xxxxxx>]

On 6/6/24 18:08, Heiko Stübner wrote:
> Hi,
>
> Am Donnerstag, 6. Juni 2024, 08:04:33 CEST schrieb Huang-Huang Bao:
> > The pinmux bits for GPIO2-B0 to GPIO2-B6 actually have 2 bits width,
> > correct the bank flag for GPIO2-B. The pinmux bits for GPIO2-B7 is
> > recalculated so it remain unchanged.
>
> I've verified the gpio2-related pin settings via the TRM, so this part
> looks good :-)
>
>
> > The pinmux bits for GPIO3-B1 to GPIO3-B6 have different register offset
> > than common rockhip pinmux, set the correct value for those pins in
> > rk3328_mux_recalced_data.
> >
> > The pinmux bits for those pins are not explicitly specified in RK3328
> > TRM, however we can get hint from pad name and its correspinding IOMUX
> > setting for pins in interface descriptions, e.g.
> > IO_SPIclkm0_GPIO2B0vccio5 with GRF_GPIO2B_IOMUX[1:0]=2'b01 setting.
> >
> > This fix has been tested on NanoPi R2S for fixing confliting pinmux bits
> > between GPIO2-15 with GPIO2-13.
>
> As you said, the gpio3-based pins are not documented in the TRM,
> but in your description above you're talking about pins in the gpio2-
> group?
>
> So where did the gpio3-related pin information come from?

Sorry I didn't explained it clearly, gpio3 information is inferred from
pad name in interface descriptions similar to gpio2's, all those pad
name has format of "*GPIO<bank><pin number>*". So I just search up
interface description rows with pad name from "GPIO2B0" to "GPIO2B6" and
from "GPIO3B1" to "GPIO4B7" to find potential IOMUX bits definitions.
For example, there is IO_TSPd5m0_CIFdata5m0_GPIO3B1vccio6 with
GRF_GPIO3BH_iomux[3:2] = 2'b01 setting for GPIO3B1.

Maybe I can list all these occurrences in v2.

> Also, could you please split this patch in two pieces, one fixing the
> gpio2-area and one for the new gpio3 pins please?

Sure.

> Thanks
> Heiko
>
> > Signed-off-by: Huang-Huang Bao <i@xxxxxx>
> > ---
> >   drivers/pinctrl/pinctrl-rockchip.c | 59 ++++++++++++++++++++++++++----
> >   1 file changed, 52 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
> > index 3bedf36a0019..23531ea0d088 100644
> > --- a/drivers/pinctrl/pinctrl-rockchip.c
> > +++ b/drivers/pinctrl/pinctrl-rockchip.c
> > @@ -634,23 +634,68 @@ static struct rockchip_mux_recalced_data rk3308_mux_recalced_data[] = {
> >   
> >   static struct rockchip_mux_recalced_data rk3328_mux_recalced_data[] = {
> >   	{
> > -		.num = 2,
> > -		.pin = 12,
> > -		.reg = 0x24,
> > -		.bit = 8,
> > -		.mask = 0x3
> > -	}, {
> > +		/* gpio2_b7_sel */
> >   		.num = 2,
> >   		.pin = 15,
> >   		.reg = 0x28,
> >   		.bit = 0,
> >   		.mask = 0x7
> >   	}, {
> > +		/* gpio2_c7_sel */
> >   		.num = 2,
> >   		.pin = 23,
> >   		.reg = 0x30,
> >   		.bit = 14,
> >   		.mask = 0x3
> > +	}, {
> > +		/* gpio3_b1_sel */
> > +		.num = 3,
> > +		.pin = 9,
> > +		.reg = 0x44,
> > +		.bit = 2,
> > +		.mask = 0x3
> > +	}, {
> > +		/* gpio3_b2_sel */
> > +		.num = 3,
> > +		.pin = 10,
> > +		.reg = 0x44,
> > +		.bit = 4,
> > +		.mask = 0x3
> > +	}, {
> > +		/* gpio3_b3_sel */
> > +		.num = 3,
> > +		.pin = 11,
> > +		.reg = 0x44,
> > +		.bit = 6,
> > +		.mask = 0x3
> > +	}, {
> > +		/* gpio3_b4_sel */
> > +		.num = 3,
> > +		.pin = 12,
> > +		.reg = 0x44,
> > +		.bit = 8,
> > +		.mask = 0x3
> > +	}, {
> > +		/* gpio3_b5_sel */
> > +		.num = 3,
> > +		.pin = 13,
> > +		.reg = 0x44,
> > +		.bit = 10,
> > +		.mask = 0x3
> > +	}, {
> > +		/* gpio3_b6_sel */
> > +		.num = 3,
> > +		.pin = 14,
> > +		.reg = 0x44,
> > +		.bit = 12,
> > +		.mask = 0x3
> > +	}, {
> > +		/* gpio3_b7_sel */
> > +		.num = 3,
> > +		.pin = 15,
> > +		.reg = 0x44,
> > +		.bit = 14,
> > +		.mask = 0x3
> >   	},
> >   };
> >   
> > @@ -3763,7 +3808,7 @@ static struct rockchip_pin_bank rk3328_pin_banks[] = {
> >   	PIN_BANK_IOMUX_FLAGS(0, 32, "gpio0", 0, 0, 0, 0),
> >   	PIN_BANK_IOMUX_FLAGS(1, 32, "gpio1", 0, 0, 0, 0),
> >   	PIN_BANK_IOMUX_FLAGS(2, 32, "gpio2", 0,
> > -			     IOMUX_WIDTH_3BIT,
> > +			     0,
> >   			     IOMUX_WIDTH_3BIT,
> >   			     0),
> >   	PIN_BANK_IOMUX_FLAGS(3, 32, "gpio3",