> > + struct bgpio_chip *bgc = to_bgpio_chip(gc); > > + struct dwapb_gpio_port *port = > > + container_of(bgc, struct dwapb_gpio_port, bgc); > > Does it make sense to introduce an inline helper like > > static inline struct dwapb_gpio_port *to_dwapb_gpio_port(struct bgpio_chip > *gc) {...} > > ? OK. > There is also another place where it could be used. > > > + struct dwapb_gpio *gpio = port->gpio; > > + unsigned long flags, val_deb; > > + unsigned long mask = bgc->pin2mask(bgc, offset); > > + > > + spin_lock_irqsave(&bgc->lock, flags); > > + > > + val_deb = dwapb_read(gpio, GPIO_PORTA_DEBOUNCE); > > + if (debounce) > > + dwapb_write(gpio, GPIO_PORTA_DEBOUNCE, mask | val_deb); > > May you put value on the first place? Like 'val_deb | mask'. OK. > > + else > > + dwapb_write(gpio, GPIO_PORTA_DEBOUNCE, ~mask & val_deb); > > Ditto. > OK. > > + ��.n��������+%������w��{.n�����{�� b���ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f