[PATCH 0/7] staging: mt7621-gpio: cleanups to move this driver out of staging
NeilBrown
neil at brown.name
Sun May 27 23:09:29 UTC 2018
On Fri, May 25 2018, Sergio Paracuellos wrote:
> This patch series review all the probably missing stuff
> in order to get this driver out of staging..
>
> All of these are described in the following mail by NeilBrown:
>
> - https://www.mail-archive.com/driverdev-devel@linuxdriverproject.org/msg76202.html
>
> I don't move the driver yet because I think is better to
> review and test this before and if all is likely to be
> alright just move all this stuff afterwards.
>
> Hope this helps.
It certainly does - thanks.
All:
Reviewed-by: NeilBrown <neil at brown.name>
except... I'm wondering about
#interrupt-cells = <1>;
There really need to be 2 cells - one to identify the interrupt and one
to request a trigger (rising,falling,high,low).
I think I copied the <1> from a poor example. Most gpio chips have
#interrupt-cells = <2>;
Sorry about that.
Otherwise they look good - I compiled and tested and it gpios still work :-)
I went through the code again -- there is always something else. These
a really trivial though:
-------------
struct mtk_data {
void __iomem *gpio_membase;
int gpio_irq;
struct irq_domain *gpio_irq_domain;
struct mtk_gc *gc_map[MTK_BANK_CNT];
};
I don't think there is any gain in having gc_map be pointers. We may
as well just allocate all 3 at once.
so
- struct mtk_gc *gc_map[MTK_BANK_CNT];
+ struct mtk_gc gc_map[MTK_BANK_CNT];
and several consequent changes in the code.
-------------
static inline struct mtk_gc
*to_mediatek_gpio(struct gpio_chip *chip)
{
struct mtk_gc *mgc;
mgc = container_of(chip, struct mtk_gc, chip);
return mgc;
}
The '*' should be at the end of the first line, not the start of the
second. Also the body of the function can
return container_of(chip, struct mtk_gc, chip);
----------
return (t & BIT(offset)) ? 0 : 1;
I think this would read better as
return (t & BIT(offset)) ? GPIOF_DIR_OUT : GPIOF_DIR_IN;
Everything else looks perfect.
Thanks,
NeilBrown
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/attachments/20180528/417c0b2c/attachment.asc>
More information about the devel
mailing list