linux-kernel - Re: [PATCH 1/2] gpio: cadence: Add quirk for Axiado AX3000 platform

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAD++jL=BxQ0oqmSJnwLPnev1rjEygVXW1okB9VkywJUbXf3puQ@mail.gmail.com>
Date: Fri, 2 Jan 2026 22:46:02 +0100
From: Linus Walleij <linusw@...nel.org>
To: Swark Yang <syang@...ado.com>
Cc: Linus Walleij <linus.walleij@...aro.org>, Bartosz Golaszewski <brgl@...nel.org>, 
	linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Tzu-Hao Wei <twei@...ado.com>, Prasad Bolisetty <pbolisetty@...ado.com>
Subject: Re: [PATCH 1/2] gpio: cadence: Add quirk for Axiado AX3000 platform

Hi Swark,

thanks for your patch!

On Tue, Dec 23, 2025 at 8:30 AM Swark Yang <syang@...ado.com> wrote:

> On the Axiado AX3000 platform, pinmux and pin configuration (such as
> direction and output enable) are configured by the hardware/firmware
> at boot time before Linux boots.
>
> To prevent conflicts, introduce a platform-specific quirk triggered by
> the "axiado,ax3000-gpio" compatible string.
>
> When this quirk is active, the driver will skip its default
> initialization of pinmux configuration and direction settings during
> probe.
>
> Co-developed-by: Tzu-Hao Wei <twei@...ado.com>
> Signed-off-by: Tzu-Hao Wei <twei@...ado.com>
> Signed-off-by: Swark Yang <syang@...ado.com>
> Signed-off-by: Prasad Bolisetty <pbolisetty@...ado.com>

> +#define CDNS_GPIO_QUIRKS_SKIP_PINMUX_CFG       BIT(1)

This is just using bits and copying stuff unnecessarily.

Do this:

struct cdns_gpio_quirks {
    bool skip_init;
};

>  struct cdns_gpio_chip {
>         struct gpio_generic_chip gen_gc;
>         void __iomem *regs;
>         u32 bypass_orig;

In this struct add:

struct cdns_gpio_quirks *quirks;

And assign it in probe.

> +struct cdns_platform_data {
> +       u32 quirks;
> +};

Skip this double-translated thing.

>  static int cdns_gpio_request(struct gpio_chip *chip, unsigned int offset)
>  {
>         struct cdns_gpio_chip *cgpio = gpiochip_get_data(chip);
> @@ -141,6 +150,16 @@ static const struct irq_chip cdns_gpio_irqchip = {
>         GPIOCHIP_IRQ_RESOURCE_HELPERS,
>  };
>
> +static const struct cdns_platform_data ax3000_gpio_def = {
> +       .quirks = CDNS_GPIO_QUIRKS_SKIP_PINMUX_CFG, };

static const struct cdns_gpio_quirks cdns_default_quirks = {
    .skip_init = true,
};

static const struct cdns_gpio_quirks ax300_gpio_quirks = {
    .skip_init = true,
};

> +static const struct of_device_id cdns_of_ids[] = {
> +       { .compatible = "axiado,ax3000-gpio", .data = &ax3000_gpio_def },

.data = &ax3000_gpio_quirks,

> +       { .compatible = "cdns,gpio-r1p02" },

.data = &cdns_default_quirks,

This becomes more manageable, as I know there is *at least* one
more variant arriving, which is for CIX semiconductors and who knows
how many quirks we will have to carry in the future.

>  static int cdns_gpio_probe(struct platform_device *pdev)
>  {
>         struct gpio_generic_chip_config config = { };
> @@ -148,6 +167,8 @@ static int cdns_gpio_probe(struct platform_device *pdev)
>         int ret, irq;
>         u32 dir_prev;
>         u32 num_gpios = 32;
> +       bool skip_pinmux_cfg = false;

Nah, just reference the quirk.

> +       const struct of_device_id *match;
>         struct clk *clk;
>
>         cgpio = devm_kzalloc(&pdev->dev, sizeof(*cgpio), GFP_KERNEL);
> @@ -165,6 +186,13 @@ static int cdns_gpio_probe(struct platform_device *pdev)
>                 return -EINVAL;
>         }
>
> +       match = of_match_node(cdns_of_ids, pdev->dev.of_node);
> +       if (match && match->data) {
> +               const struct cdns_platform_data *data = match->data;
> +
> +               skip_pinmux_cfg = data->quirks & CDNS_GPIO_QUIRKS_SKIP_PINMUX_CFG;
> +       }

Instead do:

cgpio->quirks = device_get_match_data() like Bartosz suggests.

>         dir_prev = ioread32(cgpio->regs + CDNS_GPIO_DIRECTION_MODE);
> -       iowrite32(GENMASK(num_gpios - 1, 0),
> -                 cgpio->regs + CDNS_GPIO_DIRECTION_MODE);
> +
> +       /*
> +        * The AX3000 platform performs the required configuration at boot time
> +        * before Linux boots, so this quirk disables pinmux initialization.
> +        */

> +       if (!skip_pinmux_cfg) {

Here:
if (cgpio->quirks->skip_init) {

> +               iowrite32(GENMASK(num_gpios - 1, 0),
> +                         cgpio->regs + CDNS_GPIO_DIRECTION_MODE);
> +       }
>
>         config.dev = &pdev->dev;
>         config.sz = 4;
> @@ -240,9 +275,11 @@ static int cdns_gpio_probe(struct platform_device *pdev)
>         /*
>          * Enable gpio outputs, ignored for input direction
>          */
> -       iowrite32(GENMASK(num_gpios - 1, 0),
> -                 cgpio->regs + CDNS_GPIO_OUTPUT_EN);
> -       iowrite32(0, cgpio->regs + CDNS_GPIO_BYPASS_MODE);
> +       if (!skip_pinmux_cfg) {

if (cgpio->quirks->skip_init) {

> +               iowrite32(GENMASK(num_gpios - 1, 0),
> +                         cgpio->regs + CDNS_GPIO_OUTPUT_EN);

This has nothing to do with pinmux whatsoever so it is better to call
this quirk "skip_init"

> +               iowrite32(0, cgpio->regs + CDNS_GPIO_BYPASS_MODE);

This affects the pinmuxing though. But the muxing happens outside
of the GPIO block itself.

Yours,
Linus Walleij