Re: [PATCH v6 06/13] gpio: mxc: replace of_device_is_compatible() with hwdata flags

[Peng Fan]

On Mon, Oct 12, 2026 at 12:25:41PM +0300, Andy Shevchenko wrote:
>On Sun, Oct 11, 2026 at 2:54 PM Peng Fan (OSS) <peng.fan@xxxxxxxxxxx> wrote:
>
>>
>> Replace the runtime of_device_is_compatible() check for "fsl,imx7d-gpio"
>> with a flags field in mxc_gpio_hwdata to move the power-off capability
>> from a per-instance bool populated at probe time to static per-compatible
>> data.
>>
>> Introduce MXC_GPIO_HAS_POWER_OFF and a dedicated imx7d_gpio_hwdata
>> instance that carries it, along with a mxc_gpio_has_power_off() helper
>> that replaces every former port->power_off test.
>>
>> While at it, factor the register offsets shared by imx35 and imx7d into
>> a MXC_GPIO_HW_DATA_COMMON macro to avoid duplicating twelve identical
>> initializers.
>
>...
>
>>  struct mxc_gpio_hwdata {
>>         unsigned dr_reg;
>
>>         unsigned high_level;
>>         unsigned rise_edge;
>>         unsigned fall_edge;
>> +       unsigned int flags;
>
>Obviously you need another precursor patch to move from 'unsigned' to
>'unsigned int' in the whole driver.

Could we keep as it is? I try to avoid adding more changes if not critical.

>
>>  };
>
>...
>
>> +#define MXC_GPIO_HW_DATA_COMMON        \
>> +       .dr_reg         = 0x00, \
>> +       .gdir_reg       = 0x04, \
>> +       .psr_reg        = 0x08, \
>> +       .icr1_reg       = 0x0c, \
>> +       .icr2_reg       = 0x10, \
>> +       .imr_reg        = 0x14, \
>> +       .isr_reg        = 0x18, \
>> +       .edge_sel_reg   = 0x1c, \
>> +       .low_level      = 0x00, \
>> +       .high_level     = 0x01, \
>> +       .rise_edge      = 0x02, \
>> +       .fall_edge      = 0x03
>
>Why?! Just to save a few lines? This makes it harder to follow the data part.

If I expand the macro the imx7d, imx8qm, imx8qxp hwdata, 24 lines will be
duplicated. I would keep as it is.

The current hwdata as below:
static struct mxc_gpio_hwdata imx7d_gpio_hwdata = {
        MXC_GPIO_HW_DATA_COMMON,
        .flags = MXC_GPIO_HAS_POWER_OFF,
};

static struct mxc_gpio_hwdata imx8qm_gpio_hwdata = {
        MXC_GPIO_HW_DATA_COMMON,
        .flags = MXC_GPIO_FALL_EDGE_WAKEUP_BROKEN | MXC_GPIO_HAS_PAD_WAKEUP,
};

static struct mxc_gpio_hwdata imx8qxp_gpio_hwdata = {
        MXC_GPIO_HW_DATA_COMMON,
        .flags = MXC_GPIO_HAS_PAD_WAKEUP,
};

Thanks
Peng
>
>-- 
>With Best Regards,
>Andy Shevchenko
>