Re:Re: Re: [PATCH v7 12/15] pinctrl: ambarella: add CV75 pin controller

[zl020895]

Thanks, Andy.

PINCTRL_PINGROUP only carries pin numbers. The IOMUX alt is not the
pinctrl function: the same pin is in several groups with different
alts, and a few groups mix alts.

In v8 I will switch the group tables to PINCTRL_PINGROUP and pass a
per-pin alt array as group data for set_mux(). PINCTRL_PINFUNCTION
stays as it is.

Best regards,
Long Zhao

At 2026-09-17 20:22:20, "Andy Shevchenko" <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>On Thu, Sep 17, 2026 at 05:06:45PM +0800, zl020895 wrote:
>> Thanks for the review. The small comments will be in v8,
>> including pinconf set via regmap_assign_bits.
>> 
>> PINCTRL_PINGROUP does not fit here: groups carry
>> AMBA_PINMUX(pin, alt), not a pin-number list, so I will keep
>> CV75_GROUP.
>
>I didn't get. It takes a pin as a parameter and adds the function on top
>of it. It is a combo, but I think it still fits the purpose. Please, use
>existing data structure and a macro.
>
>> At 2026-09-16 18:49:58, "Andy Shevchenko" <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>> >On Tue, Sep 15, 2026 at 07:15:42PM +0800, Long Zhao via B4 Relay wrote:
>
>...
>
>> >> +#define AMBA_PINMUX(pin, alt)		(((alt) << 12) | (pin))
>> >> +#define AMBA_PINMUX_TO_PIN(mux)		((mux) & 0xfff)
>> >> +#define AMBA_PINMUX_TO_ALT(mux)		(((mux) >> 12) & 0x7)
>> >> +
>> >> +struct amb_pinmux_group {
>> >> +	const char *name;
>> >> +	const u32 *mux;
>> >> +	unsigned int nmux;
>> >> +};
>
>^^^ left for the context.
>
>
>-- 
>With Best Regards,
>Andy Shevchenko
>